Input: cyttsp - Verbose error on soft reset

Message ID 20210322221349.1116666-1-linus.walleij@linaro.org
State New
Headers show
Series
  • Input: cyttsp - Verbose error on soft reset
Related show

Commit Message

Linus Walleij March 22, 2021, 10:13 p.m.
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(-)

-- 
2.29.2

Comments

Dmitry Torokhov March 23, 2021, 3:57 a.m. | #1
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
Linus Walleij March 24, 2021, 8:26 a.m. | #2
On Tue, Mar 23, 2021 at 4:57 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Mar 22, 2021 at 11:13:49PM +0100, Linus Walleij wrote:


> >       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:


Ah nice.

I'm starting to get this driver to work on a machine, so I hope I
will be able to make a few more serious improvements.

Yours,
Linus Walleij

Patch

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");
 
 out:
 	ts->state = CY_IDLE_STATE;