diff mbox series

[v4,1/6] dt-bindings: i3c: Convert controller description to yaml

Message ID 20210114175558.17097-2-miquel.raynal@bootlin.com
State Accepted
Commit 5e4cdca887fdb445f962b3dbc2a2514d7c025d9b
Headers show
Series Silvaco I3C master driver | expand

Commit Message

Miquel Raynal Jan. 14, 2021, 5:55 p.m. UTC
Attempting a conversion of the i3c.txt file to yaml schema with
minimal content changes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/i3c/i3c.txt | 140 -------------
 .../devicetree/bindings/i3c/i3c.yaml          | 186 ++++++++++++++++++
 2 files changed, 186 insertions(+), 140 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
 create mode 100644 Documentation/devicetree/bindings/i3c/i3c.yaml

Comments

Rob Herring Jan. 15, 2021, 5:03 p.m. UTC | #1
On Thu, Jan 14, 2021 at 06:55:53PM +0100, Miquel Raynal wrote:
> Attempting a conversion of the i3c.txt file to yaml schema with

> minimal content changes.

> 

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

> ---

>  Documentation/devicetree/bindings/i3c/i3c.txt | 140 -------------

>  .../devicetree/bindings/i3c/i3c.yaml          | 186 ++++++++++++++++++

>  2 files changed, 186 insertions(+), 140 deletions(-)

>  delete mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt

>  create mode 100644 Documentation/devicetree/bindings/i3c/i3c.yaml



> diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml

> new file mode 100644

> index 000000000000..79df533ab094

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml

> @@ -0,0 +1,186 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/i3c/i3c.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: I3C bus binding

> +

> +maintainers:

> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>

> +  - Miquel Raynal <miquel.raynal@bootlin.com>

> +

> +description: |

> +  I3C busses can be described with a node for the primary I3C controller device

> +  and a set of child nodes for each I2C or I3C slave on the bus. Each of them

> +  may, during the life of the bus, request mastership.

> +

> +properties:

> +  $nodename:

> +    pattern: "^i3c-master(@.*|-[0-9a-f])*$"

> +

> +  "#address-cells":

> +    const: 3

> +    description: |

> +      Each I2C device connected to the bus should be described in a subnode.

> +

> +      All I3C devices are supposed to support DAA (Dynamic Address Assignment),

> +      and are thus discoverable. So, by default, I3C devices do not have to be

> +      described in the device tree. This being said, one might want to attach

> +      extra resources to these devices, and those resources may have to be

> +      described in the device tree, which in turn means we have to describe

> +      I3C devices.

> +

> +      Another use case for describing an I3C device in the device tree is when

> +      this I3C device has a static I2C address and we want to assign it a

