Message-ID: <aNNrbmZfZU-1xJFm@zatzit>
Date: Wed, 24 Sep 2025 13:54:22 +1000
From: David Gibson <david@...son.dropbear.id.au>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Ayush Singh <ayush@...gleboard.org>,
	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 Tue, Sep 23, 2025 at 11:48:49AM +0200, Herve Codina wrote:
> Hi David,
> 
> On Tue, 23 Sep 2025 18:09:13 +1000
> David Gibson <david@...son.dropbear.id.au> wrote:
> 
> > 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).
> 
> In this rule I suppose that "never modify existing ones" should be understood
> as "never modify, add or remove properties in existing ones". Because, of course
> adding a full node in a existing one is allowed (rule b).

Yes, that's what I meant.  I'd add never delete subnodes as well. on
add.  Remove obviously would delete subnodes, but only exactly the
ones that were added on add.

> > b) Addons can only add nodes in places that are explicitly allowed by
> >    the connectors they're connecting to.
> 
> I fully agree with those both a) and b) rules.
> 
> > 
> > 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.
> 
> IMHO, no extra rules are needed in DT addon rules to constraint i2c devices
> to be added in a connector node, a connector sub-node or an i2c controller
> node.
> 
> This will be constrained by the connector itself (out of DT addon rules).

At this point I'm just considering the end-to-end rules we want to
enforce.  Exactly what stage of the process enforces each rule is
another question.

> I mean, according to rule b), the connector will allow some destination
> places. Either it will allow the i2c controller node or a connector sub-node.

Sure.

> This is specific to the connector definition and it should be out of
> generic DT addon rules.

Hang on... what distinction are you seeing between the "connector
definition" and "generic DT addon rules".  As I see it we're trying to
create a protocol that defines both the base rules and what a
"connector" even means.

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