dt-bindings: ps2-gpio: convert binding to json-schema

dt-bindings: ps2-gpio: convert binding to json-schema

[Danilo Krummrich]

This series converts the ps2-gpio dt-binding to the json-schema.

Additionally, it documents (in the dt-binding) and enforces to flag the
given gpios as open drain, since the PS/2 bus is open drain by
definition.

[PATCH 1/2] dt-bindings: ps2-gpio: convert binding to json-schema

[Danilo Krummrich]

Convert the ps2-gpio dt-binding documentation to DT schema format using
the json-schema.

Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
---
 .../devicetree/bindings/serio/ps2-gpio.txt    | 23 --------
 .../devicetree/bindings/serio/ps2-gpio.yaml   | 52 +++++++++++++++++++
 2 files changed, 52 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
 create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.yaml

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
deleted file mode 100644
index 7b7bc9cdf986..000000000000
--- a/Documentation/devicetree/bindings/serio/ps2-gpio.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-Device-Tree binding for ps/2 gpio device
-
-Required properties:
-	- compatible = "ps2-gpio"
-	- data-gpios: the data pin
-	- clk-gpios: the clock pin
-	- interrupts: Should trigger on the falling edge of the clock line.
-
-Optional properties:
-	- write-enable: Indicates whether write function is provided
-	to serio device. Possibly providing the write fn will not work, because
-	of the tough timing requirements.
-
-Example nodes:
-
-ps2@0 {
-	compatible = "ps2-gpio";
-	interrupt-parent = <&gpio>;
-	interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
-	data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
-	clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
-	write-enable;
-};
diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.yaml b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
new file mode 100644
index 000000000000..ec6fa7b40851
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serio/ps2-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for GPIO based PS/2
+
+maintainers:
+  - Danilo Krummrich <danilokrummrich@dk-develop.de>
+
+properties:
+  compatible:
+    items:
+      - const: ps2-gpio
+
+  data-gpios:
+    description:
+      the gpio used for the data signal
+    maxItems: 1
+
+  clk-gpios:
+    description:
+      the gpio used for the clock signal
+    maxItems: 1
+
+  interrupts:
+    description:
+      The given interrupt should trigger on the falling edge of the clock line.
+    maxItems: 1
+
+  write-enable:
+    description:
+      Indicates whether write function is provided to serio device. Possibly
+      providing the write function will not work, because of the tough timing
+      requirements.
+
+required:
+  - data-gpios
+  - clk-gpios
+  - interrupts
+
+examples:
+  - |
+    ps2@0 {
+        compatible = "ps2-gpio";
+        interrupt-parent = <&gpio>;
+        interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+        data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+        clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
+        write-enable;
+    };
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: ps2-gpio: convert binding to json-schema

[Krzysztof Kozlowski]

On 12/02/2022 00:31, Danilo Krummrich wrote:
> Convert the ps2-gpio dt-binding documentation to DT schema format using
> the json-schema.
> 
> Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
> ---
>  .../devicetree/bindings/serio/ps2-gpio.txt    | 23 --------
>  .../devicetree/bindings/serio/ps2-gpio.yaml   | 52 +++++++++++++++++++
>  2 files changed, 52 insertions(+), 23 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
>  create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
> deleted file mode 100644
> index 7b7bc9cdf986..000000000000
> --- a/Documentation/devicetree/bindings/serio/ps2-gpio.txt
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -Device-Tree binding for ps/2 gpio device
> -
> -Required properties:
> -	- compatible = "ps2-gpio"
> -	- data-gpios: the data pin
> -	- clk-gpios: the clock pin
> -	- interrupts: Should trigger on the falling edge of the clock line.
> -
> -Optional properties:
> -	- write-enable: Indicates whether write function is provided
> -	to serio device. Possibly providing the write fn will not work, because
> -	of the tough timing requirements.
> -
> -Example nodes:
> -
> -ps2@0 {
> -	compatible = "ps2-gpio";
> -	interrupt-parent = <&gpio>;
> -	interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
> -	data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> -	clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> -	write-enable;
> -};
> diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.yaml b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> new file mode 100644
> index 000000000000..ec6fa7b40851
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serio/ps2-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for GPIO based PS/2
> +
> +maintainers:
> +  - Danilo Krummrich <danilokrummrich@dk-develop.de>
> +
> +properties:
> +  compatible:
> +    items:

Hi,

I think you do not expect more compatibles, so skip "items".

> +      - const: ps2-gpio
> +
> +  data-gpios:
> +    description:
> +      the gpio used for the data signal
> +    maxItems: 1
> +
> +  clk-gpios:
> +    description:
> +      the gpio used for the clock signal
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      The given interrupt should trigger on the falling edge of the clock line.
> +    maxItems: 1
> +
> +  write-enable:

This does not look like a standard property. You need "type: boolean".

> +    description:
> +      Indicates whether write function is provided to serio device. Possibly
> +      providing the write function will not work, because of the tough timing
> +      requirements.
> +
> +required:

Also: compatible

> +  - data-gpios
> +  - clk-gpios
> +  - interrupts
> +
> +examples:
> +  - |
> +    ps2@0 {

Does it compile cleanly? You used an unit-address but there is no reg.

> +        compatible = "ps2-gpio";
> +        interrupt-parent = <&gpio>;
> +        interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
> +        data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +        clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> +        write-enable;
> +    };


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: ps2-gpio: convert binding to json-schema

[Danilo Krummrich]

On Mon, Feb 14, 2022 at 10:49:27PM +0100, Krzysztof Kozlowski wrote:
> On 12/02/2022 00:31, Danilo Krummrich wrote:
> > Convert the ps2-gpio dt-binding documentation to DT schema format using
> > the json-schema.
> > 
> > Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
> > ---
> >  .../devicetree/bindings/serio/ps2-gpio.txt    | 23 --------
> >  .../devicetree/bindings/serio/ps2-gpio.yaml   | 52 +++++++++++++++++++
> >  2 files changed, 52 insertions(+), 23 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
> >  create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.txt b/Documentation/devicetree/bindings/serio/ps2-gpio.txt
> > deleted file mode 100644
> > index 7b7bc9cdf986..000000000000
> > --- a/Documentation/devicetree/bindings/serio/ps2-gpio.txt
> > +++ /dev/null
> > @@ -1,23 +0,0 @@
> > -Device-Tree binding for ps/2 gpio device
> > -
> > -Required properties:
> > -	- compatible = "ps2-gpio"
> > -	- data-gpios: the data pin
> > -	- clk-gpios: the clock pin
> > -	- interrupts: Should trigger on the falling edge of the clock line.
> > -
> > -Optional properties:
> > -	- write-enable: Indicates whether write function is provided
> > -	to serio device. Possibly providing the write fn will not work, because
> > -	of the tough timing requirements.
> > -
> > -Example nodes:
> > -
> > -ps2@0 {
> > -	compatible = "ps2-gpio";
> > -	interrupt-parent = <&gpio>;
> > -	interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
> > -	data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> > -	clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> > -	write-enable;
> > -};
> > diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.yaml b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> > new file mode 100644
> > index 000000000000..ec6fa7b40851
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> > @@ -0,0 +1,52 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/serio/ps2-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Bindings for GPIO based PS/2
> > +
> > +maintainers:
> > +  - Danilo Krummrich <danilokrummrich@dk-develop.de>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> 
> Hi,
> 
> I think you do not expect more compatibles, so skip "items".
> 
> > +      - const: ps2-gpio
> > +
> > +  data-gpios:
> > +    description:
> > +      the gpio used for the data signal
> > +    maxItems: 1
> > +
> > +  clk-gpios:
> > +    description:
> > +      the gpio used for the clock signal
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    description:
> > +      The given interrupt should trigger on the falling edge of the clock line.
> > +    maxItems: 1
> > +
> > +  write-enable:
> 
> This does not look like a standard property. You need "type: boolean".
> 
> > +    description:
> > +      Indicates whether write function is provided to serio device. Possibly
> > +      providing the write function will not work, because of the tough timing
> > +      requirements.
> > +
> > +required:
> 
> Also: compatible
> 
> > +  - data-gpios
> > +  - clk-gpios
> > +  - interrupts
> > +
> > +examples:
> > +  - |
> > +    ps2@0 {
> 
> Does it compile cleanly? You used an unit-address but there is no reg.
Yes, it does. I will fix those up and send a v2.
> 
> > +        compatible = "ps2-gpio";
> > +        interrupt-parent = <&gpio>;
> > +        interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
> > +        data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> > +        clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> > +        write-enable;
> > +    };
> 
> 
> Best regards,
> Krzysztof

