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

["Sverdlin, Alexander" <alexander.sverdlin@siemens.com>]

Hi Luca!

On Fri, 2024-12-13 at 12:28 +0100, Luca Ceresoli wrote:
> > > 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/

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

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com