[PATCH v1 1/5] Input: of_touchscreen - explicitly choose axis

Subsystems: input (keyboard, mouse, joystick, touchscreen) drivers, the rest

STALE2333d LANDED

Landed in mainline as cc5117d6e498 on 2020-03-24.

11 messages, 2 authors, 2020-05-11 · open the first message on its own page

[PATCH v1 1/5] Input: of_touchscreen - explicitly choose axis

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-03-03 18:09:23

The 'axis + 1' calculation is implicit and potentially error prone.
Moreover, few lines before the axis is set explicitly for both X and Y.

Do the same when retrieving different properties for X and Y.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/of_touchscreen.c | 35 +++++++++++-----------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index e16ec4c7043a..97342e14b4f1 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c

@@ -66,7 +66,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 {
 	struct device *dev = input->dev.parent;
 	struct input_absinfo *absinfo;
-	unsigned int axis;
+	unsigned int axis, axis_x, axis_y;
 	unsigned int minimum, maximum, fuzz;
 	bool data_present;

@@ -74,33 +74,34 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	if (!input->absinfo)
 		return;
 
-	axis = multitouch ? ABS_MT_POSITION_X : ABS_X;
+	axis_x = multitouch ? ABS_MT_POSITION_X : ABS_X;
+	axis_y = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
+
 	data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
-						input_abs_get_min(input, axis),
+						input_abs_get_min(input, axis_x),
 						&minimum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-size-x",
 						input_abs_get_max(input,
-								  axis) + 1,
+								  axis_x) + 1,
 						&maximum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x",
-						input_abs_get_fuzz(input, axis),
+						input_abs_get_fuzz(input, axis_x),
 						&fuzz);
 	if (data_present)
-		touchscreen_set_params(input, axis, minimum, maximum - 1, fuzz);
+		touchscreen_set_params(input, axis_x, minimum, maximum - 1, fuzz);
 
-	axis = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
 	data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-y",
-						input_abs_get_min(input, axis),
+						input_abs_get_min(input, axis_y),
 						&minimum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-size-y",
 						input_abs_get_max(input,
-								  axis) + 1,
+								  axis_y) + 1,
 						&maximum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y",
-						input_abs_get_fuzz(input, axis),
+						input_abs_get_fuzz(input, axis_y),
 						&fuzz);
 	if (data_present)
-		touchscreen_set_params(input, axis, minimum, maximum - 1, fuzz);
+		touchscreen_set_params(input, axis_y, minimum, maximum - 1, fuzz);
 
 	axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE;
 	data_present = touchscreen_get_prop_u32(dev,

@@ -117,15 +118,13 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	if (!prop)
 		return;
 
-	axis = multitouch ? ABS_MT_POSITION_X : ABS_X;
-
-	prop->max_x = input_abs_get_max(input, axis);
-	prop->max_y = input_abs_get_max(input, axis + 1);
+	prop->max_x = input_abs_get_max(input, axis_x);
+	prop->max_y = input_abs_get_max(input, axis_y);
 
 	prop->invert_x =
 		device_property_read_bool(dev, "touchscreen-inverted-x");
 	if (prop->invert_x) {
-		absinfo = &input->absinfo[axis];
+		absinfo = &input->absinfo[axis_x];
 		absinfo->maximum -= absinfo->minimum;
 		absinfo->minimum = 0;
 	}

@@ -133,7 +132,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	prop->invert_y =
 		device_property_read_bool(dev, "touchscreen-inverted-y");
 	if (prop->invert_y) {
-		absinfo = &input->absinfo[axis + 1];
+		absinfo = &input->absinfo[axis_y];
 		absinfo->maximum -= absinfo->minimum;
 		absinfo->minimum = 0;
 	}

@@ -141,7 +140,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	prop->swap_x_y =
 		device_property_read_bool(dev, "touchscreen-swapped-x-y");
 	if (prop->swap_x_y)
-		swap(input->absinfo[axis], input->absinfo[axis + 1]);
+		swap(input->absinfo[axis_x], input->absinfo[axis_y]);
 }
 EXPORT_SYMBOL(touchscreen_parse_properties);

-- 
2.25.1

