Hi Wolfram,
In order for me to move forward on this new feature sending a patch for the
binding or reworking my proposal can you provide feedback on the topic and
the current implementation available in this RFC.
Best regards,
Hervé
On Wed, 5 Feb 2025 18:39:13 +0100
Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> The big picture behind this RFC series is to support a Linux device
> with a connector to physically add and remove an add-on to/from the
> main device to augment its features at runtime, adding devices on
> non-discoverable busses, using device tree overlays.
>
> The related big picture has been already presented in
> - the 'Add support for GE SUNH hot-pluggable connector' series [0]
> - the 'Runtime hotplug on non-discoverable busses with device tree
> overlays' talk at Linux Plumbers Conference 2024 [1].
>
> This series focuses on the i2c related part.
>
> An i2c bus is wired to the connector and allows an add-on board to
> connect additional i2c devices to this bus.
>
> When device tree nodes are added, the I2C core tries to probe client
> devices based on the classic DT structure:
>
> i2c@abcd0000 {
> some-client@42 { compatible = "xyz,blah"; ... };
> };
>
> However for hotplug connectors described via device tree overlays [0]
> 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.
> Rightfully, only the former will probe into an I2C adapter, and it will
> do that perhaps during boot, long before overlay insertion or after the
> overlay has been inserted for instance if the I2C adapter is remove and
> re-probed for whatever reason after the overlay insertion.
>
> The extension node won't probe into an I2C adapter or any other device
> or bus, so its subnodes ('eeprom@50') won't be interpreted as I2C
> clients by current I2C core code.
>
> The extension node is linked to the adapter node in two ways. The first
> one with the i2c-bus-extension adapter sub-node and the second one with
> the i2c-parent property in the extension node itself.
>
> The purpose of those two links is to handle device probing in several
> cases.
>
> - First case: Adapter already probed when add-on devices are added
>
> When devices are added by the overlay, an OF change notification is
> triggered so that busses can support those new devices.
>
> Going from a newly added device node, the i2c-parent property allows to
> find the corresponding I2C adapter and register the new I2C client with
> this adapter.
>
> The patch 1 in this series proposes modification to handle this case
> and, by the nature of the modification, all cases where a phandle refers
> an extension node instead of the adapter node itself.
>
> - Second case: Add-on devices already present in device-tree when
> adapter is probed
>
> In this case, everything is already described in the device-tree and
> then the adapter is probed.
>
> OF change notifications don't play a role in this case either because
> they were never triggered (the overlay was applied by the bootloader)
> or they were triggered before the adapter is probed and so were
> missed/ignored.
>
> The adapter probe process registers device already described at the
> adapter node level (current code) and, thanks to i2c-bus-extension
> adapter sub-node and its i2c-bus property, it can also follow the
> extension and registers devices described in those extension nodes.
>
> The patch 2 and 3 in this series proposes modifications to handle this
> case.
>
> I know device-tree bindings for i2c-bus-extension and i2c-parent are not
> yet provided in this RFC series.
>
> I would like to discuss the proposal before going further and write
> those needed bindinds (i2c-bus-extension needs to be added in
> i2c-controller.yaml available in dt-schema repository [2]).
>
> Best regards,
> Hervé Codina
>
> [0] https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@xxxxxxxxxxx/
> [1] https://lpc.events/event/18/contributions/1696/
> [2] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml
>
> Herve Codina (3):
> i2c: core: Follow i2c-parent when retrieving an adapter from node
> i2c: i2c-core-of: Move children registration in a dedicated function
> i2c: i2c-core-of: Handle i2c bus extensions
>
> drivers/i2c/i2c-core-base.c | 43 ++++++++++++++++++++++++++++-
> drivers/i2c/i2c-core-of.c | 54 +++++++++++++++++++++++++++++--------
> 2 files changed, 85 insertions(+), 12 deletions(-)
>
