diff mbox series

[06/10] input: iqs5xx: Simplify axis setup logic

Message ID 1611002626-5889-7-git-send-email-jeff@labundy.com
State Accepted
Commit 4a76d861d9182f2edfab96e7aa1f1e10f4f86bc0
Headers show
Series input: iqs5xx: Minor enhancements and optimizations | expand

Commit Message

Jeff LaBundy Jan. 18, 2021, 8:43 p.m. UTC
The present implementation manipulates axis swap and inversion fields
in the device to more or less duplicate what touchscreen_report_pos()
does. The resulting logic is convoluted and difficult to follow.

Instead report the maximum X and Y coordinates in earnest as they are
read from the device, then let touchscreen_parse_properties() fix the
axes up as necessary. Finally, use touchscreen_report_pos() to report
the transformed coordinates.

Last but not least, the maximum X and Y coordinates are not functions
of the number of rows/columns that comprise the touch surface. Either
coordinate is simply limited to 1 below what is reported for absolute
X or Y coordinates when no fingers are present (0xFFFF).

Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/touchscreen/iqs5xx.c | 100 ++++++++-----------------------------
 1 file changed, 21 insertions(+), 79 deletions(-)

Comments

Dmitry Torokhov Jan. 25, 2021, 4:40 a.m. UTC | #1
On Mon, Jan 18, 2021 at 02:43:42PM -0600, Jeff LaBundy wrote:
> The present implementation manipulates axis swap and inversion fields

> in the device to more or less duplicate what touchscreen_report_pos()

> does. The resulting logic is convoluted and difficult to follow.

> 

> Instead report the maximum X and Y coordinates in earnest as they are

> read from the device, then let touchscreen_parse_properties() fix the

> axes up as necessary. Finally, use touchscreen_report_pos() to report

> the transformed coordinates.

> 

> Last but not least, the maximum X and Y coordinates are not functions

> of the number of rows/columns that comprise the touch surface. Either

> coordinate is simply limited to 1 below what is reported for absolute

> X or Y coordinates when no fingers are present (0xFFFF).

> 

> Signed-off-by: Jeff LaBundy <jeff@labundy.com>


Applied, thank you.

-- 
Dmitry
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
index 9412fb8..d802cee 100644
--- a/drivers/input/touchscreen/iqs5xx.c
+++ b/drivers/input/touchscreen/iqs5xx.c
@@ -29,9 +29,9 @@ 
 
 #define IQS5XX_FW_FILE_LEN	64
 #define IQS5XX_NUM_RETRIES	10
-#define IQS5XX_NUM_POINTS	256
 #define IQS5XX_NUM_CONTACTS	5
 #define IQS5XX_WR_BYTES_MAX	2
+#define IQS5XX_XY_RES_MAX	0xFFFE
 
 #define IQS5XX_PROD_NUM_IQS550	40
 #define IQS5XX_PROD_NUM_IQS572	58
@@ -51,10 +51,6 @@ 
 #define IQS5XX_EVENT_MODE	0x01
 #define IQS5XX_TP_EVENT		0x04
 
-#define IQS5XX_FLIP_X		0x01
-#define IQS5XX_FLIP_Y		0x02
-#define IQS5XX_SWITCH_XY_AXIS	0x04
-
 #define IQS5XX_PROD_NUM		0x0000
 #define IQS5XX_SYS_INFO0	0x000F
 #define IQS5XX_SYS_INFO1	0x0010
@@ -62,9 +58,6 @@ 
 #define IQS5XX_SYS_CTRL1	0x0432
 #define IQS5XX_SYS_CFG0		0x058E
 #define IQS5XX_SYS_CFG1		0x058F
-#define IQS5XX_TOTAL_RX		0x063D
-#define IQS5XX_TOTAL_TX		0x063E
-#define IQS5XX_XY_CFG0		0x0669
 #define IQS5XX_X_RES		0x066E
 #define IQS5XX_Y_RES		0x0670
 #define IQS5XX_EXP_FILE		0x0677
