Repository navigation
soc/interconnect/csr: skip class references, reject CSRs hidden in plain Modules; Converter to LiteXModule. - #2763
Merged
Merged
Conversation
… plain Modules. The AutoCSR/AutoDoc gatherers and CSRBankArray.scan duck-type every attribute through hasattr(v, "get_csrs"), which hides two silent failure modes: - an attribute holding a class (ex: Converter's self.cls = _DownConverter) has get_csrs as a plain function, so the gatherer called it without an instance and failed with a TypeError naming neither the module nor the attribute; - a Migen Module without AutoCSR holding CSRs was skipped: its CSRs never reached the CSR map and nothing reported it. Skip class references in the three gatherers and add csr.check_dropped_csrs(), used by the get_csrs gatherer and by CSRBankArray.scan, which raises an explicit TypeError naming the parent attribute, the CSR names and the fix (switch to LiteXModule, or autocsr_exclude). The check covers CSRs held directly by plain Modules (and by nested plain Modules); a plain Module only wrapping an AutoCSR child is not followed since attributes cannot tell owned submodules from references. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Resolves the FIXME: now that the gatherers skip class references, the self.cls attribute no longer needs an autocsr/autodoc exclude. Add a test gathering CSRs/Docs of a parent holding a Converter (scenario from #2757). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2757 (which this supersedes; credit to rohanmathew05 for the analysis there).
Problem
The AutoCSR/AutoDoc gatherers (
csr.py,doc.py) andCSRBankArray.scan(csr_bus.py) duck-type every attribute throughhasattr(v, "get_csrs"). Two silent failure modes follow:Converter.cls = _DownConverter) hasget_csrsas a plain function: the gatherer called it without an instance and failed withTypeError: gatherer() missing 1 required positional argument: 'self', naming neither the module nor the attribute. This is why the one-wordModule->LiteXModuleswitch ofConverterbroke video framebuffer SoCs in interconnect/stream: Switch Converter to LiteXModule #2757.Modulewithout AutoCSR holding CSRs was skipped: its CSRs never reached the CSR map and nothing reported it.Changes
inspect.isclass).csr.check_dropped_csrs(), used by theget_csrsgatherer and byCSRBankArray.scan: raises an explicitTypeErrorwhen a plain Module holds CSRs, e.g.DUT.plain (Plain) holds CSRs ['ctrl', 'status'] but Plain is a Migen Module without AutoCSR: these CSRs would be silently dropped. Switch Plain to LiteXModule (or add 'plain' to DUT.autocsr_exclude to ignore them).autocsr_excluderemains the explicit escape hatch.stream.Converterswitched toLiteXModule(FIXME resolved), no exclude needed.get_csrs, SoC-level scan, and a parent gathering CSRs/Docs with aConverter.Not detected (on purpose): a plain Module that only wraps an AutoCSR child with CSRs. Attribute scanning cannot tell owned submodules from references (
ethcore.macinsoc.py,CSRBankArray.source), so following them would create false positives and cycles. A static scan of LiteX + LiteDRAM/LiteEth/LitePCIe/LiteScope/LiteSDCard/LiteSPI/LiteICLink/LiteSATA/LiteI2C/LiteJESD204B/litex-boards found no plain Module creating CSRs exceptcsr_bus.SRAM, which has its ownget_csrs.Tested
pytest:test/interconnect,test/cores,test/tools,test/hdl,test/build,test/software,test/supportall pass.test/soc: the 15 failures (14 CPU boot tests missing pythondata packages/toolchains, serialboot) fail identically onmaster.--no-compile:litex_sim(VexRiscv, SDRAM, Ethernet, SD card, video framebuffer),digilent_arty,xilinx_kc705,xilinx_kcu105,efinix_ti60_f225_dev_kit.Written by Claude Code (AI agent), supervised by Florent Kermarrec.
🤖 Generated with Claude Code