Re: [PATCH 1/2] libfdt: overlay: Allow resolving phandle symbols

[Ayush Singh <ayush@beagleboard.org>]

On 9/25/24 12:58, David Gibson wrote:

> On Tue, Sep 24, 2024 at 12:11:36PM +0530, Ayush Singh wrote:
>> On 9/23/24 09:11, David Gibson wrote:
>>> On Fri, Sep 20, 2024 at 10:04:56PM +0530, Ayush Singh wrote:
>>>> On 9/18/24 08:06, David Gibson wrote:
> [snip]
>>>> Well, a lot of work is still going in this direction [1]. I myself
>>>> am trying to use it for mikroBUS connectors.
>>> Sure, and I can see why: it seems tantalizingly close to working
>>> simply.  But that doesn't mean it won't have limitations that will
>>> bite you down the track.
>> Well, my previous attempts at not using devicetree for the addon boards was
>> met with 2 main arguments:
>>
>> 1. Hardware should be described with devicetree.
>>
>> 2. There can exist a fairly complicated addon board which will not work
>> without devicetree.
>>
>> Additionally, it was mentioned that if something is missing from the
>> devicetree, I should look at extending device trees instead of trying to
>> bypass it. So, here we are.
> Absolutely.  This isn't about not using a devicetree, it's about the
> mechanism for updating the devicetree with plug in components.
> Overlays, as they're currently specced, are simple and easy... but
> also crude, limited and sloppily designed.
>
>
>>>> The reason for using devicetree overlays for such connectors is
>>>> because of their loose nature (mikroBUS is also open
>>>> connector). This means that as long as the sensor/ device can make
>>>> do with the pins provided by mikroBUS connector, it can be an addon
>>>> board. And there is no limitation of staking the boards. So one can
>>>> do rpi hat -> mikrbus connectors -> grove connector -> grove some
>>>> addon board.
>>> For example, the very fact that these are loose exposes you to one
>>> pretty obvious limitation here.  Suppose some future board has a
>>> connector with enough pins that you can hang two independent grove
>>> connectors off it, and someone makes a hat/shield/whatever that does
>>> exactly that.  But now the grove addon dtbs need to be different
>>> depending on whether they attach to grove connector 1 or grove
>>> connector 2.
>>>
>>> The old "connector binding" proposals I was describing aimed to
>>> decouple the type of the connector from the instance of the connector
>>> for exactly this sort of case.
>>
>> Do you have a link to the "connector binding" proposal you are mentioning
>> here? I really believe having a better way to support such connectors is
>> really important for embedded systems. And I am okay with adding any missing
>> bits to make it a reality.
>>
>> With `PREEMPT_RT` patches being merged, it is probably a good time to
>> improve embedded linux.
> I don't think there was ever a proposal written up as such.  It was
> just an idea floating around the mailing lists.  I did manage to dig
> up what were meant to be some illustrative examples of the idea.
> Alas, without any explanatory notes.  It was last modified in 2016,
> but let's see what I can remember in terms of context.  Note that all
> of the below was a quick draft - it would certainly need polish and
> all syntax is negotiable.  In particular the use of the /plugin/
> keyword might not be compatible with its current use for overlays, so
> that would probably need changing.

Thanks. I also personally would be interested in an approach that can 
avoid symbols. The reason being I would like to be able to use the same 
addon board overlays to support mikroBUS on Zephyr.

I also like the prospect of doing connector versioning.