@@ -102,6 +95,7 @@  struct iqs5xx_private {
 	struct i2c_client *client;
 	struct input_dev *input;
 	struct gpio_desc *reset_gpio;
+	struct touchscreen_properties prop;
 	struct mutex lock;
 	u16 exp_file;
 	u8 bl_status;
@@ -498,12 +492,10 @@  static void iqs5xx_close(struct input_dev *input)
 static int iqs5xx_axis_init(struct i2c_client *client)
 {
 	struct iqs5xx_private *iqs5xx = i2c_get_clientdata(client);
-	struct touchscreen_properties prop;
+	struct touchscreen_properties *prop = &iqs5xx->prop;
 	struct input_dev *input;
+	u16 max_x, max_y;
 	int error;
-	u16 max_x, max_x_hw;
-	u16 max_y, max_y_hw;
-	u8 val;
 
 	if (!iqs5xx->input) {
 		input = devm_input_allocate_device(&client->dev);
@@ -523,89 +515,39 @@  static int iqs5xx_axis_init(struct i2c_client *client)
 		iqs5xx->input = input;
 	}
 
-	touchscreen_parse_properties(iqs5xx->input, true, &prop);
-
-	error = iqs5xx_read_byte(client, IQS5XX_TOTAL_RX, &val);
-	if (error)
-		return error;
-	max_x_hw = (val - 1) * IQS5XX_NUM_POINTS;
-
-	error = iqs5xx_read_byte(client, IQS5XX_TOTAL_TX, &val);
+	error = iqs5xx_read_word(client, IQS5XX_X_RES, &max_x);
 	if (error)
 		return error;
-	max_y_hw = (val - 1) * IQS5XX_NUM_POINTS;
 
-	error = iqs5xx_read_byte(client, IQS5XX_XY_CFG0, &val);
+	error = iqs5xx_read_word(client, IQS5XX_Y_RES, &max_y);
 	if (error)
 		return error;
 
-	if (val & IQS5XX_SWITCH_XY_AXIS)
-		swap(max_x_hw, max_y_hw);
-
-	if (prop.swap_x_y)
-		val ^= IQS5XX_SWITCH_XY_AXIS;
+	input_abs_set_max(iqs5xx->input, ABS_MT_POSITION_X, max_x);
+	input_abs_set_max(iqs5xx->input, ABS_MT_POSITION_Y, max_y);
 
-	if (prop.invert_x)
-		val ^= prop.swap_x_y ? IQS5XX_FLIP_Y : IQS5XX_FLIP_X;
+	touchscreen_parse_properties(iqs5xx->input, true, prop);
 
-	if (prop.invert_y)
-		val ^= prop.swap_x_y ? IQS5XX_FLIP_X : IQS5XX_FLIP_Y;
-
-	error = iqs5xx_write_byte(client, IQS5XX_XY_CFG0, val);
-	if (error)
-		return error;
-
-	if (prop.max_x > max_x_hw) {
+	if (prop->max_x > IQS5XX_XY_RES_MAX) {
 		dev_err(&client->dev, "Invalid maximum x-coordinate: %u > %u\n",
-			prop.max_x, max_x_hw);
+			prop->max_x, IQS5XX_XY_RES_MAX);
 		return -EINVAL;
-	} else if (prop.max_x == 0) {
-		error = iqs5xx_read_word(client, IQS5XX_X_RES, &max_x);
+	} else if (prop->max_x != max_x) {
+		error = iqs5xx_write_word(client, IQS5XX_X_RES, prop->max_x);
 		if (error)
 			return error;
-
-		input_abs_set_max(iqs5xx->input,
-				  prop.swap_x_y ? ABS_MT_POSITION_Y :
-						  ABS_MT_POSITION_X,
-				  max_x);
-	} else {
-		max_x = (u16)prop.max_x;
 	}
 
-	if (prop.max_y > max_y_hw) {
+	if (prop->max_y > IQS5XX_XY_RES_MAX) {
 		dev_err(&client->dev, "Invalid maximum y-coordinate: %u > %u\n",
-			prop.max_y, max_y_hw);
+			prop->max_y, IQS5XX_XY_RES_MAX);
 		return -EINVAL;
-	} else if (prop.max_y == 0) {
-		error = iqs5xx_read_word(client, IQS5XX_Y_RES, &max_y);
+	} else if (prop->max_y != max_y) {
+		error = iqs5xx_write_word(client, IQS5XX_Y_RES, prop->max_y);
 		if (error)
 			return error;
-
-		input_abs_set_max(iqs5xx->input,
-				  prop.swap_x_y ? ABS_MT_POSITION_X :
-						  ABS_MT_POSITION_Y,
-				  max_y);
-	} else {
-		max_y = (u16)prop.max_y;
 	}
 
-	/*
-	 * Write horizontal and vertical resolution to the device in case its
-	 * original defaults were overridden or swapped as per the properties
-	 * specified in the device tree.
-	 */
-	error = iqs5xx_write_word(client,
-				  prop.swap_x_y ? IQS5XX_Y_RES : IQS5XX_X_RES,
-				  max_x);
-	if (error)
-		return error;
-
-	error = iqs5xx_write_word(client,
-				  prop.swap_x_y ? IQS5XX_X_RES : IQS5XX_Y_RES,
-				  max_y);
-	if (error)
-		return error;
-
 	error = input_mt_init_slots(iqs5xx->input, IQS5XX_NUM_CONTACTS,
 				    INPUT_MT_DIRECT);
 	if (error)
@@ -765,10 +707,10 @@  static irqreturn_t iqs5xx_irq(int irq, void *data)
 		input_mt_slot(input, i);
 		if (input_mt_report_slot_state(input, MT_TOOL_FINGER,
 					       pressure != 0)) {
-			input_report_abs(input, ABS_MT_POSITION_X,
-					 be16_to_cpu(touch_data->abs_x));
-			input_report_abs(input, ABS_MT_POSITION_Y,
-					 be16_to_cpu(touch_data->abs_y));
+			touchscreen_report_pos(iqs5xx->input, &iqs5xx->prop,
+					       be16_to_cpu(touch_data->abs_x),
+					       be16_to_cpu(touch_data->abs_y),
+					       true);
 			input_report_abs(input, ABS_MT_PRESSURE, pressure);
 		}
 	}