[PATCH 0/2] novatek-nvt-ts: add support for NT36672A touchscreen

[PATCH 0/2] novatek-nvt-ts: add support for NT36672A touchscreen

[Joel Selvaraj via B4 Relay]

Extend the novatek touchscreen driver to support NT36672A chip which
is found in phones like Xiaomi Poco F1 [1]. Added devicetree support for
the driver and used i2c chip data to handle the variation in chip id and
wake type. Also added vcc and iovcc regulators which are used to power
the touchscreen hardware.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9

Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
---
Joel Selvaraj (2):
      dt-bindings: input: document Novatek NVT touchscreen controller
      Input: novatek-nvt-ts: add support for NT36672A touchscreen

 .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 +++++++++++++++++
 MAINTAINERS                                        |  1 +
 drivers/input/touchscreen/novatek-nvt-ts.c         | 78 ++++++++++++++++++++--
 3 files changed, 135 insertions(+), 6 deletions(-)
---
base-commit: 6578aac6a270bd6deb9f9319b991dd430de263dd
change-id: 20240518-nvt-ts-devicetree-regulator-support-ac9e49b78a16

Best regards,
-- 
Joel Selvaraj <joelselvaraj.oss@gmail.com>

[PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Joel Selvaraj via B4 Relay]

From: Joel Selvaraj <joelselvaraj.oss@gmail.com>

Document the Novatek NVT touchscreen driver which is used in devices like
the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
MAINTAINERS file.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9

Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
---
 .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 63 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
new file mode 100644
index 0000000000000..7839c6a028e4a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Novatek NVT Touchscreen Controller
+
+maintainers:
+  - Hans de Goede <hdegoede@redhat.com>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    enum:
+      - novatek,nvt-ts
+      - novatek,nt36672a-ts
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vcc-supply: true
+  iovcc-supply: true
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        touchscreen@1 {
+          compatible = "novatek,nt36672a-ts";
+          reg = <0x01>;
+          interrupts-extended = <&tlmm 31 IRQ_TYPE_EDGE_RISING>;
+          reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
+          vcc-supply = <&vreg_l22a_2p85>;
+          iovcc-supply = <&vreg_l14a_1p8>;
+          pinctrl-0 = <&ts_int_default &ts_reset_default>;
+          pinctrl-1 = <&ts_int_sleep &ts_reset_sleep>;
+          pinctrl-names = "default", "sleep";
+          touchscreen-size-x = <1080>;
+          touchscreen-size-y = <2246>;
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 90754a451bcfc..e1f744992b15f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15942,6 +15942,7 @@ NOVATEK NVT-TS I2C TOUCHSCREEN DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 L:	linux-input@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
 F:	drivers/input/touchscreen/novatek-nvt-ts.c
 
 NSDEPS

-- 
2.45.1

[PATCH 2/2] Input: novatek-nvt-ts: add support for NT36672A touchscreen

[Joel Selvaraj via B4 Relay]

From: Joel Selvaraj <joelselvaraj.oss@gmail.com>

Extend the novatek touchscreen driver to support NT36672A chip which
is found in phones like Xiaomi Poco F1 [1]. Added devicetree support for
the driver and used i2c chip data to handle the variation in chip id and
wake type. Also added vcc and iovcc regulators which are used to power
the touchscreen hardware.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9

Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
---
 drivers/input/touchscreen/novatek-nvt-ts.c | 78 +++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
index 1a797e410a3fa..83b881e14a0f3 100644
--- a/drivers/input/touchscreen/novatek-nvt-ts.c
+++ b/drivers/input/touchscreen/novatek-nvt-ts.c
@@ -31,9 +31,6 @@
 #define NVT_TS_PARAMS_CHIP_ID		0x0e
 #define NVT_TS_PARAMS_SIZE		0x0f
 
-#define NVT_TS_SUPPORTED_WAKE_TYPE	0x05
-#define NVT_TS_SUPPORTED_CHIP_ID	0x05
-
 #define NVT_TS_MAX_TOUCHES		10
 #define NVT_TS_MAX_SIZE			4096
 
@@ -51,11 +48,18 @@ static const int nvt_ts_irq_type[4] = {
 	IRQF_TRIGGER_HIGH
 };
 
+struct nvt_ts_i2c_chip_data {
+	u8 wake_type;
+	u8 chip_id;
+};
+
 struct nvt_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input;
 	struct gpio_desc *reset_gpio;
+	struct regulator_bulk_data regulators[2];
 	struct touchscreen_properties prop;
+	const struct nvt_ts_i2c_chip_data *chip;
 	int max_touches;
 	u8 buf[NVT_TS_TOUCH_SIZE * NVT_TS_MAX_TOUCHES];
 };
@@ -139,9 +143,23 @@ static irqreturn_t nvt_ts_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void nvt_ts_disable_regulators(void *_data)
+{
+	struct nvt_ts_data *data = _data;
+
+	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
+}
+
 static int nvt_ts_start(struct input_dev *dev)
 {
 	struct nvt_ts_data *data = input_get_drvdata(dev);
+	int error;
+
+	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), data->regulators);
+	if (error) {
+		dev_err(&data->client->dev, "failed to enable regulators\n");
+		return error;
+	}
 
 	enable_irq(data->client->irq);
 	gpiod_set_value_cansleep(data->reset_gpio, 0);
@@ -155,6 +173,7 @@ static void nvt_ts_stop(struct input_dev *dev)
 
 	disable_irq(data->client->irq);
 	gpiod_set_value_cansleep(data->reset_gpio, 1);
+	nvt_ts_disable_regulators(data);
 }
 
 static int nvt_ts_suspend(struct device *dev)
@@ -199,9 +218,37 @@ static int nvt_ts_probe(struct i2c_client *client)
 	if (!data)
 		return -ENOMEM;
 
+	data->chip = device_get_match_data(&client->dev);
+	if (!data->chip)
+		return -EINVAL;
+
 	data->client = client;
 	i2c_set_clientdata(client, data);
 
