linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] i2c: Introduce I2C bus extensions
@ 2025-04-01  8:10 Herve Codina
  2025-04-01  8:10 ` [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern Herve Codina
  2025-04-01  8:10 ` [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions Herve Codina
  0 siblings, 2 replies; 8+ messages in thread
From: Herve Codina @ 2025-04-01  8:10 UTC (permalink / raw)
  To: Wolfram Sang, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni, Herve Codina

Hi,

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.

This decoupling is performed thanks to the I2C bus extension feature
which is introduced and detailed in patch 2 of this series.

The implementation related to I2C bus extension has been already
proposed as an RFC in Linux [0]. The missing part in this RFC was the
binding.

This binding related to I2C controller is not available in the Linux
repository but in dt-schema repository and so, this series update the
I2C controller binding to introduce the feature:
  - Patch 1 is a fix avoid a wrong matching I2C bus node name.
  - Patch 2 is the I2C bus extension itself.

[0] https://lore.kernel.org/all/20250205173918.600037-1-herve.codina@bootlin.com/

Best regards,
Hervé Codina

Herve Codina (2):
  schemas: i2c: Avoid extra characters in i2c nodename pattern
  schemas: i2c: Introduce I2C bus extensions

 dtschema/schemas/i2c/i2c-controller.yaml | 69 +++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern
  2025-04-01  8:10 [PATCH 0/2] i2c: Introduce I2C bus extensions Herve Codina
@ 2025-04-01  8:10 ` Herve Codina
  2025-04-01 14:10   ` Rob Herring
  2025-04-01  8:10 ` [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions Herve Codina
  1 sibling, 1 reply; 8+ messages in thread
From: Herve Codina @ 2025-04-01  8:10 UTC (permalink / raw)
  To: Wolfram Sang, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni, Herve Codina

Current nodename pattern doesn't limit the end of name for an i2c node.
It can match 'i2c@10-foo'.

In order to avoid matching to an incorrect name, avoid any extra
characters in nodename pattern.

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

diff --git a/dtschema/schemas/i2c/i2c-controller.yaml b/dtschema/schemas/i2c/i2c-controller.yaml
index 487e669..018d266 100644
--- a/dtschema/schemas/i2c/i2c-controller.yaml
+++ b/dtschema/schemas/i2c/i2c-controller.yaml
@@ -14,7 +14,7 @@ maintainers:
 
 properties:
   $nodename:
-    pattern: "^i2c(@.*)?"
+    pattern: "^i2c(@.*)?$"
 
   i2c-bus:
     type: object
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
  2025-04-01  8:10 [PATCH 0/2] i2c: Introduce I2C bus extensions Herve Codina
  2025-04-01  8:10 ` [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern Herve Codina
@ 2025-04-01  8:10 ` Herve Codina
  2025-04-01 14:03   ` Rob Herring
  2025-04-29 18:04   ` Ayush Singh
  1 sibling, 2 replies; 8+ messages in thread
From: Herve Codina @ 2025-04-01  8:10 UTC (permalink / raw)
  To: Wolfram Sang, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni, Herve Codina

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
  2025-04-01  8:10 ` [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions Herve Codina
@ 2025-04-01 14:03   ` Rob Herring
  2025-04-02  8:21     ` Herve Codina
  2025-04-29 18:04   ` Ayush Singh
  1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2025-04-01 14:03 UTC (permalink / raw)
  To: Herve Codina
  Cc: Wolfram Sang, Andi Shyti, Krzysztof Kozlowski, Conor Dooley,
	linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern
  2025-04-01  8:10 ` [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern Herve Codina
@ 2025-04-01 14:10   ` Rob Herring
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2025-04-01 14:10 UTC (permalink / raw)
  To: Herve Codina
  Cc: Wolfram Sang, Andi Shyti, Krzysztof Kozlowski, Conor Dooley,
	linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni

On Tue, Apr 01, 2025 at 10:10:39AM +0200, Herve Codina wrote:
> Current nodename pattern doesn't limit the end of name for an i2c node.
> It can match 'i2c@10-foo'.
> 
> In order to avoid matching to an incorrect name, avoid any extra
> characters in nodename pattern.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  dtschema/schemas/i2c/i2c-controller.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
  2025-04-01 14:03   ` Rob Herring
@ 2025-04-02  8:21     ` Herve Codina
  2025-04-17 15:38       ` Herve Codina
  0 siblings, 1 reply; 8+ messages in thread
From: Herve Codina @ 2025-04-02  8:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Wolfram Sang, Andi Shyti, Krzysztof Kozlowski, Conor Dooley,
	linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni

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é

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
  2025-04-02  8:21     ` Herve Codina
@ 2025-04-17 15:38       ` Herve Codina
  0 siblings, 0 replies; 8+ messages in thread
From: Herve Codina @ 2025-04-17 15:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Wolfram Sang, Andi Shyti, Krzysztof Kozlowski, Conor Dooley,
	linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni

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é


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions
  2025-04-01  8:10 ` [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions Herve Codina
  2025-04-01 14:03   ` Rob Herring
@ 2025-04-29 18:04   ` Ayush Singh
  1 sibling, 0 replies; 8+ messages in thread
From: Ayush Singh @ 2025-04-29 18:04 UTC (permalink / raw)
  To: Herve Codina, Wolfram Sang, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, devicetree-spec,
	Luca Ceresoli, Thomas Petazzoni

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>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-04-29 18:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-01  8:10 [PATCH 0/2] i2c: Introduce I2C bus extensions Herve Codina
2025-04-01  8:10 ` [PATCH 1/2] schemas: i2c: Avoid extra characters in i2c nodename pattern Herve Codina
2025-04-01 14:10   ` Rob Herring
2025-04-01  8:10 ` [PATCH 2/2] schemas: i2c: Introduce I2C bus extensions Herve Codina
2025-04-01 14:03   ` Rob Herring
2025-04-02  8:21     ` Herve Codina
2025-04-17 15:38       ` Herve Codina
2025-04-29 18:04   ` Ayush Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).