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?
> }
>
> static ssize_t ili251x_firmware_update_firmware_version(struct device *dev)
> @@ -980,7 +991,6 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> if (error) {
> dev_err(dev, "Unable to cache firmware information, err: %d\n",
> error);
> - return error;
> }
> touchscreen_parse_properties(input, true, &priv->prop);
>
> --
> 2.39.1
>
Thanks.
--
Dmitry
