Re: [PATCH v3 04/12] i2c: wmt: Reduce redundant: REG_CR setting

[Wolfram Sang <wsa@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]

On Thu, Nov 02, 2023 at 10:53:54AM +0800, Hans Hu wrote:
> These Settings for the same register, REG_CR, 
> can be put together to reduce code redundancy.
> 
> Signed-off-by: Hans Hu <hanshu-oc@zhaoxin.com>

Some registers require that you first write a certain value, and only
then can write another value. Have you double checked with datasheets
that these register writes can safely be aggregated?

> ---
>  drivers/i2c/busses/i2c-wmt.c | 35 +++++++++--------------------------
>  1 file changed, 9 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-wmt.c b/drivers/i2c/busses/i2c-wmt.c
> index a44ce5fdde8a..d503e85752ea 100644
> --- a/drivers/i2c/busses/i2c-wmt.c
> +++ b/drivers/i2c/busses/i2c-wmt.c
> @@ -144,9 +144,6 @@ static int wmt_i2c_write(struct i2c_adapter *adap, struct i2c_msg *pmsg,
>  	if (!(pmsg->flags & I2C_M_NOSTART)) {
>  		val = readw(i2c_dev->base + REG_CR);
>  		val &= ~CR_TX_END;
> -		writew(val, i2c_dev->base + REG_CR);
> -
> -		val = readw(i2c_dev->base + REG_CR);
>  		val |= CR_CPU_RDY;
>  		writew(val, i2c_dev->base + REG_CR);
>  	}
> @@ -204,24 +201,15 @@ static int wmt_i2c_read(struct i2c_adapter *adap, struct i2c_msg *pmsg,
>  	u32 xfer_len = 0;
>  
>  	val = readw(i2c_dev->base + REG_CR);
> -	val &= ~CR_TX_END;
> -	writew(val, i2c_dev->base + REG_CR);
> +	val &= ~(CR_TX_END | CR_TX_NEXT_NO_ACK);
>  
> -	val = readw(i2c_dev->base + REG_CR);
> -	val &= ~CR_TX_NEXT_NO_ACK;
> -	writew(val, i2c_dev->base + REG_CR);
> -
> -	if (!(pmsg->flags & I2C_M_NOSTART)) {
> -		val = readw(i2c_dev->base + REG_CR);
> +	if (!(pmsg->flags & I2C_M_NOSTART))
>  		val |= CR_CPU_RDY;
> -		writew(val, i2c_dev->base + REG_CR);
> -	}
>  
> -	if (pmsg->len == 1) {
> -		val = readw(i2c_dev->base + REG_CR);
> +	if (pmsg->len == 1)
>  		val |= CR_TX_NEXT_NO_ACK;
> -		writew(val, i2c_dev->base + REG_CR);
> -	}
> +
> +	writew(val, i2c_dev->base + REG_CR);
>  
>  	reinit_completion(&i2c_dev->complete);
>  
> @@ -243,15 +231,10 @@ static int wmt_i2c_read(struct i2c_adapter *adap, struct i2c_msg *pmsg,
>  		pmsg->buf[xfer_len] = readw(i2c_dev->base + REG_CDR) >> 8;
>  		xfer_len++;
>  
> -		if (xfer_len == pmsg->len - 1) {
> -			val = readw(i2c_dev->base + REG_CR);
> -			val |= (CR_TX_NEXT_NO_ACK | CR_CPU_RDY);
> -			writew(val, i2c_dev->base + REG_CR);
> -		} else {
> -			val = readw(i2c_dev->base + REG_CR);
> -			val |= CR_CPU_RDY;
> -			writew(val, i2c_dev->base + REG_CR);
> -		}
> +		val = readw(i2c_dev->base + REG_CR) | CR_CPU_RDY;
> +		if (xfer_len == pmsg->len - 1)
> +			val |= CR_TX_NEXT_NO_ACK;
> +		writew(val, i2c_dev->base + REG_CR);
>  	}
>  
>  	return 0;
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]