| Message ID | 20210219110556.1858969-1-m.tretter@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6524d8eac258452e547f8a49c8a965ac6dd8a161 |
| Headers | show |
| Series | Input: st1232 - add IDLE state as ready condition | expand |
On Fri, Feb 19, 2021 at 12:05:56PM +0100, Michael Tretter 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. > > Fixes: f605be6a57b4 ("Input: st1232 - wait until device is ready before reading resolution") > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Applied, thank you.
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
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c index b4e7bcbe9b91..885f0572488d 100644 --- 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)) return 0; usleep_range(1000, 2000);
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. Fixes: f605be6a57b4 ("Input: st1232 - wait until device is ready before reading resolution") Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> --- drivers/input/touchscreen/st1232.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)