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

[Heiner Kallweit <hkallweit1@gmail.com>]

On 27.06.2023 15:46, Jean Delvare wrote:
> 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?
> 
It's my understanding that writing SMBHSTSTS_BYTE_DONE tells the host to
continue with the next byte.
We set SMBHSTCNT_LAST_BYTE whilst the host is receiving the last byte.
Apparently the host checks for SMBHSTCNT_LAST_BYTE once it received
a byte, in order to determine whether to ack the byte or not.
So SMBHSTCNT_LAST_BYTE doesn't have to be set before the host starts
receiving the last byte.
For writes SMBHSTCNT_LAST_BYTE isn't used.