[PATCH v1 3/5] Input: edt-ft5x06 - use U16_MAX instead of -1

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-03-03 18:09:22

Explicitly show what the value we supply for the touchscreen resolution
when it can't be detected. -1 is hard to compare with when unsigned short
type is in use. The change will help to avoid signed vs. unsigned error
prone comparisons.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index cb67104c6934..a05c6b597d43 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -23,6 +23,7 @@
 #include <linux/input/touchscreen.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/ratelimit.h>
 #include <linux/regulator/consumer.h>

@@ -983,8 +984,8 @@ edt_ft5x06_ts_get_parameters(struct edt_ft5x06_ts_data *tsdata)
 		tsdata->num_y = edt_ft5x06_register_read(tsdata,
 							 reg_addr->reg_num_y);
 	} else {
-		tsdata->num_x = -1;
-		tsdata->num_y = -1;
+		tsdata->num_x = U16_MAX;
+		tsdata->num_y = U16_MAX;
 	}
 }

-- 
2.25.1

[PATCH v1 4/5] Input: edt-ft5x06 - do not try to allocate too much memory

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-03-03 18:09:23

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;
+	}
+
 	error = input_mt_init_slots(input, tsdata->max_support_points,
 				INPUT_MT_DIRECT);
 	if (error) {

-- 
2.25.1

[PATCH v1 5/5] Input: edt-ft5x06 - allocate buffer once for debugging

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-03-03 18:09:23

There is no need to allocate buffer each time we switch modes. First of all,
the code is protected by checking the factory_mode state. The size of the
buffer is static and can't be changed after ->probe() anyway.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 1023d4134b8d..3895e194006a 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -544,16 +544,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
 
 	disable_irq(client->irq);
 
-	if (!tsdata->raw_buffer) {
-		tsdata->raw_bufsize = tsdata->num_x * tsdata->num_y *
-				      sizeof(u16);
-		tsdata->raw_buffer = kzalloc(tsdata->raw_bufsize, GFP_KERNEL);
-		if (!tsdata->raw_buffer) {
-			error = -ENOMEM;
-			goto err_out;
-		}
-	}
-
 	/* mode register is 0x3c when in the work mode */
 	error = edt_ft5x06_register_write(tsdata, WORK_REGISTER_OPMODE, 0x03);
 	if (error) {

@@ -581,8 +571,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
 	return 0;
 
 err_out:
-	kfree(tsdata->raw_buffer);
-	tsdata->raw_buffer = NULL;
 	tsdata->factory_mode = false;
 	enable_irq(client->irq);

@@ -622,9 +610,6 @@ static int edt_ft5x06_work_mode(struct edt_ft5x06_ts_data *tsdata)
 		return -EIO;
 	}
 
-	kfree(tsdata->raw_buffer);
-	tsdata->raw_buffer = NULL;
-
 	/* restore parameters */
 	edt_ft5x06_register_write(tsdata, reg_addr->reg_threshold,
 				  tsdata->threshold);

@@ -697,7 +682,7 @@ static ssize_t edt_ft5x06_debugfs_raw_data_read(struct file *file,
 
 	mutex_lock(&tsdata->mutex);
 
-	if (!tsdata->factory_mode || !tsdata->raw_buffer) {
+	if (!tsdata->factory_mode) {
 		error = -EIO;
 		goto out;
 	}

@@ -774,6 +759,12 @@ edt_ft5x06_ts_prepare_debugfs(struct edt_ft5x06_ts_data *tsdata,
 
 	debugfs_create_file("mode", S_IRUSR | S_IWUSR,
 			    tsdata->debug_dir, tsdata, &debugfs_mode_fops);
+
+	tsdata->raw_bufsize = tsdata->num_x * tsdata->num_y * sizeof(u16);
+	tsdata->raw_buffer = kzalloc(tsdata->raw_bufsize, GFP_KERNEL);
+	if (!tsdata->raw_buffer)
+		return;
+
 	debugfs_create_file("raw_data", S_IRUSR,
 			    tsdata->debug_dir, tsdata, &debugfs_raw_data_fops);
 }

-- 
2.25.1

[PATCH v1 2/5] Input: edt-ft5x06 - refactor condition in edt_ft5x06_debugfs_mode_set()

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-03-03 18:09:24

For better reading unroll nested conditions to simple if-else-if.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d2587724c52a..cb67104c6934 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -666,10 +666,10 @@ static int edt_ft5x06_debugfs_mode_set(void *data, u64 mode)
 
 	mutex_lock(&tsdata->mutex);
 
-	if (mode != tsdata->factory_mode) {
-		retval = mode ? edt_ft5x06_factory_mode(tsdata) :
-				edt_ft5x06_work_mode(tsdata);
-	}
+	if (mode && !tsdata->factory_mode)
+		retval = edt_ft5x06_factory_mode(tsdata);
+	else if (!mode && tsdata->factory_mode)
+		retval = edt_ft5x06_work_mode(tsdata);
 
 	mutex_unlock(&tsdata->mutex);

-- 
2.25.1

Re: [PATCH v1 5/5] Input: edt-ft5x06 - allocate buffer once for debugging

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-07 00:57:51

Hi Andy,

On Tue, Mar 03, 2020 at 08:09:17PM +0200, Andy Shevchenko wrote:

There is no need to allocate buffer each time we switch modes. First of all,
the code is protected by checking the factory_mode state. The size of the
buffer is static and can't be changed after ->probe() anyway.

Why do we need to keep memory allocated if it is not going to be used
majority of the time? How much is the code savings vs. allocated memory
size (without considering having multiple devices connected to the same
system).

Thanks.

-- 
Dmitry

Re: [PATCH v1 4/5] Input: edt-ft5x06 - do not try to allocate too much memory

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-07 01:08:43

On Tue, Mar 03, 2020 at 08:09:16PM +0200, Andy Shevchenko wrote:

quoted hunk

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.

-- 
Dmitry

Re: [PATCH v1 2/5] Input: edt-ft5x06 - refactor condition in edt_ft5x06_debugfs_mode_set()

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-07 01:11:18

On Tue, Mar 03, 2020 at 08:09:14PM +0200, Andy Shevchenko wrote:

quoted hunk

For better reading unroll nested conditions to simple if-else-if.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d2587724c52a..cb67104c6934 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -666,10 +666,10 @@ static int edt_ft5x06_debugfs_mode_set(void *data, u64 mode)
 
 	mutex_lock(&tsdata->mutex);
 
-	if (mode != tsdata->factory_mode) {
-		retval = mode ? edt_ft5x06_factory_mode(tsdata) :
-				edt_ft5x06_work_mode(tsdata);
-	}
+	if (mode && !tsdata->factory_mode)
+		retval = edt_ft5x06_factory_mode(tsdata);
+	else if (!mode && tsdata->factory_mode)
+		retval = edt_ft5x06_work_mode(tsdata);

Sorry, I find the original better, as I read it

	if mode changing then
		activate desired mode (either factory or normal mode)

Thanks.

-- 
Dmitry

Re: [PATCH v1 1/5] Input: of_touchscreen - explicitly choose axis

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-07 01:13:01

On Tue, Mar 03, 2020 at 08:09:13PM +0200, Andy Shevchenko wrote:

The 'axis + 1' calculation is implicit and potentially error prone.
Moreover, few lines before the axis is set explicitly for both X and Y.

Do the same when retrieving different properties for X and Y.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied, thank you.

quoted hunk

---
 drivers/input/touchscreen/of_touchscreen.c | 35 +++++++++++-----------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index e16ec4c7043a..97342e14b4f1 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c

@@ -66,7 +66,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 {
 	struct device *dev = input->dev.parent;
 	struct input_absinfo *absinfo;
-	unsigned int axis;
+	unsigned int axis, axis_x, axis_y;
 	unsigned int minimum, maximum, fuzz;
 	bool data_present;

@@ -74,33 +74,34 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	if (!input->absinfo)
 		return;
 
-	axis = multitouch ? ABS_MT_POSITION_X : ABS_X;
+	axis_x = multitouch ? ABS_MT_POSITION_X : ABS_X;
+	axis_y = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
+
 	data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
-						input_abs_get_min(input, axis),
+						input_abs_get_min(input, axis_x),
 						&minimum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-size-x",
 						input_abs_get_max(input,
-								  axis) + 1,
+								  axis_x) + 1,
 						&maximum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x",
-						input_abs_get_fuzz(input, axis),
+						input_abs_get_fuzz(input, axis_x),
 						&fuzz);
 	if (data_present)
-		touchscreen_set_params(input, axis, minimum, maximum - 1, fuzz);
+		touchscreen_set_params(input, axis_x, minimum, maximum - 1, fuzz);
 
-	axis = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
 	data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-y",
-						input_abs_get_min(input, axis),
+						input_abs_get_min(input, axis_y),
 						&minimum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-size-y",
 						input_abs_get_max(input,
-								  axis) + 1,
+								  axis_y) + 1,
 						&maximum) |
 		       touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y",
