All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: i801: Fix block process call transactions
@ 2024-02-14 14:59 Jean Delvare
  2024-02-14 18:24 ` Alexander Sverdlin
  2024-02-14 21:44 ` [SPAM] " Andi Shyti
  0 siblings, 2 replies; 5+ messages in thread
From: Jean Delvare @ 2024-02-14 14:59 UTC (permalink / raw
  To: Linux I2C
  Cc: Piotr Zakowski, Alexander Sverdlin, andi.shyti,
	"Shepon, Oren\"   <oren.shepon, Kozlowski, Pawel,
	" <pawel.kozlowski, Usyskin, Alexander,
	" <alexander.usyskin, Radtke, Jakub"

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@suse.de>
Reported-by: Piotr Zakowski <piotr.zakowski@intel.com>
Closes: https://lore.kernel.org/linux-i2c/20240213120553.7b0ab120@endymion.delvare/
Fixes: 315cd67c9453 ("i2c: i801: Add Block Write-Block Read Process Call support")
---
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?

 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));
 	}


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i2c: i801: Fix block process call transactions
  2024-02-14 14:59 [PATCH] i2c: i801: Fix block process call transactions Jean Delvare
@ 2024-02-14 18:24 ` Alexander Sverdlin
  2024-02-14 21:44 ` [SPAM] " Andi Shyti
  1 sibling, 0 replies; 5+ messages in thread
From: Alexander Sverdlin @ 2024-02-14 18:24 UTC (permalink / raw
  To: Jean Delvare, Linux I2C
  Cc: andi.shyti@kernel.org, linux-i2c@vger.kernel.org, Shepon, Oren,
	Kozlowski, Pawel, Usyskin, Alexander, Radtke, Jakub

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@suse.de>
> Reported-by: Piotr Zakowski <piotr.zakowski@intel.com>
> 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@gmail.com>

> ---
> 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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [SPAM] [PATCH] i2c: i801: Fix block process call transactions
  2024-02-14 14:59 [PATCH] i2c: i801: Fix block process call transactions Jean Delvare
  2024-02-14 18:24 ` Alexander Sverdlin
@ 2024-02-14 21:44 ` Andi Shyti
  2024-02-14 21:47   ` Andi Shyti
  2024-02-15 10:19   ` Jean Delvare
  1 sibling, 2 replies; 5+ messages in thread
From: Andi Shyti @ 2024-02-14 21:44 UTC (permalink / raw
  To: Jean Delvare
  Cc: Linux I2C, Piotr Zakowski, Alexander Sverdlin, Shepon, Kozlowski,
	Pawel, Usyskin, Alexander, Radtke, Jakub"

Hi Jean,

On Wed, Feb 14, 2024 at 03:59:39PM +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@suse.de>
> Reported-by: Piotr Zakowski <piotr.zakowski@intel.com>
> Closes: https://lore.kernel.org/linux-i2c/20240213120553.7b0ab120@endymion.delvare/
> Fixes: 315cd67c9453 ("i2c: i801: Add Block Write-Block Read Process Call support")
> ---

applied to i2c/i2c-host-fixes.

b4 ty failed to send the thank you letter because there are some
addresses that were not accepted
(Kozlowski@imap1.dmz-prg2.suse.org?)

Anyway, it's applied :-)

Andi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [SPAM] [PATCH] i2c: i801: Fix block process call transactions
  2024-02-14 21:44 ` [SPAM] " Andi Shyti
@ 2024-02-14 21:47   ` Andi Shyti
  2024-02-15 10:19   ` Jean Delvare
  1 sibling, 0 replies; 5+ messages in thread
From: Andi Shyti @ 2024-02-14 21:47 UTC (permalink / raw
  To: Jean Delvare
  Cc: Linux I2C, Piotr Zakowski, Alexander Sverdlin, Shepon, Kozlowski,
	Pawel, Usyskin, Alexander, Radtke, Jakub"

On Wed, Feb 14, 2024 at 10:44:17PM +0100, Andi Shyti wrote:
> On Wed, Feb 14, 2024 at 03:59:39PM +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@suse.de>
> > Reported-by: Piotr Zakowski <piotr.zakowski@intel.com>
> > Closes: https://lore.kernel.org/linux-i2c/20240213120553.7b0ab120@endymion.delvare/
> > Fixes: 315cd67c9453 ("i2c: i801: Add Block Write-Block Read Process Call support")
> > ---
> 
> applied to i2c/i2c-host-fixes.
> 
> b4 ty failed to send the thank you letter because there are some
> addresses that were not accepted
> (Kozlowski@imap1.dmz-prg2.suse.org?)
> 
> Anyway, it's applied :-)

BTW, thanks for the fast action on this bug report!

Andi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [SPAM] [PATCH] i2c: i801: Fix block process call transactions
  2024-02-14 21:44 ` [SPAM] " Andi Shyti
  2024-02-14 21:47   ` Andi Shyti
@ 2024-02-15 10:19   ` Jean Delvare
  1 sibling, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2024-02-15 10:19 UTC (permalink / raw
  To: Andi Shyti; +Cc: Linux I2C, Piotr Zakowski, Alexander Sverdlin

Hi Andi,

On Wed, 14 Feb 2024 22:44:12 +0100, Andi Shyti wrote:
> b4 ty failed to send the thank you letter because there are some
> addresses that were not accepted
> (Kozlowski@imap1.dmz-prg2.suse.org?)

Sorry for the mess, this seems to be a bug in my email client (Claws
Mail 4.1.1) when the recipient names are quoted and/or include a comma.
Ideally my SMTP server should have rejected the message as invalid, but
it tried its best to still process it.

I'll report the issue to the Claws Mail developers.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-02-15 10:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-14 14:59 [PATCH] i2c: i801: Fix block process call transactions Jean Delvare
2024-02-14 18:24 ` Alexander Sverdlin
2024-02-14 21:44 ` [SPAM] " Andi Shyti
2024-02-14 21:47   ` Andi Shyti
2024-02-15 10:19   ` Jean Delvare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.