> The idea is that a base board could define specific "connectors",
> which could describe what buses / pins / interrupts / whatever were
> exposed on that connector.  Each connector instance had some local
> aliases referencing the nodes in the base board the connector could
> alter.
>
> So, a board with a "widget" socket which exposes two interrupt lines,
> an I2C bus and an MMIO bus might look roughly like this:
>
> /dts-v1/;
>
> / {
> 	compatible = "foo,oldboard";
> 	ranges;
> 	soc@... {
> 		ranges;
> 		mmio: mmio-bus@... {
> 			#address-cells = <2>;
> 			#size-cells = <2>;
> 			ranges;
> 		};
> 		i2c: i2c@... {
> 		};
> 		intc: intc@... {
> 			#interrupt-cells = <2>;
> 		};
> 	};
>
> 	connectors {
> 		widget1 {
> 			compatible = "foo,widget-socket";
> 			w1_irqs: irqs {
> 				interrupt-controller;
> 				#address-cells = <0>;
> 				#interrupt-cells = <1>;
> 				interrupt-map-mask = <0xffffffff>;
> 				interrupt-map = <
> 					0 &intc 7 0
> 					1 &intc 8 0
> 				>;
> 			};
> 			aliases = {
> 				i2c = &i2c;
> 				intc = &w1_irqs;
> 				mmio = &mmio;
> 			};
> 		};
> 	};
> };
>
>
> A later version of the board might expose two widget sockets that are
> backwards compatible with the original widget but also add some new
> features.  It might look like this:
>
> /dts-v1/;
>
> / {
> 	compatible = "foo,newboard";
> 	ranges;
> 	soc@... {
> 		ranges;	
> 		mmio: mmio-bus@... {
> 			#address-cells = <2>;
> 			#size-cells = <2>;
> 			ranges;
> 		};
> 		i2c0: i2c@... {
> 		};
> 		i2c1: i2c@... {
> 		};
> 		intc: intc@... {
> 		};
> 	};
>
> 	connectors {
> 		widget1 {
> 			compatible = "foo,widget-socket-v2", "foo,widget-socket";
> 			w1_irqs: irqs {
> 				interrupt-controller;
> 				#address-cells = <0>;
> 				#interrupt-cells = <1>;
> 				interrupt-map-mask = <0xffffffff>;
> 				interrupt-map = <
> 					0 &intc 17 0
> 					1 &intc 8 0
> 				>;
> 			};
> 			aliases = {
> 				i2c = &i2c0;
> 				intc = &w1_irqs;
> 				mmio = &mmio;
> 			};
> 		};
> 		widget2 {
> 			compatible = "foo,widget-socket-v2", "foo,widget-socket";
> 			w2_irqs: irqs {
> 				interrupt-controller;
> 				#address-cells = <0>;
> 				#interrupt-cells = <1>;
> 				interrupt-map-mask = <0xffffffff>;
> 				interrupt-map = <
> 					0 &intc 9 0
> 					1 &intc 10 0
> 				>;
> 			};
> 			aliases = {
> 				i2c = &i2c1;
> 				widget-irqs = &w2_irqs;
> 				mmio = &mmio;
> 			};
> 		};
> 	};
> };
>
>
> A plugin device tree describing something which plugs into a widget
> socket might look like this.  Note that it specifies the *type* of
> socket it goes into ("foo,widget-socket"), but not the specific
> instance of a socket it goes into.  When plugging this into a
> "foo,newboard" you'd have to specify whether this is attaching to the
> widget1 or widget2 socket.
>
> /dts-v1/;
>
> /plugin/ foo,widget-socket {
> 	compatible = "foo,whirligig-widget";
> };
>
> &i2c {
> 	whirligig-controller@... {
> 		...
> 		interrupt-parent = <&widget-irqs>;
> 		interrupts = <0>;
> 	};
> };

I do have a few questions regarding this approach:

1. How do you propose selecting the connector should be done? We can 
rely on the connector driver for this, but the symbols [0] based 
approach does have an interesting benefit. It can work with applying 
overlays from uboot, or well any other static method where we already 
know which board we want to use. This is quite useful in embedded context.

2. Where would `compatible = "foo,whirligig-widget";` be applied to? 
Will it override the compatible property of the `widget{n}`? If I am 
right, then I guess the connector will have a single driver instead of 
each widget having a unique driver attached to it.

> A plugin could also expose a secondary connector.  Here's one which
> adds a "superbus" controller mapped into the parent's MMIO bus.
>
> /dts-v1/;
>
> /plugin/ foo,widget-socket-v2 {
> 	compatible = "foo,superduper-widget};
>
> 	connectors {
> 		compatible = "foo,super-socket";
> 		aliases {
> 			superbus = &superbus;
> 		};	
> 	};
> };
>
> &mmio {
> 	superbus: super-bridge@100000000 {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
> 	};
> };
>
> &i2c {
> 	super-controller@... {
> 		...
> 	};
> 	duper-controller@... {
> 	};
> };
>
>

[0]: 
https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/


Ayush Singh