+	/*
+	 * VCC is the analog voltage supply
+	 * IOVCC is the digital voltage supply
+	 */
+	data->regulators[0].supply = "vcc";
+	data->regulators[1].supply = "iovcc";
+	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->regulators), data->regulators);
+	if (error) {
+		dev_err(dev, "cannot get regulators: %d\n", error);
+		return error;
+	}
+
+	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), data->regulators);
+	if (error) {
+		dev_err(dev, "failed to enable regulators\n");
+		return error;
+	}
+
+	error = devm_add_action_or_reset(dev, nvt_ts_disable_regulators, data);
+	if (error) {
+		dev_err(dev, "failed to install regulator disable handler\n");
+		return error;
+	}
+
 	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
 	error = PTR_ERR_OR_ZERO(data->reset_gpio);
 	if (error) {
@@ -225,8 +272,8 @@ static int nvt_ts_probe(struct i2c_client *client)
 	if (width > NVT_TS_MAX_SIZE || height >= NVT_TS_MAX_SIZE ||
 	    data->max_touches > NVT_TS_MAX_TOUCHES ||
 	    irq_type >= ARRAY_SIZE(nvt_ts_irq_type) ||
-	    data->buf[NVT_TS_PARAMS_WAKE_TYPE] != NVT_TS_SUPPORTED_WAKE_TYPE ||
-	    data->buf[NVT_TS_PARAMS_CHIP_ID] != NVT_TS_SUPPORTED_CHIP_ID) {
+	    data->buf[NVT_TS_PARAMS_WAKE_TYPE] != data->chip->wake_type ||
+	    data->buf[NVT_TS_PARAMS_CHIP_ID] != data->chip->chip_id) {
 		dev_err(dev, "Unsupported touchscreen parameters: %*ph\n",
 			NVT_TS_PARAMS_SIZE, data->buf);
 		return -EIO;
@@ -277,8 +324,26 @@ static int nvt_ts_probe(struct i2c_client *client)
 	return 0;
 }
 
+static const struct nvt_ts_i2c_chip_data nvt_ts_data = {
+	.wake_type = 0x05,
+	.chip_id = 0x05,
+};
+
+static const struct nvt_ts_i2c_chip_data nvt_nt36672a_ts_data = {
+	.wake_type = 0x01,
+	.chip_id = 0x08,
+};
+
+static const struct of_device_id nvt_ts_of_match[] = {
+	{ .compatible = "novatek,nvt-ts", .data = &nvt_ts_data },
+	{ .compatible = "novatek,nt36672a-ts", .data = &nvt_nt36672a_ts_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, nvt_ts_of_match);
+
 static const struct i2c_device_id nvt_ts_i2c_id[] = {
-	{ "NVT-ts" },
+	{ "NVT-ts", (unsigned long) &nvt_ts_data },
+	{ "NT36672A-ts", (unsigned long) &nvt_nt36672a_ts_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, nvt_ts_i2c_id);
@@ -287,6 +352,7 @@ static struct i2c_driver nvt_ts_driver = {
 	.driver = {
 		.name	= "novatek-nvt-ts",
 		.pm	= pm_sleep_ptr(&nvt_ts_pm_ops),
+		.of_match_table = nvt_ts_of_match,
 	},
 	.probe = nvt_ts_probe,
 	.id_table = nvt_ts_i2c_id,

-- 
2.45.1

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Krzysztof Kozlowski]

On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> 
> Document the Novatek NVT touchscreen driver which is used in devices like

driver? or device?

> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
> MAINTAINERS file.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
> 
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> ---
>  .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 63 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
> new file mode 100644
> index 0000000000000..7839c6a028e4a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Novatek NVT Touchscreen Controller
> +
> +maintainers:
> +  - Hans de Goede <hdegoede@redhat.com>
> +
> +allOf:
> +  - $ref: touchscreen.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - novatek,nvt-ts

That's too generic. Looking at your driver change, it is not even needed.

> +      - novatek,nt36672a-ts

Eh, we have already panel. Why there is a need for touchscreen binding
(binding, not driver)?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  vcc-supply: true
> +  iovcc-supply: true
> +
> +unevaluatedProperties: false

This goes after required:

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Joel Selvaraj]

Hi Krzysztof Kozlowski,

On 5/21/24 11:48, Krzysztof Kozlowski wrote:
> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>
>> Document the Novatek NVT touchscreen driver which is used in devices like
> 
> driver? or device?

touchscreen "controller" would be correct I think. I will fix it in v2.

>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>> MAINTAINERS file.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>>   .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  1 +
>>   2 files changed, 63 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> new file mode 100644
>> index 0000000000000..7839c6a028e4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Novatek NVT Touchscreen Controller
>> +
>> +maintainers:
>> +  - Hans de Goede <hdegoede@redhat.com>
>> +
>> +allOf:
>> +  - $ref: touchscreen.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - novatek,nvt-ts
> 
> That's too generic. Looking at your driver change, it is not even needed.
> 
>> +      - novatek,nt36672a-ts
> 
> Eh, we have already panel. Why there is a need for touchscreen binding
> (binding, not driver)?

I am not sure I understand this correctly. Help me a bit here. For 
context, in mainline there is an existing driver for the novatek nvt 
touchscreen controller. The driver did not have devicetree support. It 
only had a i2c_device_id "NVT-ts". I don't know what is the variant of 
that Novatek touchscreen controller. To use the driver in Xiaomi Poco 
F1, I introduced a devicetree compatible for it "novatek,nvt-ts". The 
However, the Novatek touchscreen controller present in Xiaomi Poco F1 is 
"NT36672A" which has a different chip id than the one in existing 
driver. So I created a separate compatible for this touchscreen 
controller variant "novatek,nt36672a-ts". I used compatible data to 
differentiate the two variants. Since there are two variants, I am 
mentioning both here.

Between, the chip_id and wake_type are the only values that changes 
between these two variants. And these are only checked during the probe 
and is not used anywhere else in the code. If we remove this sanity 
check during probing, then there is no need for two variants and we can 
just keep the generic "novatek,nvt-ts".

Kindly let me know what is the correct thing to do here? How this should 
be handled? I will be happy to address it in v2.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  vcc-supply: true
>> +  iovcc-supply: true
>> +
>> +unevaluatedProperties: false
> 
> This goes after required:

Will fix in v2.

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
> 
> 
> Best regards,
> Krzysztof

Regards,
Joel Selvaraj

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Hans de Goede]

Hi,

On 5/21/24 6:48 PM, Krzysztof Kozlowski wrote:
> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>
>> Document the Novatek NVT touchscreen driver which is used in devices like
> 
> driver? or device?
> 
>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>> MAINTAINERS file.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>>  .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  1 +
>>  2 files changed, 63 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> new file mode 100644
>> index 0000000000000..7839c6a028e4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Novatek NVT Touchscreen Controller
>> +
>> +maintainers:
>> +  - Hans de Goede <hdegoede@redhat.com>
>> +
>> +allOf:
>> +  - $ref: touchscreen.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - novatek,nvt-ts
> 
> That's too generic. Looking at your driver change, it is not even needed.

I wrote the novatek-nvt-ts driver for an Acer Iconia One 7 B1-750 tablet,
this is a x86 tablet which ships with Android as factory OS and because
Android OS images use kernels where everything is hardcoded the ACPI tables
do not describe this touchscreen. Instead the i2c_client for the touchscreen
is manually instantiated by some x86 platform glue code. Since it is
manually instantiated it uses i2c_device_id binding rather then OF/ACPI.

The generic "NVT-ts" i2c_device_id comes from me not knowing the controller
panel type back then. In the mean time I have learned that the B1-750 uses
NVT-NT11205 controller.

So what I think needs to happen here is add a preparation patch as first
patch to this series which basically does this:

diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
index 1a797e410a3f..224fd112b25a 100644
--- a/drivers/input/touchscreen/novatek-nvt-ts.c
+++ b/drivers/input/touchscreen/novatek-nvt-ts.c
@@ -278,7 +278,7 @@ static int nvt_ts_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id nvt_ts_i2c_id[] = {
-	{ "NVT-ts" },
+	{ "NT11205-ts" },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, nvt_ts_i2c_id);
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index eb0e55c69dfe..5ecee6e66fb4 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -40,7 +40,7 @@ static const struct x86_i2c_client_info acer_b1_750_i2c_clients[] __initconst =
 	{
 		/* Novatek NVT-ts touchscreen */
 		.board_info = {
-			.type = "NVT-ts",
+			.type = "NT11205-ts",
 			.addr = 0x34,
 			.dev_name = "NVT-ts",
 		},

This solves the too-generic ID problema nd can then be merged
together with the rest of the series through the input tree.
I'll give my ack as drivers/platform/x86 subsys maintainer for
merging the x86-android-tablets change this way.

>> +      - novatek,nt36672a-ts
> 
> Eh, we have already panel. Why there is a need for touchscreen binding
> (binding, not driver)?

I believe that the nt36672a identifier is an identifier for
a novatek display assembly which contains both a DSI display
panel as well as an I2C touchscreen. Since I2C devices need
to be children of the I2C controller we need a separate node
in the device tree for the I2c touchscreen-controller and since
it is a separate node it needs it own compatible I believe ?

Regards,

Hans

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Krzysztof Kozlowski]

On 22/05/2024 16:00, Joel Selvaraj wrote:
> Hi Krzysztof Kozlowski,
> 
> On 5/21/24 11:48, Krzysztof Kozlowski wrote:
>> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>>
>>> Document the Novatek NVT touchscreen driver which is used in devices like
>>
>> driver? or device?
> 
> touchscreen "controller" would be correct I think. I will fix it in v2.
> 
>>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>>> MAINTAINERS file.
>>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>>
>>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>> ---
>>>   .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>>>   MAINTAINERS                                        |  1 +
>>>   2 files changed, 63 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>>> new file mode 100644
>>> index 0000000000000..7839c6a028e4a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>>> @@ -0,0 +1,62 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Novatek NVT Touchscreen Controller
>>> +
>>> +maintainers:
>>> +  - Hans de Goede <hdegoede@redhat.com>
>>> +
>>> +allOf:
>>> +  - $ref: touchscreen.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - novatek,nvt-ts
>>
>> That's too generic. Looking at your driver change, it is not even needed.
>>
>>> +      - novatek,nt36672a-ts
>>
>> Eh, we have already panel. Why there is a need for touchscreen binding
>> (binding, not driver)?
> 
> I am not sure I understand this correctly. Help me a bit here. For 
> context, in mainline there is an existing driver for the novatek nvt 
> touchscreen controller. The driver did not have devicetree support. It 
> only had a i2c_device_id "NVT-ts". I don't know what is the variant of 

I just got a bit confused that you add another binding for the same
device, but now I see these are different interfaces - DSI and I2C.

> that Novatek touchscreen controller. To use the driver in Xiaomi Poco 
> F1, I introduced a devicetree compatible for it "novatek,nvt-ts". The 
> However, the Novatek touchscreen controller present in Xiaomi Poco F1 is 
> "NT36672A" which has a different chip id than the one in existing 
> driver. So I created a separate compatible for this touchscreen 
> controller variant "novatek,nt36672a-ts". I used compatible data to 
> differentiate the two variants. Since there are two variants, I am 
> mentioning both here.

Just to be clear, I don't care about driver here but hardware. You have
two separate interfaces on this hardware - DSI virtual channel and I2C?


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Joel Selvaraj]

Hi Krzysztof Kozlowski,

On 5/23/24 01:18, Krzysztof Kozlowski wrote:
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - novatek,nvt-ts
>>>
>>> That's too generic. Looking at your driver change, it is not even needed.

As suggested by Hans de Goede, I will fix the too generic issue in v2.

>>>> +      - novatek,nt36672a-ts
>>>
>>> Eh, we have already panel. Why there is a need for touchscreen binding
>>> (binding, not driver)?
>>
>> I am not sure I understand this correctly. Help me a bit here. For
>> context, in mainline there is an existing driver for the novatek nvt
>> touchscreen controller. The driver did not have devicetree support. It
>> only had a i2c_device_id "NVT-ts". I don't know what is the variant of
> 
> I just got a bit confused that you add another binding for the same
> device, but now I see these are different interfaces - DSI and I2C.
> 
>> that Novatek touchscreen controller. To use the driver in Xiaomi Poco
>> F1, I introduced a devicetree compatible for it "novatek,nvt-ts". The
>> However, the Novatek touchscreen controller present in Xiaomi Poco F1 is
>> "NT36672A" which has a different chip id than the one in existing
>> driver. So I created a separate compatible for this touchscreen
>> controller variant "novatek,nt36672a-ts". I used compatible data to
>> differentiate the two variants. Since there are two variants, I am
>> mentioning both here.
> 
> Just to be clear, I don't care about driver here but hardware. You have
> two separate interfaces on this hardware - DSI virtual channel and I2C?

Yes, there are two different interface in the hardware. The display 
panel is connected via DSI and the touchscreen is connected via I2C 
interface separately.

> 
> Best regards,
> Krzysztof
> 

Regards,
Joel Selvaraj

Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller

[Joel Selvaraj]

Hi Hans de Goede

On 5/22/24 11:00, Hans de Goede wrote:
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - novatek,nvt-ts
>>
>> That's too generic. Looking at your driver change, it is not even needed.
> 
> I wrote the novatek-nvt-ts driver for an Acer Iconia One 7 B1-750 tablet,
> this is a x86 tablet which ships with Android as factory OS and because
> Android OS images use kernels where everything is hardcoded the ACPI tables
> do not describe this touchscreen. Instead the i2c_client for the touchscreen
> is manually instantiated by some x86 platform glue code. Since it is
> manually instantiated it uses i2c_device_id binding rather then OF/ACPI.
> 
> The generic "NVT-ts" i2c_device_id comes from me not knowing the controller
> panel type back then. In the mean time I have learned that the B1-750 uses
> NVT-NT11205 controller.
> 
> So what I think needs to happen here is add a preparation patch as first
> patch to this series which basically does this:
> 
> diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
> index 1a797e410a3f..224fd112b25a 100644
> --- a/drivers/input/touchscreen/novatek-nvt-ts.c
> +++ b/drivers/input/touchscreen/novatek-nvt-ts.c
> @@ -278,7 +278,7 @@ static int nvt_ts_probe(struct i2c_client *client)
>   }
>   
>   static const struct i2c_device_id nvt_ts_i2c_id[] = {
> -	{ "NVT-ts" },
> +	{ "NT11205-ts" },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, nvt_ts_i2c_id);
> diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
> index eb0e55c69dfe..5ecee6e66fb4 100644
> --- a/drivers/platform/x86/x86-android-tablets/other.c
> +++ b/drivers/platform/x86/x86-android-tablets/other.c
> @@ -40,7 +40,7 @@ static const struct x86_i2c_client_info acer_b1_750_i2c_clients[] __initconst =
>   	{
>   		/* Novatek NVT-ts touchscreen */
>   		.board_info = {
> -			.type = "NVT-ts",
> +			.type = "NT11205-ts",
>   			.addr = 0x34,
>   			.dev_name = "NVT-ts",
>   		},
> 
> This solves the too-generic ID problema nd can then be merged
> together with the rest of the series through the input tree.
> I'll give my ack as drivers/platform/x86 subsys maintainer for
> merging the x86-android-tablets change this way.

Ok, will do so in v2. Thanks.

> 
>>> +      - novatek,nt36672a-ts
>>
>> Eh, we have already panel. Why there is a need for touchscreen binding
>> (binding, not driver)?
> 
> I believe that the nt36672a identifier is an identifier for
> a novatek display assembly which contains both a DSI display
> panel as well as an I2C touchscreen. Since I2C devices need

Yeah.

> to be children of the I2C controller we need a separate node
> in the device tree for the I2c touchscreen-controller and since
> it is a separate node it needs it own compatible I believe ? >
> Regards,
> 
> Hans
> 

Regards,
Joel Selvaraj