| Message ID | 20210322221349.1116666-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ac1e4ca591c0c1369387e5155aac4071b9cdc1ca |
| Headers | show |
| Series | Input: cyttsp - Verbose error on soft reset | expand |
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.
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
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;
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(-)