All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Add Wacom I2C support to rM2
@ 2021-01-21  6:56 Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c Alistair Francis
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, Alistair Francis

Add support to the reMarkable2 (rM2) for the Wacom I2C device.

This is based on the reMarkable Linux fork and with this series I am
able to probe the Wacom digitiser.

Alistair Francis (6):
  devicetree/bindings: Initial commit of wacom,wacom-i2c
  input/touchscreen: Add device tree support to wacom_i2c
  touchscreen/wacom_i2c: Add support for distance and tilt x/y
  touchscreen/wacom_i2c: Clean up the query device fields
  touchscreen/wacom_i2c: Add support for vdd regulator
  arch/arm: reMarkable2: Enable wacom_i2c

 .../input/touchscreen/wacom,wacom-i2c.yaml    |  48 +++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm/boot/dts/imx7d-remarkable2.dts       |  41 ++++++
 arch/arm/configs/imx_v6_v7_defconfig          |   1 +
 drivers/input/touchscreen/wacom_i2c.c         | 136 +++++++++++++++---
 5 files changed, 205 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml

-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-22 15:49   ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom, wacom-i2c Marco Felsch
  2021-01-21  6:56 ` [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c Alistair Francis
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, Alistair Francis

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../input/touchscreen/wacom,wacom-i2c.yaml    | 44 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 2 files changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
new file mode 100644
index 000000000000..b36d22cd20a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/wacom,wacom-i2c.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,wacom-i2c
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    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,wacom-i2c";
+                reg = <0x9>;
+                interrupt-parent = <&gpio1>;
+                interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 041ae90b0d8f..5bca22f035a3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1202,6 +1202,8 @@ patternProperties:
     description: Vision Optical Technology Co., Ltd.
   "^vxt,.*":
     description: VXT Ltd
+  "^wacom,.*":
+    description: Wacom Co., Ltd
   "^wand,.*":
     description: Wandbord (Technexion)
   "^waveshare,.*":
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-22 15:57   ` Marco Felsch
  2021-01-21  6:56 ` [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y Alistair Francis
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, Alistair Francis

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

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../input/touchscreen/wacom,wacom-i2c.yaml       |  4 ++++
 drivers/input/touchscreen/wacom_i2c.c            | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
index b36d22cd20a2..06ad5ee561af 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
@@ -22,6 +22,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  vdd-supply:
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -40,5 +43,6 @@ examples:
                 reg = <0x9>;
                 interrupt-parent = <&gpio1>;
                 interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+                vdd-supply = <&reg_touch>;
         };
     };
diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 1afc6bde2891..ec6e0aff8deb 100644
--- a/drivers/input/touchscreen/wacom_i2c.c
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -11,7 +11,9 @@
 #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>
 
 #define WACOM_CMD_QUERY0	0x04
@@ -32,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;
@@ -187,6 +190,7 @@ 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);
 	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,
@@ -214,6 +218,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
 	}
 
 	i2c_set_clientdata(client, wac_i2c);
+
 	return 0;
 
 err_free_irq:
@@ -262,10 +267,21 @@ static const struct i2c_device_id wacom_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id wacom_i2c_of_match_table[] = {
+	{ .compatible = "wacom,wacom-i2c" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, wacom_i2c_of_match_table);
+#endif
+
 static struct i2c_driver wacom_i2c_driver = {
 	.driver	= {
 		.name	= "wacom_i2c",
 		.pm	= &wacom_i2c_pm,
+#ifdef CONFIG_OF
+		.of_match_table = of_match_ptr(wacom_i2c_of_match_table),
+#endif
 	},
 
 	.probe		= wacom_i2c_probe,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-22 16:00   ` Marco Felsch
  2021-01-21  6:56 ` [PATCH v2 4/6] touchscreen/wacom_i2c: Clean up the query device fields Alistair Francis
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, 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 ec6e0aff8deb..5f0b80d52ad5 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:
@@ -195,7 +212,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.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/6] touchscreen/wacom_i2c: Clean up the query device fields
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
                   ` (2 preceding siblings ...)
  2021-01-21  6:56 ` [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 5/6] touchscreen/wacom_i2c: Add support for vdd regulator Alistair Francis
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, Alistair Francis

Improve the query device fields to be more verbose.

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

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index 5f0b80d52ad5..a22570adc939 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 {
@@ -45,34 +62,44 @@ struct wacom_i2c {
 };
 
 static int wacom_query_device(struct i2c_client *client,
-			      struct wacom_features *features)
+				  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];
+	struct reset_control *rstc;
+
+	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,
 		},
 	};
 
+	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");
+	else
+		reset_control_reset(rstc);
+
 	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
 	if (ret < 0)
 		return ret;
@@ -89,9 +116,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.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/6] touchscreen/wacom_i2c: Add support for vdd regulator
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
                   ` (3 preceding siblings ...)
  2021-01-21  6:56 ` [PATCH v2 4/6] touchscreen/wacom_i2c: Clean up the query device fields Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-21  6:56 ` [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c Alistair Francis
  2021-01-22 16:14 ` [PATCH v2 0/6] Add Wacom I2C support to rM2 Marco Felsch
  6 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, 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>
---
 drivers/input/touchscreen/wacom_i2c.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
index a22570adc939..6d85a7bd0502 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>
@@ -56,6 +57,7 @@ struct wacom_i2c {
 	struct i2c_client *client;
 	struct input_dev *input;
 	struct touchscreen_properties props;
+	struct regulator *vdd;
 	u8 data[WACOM_QUERY_SIZE];
 	bool prox;
 	int tool;
@@ -203,11 +205,29 @@ static int wacom_i2c_probe(struct i2c_client *client,
 	struct wacom_features features = { 0 };
 	int error;
 
+	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
+	if (!wac_i2c)
+		return -ENOMEM;
+
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		dev_err(&client->dev, "i2c_check_functionality error\n");
 		return -EIO;
 	}
 
+	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;
+	}
+
 	error = wacom_query_device(client, &features);
 	if (error)
 		return error;
@@ -276,6 +296,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.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
                   ` (4 preceding siblings ...)
  2021-01-21  6:56 ` [PATCH v2 5/6] touchscreen/wacom_i2c: Add support for vdd regulator Alistair Francis
@ 2021-01-21  6:56 ` Alistair Francis
  2021-01-22 15:32   ` Marco Felsch
  2021-01-22 16:14 ` [PATCH v2 0/6] Add Wacom I2C support to rM2 Marco Felsch
  6 siblings, 1 reply; 13+ messages in thread
From: Alistair Francis @ 2021-01-21  6:56 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-imx, kernel
  Cc: linux-kernel, alistair23, Alistair Francis

Enable the wacom_i2c touchscreen for the reMarkable2.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm/boot/dts/imx7d-remarkable2.dts | 41 +++++++++++++++++++++++++
 arch/arm/configs/imx_v6_v7_defconfig    |  1 +
 2 files changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
index fba55a0e028a..8052d884a5e5 100644
--- a/arch/arm/boot/dts/imx7d-remarkable2.dts
+++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
@@ -150,6 +150,30 @@ &dma_apbh {
 	status = "disabled";
 };
 
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	pinctrl-1 = <&pinctrl_i2c1>;
+	status = "okay";
+
+	digitizer: wacom-i2c@9 {
+		pinctrl-names = "default", "sleep";
+		pinctrl-0 = <&pinctrl_wacom>;
+		pinctrl-1 = <&pinctrl_wacom>;
+		compatible = "wacom,wacom-i2c";
+		reg = <0x09>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <1 2>;
+		flip-tilt-x;
+		flip-tilt-y;
+		flip-pos-x;
+		flip-pos-y;
+		flip-distance;
+		vdd-supply = <&reg_digitizer>;
+	};
+};
+
 &sdma {
 	status = "okay";
 };
@@ -221,6 +245,16 @@ &wdog1 {
 };
 
 &iomuxc_lpsr {
+	pinctrl_wacom: wacomgrp {
+		fsl,pins = <
+			/*MX7D_PAD_LPSR_GPIO1_IO00__GPIO1_IO0	0x00000074 /* WACOM RESET */
+			MX7D_PAD_LPSR_GPIO1_IO01__GPIO1_IO1	0x00000034 /* WACOM INT */
+			MX7D_PAD_LPSR_GPIO1_IO04__GPIO1_IO4	0x00000074 /* PDCTB */
+			/*MX7D_PAD_LPSR_GPIO1_IO05__GPIO1_IO5	0x00000014 /* FWE */
+			/*MX7D_PAD_LPSR_GPIO1_IO06__GPIO1_IO6	0x00000014 /* WACOM PWR ENABLE */
+		>;
+	};
+
 	pinctrl_digitizer_reg: digitizerreggrp {
 		fsl,pins = <
 			/* DIGITIZER_PWR_EN */
@@ -236,6 +270,13 @@ MX7D_PAD_SAI1_RX_SYNC__GPIO6_IO16	0x59
 		>;
 	};
 
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX7D_PAD_I2C1_SDA__I2C1_SDA		0x4000007f
+			MX7D_PAD_I2C1_SCL__I2C1_SCL		0x4000007f
+		>;
+	};
+
 	pinctrl_uart1: uart1grp {
 		fsl,pins = <
 			MX7D_PAD_UART1_TX_DATA__UART1_DCE_TX	0x79
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index fa9229616106..2fc8dc6a8b0a 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -167,6 +167,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.29.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c
  2021-01-21  6:56 ` [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c Alistair Francis
@ 2021-01-22 15:32   ` Marco Felsch
  2021-01-23  8:04     ` Alistair Francis
  0 siblings, 1 reply; 13+ messages in thread
From: Marco Felsch @ 2021-01-22 15:32 UTC (permalink / raw)
  To: Alistair Francis
  Cc: dmitry.torokhov, linux-input, linux-imx, kernel, alistair23,
	linux-kernel

Hi,

thanks for the patch.

On 21-01-20 22:56, Alistair Francis wrote:
> Enable the wacom_i2c touchscreen for the reMarkable2.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  arch/arm/boot/dts/imx7d-remarkable2.dts | 41 +++++++++++++++++++++++++
>  arch/arm/configs/imx_v6_v7_defconfig    |  1 +

Those two changes should be splitted and the dts patch should be named:
"ARM: dts: imx7d: remarkable2: add wacom digitizer device".

>  2 files changed, 42 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
> index fba55a0e028a..8052d884a5e5 100644
> --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> @@ -150,6 +150,30 @@ &dma_apbh {
>  	status = "disabled";
>  };
>  
> +&i2c1 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1>;

No need to specify the sleep state if both are using the same pinctrl
config.

> +	status = "okay";
> +
> +	digitizer: wacom-i2c@9 {

this should be:
        wacom_digitizer: digitizer@9 {

> +		pinctrl-names = "default", "sleep";
> +		pinctrl-0 = <&pinctrl_wacom>;
> +		pinctrl-1 = <&pinctrl_wacom>;

Same here, sleep and default refer to the same state.

> +		compatible = "wacom,wacom-i2c";
> +		reg = <0x09>;

compatible and reg are always the first to properties.

> +		interrupt-parent = <&gpio1>;
> +		interrupts = <1 2>;

Please use defines.

> +		flip-tilt-x;
> +		flip-tilt-y;
> +		flip-pos-x;
> +		flip-pos-y;
> +		flip-distance;
> +		vdd-supply = <&reg_digitizer>;

Where is this regulator added?

> +	};
> +};
> +
>  &sdma {
>  	status = "okay";
>  };
> @@ -221,6 +245,16 @@ &wdog1 {
>  };
>  
>  &iomuxc_lpsr {
> +	pinctrl_wacom: wacomgrp {
> +		fsl,pins = <
> +			/*MX7D_PAD_LPSR_GPIO1_IO00__GPIO1_IO0	0x00000074 /* WACOM RESET */
> +			MX7D_PAD_LPSR_GPIO1_IO01__GPIO1_IO1	0x00000034 /* WACOM INT */
> +			MX7D_PAD_LPSR_GPIO1_IO04__GPIO1_IO4	0x00000074 /* PDCTB */
> +			/*MX7D_PAD_LPSR_GPIO1_IO05__GPIO1_IO5	0x00000014 /* FWE */
> +			/*MX7D_PAD_LPSR_GPIO1_IO06__GPIO1_IO6	0x00000014 /* WACOM PWR ENABLE */
> +		>;
> +	};

Pls, sort this alphabetical.

> +
>  	pinctrl_digitizer_reg: digitizerreggrp {
>  		fsl,pins = <
>  			/* DIGITIZER_PWR_EN */
> @@ -236,6 +270,13 @@ MX7D_PAD_SAI1_RX_SYNC__GPIO6_IO16	0x59
>  		>;
>  	};
>  
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX7D_PAD_I2C1_SDA__I2C1_SDA		0x4000007f
> +			MX7D_PAD_I2C1_SCL__I2C1_SCL		0x4000007f
> +		>;
> +	};
> +
>  	pinctrl_uart1: uart1grp {
>  		fsl,pins = <
>  			MX7D_PAD_UART1_TX_DATA__UART1_DCE_TX	0x79
> diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
> index fa9229616106..2fc8dc6a8b0a 100644
> --- a/arch/arm/configs/imx_v6_v7_defconfig
> +++ b/arch/arm/configs/imx_v6_v7_defconfig
> @@ -167,6 +167,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.29.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom, wacom-i2c
  2021-01-21  6:56 ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c Alistair Francis
@ 2021-01-22 15:49   ` Marco Felsch
  0 siblings, 0 replies; 13+ messages in thread
From: Marco Felsch @ 2021-01-22 15:49 UTC (permalink / raw)
  To: Alistair Francis
  Cc: dmitry.torokhov, linux-input, linux-imx, kernel, alistair23,
	linux-kernel

Hi,

thnaks for the patch.

On 21-01-20 22:56, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  .../input/touchscreen/wacom,wacom-i2c.yaml    | 44 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +

This should be splitted into two patches.

Regards,
  Marco


>  2 files changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> new file mode 100644
> index 000000000000..b36d22cd20a2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/wacom,wacom-i2c.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,wacom-i2c
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    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,wacom-i2c";
> +                reg = <0x9>;
> +                interrupt-parent = <&gpio1>;
> +                interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 041ae90b0d8f..5bca22f035a3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1202,6 +1202,8 @@ patternProperties:
>      description: Vision Optical Technology Co., Ltd.
>    "^vxt,.*":
>      description: VXT Ltd
> +  "^wacom,.*":
> +    description: Wacom Co., Ltd
>    "^wand,.*":
>      description: Wandbord (Technexion)
>    "^waveshare,.*":
> -- 
> 2.29.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c
  2021-01-21  6:56 ` [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c Alistair Francis
@ 2021-01-22 15:57   ` Marco Felsch
  0 siblings, 0 replies; 13+ messages in thread
From: Marco Felsch @ 2021-01-22 15:57 UTC (permalink / raw)
  To: Alistair Francis
  Cc: dmitry.torokhov, linux-input, linux-imx, kernel, alistair23,
	linux-kernel

On 21-01-20 22:56, Alistair Francis wrote:
> Allow the wacom-i2c device to be exposed via device tree.

You did a lot more than exposing.

> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  .../input/touchscreen/wacom,wacom-i2c.yaml       |  4 ++++
>  drivers/input/touchscreen/wacom_i2c.c            | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> index b36d22cd20a2..06ad5ee561af 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> @@ -22,6 +22,9 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  vdd-supply:
> +    maxItems: 1
> +

Unrelated change.

>  required:
>    - compatible
>    - reg
> @@ -40,5 +43,6 @@ examples:
>                  reg = <0x9>;
>                  interrupt-parent = <&gpio1>;
>                  interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> +                vdd-supply = <&reg_touch>;

Dito.

>          };
>      };
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> index 1afc6bde2891..ec6e0aff8deb 100644
> --- a/drivers/input/touchscreen/wacom_i2c.c
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -11,7 +11,9 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/irq.h>
> +#include <linux/input/touchscreen.h>

Unrelated change.

>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <asm/unaligned.h>
>  
>  #define WACOM_CMD_QUERY0	0x04
> @@ -32,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;
> @@ -187,6 +190,7 @@ 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);

Unrelated change, please move it into a sepreate patch.

>  	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,
> @@ -214,6 +218,7 @@ static int wacom_i2c_probe(struct i2c_client *client,
>  	}
>  
>  	i2c_set_clientdata(client, wac_i2c);
> +

Unrelated change.

>  	return 0;
>  
>  err_free_irq:
> @@ -262,10 +267,21 @@ static const struct i2c_device_id wacom_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
>  
> +#ifdef CONFIG_OF

This #ifdef can be removed using the __maybe_unused macro.

> +static const struct of_device_id wacom_i2c_of_match_table[] = {
> +	{ .compatible = "wacom,wacom-i2c" },

IMHO "wacom,wacom-i2c" is not good maybe:
 - "wacom,generic" if you don't know the device, or
 - "wacom,XYZ" where XYZ belongs to the real device name.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, wacom_i2c_of_match_table);
> +#endif
> +
>  static struct i2c_driver wacom_i2c_driver = {
>  	.driver	= {
>  		.name	= "wacom_i2c",
>  		.pm	= &wacom_i2c_pm,
> +#ifdef CONFIG_OF
> +		.of_match_table = of_match_ptr(wacom_i2c_of_match_table),
> +#endif

No need for this #ifdef.

Regards,
  Marco


>  	},
>  
>  	.probe		= wacom_i2c_probe,
> -- 
> 2.29.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y
  2021-01-21  6:56 ` [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y Alistair Francis
@ 2021-01-22 16:00   ` Marco Felsch
  0 siblings, 0 replies; 13+ messages in thread
From: Marco Felsch @ 2021-01-22 16:00 UTC (permalink / raw)
  To: Alistair Francis
  Cc: dmitry.torokhov, linux-input, linux-imx, kernel, alistair23,
	linux-kernel

Hi,

thanks for the patch. Please align all your patch-subjects belonging to
the driver to: "Input: wacom_i2c - ..."

On 21-01-20 22:56, 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 ec6e0aff8deb..5f0b80d52ad5 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:
> @@ -195,7 +212,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.29.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] Add Wacom I2C support to rM2
  2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
                   ` (5 preceding siblings ...)
  2021-01-21  6:56 ` [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c Alistair Francis
@ 2021-01-22 16:14 ` Marco Felsch
  6 siblings, 0 replies; 13+ messages in thread
From: Marco Felsch @ 2021-01-22 16:14 UTC (permalink / raw)
  To: Alistair Francis
  Cc: dmitry.torokhov, linux-input, linux-imx, kernel, alistair23,
	linux-kernel

Hi Alistair,

thanks for the patches. Before getting into the series please check all
your commit subjects. Also please check that you do only one logical
change per patch: e.g. adding DT-Support means: Add the dt-table and not
more.

I looking forward to your v2 :)

Regards,
  Marco

On 21-01-20 22:56, Alistair Francis wrote:
> Add support to the reMarkable2 (rM2) for the Wacom I2C device.
> 
> This is based on the reMarkable Linux fork and with this series I am
> able to probe the Wacom digitiser.
> 
> Alistair Francis (6):
>   devicetree/bindings: Initial commit of wacom,wacom-i2c
>   input/touchscreen: Add device tree support to wacom_i2c
>   touchscreen/wacom_i2c: Add support for distance and tilt x/y
>   touchscreen/wacom_i2c: Clean up the query device fields
>   touchscreen/wacom_i2c: Add support for vdd regulator
>   arch/arm: reMarkable2: Enable wacom_i2c
> 
>  .../input/touchscreen/wacom,wacom-i2c.yaml    |  48 +++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  arch/arm/boot/dts/imx7d-remarkable2.dts       |  41 ++++++
>  arch/arm/configs/imx_v6_v7_defconfig          |   1 +
>  drivers/input/touchscreen/wacom_i2c.c         | 136 +++++++++++++++---
>  5 files changed, 205 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,wacom-i2c.yaml
> 
> -- 
> 2.29.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c
  2021-01-22 15:32   ` Marco Felsch
@ 2021-01-23  8:04     ` Alistair Francis
  0 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2021-01-23  8:04 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Alistair Francis, Dmitry Torokhov, linux-input, dl-linux-imx,
	Sascha Hauer, Linux Kernel Mailing List

On Fri, Jan 22, 2021 at 7:32 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> Hi,
>
> thanks for the patch.
>
> On 21-01-20 22:56, Alistair Francis wrote:
> > Enable the wacom_i2c touchscreen for the reMarkable2.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > ---
> >  arch/arm/boot/dts/imx7d-remarkable2.dts | 41 +++++++++++++++++++++++++
> >  arch/arm/configs/imx_v6_v7_defconfig    |  1 +
>
> Those two changes should be splitted and the dts patch should be named:
> "ARM: dts: imx7d: remarkable2: add wacom digitizer device".
>
> >  2 files changed, 42 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > index fba55a0e028a..8052d884a5e5 100644
> > --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > @@ -150,6 +150,30 @@ &dma_apbh {
> >       status = "disabled";
> >  };
> >
> > +&i2c1 {
> > +     clock-frequency = <400000>;
> > +     pinctrl-names = "default", "sleep";
> > +     pinctrl-0 = <&pinctrl_i2c1>;
> > +     pinctrl-1 = <&pinctrl_i2c1>;
>
> No need to specify the sleep state if both are using the same pinctrl
> config.
>
> > +     status = "okay";
> > +
> > +     digitizer: wacom-i2c@9 {
>
> this should be:
>         wacom_digitizer: digitizer@9 {
>
> > +             pinctrl-names = "default", "sleep";
> > +             pinctrl-0 = <&pinctrl_wacom>;
> > +             pinctrl-1 = <&pinctrl_wacom>;
>
> Same here, sleep and default refer to the same state.
>
> > +             compatible = "wacom,wacom-i2c";
> > +             reg = <0x09>;
>
> compatible and reg are always the first to properties.
>
> > +             interrupt-parent = <&gpio1>;
> > +             interrupts = <1 2>;
>
> Please use defines.

I have addressed all your comments.

>
> > +             flip-tilt-x;
> > +             flip-tilt-y;
> > +             flip-pos-x;
> > +             flip-pos-y;
> > +             flip-distance;
> > +             vdd-supply = <&reg_digitizer>;
>
> Where is this regulator added?

It's already in the DT, it will be added with the initial commit
(which is currently on the list).

Alistair

>
> > +     };
> > +};
> > +
> >  &sdma {
> >       status = "okay";
> >  };
> > @@ -221,6 +245,16 @@ &wdog1 {
> >  };
> >
> >  &iomuxc_lpsr {
> > +     pinctrl_wacom: wacomgrp {
> > +             fsl,pins = <
> > +                     /*MX7D_PAD_LPSR_GPIO1_IO00__GPIO1_IO0   0x00000074 /* WACOM RESET */
> > +                     MX7D_PAD_LPSR_GPIO1_IO01__GPIO1_IO1     0x00000034 /* WACOM INT */
> > +                     MX7D_PAD_LPSR_GPIO1_IO04__GPIO1_IO4     0x00000074 /* PDCTB */
> > +                     /*MX7D_PAD_LPSR_GPIO1_IO05__GPIO1_IO5   0x00000014 /* FWE */
> > +                     /*MX7D_PAD_LPSR_GPIO1_IO06__GPIO1_IO6   0x00000014 /* WACOM PWR ENABLE */
> > +             >;
> > +     };
>
> Pls, sort this alphabetical.
>
> > +
> >       pinctrl_digitizer_reg: digitizerreggrp {
> >               fsl,pins = <
> >                       /* DIGITIZER_PWR_EN */
> > @@ -236,6 +270,13 @@ MX7D_PAD_SAI1_RX_SYNC__GPIO6_IO16        0x59
> >               >;
> >       };
> >
> > +     pinctrl_i2c1: i2c1grp {
> > +             fsl,pins = <
> > +                     MX7D_PAD_I2C1_SDA__I2C1_SDA             0x4000007f
> > +                     MX7D_PAD_I2C1_SCL__I2C1_SCL             0x4000007f
> > +             >;
> > +     };
> > +
> >       pinctrl_uart1: uart1grp {
> >               fsl,pins = <
> >                       MX7D_PAD_UART1_TX_DATA__UART1_DCE_TX    0x79
> > diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
> > index fa9229616106..2fc8dc6a8b0a 100644
> > --- a/arch/arm/configs/imx_v6_v7_defconfig
> > +++ b/arch/arm/configs/imx_v6_v7_defconfig
> > @@ -167,6 +167,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.29.2
> >
> >
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-23  8:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21  6:56 [PATCH v2 0/6] Add Wacom I2C support to rM2 Alistair Francis
2021-01-21  6:56 ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom,wacom-i2c Alistair Francis
2021-01-22 15:49   ` [PATCH v2 1/6] devicetree/bindings: Initial commit of wacom, wacom-i2c Marco Felsch
2021-01-21  6:56 ` [PATCH v2 2/6] input/touchscreen: Add device tree support to wacom_i2c Alistair Francis
2021-01-22 15:57   ` Marco Felsch
2021-01-21  6:56 ` [PATCH v2 3/6] touchscreen/wacom_i2c: Add support for distance and tilt x/y Alistair Francis
2021-01-22 16:00   ` Marco Felsch
2021-01-21  6:56 ` [PATCH v2 4/6] touchscreen/wacom_i2c: Clean up the query device fields Alistair Francis
2021-01-21  6:56 ` [PATCH v2 5/6] touchscreen/wacom_i2c: Add support for vdd regulator Alistair Francis
2021-01-21  6:56 ` [PATCH v2 6/6] arch/arm: reMarkable2: Enable wacom_i2c Alistair Francis
2021-01-22 15:32   ` Marco Felsch
2021-01-23  8:04     ` Alistair Francis
2021-01-22 16:14 ` [PATCH v2 0/6] Add Wacom I2C support to rM2 Marco Felsch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.