Message-ID: <aNJVqSpdAJzGliNx@zatzit>
Date: Tue, 23 Sep 2025 18:09:13 +1000
From: David Gibson <david@...son.dropbear.id.au>
To: Ayush Singh <ayush@...gleboard.org>
Cc: Herve Codina <herve.codina@...tlin.com>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	Rob Herring <robh@...nel.org>, Andrew Davis <afd@...com>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Luca Ceresoli <luca.ceresoli@...tlin.com>,
	devicetree@...r.kernel.org, Jason Kridner <jkridner@...il.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	devicetree-compiler@...r.kernel.org, linux-kernel@...r.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: Device tree representation of (hotplug) connectors: discussion
 at ELCE

On Fri, Sep 19, 2025 at 10:47:17AM +0530, Ayush Singh wrote:
> On 9/19/25 10:22, David Gibson wrote:
> 
> > On Thu, Sep 18, 2025 at 09:44:09AM +0200, Herve Codina wrote:
> > > Hi David,
> > > 
> > > On Thu, 18 Sep 2025 13:16:32 +1000
> > > David Gibson <david@...son.dropbear.id.au> wrote:
> > > 
> > > ...
> > > 
> > > > > > Thoughts above suggest a different direction, but here's what I was
> > > > > > thinking before:
> > > > > > 
> > > > > > base board:
> > > > > > 
> > > > > > 	connector {
> > > > > > 		/export/ "i2c" &i2c0;
> > > > > > 	};
> > > > > > 
> > > > > > addon:
> > > > > > 	eeprom@10 {
> > > > > > 		compatible = "foo,eeprom";
> > > > > > 		bus-reg = <&i2c 0x10>;
> > > > > > 	}
> > > > > > 
> > > > > > Or, if the addon had multiple i2c devices, maybe something like:
> > > > > > 
> > > > > > 	board-i2c {
> > > > > > 		compatible = "i2c-simple-bridge";
> > > > > > 		bus-ranges = <&i2c 0 0x3ff>; /* Whole addr space */
> > > > > > 		eeprom@10 {
> > > > > > 			compatible = "foo,eeprom";
> > > > > > 			reg = <0x10>;
> > > > > > 		}
> > > > > > 		widget@20 {
> > > > > > 			compatible = "vendor,widget";
> > > > > > 			reg = <0x20>;
> > > > > > 		}
> > > > > > 	}
> > > > > > 
> > > > > > Writing that, I realise I2C introduces some complications for this.
> > > > > > Because it has #size-cells = <0>, ranges doesn't really work (without
> > > > > > listing every single address to be translated).  Likewise, because we
> > > > > > always need the parent bus phandle, we can't use the trick of an empty
> > > > > > 'ranges' to mean an identity mapping.
> > > > > > 
> > > > > > We could invent encodings to address those, but given the addon with
> > > > > > multiple connectors case provides another incentive for a single
> > > > > > connector to allow adding nodes in multiple (but strictly enumerated)
> > > > > > places in the base device tree provides a better approach.
> > > > > and the "place in base device tree" is the goal of the extension bus.
> > > > > 
> > > > > The strict enumeration of nodes enumerated is done by two means:
> > > > >   - extension busses at connector level
> > > > >     Those extensions are described as connector sub-nodes.
> > > > >     The addon DT can only add nodes in those sub-nodes to describe devices
> > > > >     connected to the relared extension bus.
> > > > >   - export symbols
> > > > >     An addon DT can only use symbols exported to reference symbols outside
> > > > >     the addon DT itself.
> > > > > 
> > > > > Can I assume that bus extensions we proposed (i2c-bus-extension and
> > > > > spi-bus-extension) could be a correct solution ?
> > > > Maybe?  I prefer the idea of a universal mechanism, not one that's
> > > > defined per-bus-type.
> > > > 
> > > > 
> > > > Also, IIUC the way bus extension operates is a bit different - nodes
> > > > would be "physically" added under the bus extension node, but treated
> > > > logically as if they go under the main bus.  What I'm proposing here
> > > > is something at the actualy overlay application layer that allows
> > > > nodes to be added to different parts of the base device tree - so you
> > > > could add your i2c device under the main i2c bus.
> > > I think we should avoid this kind of node dispatching here and there in
> > > the base DT.
> > Until I saw Geert's multi-connector case, I would have agreed.  That
> > case makes me thing differently: in order to support that case we
> > already have to handle adding information in multiple places (under
> > all of the connectors the addon uses).  Given we have to handle that
> > anyway, I wonder if it makes more sense to lean into that, and allow
> > updates to multiple (strictly enumerated) places.
> 
> Well, I don't love this idea. Here are my main qalms about the approach of
> adding devices directly to the actual i2c/spi etc nodes.
> 
> 1. In boards with multiple connectors, they sometimes share the same i2c.
> Now assume that someone decided to connect the same i2c device to both the
> connectors. If we are using something like bus extension, while the node
> would be added, it will fail in the registration since you cannot add the
> same address device a second time. However, if we are adding the device
> directly to the `main_i2c`, the overlay application will just end up
> modifying the exact same device node. There is no error, or even a 2nd
> device node in this case. It is just lost.
> 
> 2. How well will overlay adding and removing work when the same tree nodes
> are modified by multiple connectors? I have not looked at the internals of
> overlay resolution so not sure, but I don't want dynamic addition and
> removal of devices in independent connectors to somehow become coupled.

Ah, right.  To be clear: we absolutely don't want multiple addons
altering the same nodes.  But I think we could do that in ways other
than putting everything under a connector.  This is exactly why I
think we should think this through as an end-to-end problem, rather
trying to do it as a tweak to the existing (crap) overlay system.

So, if we're thinking of this as an entirely new way of updating the
base dt - not "an overlay" - we can decide on the rules to ensure that
addition and removal is sane.  Two obvious ones I think we should
definitely have are:

a) Addons can only add completely new nodes, never modify existing
   ones.  This means that whatever addons are present at runtime,
   every node has a single well defined owner (either base board or
   addon).

b) Addons can only add nodes in places that are explicitly allowed by
   the connectors they're connecting to.

We could consider further rules as well though.  For example, we could
say that i2c devices in an addon shouldn't be added directly under the
base board's i2c controller, but under a subnode of that i2c
controller assigned to that connector (which would likely have an
empty 'ranges' property meaning addresses are mapped without
translation).  Not really sure if that rule has more benefits or
drawbacks, but it's worth contemplating.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists