Re: [PATCH v2] i2c: i801: Remove i801_set_block_buffer_mode

[Jean Delvare <jdelvare@suse.de>]

On Thu, 18 Nov 2021 23:58:17 +0100, Heiner Kallweit wrote:
> If FEATURE_BLOCK_BUFFER is set then bit SMBAUXCTL_E32B is supported
> and there's no benefit in reading it back. Origin of this check
> seems to be 14 yrs ago when people were not completely sure which
> chip versions support the block buffer mode.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - made commit message less offending ;)
> - re-added parentheses to a & b && c expression
> ---
>  drivers/i2c/busses/i2c-i801.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 05187457f..f5c1589da 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -521,9 +521,11 @@ static int i801_block_transaction_by_block(struct i801_priv *priv,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/* Set block buffer mode */
> +	outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
> +
>  	inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
>  
> -	/* Use 32-byte buffer to process this transaction */
>  	if (read_write == I2C_SMBUS_WRITE) {
>  		len = data->block[0];
>  		outb_p(len, SMBHSTDAT0(priv));
> @@ -750,14 +752,6 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
>  	return i801_check_post(priv, status);
>  }
>  
> -static int i801_set_block_buffer_mode(struct i801_priv *priv)
> -{
> -	outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
> -	if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0)
> -		return -EIO;
> -	return 0;
> -}
> -
>  /* Block transaction function */
>  static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
>  				  char read_write, int command)
> @@ -791,9 +785,8 @@ static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *
>  	/* Experience has shown that the block buffer can only be used for
>  	   SMBus (not I2C) block transactions, even though the datasheet
>  	   doesn't mention this limitation. */
> -	if ((priv->features & FEATURE_BLOCK_BUFFER)
> -	 && command != I2C_SMBUS_I2C_BLOCK_DATA
> -	 && i801_set_block_buffer_mode(priv) == 0)
> +	if ((priv->features & FEATURE_BLOCK_BUFFER) &&
> +	    command != I2C_SMBUS_I2C_BLOCK_DATA)
>  		result = i801_block_transaction_by_block(priv, data,
>  							 read_write,
>  							 command);

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

-- 
Jean Delvare
SUSE L3 Support