Page MenuHomeFreeBSD

jedec_dimm(4): Refactor offset adjustment and page0 reset
ClosedPublic

Authored by rpokala on Apr 27 2023, 5:08 AM.
Tags
None
Referenced Files
F103337180: D39842.diff
Sat, Nov 23, 5:56 PM
Unknown Object (File)
Wed, Nov 20, 9:02 AM
Unknown Object (File)
Oct 2 2024, 5:47 PM
Unknown Object (File)
Sep 7 2024, 7:51 PM
Unknown Object (File)
Aug 25 2024, 11:58 PM
Unknown Object (File)
Jul 1 2024, 11:47 AM
Unknown Object (File)
Jun 29 2024, 9:13 AM
Unknown Object (File)
May 8 2024, 8:12 AM
Subscribers

Details

Summary

Offsets greater than 255 bytes reside on page1 of the SPD device.
Accessing them requires switching to page1, and adjusting the absolute
offset to be relative to the start of page1. After the access, the page
must be set back to page0. These operations are performed in several
places, so break them out into their own functions.

Also, replace a pair of default cases, which should be impossible due to
earlier checks, with __assert_unreachable().

MFC after: 1 week
Sponsored by: Panasas

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

seems reasonable, though there's a lot to test to make sure you didn't miss a spot where you'd need to restore page0.

sys/dev/jedec_dimm/jedec_dimm.c
239

I'd think you'd want a % operation here since it's easy and future proofs a bit better of page 2 is needed.

This revision is now accepted and ready to land.Apr 27 2023, 5:49 PM
In D39842#907259, @imp wrote:

seems reasonable, though there's a lot to test to make sure you didn't miss a spot where you'd need to restore page0.

Almost everything the driver does (other than reading the temperature) happens once during jedec_dimm_attach(). The only place that isn't hit during normal operation is jedec_dimm_dump(), and I tested that with sysctl debug.bootverbose=1 before loading the new jedec_dimm(4) driver. It printed out the full 512-byte register dump on DDR4.

sys/dev/jedec_dimm/jedec_dimm.c
239

Only DDR3 and DDR4 are supported by this driver at this time, and they only use page0 and page0/page1.

While it looks like DDR5 does in fact extend past page1, but there are quite a few other changes needed to support that, so I'd address this at that time.