On 2/21/23 22:14, Dmitry Torokhov wrote:
On Tue, Feb 21, 2023 at 09:12:29PM +0100, Marek Vasut wrote:
On 2/21/23 20:40, Dmitry Torokhov wrote:
Hi Marek,
On Fri, Feb 17, 2023 at 03:52:00AM +0100, Marek Vasut wrote:
Probe the touch controller driver even if resolution information is not
available. This can happen e.g. in case the touch controller suffered a
failed firmware update and is stuck in bootloader mode.
Signed-off-by: Marek Vasut <marex@xxxxxxx>
---
Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Cc: Joe Hung <joe_hung@xxxxxxxxxx>
Cc: Luca Hsu <luca_hsu@xxxxxxxxxx>
---
V2: Add dev_warn() in case resolution is invalid
---
drivers/input/touchscreen/ili210x.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 4897fafa4204d..d64b6d77d2e08 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -370,22 +370,33 @@ static int ili251x_firmware_update_resolution(struct device *dev)
/* The firmware update blob might have changed the resolution. */
error = priv->chip->read_reg(client, REG_PANEL_INFO, &rs, sizeof(rs));
- if (error)
- return error;
+ if (!error) {
+ resx = le16_to_cpup((__le16 *)rs);
+ resy = le16_to_cpup((__le16 *)(rs + 2));
- resx = le16_to_cpup((__le16 *)rs);
- resy = le16_to_cpup((__le16 *)(rs + 2));
+ /* The value reported by the firmware is invalid. */
+ if (!resx || resx == 0xffff || !resy || resy == 0xffff)
+ error = -EINVAL;
+ }
- /* The value reported by the firmware is invalid. */
- if (!resx || resx == 0xffff || !resy || resy == 0xffff)
- return -EINVAL;
+ /*
+ * In case of error, the firmware might be stuck in bootloader mode,
+ * e.g. after a failed firmware update. Set maximum resolution, but
+ * do not fail to probe, so the user can re-trigger the firmware
+ * update and recover the touch controller.
+ */
+ if (error) {
+ dev_warn(dev, "Invalid resolution reported by controller.\n");
+ resx = 16384;
+ resy = 16384;
+ }
input_abs_set_max(priv->input, ABS_X, resx - 1);
input_abs_set_max(priv->input, ABS_Y, resy - 1);
input_abs_set_max(priv->input, ABS_MT_POSITION_X, resx - 1);
input_abs_set_max(priv->input, ABS_MT_POSITION_Y, resy - 1);
- return 0;
+ return error;
I think this will make ili251x_firmware_update_cached_state() continue
failing when it reports invalid coordinates. Was this intended?
This is actually correct, ili251x_firmware_update_cached_state() will fail,
but ili210x_i2c_probe() won't stop there anymore, see the second hunk of
this patch. The driver will instantiate the controller, so user can load
correct firmware into it and recover the hardware.
I was concerned about call from ili210x_firmware_update_store() which
will continue returning error.
It hopefully won't, because at that point the controller would be
running new and working firmware, which would report the correct values.
If the firmware is broken, then yes, it would fail.
