MJoergen / HyperRAM

Portable HyperRAM controller
MIT License
49 stars 11 forks source link

ISSI die rev D errata #2

Closed AnttiLukats closed 1 year ago

AnttiLukats commented 1 year ago

there is a errata (you need request the document from ISSI) for ISSI revision D dies, in short it says that all writes must be at least two clocks long, that is you need to write always 4 bytes (using RWDS to mask unused bytes).

Does this core support this errata? I a guessing the core does short writes and always would hit this ISSI errata?

MJoergen commented 1 year ago

Thank you for this information. I was not aware of this errata.

You are correct, that the current implementation will happily issue writes of just 1 clock cycle (i.e. 2 bytes), thus violating this errata.

As per my comment on the other issue (#1), I might actually prefer to leave this implementation agnostic of the errata, and instead create a separate module (with Avalon in and Avalon out), converting any 2-byte writes to 4-byte writes with appropriately cleared byte-enables. Such a module should be very simple to write, and this approach would therefore leave the HyperRAM Controller unchanged. Again, the idea is to avoid unnecessary complexity and "feature creep".

Let me know what you think about this approach.

AnttiLukats commented 1 year ago

You say should be very simple, could you do this? I can now really confirm that this ERRATA is real, I have a target system where I can swap rev B and rev D dies and I see that rev D fail with some tests where die B works without errors.

MJoergen commented 1 year ago

Adding here a link to the errata : https://www.issi.com/WW/pdf/appnotes/sram/AN66WX001_VariableMode_MinimumDataSize.pdf

MJoergen commented 1 year ago

Thinking more about this errata, I think the proposed solution in #1 is really the same as the work-around needed here. In other words, using the module https://github.com/MJoergen/Avalon/blob/main/avm_decrease.vhd creates a 32-bit (i.e. 4 byte) interface to the HyperRAM. This completely avoids the issue mentioned in the errata. Then no extra module is needed.

Would that work for your application ?

AnttiLukats commented 1 year ago

Yes I guess

Michael Jørgensen @.***> schrieb am Sa., 4. März 2023, 09:31:

Thinking more about this errata, I think the proposed solution in #1 https://github.com/MJoergen/HyperRAM/issues/1 is really the same as the work-around needed here. In other words, using the module https://github.com/MJoergen/Avalon/blob/main/avm_decrease.vhd creates a 32-bit (i.e. 4 byte) interface to the HyperRAM. This completely avoids the issue mentioned in the errata. Then no extra module is needed.

Would that work for your application ?

— Reply to this email directly, view it on GitHub https://github.com/MJoergen/HyperRAM/issues/2#issuecomment-1454664307, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJGOKY54BHZPVY6FAFD76DW2L4VBANCNFSM6AAAAAAVOL3X6U . You are receiving this because you authored the thread.Message ID: @.***>

sy2002 commented 1 year ago

@AnttiLukats Thank you for bringing this issue up. Are you by chance active in the MEGA65 Discord? In the #other-cores channel, we have some people who have problems with the C64 core's REU simulation. The REU simulation uses HyperRAM. The people who have problems are owners of the very newest batch of MEGA65. Owners of older batches (including me and @MJoergen) don't have any problems. Do you by chance know, if the very newest MEGA65's have a newer revision of HyperRAM than the older ones? Because then solving this very issue here in Michael's controller might solve the whole MEGA65 C64 core issue..

MJoergen commented 1 year ago

This issue is fixed in commit 483027b.

AnttiLukats commented 1 year ago

Thanks