Re: [PATCH] touchscreen: elants: fix a missing check of return values

[Dmitry Torokhov]

Hi Kangjie,

On Fri, Dec 21, 2018 at 12:59:16AM -0600, Kangjie Lu wrote:
> elants_i2c_send() may fail, let's check its return values. The fix does
> the check and reports an error message upon the failure.
> 
> Signed-off-by: Kangjie Lu <kjlu@xxxxxxx>
> ---
>  drivers/input/touchscreen/elants_i2c.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index f2cb23121833..cb3c1470bb68 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -245,8 +245,14 @@ static int elants_i2c_calibrate(struct elants_data *ts)
>  	ts->state = ELAN_WAIT_RECALIBRATION;
>  	reinit_completion(&ts->cmd_done);
>  
> -	elants_i2c_send(client, w_flashkey, sizeof(w_flashkey));
> -	elants_i2c_send(client, rek, sizeof(rek));
> +	error = elants_i2c_send(client, w_flashkey, sizeof(w_flashkey));
> +	error |= elants_i2c_send(client, rek, sizeof(rek));

I dislike this kind of error handling as this may result in invalid
error code being reported, in case 2 commands produce different results.

> +	if (error) {
> +		dev_err(&client->dev,
> +				"error in sending I2C messages for calibration: %d\n",
> +				error);
> +		return error;

If we just return like you do it here, interrupts will stay disabled and
touchscreen will be completely dead. With the old code we'd report
timeout on calibration, and touchscreen would have chance of working. We
would also be able to retry calibration.

> +	}
>  
>  	enable_irq(client->irq);
>  
> -- 
> 2.17.2 (Apple Git-113)
> 

Thanks.

-- 
Dmitry