[v1,4/5] Input: edt-ft5x06 - do not try to allocate too much memory
diff mbox series

Message ID 20200303180917.12563-4-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • [v1,1/5] Input: of_touchscreen - explicitly choose axis
Related show

Commit Message

Andy Shevchenko March 3, 2020, 6:09 p.m. UTC
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(+)

Comments

Dmitry Torokhov March 7, 2020, 1:08 a.m. UTC | #1
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.
Andy Shevchenko May 11, 2020, 10:08 a.m. UTC | #2
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.

Patch
diff mbox series

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) {