> +      specific I3C dynamic address before the DAA takes place (so that other

> +      devices on the bus can't take this dynamic address).

> +

> +  "#size-cells":

> +    const: 0

> +

> +  i3c-scl-hz:

> +    $ref: /schemas/types.yaml#/definitions/uint32


With a unit suffix, you don't need the type.

> +    description: |

> +      Frequency of the SCL signal used for I3C transfers. When undefined, the

> +      default value should be 12.5MHz.

> +

> +      May not be supported by all controllers.

> +

> +  i2c-scl-hz:

> +    $ref: /schemas/types.yaml#/definitions/uint32

> +    description: |

> +      Frequency of the SCL signal used for I2C transfers. When undefined, the

> +      default should be to look at LVR (Legacy Virtual Register) values of

> +      I2C devices described in the device tree to determine the maximum I2C

> +      frequency.

> +

> +      May not be supported by all controllers.

> +

> +required:

> +  - "#address-cells"

> +  - "#size-cells"

> +

> +patternProperties:

> +  "^.*@[0-9a-f]+$":


You can drop '^.*':

"@[0-9a-f]+$"

> +    type: object

> +    description: |

> +      I2C child, should be named: <device-type>@<i2c-address>

> +

> +      All properties described in Documentation/devicetree/bindings/i2c/i2c.txt

> +      are valid here, except the reg property whose content is changed.

> +

> +    properties:

> +      compatible:

> +        description:

> +          Compatible of the I2C device.

> +

> +      reg:

> +        items:

> +          - description: |

> +              1st cell

> +              ========

> +

> +              I2C address. 10 bit addressing is not supported. Devices with

> +              10-bit address can't be properly passed through DEFSLVS command.

> +

> +              2nd cell

> +              ========

> +

> +              Should be 0.

> +

> +              3rd cell

> +              ========

> +

> +              Shall encode the I3C LVR (Legacy Virtual Register):

> +              bit[31:8]: unused/ignored

> +              bit[7:5]: I2C device index. Possible values:

> +                * 0: I2C device has a 50 ns spike filter

> +                * 1: I2C device does not have a 50 ns spike filter but supports

> +                     high frequency on SCL

> +                * 2: I2C device does not have a 50 ns spike filter and is not

> +                     tolerant to high frequencies

> +                * 3-7: reserved

> +              bit[4]: tell whether the device operates in FM (Fast Mode) or

> +                      FM+ mode:

> +                * 0: FM+ mode

> +                * 1: FM mode

> +              bit[3:0]: device type

> +                * 0-15: reserved


We can do a bit better:

reg:
  items:
    - items:  # Note: drop the '-' if we support more than 1 entry
        - description: I2C address...
          maximum: 0x7f  # Not sure this works, do we support the high 
                         # flag bits here?
        - const: 0
        - description: I3C LVR (Legacy Virtual Register)...

> +

> +    required:

> +      - compatible

> +      - reg

> +

> +  "^.*@[0-9a-f]+,[0-9a-f]+$":

> +    type: object

> +    description: |

> +      I3C child, should be named: <device-type>@<static-i2c-address>,<i3c-pid>

> +

> +    properties:

> +      reg:

> +        items:

> +          - description: |

> +              1st cell

> +              ========

> +

> +              Encodes the static I2C address. Should be 0 if the device does not

> +              have one (0 is not a valid I2C address).

> +

> +              2nd and 3rd cells

> +              =================

> +

> +              ProvisionalID (following the PID definition provided by the I3C

> +              specification).

> +

> +              Cell 2 contains the manufacturer ID left-shifted by 1. Cell 3

> +              contains the ORing of the part ID left-shifted by 16, the instance

> +              ID left-shifted by 12 and extra information.


Similar rework here.

> +

> +      assigned-address:

> +        $ref: /schemas/types.yaml#/definitions/uint32

> +        minimum: 0x01

> +        maximum: 0xFF

> +        description: |

> +          Dynamic address to be assigned to this device. This property is only

> +          valid if the I3C device has a static address (first cell of the reg

> +          property != 0).

> +

> +    required:

> +      - reg

> +

> +additionalProperties: true

> +

> +examples:

> +  - |

> +    i3c-master@d040000 {

> +        compatible = "cdns,i3c-master";

> +        clocks = <&coreclock>, <&i3csysclock>;

> +        clock-names = "pclk", "sysclk";

> +        interrupts = <3 0>;

> +        reg = <0x0d040000 0x1000>;

> +        #address-cells = <3>;

> +        #size-cells = <0>;

> +        i2c-scl-hz = <100000>;

> +

> +        /* I2C device. */

> +        nunchuk: nunchuk@52 {

> +            compatible = "nintendo,nunchuk";

> +            reg = <0x52 0x0 0x10>;

> +        };

> +

> +        /* I3C device with a static I2C address. */

> +        thermal_sensor: sensor@68,39200144004 {

> +            reg = <0x68 0x392 0x144004>;

> +            assigned-address = <0xa>;

> +        };

> +

> +        /*

> +         * I3C device without a static I2C address but requiring

> +         * resources described in the DT.

> +         */

> +        sensor@0,39200154004 {

> +            reg = <0x0 0x392 0x154004>;

> +            clocks = <&clock_provider 0>;

> +        };

> +    };

> -- 

> 2.20.1

>
Miquel Raynal Jan. 18, 2021, 3:25 p.m. UTC | #2
Hi Rob,

> > +      reg:

> > +        items:

> > +          - description: |

> > +              1st cell

> > +              ========

> > +

> > +              I2C address. 10 bit addressing is not supported. Devices with

> > +              10-bit address can't be properly passed through DEFSLVS command.

> > +

> > +              2nd cell

> > +              ========

> > +

> > +              Should be 0.

> > +

> > +              3rd cell

> > +              ========

> > +

> > +              Shall encode the I3C LVR (Legacy Virtual Register):

> > +              bit[31:8]: unused/ignored

> > +              bit[7:5]: I2C device index. Possible values:

> > +                * 0: I2C device has a 50 ns spike filter

> > +                * 1: I2C device does not have a 50 ns spike filter but supports

> > +                     high frequency on SCL

> > +                * 2: I2C device does not have a 50 ns spike filter and is not

> > +                     tolerant to high frequencies

> > +                * 3-7: reserved

> > +              bit[4]: tell whether the device operates in FM (Fast Mode) or

> > +                      FM+ mode:

> > +                * 0: FM+ mode

> > +                * 1: FM mode

> > +              bit[3:0]: device type

> > +                * 0-15: reserved  

> 

> We can do a bit better:

> 

> reg:

>   items:

>     - items:  # Note: drop the '-' if we support more than 1 entry

>         - description: I2C address...

>           maximum: 0x7f  # Not sure this works, do we support the high 

>                          # flag bits here?

>         - const: 0

>         - description: I3C LVR (Legacy Virtual Register)...


I definitely think that it is a good move to properly define the fact
that we can accept only a single reg entry with three cells -and their
content, overall-, but this syntax does not work and I really don't find
the right way to describe it.

The error I get is:

---8<---
       reg:
         items:
           - items:
               - description: first item
               - description: second item
--->8---

schemas/i3c/i3c.yaml: ignoring, error in schema: patternProperties: @[0-9a-f]+$: properties: reg
<path>/i3c.yaml: patternProperties:@[0-9a-f]+$:properties:reg: 'anyOf' conditional failed, one must be fixed:
	'maxItems' is a required property
	'items' is not one of ['maxItems', 'description', 'deprecated']
	'items' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'default', '$ref']
	<path>/i3c.yaml: patternProperties:@[0-9a-f]+$:properties:reg:items: 'oneOf' conditional failed, one must be fixed:
		[{'items': [{'description': 'first item'}, {'description': 'second item'}]}] is not of type 'object'
		'items' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'default', '$ref']

