litex.soc.doc generation error

gregdavill commented 3 years ago

I'm updating some older icebreaker litex examples to use more upstream LiteX features. One of which is LiteSPI The SoC works, as expected, but it appears I'm no longer able to generate documentation...

I've been able to replicate this in the litex_boards.1bitsquared_icebreaker target.

diff --git a/litex_boards/targets/1bitsquared_icebreaker.py b/litex_boards/targets/1bitsquared_icebreaker.py
index 0ab0434..a2c0daf 100755
--- a/litex_boards/targets/1bitsquared_icebreaker.py
+++ b/litex_boards/targets/1bitsquared_icebreaker.py
@@ -160,6 +160,9 @@ def main():
     builder = Builder(soc, **builder_argdict(args))
     builder.build(run=args.build)

+    from litex.soc.doc import generate_docs
+    generate_docs(soc, "build/docs/")
+
     if args.load:
         prog = soc.platform.create_programmer()
         prog.load_bitstream(os.path.join(builder.gateware_dir, soc.build_name + ".bin"))

Then running creates the following error.

$ ./1bitsquared_icebreaker.py

--- snip ---

Traceback (most recent call last):
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex-boards/litex_boards/targets/1bitsquared_icebreaker.py", line 174, in <module>
    main()
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex-boards/litex_boards/targets/1bitsquared_icebreaker.py", line 164, in main
    generate_docs(soc, "build/docs/")
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex/litex/soc/doc/__init__.py", line 99, in generate_docs
    documented_region = DocumentedCSRRegion(
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex/litex/soc/doc/csr.py", line 80, in __init__
    docs = module.get_module_documentation()
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex/litex/soc/integration/doc.py", line 141, in gatherer
    return sorted(r, key=lambda x: x.duid)
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/litex/litex/soc/integration/doc.py", line 141, in <lambda>
    return sorted(r, key=lambda x: x.duid)
  File "/home/greg/Projects/icebreaker-litex-examples/soc/deps/migen/migen/fhdl/module.py", line 136, in __getattr__
    raise AttributeError("'"+self.__class__.__name__+"' object has no attribute '"+name+"'")
AttributeError: 'LiteSPISDRPHYCore' object has no attribute 'duid'

mithro commented 3 years ago

@kgugala - Who added this code and thus should look at this issue?

gregdavill commented 3 years ago

I did try to dig into the issue. But all I can see is that LiteSPISDRPHYCore does inherit from ModuleDoc which inherits from DUID. So I'm not really sure why it's not getting a duid.

I don't really understand python inner workings enough to dig further.

If you add this it works:

diff --git a/litespi/phy/generic_sdr.py b/litespi/phy/generic_sdr.py
index a28d26f..ca714e8 100644
--- a/litespi/phy/generic_sdr.py
+++ b/litespi/phy/generic_sdr.py
@@ -60,6 +60,7 @@ class LiteSPISDRPHYCore(Module, AutoCSR, AutoDoc, ModuleDoc):
         Register which holds a clock divisor value applied to clkgen.
     """
     def __init__(self, pads, flash, device, clock_domain, default_divisor, cs_delay):
+        ModuleDoc.__init__(self)
         self.source           = source = stream.Endpoint(spi_phy2core_layout)
         self.sink             = sink   = stream.Endpoint(spi_core2phy_layout)
         self.cs               = Signal()

But that's against the docs for ModuleDoc which state

If you inherit from :obj:`ModuleDoc`, then there is no need to call ``__init__()``

It also seems like we're picking up some warnings, there is quite a few of these.

docs/spiflash_phy.rst:11: WARNING: duplicate label parameters, other instance in docs/spiflash_core.rst
docs/spiflash_phy.rst:34: WARNING: duplicate label attributes, other instance in docs/spiflash_core.rst
docs/spiflash_phy.rst:46: WARNING: duplicate label litespi phy instantiator, other instance in docs/spiflash_phy.rst

Disasm commented 3 years ago

Probably LiteSPISDRPHYCore should be somehow excluded from the parent one, otherwise litex generates docs both for the parent one and (twice) for the inner one. This would also solve the problem with this module.

Another question is why this hack is required (actually setting self.duid is enough). Probably it's a bug in an unrelated part of litex, because other modules with a similar structure don't reveal any problems.

enjoy-digital commented 3 years ago

BTW it's possible to test Doc generation with by adding --doc to the build command: python3 -m litex_boards.targets.1bitsquared_icebreaker --doc

gregdavill commented 3 years ago

Ahh great! Thanks!

Yes that's working much better now! The generated LiteSPI documentation isn't getting all the module docstrings, which is was before.

litex-hub / litespi

litex.soc.doc generation error #61