[PATCH v2 1/3] input: touchscreem: ad7877: add match table

[PATCH v2 1/3] input: touchscreem: ad7877: add match table

[Antoniu Miclaus]

Add match table for the ad7877 driver and define the compatible string.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v2.
 drivers/input/touchscreen/ad7877.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index a0598e9c7aff..7886454a19c6 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -805,10 +805,17 @@ static int ad7877_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ad7877_pm, ad7877_suspend, ad7877_resume);
 
+static const struct of_device_id ad7877_of_match[] = {
+	{ .compatible = "adi,ad7877", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7877_of_match);
+
 static struct spi_driver ad7877_driver = {
 	.driver = {
 		.name		= "ad7877",
 		.dev_groups	= ad7877_groups,
+		.of_match_table = ad7877_of_match,
 		.pm		= pm_sleep_ptr(&ad7877_pm),
 	},
 	.probe		= ad7877_probe,
-- 
2.46.0

[PATCH v2 2/3] input: touchscreen: ad7877: add dt support

[Antoniu Miclaus]

Add devicetree support within the driver.

Make the old platform data approach optional.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
new in v2.
 drivers/input/touchscreen/ad7877.c | 68 +++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 7886454a19c6..3fa38043b561 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -27,6 +27,7 @@
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/ad7877.h>
@@ -667,6 +668,68 @@ static void ad7877_setup_ts_def_msg(struct spi_device *spi, struct ad7877 *ts)
 	}
 }
 
+static struct ad7877_platform_data *ad7877_parse_props(struct device *dev)
+{
+	struct ad7877_platform_data *pdata;
+	u32 value, average;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->model = (uintptr_t)device_get_match_data(dev);
+
+	device_property_read_u8(dev, "adi,stopacq-polarity",
+				&pdata->stopacq_polarity);
+	device_property_read_u8(dev, "adi,first-conv-delay",
+				&pdata->first_conversion_delay);
+	device_property_read_u8(dev, "adi,pen-down-acc-interval",
+				&pdata->pen_down_acc_interval);
+	device_property_read_u8(dev, "adi,acquisition-time",
+				&pdata->acquisition_time);
+
+	device_property_read_u16(dev, "adi,vref-delay-usecs",
+				 &pdata->vref_delay_usecs);
+
+	device_property_read_u32(dev, "touchscreen-x-plate-ohms", &value);
+	pdata->x_plate_ohms = (u16)value;
+	device_property_read_u32(dev, "touchscreen-y-plate-ohms", &value);
+	pdata->y_plate_ohms = (u16)value;
+	device_property_read_u32(dev, "touchscreen-min-x", &value);
+	pdata->x_min = (u16)value;
+	device_property_read_u32(dev, "touchscreen-min-y", &value);
+	pdata->y_min = (u16)value;
+	device_property_read_u32(dev, "touchscreen-max-x", &value);
+	pdata->x_max = (u16)value;
+	device_property_read_u32(dev, "touchscreen-max-y", &value);
+	pdata->y_max = (u16)value;
+	device_property_read_u32(dev, "touchscreen-max-pressure", &value);
+	pdata->pressure_max = (u16)value;
+	device_property_read_u32(dev, "touchscreen-min-pressure", &value);
+	pdata->pressure_min = (u16)value;
+	device_property_read_u32(dev, "touchscreen-average-samples", &average);
+	switch (average) {
+	case 1:
+		pdata->averaging = 0;
+		break;
+	case 4:
+		pdata->averaging = 1;
+		break;
+	case 8:
+		pdata->averaging = 2;
+		break;
+	case 16:
+		pdata->averaging = 3;
+		break;
+	default:
+		dev_err(dev,
+			"touchscreen-average-samples must be 1, 4, 8, or 16\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return pdata;
+}
+
 static int ad7877_probe(struct spi_device *spi)
 {
 	struct ad7877			*ts;
@@ -681,8 +744,9 @@ static int ad7877_probe(struct spi_device *spi)
 	}
 
 	if (!pdata) {
-		dev_dbg(&spi->dev, "no platform data?\n");
-		return -ENODEV;
+		pdata = ad7877_parse_props(&spi->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
 	}
 
 	/* don't exceed max specified SPI CLK frequency */
-- 
2.46.0

[PATCH v2 3/3] dt-bindings: touchscreen: ad7877: add bindings

[Antoniu Miclaus]

Add device tree bindings for the ad7877 driver.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v2:
 - add only the used properties from touchscreen.yaml
 - add vendor properties.
 - update dt example.
 .../input/touchscreen/adi,ad7877.yaml         | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
new file mode 100644
index 000000000000..035e2d5bbcb8
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/adi,ad7877.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7877 Touch Screen Controller
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+  Analog Devices Touch Screen Controller
+  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7877.pdf
+
+allOf:
+  - $ref: touchscreen.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7877
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  spi-max-frequency:
+    description: AD7877 SPI bus clock frequency.
+    minimum: 10000
+    maximum: 20000000
+
+  adi,stopacq-polarity:
+    description: The polarity of the signal applied to the STOPACQ pin.
+                 0 = active low
+                 1 = active high
+    $ref: /schemas/types.yaml#/definitions/uint8
+    enum: [0, 1]
+
+  adi,first-conv-delay:
+    description: Delay before the first conversion.
+                 0 = 0.5us
+                 1 = 128us
+                 2 = 1ms
+                 3 = 8ms
+    $ref: /schemas/types.yaml#/definitions/uint8
+    enum: [0, 1, 2, 3]
+
+  adi,pen-down-acc-interval:
+    description: Enable the ADC to repeatedly perform conversions.
+                  0 = covert once
+                  1 = every 0.5 ms
+                  2 = every 1 ms
+                  3 = every 8 ms
+    $ref: /schemas/types.yaml#/definitions/uint8
+    enum: [0, 1, 2, 3]
+
+  adi,acquisition-time:
+    description:  Select acquisition times for the ADC.
+                  0 = 2us
+                  1 = 4us
+                  2 = 8us
+                  3 = 16us
+    $ref: /schemas/types.yaml#/definitions/uint8
+    enum: [0, 1, 2, 3]
+
+  adi,vref-delay-usecs:
+    description: Delay required for the SPI transfers depending on the VREF used.
+    $ref: /schemas/types.yaml#/definitions/uint16
+
+  touchscreen-average-samples:
+    enum: [1, 4, 8, 16]
+
+  touchscreen-x-plate-ohms: true
+  touchscreen-y-plate-ohms: true
+  touchscreen-min-x: true
+  touchscreen-min-y: true
+  touchscreen-max-x: true
+  touchscreen-max-y: true
+  touchscreen-max-pressure: true
+  touchscreen-min-pressure: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - touchscreen-average-samples
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@0 {
+        compatible = "adi,ad7877";
+        reg = <0>;
+        spi-max-frequency = <20000000>;
+        interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
+        interrupt-parent = <&gpio>;
+        touchscreen-average-samples = <16>;
+      };
+    };
+...
-- 
2.46.0

Re: [PATCH v2 3/3] dt-bindings: touchscreen: ad7877: add bindings

[Krzysztof Kozlowski]

On Mon, Sep 02, 2024 at 11:24:33AM +0300, Antoniu Miclaus wrote:
> Add device tree bindings for the ad7877 driver.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v2:
>  - add only the used properties from touchscreen.yaml
>  - add vendor properties.
>  - update dt example.
>  .../input/touchscreen/adi,ad7877.yaml         | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
> new file mode 100644
> index 000000000000..035e2d5bbcb8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/adi,ad7877.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7877 Touch Screen Controller
> +
> +maintainers:
> +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> +  Analog Devices Touch Screen Controller
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7877.pdf
> +
> +allOf:
> +  - $ref: touchscreen.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7877
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    description: AD7877 SPI bus clock frequency.
> +    minimum: 10000
> +    maximum: 20000000
> +
> +  adi,stopacq-polarity:
> +    description: The polarity of the signal applied to the STOPACQ pin.
> +                 0 = active low
> +                 1 = active high
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    enum: [0, 1]

I think I was already commenting on this for analog... If using numbers,
why this is reversed from standard GPIO property? Or maybe this should
be just string.

> +
> +  adi,first-conv-delay:
> +    description: Delay before the first conversion.

No, use proper unit suffix.

> +                 0 = 0.5us
> +                 1 = 128us
> +                 2 = 1ms
> +                 3 = 8ms
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    enum: [0, 1, 2, 3]
> +
> +  adi,pen-down-acc-interval:

Use proper unit suffix.

> +    description: Enable the ADC to repeatedly perform conversions.
> +                  0 = covert once
> +                  1 = every 0.5 ms
> +                  2 = every 1 ms
> +                  3 = every 8 ms
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    enum: [0, 1, 2, 3]

How is it supposed to work? These are optional but there are no
defaults?

> +
> +  adi,acquisition-time:
> +    description:  Select acquisition times for the ADC.
> +                  0 = 2us
> +                  1 = 4us
> +                  2 = 8us
> +                  3 = 16us

Same problem

> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    enum: [0, 1, 2, 3]
> +
> +  adi,vref-delay-usecs:
> +    description: Delay required for the SPI transfers depending on the VREF used.
> +    $ref: /schemas/types.yaml#/definitions/uint16
> +
> +  touchscreen-average-samples:
> +    enum: [1, 4, 8, 16]
> +
> +  touchscreen-x-plate-ohms: true
> +  touchscreen-y-plate-ohms: true
> +  touchscreen-min-x: true
> +  touchscreen-min-y: true
> +  touchscreen-max-x: true
> +  touchscreen-max-y: true
> +  touchscreen-max-pressure: true
> +  touchscreen-min-pressure: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - touchscreen-average-samples
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      touchscreen@0 {
> +        compatible = "adi,ad7877";
> +        reg = <0>;
> +        spi-max-frequency = <20000000>;
> +        interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
> +        interrupt-parent = <&gpio>;
> +        touchscreen-average-samples = <16>;

Make the example complete.

Best regards,
Krzysztof

Re: [PATCH v2 2/3] input: touchscreen: ad7877: add dt support

[Dmitry Torokhov]

Hi Antoniu,

On Mon, Sep 02, 2024 at 11:24:32AM +0300, Antoniu Miclaus wrote:
> Add devicetree support within the driver.
> 
> Make the old platform data approach optional.

Nobody is using it, so simply remove it.

> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> new in v2.
>  drivers/input/touchscreen/ad7877.c | 68 +++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
> index 7886454a19c6..3fa38043b561 100644
> --- a/drivers/input/touchscreen/ad7877.c
> +++ b/drivers/input/touchscreen/ad7877.c
> @@ -27,6 +27,7 @@
>  #include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/pm.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
>  #include <linux/spi/ad7877.h>
> @@ -667,6 +668,68 @@ static void ad7877_setup_ts_def_msg(struct spi_device *spi, struct ad7877 *ts)
>  	}
>  }
>  
> +static struct ad7877_platform_data *ad7877_parse_props(struct device *dev)
> +{
> +	struct ad7877_platform_data *pdata;
> +	u32 value, average;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pdata->model = (uintptr_t)device_get_match_data(dev);
> +
> +	device_property_read_u8(dev, "adi,stopacq-polarity",
> +				&pdata->stopacq_polarity);
> +	device_property_read_u8(dev, "adi,first-conv-delay",
> +				&pdata->first_conversion_delay);
> +	device_property_read_u8(dev, "adi,pen-down-acc-interval",
> +				&pdata->pen_down_acc_interval);
> +	device_property_read_u8(dev, "adi,acquisition-time",
> +				&pdata->acquisition_time);
> +
> +	device_property_read_u16(dev, "adi,vref-delay-usecs",
> +				 &pdata->vref_delay_usecs);
> +
> +	device_property_read_u32(dev, "touchscreen-x-plate-ohms", &value);
> +	pdata->x_plate_ohms = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-y-plate-ohms", &value);
> +	pdata->y_plate_ohms = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-min-x", &value);
> +	pdata->x_min = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-min-y", &value);
> +	pdata->y_min = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-max-x", &value);
> +	pdata->x_max = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-max-y", &value);
> +	pdata->y_max = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-max-pressure", &value);
> +	pdata->pressure_max = (u16)value;
> +	device_property_read_u32(dev, "touchscreen-min-pressure", &value);
> +	pdata->pressure_min = (u16)value;

Please use touchscreen_parse_properties() and also apply transformations
via touchscreen_report_pos() instead of rolling your own logic.

Thanks.

-- 
Dmitry