Hi Jean, thanks for looking into this! On Wed, 2024-02-14 at 15:59 +0100, Jean Delvare wrote: > According to the Intel datasheets, software must reset the block > buffer index twice for block process call transactions: once before > writing the outgoing data to the buffer, and once again before > reading the incoming data from the buffer. > > The driver is currently missing the second reset, causing the wrong > portion of the block buffer to be read. > > Signed-off-by: Jean Delvare <jdelvare@xxxxxxx> > Reported-by: Piotr Zakowski <piotr.zakowski@xxxxxxxxx> > Closes: https://lore.kernel.org/linux-i2c/20240213120553.7b0ab120@endymion.delvare/ > Fixes: 315cd67c9453 ("i2c: i801: Add Block Write-Block Read Process Call support") According to all the datasheets I've been able to find, the patch makes sense to me. Reviewed-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx> > --- > Piotr, does this change make your tests succeed? > > Alexander, I don't suppose you still have access to the hardware on > which you were using block process call transactions back in 2019, but > maybe you remember having to do the same change to make it work? Nope, I cannot explain how this went unnoticed... I don't think we had a downstream patch on top back than. > drivers/i2c/busses/i2c-i801.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- linux-6.6.orig/drivers/i2c/busses/i2c-i801.c > +++ linux-6.6/drivers/i2c/busses/i2c-i801.c > @@ -500,11 +500,10 @@ static int i801_block_transaction_by_blo > /* Set block buffer mode */ > outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv)); > > - inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */ > - > if (read_write == I2C_SMBUS_WRITE) { > len = data->block[0]; > outb_p(len, SMBHSTDAT0(priv)); > + inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */ > for (i = 0; i < len; i++) > outb_p(data->block[i+1], SMBBLKDAT(priv)); > } > @@ -522,6 +521,7 @@ static int i801_block_transaction_by_blo > } > > data->block[0] = len; > + inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */ > for (i = 0; i < len; i++) > data->block[i + 1] = inb_p(SMBBLKDAT(priv)); > } > -- Alexander Sverdlin.
