Message-ID: <20250919172036.2f2b4bab@booty>
Date: Fri, 19 Sep 2025 17:20:36 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Ayush Singh <ayush@...gleboard.org>
Cc: David Gibson <david@...son.dropbear.id.au>, 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>, 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, 19 Sep 2025 10:47:17 +0530
Ayush Singh <ayush@...gleboard.org> 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.

Thinking out loud: what about preventing loading any overlay that does
more than just adding nodes? IOW forbidding to create properties in
nodes already in the live tree, and modifying existing properties.

I think being very restrictive in terms of overlays the implementation
can accept is a good idea in general. A requirement can be relaxed in
the future, but forbidding what used to be allowed would be a nightmare.

Best regards,
Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists