| Message ID | 20200303180917.12563-4-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/5] Input: of_touchscreen - explicitly choose axis | expand |
On Tue, Mar 03, 2020 at 08:09:16PM +0200, Andy Shevchenko wrote: > When mode switch happens we try to allocate too much memory in case > when num_x and num_y are being assigned to their maximum. > > Since the resolution should come from property in such case, reassign > values back to num_x and num_y to prevent too much memory allocation. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/input/touchscreen/edt-ft5x06.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > index a05c6b597d43..1023d4134b8d 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -1178,6 +1178,13 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > touchscreen_parse_properties(input, true, &tsdata->prop); > > + if (tsdata->num_x == U16_MAX && tsdata->prop.max_x && > + tsdata->num_y == U16_MAX && tsdata->prop.max_y) { > + /* Reassign num_x and num_y from properties */ > + tsdata->num_x = tsdata->prop.max_x; > + tsdata->num_y = tsdata->prop.max_y; No. num_x and num_y reprsenet number of electrodes on a given axis and we should not be assigning maximum coordinates to them. Moreover, the factory mode can only be activated on M06, where we do read these values from registers, so we will not be allocating too much memory. If anything, we should add error handling for edt_ft5x06_register_read() when trying to fetch num_x and num_y. Thanks.
On Fri, Mar 06, 2020 at 05:08:37PM -0800, Dmitry Torokhov wrote: > On Tue, Mar 03, 2020 at 08:09:16PM +0200, Andy Shevchenko wrote: > > When mode switch happens we try to allocate too much memory in case > > when num_x and num_y are being assigned to their maximum. > > > > Since the resolution should come from property in such case, reassign > > values back to num_x and num_y to prevent too much memory allocation. > > + if (tsdata->num_x == U16_MAX && tsdata->prop.max_x && > > + tsdata->num_y == U16_MAX && tsdata->prop.max_y) { > > + /* Reassign num_x and num_y from properties */ > > + tsdata->num_x = tsdata->prop.max_x; > > + tsdata->num_y = tsdata->prop.max_y; > > No. num_x and num_y reprsenet number of electrodes on a given axis and > we should not be assigning maximum coordinates to them. Thank you for explanation. > Moreover, the factory mode can only be activated on M06, where we do > read these values from registers, so we will not be allocating too much > memory. If anything, we should add error handling for > edt_ft5x06_register_read() when trying to fetch num_x and num_y.
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index a05c6b597d43..1023d4134b8d 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -1178,6 +1178,13 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, touchscreen_parse_properties(input, true, &tsdata->prop); + if (tsdata->num_x == U16_MAX && tsdata->prop.max_x && + tsdata->num_y == U16_MAX && tsdata->prop.max_y) { + /* Reassign num_x and num_y from properties */ + tsdata->num_x = tsdata->prop.max_x; + tsdata->num_y = tsdata->prop.max_y; + } + error = input_mt_init_slots(input, tsdata->max_support_points, INPUT_MT_DIRECT); if (error) {
When mode switch happens we try to allocate too much memory in case when num_x and num_y are being assigned to their maximum. Since the resolution should come from property in such case, reassign values back to num_x and num_y to prevent too much memory allocation. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/input/touchscreen/edt-ft5x06.c | 7 +++++++ 1 file changed, 7 insertions(+)