Re: [PATCH] Input: cyttsp - Verbose error on soft reset

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

Hi Linus,

On Mon, Mar 22, 2021 at 11:13:49PM +0100, Linus Walleij wrote:
> The first thing the Cypress driver does when starting
> up is to try a soft reset. This is the first point where
> the driver SPI/I2C communication can fail, so put out some
> nice debug text:
> 
>   cyttsp-spi spi2.0: failed to send soft reset
> 
> Instead of just:
> 
>   cyttsp-spi: probe of spi2.0 failed with error -5
> 
> This is more helpful.
> 
> Cc: Ferruh Yigit <fery@cypress.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/input/touchscreen/cyttsp_core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> index 73c854f35f33..b9772192b5ea 100644
> --- a/drivers/input/touchscreen/cyttsp_core.c
> +++ b/drivers/input/touchscreen/cyttsp_core.c
> @@ -248,12 +248,16 @@ static int cyttsp_soft_reset(struct cyttsp *ts)
>  	enable_irq(ts->irq);
>  
>  	retval = ttsp_send_command(ts, CY_SOFT_RESET_MODE);
> -	if (retval)
> +	if (retval) {
> +		dev_err(ts->dev, "failed to send soft reset\n");
>  		goto out;
> +	}
>  
>  	timeout = wait_for_completion_timeout(&ts->bl_ready,
>  			msecs_to_jiffies(CY_DELAY_DFLT * CY_DELAY_MAX));
>  	retval = timeout ? 0 : -EIO;
> +	if (retval)
> +		dev_err(ts->dev, "timeout waiting for soft reset\n");

I think if we have conditional for the error message then having ternary
above does not make sense. I changed this to:

	if (!wait_for_completion_timeout(&ts->bl_ready,
			msecs_to_jiffies(CY_DELAY_DFLT * CY_DELAY_MAX))) {
		dev_err(ts->dev, "timeout waiting for soft reset\n");
		retval = -EIO;
	}

Thanks.

-- 
Dmitry