I tried defining a phandle array, playing with the dashes, using allOf,
adding maxItems, none of it worked so far so any advice will be highly
appreciated!

Thanks,
Miquèl
Miquel Raynal Jan. 18, 2021, 5:21 p.m. UTC | #3
Hi Rob,

Just another question.

> > +patternProperties:

> > +  "^.*@[0-9a-f]+$":  

> 

> You can drop '^.*':

> 

> "@[0-9a-f]+$"


Here you advise to drop the beginning + wildcard of the regex matching
for I2C children, which indeed makes sense to me.

[...]

> > +  "^.*@[0-9a-f]+,[0-9a-f]+$":


Later in the file, we use another regex to match I3C devices. But here
if I drop ^.* from the regex, I get the following error:

schemas/i3c/i3c.yaml: ignoring, error in schema: patternProperties: @[0-9a-f]+,[0-9a-f]+$
<path>/i3c.yaml: patternProperties:@[0-9a-f]+,[0-9a-f]+$: 'oneOf' conditional failed, one must be fixed:
	Additional properties are not allowed ('properties', 'required' were unexpected)
	<path>/i3c.yaml: patternProperties:@[0-9a-f]+,[0-9a-f]+$: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
	Additional properties are not allowed ('properties', 'required', 'type' were unexpected)
	<path>/i3c.yaml: patternProperties:@[0-9a-f]+,[0-9a-f]+$: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
	'boolean' was expected

