efabless / caravel_mgmt_soc_litex

https://caravel-mgmt-soc-litex.readthedocs.io/en/latest/
Apache License 2.0
24 stars 14 forks source link

mprj wishbone address range is limited to 0x3010_0000 instead of 0x3fff_ffff #21

Closed mattvenn closed 1 year ago

mattvenn commented 2 years ago

https://github.com/efabless/caravel_mgmt_soc_litex/blob/3fee299f8177cebf7919eb0e6da7f0f5ad7af31d/litex/caravel.py#L186

mattvenn commented 2 years ago

@suppamax said:

(Here)[https://caravel-harness.readthedocs.io/en/latest/memory-mapped-io-summary.html] a wishbone base address is mentioned: should I add a "wishbone last addr" field?

I cannot find a wishbone-related documentation in the management SoC repo (and it looks like there is no readthedocs page yet).

Those docs are out of date from when caravel contained the cpu as well. Now this repo contains the litex management soc containing the cpu so this repo should generate the docs. You can see them in https://github.com/efabless/caravel_mgmt_soc_litex/tree/main/docs/generated

so far they don't have a readthedocs (also needs to get done).

RE your question, I'd say yes, either an end address or a width would work.

suppamax commented 2 years ago

Do I understand it correctly - the idea is to

  1. extend that outdated document with the warning (size or end addr)
  2. consolidate the documentation for caravel_mgmt_soc_litex
mattvenn commented 2 years ago

I think leave the docs stuff for me. The idea is we will have at least 2 repos providing docs for caravel:

Make sense?

egorxe commented 2 years ago

May be it would be better to extend this address space for future MPWs in addition to documenting it? 1 MByte may be not enough for some projects (SDRAM/HyperRAM controllers for example) and there seems to be no reason why it could not be extended up to previous size (256 MBytes). Currently this address space is empty anyway.

clp510 commented 2 years ago

is there any update on this bug ?

suppamax commented 2 years ago

@clp510 the last information I got are here The code should have been fixed, there is a PR for a dedicated test, I don't know about the documentation.

jeffdi commented 1 year ago

duplicates issue #10