diff mbox series

[v4,10/17] dt-bindings: i2c: i2c-fsi: Convert to json-schema

Message ID 20240429210131.373487-11-eajames@linux.ibm.com
State Superseded
Headers show
Series ARM: dts: aspeed: Add IBM P11 BMC Boards | expand

Commit Message

Eddie James April 29, 2024, 9:01 p.m. UTC
Convert to json-schema for the FSI-attached I2C controller.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
Changes since v3:
 - Update MAINTAINERS
 - Change commit message to match similar commits

 .../devicetree/bindings/i2c/i2c-fsi.txt       | 40 -------------
 .../devicetree/bindings/i2c/ibm,i2c-fsi.yaml  | 58 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 59 insertions(+), 41 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml

Comments

Krzysztof Kozlowski April 30, 2024, 7:35 a.m. UTC | #1
On 29/04/2024 23:01, Eddie James wrote:
> Convert to json-schema for the FSI-attached I2C controller.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> Changes since v3:
>  - Update MAINTAINERS
>  - Change commit message to match similar commits
> 
>  .../devicetree/bindings/i2c/i2c-fsi.txt       | 40 -------------
>  .../devicetree/bindings/i2c/ibm,i2c-fsi.yaml  | 58 +++++++++++++++++++


Please split independent patches to separate patchsets, so they can be
reviewed and picked up by respective maintainers.

I don't see any dependency here. Neither in 1st patch.


>  MAINTAINERS                                   |  2 +-
>  3 files changed, 59 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
> 

...