-						input_abs_get_fuzz(input, axis),
+						input_abs_get_fuzz(input, axis_y),
 						&fuzz);
 	if (data_present)
-		touchscreen_set_params(input, axis, minimum, maximum - 1, fuzz);
+		touchscreen_set_params(input, axis_y, minimum, maximum - 1, fuzz);
 
 	axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE;
 	data_present = touchscreen_get_prop_u32(dev,

@@ -117,15 +118,13 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	if (!prop)
 		return;
 
-	axis = multitouch ? ABS_MT_POSITION_X : ABS_X;
-
-	prop->max_x = input_abs_get_max(input, axis);
-	prop->max_y = input_abs_get_max(input, axis + 1);
+	prop->max_x = input_abs_get_max(input, axis_x);
+	prop->max_y = input_abs_get_max(input, axis_y);
 
 	prop->invert_x =
 		device_property_read_bool(dev, "touchscreen-inverted-x");
 	if (prop->invert_x) {
-		absinfo = &input->absinfo[axis];
+		absinfo = &input->absinfo[axis_x];
 		absinfo->maximum -= absinfo->minimum;
 		absinfo->minimum = 0;
 	}

@@ -133,7 +132,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	prop->invert_y =
 		device_property_read_bool(dev, "touchscreen-inverted-y");
 	if (prop->invert_y) {
-		absinfo = &input->absinfo[axis + 1];
+		absinfo = &input->absinfo[axis_y];
 		absinfo->maximum -= absinfo->minimum;
 		absinfo->minimum = 0;
 	}

@@ -141,7 +140,7 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
 	prop->swap_x_y =
 		device_property_read_bool(dev, "touchscreen-swapped-x-y");
 	if (prop->swap_x_y)
-		swap(input->absinfo[axis], input->absinfo[axis + 1]);
+		swap(input->absinfo[axis_x], input->absinfo[axis_y]);
 }
 EXPORT_SYMBOL(touchscreen_parse_properties);

-- 
2.25.1

-- 
Dmitry

Re: [PATCH v1 4/5] Input: edt-ft5x06 - do not try to allocate too much memory

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-05-11 10:08:50

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:

quoted

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.

quoted

+	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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/5] Input: edt-ft5x06 - use U16_MAX instead of -1

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-05-11 10:09:19

On Tue, Mar 03, 2020 at 08:09:15PM +0200, Andy Shevchenko wrote:

Explicitly show what the value we supply for the touchscreen resolution
when it can't be detected. -1 is hard to compare with when unsigned short
type is in use. The change will help to avoid signed vs. unsigned error
prone comparisons.

So, this left without comment, does it mean you are going to apply it?

quoted hunk

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index cb67104c6934..a05c6b597d43 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c

@@ -23,6 +23,7 @@
 #include <linux/input/touchscreen.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/ratelimit.h>
 #include <linux/regulator/consumer.h>

@@ -983,8 +984,8 @@ edt_ft5x06_ts_get_parameters(struct edt_ft5x06_ts_data *tsdata)
 		tsdata->num_y = edt_ft5x06_register_read(tsdata,
 							 reg_addr->reg_num_y);
 	} else {
-		tsdata->num_x = -1;
-		tsdata->num_y = -1;
+		tsdata->num_x = U16_MAX;
+		tsdata->num_y = U16_MAX;
 	}
 }

-- 
2.25.1

-- 
With Best Regards,
Andy Shevchenko

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help