Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RACL] Add support for ranges and enable racl for sram_ctrl #26094

Merged
merged 3 commits into from
Feb 13, 2025
Merged

[RACL] Add support for ranges and enable racl for sram_ctrl #26094

merged 3 commits into from
Feb 13, 2025

Conversation

davidschrammel
Copy link
Contributor

@davidschrammel davidschrammel commented Feb 3, 2025 •
edited
Loading

This PR adds racl support for the ram tlul interface of sram_ctrl. (RACL for its regs interface was addded in #26047)
However, for this it was necessary to add so-called RACL ranges.
Due to this new type, the format of the racl mapping hjson files changes.
Furthermore, this PR also allows non-* mappings now (as mentioned in #25986).
I could not reasonably split the changes in raclgen/lib.py into separate commits. I hope this is still okay.

Note: This PR is based on #26096 and should not be merged before #26096.
I will rebase this PR and drop the first commit when that happens.

@davidschrammel davidschrammel requested review from Razer6, vogelpi and rswarbrick and removed request for msfschaffner February 3, 2025 12:08
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch 3 times, most recently from b4764e9 to a9c676b Compare February 3, 2025 12:21
Razer6
Razer6 reviewed Feb 3, 2025
View reviewed changes
Copy link
Member

@Razer6 Razer6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments after a quick skim

hw/top_darjeeling/data/racl/all_rd_wr_mapping.hjson Outdated Show resolved Hide resolved
hw/top_darjeeling/data/racl/soc_rot_mapping.hjson Show resolved Hide resolved
hw/top_darjeeling/rtl/autogen/top_racl_pkg.sv Outdated Show resolved Hide resolved
hw/ip/tlul/rtl/tlul_adapter_sram_racl.sv Outdated Show resolved Hide resolved
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch 12 times, most recently from a53c3ff to f2f911a Compare February 3, 2025 23:11
@davidschrammel davidschrammel marked this pull request as draft February 4, 2025 11:51
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch 5 times, most recently from a411754 to bef16f8 Compare February 4, 2025 15:15
@davidschrammel davidschrammel marked this pull request as ready for review February 4, 2025 16:51
nasahlpa
nasahlpa approved these changes Feb 5, 2025
View reviewed changes
hw/ip/tlul/rtl/tlul_adapter_sram_racl.sv Outdated Show resolved Hide resolved
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch 2 times, most recently from 66954c6 to ed775a6 Compare February 5, 2025 09:25
@davidschrammel davidschrammel mentioned this pull request Feb 6, 2025
Open
andreaskurth
andreaskurth reviewed Feb 6, 2025
View reviewed changes
util/raclgen/lib.py Outdated Show resolved Hide resolved
util/raclgen/lib.py Outdated Show resolved Hide resolved
util/raclgen/lib.py Outdated Show resolved Hide resolved
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch from ed775a6 to cc6da8f Compare February 6, 2025 14:09
andreaskurth
andreaskurth approved these changes Feb 6, 2025
View reviewed changes
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thx @davidschrammel!

@andreaskurth
Copy link
Contributor

Please rebase to resolve conflicts in util/topgen.py

@andreaskurth
Copy link
Contributor

Left a few more questions/suggestions, as I had apparently focused my previous review too much on code generation and not enough on regular RTL code changes -- apologies!

@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch 2 times, most recently from 44a70ee to ad12241 Compare February 8, 2025 00:45
@davidschrammel davidschrammel mentioned this pull request Feb 10, 2025
Open
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch from ad12241 to f1522e1 Compare February 11, 2025 16:03
@davidschrammel
Copy link
Contributor Author

@andreaskurth Is there anything else that needs to be changed before this is allowed to be merged?

@matutem
Copy link
Contributor

matutem commented Feb 12, 2025

I don't understand why spi_device and spi_host rtl got racl info added and no other IP did.

@davidschrammel
Copy link
Contributor Author

I don't understand why spi_device and spi_host rtl got racl info added and no other IP did.

With the addition of ranges, we changed the interface of tlul_adapter_sram_racl.
And spi_device and spi_host are currently the only ones that use tlul_adapter_sram_racl.
Hence no other IPs are changed at the moment.

@davidschrammel
[racl,raclgen] Unify raclgen template across earlgrey and darjeeling
ad3abc7 
Signed-off-by: David Schrammel <[email protected]>
@davidschrammel
[racl,raclgen,topgen] raclgen: Add support for ranges and non-* mappings
d380788 
Signed-off-by: David Schrammel <[email protected]>
@davidschrammel
[racl,rtl,sram_ctrl,spi_host,spi_device] Enable RACL ranges for sram_…
c0d9953 
…ctrl

And implement ranges for tlul_adapter_sram_racl
and rewire parameters where it is already being used.

Signed-off-by: David Schrammel <[email protected]>
@davidschrammel davidschrammel force-pushed the racl-support-sram_ctrl-2 branch from f1522e1 to c0d9953 Compare February 12, 2025 16:39
@davidschrammel
Copy link
Contributor Author

(Rebased to solve conflict in util/topgen/merge.py)

matutem
matutem approved these changes Feb 12, 2025
View reviewed changes
Copy link
Contributor

@matutem matutem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andreaskurth
Copy link
Contributor

CHANGE AUTHORIZED: hw/ip/spi_device/rtl/spi_device.sv
CHANGE AUTHORIZED: hw/ip/spi_host/rtl/spi_host_window.sv
CHANGE AUTHORIZED: hw/ip/sram_ctrl/data/sram_ctrl.hjson
CHANGE AUTHORIZED: hw/ip/sram_ctrl/rtl/sram_ctrl.sv
CHANGE AUTHORIZED: hw/ip/sram_ctrl/rtl/sram_ctrl_ram_reg_top.sv
CHANGE AUTHORIZED: hw/ip/tlul/rtl/tlul_adapter_sram_racl.sv

RACL ranges are getting added to those IPs. No functional change with default parameters (RACL disabled).

@nasahlpa
Copy link
Member

CHANGE AUTHORIZED: hw/ip/spi_device/rtl/spi_device.sv
CHANGE AUTHORIZED: hw/ip/spi_host/rtl/spi_host_window.sv
CHANGE AUTHORIZED: hw/ip/sram_ctrl/data/sram_ctrl.hjson
CHANGE AUTHORIZED: hw/ip/sram_ctrl/rtl/sram_ctrl.sv
CHANGE AUTHORIZED: hw/ip/sram_ctrl/rtl/sram_ctrl_ram_reg_top.sv
CHANGE AUTHORIZED: hw/ip/tlul/rtl/tlul_adapter_sram_racl.sv

No functional change as RACL is disabled when using the default parameters.

@andreaskurth andreaskurth merged commit 671369d into lowRISC:master Feb 13, 2025
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Razer6 Razer6 Razer6 left review comments

@andreaskurth andreaskurth andreaskurth approved these changes

@nasahlpa nasahlpa nasahlpa approved these changes

@matutem matutem matutem approved these changes

@rswarbrick rswarbrick Awaiting requested review from rswarbrick

@vogelpi vogelpi Awaiting requested review from vogelpi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@davidschrammel @andreaskurth @matutem @nasahlpa @Razer6