I can keep this extra "^.*" but I would like to understand the error
better because this does not look right.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt
deleted file mode 100644
index 4ffe059f0fec..000000000000
--- a/Documentation/devicetree/bindings/i3c/i3c.txt
+++ /dev/null
@@ -1,140 +0,0 @@ 
-Generic device tree bindings for I3C busses
-===========================================
-
-This document describes generic bindings that should be used to describe I3C
-busses in a device tree.
-
-Required properties
--------------------
-
-- #address-cells  - should be <3>. Read more about addresses below.
-- #size-cells     - should be <0>.
-- compatible      - name of the I3C master controller driving the I3C bus
-
-For other required properties e.g. to describe register sets,
-clocks, etc. check the binding documentation of the specific driver.
-The node describing an I3C bus should be named i3c-master.
-
-Optional properties
--------------------
-
-These properties may not be supported by all I3C master drivers. Each I3C
-master bindings should specify which of them are supported.
-
-- i3c-scl-hz: frequency of the SCL signal used for I3C transfers.
-	      When undefined the core sets it to 12.5MHz.
-
-- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
-	      When undefined, the core looks at LVR (Legacy Virtual Register)
-	      values of I2C devices described in the device tree to determine
-	      the maximum I2C frequency.
-
-I2C devices
-===========
-
-Each I2C device connected to the bus should be described in a subnode. All
-properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
-valid here, but several new properties have been added.
-
-New constraint on existing properties:
---------------------------------------
-- reg: contains 3 cells
-  + first cell : still encoding the I2C address. 10 bit addressing is not
-    supported. Devices with 10 bit address can't be properly passed through
-    DEFSLVS command.
-
-  + second cell: shall be 0
-
-  + third cell: shall encode the I3C LVR (Legacy Virtual Register)
-	bit[31:8]: unused/ignored
-	bit[7:5]: I2C device index. Possible values
-	* 0: I2C device has a 50 ns spike filter
-	* 1: I2C device does not have a 50 ns spike filter but supports high
-	     frequency on SCL
-	* 2: I2C device does not have a 50 ns spike filter and is not tolerant
-	     to high frequencies
-	* 3-7: reserved
-
-	bit[4]: tell whether the device operates in FM (Fast Mode) or FM+ mode
-	* 0: FM+ mode
-	* 1: FM mode
-
-	bit[3:0]: device type
-	* 0-15: reserved
-
-The I2C node unit-address should always match the first cell of the reg
-property: <device-type>@<i2c-address>.
-
-I3C devices
-===========
-
-All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
-are thus discoverable. So, by default, I3C devices do not have to be described
-in the device tree.
-This being said, one might want to attach extra resources to these devices,
-and those resources may have to be described in the device tree, which in turn
-means we have to describe I3C devices.
-
-Another use case for describing an I3C device in the device tree is when this
-I3C device has a static I2C address and we want to assign it a specific I3C
-dynamic address before the DAA takes place (so that other devices on the bus
-can't take this dynamic address).
-
-The I3C device should be names <device-type>@<static-i2c-address>,<i3c-pid>,
-where device-type is describing the type of device connected on the bus
-(gpio-controller, sensor, ...).
-
-Required properties
--------------------
-- reg: contains 3 cells
-  + first cell : encodes the static I2C address. Should be 0 if the device does
-		 not have one (0 is not a valid I2C address).
-
-  + second and third cells: should encode the ProvisionalID. The second cell
-			    contains the manufacturer ID left-shifted by 1.
-			    The third cell contains ORing of the part ID
-			    left-shifted by 16, the instance ID left-shifted
-			    by 12 and the extra information. This encoding is
-			    following the PID definition provided by the I3C
-			    specification.
-
-Optional properties
--------------------
-- assigned-address: dynamic address to be assigned to this device. This
-		    property is only valid if the I3C device has a static
-		    address (first cell of the reg property != 0).
-
-
-Example:
-
-	i3c-master@d040000 {
-		compatible = "cdns,i3c-master";
-		clocks = <&coreclock>, <&i3csysclock>;
-		clock-names = "pclk", "sysclk";
-		interrupts = <3 0>;
-		reg = <0x0d040000 0x1000>;
-		#address-cells = <3>;
-		#size-cells = <0>;
-		i2c-scl-hz = <100000>;
-
-		/* I2C device. */
-		nunchuk: nunchuk@52 {
-			compatible = "nintendo,nunchuk";
-			reg = <0x52 0x0 0x10>;
-		};
-
-		/* I3C device with a static I2C address. */
-		thermal_sensor: sensor@68,39200144004 {
-			reg = <0x68 0x392 0x144004>;
-			assigned-address = <0xa>;
-		};
-
-		/*
-		 * I3C device without a static I2C address but requiring
-		 * resources described in the DT.
-		 */
-		sensor@0,39200154004 {
-			reg = <0x0 0x392 0x154004>;
-			clocks = <&clock_provider 0>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
new file mode 100644
index 000000000000..79df533ab094
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
@@ -0,0 +1,186 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: I3C bus binding
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+description: |
+  I3C busses can be described with a node for the primary I3C controller device
+  and a set of child nodes for each I2C or I3C slave on the bus. Each of them
+  may, during the life of the bus, request mastership.
+
+properties:
+  $nodename:
+    pattern: "^i3c-master(@.*|-[0-9a-f])*$"
+
+  "#address-cells":
+    const: 3
+    description: |
+      Each I2C device connected to the bus should be described in a subnode.
+
+      All I3C devices are supposed to support DAA (Dynamic Address Assignment),
+      and are thus discoverable. So, by default, I3C devices do not have to be
+      described in the device tree. This being said, one might want to attach
+      extra resources to these devices, and those resources may have to be
+      described in the device tree, which in turn means we have to describe
+      I3C devices.
+
+      Another use case for describing an I3C device in the device tree is when
+      this I3C device has a static I2C address and we want to assign it a
+      specific I3C dynamic address before the DAA takes place (so that other
+      devices on the bus can't take this dynamic address).
+
+  "#size-cells":
+    const: 0
+
+  i3c-scl-hz:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Frequency of the SCL signal used for I3C transfers. When undefined, the
+      default value should be 12.5MHz.
+
+      May not be supported by all controllers.
+
+  i2c-scl-hz:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Frequency of the SCL signal used for I2C transfers. When undefined, the
+      default should be to look at LVR (Legacy Virtual Register) values of
+      I2C devices described in the device tree to determine the maximum I2C
+      frequency.
+
+      May not be supported by all controllers.
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+    description: |
+      I2C child, should be named: <device-type>@<i2c-address>
+
+      All properties described in Documentation/devicetree/bindings/i2c/i2c.txt
+      are valid here, except the reg property whose content is changed.
+
+    properties:
+      compatible:
+        description:
+          Compatible of the I2C device.
+
+      reg:
+        items:
+          - description: |
+              1st cell
+              ========
+
+              I2C address. 10 bit addressing is not supported. Devices with
+              10-bit address can't be properly passed through DEFSLVS command.
+
+              2nd cell
+              ========
+
+              Should be 0.
+
+              3rd cell
+              ========
+
+              Shall encode the I3C LVR (Legacy Virtual Register):
+              bit[31:8]: unused/ignored
+              bit[7:5]: I2C device index. Possible values:
+                * 0: I2C device has a 50 ns spike filter
+                * 1: I2C device does not have a 50 ns spike filter but supports
+                     high frequency on SCL
+                * 2: I2C device does not have a 50 ns spike filter and is not
+                     tolerant to high frequencies
+                * 3-7: reserved
+              bit[4]: tell whether the device operates in FM (Fast Mode) or
+                      FM+ mode:
+                * 0: FM+ mode
+                * 1: FM mode
+              bit[3:0]: device type
+                * 0-15: reserved
+
+    required:
+      - compatible
+      - reg
+
+  "^.*@[0-9a-f]+,[0-9a-f]+$":
+    type: object
+    description: |
+      I3C child, should be named: <device-type>@<static-i2c-address>,<i3c-pid>
+
+    properties:
+      reg:
+        items:
+          - description: |
+              1st cell
+              ========
+
+              Encodes the static I2C address. Should be 0 if the device does not
+              have one (0 is not a valid I2C address).
+
+              2nd and 3rd cells
+              =================
+
+              ProvisionalID (following the PID definition provided by the I3C
+              specification).
+
+              Cell 2 contains the manufacturer ID left-shifted by 1. Cell 3
+              contains the ORing of the part ID left-shifted by 16, the instance
+              ID left-shifted by 12 and extra information.
+
+      assigned-address:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0x01
+        maximum: 0xFF
+        description: |
+          Dynamic address to be assigned to this device. This property is only
+          valid if the I3C device has a static address (first cell of the reg
+          property != 0).
+
+    required:
+      - reg
+
+additionalProperties: true
+
+examples:
+  - |
+    i3c-master@d040000 {
+        compatible = "cdns,i3c-master";
+        clocks = <&coreclock>, <&i3csysclock>;
+        clock-names = "pclk", "sysclk";
+        interrupts = <3 0>;
+        reg = <0x0d040000 0x1000>;
+        #address-cells = <3>;
+        #size-cells = <0>;
+        i2c-scl-hz = <100000>;
+
+        /* I2C device. */
+        nunchuk: nunchuk@52 {
+            compatible = "nintendo,nunchuk";
+            reg = <0x52 0x0 0x10>;
+        };
+
+        /* I3C device with a static I2C address. */
+        thermal_sensor: sensor@68,39200144004 {
+            reg = <0x68 0x392 0x144004>;
+            assigned-address = <0xa>;
+        };
+
+        /*
+         * I3C device without a static I2C address but requiring
+         * resources described in the DT.
+         */
+        sensor@0,39200154004 {
+            reg = <0x0 0x392 0x154004>;
+            clocks = <&clock_provider 0>;
+        };
+    };