[PATCH v4 01/10] dt-bindings: Add Wacom to vendor bindings

[PATCH v4 01/10] dt-bindings: Add Wacom to vendor bindings

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index a8e1e8d2ef20..996f4de2fff5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1216,6 +1216,8 @@ patternProperties:
     description: Vision Optical Technology Co., Ltd.
   "^vxt,.*":
     description: VXT Ltd
+  "^wacom,.*":
+    description: Wacom Co., Ltd
   "^wand,.*":
     description: Wandbord (Technexion)
   "^waveshare,.*":
-- 
2.31.0

[PATCH v4 02/10] dt-bindings: touchscreen: Initial commit of wacom,generic

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../input/touchscreen/wacom,generic.yaml      | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,generic.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,generic.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,generic.yaml
new file mode 100644
index 000000000000..19bbfc55ed76
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,generic.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/wacom,generic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wacom I2C Controller
+
+maintainers:
+  - Alistair Francis <alistair@alistair23.me>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    const: wacom,generic
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include "dt-bindings/interrupt-controller/irq.h"
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        digitiser@9 {
+                compatible = "wacom,generic";
+                reg = <0x9>;
+                interrupt-parent = <&gpio1>;
+                interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+                vdd-supply = <&reg_touch>;
+        };
+    };
-- 
2.31.0

[PATCH v4 03/10] Input: wacom_i2c - Add device tree support to wacom_i2c

[Alistair Francis]

Allow the wacom-i2c device to be exposed via device tree.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
v4:
 - Avoid unused variable warning by not using of_match_ptr()

 drivers/input/touchscreen/wacom_i2c.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 1afc6bde2891..eada68770671 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <asm/unaligned.h>
 
 #define WACOM_CMD_QUERY0	0x04
@@ -262,10 +263,17 @@ static const struct i2c_device_id wacom_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
 
+static const struct of_device_id wacom_i2c_of_match_table[] = {
+	{ .compatible = "wacom,generic" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, wacom_i2c_of_match_table);
+
 static struct i2c_driver wacom_i2c_driver = {
 	.driver	= {
 		.name	= "wacom_i2c",
 		.pm	= &wacom_i2c_pm,
+		.of_match_table = wacom_i2c_of_match_table,
 	},
 
 	.probe		= wacom_i2c_probe,
-- 
2.31.0

[PATCH v4 04/10] Input: wacom_i2c - Add touchscren properties

[Alistair Francis]

Connect touchscreen properties to the wacom_i2c.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
v4:
 - Add touchscreen_report_pos() as well

 drivers/input/touchscreen/wacom_i2c.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index eada68770671..ee1829dd35f4 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -11,6 +11,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
+#include <linux/input/touchscreen.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
 #include <asm/unaligned.h>
@@ -33,6 +34,7 @@ struct wacom_features {
 struct wacom_i2c {
 	struct i2c_client *client;
 	struct input_dev *input;
+	struct touchscreen_properties props;
 	u8 data[WACOM_QUERY_SIZE];
 	bool prox;
 	int tool;
@@ -188,6 +190,9 @@ static int wacom_i2c_probe(struct i2c_client *client,
 	__set_bit(BTN_STYLUS2, input->keybit);
 	__set_bit(BTN_TOUCH, input->keybit);
 
+	touchscreen_parse_properties(input, true, &wac_i2c->props);
+	touchscreen_report_pos(input, &wac_i2c->props, features.x_max,
+			       features.y_max, true);
 	input_set_abs_params(input, ABS_X, 0, features.x_max, 0, 0);
 	input_set_abs_params(input, ABS_Y, 0, features.y_max, 0, 0);
 	input_set_abs_params(input, ABS_PRESSURE,
-- 
2.31.0

[PATCH v4 05/10] Input: wacom_i2c - Add support for distance and tilt x/y

[Alistair Francis]

This is based on the out of tree rM2 driver.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/input/touchscreen/wacom_i2c.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index ee1829dd35f4..3b4bc514dc3f 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -22,12 +22,16 @@
 #define WACOM_CMD_QUERY3	0x02
 #define WACOM_CMD_THROW0	0x05
 #define WACOM_CMD_THROW1	0x00
-#define WACOM_QUERY_SIZE	19
+#define WACOM_QUERY_SIZE	22
 
 struct wacom_features {
 	int x_max;
 	int y_max;
 	int pressure_max;
+	int distance_max;
+	int distance_physical_max;
+	int tilt_x_max;
+	int tilt_y_max;
 	char fw_version;
 };
 
@@ -79,6 +83,10 @@ static int wacom_query_device(struct i2c_client *client,
 	features->y_max = get_unaligned_le16(&data[5]);
 	features->pressure_max = get_unaligned_le16(&data[11]);
 	features->fw_version = get_unaligned_le16(&data[13]);
+	features->distance_max = data[15];
+	features->distance_physical_max = data[16];
+	features->tilt_x_max = get_unaligned_le16(&data[17]);
+	features->tilt_y_max = get_unaligned_le16(&data[19]);
 
 	dev_dbg(&client->dev,
 		"x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
@@ -95,6 +103,7 @@ static irqreturn_t wacom_i2c_irq(int irq, void *dev_id)
 	u8 *data = wac_i2c->data;
 	unsigned int x, y, pressure;
 	unsigned char tsw, f1, f2, ers;
+	short tilt_x, tilt_y, distance;
 	int error;
 
 	error = i2c_master_recv(wac_i2c->client,
@@ -109,6 +118,11 @@ static irqreturn_t wacom_i2c_irq(int irq, void *dev_id)
 	x = le16_to_cpup((__le16 *)&data[4]);
 	y = le16_to_cpup((__le16 *)&data[6]);
 	pressure = le16_to_cpup((__le16 *)&data[8]);
+	distance = data[10];
+
+	/* Signed */
+	tilt_x = le16_to_cpup((__le16 *)&data[11]);
+	tilt_y = le16_to_cpup((__le16 *)&data[13]);
 
 	if (!wac_i2c->prox)
 		wac_i2c->tool = (data[3] & 0x0c) ?
@@ -123,6 +137,9 @@ static irqreturn_t wacom_i2c_irq(int irq, void *dev_id)
 	input_report_abs(input, ABS_X, x);
 	input_report_abs(input, ABS_Y, y);
 	input_report_abs(input, ABS_PRESSURE, pressure);
+	input_report_abs(input, ABS_DISTANCE, distance);
+	input_report_abs(input, ABS_TILT_X, tilt_x);
+	input_report_abs(input, ABS_TILT_Y, tilt_y);
 	input_sync(input);
 
 out:
@@ -197,7 +214,11 @@ static int wacom_i2c_probe(struct i2c_client *client,
 	input_set_abs_params(input, ABS_Y, 0, features.y_max, 0, 0);
 	input_set_abs_params(input, ABS_PRESSURE,
 			     0, features.pressure_max, 0, 0);
-
+	input_set_abs_params(input, ABS_DISTANCE, 0, features.distance_max, 0, 0);
+	input_set_abs_params(input, ABS_TILT_X, -features.tilt_x_max,
+			     features.tilt_x_max, 0, 0);
+	input_set_abs_params(input, ABS_TILT_Y, -features.tilt_y_max,
+			     features.tilt_y_max, 0, 0);
 	input_set_drvdata(input, wac_i2c);
 
 	error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
-- 
2.31.0

[PATCH v4 06/10] Input: wacom_i2c - Clean up the query device fields

[Alistair Francis]

Improve the query device fields to be more verbose.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
v4:
 - Remove the reset_control_reset() logic

 drivers/input/touchscreen/wacom_i2c.c | 64 ++++++++++++++++++---------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 3b4bc514dc3f..84c7ccb737bd 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -13,15 +13,32 @@
 #include <linux/irq.h>
 #include <linux/input/touchscreen.h>
 #include <linux/interrupt.h>
+#include <linux/reset.h>
 #include <linux/of.h>
 #include <asm/unaligned.h>
 
-#define WACOM_CMD_QUERY0	0x04
-#define WACOM_CMD_QUERY1	0x00
-#define WACOM_CMD_QUERY2	0x33
-#define WACOM_CMD_QUERY3	0x02
-#define WACOM_CMD_THROW0	0x05
-#define WACOM_CMD_THROW1	0x00
+// Registers
+#define WACOM_COMMAND_LSB   0x04
+#define WACOM_COMMAND_MSB   0x00
+
+#define WACOM_DATA_LSB      0x05
+#define WACOM_DATA_MSB      0x00
+
+// Report types
+#define REPORT_FEATURE      0x30
+
+// Requests / operations
+#define OPCODE_GET_REPORT   0x02
+
+// Power settings
+#define POWER_ON            0x00
+#define POWER_SLEEP         0x01
+
+// Input report ids
+#define WACOM_PEN_DATA_REPORT           2
+#define WACOM_SHINONOME_REPORT          26
+
+#define WACOM_QUERY_REPORT	3
 #define WACOM_QUERY_SIZE	22
 
 struct wacom_features {
@@ -48,27 +65,30 @@ static int wacom_query_device(struct i2c_client *client,
 			      struct wacom_features *features)
 {
 	int ret;
-	u8 cmd1[] = { WACOM_CMD_QUERY0, WACOM_CMD_QUERY1,
-			WACOM_CMD_QUERY2, WACOM_CMD_QUERY3 };
-	u8 cmd2[] = { WACOM_CMD_THROW0, WACOM_CMD_THROW1 };
 	u8 data[WACOM_QUERY_SIZE];
+
+	u8 get_query_data_cmd[] = {
+		WACOM_COMMAND_LSB,
+		WACOM_COMMAND_MSB,
+		REPORT_FEATURE | WACOM_QUERY_REPORT,
+		OPCODE_GET_REPORT,
+		WACOM_DATA_LSB,
+		WACOM_DATA_MSB,
+	};
+
 	struct i2c_msg msgs[] = {
+		// Request reading of feature ReportID: 3 (Pen Query Data)
 		{
 			.addr = client->addr,
 			.flags = 0,
-			.len = sizeof(cmd1),
-			.buf = cmd1,
-		},
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = sizeof(cmd2),
-			.buf = cmd2,
+			.len = sizeof(get_query_data_cmd),
+			.buf = get_query_data_cmd,
 		},
+		// Read 21 bytes
 		{
 			.addr = client->addr,
 			.flags = I2C_M_RD,
-			.len = sizeof(data),
+			.len = WACOM_QUERY_SIZE - 1,
 			.buf = data,
 		},
 	};
@@ -89,9 +109,13 @@ static int wacom_query_device(struct i2c_client *client,
 	features->tilt_y_max = get_unaligned_le16(&data[19]);
 
 	dev_dbg(&client->dev,
-		"x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
+		"x_max:%d, y_max:%d, pressure:%d, fw:%d, "
+		"distance: %d, phys distance: %d, "
+		"tilt_x_max: %d, tilt_y_max: %d\n",
 		features->x_max, features->y_max,
-		features->pressure_max, features->fw_version);
+		features->pressure_max, features->fw_version,
+		features->distance_max, features->distance_physical_max,
+		features->tilt_x_max, features->tilt_y_max);
 
 	return 0;
 }
-- 
2.31.0

[PATCH v4 07/10] Input: wacom_i2c - Add support for reset control

[Alistair Francis]

From: Alistair Francis <alistair@alistair22.me>

Signed-off-by: Alistair Francis <alistair@alistair22.me>
---
v4:
 - Initial commit

 drivers/input/touchscreen/wacom_i2c.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 84c7ccb737bd..28004b1180c9 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -55,6 +55,7 @@ struct wacom_features {
 struct wacom_i2c {
 	struct i2c_client *client;
 	struct input_dev *input;
+	struct reset_control *rstc;
 	struct touchscreen_properties props;
 	u8 data[WACOM_QUERY_SIZE];
 	bool prox;
@@ -175,6 +176,8 @@ static int wacom_i2c_open(struct input_dev *dev)
 	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
 	struct i2c_client *client = wac_i2c->client;
 
+	reset_control_reset(wac_i2c->rstc);
+
 	enable_irq(client->irq);
 
 	return 0;
@@ -193,6 +196,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
 {
 	struct wacom_i2c *wac_i2c;
 	struct input_dev *input;
+	struct reset_control *rstc;
 	struct wacom_features features = { 0 };
 	int error;
 
@@ -201,6 +205,12 @@ static int wacom_i2c_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
+	rstc = devm_reset_control_get_optional_exclusive(&client->dev, NULL);
+	if (IS_ERR(rstc)) {
+		dev_err(&client->dev, "Failed to get reset control before init\n");
+		return PTR_ERR(rstc);
+	}
+
 	error = wacom_query_device(client, &features);
 	if (error)
 		return error;
@@ -214,6 +224,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
 
 	wac_i2c->client = client;
 	wac_i2c->input = input;
+	wac_i2c->rstc = rstc;
 
 	input->name = "Wacom I2C Digitizer";
 	input->id.bustype = BUS_I2C;
-- 
2.31.0

[PATCH v4 08/10] Input: wacom_i2c - Add support for vdd regulator

[Alistair Francis]

Add support for a VDD regulator. This allows the kernel to prove the
Wacom-I2C device on the rM2.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
v4:
 - Don't double allocate wac_i2c

 drivers/input/touchscreen/wacom_i2c.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 28004b1180c9..c78195b6b3b1 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -13,6 +13,7 @@
 #include <linux/irq.h>
 #include <linux/input/touchscreen.h>
 #include <linux/interrupt.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/of.h>
 #include <asm/unaligned.h>
@@ -57,6 +58,7 @@ struct wacom_i2c {
 	struct input_dev *input;
 	struct reset_control *rstc;
 	struct touchscreen_properties props;
+	struct regulator *vdd;
 	u8 data[WACOM_QUERY_SIZE];
 	bool prox;
 	int tool;
@@ -222,6 +224,20 @@ static int wacom_i2c_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}
 
+	wac_i2c->vdd = regulator_get(&client->dev, "vdd");
+	if (IS_ERR(wac_i2c->vdd)) {
+		error = PTR_ERR(wac_i2c->vdd);
+		kfree(wac_i2c);
+		return error;
+	}
+
+	error = regulator_enable(wac_i2c->vdd);
+	if (error) {
+		regulator_put(wac_i2c->vdd);
+		kfree(wac_i2c);
+		return error;
+	}
+
 	wac_i2c->client = client;
 	wac_i2c->input = input;
 	wac_i2c->rstc = rstc;
@@ -281,6 +297,8 @@ static int wacom_i2c_probe(struct i2c_client *client,
 err_free_irq:
 	free_irq(client->irq, wac_i2c);
 err_free_mem:
+	regulator_disable(wac_i2c->vdd);
+	regulator_put(wac_i2c->vdd);
 	input_free_device(input);
 	kfree(wac_i2c);
 
-- 
2.31.0

[PATCH v4 09/10] ARM: imx_v6_v7_defconfig: Enable Wacom I2C

[Alistair Francis]

Enable the Wacom I2C in the imx defconfig as it is used by the
reMarkable2 tablet.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm/configs/imx_v6_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 70928cc48939..cd80e85d37cf 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -174,6 +174,7 @@ CONFIG_TOUCHSCREEN_DA9052=y
 CONFIG_TOUCHSCREEN_EGALAX=y
 CONFIG_TOUCHSCREEN_GOODIX=y
 CONFIG_TOUCHSCREEN_ILI210X=y
+CONFIG_TOUCHSCREEN_WACOM_I2C=y
 CONFIG_TOUCHSCREEN_MAX11801=y
 CONFIG_TOUCHSCREEN_IMX6UL_TSC=y
 CONFIG_TOUCHSCREEN_EDT_FT5X06=y
-- 
2.31.0

Re: [PATCH v4 04/10] Input: wacom_i2c - Add touchscren properties

[Dmitry Torokhov]

On Thu, Mar 25, 2021 at 09:52:24PM -0400, Alistair Francis wrote:
> Connect touchscreen properties to the wacom_i2c.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
> v4:
>  - Add touchscreen_report_pos() as well
> 
>  drivers/input/touchscreen/wacom_i2c.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index eada68770671..ee1829dd35f4 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -11,6 +11,7 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/irq.h>
> +#include <linux/input/touchscreen.h>
>  #include <linux/interrupt.h>
>  #include <linux/of.h>
>  #include <asm/unaligned.h>
> @@ -33,6 +34,7 @@ struct wacom_features {
>  struct wacom_i2c {
>  	struct i2c_client *client;
>  	struct input_dev *input;
> +	struct touchscreen_properties props;
>  	u8 data[WACOM_QUERY_SIZE];
>  	bool prox;
>  	int tool;
> @@ -188,6 +190,9 @@ static int wacom_i2c_probe(struct i2c_client *client,
>  	__set_bit(BTN_STYLUS2, input->keybit);
>  	__set_bit(BTN_TOUCH, input->keybit);
>  
> +	touchscreen_parse_properties(input, true, &wac_i2c->props);
> +	touchscreen_report_pos(input, &wac_i2c->props, features.x_max,
> +			       features.y_max, true);

??? This goes into wacom_i2c_irq() where it previously used
input_report_abs() for X and Y so that transformations (swap, mirrot)
requested via device properties are applied to the coordinates.

Thanks.

-- 
Dmitry

Re: [PATCH v4 03/10] Input: wacom_i2c - Add device tree support to wacom_i2c

[Dmitry Torokhov]

Hi Alistair,

On Thu, Mar 25, 2021 at 09:52:23PM -0400, Alistair Francis wrote:
> Allow the wacom-i2c device to be exposed via device tree.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
> v4:
>  - Avoid unused variable warning by not using of_match_ptr()
> 
>  drivers/input/touchscreen/wacom_i2c.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index 1afc6bde2891..eada68770671 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/irq.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <asm/unaligned.h>
>  
>  #define WACOM_CMD_QUERY0	0x04
> @@ -262,10 +263,17 @@ static const struct i2c_device_id wacom_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
>  
> +static const struct of_device_id wacom_i2c_of_match_table[] = {
> +	{ .compatible = "wacom,generic" },

No, "generic" is not something we want in device tree binding. What is
the version of the controller used in your device? Put it instead of
"generic". Or if you know the earliest model with this protocol then it
can be used.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, wacom_i2c_of_match_table);
> +
>  static struct i2c_driver wacom_i2c_driver = {
>  	.driver	= {
>  		.name	= "wacom_i2c",
>  		.pm	= &wacom_i2c_pm,
> +		.of_match_table = wacom_i2c_of_match_table,
>  	},
>  
>  	.probe		= wacom_i2c_probe,
> -- 
> 2.31.0
> 

Thanks.

-- 
Dmitry

Re: [PATCH v4 05/10] Input: wacom_i2c - Add support for distance and tilt x/y

[Dmitry Torokhov]

On Thu, Mar 25, 2021 at 09:52:25PM -0400, Alistair Francis wrote:
> This is based on the out of tree rM2 driver.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  drivers/input/touchscreen/wacom_i2c.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index ee1829dd35f4..3b4bc514dc3f 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -22,12 +22,16 @@
>  #define WACOM_CMD_QUERY3	0x02
>  #define WACOM_CMD_THROW0	0x05
>  #define WACOM_CMD_THROW1	0x00
> -#define WACOM_QUERY_SIZE	19
> +#define WACOM_QUERY_SIZE	22
>  
>  struct wacom_features {
>  	int x_max;
>  	int y_max;
>  	int pressure_max;
> +	int distance_max;
> +	int distance_physical_max;
> +	int tilt_x_max;
> +	int tilt_y_max;
>  	char fw_version;
>  };
>  
> @@ -79,6 +83,10 @@ static int wacom_query_device(struct i2c_client *client,
>  	features->y_max = get_unaligned_le16(&data[5]);
>  	features->pressure_max = get_unaligned_le16(&data[11]);
>  	features->fw_version = get_unaligned_le16(&data[13]);
> +	features->distance_max = data[15];
> +	features->distance_physical_max = data[16];
> +	features->tilt_x_max = get_unaligned_le16(&data[17]);
> +	features->tilt_y_max = get_unaligned_le16(&data[19]);

Do other models also support distance and tilt? If not, this needs to be
made conditional.

Thanks.


-- 
Dmitry

Re: [PATCH v4 07/10] Input: wacom_i2c - Add support for reset control

[Dmitry Torokhov]

Hi Alistair,

On Thu, Mar 25, 2021 at 09:52:27PM -0400, Alistair Francis wrote:
> From: Alistair Francis <alistair@alistair22.me>
> 
> Signed-off-by: Alistair Francis <alistair@alistair22.me>
> ---
> v4:
>  - Initial commit
> 
>  drivers/input/touchscreen/wacom_i2c.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index 84c7ccb737bd..28004b1180c9 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -55,6 +55,7 @@ struct wacom_features {
>  struct wacom_i2c {
>  	struct i2c_client *client;
>  	struct input_dev *input;
> +	struct reset_control *rstc;
>  	struct touchscreen_properties props;
>  	u8 data[WACOM_QUERY_SIZE];
>  	bool prox;
> @@ -175,6 +176,8 @@ static int wacom_i2c_open(struct input_dev *dev)
>  	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
>  	struct i2c_client *client = wac_i2c->client;
>  
> +	reset_control_reset(wac_i2c->rstc);

Why does this device need to be reset on every open compared to doing it
in probe?

> +
>  	enable_irq(client->irq);
>  
>  	return 0;
> @@ -193,6 +196,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
>  {
>  	struct wacom_i2c *wac_i2c;
>  	struct input_dev *input;
> +	struct reset_control *rstc;
>  	struct wacom_features features = { 0 };
>  	int error;
>  
> @@ -201,6 +205,12 @@ static int wacom_i2c_probe(struct i2c_client *client,
>  		return -EIO;
>  	}
>  
> +	rstc = devm_reset_control_get_optional_exclusive(&client->dev, NULL);
> +	if (IS_ERR(rstc)) {
> +		dev_err(&client->dev, "Failed to get reset control before init\n");
> +		return PTR_ERR(rstc);
> +	}

I think majority users will have this controller reset line connected to
a GPIO. I briefly looked into reset controller code and I do not see it
supporting this case. How is this device connected on your board?

> +
>  	error = wacom_query_device(client, &features);
>  	if (error)
>  		return error;
> @@ -214,6 +224,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
>  
>  	wac_i2c->client = client;
>  	wac_i2c->input = input;
> +	wac_i2c->rstc = rstc;
>  
>  	input->name = "Wacom I2C Digitizer";
>  	input->id.bustype = BUS_I2C;
> -- 
> 2.31.0
> 

Thanks.

-- 
Dmitry

Re: [PATCH v4 04/10] Input: wacom_i2c - Add touchscren properties

[Alistair Francis]

 On Mon, Mar 29, 2021 at 3:08 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Thu, Mar 25, 2021 at 09:52:24PM -0400, Alistair Francis wrote:
> > Connect touchscreen properties to the wacom_i2c.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > ---
> > v4:
> >  - Add touchscreen_report_pos() as well
> >
> >  drivers/input/touchscreen/wacom_i2c.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> > index eada68770671..ee1829dd35f4 100644
> > --- a/drivers/input/touchscreen/wacom_i2c.c
> > +++ b/drivers/input/touchscreen/wacom_i2c.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/slab.h>
> >  #include <linux/irq.h>
> > +#include <linux/input/touchscreen.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/of.h>
> >  #include <asm/unaligned.h>
> > @@ -33,6 +34,7 @@ struct wacom_features {
> >  struct wacom_i2c {
> >       struct i2c_client *client;
> >       struct input_dev *input;
> > +     struct touchscreen_properties props;
> >       u8 data[WACOM_QUERY_SIZE];
> >       bool prox;
> >       int tool;
> > @@ -188,6 +190,9 @@ static int wacom_i2c_probe(struct i2c_client *client,
> >       __set_bit(BTN_STYLUS2, input->keybit);
> >       __set_bit(BTN_TOUCH, input->keybit);
> >
> > +     touchscreen_parse_properties(input, true, &wac_i2c->props);
> > +     touchscreen_report_pos(input, &wac_i2c->props, features.x_max,
> > +                            features.y_max, true);
>
> ??? This goes into wacom_i2c_irq() where it previously used
> input_report_abs() for X and Y so that transformations (swap, mirrot)
> requested via device properties are applied to the coordinates.

Ah sorry. I misunderstood what touchscreen_report_pos() does (and
didn't read it).

Looking at the actual code it seems that I need to remove

input_report_abs(input, ABS_Y, y);
input_report_abs(input, ABS_X, x);

from wacom_i2c_irq() and add touchscreen_report_pos() to wacom_i2c_irq() instead

I'll do that in the next version.

Alistair

>
> Thanks.
>
> --
> Dmitry

Re: [PATCH v4 07/10] Input: wacom_i2c - Add support for reset control

[Alistair Francis]

On Tue, Mar 30, 2021 at 6:33 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Alistair,
>
> On Thu, Mar 25, 2021 at 09:52:27PM -0400, Alistair Francis wrote:
> > From: Alistair Francis <alistair@alistair22.me>
> >
> > Signed-off-by: Alistair Francis <alistair@alistair22.me>
> > ---
> > v4:
> >  - Initial commit
> >
> >  drivers/input/touchscreen/wacom_i2c.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> > index 84c7ccb737bd..28004b1180c9 100644
> > --- a/drivers/input/touchscreen/wacom_i2c.c
> > +++ b/drivers/input/touchscreen/wacom_i2c.c
> > @@ -55,6 +55,7 @@ struct wacom_features {
> >  struct wacom_i2c {
> >       struct i2c_client *client;
> >       struct input_dev *input;
> > +     struct reset_control *rstc;
> >       struct touchscreen_properties props;
> >       u8 data[WACOM_QUERY_SIZE];
> >       bool prox;
> > @@ -175,6 +176,8 @@ static int wacom_i2c_open(struct input_dev *dev)
> >       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> >       struct i2c_client *client = wac_i2c->client;
> >
> > +     reset_control_reset(wac_i2c->rstc);
>
> Why does this device need to be reset on every open compared to doing it
> in probe?
>
> > +
> >       enable_irq(client->irq);
> >
> >       return 0;
> > @@ -193,6 +196,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
> >  {
> >       struct wacom_i2c *wac_i2c;
> >       struct input_dev *input;
> > +     struct reset_control *rstc;
> >       struct wacom_features features = { 0 };
> >       int error;
> >
> > @@ -201,6 +205,12 @@ static int wacom_i2c_probe(struct i2c_client *client,
> >               return -EIO;
> >       }
> >
> > +     rstc = devm_reset_control_get_optional_exclusive(&client->dev, NULL);
> > +     if (IS_ERR(rstc)) {
> > +             dev_err(&client->dev, "Failed to get reset control before init\n");
> > +             return PTR_ERR(rstc);
> > +     }
>
> I think majority users will have this controller reset line connected to
> a GPIO. I briefly looked into reset controller code and I do not see it
> supporting this case. How is this device connected on your board?

That's a good question. I am going to drop this patch as I'm not
convinced it's required.

Alistair

>
> > +
> >       error = wacom_query_device(client, &features);
> >       if (error)
> >               return error;
> > @@ -214,6 +224,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
> >
> >       wac_i2c->client = client;
> >       wac_i2c->input = input;
> > +     wac_i2c->rstc = rstc;
> >
> >       input->name = "Wacom I2C Digitizer";
> >       input->id.bustype = BUS_I2C;
> > --
> > 2.31.0
> >
>
> Thanks.
>
> --
> Dmitry