[PATCH 0/4] serial: sc16is7xx: device tree improvements

[PATCH 0/4] serial: sc16is7xx: device tree improvements

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Hello,
this patch series improves DT properties naming and handling, and converts
sc16is7xx bindings from text to YAML.

Patch 1 make sure we use the device_property APIs when configuring irda mode.

Patch 2 is in preparation for patch 3, to make sure we recognize
"nxp,irda-mode-ports" DT property as well as the (soon to be) legacy property
"irda-mode-ports".

Patch 3 adds a vendor prefix to irda-mode-ports property.

Patch 4 is the actual conversion of bindings to YAML.
Conversion to YAML has been suggested by Andy Shevchenko during review for
another patch series for the sc16is7xx:
Link: https://lkml.org/lkml/2023/5/25/743

Thank you.

Hugo Villeneuve (4):
  serial: sc16is7xx: use device_property APIs when configuring irda mode
  serial: sc16is7xx: recognize "nxp,irda-mode-ports" DT property
  dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property
  dt-bindings: sc16is7xx: convert to YAML

 .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
 .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
 drivers/tty/serial/sc16is7xx.c                |  40 ++++--
 3 files changed, 156 insertions(+), 128 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
 create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml


base-commit: c980248179d655d33af47f0b0bec1ce8660994c6
-- 
2.30.2

[PATCH 1/4] serial: sc16is7xx: use device_property APIs when configuring irda mode

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Convert driver to be property provider agnostic and allow it to be
used on non-OF platforms.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/sc16is7xx.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 1fe2c3e08a35..db2bb1c0d36c 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1408,6 +1408,29 @@ static int sc16is7xx_setup_gpio_chip(struct sc16is7xx_port *s)
 }
 #endif
 
+static void sc16is7xx_setup_irda_ports(struct sc16is7xx_port *s)
+{
+	int i;
+	int ret;
+	int count;
+	u32 irda_port[2];
+	struct device *dev = s->p[0].port.dev;
+
+	count = device_property_count_u32(dev, "irda-mode-ports");
+	if (count < 0 || count > ARRAY_SIZE(irda_port))
+		return;
+
+	ret = device_property_read_u32_array(dev, "irda-mode-ports",
+					     irda_port, count);
+	if (ret)
+		return;
+
+	for (i = 0; i < count; i++) {
+		if (irda_port[i] < s->devtype->nr_uart)
+			s->p[irda_port[i]].irda_mode = true;
+	}
+}
+
 /*
  * Configure ports designated to operate as modem control lines.
  */
@@ -1590,16 +1613,7 @@ static int sc16is7xx_probe(struct device *dev,
 		sc16is7xx_power(&s->p[i].port, 0);
 	}
 
-	if (dev->of_node) {
-		struct property *prop;
-		const __be32 *p;
-		u32 u;
-
-		of_property_for_each_u32(dev->of_node, "irda-mode-ports",
-					 prop, p, u)
-			if (u < devtype->nr_uart)
-				s->p[u].irda_mode = true;
-	}
+	sc16is7xx_setup_irda_ports(s);
 
 	ret = sc16is7xx_setup_mctrl_ports(s);
 	if (ret)
-- 
2.30.2

[PATCH 2/4] serial: sc16is7xx: recognize "nxp,irda-mode-ports" DT property

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Vendor prefix "nxp," will be added in an upcoming commit to DT binding
property "irda-mode-ports".

Prepare to recognize this new property "nxp,irda-mode-ports" and
retain backward compatibility with old DTBs by falling back to
"irda-mode-ports" when needed.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/sc16is7xx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index db2bb1c0d36c..b0154e901358 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1415,13 +1415,19 @@ static void sc16is7xx_setup_irda_ports(struct sc16is7xx_port *s)
 	int count;
 	u32 irda_port[2];
 	struct device *dev = s->p[0].port.dev;
+	const char *prop_name = "nxp,irda-mode-ports";
+
+	count = device_property_count_u32(dev, prop_name);
+	if (count < 0) {
+		/* For backward compatibility with old DTBs. */
+		prop_name = "irda-mode-ports";
+		count = device_property_count_u32(dev, prop_name);
+	}
 
-	count = device_property_count_u32(dev, "irda-mode-ports");
 	if (count < 0 || count > ARRAY_SIZE(irda_port))
 		return;
 
-	ret = device_property_read_u32_array(dev, "irda-mode-ports",
-					     irda_port, count);
+	ret = device_property_read_u32_array(dev, prop_name, irda_port, count);
 	if (ret)
 		return;
 
-- 
2.30.2

[PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

The NXP-specific "irda-mode-ports" property lacks a proper vendor
prefix. Add "nxp," prefix to comply with DT best practises.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/serial/nxp,sc16is7xx.txt          | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
index 1a7e4bff0456..d89815c5c562 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
@@ -21,8 +21,8 @@ Optional properties:
   the second cell is used to specify the GPIO polarity:
     0 = active high,
     1 = active low.
-- irda-mode-ports: An array that lists the indices of the port that
-		   should operate in IrDA mode.
+- nxp,irda-mode-ports: An array that lists the indices of the port that
+		       should operate in IrDA mode.
 - nxp,modem-control-line-ports: An array that lists the indices of the port that
 				should have shared GPIO lines configured as
 				modem control lines.
@@ -80,8 +80,8 @@ Optional properties:
   the second cell is used to specify the GPIO polarity:
     0 = active high,
     1 = active low.
-- irda-mode-ports: An array that lists the indices of the port that
-		   should operate in IrDA mode.
+- nxp,irda-mode-ports: An array that lists the indices of the port that
+		       should operate in IrDA mode.
 - nxp,modem-control-line-ports: An array that lists the indices of the port that
 				should have shared GPIO lines configured as
 				modem control lines.
-- 
2.30.2

[PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Convert binding from text format to YAML.

Additions to original text binding:
  - add rs485 reference.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
 .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
 2 files changed, 126 insertions(+), 118 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
 create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
deleted file mode 100644
index d89815c5c562..000000000000
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
+++ /dev/null
@@ -1,118 +0,0 @@
-* NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
-* i2c as bus
-
-Required properties:
-- compatible: Should be one of the following:
-  - "nxp,sc16is740" for NXP SC16IS740,
-  - "nxp,sc16is741" for NXP SC16IS741,
-  - "nxp,sc16is750" for NXP SC16IS750,
-  - "nxp,sc16is752" for NXP SC16IS752,
-  - "nxp,sc16is760" for NXP SC16IS760,
-  - "nxp,sc16is762" for NXP SC16IS762.
-- reg: I2C address of the SC16IS7xx device.
-- interrupts: Should contain the UART interrupt
-- clocks: Reference to the IC source clock.
-	OR (when there is no clock provider visible to the platform)
-- clock-frequency: The source clock frequency for the IC.
-
-Optional properties:
-- gpio-controller: Marks the device node as a GPIO controller.
-- #gpio-cells: Should be two. The first cell is the GPIO number and
-  the second cell is used to specify the GPIO polarity:
-    0 = active high,
-    1 = active low.
-- nxp,irda-mode-ports: An array that lists the indices of the port that
-		       should operate in IrDA mode.
-- nxp,modem-control-line-ports: An array that lists the indices of the port that
-				should have shared GPIO lines configured as
-				modem control lines.
-
-Example:
-        sc16is750: sc16is750@51 {
-                compatible = "nxp,sc16is750";
-                reg = <0x51>;
-                clocks = <&clk20m>;
-                interrupt-parent = <&gpio3>;
-                interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-                gpio-controller;
-                #gpio-cells = <2>;
-        };
-
-	sc16is752: sc16is752@53 {
-		compatible = "nxp,sc16is752";
-		reg = <0x53>;
-		clocks = <&clk20m>;
-		interrupt-parent = <&gpio3>;
-		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
-		gpio-controller; /* Port 0 as GPIOs */
-		#gpio-cells = <2>;
-	};
-
-	sc16is752: sc16is752@54 {
-		compatible = "nxp,sc16is752";
-		reg = <0x54>;
-		clocks = <&clk20m>;
-		interrupt-parent = <&gpio3>;
-		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
-	};
-
-* spi as bus
-
-Required properties:
-- compatible: Should be one of the following:
-  - "nxp,sc16is740" for NXP SC16IS740,
-  - "nxp,sc16is741" for NXP SC16IS741,
-  - "nxp,sc16is750" for NXP SC16IS750,
-  - "nxp,sc16is752" for NXP SC16IS752,
-  - "nxp,sc16is760" for NXP SC16IS760,
-  - "nxp,sc16is762" for NXP SC16IS762.
-- reg: SPI chip select number.
-- interrupts: Specifies the interrupt source of the parent interrupt
-  controller. The format of the interrupt specifier depends on the
-  parent interrupt controller.
-- clocks: phandle to the IC source clock.
-
-Optional properties:
-- gpio-controller: Marks the device node as a GPIO controller.
-- #gpio-cells: Should be two. The first cell is the GPIO number and
-  the second cell is used to specify the GPIO polarity:
-    0 = active high,
-    1 = active low.
-- nxp,irda-mode-ports: An array that lists the indices of the port that
-		       should operate in IrDA mode.
-- nxp,modem-control-line-ports: An array that lists the indices of the port that
-				should have shared GPIO lines configured as
-				modem control lines.
-
-Example:
-	sc16is750: sc16is750@0 {
-		compatible = "nxp,sc16is750";
-		reg = <0>;
-		clocks = <&clk20m>;
-		interrupt-parent = <&gpio3>;
-		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-		gpio-controller;
-		#gpio-cells = <2>;
-	};
-
-	sc16is752: sc16is752@1 {
-		compatible = "nxp,sc16is752";
-		reg = <1>;
-		clocks = <&clk20m>;
-		interrupt-parent = <&gpio3>;
-		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
-		gpio-controller; /* Port 0 as GPIOs */
-		#gpio-cells = <2>;
-	};
-
-	sc16is752: sc16is752@2 {
-		compatible = "nxp,sc16is752";
-		reg = <2>;
-		clocks = <&clk20m>;
-		interrupt-parent = <&gpio3>;
-		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
-		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
-	};
diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
new file mode 100644
index 000000000000..508639e09e06
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/nxp,sc16is7xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
+
+maintainers:
+  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
+
+properties:
+  compatible:
+    enum:
+      - nxp,sc16is740
+      - nxp,sc16is741
+      - nxp,sc16is750
+      - nxp,sc16is752
+      - nxp,sc16is760
+      - nxp,sc16is762
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: Reference to the IC source clock.
+
+  clock-frequency:
+    description: |
+      When there is no clock provider visible to the platform, this
+      is the source crystal or external clock frequency for the IC in Hz.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 80000000
+
+  gpio-controller:
+    description: Marks the device node as a GPIO controller.
+    type: boolean
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-line-names:
+    minItems: 1
+    maxItems: 8
+
+  nxp,irda-mode-ports:
+    description: |
+      An array that lists the indices of the port that should operate in IrDA
+      mode:
+      0: port A
+      1: port B
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    items:
+      minimum: 0
+      maximum: 1
+
+  nxp,modem-control-line-ports:
+    description: |
+      An array that lists the indices of the port that should have shared GPIO
+      lines configured as modem control lines:
+      0: port A
+      1: port B
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    items:
+      minimum: 0
+      maximum: 1
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - $ref: /schemas/serial/serial.yaml#
+  - $ref: /schemas/serial/rs485.yaml#
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+oneOf:
+  - required:
+      - clocks
+  - required:
+      - clock-frequency
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    serial0: serial@51 {
+        compatible = "nxp,sc16is750";
+        reg = <0x51>;
+        clocks = <&clk20m>;
+        interrupt-parent = <&gpio3>;
+        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+        gpio-controller;
+        #gpio-cells = <2>;
+    };
+
+    serial1: serial@53 {
+        compatible = "nxp,sc16is752";
+        reg = <0x53>;
+        clocks = <&clk20m>;
+        interrupt-parent = <&gpio3>;
+        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+        nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
+        gpio-controller; /* Port 0 as GPIOs */
+        #gpio-cells = <2>;
+    };
+
+    serial2: serial@54 {
+        compatible = "nxp,sc16is752";
+        reg = <0x54>;
+        clocks = <&clk20m>;
+        interrupt-parent = <&gpio3>;
+        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+        nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
+    };
-- 
2.30.2

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Rob Herring]

On Wed, 20 Sep 2023 11:20:15 -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Convert binding from text format to YAML.
> 
> Additions to original text binding:
>   - add rs485 reference.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
>  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
>  2 files changed, 126 insertions(+), 118 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.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:
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:27.13-26: Warning (reg_format): /example-0/serial@51:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:37.13-26: Warning (reg_format): /example-0/serial@53:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:48.13-26: Warning (reg_format): /example-0/serial@54:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230920152015.1376838-5-hugo@hugovil.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[m.brock]

Hugo Villeneuve wrote on 2023-09-20 17:20:
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> @@ -21,8 +21,8 @@ Optional properties:
>    the second cell is used to specify the GPIO polarity:
>      0 = active high,
>      1 = active low.
> -- irda-mode-ports: An array that lists the indices of the port that
> -		   should operate in IrDA mode.
> +- nxp,irda-mode-ports: An array that lists the indices of the port 
> that
> +		       should operate in IrDA mode.
>  - nxp,modem-control-line-ports: An array that lists the indices of
> the port that

Are you sure you want this line removed?

>  				should have shared GPIO lines configured as
>  				modem control lines.
> @@ -80,8 +80,8 @@ Optional properties:
>    the second cell is used to specify the GPIO polarity:
>      0 = active high,
>      1 = active low.
> -- irda-mode-ports: An array that lists the indices of the port that
> -		   should operate in IrDA mode.
> +- nxp,irda-mode-ports: An array that lists the indices of the port 
> that
> +		       should operate in IrDA mode.
>  - nxp,modem-control-line-ports: An array that lists the indices of
> the port that

Same here.

>  				should have shared GPIO lines configured as
>  				modem control lines.

Greets,
Maarten

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Hugo Villeneuve]

On Thu, 21 Sep 2023 12:29:13 +0200
m.brock@vanmierlo.com wrote:

> Hugo Villeneuve wrote on 2023-09-20 17:20:
> > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > @@ -21,8 +21,8 @@ Optional properties:
> >    the second cell is used to specify the GPIO polarity:
> >      0 = active high,
> >      1 = active low.
> > -- irda-mode-ports: An array that lists the indices of the port that
> > -		   should operate in IrDA mode.
> > +- nxp,irda-mode-ports: An array that lists the indices of the port 
> > that
> > +		       should operate in IrDA mode.
> >  - nxp,modem-control-line-ports: An array that lists the indices of
> > the port that
> 
> Are you sure you want this line removed?

Hi Maarten,
the line "- nxp,modem-control-line-ports..." is not removed, but
because it starts with a dash in the source file, maybe it is confusing
when looking at the patch only?

Hugo.

> 
> >  				should have shared GPIO lines configured as
> >  				modem control lines.
> > @@ -80,8 +80,8 @@ Optional properties:
> >    the second cell is used to specify the GPIO polarity:
> >      0 = active high,
> >      1 = active low.
> > -- irda-mode-ports: An array that lists the indices of the port that
> > -		   should operate in IrDA mode.
> > +- nxp,irda-mode-ports: An array that lists the indices of the port 
> > that
> > +		       should operate in IrDA mode.
> >  - nxp,modem-control-line-ports: An array that lists the indices of
> > the port that
> 
> Same here.
> 
> >  				should have shared GPIO lines configured as
> >  				modem control lines.
> 
> Greets,
> Maarten
> 
> 

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4855 bytes --]

Hey,

On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Convert binding from text format to YAML.
> 
> Additions to original text binding:
>   - add rs485 reference.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
>  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
>  2 files changed, 126 insertions(+), 118 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml

> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> new file mode 100644
> index 000000000000..508639e09e06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/nxp,sc16is7xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)

nit, but my OCD was triggered - capitalise the a in "advanced"?

> +
> +maintainers:
> +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,sc16is740
> +      - nxp,sc16is741
> +      - nxp,sc16is750
> +      - nxp,sc16is752
> +      - nxp,sc16is760
> +      - nxp,sc16is762
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description: Reference to the IC source clock.

You could probably drop this, if it only has one clock it's a bit
redundant.

> +  clock-frequency:
> +    description: |

This | should not be needed here, there's no formatting to preserve.

> +      When there is no clock provider visible to the platform, this
> +      is the source crystal or external clock frequency for the IC in Hz.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 80000000
> +
> +  gpio-controller:
> +    description: Marks the device node as a GPIO controller.
> +    type: boolean
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-line-names:
> +    minItems: 1
> +    maxItems: 8
> +
> +  nxp,irda-mode-ports:
> +    description: |
> +      An array that lists the indices of the port that should operate in IrDA
> +      mode:
> +      0: port A
> +      1: port B
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      minimum: 0
> +      maximum: 1
> +
> +  nxp,modem-control-line-ports:
> +    description: |
> +      An array that lists the indices of the port that should have shared GPIO
> +      lines configured as modem control lines:
> +      0: port A
> +      1: port B
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      minimum: 0
> +      maximum: 1
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - $ref: /schemas/serial/serial.yaml#
> +  - $ref: /schemas/serial/rs485.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +oneOf:
> +  - required:
> +      - clocks
> +  - required:
> +      - clock-frequency
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    serial0: serial@51 {

These labels are not used and should be removed.

Otherwise, conversion looks aight to me.

Thanks,
Conor.

> +        compatible = "nxp,sc16is750";
> +        reg = <0x51>;
> +        clocks = <&clk20m>;
> +        interrupt-parent = <&gpio3>;
> +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +    };
> +
> +    serial1: serial@53 {
> +        compatible = "nxp,sc16is752";
> +        reg = <0x53>;
> +        clocks = <&clk20m>;
> +        interrupt-parent = <&gpio3>;
> +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +        nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> +        gpio-controller; /* Port 0 as GPIOs */
> +        #gpio-cells = <2>;
> +    };
> +
> +    serial2: serial@54 {
> +        compatible = "nxp,sc16is752";
> +        reg = <0x54>;
> +        clocks = <&clk20m>;
> +        interrupt-parent = <&gpio3>;
> +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +        nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> +    };
> -- 
> 2.30.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]

On Wed, Sep 20, 2023 at 11:20:14AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> The NXP-specific "irda-mode-ports" property lacks a proper vendor
> prefix. Add "nxp," prefix to comply with DT best practises.

Looks like you've made changes to the driver in a way both will work,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../devicetree/bindings/serial/nxp,sc16is7xx.txt          | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> index 1a7e4bff0456..d89815c5c562 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> @@ -21,8 +21,8 @@ Optional properties:
>    the second cell is used to specify the GPIO polarity:
>      0 = active high,
>      1 = active low.
> -- irda-mode-ports: An array that lists the indices of the port that
> -		   should operate in IrDA mode.
> +- nxp,irda-mode-ports: An array that lists the indices of the port that
> +		       should operate in IrDA mode.
>  - nxp,modem-control-line-ports: An array that lists the indices of the port that
>  				should have shared GPIO lines configured as
>  				modem control lines.
> @@ -80,8 +80,8 @@ Optional properties:
>    the second cell is used to specify the GPIO polarity:
>      0 = active high,
>      1 = active low.
> -- irda-mode-ports: An array that lists the indices of the port that
> -		   should operate in IrDA mode.
> +- nxp,irda-mode-ports: An array that lists the indices of the port that
> +		       should operate in IrDA mode.
>  - nxp,modem-control-line-ports: An array that lists the indices of the port that
>  				should have shared GPIO lines configured as
>  				modem control lines.
> -- 
> 2.30.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Hugo Villeneuve]

On Thu, 21 Sep 2023 14:45:06 +0100
Conor Dooley <conor@kernel.org> wrote:

> Hey,
> 
> On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Convert binding from text format to YAML.
> > 
> > Additions to original text binding:
> >   - add rs485 reference.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
> >  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
> >  2 files changed, 126 insertions(+), 118 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> >  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> 
> > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > new file mode 100644
> > index 000000000000..508639e09e06
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > @@ -0,0 +1,126 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/serial/nxp,sc16is7xx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
> 
> nit, but my OCD was triggered - capitalise the a in "advanced"?

Hi Conor,
no problem, I like that :)
Will fix it for V2.

 
> > +
> > +maintainers:
> > +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nxp,sc16is740
> > +      - nxp,sc16is741
> > +      - nxp,sc16is750
> > +      - nxp,sc16is752
> > +      - nxp,sc16is760
> > +      - nxp,sc16is762
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: Reference to the IC source clock.
> 
> You could probably drop this, if it only has one clock it's a bit
> redundant.

I don't understand, because there is already a 'clocks' property in
the examples (and also in my real board DTS file) and if I remove it
here it will cause a warning:

serial@51: Unevaluated properties are not allowed ('clocks' was
unexpected)

 
> > +  clock-frequency:
> > +    description: |
> 
> This | should not be needed here, there's no formatting to preserve.

Done.


> 
> > +      When there is no clock provider visible to the platform, this
> > +      is the source crystal or external clock frequency for the IC in Hz.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 80000000
> > +
> > +  gpio-controller:
> > +    description: Marks the device node as a GPIO controller.
> > +    type: boolean
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio-line-names:
> > +    minItems: 1
> > +    maxItems: 8
> > +
> > +  nxp,irda-mode-ports:
> > +    description: |
> > +      An array that lists the indices of the port that should operate in IrDA
> > +      mode:
> > +      0: port A
> > +      1: port B
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 1
> > +    maxItems: 2
> > +    items:
> > +      minimum: 0
> > +      maximum: 1
> > +
> > +  nxp,modem-control-line-ports:
> > +    description: |
> > +      An array that lists the indices of the port that should have shared GPIO
> > +      lines configured as modem control lines:
> > +      0: port A
> > +      1: port B
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 1
> > +    maxItems: 2
> > +    items:
> > +      minimum: 0
> > +      maximum: 1
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +  - $ref: /schemas/serial/serial.yaml#
> > +  - $ref: /schemas/serial/rs485.yaml#
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +oneOf:
> > +  - required:
> > +      - clocks
> > +  - required:
> > +      - clock-frequency
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    serial0: serial@51 {
> 
> These labels are not used and should be removed.

Done.

Thank you,
Hugo.



> Otherwise, conversion looks aight to me.
> 
> Thanks,
> Conor.
> 
> > +        compatible = "nxp,sc16is750";
> > +        reg = <0x51>;
> > +        clocks = <&clk20m>;
> > +        interrupt-parent = <&gpio3>;
> > +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +    };
> > +
> > +    serial1: serial@53 {
> > +        compatible = "nxp,sc16is752";
> > +        reg = <0x53>;
> > +        clocks = <&clk20m>;
> > +        interrupt-parent = <&gpio3>;
> > +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > +        nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> > +        gpio-controller; /* Port 0 as GPIOs */
> > +        #gpio-cells = <2>;
> > +    };
> > +
> > +    serial2: serial@54 {
> > +        compatible = "nxp,sc16is752";
> > +        reg = <0x54>;
> > +        clocks = <&clk20m>;
> > +        interrupt-parent = <&gpio3>;
> > +        interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > +        nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> > +    };
> > -- 
> > 2.30.2
> > 

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Rob Herring]

On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Convert binding from text format to YAML.
> 
> Additions to original text binding:
>   - add rs485 reference.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
>  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
>  2 files changed, 126 insertions(+), 118 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> deleted file mode 100644
> index d89815c5c562..000000000000
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> +++ /dev/null
> @@ -1,118 +0,0 @@
> -* NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
> -* i2c as bus
> -
> -Required properties:
> -- compatible: Should be one of the following:
> -  - "nxp,sc16is740" for NXP SC16IS740,
> -  - "nxp,sc16is741" for NXP SC16IS741,
> -  - "nxp,sc16is750" for NXP SC16IS750,
> -  - "nxp,sc16is752" for NXP SC16IS752,
> -  - "nxp,sc16is760" for NXP SC16IS760,
> -  - "nxp,sc16is762" for NXP SC16IS762.
> -- reg: I2C address of the SC16IS7xx device.
> -- interrupts: Should contain the UART interrupt
> -- clocks: Reference to the IC source clock.
> -	OR (when there is no clock provider visible to the platform)
> -- clock-frequency: The source clock frequency for the IC.
> -
> -Optional properties:
> -- gpio-controller: Marks the device node as a GPIO controller.
> -- #gpio-cells: Should be two. The first cell is the GPIO number and
> -  the second cell is used to specify the GPIO polarity:
> -    0 = active high,
> -    1 = active low.
> -- nxp,irda-mode-ports: An array that lists the indices of the port that
> -		       should operate in IrDA mode.
> -- nxp,modem-control-line-ports: An array that lists the indices of the port that
> -				should have shared GPIO lines configured as
> -				modem control lines.
> -
> -Example:
> -        sc16is750: sc16is750@51 {
> -                compatible = "nxp,sc16is750";
> -                reg = <0x51>;
> -                clocks = <&clk20m>;
> -                interrupt-parent = <&gpio3>;
> -                interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -                gpio-controller;
> -                #gpio-cells = <2>;
> -        };
> -
> -	sc16is752: sc16is752@53 {
> -		compatible = "nxp,sc16is752";
> -		reg = <0x53>;
> -		clocks = <&clk20m>;
> -		interrupt-parent = <&gpio3>;
> -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> -		gpio-controller; /* Port 0 as GPIOs */
> -		#gpio-cells = <2>;
> -	};
> -
> -	sc16is752: sc16is752@54 {
> -		compatible = "nxp,sc16is752";
> -		reg = <0x54>;
> -		clocks = <&clk20m>;
> -		interrupt-parent = <&gpio3>;
> -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> -	};
> -
> -* spi as bus
> -
> -Required properties:
> -- compatible: Should be one of the following:
> -  - "nxp,sc16is740" for NXP SC16IS740,
> -  - "nxp,sc16is741" for NXP SC16IS741,
> -  - "nxp,sc16is750" for NXP SC16IS750,
> -  - "nxp,sc16is752" for NXP SC16IS752,
> -  - "nxp,sc16is760" for NXP SC16IS760,
> -  - "nxp,sc16is762" for NXP SC16IS762.
> -- reg: SPI chip select number.
> -- interrupts: Specifies the interrupt source of the parent interrupt
> -  controller. The format of the interrupt specifier depends on the
> -  parent interrupt controller.
> -- clocks: phandle to the IC source clock.
> -
> -Optional properties:
> -- gpio-controller: Marks the device node as a GPIO controller.
> -- #gpio-cells: Should be two. The first cell is the GPIO number and
> -  the second cell is used to specify the GPIO polarity:
> -    0 = active high,
> -    1 = active low.
> -- nxp,irda-mode-ports: An array that lists the indices of the port that
> -		       should operate in IrDA mode.
> -- nxp,modem-control-line-ports: An array that lists the indices of the port that
> -				should have shared GPIO lines configured as
> -				modem control lines.
> -
> -Example:
> -	sc16is750: sc16is750@0 {
> -		compatible = "nxp,sc16is750";
> -		reg = <0>;
> -		clocks = <&clk20m>;
> -		interrupt-parent = <&gpio3>;
> -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -		gpio-controller;
> -		#gpio-cells = <2>;
> -	};
> -
> -	sc16is752: sc16is752@1 {
> -		compatible = "nxp,sc16is752";
> -		reg = <1>;
> -		clocks = <&clk20m>;
> -		interrupt-parent = <&gpio3>;
> -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> -		gpio-controller; /* Port 0 as GPIOs */
> -		#gpio-cells = <2>;
> -	};
> -
> -	sc16is752: sc16is752@2 {
> -		compatible = "nxp,sc16is752";
> -		reg = <2>;
> -		clocks = <&clk20m>;
> -		interrupt-parent = <&gpio3>;
> -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> -		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> -	};
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> new file mode 100644
> index 000000000000..508639e09e06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/nxp,sc16is7xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
> +
> +maintainers:
> +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,sc16is740
> +      - nxp,sc16is741
> +      - nxp,sc16is750
> +      - nxp,sc16is752
> +      - nxp,sc16is760
> +      - nxp,sc16is762
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description: Reference to the IC source clock.
> +
> +  clock-frequency:
> +    description: |
> +      When there is no clock provider visible to the platform, this
> +      is the source crystal or external clock frequency for the IC in Hz.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Don't need a type here. 'clock-frequency' is already defined.

> +    minimum: 1
> +    maximum: 80000000
> +
> +  gpio-controller:
> +    description: Marks the device node as a GPIO controller.
> +    type: boolean

Just: 

gpio-controller: true

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Hugo Villeneuve]

On Wed, 20 Sep 2023 18:13:10 -0500
Rob Herring <robh@kernel.org> wrote:

> 
> On Wed, 20 Sep 2023 11:20:15 -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Convert binding from text format to YAML.
> > 
> > Additions to original text binding:
> >   - add rs485 reference.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
> >  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
> >  2 files changed, 126 insertions(+), 118 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> >  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.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:
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:27.13-26: Warning (reg_format): /example-0/serial@51:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:37.13-26: Warning (reg_format): /example-0/serial@53:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dts:48.13-26: Warning (reg_format): /example-0/serial@54:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

Hi Rob,
will fix it for V2.

I was already using 'make DT_CHECKER_FLAGS=-m dt_binding_check', but I
incorrectly introduced a space between DT_SCHEMA_FILES= and the
filename, and so it was ignored and all schemas were checked, and I lost
the warnings in all the noise. I now have fixed it.

Also, if someone is interested, I was trying to specify multiple files
with DT_SCHEMA_FILES, and I just found out (after analysing the
Makefile) that you need to separate them with the ":" character. It
would be a good thing to add it to the documentation, I could submit a
patch for this if you like.

Thank you,
Hugo.


> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230920152015.1376838-5-hugo@hugovil.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> 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 after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 
> 

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Rob Herring]

On Wed, Sep 20, 2023 at 11:20:14AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> The NXP-specific "irda-mode-ports" property lacks a proper vendor
> prefix. Add "nxp," prefix to comply with DT best practises.

The opposite direction would be worth carrying in the driver, but I 
don't think adding a vendor prefix is. I would just leave this as-is.

Rob

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Hugo Villeneuve]

On Thu, 21 Sep 2023 10:33:19 -0500
Rob Herring <robh@kernel.org> wrote:

> On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Convert binding from text format to YAML.
> > 
> > Additions to original text binding:
> >   - add rs485 reference.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../bindings/serial/nxp,sc16is7xx.txt         | 118 ----------------
> >  .../bindings/serial/nxp,sc16is7xx.yaml        | 126 ++++++++++++++++++
> >  2 files changed, 126 insertions(+), 118 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> >  create mode 100644 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > deleted file mode 100644
> > index d89815c5c562..000000000000
> > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > +++ /dev/null
> > @@ -1,118 +0,0 @@
> > -* NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
> > -* i2c as bus
> > -
> > -Required properties:
> > -- compatible: Should be one of the following:
> > -  - "nxp,sc16is740" for NXP SC16IS740,
> > -  - "nxp,sc16is741" for NXP SC16IS741,
> > -  - "nxp,sc16is750" for NXP SC16IS750,
> > -  - "nxp,sc16is752" for NXP SC16IS752,
> > -  - "nxp,sc16is760" for NXP SC16IS760,
> > -  - "nxp,sc16is762" for NXP SC16IS762.
> > -- reg: I2C address of the SC16IS7xx device.
> > -- interrupts: Should contain the UART interrupt
> > -- clocks: Reference to the IC source clock.
> > -	OR (when there is no clock provider visible to the platform)
> > -- clock-frequency: The source clock frequency for the IC.
> > -
> > -Optional properties:
> > -- gpio-controller: Marks the device node as a GPIO controller.
> > -- #gpio-cells: Should be two. The first cell is the GPIO number and
> > -  the second cell is used to specify the GPIO polarity:
> > -    0 = active high,
> > -    1 = active low.
> > -- nxp,irda-mode-ports: An array that lists the indices of the port that
> > -		       should operate in IrDA mode.
> > -- nxp,modem-control-line-ports: An array that lists the indices of the port that
> > -				should have shared GPIO lines configured as
> > -				modem control lines.
> > -
> > -Example:
> > -        sc16is750: sc16is750@51 {
> > -                compatible = "nxp,sc16is750";
> > -                reg = <0x51>;
> > -                clocks = <&clk20m>;
> > -                interrupt-parent = <&gpio3>;
> > -                interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -                gpio-controller;
> > -                #gpio-cells = <2>;
> > -        };
> > -
> > -	sc16is752: sc16is752@53 {
> > -		compatible = "nxp,sc16is752";
> > -		reg = <0x53>;
> > -		clocks = <&clk20m>;
> > -		interrupt-parent = <&gpio3>;
> > -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> > -		gpio-controller; /* Port 0 as GPIOs */
> > -		#gpio-cells = <2>;
> > -	};
> > -
> > -	sc16is752: sc16is752@54 {
> > -		compatible = "nxp,sc16is752";
> > -		reg = <0x54>;
> > -		clocks = <&clk20m>;
> > -		interrupt-parent = <&gpio3>;
> > -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> > -	};
> > -
> > -* spi as bus
> > -
> > -Required properties:
> > -- compatible: Should be one of the following:
> > -  - "nxp,sc16is740" for NXP SC16IS740,
> > -  - "nxp,sc16is741" for NXP SC16IS741,
> > -  - "nxp,sc16is750" for NXP SC16IS750,
> > -  - "nxp,sc16is752" for NXP SC16IS752,
> > -  - "nxp,sc16is760" for NXP SC16IS760,
> > -  - "nxp,sc16is762" for NXP SC16IS762.
> > -- reg: SPI chip select number.
> > -- interrupts: Specifies the interrupt source of the parent interrupt
> > -  controller. The format of the interrupt specifier depends on the
> > -  parent interrupt controller.
> > -- clocks: phandle to the IC source clock.
> > -
> > -Optional properties:
> > -- gpio-controller: Marks the device node as a GPIO controller.
> > -- #gpio-cells: Should be two. The first cell is the GPIO number and
> > -  the second cell is used to specify the GPIO polarity:
> > -    0 = active high,
> > -    1 = active low.
> > -- nxp,irda-mode-ports: An array that lists the indices of the port that
> > -		       should operate in IrDA mode.
> > -- nxp,modem-control-line-ports: An array that lists the indices of the port that
> > -				should have shared GPIO lines configured as
> > -				modem control lines.
> > -
> > -Example:
> > -	sc16is750: sc16is750@0 {
> > -		compatible = "nxp,sc16is750";
> > -		reg = <0>;
> > -		clocks = <&clk20m>;
> > -		interrupt-parent = <&gpio3>;
> > -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -		gpio-controller;
> > -		#gpio-cells = <2>;
> > -	};
> > -
> > -	sc16is752: sc16is752@1 {
> > -		compatible = "nxp,sc16is752";
> > -		reg = <1>;
> > -		clocks = <&clk20m>;
> > -		interrupt-parent = <&gpio3>;
> > -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> > -		gpio-controller; /* Port 0 as GPIOs */
> > -		#gpio-cells = <2>;
> > -	};
> > -
> > -	sc16is752: sc16is752@2 {
> > -		compatible = "nxp,sc16is752";
> > -		reg = <2>;
> > -		clocks = <&clk20m>;
> > -		interrupt-parent = <&gpio3>;
> > -		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > -		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> > -	};
> > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > new file mode 100644
> > index 000000000000..508639e09e06
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> > @@ -0,0 +1,126 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/serial/nxp,sc16is7xx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP SC16IS7xx advanced Universal Asynchronous Receiver-Transmitter (UART)
> > +
> > +maintainers:
> > +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nxp,sc16is740
> > +      - nxp,sc16is741
> > +      - nxp,sc16is750
> > +      - nxp,sc16is752
> > +      - nxp,sc16is760
> > +      - nxp,sc16is762
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: Reference to the IC source clock.
> > +
> > +  clock-frequency:
> > +    description: |
> > +      When there is no clock provider visible to the platform, this
> > +      is the source crystal or external clock frequency for the IC in Hz.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Don't need a type here. 'clock-frequency' is already defined.

Hi Rob,
will be done for V2.


> > +    minimum: 1
> > +    maximum: 80000000
> > +
> > +  gpio-controller:
> > +    description: Marks the device node as a GPIO controller.
> > +    type: boolean
> 
> Just: 
> 
> gpio-controller: true

Done.

Thank you,
Hugo.

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Hugo Villeneuve]

On Thu, 21 Sep 2023 11:24:24 -0500
Rob Herring <robh@kernel.org> wrote:

> On Wed, Sep 20, 2023 at 11:20:14AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > The NXP-specific "irda-mode-ports" property lacks a proper vendor
> > prefix. Add "nxp," prefix to comply with DT best practises.
> 
> The opposite direction would be worth carrying in the driver, but I 
> don't think adding a vendor prefix is. I would just leave this as-is.
> 
> Rob

Hi Rob,
I am not following you, can you clarify?

Hugo.

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Thu, Sep 21, 2023 at 11:30:40AM -0400, Hugo Villeneuve wrote:
> On Thu, 21 Sep 2023 14:45:06 +0100
> Conor Dooley <conor@kernel.org> wrote:
> > On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:

> > > +  clocks:
> > > +    maxItems: 1
> > > +    description: Reference to the IC source clock.
> > 
> > You could probably drop this, if it only has one clock it's a bit
> > redundant.
> 
> I don't understand, because there is already a 'clocks' property in
> the examples (and also in my real board DTS file) and if I remove it
> here it will cause a warning:
> 
> serial@51: Unevaluated properties are not allowed ('clocks' was
> unexpected)

Sorry, I was talking about the description.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/4] dt-bindings: sc16is7xx: convert to YAML

[Hugo Villeneuve]

On Fri, 22 Sep 2023 12:35:07 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Thu, Sep 21, 2023 at 11:30:40AM -0400, Hugo Villeneuve wrote:
> > On Thu, 21 Sep 2023 14:45:06 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > > On Wed, Sep 20, 2023 at 11:20:15AM -0400, Hugo Villeneuve wrote:
> 
> > > > +  clocks:
> > > > +    maxItems: 1
> > > > +    description: Reference to the IC source clock.
> > > 
> > > You could probably drop this, if it only has one clock it's a bit
> > > redundant.
> > 
> > I don't understand, because there is already a 'clocks' property in
> > the examples (and also in my real board DTS file) and if I remove it
> > here it will cause a warning:
> > 
> > serial@51: Unevaluated properties are not allowed ('clocks' was
> > unexpected)
> 
> Sorry, I was talking about the description.

OK, makes sense. I will remove it.

Hugo.

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Rob Herring]

On Thu, Sep 21, 2023 at 11:32 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> On Thu, 21 Sep 2023 11:24:24 -0500
> Rob Herring <robh@kernel.org> wrote:
>
> > On Wed, Sep 20, 2023 at 11:20:14AM -0400, Hugo Villeneuve wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > >
> > > The NXP-specific "irda-mode-ports" property lacks a proper vendor
> > > prefix. Add "nxp," prefix to comply with DT best practises.
> >
> > The opposite direction would be worth carrying in the driver, but I
> > don't think adding a vendor prefix is. I would just leave this as-is.
> >
> > Rob
>
> Hi Rob,
> I am not following you, can you clarify?

Just keep the property as-is and don't add a 'nxp,' prefixed version.
It's not worth carrying the fallback in the driver.

If you were dropping a vendor prefix from a property, then that's
worth it because there might be other users added.

Rob

Re: [PATCH 3/4] dt-bindings: sc16is7xx: add vendor prefix to irda-mode-ports property

[Hugo Villeneuve]

On Fri, 22 Sep 2023 14:52:06 -0500
Rob Herring <robh@kernel.org> wrote:

> On Thu, Sep 21, 2023 at 11:32 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > On Thu, 21 Sep 2023 11:24:24 -0500
> > Rob Herring <robh@kernel.org> wrote:
> >
> > > On Wed, Sep 20, 2023 at 11:20:14AM -0400, Hugo Villeneuve wrote:
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > >
> > > > The NXP-specific "irda-mode-ports" property lacks a proper vendor
> > > > prefix. Add "nxp," prefix to comply with DT best practises.
> > >
> > > The opposite direction would be worth carrying in the driver, but I
> > > don't think adding a vendor prefix is. I would just leave this as-is.
> > >
> > > Rob
> >
> > Hi Rob,
> > I am not following you, can you clarify?
> 
> Just keep the property as-is and don't add a 'nxp,' prefixed version.
> It's not worth carrying the fallback in the driver.
> 
> If you were dropping a vendor prefix from a property, then that's
> worth it because there might be other users added.
> 
> Rob

Hi Rob,
ok, then I will drop this patch as well as the other titled
'serial: sc16is7xx: recognize "nxp,irda-mode-ports" DT property' for
V2.

Hugo.