[PATCH RFC V2 0/3] Input: goodix - add axis swapping and axis inversion support

[PATCH RFC V2 0/3] Input: goodix - add axis swapping and axis inversion support

[Karsten Merker]

Hello,

this is v2 of my "Input: goodix - add axis swapping and axis inversion
support" patchset.
The goodix touchscreen driver has gained device-tree support in kernel
4.1, but doesn't currently support the touchscreen-swapped-x-y,
touchscreen-inverted-x and touchscreen-inverted-y properties.

On systems which combine a portrait-mode display with a landscape-mode
touchscreen, such as e.g. the MSI Primo 81 tablet, support for these
features is necessary to have the touchscreen and the display use the
same coordinate system.

With support for axis inversion, the "rotated_screen" flag in the
driver can also be removed, as "rotated_screen" is just a special case
of x/y axis inversion.

This patchset sits on top of the "[PATCH v8 0/9] Goodix touchscreen
enhancements" series by Irina Tirdea:
https://www.spinics.net/lists/linux-input/msg41437.html

I have successfully tested the axis swapping on an (arm-based) MSI
Primo 81 tablet, but I lack appropriate hardware to do a real-world
test of the "rotated_screen" code path, so I would appreciate very
much if somebody with appropriate hardware (WinBook TW100 or TW700)
could give it a try.

Regards,
Karsten

Changelog:

v1: * Initial version (based von v6 of Irina Tirdea's "Goodix
      touchscreen enhancements" series).
      Reviewed-by: Bastien Nocera <hadess@hadess.net>

v2: * Rebase against v8 of Irina Tirdea's "Goodix touchscreen
      enhancements" series.
    * Fix a typo in the commit message.
    * Add an update for the goodix dt bindings documentation
      (patch No. 3).


Karsten Merker (3):
  Input: goodix - add dt axis swapping and axis inversion support
  Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"
  Input: goodix - update dt bindings documentation (axis
    swapping/inversion)

 .../bindings/input/touchscreen/goodix.txt          |  6 ++++
 drivers/input/touchscreen/goodix.c                 | 33 ++++++++++++++++++----
 2 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.1.4

[PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support

[Karsten Merker]

Implement support for the following device-tree properties
in the goodix touchscreen driver:

 - touchscreen-inverted-x:  X axis is inverted (boolean)
 - touchscreen-inverted-y:  Y axis is inverted (boolean)
 - touchscreen-swapped-x-y: X and Y axis are swapped (boolean)

These are necessary on tablets which have a display in portrait
format while the touchscreen is in landscape format, such as e.g.
the MSI Primo 81.

Signed-off-by: Karsten Merker <merker@debian.org>
---
 drivers/input/touchscreen/goodix.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 22bfc4b..a05bdad 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -2,6 +2,7 @@
  *  Driver for Goodix Touchscreens
  *
  *  Copyright (c) 2014 Red Hat Inc.
+ *  Copyright (c) 2015 K. Merker <merker@debian.org>
  *
  *  This code is based on gt9xx.c authored by andrew@goodix.com:
  *
@@ -53,6 +54,9 @@ struct goodix_ts_data {
 	atomic_t open_count;
 	/* Protects power management calls and access to suspended flag */
 	struct mutex mutex;
+	bool swapped_x_y;
+	bool inverted_x;
+	bool inverted_y;
 };
 
 #define GOODIX_GPIO_INT_NAME		"irq"
@@ -271,6 +275,14 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
 		input_y = ts->abs_y_max - input_y;
 	}
 
+	/* Inversions have to happen before axis swapping */
+	if (ts->inverted_x)
+		input_x = ts->abs_x_max - input_x;
+	if (ts->inverted_y)
+		input_y = ts->abs_y_max - input_y;
+	if (ts->swapped_x_y)
+		swap(input_x, input_y);
+
 	input_mt_slot(ts->input_dev, id);
 	input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
 	input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
@@ -666,6 +678,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 			 error);
 		ts->abs_x_max = GOODIX_MAX_WIDTH;
 		ts->abs_y_max = GOODIX_MAX_HEIGHT;
+		if (ts->swapped_x_y)
+			swap(ts->abs_x_max, ts->abs_y_max);
 		ts->int_trigger_type = GOODIX_INT_TRIGGER;
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 		return;
@@ -673,6 +687,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 
 	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
 	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
+	if (ts->swapped_x_y)
+		swap(ts->abs_x_max, ts->abs_y_max);
 	ts->int_trigger_type = config[TRIGGER_LOC] & 0x03;
 	ts->max_touch_num = config[MAX_CONTACTS_LOC] & 0x0f;
 	if (!ts->abs_x_max || !ts->abs_y_max || !ts->max_touch_num) {
@@ -680,6 +696,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 			"Invalid config, using defaults\n");
 		ts->abs_x_max = GOODIX_MAX_WIDTH;
 		ts->abs_y_max = GOODIX_MAX_HEIGHT;
+		if (ts->swapped_x_y)
+			swap(ts->abs_x_max, ts->abs_y_max);
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 	}
 
@@ -950,6 +968,15 @@ static int goodix_ts_probe(struct i2c_client *client,
 		return 0;
 	}
 
+#ifdef CONFIG_OF
+	ts->swapped_x_y = of_property_read_bool(client->dev.of_node,
+						"touchscreen-swapped-x-y");
+	ts->inverted_x = of_property_read_bool(client->dev.of_node,
+					       "touchscreen-inverted-x");
+	ts->inverted_y = of_property_read_bool(client->dev.of_node,
+					       "touchscreen-inverted-y");
+#endif
+
 	return goodix_configure_dev(ts);
 
 err_free_cfg_name:
-- 
2.1.4

[PATCH RFC V2 2/3] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

[Karsten Merker]

The goodix touchscreen driver uses a "rotated_screen" flag for
systems on which the touchscreen is mounted rotated by 180
degrees with respect to the display.  With the addition of
support for the dt properties "touchscreen-inverted-x" and
"touchscreen-inverted-y", a separate "rotated_screen" flag
is not necessary any more. This patch replaces it by setting
the inverted_x and inverted_y flags instead.

Signed-off-by: Karsten Merker <merker@debian.org>
---
 drivers/input/touchscreen/goodix.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index a05bdad..d910b27 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -40,7 +40,6 @@ struct goodix_ts_data {
 	int abs_y_max;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
-	bool rotated_screen;
 	int cfg_len;
 	struct gpio_desc *gpiod_int;
 	struct gpio_desc *gpiod_rst;
@@ -270,11 +269,6 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
 	int input_y = get_unaligned_le16(&coor_data[3]);
 	int input_w = get_unaligned_le16(&coor_data[5]);
 
-	if (ts->rotated_screen) {
-		input_x = ts->abs_x_max - input_x;
-		input_y = ts->abs_y_max - input_y;
-	}
-
 	/* Inversions have to happen before axis swapping */
 	if (ts->inverted_x)
 		input_x = ts->abs_x_max - input_x;
@@ -701,10 +695,12 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 	}
 
-	ts->rotated_screen = dmi_check_system(rotated_screen);
-	if (ts->rotated_screen)
+	if (dmi_check_system(rotated_screen)) {
+		ts->inverted_x = true;
+		ts->inverted_y = true;
 		dev_dbg(&ts->client->dev,
 			 "Applying '180 degrees rotated screen' quirk\n");
+	}
 }
 
 /**
-- 
2.1.4

[PATCH RFC V2 3/3] Input: goodix - update dt bindings documentation (axis swapping/inversion)

[Karsten Merker]

The goodix touchscreen driver has gained support for the
optional touchscreen-inverted-x, touchscreen-inverted-y
and touchscreen-swapped-x-y properties as described in
Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt.

Document these properties in the goodix bindings description.

Signed-off-by: Karsten Merker <merker@debian.org>
---
 Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 4db3393..4ecd0e1 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -14,6 +14,7 @@ Required properties:
  - interrupts		: Interrupt to which the chip is connected
  - irq-gpio		: GPIO pin used for IRQ
  - reset-gpio		: GPIO pin used for reset
+
 Optional properties:
 
  - esd-recovery-timeout-ms : ESD poll time (in milli seconds) for the driver to
@@ -21,6 +22,11 @@ Optional properties:
                             device. ESD is disabled if this property is not set
                             or is set to 0.
 
+ - touchscreen-inverted-x  : X axis is inverted (boolean)
+ - touchscreen-inverted-y  : Y axis is inverted (boolean)
+ - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
+                             (swapping is done after inverting the axis)
+
 Example:
 
 	i2c@00000000 {
-- 
2.1.4

Re: [PATCH RFC V2 0/3] Input: goodix - add axis swapping and axis inversion support

[Bastien Nocera]

On Fri, 2015-10-09 at 19:55 +0200, Karsten Merker wrote:
> Hello,
> 
> this is v2 of my "Input: goodix - add axis swapping and axis
> inversion
> support" patchset.
> The goodix touchscreen driver has gained device-tree support in
> kernel
> 4.1, but doesn't currently support the touchscreen-swapped-x-y,
> touchscreen-inverted-x and touchscreen-inverted-y properties.
> 
> On systems which combine a portrait-mode display with a landscape-
> mode
> touchscreen, such as e.g. the MSI Primo 81 tablet, support for these
> features is necessary to have the touchscreen and the display use the
> same coordinate system.
> 
> With support for axis inversion, the "rotated_screen" flag in the
> driver can also be removed, as "rotated_screen" is just a special
> case
> of x/y axis inversion.
> 
> This patchset sits on top of the "[PATCH v8 0/9] Goodix touchscreen
> enhancements" series by Irina Tirdea:
> https://www.spinics.net/lists/linux-input/msg41437.html
> 
> I have successfully tested the axis swapping on an (arm-based) MSI
> Primo 81 tablet, but I lack appropriate hardware to do a real-world
> test of the "rotated_screen" code path, so I would appreciate very
> much if somebody with appropriate hardware (WinBook TW100 or TW700)
> could give it a try.


I've pushed the first 2 patches of this patchset to:
https://github.com/hadess/gt9xx/tree/karsten-merker

And tested on the TW100. You can add:
Tested-by: Bastien Nocera <hadess@hadess.net>

To the first 2 patches.

Cheers

Re: [PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support

[Bastien Nocera]

On Fri, 2015-10-09 at 19:55 +0200, Karsten Merker wrote:
> Implement support for the following device-tree properties
> in the goodix touchscreen driver:
> 
>  - touchscreen-inverted-x:  X axis is inverted (boolean)
>  - touchscreen-inverted-y:  Y axis is inverted (boolean)
>  - touchscreen-swapped-x-y: X and Y axis are swapped (boolean)
> 
> These are necessary on tablets which have a display in portrait
> format while the touchscreen is in landscape format, such as e.g.
> the MSI Primo 81.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>

Acked-by: Bastien Nocera <hadess@hadess.net>

Re: [PATCH RFC V2 2/3] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

[Bastien Nocera]

On Fri, 2015-10-09 at 19:55 +0200, Karsten Merker wrote:
> The goodix touchscreen driver uses a "rotated_screen" flag for
> systems on which the touchscreen is mounted rotated by 180
> degrees with respect to the display.  With the addition of
> support for the dt properties "touchscreen-inverted-x" and
> "touchscreen-inverted-y", a separate "rotated_screen" flag
> is not necessary any more. This patch replaces it by setting
> the inverted_x and inverted_y flags instead.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>

Acked-by: Bastien Nocera <hadess@hadess.net>

Re: [PATCH RFC V2 2/3] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

[Bastien Nocera]

On Fri, 2015-10-09 at 19:55 +0200, Karsten Merker wrote:
> The goodix touchscreen driver uses a "rotated_screen" flag for
> systems on which the touchscreen is mounted rotated by 180
> degrees with respect to the display.  With the addition of
> support for the dt properties "touchscreen-inverted-x" and
> "touchscreen-inverted-y", a separate "rotated_screen" flag
> is not necessary any more. This patch replaces it by setting
> the inverted_x and inverted_y flags instead.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>

Acked-by: Bastien Nocera <hadess@hadess.net>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support

[Tirdea, Irina]

> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-owner@vger.kernel.org] On Behalf Of Karsten Merker
> Sent: 09 October, 2015 20:56
> To: Bastien Nocera; Dmitry Torokhov; Tirdea, Irina; Aleksei Mamlin; linux-input@vger.kernel.org; Ian Campbell
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Chen-Yu Tsai; Karsten Merker
> Subject: [PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support
> 
> Implement support for the following device-tree properties
> in the goodix touchscreen driver:
> 
>  - touchscreen-inverted-x:  X axis is inverted (boolean)
>  - touchscreen-inverted-y:  Y axis is inverted (boolean)
>  - touchscreen-swapped-x-y: X and Y axis are swapped (boolean)
> 
> These are necessary on tablets which have a display in portrait
> format while the touchscreen is in landscape format, such as e.g.
> the MSI Primo 81.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>
> ---
>  drivers/input/touchscreen/goodix.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 22bfc4b..a05bdad 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -2,6 +2,7 @@
>   *  Driver for Goodix Touchscreens
>   *
>   *  Copyright (c) 2014 Red Hat Inc.
> + *  Copyright (c) 2015 K. Merker <merker@debian.org>
>   *
>   *  This code is based on gt9xx.c authored by andrew@goodix.com:
>   *
> @@ -53,6 +54,9 @@ struct goodix_ts_data {
>  	atomic_t open_count;
>  	/* Protects power management calls and access to suspended flag */
>  	struct mutex mutex;
> +	bool swapped_x_y;
> +	bool inverted_x;
> +	bool inverted_y;
>  };
> 
>  #define GOODIX_GPIO_INT_NAME		"irq"
> @@ -271,6 +275,14 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
>  		input_y = ts->abs_y_max - input_y;
>  	}
> 
> +	/* Inversions have to happen before axis swapping */
> +	if (ts->inverted_x)
> +		input_x = ts->abs_x_max - input_x;
> +	if (ts->inverted_y)
> +		input_y = ts->abs_y_max - input_y;
> +	if (ts->swapped_x_y)
> +		swap(input_x, input_y);
> +
>  	input_mt_slot(ts->input_dev, id);
>  	input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
>  	input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
> @@ -666,6 +678,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
>  			 error);
>  		ts->abs_x_max = GOODIX_MAX_WIDTH;
>  		ts->abs_y_max = GOODIX_MAX_HEIGHT;
> +		if (ts->swapped_x_y)
> +			swap(ts->abs_x_max, ts->abs_y_max);
>  		ts->int_trigger_type = GOODIX_INT_TRIGGER;
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;
>  		return;
> @@ -673,6 +687,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
> 
>  	ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
>  	ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
> +	if (ts->swapped_x_y)
> +		swap(ts->abs_x_max, ts->abs_y_max);
>  	ts->int_trigger_type = config[TRIGGER_LOC] & 0x03;
>  	ts->max_touch_num = config[MAX_CONTACTS_LOC] & 0x0f;
>  	if (!ts->abs_x_max || !ts->abs_y_max || !ts->max_touch_num) {
> @@ -680,6 +696,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
>  			"Invalid config, using defaults\n");
>  		ts->abs_x_max = GOODIX_MAX_WIDTH;
>  		ts->abs_y_max = GOODIX_MAX_HEIGHT;
> +		if (ts->swapped_x_y)
> +			swap(ts->abs_x_max, ts->abs_y_max);
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;
>  	}
> 
> @@ -950,6 +968,15 @@ static int goodix_ts_probe(struct i2c_client *client,
>  		return 0;
>  	}
> 
> +#ifdef CONFIG_OF
> +	ts->swapped_x_y = of_property_read_bool(client->dev.of_node,
> +						"touchscreen-swapped-x-y");
> +	ts->inverted_x = of_property_read_bool(client->dev.of_node,
> +					       "touchscreen-inverted-x");
> +	ts->inverted_y = of_property_read_bool(client->dev.of_node,
> +					       "touchscreen-inverted-y");
> +#endif
> +

