mbox series

[0/2] i2c: Introduce I2C bus extensions

Message ID 20250401081041.114333-1-herve.codina@bootlin.com
Headers show
Series i2c: Introduce I2C bus extensions | expand

Message

Herve Codina April 1, 2025, 8:10 a.m. UTC 
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(-)

Comments

Herve Codina April 2, 2025, 8:21 a.m. UTC | #1 
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é