i2c-imc-skylake: implement Receive Byte and Send Byte
The engine encodes an address-only transaction in command word bit 18, which is what SMBus Receive Byte and Send Byte need and what was missing for ee1004 to bind. The bit is not documented for the PCU function. It was located by structure rather than by sweeping: SMBCMD on the iMC channel function (datasheet ref. 614073 section 3.1.8) places PNTR_SEL immediately above WORD_ACCESS, and word access here is bit 17. Bit 17 had itself been unknown until it turned out to be word access, and an earlier sweep of this register covered bits 31:20 only, so 18 was untried rather than already ruled out. Validated against the device rather than against the register. An EEPROM auto-increments its pointer after a read, so priming the pointer and repeating the read with the bit set has a predicted answer: the next byte, not the one named. Eight consecutive reads returned SPD bytes 0x01 through 0x08 in order, matching an ordinary dump. Send Byte was confirmed on the DDR4 page-select addresses, which hold a latch and store no data, so the page state is itself the readout. Result: ee1004 binds on all four SPD addresses and decode-dimms decodes all four modules with both CRCs valid on each. The part number comes off page 1, so the page select runs end to end. The four module serial numbers are unchanged from the baseline recorded before this series. SMBus Quick stays unimplemented. Beyond having no obvious encoding, a write into the 0x30-0x37 range is how an EE1004 SPD is write-protected, permanently on many modules, so an adapter on the memory bus should not advertise a probe that writes. i2cdetect scans in read mode. Also fix test-smoke.sh resolving the module through modinfo, which silently tested the installed DKMS build instead of the one just compiled. It now prefers the local .ko and prints which it loaded. Smoke suite: 19 passed, 0 failed. checkpatch --strict 0/0/0, in-tree W=1 and Sparse clean.
This commit is contained in:
Simone
@@ -1,4 +1,4 @@
|
||||
Subject: [PATCH v4] i2c: imc-skylake: add Intel Skylake-X iMC SMBus adapter
|
||||
Subject: [PATCH v5] i2c: imc-skylake: add Intel Skylake-X iMC SMBus adapter
|
||||
|
||||
The integrated memory controller of Intel Skylake-X and Cascade Lake-X
|
||||
processors contains an SMBus engine wired to the DDR4 DIMMs. This driver
|
||||
@@ -10,7 +10,7 @@ v2 was posted as i2c-imc-x299 and received no replies:
|
||||
|
||||
https://lore.kernel.org/linux-i2c/[email protected]/
|
||||
|
||||
v3 was never posted.
|
||||
v3 and v4 were never posted.
|
||||
|
||||
Correction to v2
|
||||
~~~~~~~~~~~~~~~~
|
||||
@@ -104,16 +104,30 @@ implementation for the same engine, which predates this work:
|
||||
|
||||
https://github.com/namazso/PawnIO.Modules/blob/c975ca1d1b8a9c7473a175fa1ca24d83b6cbc8f3/SmbusIntelSkylakeIMC.p
|
||||
|
||||
It also implements SMBus BYTE through a pointer-select bit. That is not
|
||||
advertised here because it has not been validated on the Linux system, so
|
||||
i2cdetect cannot scan these buses and devices are addressed directly.
|
||||
It also implements SMBus BYTE through a pointer-select bit, without saying
|
||||
which bit. That bit is 18 here. The datasheet gave it away: SMBCMD on the
|
||||
iMC channel functions places PNTR_SEL immediately above WORD_ACCESS, and
|
||||
word access on the PCU function is bit 17.
|
||||
|
||||
That omission costs more than the scan. Both functionality combinations
|
||||
ee1004 accepts include I2C_FUNC_SMBUS_BYTE, so ee1004 does not bind to this
|
||||
adapter and decode-dimms does not work through it; SPD bytes are readable
|
||||
with i2cget, and jc42 binds fine where a sensor is populated. Implementing
|
||||
Receive Byte is the most useful thing still missing, and I would rather add
|
||||
it in a follow-up, validated, than advertise it now.
|
||||
Bit 18 is the only part of this driver that rests on an undocumented
|
||||
encoding, so it is worth saying exactly what was measured rather than
|
||||
asserting it works. With the bit set on a read, eight consecutive
|
||||
transfers to an SPD EEPROM returned bytes 0x01 through 0x08 in order,
|
||||
matching an ordinary dump of the same range. That is the auto-increment
|
||||
of a Receive Byte; a transfer that still named a register would have
|
||||
returned the same byte eight times. With the bit set on a write, a Send
|
||||
Byte to the DDR4 page-select addresses moves the 256-byte window: byte
|
||||
0x00 stops reading as the JEDEC 0x23 after selecting page 1 and returns
|
||||
to it after selecting page 0.
|
||||
|
||||
ee1004 consequently binds, and decode-dimms reads all four DIMMs on this
|
||||
system with both SPD CRCs valid on each. jc42 binds where a sensor is
|
||||
populated. i2cdetect scans in read mode.
|
||||
|
||||
SMBus Quick is still not implemented, and not only because the engine has
|
||||
no obvious encoding for it. A write to the 0x30-0x37 range is how an
|
||||
EE1004 SPD is write-protected, permanently on many modules. An adapter
|
||||
whose bus is the memory bus should not advertise a probe that writes.
|
||||
|
||||
Changes since v2
|
||||
~~~~~~~~~~~~~~~~
|
||||
@@ -139,7 +153,9 @@ Changes since v2
|
||||
- Drop the ACPI dependency.
|
||||
- Refuse to bind when the iMC reports TSOD polling enabled.
|
||||
- Correct the claim that BYTE_DATA and WORD_DATA cover ee1004: they do
|
||||
not, ee1004 also needs Receive Byte.
|
||||
not, ee1004 also needs Receive Byte and Send Byte.
|
||||
- Implement both, on the pointer-select bit, so ee1004 binds and
|
||||
decode-dimms works.
|
||||
|
||||
Validation
|
||||
~~~~~~~~~~
|
||||
@@ -147,12 +163,18 @@ Validation
|
||||
diagnostics.
|
||||
- checkpatch.pl --strict: clean.
|
||||
- Sparse with __CHECK_ENDIAN__: clean.
|
||||
- Read-only hardware test on one Intel Core i9-10900X (Cascade Lake-X)
|
||||
system with four DDR4 DIMMs: both adapters, four SPD EEPROMs, BYTE_DATA,
|
||||
WORD_DATA byte order, ENXIO for an absent device, 100 concurrent reads
|
||||
per channel, the default-off gate, and 20 unload/reload cycles: 15
|
||||
checks, 0 failures, no kernel warning or oops, no interference
|
||||
reported, every command word confirmed in the register.
|
||||
- Hardware test on one Intel Core i9-10900X (Cascade Lake-X) system with
|
||||
four DDR4 DIMMs: both adapters, four SPD EEPROMs, BYTE_DATA, WORD_DATA
|
||||
byte order, Receive Byte tracking the EEPROM pointer, Send Byte moving
|
||||
the SPD page and putting it back, ENXIO for an absent device, 100
|
||||
concurrent reads per channel, the default-off gate, and 20
|
||||
unload/reload cycles: 19 checks, 0 failures, no kernel warning or oops,
|
||||
no interference reported, every command word confirmed in the register.
|
||||
- ee1004 bound on all four SPD addresses and decode-dimms decoded all
|
||||
four modules, each with the CRC of bytes 0-125 and of bytes 128-253
|
||||
valid. The part number reads out of page 1, so the page select is
|
||||
exercised end to end. The four module serial numbers were identical
|
||||
before and after every experiment in this series.
|
||||
- Engine registers sampled every 2 s for one hour with the driver
|
||||
unloaded: 1791 samples, 0 changes. No firmware activity while the
|
||||
system is up.
|
||||
@@ -182,7 +204,7 @@ Validation
|
||||
after.
|
||||
|
||||
Still missing: an independent Tested-by, ideally on Skylake-X rather than
|
||||
Cascade Lake-X, and SMBus Receive Byte so that ee1004 binds.
|
||||
Cascade Lake-X.
|
||||
|
||||
One note on the register interface, since a reader with the datasheet will
|
||||
ask. That document describes an SMBus interface on the iMC channel
|
||||
|
||||
@@ -90,21 +90,34 @@ function and are reached with the ordinary ``pci_read_config_dword()`` and
|
||||
Supported transactions
|
||||
----------------------
|
||||
|
||||
The driver implements SMBus Read/Write Byte Data and Read/Write Word Data.
|
||||
The driver implements SMBus Receive Byte and Send Byte, Read/Write Byte Data
|
||||
and Read/Write Word Data.
|
||||
|
||||
That is enough for ``jc42`` (TSOD), which asks for exactly those two. It is
|
||||
**not** enough for ``ee1004``: both of its accepted combinations include
|
||||
``I2C_FUNC_SMBUS_BYTE``, which this adapter does not implement, so the driver
|
||||
probe fails with ``-EPFNOSUPPORT``. SPD bytes are still readable directly with
|
||||
``i2cget``. Implementing SMBus Receive Byte would make ``ee1004`` and
|
||||
``decode-dimms`` work; it is the most useful thing still missing.
|
||||
That covers ``jc42`` (TSOD) and ``ee1004``, so ``decode-dimms`` reads the whole
|
||||
512-byte DDR4 SPD, including the manufacturer and part-number fields that live
|
||||
on page 1.
|
||||
|
||||
SMBus Quick and Receive Byte are not implemented, so ``i2cdetect`` cannot scan
|
||||
these buses. Address a device directly instead, for example SPD byte 2 of the
|
||||
EEPROM at 0x50::
|
||||
Receive Byte and Send Byte need a command word that puts no register byte on
|
||||
the bus. The engine encodes that in bit 18, which is not documented for this
|
||||
function. It was found by noting that the interface Intel does document for
|
||||
the iMC channel functions (SMBCMD, datasheet reference 614073, section 3.1.8)
|
||||
places ``PNTR_SEL`` immediately above ``WORD_ACCESS``, and that word access
|
||||
here is bit 17. With the bit set, consecutive reads of an SPD EEPROM return
|
||||
consecutive bytes, which is the auto-increment behaviour of a Receive Byte and
|
||||
cannot be produced by a transfer that names a register.
|
||||
|
||||
$ i2cget -y 6 0x50 0x02
|
||||
0x0c
|
||||
SMBus Quick is not implemented. ``i2cdetect`` therefore has to be run in
|
||||
read mode::
|
||||
|
||||
$ i2cdetect -y -r 6
|
||||
0 1 2 3 4 5 6 7 8 9 a b c d e f
|
||||
20: -- -- -- -- -- -- -- 27 -- -- -- -- -- -- -- --
|
||||
30: 30 31 -- -- 34 35 36 -- -- -- -- -- -- -- -- --
|
||||
50: 50 -- 52 -- -- -- -- -- -- -- -- -- -- -- -- --
|
||||
|
||||
Not advertising Quick is deliberate as well as accurate. A write to the
|
||||
0x30-0x37 range is how an EE1004 SPD is write-protected, permanently on many
|
||||
modules, so an adapter on a memory bus should not offer probing that writes.
|
||||
|
||||
Block transfers are not implemented. The engine has block primitives, but no
|
||||
device on this bus needs them.
|
||||
@@ -126,8 +139,18 @@ description of these buses and the driver does not scan.
|
||||
Whether one is populated is visible in ``SMBCNTL.TSOD_PRESENT``; on the
|
||||
development system that mask is empty and no ``jc42`` binds.
|
||||
|
||||
``ee1004`` will not bind for the reason given above. Read SPD bytes directly
|
||||
instead.
|
||||
``ee1004`` binds on each populated SPD address::
|
||||
|
||||
# echo ee1004 0x50 > /sys/bus/i2c/devices/i2c-6/new_device
|
||||
ee1004 6-0050: 512 byte EE1004-compliant SPD EEPROM, read-only
|
||||
|
||||
Note that a DDR4 SPD is a 512-byte array reached through a 256-byte window, and
|
||||
which half is visible is a property of the bus, not of the transfer. ``ee1004``
|
||||
selects a page when it needs one and leaves it selected, so a raw ``i2cget``
|
||||
issued afterwards may be looking at page 1. Select page 0 explicitly before
|
||||
reading SPD by hand::
|
||||
|
||||
# i2cset -y 6 0x36 0x00 c
|
||||
|
||||
Module parameters
|
||||
-----------------
|
||||
|
||||
Reference in New Issue
Block a user