[PATCH 2/2] schemas: i2c: Introduce I2C bus extensions

Herve Codina posted 2 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
Posted by Herve Codina 1 month, 1 week ago
An I2C bus can be wired to the connector and allows an add-on board to
connect additional I2C devices to this bus.

Those additional I2C devices could be described as sub-nodes of the I2C
bus controller node however for hotplug connectors described via device
tree overlays there is additional level of indirection, which is needed
to decouple the overlay and the base tree:

  --- base device tree ---

  i2c1: i2c@abcd0000 {
      compatible = "xyz,i2c-ctrl";
      i2c-bus-extension@0 {
          i2c-bus = <&i2c_ctrl>;
      };
      ...
  };

  i2c5: i2c@cafe0000 {
      compatible = "xyz,i2c-ctrl";
      i2c-bus-extension@0 {
          i2c-bus = <&i2c-sensors>;
      };
      ...
  };

  connector {
      i2c_ctrl: i2c-ctrl {
          i2c-parent = <&i2c1>;
          #address-cells = <1>;
          #size-cells = <0>;
      };

      i2c-sensors {
          i2c-parent = <&i2c5>;
          #address-cells = <1>;
          #size-cells = <0>;
      };
  };

  --- device tree overlay ---

  ...
  // This node will overlay on the i2c-ctrl node of the base tree
  i2c-ctrl {
      eeprom@50 { compatible = "atmel,24c64"; ... };
  };
  ...

  --- resulting device tree ---

  i2c1: i2c@abcd0000 {
      compatible = "xyz,i2c-ctrl";
      i2c-bus-extension@0 {
          i2c-bus = <&i2c_ctrl>;
      };
      ...
  };

  i2c5: i2c@cafe0000 {
      compatible = "xyz,i2c-ctrl";
      i2c-bus-extension@0 {
          i2c-bus = <&i2c-sensors>;
      };
      ...
  };

  connector {
      i2c-ctrl {
          i2c-parent = <&i2c1>;
          #address-cells = <1>;
          #size-cells = <0>;

          eeprom@50 { compatible = "atmel,24c64"; ... };
      };

      i2c-sensors {
          i2c-parent = <&i2c5>;
          #address-cells = <1>;
          #size-cells = <0>;
      };
  };

Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus
that is on the hot-pluggable add-on. On hot-plugging it will physically
connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl'
node an "extension node".