> diff --git a/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
> new file mode 100644
> index 000000000000..8ff5585a3aa5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/ibm,i2c-fsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: IBM FSI-attached I2C controller
> +
> +maintainers:
> +  - Eddie James <eajames@linux.ibm.com>
> +
> +description:
> +  This I2C controller is an FSI CFAM engine, providing access to a number of
> +  I2C busses. Therefore this node will always be a child of an FSI CFAM node.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ibm,i2c-fsi
> +
> +  reg:
> +    items:
> +      - description: FSI slave address
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c@1800 {
> +        compatible = "ibm,i2c-fsi";
> +        reg = <0x1800 0x400>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c-bus@0 {
> +            reg = <0>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;

This does not look right. Why do you have multiple i2c-bus children? I
do not think i2c-controller.yaml schema allows this.

Best regards,
Krzysztof
Eddie James May 1, 2024, 4:16 p.m. UTC | #2
On 4/30/24 02:35, Krzysztof Kozlowski wrote:
> On 29/04/2024 23:01, Eddie James wrote:
>> Convert to json-schema for the FSI-attached I2C controller.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> Changes since v3:
>>   - Update MAINTAINERS
>>   - Change commit message to match similar commits
>>
>>   .../devicetree/bindings/i2c/i2c-fsi.txt       | 40 -------------
>>   .../devicetree/bindings/i2c/ibm,i2c-fsi.yaml  | 58 +++++++++++++++++++
>
> Please split independent patches to separate patchsets, so they can be
> reviewed and picked up by respective maintainers.
>
> I don't see any dependency here. Neither in 1st patch.


OK, I guess that makes it complicated for Andrew to pull together with 
the device tree changes in a way that avoids warnings, but I agree there 
is no direct dependency.


>
>
>>   MAINTAINERS                                   |  2 +-
>>   3 files changed, 59 insertions(+), 41 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
>>   create mode 100644 Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
>>
> ...
>
>> diff --git a/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
>> new file mode 100644
>> index 000000000000..8ff5585a3aa5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/i2c/ibm,i2c-fsi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: IBM FSI-attached I2C controller
>> +
>> +maintainers:
>> +  - Eddie James <eajames@linux.ibm.com>
>> +
>> +description:
>> +  This I2C controller is an FSI CFAM engine, providing access to a number of
>> +  I2C busses. Therefore this node will always be a child of an FSI CFAM node.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ibm,i2c-fsi
>> +
>> +  reg:
>> +    items:
>> +      - description: FSI slave address
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +allOf:
>> +  - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c@1800 {
>> +        compatible = "ibm,i2c-fsi";
>> +        reg = <0x1800 0x400>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        i2c-bus@0 {
>> +            reg = <0>;
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
> This does not look right. Why do you have multiple i2c-bus children? I
> do not think i2c-controller.yaml schema allows this.


It does seem to allow it, as this validates here and in the device tree. 
It is this way because the I2C controller provides multiple busses. 
Should I change it so to add "bus" pattern properties that reference 
i2c-controller.yaml?


Thanks,

Eddie


>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski May 2, 2024, 12:58 p.m. UTC | #3
On 01/05/2024 18:16, Eddie James wrote:
> 
> On 4/30/24 02:35, Krzysztof Kozlowski wrote:
>> On 29/04/2024 23:01, Eddie James wrote:
>>> Convert to json-schema for the FSI-attached I2C controller.
>>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>> Changes since v3:
>>>   - Update MAINTAINERS
>>>   - Change commit message to match similar commits
>>>
>>>   .../devicetree/bindings/i2c/i2c-fsi.txt       | 40 -------------
>>>   .../devicetree/bindings/i2c/ibm,i2c-fsi.yaml  | 58 +++++++++++++++++++
>>
>> Please split independent patches to separate patchsets, so they can be
>> reviewed and picked up by respective maintainers.
>>
>> I don't see any dependency here. Neither in 1st patch.
> 
> 
> OK, I guess that makes it complicated for Andrew to pull together with 
> the device tree changes in a way that avoids warnings, but I agree there 
> is no direct dependency.

SoC tree should not pull subsystem patches. Plus DTS must be separate
from drivers...


>>> +
>>> +examples:
>>> +  - |
>>> +    i2c@1800 {
>>> +        compatible = "ibm,i2c-fsi";
>>> +        reg = <0x1800 0x400>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        i2c-bus@0 {
>>> +            reg = <0>;
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>> This does not look right. Why do you have multiple i2c-bus children? I
>> do not think i2c-controller.yaml schema allows this.
> 
> 
> It does seem to allow it, as this validates here and in the device tree. 

Only because children are treated as I2C devices.

> It is this way because the I2C controller provides multiple busses. 

It does not look like I2C controller anymore. I think I2C controller
sits on the bus, not on multiple busses. How are SDA/SCL lines connected?

This looks like you are describing something which is not I2C bus
controller as I2C bus controller...

I'll let I2C maintainer comment on that - is this real I2C bus which
consists of multiple buses.


> Should I change it so to add "bus" pattern properties that reference 
> i2c-controller.yaml?

Not sure if I get it right... whatever is the I2C bus controller, should
reference i2c-controller.yaml. Not some other entity.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-fsi.txt b/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
deleted file mode 100644
index b1be2ceb7e69..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-fsi.txt
+++ /dev/null
@@ -1,40 +0,0 @@ 
-Device-tree bindings for FSI-attached I2C master and busses
------------------------------------------------------------
-
-Required properties:
- - compatible = "ibm,i2c-fsi";
- - reg = < address size >;		: The FSI CFAM address and address
-					  space size.
- - #address-cells = <1>;		: Number of address cells in child
-					  nodes.
- - #size-cells = <0>;			: Number of size cells in child nodes.
- - child nodes				: Nodes to describe busses off the I2C
-					  master.
-
-Child node required properties:
- - reg = < port number >		: The port number on the I2C master.
-
-Child node optional properties:
- - child nodes				: Nodes to describe devices on the I2C
-					  bus.
-
-Examples:
-
-    i2c@1800 {
-        compatible = "ibm,i2c-fsi";
-        reg = < 0x1800 0x400 >;
-        #address-cells = <1>;
-        #size-cells = <0>;
-
-        i2c-bus@0 {
-            reg = <0>;
-        };
-
-        i2c-bus@1 {
-            reg = <1>;
-
-            eeprom@50 {
-                compatible = "vendor,dev-name";
-            };
-        };
-    };
diff --git a/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
new file mode 100644
index 000000000000..8ff5585a3aa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
@@ -0,0 +1,58 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/ibm,i2c-fsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM FSI-attached I2C controller
+
+maintainers:
+  - Eddie James <eajames@linux.ibm.com>
+
+description:
+  This I2C controller is an FSI CFAM engine, providing access to a number of
+  I2C busses. Therefore this node will always be a child of an FSI CFAM node.
+
+properties:
+  compatible:
+    enum:
+      - ibm,i2c-fsi
+
+  reg:
+    items:
+      - description: FSI slave address
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c@1800 {
+        compatible = "ibm,i2c-fsi";
+        reg = <0x1800 0x400>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-bus@0 {
+            reg = <0>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+        };
+
+        i2c-bus@1 {
+            reg = <1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            eeprom@50 {
+                compatible = "atmel,24c64";
+                reg = <0x50>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index eea74166a2d9..9021e6ec4e6d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8924,7 +8924,7 @@  M:	Eddie James <eajames@linux.ibm.com>
 L:	linux-i2c@vger.kernel.org
 L:	openbmc@lists.ozlabs.org (moderated for non-subscribers)
 S:	Maintained
-F:	Documentation/devicetree/bindings/i2c/i2c-fsi.txt
+F:	Documentation/devicetree/bindings/i2c/ibm,i2c-fsi.yaml
 F:	drivers/i2c/busses/i2c-fsi.c
 
 FSI-ATTACHED SPI DRIVER