- Danilo

Re: [PATCH 1/2] dt-bindings: ps2-gpio: convert binding to json-schema

[Rob Herring]

On Sat, 12 Feb 2022 00:31:36 +0100, Danilo Krummrich wrote:
> Convert the ps2-gpio dt-binding documentation to DT schema format using
> the json-schema.
> 
> Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
> ---
>  .../devicetree/bindings/serio/ps2-gpio.txt    | 23 --------
>  .../devicetree/bindings/serio/ps2-gpio.yaml   | 52 +++++++++++++++++++
>  2 files changed, 52 insertions(+), 23 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.txt
>  create mode 100644 Documentation/devicetree/bindings/serio/ps2-gpio.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/serio/ps2-gpio.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/serio/ps2-gpio.yaml: ignoring, error in schema: 
Error: Documentation/devicetree/bindings/serio/ps2-gpio.example.dts:22.30-31 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/serio/ps2-gpio.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1591901

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

[PATCH 2/2] input: ps2-gpio: enforce and document open drain

[Danilo Krummrich]

The PS/2 bus defines the data and clock line be open drain, therefore
document this in the dt-binding and enforce the particular flag in the
driver.

Without enforcing to flag at least the clock gpio as open drain we run
into the following warning:

WARNING: CPU: 1 PID: 40 at drivers/gpio/gpiolib.c:3175 gpiochip_enable_irq+0x54/0x90

gpiochip_enable_irq() warns on a GPIO being configured as output and
using IRQ without being flagged as open drain.

Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
---
 .../devicetree/bindings/serio/ps2-gpio.yaml        | 14 ++++++++++----
 drivers/input/serio/ps2-gpio.c                     |  9 +++++++--
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/serio/ps2-gpio.yaml b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
index ec6fa7b40851..5e1951b4f511 100644
--- a/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
+++ b/Documentation/devicetree/bindings/serio/ps2-gpio.yaml
@@ -16,12 +16,18 @@ properties:
 
   data-gpios:
     description:
-      the gpio used for the data signal
+      the gpio used for the data signal - this should be flagged as
+      active high using open drain with (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)
+      from <dt-bindings/gpio/gpio.h> since the signal is open drain by
+      definition
     maxItems: 1
 
   clk-gpios:
     description:
-      the gpio used for the clock signal
+      the gpio used for the clock signal - this should be flagged as
+      active high using open drain with (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)
+      from <dt-bindings/gpio/gpio.h> since the signal is open drain by
+      definition
     maxItems: 1
 
   interrupts:
@@ -46,7 +52,7 @@ examples:
         compatible = "ps2-gpio";
         interrupt-parent = <&gpio>;
         interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
-        data-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
-        clk-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
+        data-gpios = <&gpio 24 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+        clk-gpios = <&gpio 23 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
         write-enable;
     };
diff --git a/drivers/input/serio/ps2-gpio.c b/drivers/input/serio/ps2-gpio.c
index 460d520ac865..fdaff7a415cd 100644
--- a/drivers/input/serio/ps2-gpio.c
+++ b/drivers/input/serio/ps2-gpio.c
@@ -362,14 +362,19 @@ static irqreturn_t ps2_gpio_irq(int irq, void *dev_id)
 static int ps2_gpio_get_props(struct device *dev,
 				 struct ps2_gpio_data *drvdata)
 {
-	drvdata->gpio_data = devm_gpiod_get(dev, "data", GPIOD_IN);
+	enum gpiod_flags gflags;
+
+	/* Enforce open drain, since this is required by the PS/2 bus. */
+	gflags = GPIOD_IN | GPIOD_FLAGS_BIT_OPEN_DRAIN;
+
+	drvdata->gpio_data = devm_gpiod_get(dev, "data", gflags);
 	if (IS_ERR(drvdata->gpio_data)) {
 		dev_err(dev, "failed to request data gpio: %ld",
 			PTR_ERR(drvdata->gpio_data));
 		return PTR_ERR(drvdata->gpio_data);
 	}
 
-	drvdata->gpio_clk = devm_gpiod_get(dev, "clk", GPIOD_IN);
+	drvdata->gpio_clk = devm_gpiod_get(dev, "clk", gflags);
 	if (IS_ERR(drvdata->gpio_clk)) {
 		dev_err(dev, "failed to request clock gpio: %ld",
 			PTR_ERR(drvdata->gpio_clk));
-- 
2.34.1

Re: [PATCH 2/2] input: ps2-gpio: enforce and document open drain

[Krzysztof Kozlowski]

On 12/02/2022 00:31, Danilo Krummrich wrote:
> The PS/2 bus defines the data and clock line be open drain, therefore
> document this in the dt-binding and enforce the particular flag in the
> driver.
> 
> Without enforcing to flag at least the clock gpio as open drain we run
> into the following warning:
> 
> WARNING: CPU: 1 PID: 40 at drivers/gpio/gpiolib.c:3175 gpiochip_enable_irq+0x54/0x90
> 
> gpiochip_enable_irq() warns on a GPIO being configured as output and
> using IRQ without being flagged as open drain.
> 
> Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
> ---
>  .../devicetree/bindings/serio/ps2-gpio.yaml        | 14 ++++++++++----
>  drivers/input/serio/ps2-gpio.c                     |  9 +++++++--
>  2 files changed, 17 insertions(+), 6 deletions(-)
> 

Bindings as separate patch, please.

Best regards,
Krzysztof

Re: [PATCH 2/2] input: ps2-gpio: enforce and document open drain

[Danilo Krummrich]

On Mon, Feb 14, 2022 at 10:50:21PM +0100, Krzysztof Kozlowski wrote:
> On 12/02/2022 00:31, Danilo Krummrich wrote:
> > The PS/2 bus defines the data and clock line be open drain, therefore
> > document this in the dt-binding and enforce the particular flag in the
> > driver.
> > 
> > Without enforcing to flag at least the clock gpio as open drain we run
> > into the following warning:
> > 
> > WARNING: CPU: 1 PID: 40 at drivers/gpio/gpiolib.c:3175 gpiochip_enable_irq+0x54/0x90
> > 
> > gpiochip_enable_irq() warns on a GPIO being configured as output and
> > using IRQ without being flagged as open drain.
> > 
> > Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
> > ---
> >  .../devicetree/bindings/serio/ps2-gpio.yaml        | 14 ++++++++++----
> >  drivers/input/serio/ps2-gpio.c                     |  9 +++++++--
> >  2 files changed, 17 insertions(+), 6 deletions(-)
> > 
> 
> Bindings as separate patch, please.
Are you fine with adding this additional documentation directly to the patch
converting the binding to a json-schema or do you prefer it to be a separate
patch?
> 
> Best regards,
> Krzysztof

- Danilo

Re: [PATCH 2/2] input: ps2-gpio: enforce and document open drain

[Krzysztof Kozlowski]

On 14/02/2022 23:42, Danilo Krummrich wrote:
> On Mon, Feb 14, 2022 at 10:50:21PM +0100, Krzysztof Kozlowski wrote:
>> On 12/02/2022 00:31, Danilo Krummrich wrote:
>>> The PS/2 bus defines the data and clock line be open drain, therefore
>>> document this in the dt-binding and enforce the particular flag in the
>>> driver.
>>>
>>> Without enforcing to flag at least the clock gpio as open drain we run
>>> into the following warning:
>>>
>>> WARNING: CPU: 1 PID: 40 at drivers/gpio/gpiolib.c:3175 gpiochip_enable_irq+0x54/0x90
>>>
>>> gpiochip_enable_irq() warns on a GPIO being configured as output and
>>> using IRQ without being flagged as open drain.
>>>
>>> Signed-off-by: Danilo Krummrich <danilokrummrich@dk-develop.de>
>>> ---
>>>  .../devicetree/bindings/serio/ps2-gpio.yaml        | 14 ++++++++++----
>>>  drivers/input/serio/ps2-gpio.c                     |  9 +++++++--
>>>  2 files changed, 17 insertions(+), 6 deletions(-)
>>>
>>
>> Bindings as separate patch, please.
> Are you fine with adding this additional documentation directly to the patch
> converting the binding to a json-schema or do you prefer it to be a separate
> patch?

I propose a separate patch, so you will have two bindings patches.

Best regards,
Krzysztof