In order to decouple the overlay from the base tree, the I2C adapter
(i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes.

The extension node is linked to the I2C bus controller in two ways. The
first one with the i2c-bus-extension available in I2C controller
sub-node and the second one with the i2c-parent property available in
the extension node itself.

The purpose of those two links is to provide the link in both direction
from the I2C controller to the I2C extension and from the I2C extension
to the I2C controller.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 dtschema/schemas/i2c/i2c-controller.yaml | 67 ++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/dtschema/schemas/i2c/i2c-controller.yaml b/dtschema/schemas/i2c/i2c-controller.yaml
index 018d266..509b581 100644
--- a/dtschema/schemas/i2c/i2c-controller.yaml
+++ b/dtschema/schemas/i2c/i2c-controller.yaml
@@ -30,6 +30,13 @@ properties:
     minimum: 1
     maximum: 5000000
 
+  i2c-parent:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      In case of an I2C bus extension, reference to the I2C bus controller
+      this extension is connected to. In other word, reference the I2C bus
+      controller on the fixed side that drives the bus extension.
+
   i2c-scl-falling-time-ns:
     description:
       Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
@@ -159,6 +166,25 @@ allOf:
         - i2c-scl-has-clk-low-timeout
 
 patternProperties:
+  'i2c-bus-extension@[0-9a-f]+$':
+    type: object
+    description:
+      An I2C bus extension connected to an I2C bus. Those extensions allow to
+      decouple I2C busses when they are wired to connectors.
+
+    properties:
+      reg:
+        maxItems: 1
+
+      i2c-bus:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description:
+          Reference to the extension bus.
+
+    required:
+      - reg
+      - i2c-bus
+
   '@[0-9a-f]+$':
     type: object
 
@@ -221,3 +247,44 @@ dependentRequired:
   i2c-digital-filter-width-ns: [ i2c-digital-filter ]
 
 additionalProperties: true
+
+examples:
+  # I2C bus extension example involving an I2C bus controller and a connector.
+  #
+  #  +--------------+     +-------------+     +-------------+
+  #  | i2c@abcd0000 |     |  Connector  |     | Addon board |
+  #  |    (i2c1)    +-----+ (i2c-addon) +-----+ (device@10) |
+  #  |              |     |             |     |             |
+  #  +--------------+     +-------------+     +-------------+
+  #
+  # The i2c1 I2C bus is wired from a I2C controller to a connector. It is
+  # identified at connector level as i2c-addon bus.
+  # An addon board can be connected to this connector and connects a device
+  # (device@10) to this i2c-addon extension bus.
+  - |
+    i2c1: i2c@abcd0000 {
+        compatible = "xyz,i2c-ctrl";
+        reg = <0xabcd0000 0x100>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-bus-extension@0 {
+            reg = <0>;
+            i2c-bus = <&i2c_addon>;
+        };
+    };
+
+    connector {
+        i2c_addon: i2c-addon {
+            i2c-parent = <&i2c1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            device@10 {
+                compatible = "xyz,foo";
+                reg = <0x10>;
+            };
+        };
+    };
+
+...
-- 
2.49.0
Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
Posted by Ayush Singh 1 week, 6 days ago
On 4/1/25 13:40, Herve Codina wrote:

> An I2C bus can be wired to the connector and allows an add-on board to
> connect additional I2C devices to this bus.
>
> Those additional I2C devices could be described as sub-nodes of the I2C
> bus controller node however for hotplug connectors described via device
> tree overlays there is additional level of indirection, which is needed
> to decouple the overlay and the base tree:
>
>    --- base device tree ---
>
>    i2c1: i2c@abcd0000 {
>        compatible = "xyz,i2c-ctrl";
>        i2c-bus-extension@0 {
>            i2c-bus = <&i2c_ctrl>;
>        };
>        ...
>    };
>
>    i2c5: i2c@cafe0000 {
>        compatible = "xyz,i2c-ctrl";
>        i2c-bus-extension@0 {
>            i2c-bus = <&i2c-sensors>;
>        };
>        ...
>    };
>
>    connector {
>        i2c_ctrl: i2c-ctrl {
>            i2c-parent = <&i2c1>;
>            #address-cells = <1>;
>            #size-cells = <0>;
>        };
>
>        i2c-sensors {
>            i2c-parent = <&i2c5>;
>            #address-cells = <1>;
>            #size-cells = <0>;
>        };
>    };
>
>    --- device tree overlay ---
>
>    ...
>    // This node will overlay on the i2c-ctrl node of the base tree
>    i2c-ctrl {
>        eeprom@50 { compatible = "atmel,24c64"; ... };
>    };
>    ...
>
>    --- resulting device tree ---
>
>    i2c1: i2c@abcd0000 {
>        compatible = "xyz,i2c-ctrl";
>        i2c-bus-extension@0 {
>            i2c-bus = <&i2c_ctrl>;
>        };
>        ...
>    };
>
>    i2c5: i2c@cafe0000 {
>        compatible = "xyz,i2c-ctrl";
>        i2c-bus-extension@0 {
>            i2c-bus = <&i2c-sensors>;
>        };
>        ...
>    };
>
>    connector {
>        i2c-ctrl {
>            i2c-parent = <&i2c1>;
>            #address-cells = <1>;
>            #size-cells = <0>;
>
>            eeprom@50 { compatible = "atmel,24c64"; ... };
>        };
>
>        i2c-sensors {
>            i2c-parent = <&i2c5>;
>            #address-cells = <1>;
>            #size-cells = <0>;
>        };
>    };
>
> Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus
> that is on the hot-pluggable add-on. On hot-plugging it will physically
> connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl'
> node an "extension node".
>
> In order to decouple the overlay from the base tree, the I2C adapter
> (i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes.
>
> The extension node is linked to the I2C bus controller in two ways. The
> first one with the i2c-bus-extension available in I2C controller
> sub-node and the second one with the i2c-parent property available in
> the extension node itself.
>
> The purpose of those two links is to provide the link in both direction
> from the I2C controller to the I2C extension and from the I2C extension
> to the I2C controller.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>   dtschema/schemas/i2c/i2c-controller.yaml | 67 ++++++++++++++++++++++++
>   1 file changed, 67 insertions(+)
>
> diff --git a/dtschema/schemas/i2c/i2c-controller.yaml b/dtschema/schemas/i2c/i2c-controller.yaml
> index 018d266..509b581 100644
> --- a/dtschema/schemas/i2c/i2c-controller.yaml
> +++ b/dtschema/schemas/i2c/i2c-controller.yaml
> @@ -30,6 +30,13 @@ properties:
>       minimum: 1
>       maximum: 5000000
>   
> +  i2c-parent:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      In case of an I2C bus extension, reference to the I2C bus controller
> +      this extension is connected to. In other word, reference the I2C bus
> +      controller on the fixed side that drives the bus extension.
> +
>     i2c-scl-falling-time-ns:
>       description:
>         Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
> @@ -159,6 +166,25 @@ allOf:
>           - i2c-scl-has-clk-low-timeout
>   
>   patternProperties:
> +  'i2c-bus-extension@[0-9a-f]+$':
> +    type: object
> +    description:
> +      An I2C bus extension connected to an I2C bus. Those extensions allow to
> +      decouple I2C busses when they are wired to connectors.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      i2c-bus:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description:
> +          Reference to the extension bus.
> +
> +    required:
> +      - reg
> +      - i2c-bus
> +
>     '@[0-9a-f]+$':
>       type: object
>   
> @@ -221,3 +247,44 @@ dependentRequired:
>     i2c-digital-filter-width-ns: [ i2c-digital-filter ]
>   
>   additionalProperties: true
> +
> +examples:
> +  # I2C bus extension example involving an I2C bus controller and a connector.
> +  #
> +  #  +--------------+     +-------------+     +-------------+
> +  #  | i2c@abcd0000 |     |  Connector  |     | Addon board |
> +  #  |    (i2c1)    +-----+ (i2c-addon) +-----+ (device@10) |
> +  #  |              |     |             |     |             |
> +  #  +--------------+     +-------------+     +-------------+
> +  #
> +  # The i2c1 I2C bus is wired from a I2C controller to a connector. It is
> +  # identified at connector level as i2c-addon bus.
> +  # An addon board can be connected to this connector and connects a device
> +  # (device@10) to this i2c-addon extension bus.
> +  - |
> +    i2c1: i2c@abcd0000 {
> +        compatible = "xyz,i2c-ctrl";
> +        reg = <0xabcd0000 0x100>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c-bus-extension@0 {
> +            reg = <0>;
> +            i2c-bus = <&i2c_addon>;
> +        };
> +    };
> +
> +    connector {
> +        i2c_addon: i2c-addon {
> +            i2c-parent = <&i2c1>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            device@10 {
> +                compatible = "xyz,foo";
> +                reg = <0x10>;
> +            };
> +        };
> +    };
> +
> +...


Reviewed-by: Ayush Singh <ayush@beagleboard.org>
Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
Posted by Rob Herring 1 month, 1 week ago
On Tue, Apr 1, 2025 at 3:11 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> An I2C bus can be wired to the connector and allows an add-on board to
> connect additional I2C devices to this bus.
>
> Those additional I2C devices could be described as sub-nodes of the I2C
> bus controller node however for hotplug connectors described via device
> tree overlays there is additional level of indirection, which is needed
> to decouple the overlay and the base tree:
>
>   --- base device tree ---
>
>   i2c1: i2c@abcd0000 {
>       compatible = "xyz,i2c-ctrl";
>       i2c-bus-extension@0 {

What does 0 represent? Some fake I2C address?

Why do you even need a child node here?

>           i2c-bus = <&i2c_ctrl>;
>       };
>       ...
>   };
>
>   i2c5: i2c@cafe0000 {
>       compatible = "xyz,i2c-ctrl";
>       i2c-bus-extension@0 {
>           i2c-bus = <&i2c-sensors>;
>       };
>       ...
>   };
>
>   connector {
>       i2c_ctrl: i2c-ctrl {
>           i2c-parent = <&i2c1>;
>           #address-cells = <1>;
>           #size-cells = <0>;
>       };
>
>       i2c-sensors {
>           i2c-parent = <&i2c5>;
>           #address-cells = <1>;
>           #size-cells = <0>;
>       };
>   };
>
>   --- device tree overlay ---
>
>   ...
>   // This node will overlay on the i2c-ctrl node of the base tree
>   i2c-ctrl {
>       eeprom@50 { compatible = "atmel,24c64"; ... };
>   };
>   ...
>
>   --- resulting device tree ---
>
>   i2c1: i2c@abcd0000 {
>       compatible = "xyz,i2c-ctrl";
>       i2c-bus-extension@0 {
>           i2c-bus = <&i2c_ctrl>;
>       };
>       ...
>   };
>
>   i2c5: i2c@cafe0000 {
>       compatible = "xyz,i2c-ctrl";
>       i2c-bus-extension@0 {
>           i2c-bus = <&i2c-sensors>;
>       };
>       ...
>   };
>
>   connector {
>       i2c-ctrl {
>           i2c-parent = <&i2c1>;
>           #address-cells = <1>;
>           #size-cells = <0>;
>
>           eeprom@50 { compatible = "atmel,24c64"; ... };
>       };
>
>       i2c-sensors {
>           i2c-parent = <&i2c5>;
>           #address-cells = <1>;
>           #size-cells = <0>;
>       };
>   };
>
> Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus
> that is on the hot-pluggable add-on. On hot-plugging it will physically
> connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl'
> node an "extension node".
>
> In order to decouple the overlay from the base tree, the I2C adapter
> (i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes.
>
> The extension node is linked to the I2C bus controller in two ways. The
> first one with the i2c-bus-extension available in I2C controller
> sub-node and the second one with the i2c-parent property available in
> the extension node itself.
>
> The purpose of those two links is to provide the link in both direction
> from the I2C controller to the I2C extension and from the I2C extension
> to the I2C controller.

Why do you need both directions? An i2c controller can search the tree
for i2c-parent and find the one's that belong to it. Or the connector
can register with the I2C controller somehow.

Rob
Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
Posted by Herve Codina 1 month, 1 week ago
Hi Rob,

On Tue, 1 Apr 2025 09:03:23 -0500
Rob Herring <robh@kernel.org> wrote:

> On Tue, Apr 1, 2025 at 3:11 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > An I2C bus can be wired to the connector and allows an add-on board to
> > connect additional I2C devices to this bus.
> >
> > Those additional I2C devices could be described as sub-nodes of the I2C
> > bus controller node however for hotplug connectors described via device
> > tree overlays there is additional level of indirection, which is needed
> > to decouple the overlay and the base tree:
> >
> >   --- base device tree ---
> >
> >   i2c1: i2c@abcd0000 {
> >       compatible = "xyz,i2c-ctrl";
> >       i2c-bus-extension@0 {  
> 
> What does 0 represent? Some fake I2C address?
> 
> Why do you even need a child node here?

0 represent the extension number. Multiple extensions can be connected
to a single i2c controller:
                                      +-------------+
  +------------+                 .----+ Connector 1 |
  |   i2c      |                 |    +-------------+
  | controller +---- i2c bus ----+
  +------------+                 |    +-------------+
                                 '----+ Connector 2 |
                                      +-------------+

I need a child node because extensions don't modify existing hardware (adding/removing
a property) but add an entry point, the extension for a new set of devices.
As it is not a existing hardware modification it is better represented as a node.


Those extensions can be chained:
  +-----------------------------------+         +-------------------+
  | Base board                        |         | addon board       |
  |                                   |         |    +------------+ |
  | +------------+                    |         | .--+ i2c device | |
  | |   i2c      |             +-------------+  | |  +------------+ |
  | | controller +-- i2c bus --+ connector A +----+                 |
  | +------------+             +-------------+  | |          +-------------+
  +-----------------------------------+         | '----------+ connector B |
                                                |            +-------------+
                                                +-------------------+
The addon board is described using an overlay.

In that case, we have:
- base-board.dts:
    i2c0: i2c@cafe0000 {
        compatible = "xyz,i2c-ctrl";
        #address-cells = <1>;
        #size-cells = <0>;

        i2c-bus-extension@0 {
            reg = <0>;
            i2c-bus = <&i2c_connector_a>;
        };
        ...
    };

    connector-a {
        devices {
           /* Entry point for devices available on the addon
            * board that are not connected to a bus such as
            * fixed-clock, fixed-regulator, connectors, ...
            */
        };
        i2c_connector_a: i2c-connector-a {
            /* The i2c available at connector */
            #address-cells = <1>;
            #size-cells = <0>;
            i2c_parent <&i2c0>;
       };
    };

- addon-board.dtso
    __overlay__ { /* This is applied at connector_a node */
        i2c_connector_a: i2c-connector-a {
            /* We do not modify the existing device i2c_connector_a
             * by changing, adding or removing its properties but
             * we add new devices (sub-nodes)
             */

            /* The i2c device available in the addon-board */
            i2c-device@0x10 {
                compatible = "foo,bar";
                reg = 0x10;
            };

            /* The i2c extension forwarding the i2c bus */
            i2c-bus-extension@0 {
	        reg = <0>;
                i2c-bus = <&i2c_connector_b>;
            };
       };
       
       devices {
          /* addon-board connector b */
          connector_b {
              i2c_connector_b: i2c_connector_b {
              /* The i2c available at connector */
              #address-cells = <1>;
              #size-cells = <0>;
              i2c_parent = <&i2c_connector_a>;
          };
       };
   };

Without a child node for i2c-bus-extension, we need to add
properties on already existing node (i2c-connector-a) to add
the bus extension and adding/modifying/removing a property
on a device-tree node correspond to modifying the device
itself (description changed) whereas adding/removing sub-nodes
correspond to adding/removing devices handled by the parent
parent node of those sub-nodes.

From the controller point of view, an extension is "a collection of
devices described somewhere else in the device-tree and connected
to the I2C SDA/SCL pins". Having that described as a sub-node seems
correct.

> 
> >           i2c-bus = <&i2c_ctrl>;
> >       };
> >       ...
> >   };
> >
> >   i2c5: i2c@cafe0000 {
> >       compatible = "xyz,i2c-ctrl";
> >       i2c-bus-extension@0 {
> >           i2c-bus = <&i2c-sensors>;
> >       };
> >       ...
> >   };
> >
> >   connector {
> >       i2c_ctrl: i2c-ctrl {
> >           i2c-parent = <&i2c1>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >
> >       i2c-sensors {
> >           i2c-parent = <&i2c5>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >   };
> >
> >   --- device tree overlay ---
> >
> >   ...
> >   // This node will overlay on the i2c-ctrl node of the base tree
> >   i2c-ctrl {
> >       eeprom@50 { compatible = "atmel,24c64"; ... };
> >   };
> >   ...
> >
> >   --- resulting device tree ---
> >
> >   i2c1: i2c@abcd0000 {
> >       compatible = "xyz,i2c-ctrl";
> >       i2c-bus-extension@0 {
> >           i2c-bus = <&i2c_ctrl>;
> >       };
> >       ...
> >   };
> >
> >   i2c5: i2c@cafe0000 {
> >       compatible = "xyz,i2c-ctrl";
> >       i2c-bus-extension@0 {
> >           i2c-bus = <&i2c-sensors>;
> >       };
> >       ...
> >   };
> >
> >   connector {
> >       i2c-ctrl {
> >           i2c-parent = <&i2c1>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >
> >           eeprom@50 { compatible = "atmel,24c64"; ... };
> >       };
> >
> >       i2c-sensors {
> >           i2c-parent = <&i2c5>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >   };
> >
> > Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus
> > that is on the hot-pluggable add-on. On hot-plugging it will physically
> > connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl'
> > node an "extension node".
> >
> > In order to decouple the overlay from the base tree, the I2C adapter
> > (i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes.
> >
> > The extension node is linked to the I2C bus controller in two ways. The
> > first one with the i2c-bus-extension available in I2C controller
> > sub-node and the second one with the i2c-parent property available in
> > the extension node itself.
> >
> > The purpose of those two links is to provide the link in both direction
> > from the I2C controller to the I2C extension and from the I2C extension
> > to the I2C controller.  
> 
> Why do you need both directions? An i2c controller can search the tree
> for i2c-parent and find the one's that belong to it. Or the connector
> can register with the I2C controller somehow.

Yes, but this is sub-optimal compare to the double-link references.

I discarded any kind of registering from the connector which implies
extra complexity compared to a simple double-link reference. In the I2C
path, the connector is really a passive component and fully transparent.
It should be transparent as well in the implementation.

Using only i2c-parent (i.e. the reference from extension to i2c controller)
works when the walk from extension to i2c controller but using it on the
other direction is not as trivial as it could be.

Indeed, starting from the i2c controller, we have to search for the entire
tree any i2c-parent that references the i2c controller.
Those i2c-parent can exist in node that are not at all related to i2c
extensions.

i2c-parent in all cases represents an i2c parent bus but not only for i2c
extensions. I2C muxes and some other devices use this property.

Here also, searching the entire tree for i2c-parent and being sure that
the property found is related to an I2C extension adds extra complexity
that is simply not present with the double-link references.

Best regards,
Hervé
Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
Posted by Herve Codina 3 weeks, 4 days ago
Hi Rob, device-tree maintainers,

This discussion started a few weeks ago and hasn't reached any conclusion.
I answered questions and comments but didn't receive any feedback.

In order to move forward on that topic, can we revive this discussion?

As a reminder, topics started in the discussion were the following:
 - i2c-bus-extension@0: usage of a subnode with unit address
 - Presence of phandles in both direction (double linked list)

Best regards,
Hervé

On Wed, 2 Apr 2025 10:21:23 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Rob,
> 
> On Tue, 1 Apr 2025 09:03:23 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, Apr 1, 2025 at 3:11 AM Herve Codina <herve.codina@bootlin.com> wrote:  
> > >
> > > An I2C bus can be wired to the connector and allows an add-on board to
> > > connect additional I2C devices to this bus.
> > >
> > > Those additional I2C devices could be described as sub-nodes of the I2C
> > > bus controller node however for hotplug connectors described via device
> > > tree overlays there is additional level of indirection, which is needed
> > > to decouple the overlay and the base tree:
> > >
> > >   --- base device tree ---
> > >
> > >   i2c1: i2c@abcd0000 {
> > >       compatible = "xyz,i2c-ctrl";
> > >       i2c-bus-extension@0 {    
> > 
> > What does 0 represent? Some fake I2C address?
> > 
> > Why do you even need a child node here?  
> 
> 0 represent the extension number. Multiple extensions can be connected
> to a single i2c controller:
>                                       +-------------+
>   +------------+                 .----+ Connector 1 |
>   |   i2c      |                 |    +-------------+
>   | controller +---- i2c bus ----+
>   +------------+                 |    +-------------+
>                                  '----+ Connector 2 |
>                                       +-------------+
> 
> I need a child node because extensions don't modify existing hardware (adding/removing
> a property) but add an entry point, the extension for a new set of devices.
> As it is not a existing hardware modification it is better represented as a node.
> 
> 
> Those extensions can be chained:
>   +-----------------------------------+         +-------------------+
>   | Base board                        |         | addon board       |
>   |                                   |         |    +------------+ |
>   | +------------+                    |         | .--+ i2c device | |
>   | |   i2c      |             +-------------+  | |  +------------+ |
>   | | controller +-- i2c bus --+ connector A +----+                 |
>   | +------------+             +-------------+  | |          +-------------+
>   +-----------------------------------+         | '----------+ connector B |
>                                                 |            +-------------+
>                                                 +-------------------+
> The addon board is described using an overlay.
> 
> In that case, we have:
> - base-board.dts:
>     i2c0: i2c@cafe0000 {
>         compatible = "xyz,i2c-ctrl";
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         i2c-bus-extension@0 {
>             reg = <0>;
>             i2c-bus = <&i2c_connector_a>;
>         };
>         ...
>     };
> 
>     connector-a {
>         devices {
>            /* Entry point for devices available on the addon
>             * board that are not connected to a bus such as
>             * fixed-clock, fixed-regulator, connectors, ...
>             */
>         };
>         i2c_connector_a: i2c-connector-a {
>             /* The i2c available at connector */
>             #address-cells = <1>;
>             #size-cells = <0>;
>             i2c_parent <&i2c0>;
>        };
>     };
> 
> - addon-board.dtso
>     __overlay__ { /* This is applied at connector_a node */
>         i2c_connector_a: i2c-connector-a {
>             /* We do not modify the existing device i2c_connector_a
>              * by changing, adding or removing its properties but
>              * we add new devices (sub-nodes)
>              */
> 
>             /* The i2c device available in the addon-board */
>             i2c-device@0x10 {
>                 compatible = "foo,bar";
>                 reg = 0x10;
>             };
> 
>             /* The i2c extension forwarding the i2c bus */
>             i2c-bus-extension@0 {
> 	        reg = <0>;
>                 i2c-bus = <&i2c_connector_b>;
>             };
>        };
>        
>        devices {
>           /* addon-board connector b */
>           connector_b {
>               i2c_connector_b: i2c_connector_b {
>               /* The i2c available at connector */
>               #address-cells = <1>;
>               #size-cells = <0>;
>               i2c_parent = <&i2c_connector_a>;
>           };
>        };
>    };
> 
> Without a child node for i2c-bus-extension, we need to add
> properties on already existing node (i2c-connector-a) to add
> the bus extension and adding/modifying/removing a property
> on a device-tree node correspond to modifying the device
> itself (description changed) whereas adding/removing sub-nodes
> correspond to adding/removing devices handled by the parent
> parent node of those sub-nodes.
> 
> From the controller point of view, an extension is "a collection of
> devices described somewhere else in the device-tree and connected
> to the I2C SDA/SCL pins". Having that described as a sub-node seems
> correct.
> 
> >   
> > >           i2c-bus = <&i2c_ctrl>;
> > >       };
> > >       ...
> > >   };
> > >
> > >   i2c5: i2c@cafe0000 {
> > >       compatible = "xyz,i2c-ctrl";
> > >       i2c-bus-extension@0 {
> > >           i2c-bus = <&i2c-sensors>;
> > >       };
> > >       ...
> > >   };
> > >
> > >   connector {
> > >       i2c_ctrl: i2c-ctrl {
> > >           i2c-parent = <&i2c1>;
> > >           #address-cells = <1>;
> > >           #size-cells = <0>;
> > >       };
> > >
> > >       i2c-sensors {
> > >           i2c-parent = <&i2c5>;
> > >           #address-cells = <1>;
> > >           #size-cells = <0>;
> > >       };
> > >   };
> > >
> > >   --- device tree overlay ---
> > >
> > >   ...
> > >   // This node will overlay on the i2c-ctrl node of the base tree
> > >   i2c-ctrl {
> > >       eeprom@50 { compatible = "atmel,24c64"; ... };
> > >   };
> > >   ...
> > >
> > >   --- resulting device tree ---
> > >
> > >   i2c1: i2c@abcd0000 {
> > >       compatible = "xyz,i2c-ctrl";
> > >       i2c-bus-extension@0 {
> > >           i2c-bus = <&i2c_ctrl>;
> > >       };
> > >       ...
> > >   };
> > >
> > >   i2c5: i2c@cafe0000 {
> > >       compatible = "xyz,i2c-ctrl";
> > >       i2c-bus-extension@0 {
> > >           i2c-bus = <&i2c-sensors>;
> > >       };
> > >       ...
> > >   };
> > >
> > >   connector {
> > >       i2c-ctrl {
> > >           i2c-parent = <&i2c1>;
> > >           #address-cells = <1>;
> > >           #size-cells = <0>;
> > >
> > >           eeprom@50 { compatible = "atmel,24c64"; ... };
> > >       };
> > >
> > >       i2c-sensors {
> > >           i2c-parent = <&i2c5>;
> > >           #address-cells = <1>;
> > >           #size-cells = <0>;
> > >       };
> > >   };
> > >
> > > Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus
> > > that is on the hot-pluggable add-on. On hot-plugging it will physically
> > > connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl'
> > > node an "extension node".
> > >
> > > In order to decouple the overlay from the base tree, the I2C adapter
> > > (i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes.
> > >
> > > The extension node is linked to the I2C bus controller in two ways. The
> > > first one with the i2c-bus-extension available in I2C controller
> > > sub-node and the second one with the i2c-parent property available in
> > > the extension node itself.
> > >
> > > The purpose of those two links is to provide the link in both direction
> > > from the I2C controller to the I2C extension and from the I2C extension
> > > to the I2C controller.    
> > 
> > Why do you need both directions? An i2c controller can search the tree
> > for i2c-parent and find the one's that belong to it. Or the connector
> > can register with the I2C controller somehow.  
> 
> Yes, but this is sub-optimal compare to the double-link references.
> 
> I discarded any kind of registering from the connector which implies
> extra complexity compared to a simple double-link reference. In the I2C
> path, the connector is really a passive component and fully transparent.
> It should be transparent as well in the implementation.
> 
> Using only i2c-parent (i.e. the reference from extension to i2c controller)
> works when the walk from extension to i2c controller but using it on the
> other direction is not as trivial as it could be.
> 
> Indeed, starting from the i2c controller, we have to search for the entire
> tree any i2c-parent that references the i2c controller.
> Those i2c-parent can exist in node that are not at all related to i2c
> extensions.
> 
> i2c-parent in all cases represents an i2c parent bus but not only for i2c
> extensions. I2C muxes and some other devices use this property.
> 
> Here also, searching the entire tree for i2c-parent and being sure that
> the property found is related to an I2C extension adds extra complexity
> that is simply not present with the double-link references.
> 
> Best regards,
> Hervé