Hi,
This converts all MIPI HSI subystem DT bindings to YAML.
I ran the following tests:
1. Check binding files
make -j$(nproc) dt_binding_check DT_SCHEMA_FILES=/hsi/
2. Check OMAP3 and nokia-modem DT
make -j$(nproc) CHECK_DTBS=y ti/omap/omap3-n900.dtb ti/omap/omap3-n950.dtb ti/omap/omap3-n9.dtb
3. Check OMAP4 DT (OMAP4 HSI is not used upstream, so one is enough)
make -j$(nproc) CHECK_DTBS=y ti/omap/omap4-droid4-xt894.dtb
FWIW I noticed a lots of warnings for OMAP3 & OMAP4, but
none related to HSI/SSI.
Greetings,
-- Sebastian
---
Sebastian Reichel (3):
dt-bindings: hsi: hsi-client: convert to YAML
dt-bindings: hsi: nokia-modem: convert to YAML
dt-bindings: hsi: omap-ssi: convert to YAML
.../devicetree/bindings/hsi/client-devices.txt | 44 -----
.../devicetree/bindings/hsi/hsi-client.yaml | 84 +++++++++
.../devicetree/bindings/hsi/nokia-modem.txt | 59 -------
.../devicetree/bindings/hsi/nokia-modem.yaml | 101 +++++++++++
Documentation/devicetree/bindings/hsi/omap-ssi.txt | 102 -----------
.../devicetree/bindings/hsi/ti,omap-ssi.yaml | 196 +++++++++++++++++++++
6 files changed, 381 insertions(+), 205 deletions(-)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240325-hsi-dt-binding-a5d662e3d601
Best regards,
--
Sebastian Reichel <[email protected]>
Convert the legacy txt binding to modern YAML.
No semantic change.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../devicetree/bindings/hsi/nokia-modem.txt | 59 ------------
.../devicetree/bindings/hsi/nokia-modem.yaml | 101 +++++++++++++++++++++
2 files changed, 101 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/hsi/nokia-modem.txt b/Documentation/devicetree/bindings/hsi/nokia-modem.txt
deleted file mode 100644
index 53de1d9d0b95..000000000000
--- a/Documentation/devicetree/bindings/hsi/nokia-modem.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-Nokia modem client bindings
-
-The Nokia modem HSI client follows the common HSI client binding
-and inherits all required properties. The following additional
-properties are needed by the Nokia modem HSI client:
-
-Required properties:
-- compatible: Should be one of
- "nokia,n900-modem"
- "nokia,n950-modem"
- "nokia,n9-modem"
-- hsi-channel-names: Should contain the following strings
- "mcsaab-control"
- "speech-control"
- "speech-data"
- "mcsaab-data"
-- gpios: Should provide a GPIO handler for each GPIO listed in
- gpio-names
-- gpio-names: Should contain the following strings
- "cmt_apeslpx" (for n900, n950, n9)
- "cmt_rst_rq" (for n900, n950, n9)
- "cmt_en" (for n900, n950, n9)
- "cmt_rst" (for n900)
- "cmt_bsi" (for n900)
-- interrupts: Should be IRQ handle for modem's reset indication
-
-Example:
-
-&ssi_port {
- modem: hsi-client {
- compatible = "nokia,n900-modem";
-
- pinctrl-names = "default";
- pinctrl-0 = <&modem_pins>;
-
- hsi-channel-ids = <0>, <1>, <2>, <3>;
- hsi-channel-names = "mcsaab-control",
- "speech-control",
- "speech-data",
- "mcsaab-data";
- hsi-speed-kbps = <55000>;
- hsi-mode = "frame";
- hsi-flow = "synchronized";
- hsi-arb-mode = "round-robin";
-
- interrupts-extended = <&gpio3 8 IRQ_TYPE_EDGE_FALLING>; /* 72 */
-
- gpios = <&gpio3 6 GPIO_ACTIVE_HIGH>, /* 70 */
- <&gpio3 9 GPIO_ACTIVE_HIGH>, /* 73 */
- <&gpio3 10 GPIO_ACTIVE_HIGH>, /* 74 */
- <&gpio3 11 GPIO_ACTIVE_HIGH>, /* 75 */
- <&gpio5 29 GPIO_ACTIVE_HIGH>; /* 157 */
- gpio-names = "cmt_apeslpx",
- "cmt_rst_rq",
- "cmt_en",
- "cmt_rst",
- "cmt_bsi";
- };
-};
diff --git a/Documentation/devicetree/bindings/hsi/nokia-modem.yaml b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
new file mode 100644
index 000000000000..c57cbcc7b722
--- /dev/null
+++ b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hsi/nokia-modem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nokia modem
+
+maintainers:
+ - Sebastian Reichel <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - nokia,n900-modem
+ - nokia,n950-modem
+ - nokia,n9-modem
+
+ hsi-channel-names:
+ items:
+ - const: mcsaab-control
+ - const: speech-control
+ - const: speech-data
+ - const: mcsaab-data
+
+ interrupts:
+ items:
+ - description: modem reset indication
+
+ gpios:
+ minItems: 3
+ maxItems: 5
+
+ gpio-names:
+ items:
+ - const: cmt_apeslpx
+ - const: cmt_rst_rq
+ - const: cmt_en
+ - const: cmt_rst
+ - const: cmt_bsi
+ minItems: 3
+
+required:
+ - gpios
+ - gpio-names
+ - interrupts
+
+allOf:
+ - $ref: hsi-client.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nokia,n950-modem
+ - nokia,n9-modem
+ then:
+ properties:
+ gpios:
+ maxItems: 3
+ gpio-names:
+ maxItems: 3
+ else:
+ properties:
+ gpios:
+ minItems: 5
+ gpio-names:
+ minItems: 5
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ hsi-client {
+ compatible = "nokia,n900-modem";
+
+ hsi-channel-ids = <0>, <1>, <2>, <3>;
+ hsi-channel-names = "mcsaab-control",
+ "speech-control",
+ "speech-data",
+ "mcsaab-data";
+ hsi-speed-kbps = <55000>;
+ hsi-mode = "frame";
+ hsi-flow = "synchronized";
+ hsi-arb-mode = "round-robin";
+
+ interrupts-extended = <&gpio3 8 IRQ_TYPE_EDGE_FALLING>;
+
+ gpios = <&gpio3 6 GPIO_ACTIVE_HIGH>,
+ <&gpio3 9 GPIO_ACTIVE_HIGH>,
+ <&gpio3 10 GPIO_ACTIVE_HIGH>,
+ <&gpio3 11 GPIO_ACTIVE_HIGH>,
+ <&gpio5 29 GPIO_ACTIVE_HIGH>;
+ gpio-names = "cmt_apeslpx",
+ "cmt_rst_rq",
+ "cmt_en",
+ "cmt_rst",
+ "cmt_bsi";
+ };
--
2.43.0
Convert the legacy txt binding to modern YAML and rename from
client-devices to hsi-client. No semantic change.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../devicetree/bindings/hsi/client-devices.txt | 44 ------------
.../devicetree/bindings/hsi/hsi-client.yaml | 84 ++++++++++++++++++++++
2 files changed, 84 insertions(+), 44 deletions(-)
diff --git a/Documentation/devicetree/bindings/hsi/client-devices.txt b/Documentation/devicetree/bindings/hsi/client-devices.txt
deleted file mode 100644
index 104c9a3e57a4..000000000000
--- a/Documentation/devicetree/bindings/hsi/client-devices.txt
+++ /dev/null
@@ -1,44 +0,0 @@
-Each HSI port is supposed to have one child node, which
-symbols the remote device connected to the HSI port. The
-following properties are standardized for HSI clients:
-
-Required HSI configuration properties:
-
-- hsi-channel-ids: A list of channel ids
-
-- hsi-rx-mode: Receiver Bit transmission mode ("stream" or "frame")
-- hsi-tx-mode: Transmitter Bit transmission mode ("stream" or "frame")
-- hsi-mode: May be used instead hsi-rx-mode and hsi-tx-mode if
- the transmission mode is the same for receiver and
- transmitter
-- hsi-speed-kbps: Max bit transmission speed in kbit/s
-- hsi-flow: RX flow type ("synchronized" or "pipeline")
-- hsi-arb-mode: Arbitration mode for TX frame ("round-robin", "priority")
-
-Optional HSI configuration properties:
-
-- hsi-channel-names: A list with one name per channel specified in the
- hsi-channel-ids property
-
-
-Device Tree node example for an HSI client:
-
-hsi-controller {
- hsi-port {
- modem: hsi-client {
- compatible = "nokia,n900-modem";
-
- hsi-channel-ids = <0>, <1>, <2>, <3>;
- hsi-channel-names = "mcsaab-control",
- "speech-control",
- "speech-data",
- "mcsaab-data";
- hsi-speed-kbps = <55000>;
- hsi-mode = "frame";
- hsi-flow = "synchronized";
- hsi-arb-mode = "round-robin";
-
- /* more client specific properties */
- };
- };
-};
diff --git a/Documentation/devicetree/bindings/hsi/hsi-client.yaml b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
new file mode 100644
index 000000000000..df6e1fdd2702
--- /dev/null
+++ b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hsi/hsi-client.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HSI bus peripheral
+
+description:
+ Each HSI port is supposed to have one child node, which
+ symbols the remote device connected to the HSI port.
+
+maintainers:
+ - Sebastian Reichel <[email protected]>
+
+properties:
+ $nodename:
+ const: hsi-client
+
+ hsi-channel-ids:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 8
+
+ hsi-channel-names:
+ minItems: 1
+ maxItems: 8
+
+ hsi-rx-mode:
+ enum: [stream, frame]
+ description: Receiver Bit transmission mode
+
+ hsi-tx-mode:
+ enum: [stream, frame]
+ description: Transmitter Bit transmission mode
+
+ hsi-mode:
+ enum: [stream, frame]
+ description:
+ May be used instead hsi-rx-mode and hsi-tx-mode if the
+ transmission mode is the same for receiver and transmitter.
+
+ hsi-speed-kbps:
+ description: Max bit transmission speed in kbit/s
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ hsi-flow:
+ enum: [synchronized, pipeline]
+ description: RX flow type
+
+ hsi-arb-mode:
+ enum: [round-robin, priority]
+ description: Arbitration mode for TX frame
+
+additionalProperties: true
+
+required:
+ - compatible
+ - hsi-channel-ids
+ - hsi-speed-kbps
+ - hsi-flow
+ - hsi-arb-mode
+
+anyOf:
+ - required:
+ - hsi-mode
+ - required:
+ - hsi-rx-mode
+ - hsi-tx-mode
+
+allOf:
+ - if:
+ required:
+ - hsi-mode
+ then:
+ properties:
+ hsi-rx-mode: false
+ hsi-tx-mode: false
+ - if:
+ required:
+ - hsi-rx-mode
+ then:
+ properties:
+ hsi-mode: false
--
2.43.0
Convert the legacy txt binding to modern YAML.
No semantic change.
Signed-off-by: Sebastian Reichel <[email protected]>
---
Documentation/devicetree/bindings/hsi/omap-ssi.txt | 102 -----------
.../devicetree/bindings/hsi/ti,omap-ssi.yaml | 196 +++++++++++++++++++++
2 files changed, 196 insertions(+), 102 deletions(-)
diff --git a/Documentation/devicetree/bindings/hsi/omap-ssi.txt b/Documentation/devicetree/bindings/hsi/omap-ssi.txt
deleted file mode 100644
index 77a0c3c3036e..000000000000
--- a/Documentation/devicetree/bindings/hsi/omap-ssi.txt
+++ /dev/null
@@ -1,102 +0,0 @@
-OMAP SSI controller bindings
-
-OMAP3's Synchronous Serial Interface (SSI) controller implements a
-legacy variant of MIPI's High Speed Synchronous Serial Interface (HSI),
-while the controller found inside OMAP4 is supposed to be fully compliant
-with the HSI standard.
-
-Required properties:
-- compatible: Should include "ti,omap3-ssi" or "ti,omap4-hsi"
-- reg-names: Contains the values "sys" and "gdd" (in this order).
-- reg: Contains a matching register specifier for each entry
- in reg-names.
-- interrupt-names: Contains the value "gdd_mpu".
-- interrupts: Contains matching interrupt information for each entry
- in interrupt-names.
-- ranges: Represents the bus address mapping between the main
- controller node and the child nodes below.
-- clock-names: Must include the following entries:
- "ssi_ssr_fck": The OMAP clock of that name
- "ssi_sst_fck": The OMAP clock of that name
- "ssi_ick": The OMAP clock of that name
-- clocks: Contains a matching clock specifier for each entry in
- clock-names.
-- #address-cells: Should be set to <1>
-- #size-cells: Should be set to <1>
-
-Each port is represented as a sub-node of the ti,omap3-ssi device.
-
-Required Port sub-node properties:
-- compatible: Should be set to the following value
- ti,omap3-ssi-port (applicable to OMAP34xx devices)
- ti,omap4-hsi-port (applicable to OMAP44xx devices)
-- reg-names: Contains the values "tx" and "rx" (in this order).
-- reg: Contains a matching register specifier for each entry
- in reg-names.
-- interrupts: Should contain interrupt specifiers for mpu interrupts
- 0 and 1 (in this order).
-- ti,ssi-cawake-gpio: Defines which GPIO pin is used to signify CAWAKE
- events for the port. This is an optional board-specific
- property. If it's missing the port will not be
- enabled.
-
-Optional properties:
-- ti,hwmods: Shall contain TI interconnect module name if needed
- by the SoC
-
-Example for Nokia N900:
-
-ssi-controller@48058000 {
- compatible = "ti,omap3-ssi";
-
- /* needed until hwmod is updated to use the compatible string */
- ti,hwmods = "ssi";
-
- reg = <0x48058000 0x1000>,
- <0x48059000 0x1000>;
- reg-names = "sys",
- "gdd";
-
- interrupts = <55>;
- interrupt-names = "gdd_mpu";
-
- clocks = <&ssi_ssr_fck>,
- <&ssi_sst_fck>,
- <&ssi_ick>;
- clock-names = "ssi_ssr_fck",
- "ssi_sst_fck",
- "ssi_ick";
-
- #address-cells = <1>;
- #size-cells = <1>;
- ranges;
-
- ssi-port@4805a000 {
- compatible = "ti,omap3-ssi-port";
-
- reg = <0x4805a000 0x800>,
- <0x4805a800 0x800>;
- reg-names = "tx",
- "rx";
-
- interrupt-parent = <&intc>;
- interrupts = <67>,
- <68>;
-
- ti,ssi-cawake-gpio = <&gpio5 23 GPIO_ACTIVE_HIGH>; /* 151 */
- }
-
- ssi-port@4805a000 {
- compatible = "ti,omap3-ssi-port";
-
- reg = <0x4805b000 0x800>,
- <0x4805b800 0x800>;
- reg-names = "tx",
- "rx";
-
- interrupt-parent = <&intc>;
- interrupts = <69>,
- <70>;
-
- }
-}
diff --git a/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml b/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml
new file mode 100644
index 000000000000..eb82f85c25b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hsi/ti,omap-ssi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SSI Controller on OMAP SoCs
+
+description:
+ OMAP3's Synchronous Serial Interface (SSI) controller implements a
+ legacy variant of MIPI's High Speed Synchronous Serial Interface (HSI),
+ while the controller found inside OMAP4 is supposed to be fully compliant
+ with the HSI standard.
+
+maintainers:
+ - Sebastian Reichel <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - ti,omap3-ssi
+ - ti,omap4-hsi
+
+ reg:
+ items:
+ - description: registers for sys
+ - description: registers for gdd
+
+ reg-names:
+ items:
+ - const: sys
+ - const: gdd
+
+ ranges: true
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 3
+
+ clock-names:
+ minItems: 1
+ maxItems: 3
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ const: gdd_mpu
+
+ ti,hwmods:
+ const: ssi
+ deprecated: true
+
+patternProperties:
+ "[hs]si-port@[0-9a-f]+":
+ type: object
+
+ additionalProperties: false
+
+ properties:
+ compatible:
+ enum:
+ - ti,omap3-ssi-port
+ - ti,omap4-hsi-port
+
+ reg:
+ items:
+ - description: TX registers
+ - description: RX registers
+
+ reg-names:
+ items:
+ - const: tx
+ - const: rx
+
+ interrupts:
+ items:
+ - description: MPU interrupt 0
+ - description: MPU interrupt 1
+ minItems: 1
+
+ ti,ssi-cawake-gpio:
+ description: GPIO signifying CAWAKE events
+ maxItems: 1
+
+ hsi-client:
+ type: object
+ $ref: /schemas/hsi/hsi-client.yaml#
+
+ required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+
+ allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,omap3-ssi-port
+ then:
+ properties:
+ $nodename:
+ pattern: "^ssi-port@(.*)?$"
+ interrupts:
+ minItems: 2
+ else:
+ properties:
+ $nodename:
+ pattern: "^hsi-port@(.*)?$"
+ interrupts:
+ maxItems: 1
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - ranges
+ - "#address-cells"
+ - "#size-cells"
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,omap3-ssi
+ then:
+ properties:
+ clocks:
+ minItems: 3
+ clock-names:
+ items:
+ - const: ssi_ssr_fck
+ - const: ssi_sst_fck
+ - const: ssi_ick
+ else:
+ properties:
+ clocks:
+ maxItems: 1
+ clock-names:
+ items:
+ - const: hsi_fck
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ ssi-controller@48058000 {
+ compatible = "ti,omap3-ssi";
+ reg = <0x48058000 0x1000>,
+ <0x48059000 0x1000>;
+ reg-names = "sys", "gdd";
+ ranges;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ clocks = <&ssi_ssr_fck>,
+ <&ssi_sst_fck>,
+ <&ssi_ick>;
+ clock-names = "ssi_ssr_fck",
+ "ssi_sst_fck",
+ "ssi_ick";
+ interrupts = <55>;
+ interrupt-names = "gdd_mpu";
+
+ ssi-port@4805a000 {
+ compatible = "ti,omap3-ssi-port";
+ reg = <0x4805a000 0x800>,
+ <0x4805a800 0x800>;
+ reg-names = "tx", "rx";
+ interrupt-parent = <&intc>;
+ interrupts = <67>, <68>;
+ ti,ssi-cawake-gpio = <&gpio5 23 GPIO_ACTIVE_HIGH>;
+ };
+
+ ssi-port@4805b000 {
+ compatible = "ti,omap3-ssi-port";
+ reg = <0x4805b000 0x800>,
+ <0x4805b800 0x800>;
+ reg-names = "tx", "rx";
+ interrupt-parent = <&intc>;
+ interrupts = <69>, <70>;
+ };
+ };
--
2.43.0
On 25/03/2024 22:45, Sebastian Reichel wrote:
> Convert the legacy txt binding to modern YAML and rename from
> client-devices to hsi-client. No semantic change.
There is semantic change: missing example (which is reasonable for
shared schema) but more importantly: some properties are now excluding
each other.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
..
> diff --git a/Documentation/devicetree/bindings/hsi/hsi-client.yaml b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
> new file mode 100644
> index 000000000000..df6e1fdd2702
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hsi/hsi-client.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HSI bus peripheral
> +
> +description:
> + Each HSI port is supposed to have one child node, which
> + symbols the remote device connected to the HSI port.
> +
> +maintainers:
> + - Sebastian Reichel <[email protected]>
> +
> +properties:
> + $nodename:
> + const: hsi-client
Why? Does anything depend on this? It breaks generic-node-name rule. It
seems you need it only to match the schema, but this just point to main
problem - missing bus schema.
> +
> + hsi-channel-ids:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 8
> +
> + hsi-channel-names:
> + minItems: 1
> + maxItems: 8
> +
> + hsi-rx-mode:
> + enum: [stream, frame]
> + description: Receiver Bit transmission mode
> +
> + hsi-tx-mode:
> + enum: [stream, frame]
> + description: Transmitter Bit transmission mode
> +
> + hsi-mode:
> + enum: [stream, frame]
> + description:
> + May be used instead hsi-rx-mode and hsi-tx-mode if the
> + transmission mode is the same for receiver and transmitter.
> +
> + hsi-speed-kbps:
> + description: Max bit transmission speed in kbit/s
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + hsi-flow:
> + enum: [synchronized, pipeline]
> + description: RX flow type
> +
> + hsi-arb-mode:
> + enum: [round-robin, priority]
> + description: Arbitration mode for TX frame
> +
> +additionalProperties: true
> +
> +required:
> + - compatible
> + - hsi-channel-ids
> + - hsi-speed-kbps
> + - hsi-flow
> + - hsi-arb-mode
> +
> +anyOf:
> + - required:
> + - hsi-mode
> + - required:
> + - hsi-rx-mode
> + - hsi-tx-mode
> +
> +allOf:
> + - if:
> + required:
> + - hsi-mode
> + then:
> + properties:
> + hsi-rx-mode: false
> + hsi-tx-mode: false
I don't understand what you are trying to achieve here and with anyOf.
It looks like just oneOf. OTOH, old binding did not exclude these
properties.
> + - if:
> + required:
> + - hsi-rx-mode
> + then:
> + properties:
> + hsi-mode: false
>
Best regards,
Krzysztof
On 25/03/2024 22:45, Sebastian Reichel wrote:
> Convert the legacy txt binding to modern YAML.
> No semantic change.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../devicetree/bindings/hsi/nokia-modem.txt | 59 ------------
> .../devicetree/bindings/hsi/nokia-modem.yaml | 101 +++++++++++++++++++++
> 2 files changed, 101 insertions(+), 59 deletions(-)
>
> -};
> diff --git a/Documentation/devicetree/bindings/hsi/nokia-modem.yaml b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
> new file mode 100644
> index 000000000000..c57cbcc7b722
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
Filename should match compatibles. nokia,n9-modem.yaml or nokia,modem.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hsi/nokia-modem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nokia modem
> +
> +maintainers:
> + - Sebastian Reichel <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - nokia,n900-modem
> + - nokia,n950-modem
> + - nokia,n9-modem
> +
Aren't hsi-channel-ids related to hsi-channel-names? If so, they should
be here with constraints.
> + hsi-channel-names:
> + items:
> + - const: mcsaab-control
> + - const: speech-control
> + - const: speech-data
> + - const: mcsaab-data
> +
> + interrupts:
> + items:
> + - description: modem reset indication
> +
> + gpios:
> + minItems: 3
> + maxItems: 5
> +
> + gpio-names:
> + items:
> + - const: cmt_apeslpx
> + - const: cmt_rst_rq
> + - const: cmt_en
> + - const: cmt_rst
> + - const: cmt_bsi
> + minItems: 3
> +
> +required:
> + - gpios
> + - gpio-names
> + - interrupts
> +
> +allOf:
> + - $ref: hsi-client.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - nokia,n950-modem
> + - nokia,n9-modem
> + then:
> + properties:
> + gpios:
> + maxItems: 3
> + gpio-names:
> + maxItems: 3
> + else:
> + properties:
> + gpios:
> + minItems: 5
> + gpio-names:
> + minItems: 5
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + hsi-client {
This should be "modem".
> + compatible = "nokia,n900-modem";
> +
Best regards,
Krzysztof
On 25/03/2024 22:45, Sebastian Reichel wrote:
> Convert the legacy txt binding to modern YAML.
> No semantic change.
You deprecated a property: ti,hwmods. Also will be one more change:
ti,ssi-cawake-gpio->gpios
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> Documentation/devicetree/bindings/hsi/omap-ssi.txt | 102 -----------
> .../devicetree/bindings/hsi/ti,omap-ssi.yaml | 196 +++++++++++++++++++++
> 2 files changed, 196 insertions(+), 102 deletions(-)
>
..
> diff --git a/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml b/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml
> new file mode 100644
> index 000000000000..eb82f85c25b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hsi/ti,omap-ssi.yaml
> @@ -0,0 +1,196 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hsi/ti,omap-ssi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SSI Controller on OMAP SoCs
> +
> +description:
> + OMAP3's Synchronous Serial Interface (SSI) controller implements a
> + legacy variant of MIPI's High Speed Synchronous Serial Interface (HSI),
> + while the controller found inside OMAP4 is supposed to be fully compliant
> + with the HSI standard.
> +
> +maintainers:
> + - Sebastian Reichel <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - ti,omap3-ssi
> + - ti,omap4-hsi
> +
> + reg:
> + items:
> + - description: registers for sys
> + - description: registers for gdd
> +
> + reg-names:
> + items:
> + - const: sys
> + - const: gdd
> +
> + ranges: true
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 3
> +
> + clock-names:
> + minItems: 1
> + maxItems: 3
> +
> + interrupts:
> + maxItems: 1
> +
> + interrupt-names:
> + const: gdd_mpu
> +
> + ti,hwmods:
> + const: ssi
> + deprecated: true
> +
> +patternProperties:
> + "[hs]si-port@[0-9a-f]+":
Does anything actually depends on the name? Can these be "port@[0-9a-f]+"?
> + type: object
> +
Drop blank line.
> + additionalProperties: false
> +
> + properties:
> + compatible:
> + enum:
> + - ti,omap3-ssi-port
> + - ti,omap4-hsi-port
> +
> + reg:
> + items:
> + - description: TX registers
> + - description: RX registers
> +
> + reg-names:
> + items:
> + - const: tx
> + - const: rx
> +
> + interrupts:
> + items:
> + - description: MPU interrupt 0
> + - description: MPU interrupt 1
> + minItems: 1
> +
> + ti,ssi-cawake-gpio:
ti,ssi-cawake-gpios
> + description: GPIO signifying CAWAKE events
> + maxItems: 1
> +
> + hsi-client:
> + type: object
On this level, should be explicit: additionalProperties: true
> + $ref: /schemas/hsi/hsi-client.yaml#
> +
> + required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> +
> + allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,omap3-ssi-port
> + then:
> + properties:
> + $nodename:
> + pattern: "^ssi-port@(.*)?$"
> + interrupts:
> + minItems: 2
> + else:
> + properties:
> + $nodename:
> + pattern: "^hsi-port@(.*)?$"
> + interrupts:
> + maxItems: 1
> +
> +additionalProperties: false
This goes after allOf: block
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - ranges
> + - "#address-cells"
> + - "#size-cells"
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,omap3-ssi
> + then:
> + properties:
> + clocks:
> + minItems: 3
> + clock-names:
> + items:
> + - const: ssi_ssr_fck
> + - const: ssi_sst_fck
> + - const: ssi_ick
> + else:
> + properties:
> + clocks:
> + maxItems: 1
> + clock-names:
> + items:
> + - const: hsi_fck
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + ssi-controller@48058000 {
> + compatible = "ti,omap3-ssi";
> + reg = <0x48058000 0x1000>,
> + <0x48059000 0x1000>;
> + reg-names = "sys", "gdd";
> + ranges;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&ssi_ssr_fck>,
> + <&ssi_sst_fck>,
> + <&ssi_ick>;
> + clock-names = "ssi_ssr_fck",
> + "ssi_sst_fck",
> + "ssi_ick";
> + interrupts = <55>;
> + interrupt-names = "gdd_mpu";
> +
> + ssi-port@4805a000 {
> + compatible = "ti,omap3-ssi-port";
Use 4 spaces for example indentation.
> + reg = <0x4805a000 0x800>,
Best regards,
Krzysztof
Hi,
On Tue, Mar 26, 2024 at 08:18:39AM +0100, Krzysztof Kozlowski wrote:
> On 25/03/2024 22:45, Sebastian Reichel wrote:
> > Convert the legacy txt binding to modern YAML and rename from
> > client-devices to hsi-client. No semantic change.
>
> There is semantic change: missing example (which is reasonable for
> shared schema)
Right, I should have mentioned that.
> but more importantly: some properties are now excluding each
> other.
I think that requirement was already there.
> > Signed-off-by: Sebastian Reichel <[email protected]>
> > ---
>
> ...
>
> > diff --git a/Documentation/devicetree/bindings/hsi/hsi-client.yaml b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
> > new file mode 100644
> > index 000000000000..df6e1fdd2702
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hsi/hsi-client.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hsi/hsi-client.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HSI bus peripheral
> > +
> > +description:
> > + Each HSI port is supposed to have one child node, which
> > + symbols the remote device connected to the HSI port.
> > +
> > +maintainers:
> > + - Sebastian Reichel <[email protected]>
> > +
> > +properties:
> > + $nodename:
> > + const: hsi-client
>
> Why? Does anything depend on this? It breaks generic-node-name rule. It
> seems you need it only to match the schema, but this just point to main
> problem - missing bus schema.
Ah, that's a good point. It makes a lot more sense to get the
nodename from the actual client. I will work this over.
> > +
> > + hsi-channel-ids:
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + minItems: 1
> > + maxItems: 8
> > +
> > + hsi-channel-names:
> > + minItems: 1
> > + maxItems: 8
> > +
> > + hsi-rx-mode:
> > + enum: [stream, frame]
> > + description: Receiver Bit transmission mode
> > +
> > + hsi-tx-mode:
> > + enum: [stream, frame]
> > + description: Transmitter Bit transmission mode
> > +
> > + hsi-mode:
> > + enum: [stream, frame]
> > + description:
> > + May be used instead hsi-rx-mode and hsi-tx-mode if the
> > + transmission mode is the same for receiver and transmitter.
> > +
> > + hsi-speed-kbps:
> > + description: Max bit transmission speed in kbit/s
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + hsi-flow:
> > + enum: [synchronized, pipeline]
> > + description: RX flow type
> > +
> > + hsi-arb-mode:
> > + enum: [round-robin, priority]
> > + description: Arbitration mode for TX frame
> > +
> > +additionalProperties: true
> > +
> > +required:
> > + - compatible
> > + - hsi-channel-ids
> > + - hsi-speed-kbps
> > + - hsi-flow
> > + - hsi-arb-mode
> > +
> > +anyOf:
> > + - required:
> > + - hsi-mode
> > + - required:
> > + - hsi-rx-mode
> > + - hsi-tx-mode
> > +
> > +allOf:
> > + - if:
> > + required:
> > + - hsi-mode
> > + then:
> > + properties:
> > + hsi-rx-mode: false
> > + hsi-tx-mode: false
>
> I don't understand what you are trying to achieve here and with anyOf.
> It looks like just oneOf. OTOH, old binding did not exclude these
> properties.
So the anyOf ensures, that either hsi-mode or hsi-rx-mode +
hsi-tx-mode are specified. Those properties were previously
listed as required and they are indeed mandatory by the Linux
kernel implementation.
The old binding also has this:
hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode
So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not
all properties at the same time. That's what the allOf ensures:
if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may
not be specified.
> > + - if:
> > + required:
> > + - hsi-rx-mode
> > + then:
> > + properties:
> > + hsi-mode: false
> >
Thanks for the review,
-- Sebastian
On 26/03/2024 13:45, Sebastian Reichel wrote:
>
>> but more importantly: some properties are now excluding each
>> other.
>
> I think that requirement was already there.
Right.
..
>>> +
>>> +allOf:
>>> + - if:
>>> + required:
>>> + - hsi-mode
>>> + then:
>>> + properties:
>>> + hsi-rx-mode: false
>>> + hsi-tx-mode: false
>>
>> I don't understand what you are trying to achieve here and with anyOf.
>> It looks like just oneOf. OTOH, old binding did not exclude these
>> properties.
>
> So the anyOf ensures, that either hsi-mode or hsi-rx-mode +
> hsi-tx-mode are specified. Those properties were previously
Not entirely. anyOf should succeed also when none of them are present,
which is not what you want in such case.
> listed as required and they are indeed mandatory by the Linux
> kernel implementation.
>
> The old binding also has this:
>
> hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode
>
> So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not
> all properties at the same time. That's what the allOf ensures:
> if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may
> not be specified.
Then wouldn't this work for you:
https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
?
But if you really want them to be optional but excluding, then simpler
syntax is:
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
Hi,
On Tue, Mar 26, 2024 at 08:21:05AM +0100, Krzysztof Kozlowski wrote:
> On 25/03/2024 22:45, Sebastian Reichel wrote:
> > Convert the legacy txt binding to modern YAML.
> > No semantic change.
> >
> > Signed-off-by: Sebastian Reichel <[email protected]>
> > ---
> > .../devicetree/bindings/hsi/nokia-modem.txt | 59 ------------
> > .../devicetree/bindings/hsi/nokia-modem.yaml | 101 +++++++++++++++++++++
> > 2 files changed, 101 insertions(+), 59 deletions(-)
> >
>
>
> > -};
> > diff --git a/Documentation/devicetree/bindings/hsi/nokia-modem.yaml b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
> > new file mode 100644
> > index 000000000000..c57cbcc7b722
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hsi/nokia-modem.yaml
>
> Filename should match compatibles. nokia,n9-modem.yaml or nokia,modem.yaml
Ack, I will switch to nokia,modem.yaml in v2.
> > @@ -0,0 +1,101 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hsi/nokia-modem.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Nokia modem
> > +
> > +maintainers:
> > + - Sebastian Reichel <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - nokia,n900-modem
> > + - nokia,n950-modem
> > + - nokia,n9-modem
> > +
>
> Aren't hsi-channel-ids related to hsi-channel-names? If so, they
> should be here with constraints.
Indeed. I forgot them, since they were not in the old binding :)
I will constraint them to min/max items 4 in v2.
> > + hsi-channel-names:
> > + items:
> > + - const: mcsaab-control
> > + - const: speech-control
> > + - const: speech-data
> > + - const: mcsaab-data
[...]
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + hsi-client {
>
> This should be "modem".
Ack.
>
> > + compatible = "nokia,n900-modem";
> > +
Thanks for the review,
-- Sebastian
Hi,
On Tue, Mar 26, 2024 at 08:26:54AM +0100, Krzysztof Kozlowski wrote:
> On 25/03/2024 22:45, Sebastian Reichel wrote:
> > Convert the legacy txt binding to modern YAML.
> > No semantic change.
>
> You deprecated a property: ti,hwmods. Also will be one more change:
> ti,ssi-cawake-gpio->gpios
and I will introduce a simple bus binding for referencing the
peripheral. I added a list to the commit message for v2.
> > Documentation/devicetree/bindings/hsi/omap-ssi.txt | 102 -----------
> > .../devicetree/bindings/hsi/ti,omap-ssi.yaml | 196 +++++++++++++++++++++
> > 2 files changed, 196 insertions(+), 102 deletions(-)
[...]
> > +patternProperties:
> > + "[hs]si-port@[0-9a-f]+":
>
> Does anything actually depends on the name? Can these be "port@[0-9a-f]+"?
That should work. The Linux driver is looping over all child nodes
and checking the compatible.
[...]
> > + ti,ssi-cawake-gpio:
>
> ti,ssi-cawake-gpios
mh, the kernel supports the extra s since 2016. I guess that should
be good enough. I will fix it in v2.
All other comments will be fixed in v2.
Thanks for the review,
-- Sebastian
Hi,
On Tue, Mar 26, 2024 at 01:56:22PM +0100, Krzysztof Kozlowski wrote:
> >>> +allOf:
> >>> + - if:
> >>> + required:
> >>> + - hsi-mode
> >>> + then:
> >>> + properties:
> >>> + hsi-rx-mode: false
> >>> + hsi-tx-mode: false
> >>
> >> I don't understand what you are trying to achieve here and with anyOf.
> >> It looks like just oneOf. OTOH, old binding did not exclude these
> >> properties.
> >
> > So the anyOf ensures, that either hsi-mode or hsi-rx-mode +
> > hsi-tx-mode are specified. Those properties were previously
>
> Not entirely. anyOf should succeed also when none of them are present,
> which is not what you want in such case.
Right, this should be oneOf instead of anyOf. I fixed that for v2.
> > listed as required and they are indeed mandatory by the Linux
> > kernel implementation.
> >
> > The old binding also has this:
> >
> > hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode
> >
> > So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not
> > all properties at the same time. That's what the allOf ensures:
> > if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may
> > not be specified.
>
> Then wouldn't this work for you:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
I suppose you mean using "then: not: required: PROPERTY" instead of
"then: PROPERTY: false"? The variant using "PROPERTY: false" is what
is being used in example-schema.yaml:
https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/example-schema.yaml#L225
IMHO the "not: required: property" is harder to understand. I would
expect that to mean "the property is not required (i.e. optional)"
instead of "the property is not allowed".
-- Sebastian
On 26/03/2024 16:15, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Mar 26, 2024 at 01:56:22PM +0100, Krzysztof Kozlowski wrote:
>>>>> +allOf:
>>>>> + - if:
>>>>> + required:
>>>>> + - hsi-mode
>>>>> + then:
>>>>> + properties:
>>>>> + hsi-rx-mode: false
>>>>> + hsi-tx-mode: false
>>>>
>>>> I don't understand what you are trying to achieve here and with anyOf.
>>>> It looks like just oneOf. OTOH, old binding did not exclude these
>>>> properties.
>>>
>>> So the anyOf ensures, that either hsi-mode or hsi-rx-mode +
>>> hsi-tx-mode are specified. Those properties were previously
>>
>> Not entirely. anyOf should succeed also when none of them are present,
>> which is not what you want in such case.
>
> Right, this should be oneOf instead of anyOf. I fixed that for v2.
>
>>> listed as required and they are indeed mandatory by the Linux
>>> kernel implementation.
>>>
>>> The old binding also has this:
>>>
>>> hsi-mode: May be used ***instead*** hsi-rx-mode and hsi-tx-mode
>>>
>>> So it's either hsi-rx-mode + hsi-tx-mode OR hsi-mode, but not
>>> all properties at the same time. That's what the allOf ensures:
>>> if hsi-mode is specified, then hsi-rx-mode and hsi-tx-mode may
>>> not be specified.
>>
>> Then wouldn't this work for you:
>> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
>
> I suppose you mean using "then: not: required: PROPERTY" instead of
> "then: PROPERTY: false"? The variant using "PROPERTY: false" is what
> is being used in example-schema.yaml:
No, I pointed to specific line with code for you.
>
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/example-schema.yaml#L225
>
> IMHO the "not: required: property" is harder to understand. I would
> expect that to mean "the property is not required (i.e. optional)"
> instead of "the property is not allowed".
Best regards,
Krzysztof