If interrupt and reset gpio pins are declared in the DT configuration, this code will not
be executed.  To make the properties available for all configurations (with/without
gpio pins declared), you should read the properties inside goodix_configure_dev().

You could also use device_property_read_bool() instead, so that these properties can
be used with ACPI 5.1. as well.

Thanks,
Irina

>  	return goodix_configure_dev(ts);
> 
>  err_free_cfg_name:
> --
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH RFC V2 0/3] Input: goodix - add axis swapping and axis inversion support

[Tirdea, Irina]

> -----Original Message-----
> From: Karsten Merker [mailto:merker@debian.org]
> Sent: 09 October, 2015 20:56
> To: Bastien Nocera; Dmitry Torokhov; Tirdea, Irina; Aleksei Mamlin; linux-input@vger.kernel.org; Ian Campbell
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Chen-Yu Tsai; Karsten Merker
> Subject: [PATCH RFC V2 0/3] Input: goodix - add axis swapping and axis inversion support
> 
> Hello,
> 
> this is v2 of my "Input: goodix - add axis swapping and axis inversion
> support" patchset.
> The goodix touchscreen driver has gained device-tree support in kernel
> 4.1, but doesn't currently support the touchscreen-swapped-x-y,
> touchscreen-inverted-x and touchscreen-inverted-y properties.
> 
> On systems which combine a portrait-mode display with a landscape-mode
> touchscreen, such as e.g. the MSI Primo 81 tablet, support for these
> features is necessary to have the touchscreen and the display use the
> same coordinate system.
> 
> With support for axis inversion, the "rotated_screen" flag in the
> driver can also be removed, as "rotated_screen" is just a special case
> of x/y axis inversion.
> 
> This patchset sits on top of the "[PATCH v8 0/9] Goodix touchscreen
> enhancements" series by Irina Tirdea:
> https://www.spinics.net/lists/linux-input/msg41437.html
> 
> I have successfully tested the axis swapping on an (arm-based) MSI
> Primo 81 tablet, but I lack appropriate hardware to do a real-world
> test of the "rotated_screen" code path, so I would appreciate very
> much if somebody with appropriate hardware (WinBook TW100 or TW700)
> could give it a try.
> 
> Regards,
> Karsten
> 

