Re: [PATCH 3/4] i2c: i801: Improve i801_block_transaction_byte_by_byte

[Jean Delvare <jdelvare@suse.de>]

Hi Heiner, Andi,

On Sat, 04 Mar 2023 22:36:34 +0100, Heiner Kallweit wrote:
> Here we don't have to write SMBHSTCNT in each iteration of the loop.
> Bit SMBHSTCNT_START is internally cleared immediately, therefore
> we don't have to touch the value of SMBHSTCNT until the last byte.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 7641bd0ac..e1350a8cc 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -677,11 +677,11 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
>  	for (i = 1; i <= len; i++) {
>  		if (i == len && read_write == I2C_SMBUS_READ)
>  			smbcmd |= SMBHSTCNT_LAST_BYTE;
> -		outb_p(smbcmd, SMBHSTCNT(priv));
>  
>  		if (i == 1)
> -			outb_p(inb(SMBHSTCNT(priv)) | SMBHSTCNT_START,
> -			       SMBHSTCNT(priv));
> +			outb_p(smbcmd | SMBHSTCNT_START, SMBHSTCNT(priv));
> +		else if (smbcmd & SMBHSTCNT_LAST_BYTE)
> +			outb_p(smbcmd, SMBHSTCNT(priv));
>  
>  		status = i801_wait_byte_done(priv);
>  		if (status)

I tested this and it works, but I don't understand how.

I thought that writing to SMBHSTCNT was what was telling the host
controller to proceed with the next byte. If writing to SMBHSTCNT for
each byte isn't needed, then what causes the next byte to be processed?
Does this happen as soon as SMBHSTSTS_BYTE_DONE is written? If so, then
what guarantees that we set SMBHSTCNT_LAST_BYTE *before* the last byte
is actually processed?

-- 
Jean Delvare
SUSE L3 Support