Re: [PATCH v4 5/8] i2c: i2c-core-of: follow i2c-parent phandle to probe devices from added nodes

Hi Luca!

On Fri, 2024-12-13 at 12:28 +0100, Luca Ceresoli wrote:

quoted

quoted

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"; ... };
   i2c5: i2c@cafe0000 { compatible = "xyz,i2c-ctrl"; ... };

   connector {
       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  

Why don't you overlay it right over &i2c1?
It should have worked since commit ea7513bbc041
("i2c/of: Add OF_RECONFIG notifier handler").
Doesn't it work for your use-case?

One reason is decoupling the base board and addon. A different base
board may wire the same connector pins to 'i2c4' instead of 'i2c1'. We
want a single overlay to describe the addon, independently of the base
board, so it has to mention only connector pins, not base board
hardware.

Another reason is that using phandles to labels in the base tree in the
overlay (such as &i2c1) would need properties added by the __symbols__
node, and overlays adding properties to nodes in the live tree are not
welcome. This is both for a conceptual reason (adding an overlay ==
adding hardware and not _changing_ hardware, so adding nodes should be
enough) and an implementation one (properties added to nodes in the
live tree become deadprops and thus leak memory.

This topic was discussed at the latest Linux Plumbers Conference last
September. Slides and video of the discussion are available here:
https://lpc.events/event/18/contributions/1696/

More info are in the cover letter. Discussion leading to this
implementation started after v2:
https://lore.kernel.org/all/20240510163625.GA336987-robh@kernel.org/ (local)

I see! Thank you for the explanation and for the references!

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com