Re: [PATCHv2 5/5] input/rmi4/rmi_smbus.c: don't increment rmiaddr

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Sat, Nov 23, 2019 at 05:27:41PM +0100, Hans Verkuil wrote:
> This increment of rmi_smbus in rmi_smb_read/write_block() causes
> garbage to be read/written.
> 
> The first read of SMB_MAX_COUNT bytes is fine, but after that
> it is nonsense. Trial-and-error showed that by dropping the
> increment of rmiaddr everything is fine and the F54 function
> properly works.
> 
> I tried a hack with rmi_smb_write_block() as well (writing to the
> same F54 touchpad data area, then reading it back), and that
> suggests that there too the rmiaddr increment has to be dropped.
> It makes sense that if it has to be dropped for read, then it has
> to be dropped for write as well.
> 
> It looks like the initial work with F54 was done using i2c, not smbus,
> and it seems nobody ever tested F54 with smbus. The other functions
> all read/write less than SMB_MAX_COUNT as far as I can tell, so this
> issue was never noticed with non-F54 functions.
> 
> With this change I can read out the touchpad data correctly on my
> Lenovo X1 Carbon 6th Gen laptop.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Applied, thank you.

> ---
>  drivers/input/rmi4/rmi_smbus.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> index 2407ea43de59..b313c579914f 100644
> --- a/drivers/input/rmi4/rmi_smbus.c
> +++ b/drivers/input/rmi4/rmi_smbus.c
> @@ -163,7 +163,6 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
>  		/* prepare to write next block of bytes */
>  		cur_len -= SMB_MAX_COUNT;
>  		databuff += SMB_MAX_COUNT;
> -		rmiaddr += SMB_MAX_COUNT;
>  	}
>  exit:
>  	mutex_unlock(&rmi_smb->page_mutex);
> @@ -215,7 +214,6 @@ static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
>  		/* prepare to read next block of bytes */
>  		cur_len -= SMB_MAX_COUNT;
>  		databuff += SMB_MAX_COUNT;
> -		rmiaddr += SMB_MAX_COUNT;
>  	}
> 
>  	retval = 0;
> -- 
> 2.24.0
> 
> 

-- 
Dmitry