Skip to content

Conversation

@mmicko
Copy link
Contributor

@mmicko mmicko commented Jul 2, 2025

This one requires to be applied as well

enjoy-digital/litedram#366

enjoy-digital/litex#2274

Subsignal("ras_n", Pins("IO_SB_B5")),
Subsignal("cas_n", Pins("IO_SB_B3")),
Subsignal("we_n", Pins("IO_SB_A5")),
Subsignal("dq", Pins(
Copy link
Contributor

Choose a reason for hiding this comment

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

please align Pins

Comment on lines 49 to 51
Subsignal("r", Pins("IO_NA_B1 IO_NA_A1 IO_NA_A2 IO_NA_A4")),
Subsignal("g", Pins("IO_NA_B4 IO_NA_A7 IO_NA_B7 IO_NA_B2")),
Subsignal("b", Pins("IO_NA_A3 IO_NA_A5 IO_NA_A6 IO_NA_A8")),
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 70 to 71
Subsignal("left", Pins("IO_NB_B0")),
Subsignal("right", Pins("IO_NB_A0")),
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 33 to 36
self.rst = Signal()
rst_n = Signal()
self.cd_sys = ClockDomain()
self.cd_sys_ps = ClockDomain()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please align =

Comment on lines 94 to 96
sdrphy_cls = GENSDRPHY

self.sdrphy = sdrphy_cls(platform.request("sdram"), sys_clk_freq)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not directly using GENSDRPHY ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this, was leftover from experimenting. Also aligned Pins and assignments.

@trabucayre trabucayre merged commit 42d4615 into litex-hub:master Jul 2, 2025
1 of 2 checks passed
@trabucayre
Copy link
Contributor

Applied. Thanks @mmicko !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants