Re: [PATCH] i2c: i801: Fix handling SMBHSTCNT_PEC_EN

[Jean Delvare <jdelvare@suse.de>]

On Wed, 21 Jul 2021 14:46:20 +0200, Jean Delvare wrote:
> As for testing, I also don't have a PEC-cable device at hand. However,
> we may still be able to test this change:
> * If you have a device at 0x69 on the i801 SMBus of any of your system,
>   that would be a clock device, which almost always support PEC.
> * If you have EEPROMs on your i801 SMBus, you may be lucky and find a
>   sequence of bytes where the PEC computation leads to exactly the
>   value of the following byte. I remember doing that years ago, sadly I
>   can no longer find the script I wrote at that time. Be careful when
>   accessing SPD EEPROMs, you want to read from them, not write to them
>   ;-) Incidentally i2c-tools was just improved to allow arbitrary SMBus
>   block read commands so i2cget can be used for easy testing from
>   user-space.

Well, what I wrote above wasn't accurate (bad memory I suppose). While
SMBus Block Read commands are OK to test the clock devices at 0x69,
they are not the best choice to poke a read-only EEPROM, as the first
byte will be interpreted as the block length, and if it is not between
1 and 32, it is invalid and the transaction will fail, regardless of
PEC. Which in turn dramatically decreases the chances that at least one
offset in your EEPROM will work and be usable for testing purposes.

Furthermore, i2cget has a safety to prevent you from messing up with
your SPD EEPROMs, that will deny using PEC at all in the I2C address
range 0x50-0x57. Which is exactly what I was suggesting to do. So I had
to recompile i2cget without the safety in order to preform my tests. To
be honest I think the safety is overkill (as far as I can see PEC would
only trash data in "c" mode so we could limit the safety to that mode)
but my testing being clearly a protocol abuse, I'm fine with having to
modify the source code to do it.

Anyway, for the record, my hackish testing protocol is as follows:

# rmmod at24
# modprobe i2c-dev
# for i in `seq 0 254` ; do echo $i ; ./tools/i2cget -y 4 0x50 $i bp ; sleep 1 ; done

Then I basically look at commands failing (on PEC error), until I am
lucky enough that the next byte in the EEPROM matches the expected PEC
value. I had 2 such offsets on my first SPD EEPROM (82 and 163).
Meaning that I was able to test your patch and I can confirm that it
works OK (testing limited to the 8 Series/C220 Series [8086:8c22]
device and SMBus Read Byte transactions, but I have no reason to
believe other devices or other transaction types would behave
differently).

Tested-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support