Hi Karsten,

I took a look at your patches and also did a quick test on my setup.
Code looks good, I have just one comment I've mentioned on the
first patch in the series.

Thanks,
Irina

> Changelog:
> 
> v1: * Initial version (based von v6 of Irina Tirdea's "Goodix
>       touchscreen enhancements" series).
>       Reviewed-by: Bastien Nocera <hadess@hadess.net>
> 
> v2: * Rebase against v8 of Irina Tirdea's "Goodix touchscreen
>       enhancements" series.
>     * Fix a typo in the commit message.
>     * Add an update for the goodix dt bindings documentation
>       (patch No. 3).
> 
> 
> Karsten Merker (3):
>   Input: goodix - add dt axis swapping and axis inversion support
>   Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"
>   Input: goodix - update dt bindings documentation (axis
>     swapping/inversion)
> 
>  .../bindings/input/touchscreen/goodix.txt          |  6 ++++
>  drivers/input/touchscreen/goodix.c                 | 33 ++++++++++++++++++----
>  2 files changed, 34 insertions(+), 5 deletions(-)
> 
> --
> 2.1.4

RE: [PATCH RFC V2 2/3] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"

[Tirdea, Irina]

> -----Original Message-----
> From: Karsten Merker [mailto:merker@debian.org]
> Sent: 09 October, 2015 20:56
> To: Bastien Nocera; Dmitry Torokhov; Tirdea, Irina; Aleksei Mamlin; linux-input@vger.kernel.org; Ian Campbell
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Chen-Yu Tsai; Karsten Merker
> Subject: [PATCH RFC V2 2/3] Input: goodix - use "inverted_[xy]" flags instead of "rotated_screen"
> 
> The goodix touchscreen driver uses a "rotated_screen" flag for
> systems on which the touchscreen is mounted rotated by 180
> degrees with respect to the display.  With the addition of
> support for the dt properties "touchscreen-inverted-x" and
> "touchscreen-inverted-y", a separate "rotated_screen" flag
> is not necessary any more. This patch replaces it by setting
> the inverted_x and inverted_y flags instead.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>
> ---

Reviewed-by: Irina Tirdea <irina.tirdea@intel.com>

>  drivers/input/touchscreen/goodix.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index a05bdad..d910b27 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -40,7 +40,6 @@ struct goodix_ts_data {
>  	int abs_y_max;
>  	unsigned int max_touch_num;
>  	unsigned int int_trigger_type;
> -	bool rotated_screen;
>  	int cfg_len;
>  	struct gpio_desc *gpiod_int;
>  	struct gpio_desc *gpiod_rst;
> @@ -270,11 +269,6 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
>  	int input_y = get_unaligned_le16(&coor_data[3]);
>  	int input_w = get_unaligned_le16(&coor_data[5]);
> 
> -	if (ts->rotated_screen) {
> -		input_x = ts->abs_x_max - input_x;
> -		input_y = ts->abs_y_max - input_y;
> -	}
> -
>  	/* Inversions have to happen before axis swapping */
>  	if (ts->inverted_x)
>  		input_x = ts->abs_x_max - input_x;
> @@ -701,10 +695,12 @@ static void goodix_read_config(struct goodix_ts_data *ts)
>  		ts->max_touch_num = GOODIX_MAX_CONTACTS;
>  	}
> 
> -	ts->rotated_screen = dmi_check_system(rotated_screen);
> -	if (ts->rotated_screen)
> +	if (dmi_check_system(rotated_screen)) {
> +		ts->inverted_x = true;
> +		ts->inverted_y = true;
>  		dev_dbg(&ts->client->dev,
>  			 "Applying '180 degrees rotated screen' quirk\n");
> +	}
>  }
> 
>  /**
> --
> 2.1.4

RE: [PATCH RFC V2 3/3] Input: goodix - update dt bindings documentation (axis swapping/inversion)

[Tirdea, Irina]

> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-owner@vger.kernel.org] On Behalf Of Karsten Merker
> Sent: 09 October, 2015 20:56
> To: Bastien Nocera; Dmitry Torokhov; Tirdea, Irina; Aleksei Mamlin; linux-input@vger.kernel.org; Ian Campbell
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Chen-Yu Tsai; Karsten Merker
> Subject: [PATCH RFC V2 3/3] Input: goodix - update dt bindings documentation (axis swapping/inversion)
> 
> The goodix touchscreen driver has gained support for the
> optional touchscreen-inverted-x, touchscreen-inverted-y
> and touchscreen-swapped-x-y properties as described in
> Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt.
> 
> Document these properties in the goodix bindings description.
> 
> Signed-off-by: Karsten Merker <merker@debian.org>

Reviewed-by: Irina Tirdea <irina.tirdea@intel.com>

> ---
>  Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 4db3393..4ecd0e1 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -14,6 +14,7 @@ Required properties:
>   - interrupts		: Interrupt to which the chip is connected
>   - irq-gpio		: GPIO pin used for IRQ
>   - reset-gpio		: GPIO pin used for reset
> +
>  Optional properties:
> 
>   - esd-recovery-timeout-ms : ESD poll time (in milli seconds) for the driver to
> @@ -21,6 +22,11 @@ Optional properties:
>                              device. ESD is disabled if this property is not set
>                              or is set to 0.
> 
> + - touchscreen-inverted-x  : X axis is inverted (boolean)
> + - touchscreen-inverted-y  : Y axis is inverted (boolean)
> + - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
> +                             (swapping is done after inverting the axis)
> +
>  Example:
> 
>  	i2c@00000000 {
> --
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support

[Karsten Merker]

On Tue, Oct 13, 2015 at 06:58:07AM +0000, Tirdea, Irina wrote:
> 
> > -----Original Message-----
> > From: linux-input-owner@vger.kernel.org [mailto:linux-input-owner@vger.kernel.org] On Behalf Of Karsten Merker
> > Sent: 09 October, 2015 20:56
> > To: Bastien Nocera; Dmitry Torokhov; Tirdea, Irina; Aleksei Mamlin; linux-input@vger.kernel.org; Ian Campbell
> > Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Chen-Yu Tsai; Karsten Merker
> > Subject: [PATCH RFC V2 1/3] Input: goodix - add dt axis swapping and axis inversion support
> > 
> > Implement support for the following device-tree properties
> > in the goodix touchscreen driver:
> > 
> >  - touchscreen-inverted-x:  X axis is inverted (boolean)
> >  - touchscreen-inverted-y:  Y axis is inverted (boolean)
> >  - touchscreen-swapped-x-y: X and Y axis are swapped (boolean)
> > 
> > These are necessary on tablets which have a display in portrait
> > format while the touchscreen is in landscape format, such as e.g.
> > the MSI Primo 81.
[...]
> > @@ -950,6 +968,15 @@ static int goodix_ts_probe(struct i2c_client *client,
> >  		return 0;
> >  	}
> > 
> > +#ifdef CONFIG_OF
> > +	ts->swapped_x_y = of_property_read_bool(client->dev.of_node,
> > +						"touchscreen-swapped-x-y");
> > +	ts->inverted_x = of_property_read_bool(client->dev.of_node,
> > +					       "touchscreen-inverted-x");
> > +	ts->inverted_y = of_property_read_bool(client->dev.of_node,
> > +					       "touchscreen-inverted-y");
> > +#endif
> > +
> 
> If interrupt and reset gpio pins are declared in the DT configuration, this code will not
> be executed.  To make the properties available for all configurations (with/without
> gpio pins declared), you should read the properties inside goodix_configure_dev().
> 
> You could also use device_property_read_bool() instead, so that these properties can
> be used with ACPI 5.1. as well.

Hello,

many thanks for the review.

I'll address both points in a v3 of the patch.

Regards,
Karsten
-- 
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung
sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der
Werbung sowie der Markt- oder Meinungsforschung.