| Message ID | 20210831202506.181927-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 235300ed8c6ccf8d3044f4ad25b1e984d5dd20d0 |
| Headers | show |
| Series | [v3,1/3] Input: ili210x - use resolution from ili251x firmware | expand |
On 8/31/21 10:25 PM, Marek Vasut wrote: > The ili251x firmware protocol permits readout of panel resolution, > implement this, but make it possible to override this value using > DT bindings. This way, older DTs which contain touchscreen-size-x > and touchscreen-size-y properties will behave just like before and > new DTs may avoid specifying these for ILI251x. > > Note that the command format is different on other controllers, so > this functionality is isolated to ILI251x. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: Joe Hung <joe_hung@ilitek.com> > Cc: Luca Hsu <luca_hsu@ilitek.com> > --- > V2: New patch > V3: - Use le16_to_cpup() byte-swap resolution data > - Check Y-resolution range up to 0xffff too > - Use .has_firmware_proto flag to discern supported MCU protocol > - Use input_abs_set_max() per include/linux/input.h INPUT_GENERATE_ABS_ACCESSORS > - Rename variable ret to error > - Add a wrapper function ili251x_firmware_update_cached_state(), > which would call pull other cacheable state from the controller > in subsequent patch (hence also the ret variable in it which > looks like it could be removed, this will reduce the number of > changes in the next patch). > - Wait for the firmware to fully stabilize itself after reset. > No, those 200 milliseconds is not a mistake, but a value taken > from example code. Anything less sometimes does report partly > invalid values. I hope I addressed all the feedback from V2 in this V3. I haven't heard anything about these patches for a long time. Do you think they can be applied now ?
Hi Marek, On Tue, Aug 31, 2021 at 10:25:04PM +0200, Marek Vasut wrote: > +static int ili251x_firmware_update_resolution(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct ili210x *priv = i2c_get_clientdata(client); > + __le16 resx, resy; These are normal variables, so I changed it to u16, otherwise sparse is rightfully unhappy. > + u8 rs[10]; > + int error; > + > + /* 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; > + > + 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) > + return -EINVAL; > + > + 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; > +} > + > +static int ili251x_firmware_update_cached_state(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct ili210x *priv = i2c_get_clientdata(client); > + int ret; Changed this to "error" and applied, thank you.
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c index 30576a5f2f04..baaddf95dd92 100644 --- a/drivers/input/touchscreen/ili210x.c +++ b/drivers/input/touchscreen/ili210x.c @@ -35,6 +35,7 @@ struct ili2xxx_chip { unsigned int max_touches; unsigned int resolution; bool has_calibrate_reg; + bool has_firmware_proto; bool has_pressure_reg; }; @@ -268,6 +269,7 @@ static const struct ili2xxx_chip ili251x_chip = { .continue_polling = ili251x_check_continue_polling, .max_touches = 10, .has_calibrate_reg = true, + .has_firmware_proto = true, .has_pressure_reg = true, }; @@ -323,6 +325,54 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data) return IRQ_HANDLED; } +static int ili251x_firmware_update_resolution(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct ili210x *priv = i2c_get_clientdata(client); + __le16 resx, resy; + u8 rs[10]; + int error; + + /* 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; + + 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) + return -EINVAL; + + 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; +} + +static int ili251x_firmware_update_cached_state(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct ili210x *priv = i2c_get_clientdata(client); + int ret; + + if (!priv->chip->has_firmware_proto) + return 0; + + /* Wait for firmware to boot and stabilize itself. */ + msleep(200); + + /* Firmware does report valid information. */ + ret = ili251x_firmware_update_resolution(dev); + if (ret) + return ret; + + return 0; +} + static ssize_t ili210x_calibrate(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -449,6 +499,12 @@ static int ili210x_i2c_probe(struct i2c_client *client, input_set_abs_params(input, ABS_MT_POSITION_Y, 0, max_xy, 0, 0); if (priv->chip->has_pressure_reg) input_set_abs_params(input, ABS_MT_PRESSURE, 0, 0xa, 0, 0); + error = ili251x_firmware_update_cached_state(dev); + if (error) { + dev_err(dev, "Unable to cache firmware information, err: %d\n", + error); + return error; + } touchscreen_parse_properties(input, true, &priv->prop); error = input_mt_init_slots(input, priv->chip->max_touches,
The ili251x firmware protocol permits readout of panel resolution, implement this, but make it possible to override this value using DT bindings. This way, older DTs which contain touchscreen-size-x and touchscreen-size-y properties will behave just like before and new DTs may avoid specifying these for ILI251x. Note that the command format is different on other controllers, so this functionality is isolated to ILI251x. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Joe Hung <joe_hung@ilitek.com> Cc: Luca Hsu <luca_hsu@ilitek.com> --- V2: New patch V3: - Use le16_to_cpup() byte-swap resolution data - Check Y-resolution range up to 0xffff too - Use .has_firmware_proto flag to discern supported MCU protocol - Use input_abs_set_max() per include/linux/input.h INPUT_GENERATE_ABS_ACCESSORS - Rename variable ret to error - Add a wrapper function ili251x_firmware_update_cached_state(), which would call pull other cacheable state from the controller in subsequent patch (hence also the ret variable in it which looks like it could be removed, this will reduce the number of changes in the next patch). - Wait for the firmware to fully stabilize itself after reset. No, those 200 milliseconds is not a mistake, but a value taken from example code. Anything less sometimes does report partly invalid values. --- drivers/input/touchscreen/ili210x.c | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)