Re: [PATCH V3 2/2] input: egalax_ts: fix the get_firmware_command

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

Hi Haibo,

On Wed, Apr 15, 2020 at 04:01:03PM +0800, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> According to the User Guide, the get firmware command is
> { 0x03, 0x03, 0xa, 0x01, 'D' }, ASCII value of 'D' is 0x44.
> 
> This patch fix that.

You are absolutely right that 0x03 0x03 0x0a 0x01 0x44 is the proper
sequence for the "get firmware version" command, however, despite the function
name, we are not fetching firmware here, but rather try to check if
device operates normally via the "check active" command. So if anything
we should rename the function to egalax_check_active(). We should also
try reading the data sent back by the device and verify that it is what
we expect.

And if you indeed want to retrieve firmware version and controller type,
that should be separate functions.

> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/input/touchscreen/egalax_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
> index d3dc6d14bb78..1da6ddb9b4ee 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -171,7 +171,7 @@ static int egalax_wake_up_device(struct i2c_client *client)
>  
>  static int egalax_firmware_version(struct i2c_client *client)
>  {
> -	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x41 };
> +	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x44 };
>  	int ret;
>  
>  	ret = i2c_master_send(client, cmd, MAX_I2C_DATA_LEN);
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry