Re: [PATCH] Input: st1232 - add IDLE state as ready condition

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Michael,

Thanks for your patch!

On Fri, Feb 19, 2021 at 12:06 PM Michael Tretter
<m.tretter@pengutronix.de> wrote:
> The st1232 can switch from NORMAL to IDLE state after the configured
> idle time (by default 8 s). If the st1232 is not reset during probe, it
> might already be ready but in IDLE state. Since it does not enter NORMAL
> state in this case, probe fails.
>
> Fix the wait function to report the IDLE state as ready, too.

Sorry for breaking your setup.  In fact I should have noticed, as I do
have a second board (KZM-A9-GT) with an st1232 that does not have the
reset line wired.  Unfortunately the display on that board is broken, so
I don't test display-related features on that board.  I've just verified
the touchscreen still works though, and does exhibit your problem.

> Fixes: f605be6a57b4 ("Input: st1232 - wait until device is ready before reading resolution")
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>

> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -94,7 +94,7 @@ static int st1232_ts_wait_ready(struct st1232_ts_data *ts)
>
>         for (retries = 10; retries; retries--) {
>                 error = st1232_ts_read_data(ts, REG_STATUS, 1);
> -               if (!error && ts->read_buf[0] == (STATUS_NORMAL | ERROR_NONE))
> +               if (!error && ts->read_buf[0] == (STATUS_NORMAL | STATUS_IDLE | ERROR_NONE))

NORMAL (0x0) and IDLE (0x4) are really two different states. Hence you
cannot check for both using a bitmask, as that checks for IDLE only,
breaking operation for devices that are in NORMAL state.
The retry period is also much shorter than the default 8s it takes for
the device to enter IDLE state, so probing fails on Armadillo-800-EVA.

As Dmitry has already applied your fix, I have sent a new fix on top
"[PATCH] Input: st1232 - Fix NORMAL vs. IDLE state handling"
https://lore.kernel.org/r/20210223090201.1430542-1-geert+renesas@glider.be

>                         return 0;
>
>                 usleep_range(1000, 2000);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds