Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

Hi Phil,

On Thu, 13 Feb 2025 15:18:45 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> Hi Andrea,
> 
> The problem with this approach (loading an overlay from the RP1 PCIe
> driver), and it's one that I have raised with you offline, is that
> (unless anyone can prove otherwise) it becomes impossible to create a
> Pi 5 DTS file which makes use of the RP1's resources. How do you
> declare something as simple as a button wired to an RP1 GPIO, or fan
> connected to a PWM output?

The driver could be improved in a second step.
For instance, it could load the dtbo from user-space using request_firmare()
instead of loading the embedded dtbo.

> 
> If this is the preferred route to upstream adoption, I would prefer it
> if rp1.dtso could be split in two - an rp1.dtsi similar to what we
> have downstream, and an rp1.dtso that #includes it. In this way we can
> keep the patching and duplication to a minimum.

Indeed, having a rp1.dtsi avoid duplication but how the rp1.dtso in
the the kernel sources could include user customization (button, fan, ...)
without being modified ?
At least we have to '#include <my_rp1_customizations.dtsi>'.

Requesting the dtbo from user-space allows to let the user to create
its own dtso without the need to modify the one in kernel sources.

Does it make sense ?

Best regards,
Hervé

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrew Lunn]

On Thu, Feb 13, 2025 at 05:14:35PM +0100, Herve Codina wrote:
> Hi Phil,
> 
> On Thu, 13 Feb 2025 15:18:45 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
> 
> > Hi Andrea,
> > 
> > The problem with this approach (loading an overlay from the RP1 PCIe
> > driver), and it's one that I have raised with you offline, is that
> > (unless anyone can prove otherwise) it becomes impossible to create a
> > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > declare something as simple as a button wired to an RP1 GPIO, or fan
> > connected to a PWM output?

Where is this button or fan? On a pluggable board? Isn't that what
overlays are for, and they are stackable. So when you probe the
pluggable board via its eeprom etc, you find the overlay and load it?

Or do you mean a custom board, which has a CPU, RP1 and the button and
fan are directly on this custom board? You then want a board DTS which
includes all these pieces?

	Andrew

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Hi Andrew,

On Thu, 13 Feb 2025 at 16:27, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Feb 13, 2025 at 05:14:35PM +0100, Herve Codina wrote:
> > Hi Phil,
> >
> > On Thu, 13 Feb 2025 15:18:45 +0000
> > Phil Elwell <phil@raspberrypi.com> wrote:
> >
> > > Hi Andrea,
> > >
> > > The problem with this approach (loading an overlay from the RP1 PCIe
> > > driver), and it's one that I have raised with you offline, is that
> > > (unless anyone can prove otherwise) it becomes impossible to create a
> > > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > > declare something as simple as a button wired to an RP1 GPIO, or fan
> > > connected to a PWM output?
>
> Where is this button or fan? On a pluggable board? Isn't that what
> overlays are for, and they are stackable. So when you probe the
> pluggable board via its eeprom etc, you find the overlay and load it?

In the Raspberry Pi ecosystem it would be the firmware that applies
the overlay, and it can't do that if the resources the overlay refers
to are not yet present in the dtb.

> Or do you mean a custom board, which has a CPU, RP1 and the button and
> fan are directly on this custom board? You then want a board DTS which
> includes all these pieces?

That depends on whether you count the Raspberry Pi 5 as a custom board.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

On Thu, 13 Feb 2025 16:30:44 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> Hi Andrew,
> 
> On Thu, 13 Feb 2025 at 16:27, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Thu, Feb 13, 2025 at 05:14:35PM +0100, Herve Codina wrote:  
> > > Hi Phil,
> > >
> > > On Thu, 13 Feb 2025 15:18:45 +0000
> > > Phil Elwell <phil@raspberrypi.com> wrote:
> > >  
> > > > Hi Andrea,
> > > >
> > > > The problem with this approach (loading an overlay from the RP1 PCIe
> > > > driver), and it's one that I have raised with you offline, is that
> > > > (unless anyone can prove otherwise) it becomes impossible to create a
> > > > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > > > declare something as simple as a button wired to an RP1 GPIO, or fan
> > > > connected to a PWM output?  
> >
> > Where is this button or fan? On a pluggable board? Isn't that what
> > overlays are for, and they are stackable. So when you probe the
> > pluggable board via its eeprom etc, you find the overlay and load it?  
> 
> In the Raspberry Pi ecosystem it would be the firmware that applies
> the overlay, and it can't do that if the resources the overlay refers
> to are not yet present in the dtb.

What do you mean by the 'the resources are not yet present in the dtb' ?

Also what you call the 'firmware' is the bootloader ? the kernel ?
Can you tell me who is the 'firmware' what is the mecanisme it uses to
load the overlay.

Best regards,
Hervé


> 
> > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > fan are directly on this custom board? You then want a board DTS which
> > includes all these pieces?  
> 
> That depends on whether you count the Raspberry Pi 5 as a custom board.
> 

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Hervé,

On Thu, 13 Feb 2025 at 17:07, Herve Codina <herve.codina@bootlin.com> wrote:
>
> On Thu, 13 Feb 2025 16:30:44 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>
> > Hi Andrew,
> >
> > On Thu, 13 Feb 2025 at 16:27, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Thu, Feb 13, 2025 at 05:14:35PM +0100, Herve Codina wrote:
> > > > Hi Phil,
> > > >
> > > > On Thu, 13 Feb 2025 15:18:45 +0000
> > > > Phil Elwell <phil@raspberrypi.com> wrote:
> > > >
> > > > > Hi Andrea,
> > > > >
> > > > > The problem with this approach (loading an overlay from the RP1 PCIe
> > > > > driver), and it's one that I have raised with you offline, is that
> > > > > (unless anyone can prove otherwise) it becomes impossible to create a
> > > > > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > > > > declare something as simple as a button wired to an RP1 GPIO, or fan
> > > > > connected to a PWM output?
> > >
> > > Where is this button or fan? On a pluggable board? Isn't that what
> > > overlays are for, and they are stackable. So when you probe the
> > > pluggable board via its eeprom etc, you find the overlay and load it?
> >
> > In the Raspberry Pi ecosystem it would be the firmware that applies
> > the overlay, and it can't do that if the resources the overlay refers
> > to are not yet present in the dtb.
>
> What do you mean by the 'the resources are not yet present in the dtb' ?

Consider the fan connector on the Pi 5 PCB. It is wired to GPIO 45 on
RP1. In our current Pi 5 dtb there is an instance of pwm-fan whose
"pwms" property links to rp1_pwm node, which declares a PWM block on
RP1. Similarly, the camera and display ports make use of I2C
interfaces on RP1. The camera and display overlays, applied by the
firmware, have references to those interfaces.

If RP1 is not present in the dtb then neither of those scenarios -
board features and overlays applied the firmware - will work because
the necessary nodes and symbols are not present until the kernel has
started running, at which point dtb has been handed over.

> Also what you call the 'firmware' is the bootloader ? the kernel ?
> Can you tell me who is the 'firmware' what is the mecanisme it uses to
> load the overlay.

In the case of the Pi 5, the firmware is an EEPROM image containing
code run by the VPU embedded processor. It can load overlays using the
same mechanisms it uses to load the kernel - SD/EMMC, NVME, USB, TFTP,
etc. The same problem would exist for U-boot. Even though RP1 has a
standard XHCI controller, U-boot wouldn't be able to detect it and
make use of it, say to load a kernel from a USB stick, because it
isn't declared in the DTB.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrew Lunn]

> etc. The same problem would exist for U-boot. Even though RP1 has a
> standard XHCI controller

Although it is a standard XHCI controller, i assume it is not a PCIe
XHCI controller which can just be enumerated as a PCIe device?

	Andrew

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

On Thu, 13 Feb 2025 at 17:51, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > etc. The same problem would exist for U-boot. Even though RP1 has a
> > standard XHCI controller
>
> Although it is a standard XHCI controller, i assume it is not a PCIe
> XHCI controller which can just be enumerated as a PCIe device?

There are too many sub-devices in RP1 for them each to get their own
function. Instead, they all share a single function, with DT to
declare them.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrew Lunn]

> > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > fan are directly on this custom board? You then want a board DTS which
> > includes all these pieces?
> 
> That depends on whether you count the Raspberry Pi 5 as a custom board.

So you mean the Pi 5 board would itself make use of the resources the
RP1 device has? They are not simply connected to headers for plugin
boards, but used by the main board? Hence you want to describe them in
the board .DTS file.

	Andrew

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > fan are directly on this custom board? You then want a board DTS which
> > > includes all these pieces?
> >
> > That depends on whether you count the Raspberry Pi 5 as a custom board.
>
> So you mean the Pi 5 board would itself make use of the resources the
> RP1 device has? They are not simply connected to headers for plugin
> boards, but used by the main board? Hence you want to describe them in
> the board .DTS file.

That's correct. But even for plug-in devices, those which are on
non-discoverable buses need overlays to declare them, which causes a
problem when the overlay application happens before the kernel is
started.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

Hi Phil,

On Thu, 13 Feb 2025 17:57:37 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:
> >  
> > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > fan are directly on this custom board? You then want a board DTS which
> > > > includes all these pieces?  
> > >
> > > That depends on whether you count the Raspberry Pi 5 as a custom board.  
> >
> > So you mean the Pi 5 board would itself make use of the resources the
> > RP1 device has? They are not simply connected to headers for plugin
> > boards, but used by the main board? Hence you want to describe them in
> > the board .DTS file.  
> 
> That's correct. But even for plug-in devices, those which are on
> non-discoverable buses need overlays to declare them, which causes a
> problem when the overlay application happens before the kernel is
> started.
> 

Hum, I see.

We worked on overlay usage on non-discoverable buses wired to a connector
and we did a talk about issues we are facing on at Plumber [0].

You can also find our big picture in [1] and a last contribution introducing
export-symbols feature in [2]. export-symbols is also under discussion on
some other threads.

Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
an addon board to add devices on an i2c bus provided by a base board and
wired to an connector the addon board is connected to.

Maybe in your case, you can decouple resources (gpio, pwm) provided by the
addon board and used by the base board using also nexus node.

We use a nexus node [4] (not presented at the Plumbers talk because the idea
came during 'out of talk' discussions in Plumbers) in order to allow our
addon board to use resources provided by the base board.

In your case, if I understood, you are in the other direction but why not
using also a nexus node to decouple and translate resources in this other
direction ?

Don't know if this idea can help but feel free to ask for some more
information if needed.

[0] https://lpc.events/event/18/contributions/1696/
[1] https://lore.kernel.org/lkml/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/
[2] https://lore.kernel.org/all/20241209151830.95723-1-herve.codina@bootlin.com/
[3] https://lore.kernel.org/all/20250205173918.600037-1-herve.codina@bootlin.com/
[4] https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping

Best regards,
Hervé

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Once more, with plain text, which I'd hoped the Android GMail client
would work out for itself.

On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:
>
> Hi Phil,
>
> On Thu, 13 Feb 2025 17:57:37 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>
> > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > includes all these pieces?
> > > >
> > > > That depends on whether you count the Raspberry Pi 5 as a custom board.
> > >
> > > So you mean the Pi 5 board would itself make use of the resources the
> > > RP1 device has? They are not simply connected to headers for plugin
> > > boards, but used by the main board? Hence you want to describe them in
> > > the board .DTS file.
> >
> > That's correct. But even for plug-in devices, those which are on
> > non-discoverable buses need overlays to declare them, which causes a
> > problem when the overlay application happens before the kernel is
> > started.
> >
>
> Hum, I see.
>
> We worked on overlay usage on non-discoverable buses wired to a connector
> and we did a talk about issues we are facing on at Plumber [0].
>
> You can also find our big picture in [1] and a last contribution introducing
> export-symbols feature in [2]. export-symbols is also under discussion on
> some other threads.
>
> Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> an addon board to add devices on an i2c bus provided by a base board and
> wired to an connector the addon board is connected to.
>
> Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> addon board and used by the base board using also nexus node.
>
> We use a nexus node [4] (not presented at the Plumbers talk because the idea
> came during 'out of talk' discussions in Plumbers) in order to allow our
> addon board to use resources provided by the base board.
>
> In your case, if I understood, you are in the other direction but why not
> using also a nexus node to decouple and translate resources in this other
> direction ?
>
> Don't know if this idea can help but feel free to ask for some more
> information if needed.

Nexus nodes look interesting - I see them as adding a layer of
abstraction such that, for example, boards can declare which of their
specific resources performs a common function so that clients can
treat them all the same. We do the same thing in a limited way by
using common labels on nodes, but this goes much further.

In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
dtb declares the connector node and the overlay fills in the content
with references to its GPIO controller, PWM controller etc. However, I
think the overlay would also have to be board specific because it's
not possible to patch part of a property from an overlay, so you'd end
up overwriting the GPIO number as well as the controller reference.

What is needed to make this work is the ability to cope with
unresolved references in the base dtb, to be resolved as each overlay
is applied, with runtime checking that each reference is resolved
before it is used, all of which sounds like a nightmare. Plus, we
really don't want to have to change the way all our camera and display
overlays work on all Raspberry Pis just to accommodate somebody's idea
of how RP1 should be handled.

Besides, Occam's razor surely applies.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

Hi Phil,

On Thu, 13 Feb 2025 20:15:06 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> Once more, with plain text, which I'd hoped the Android GMail client
> would work out for itself.
> 
> On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:
> >
> > Hi Phil,
> >
> > On Thu, 13 Feb 2025 17:57:37 +0000
> > Phil Elwell <phil@raspberrypi.com> wrote:
> >  
> > > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:  
> > > >  
> > > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > > includes all these pieces?  
> > > > >
> > > > > That depends on whether you count the Raspberry Pi 5 as a custom board.  
> > > >
> > > > So you mean the Pi 5 board would itself make use of the resources the
> > > > RP1 device has? They are not simply connected to headers for plugin
> > > > boards, but used by the main board? Hence you want to describe them in
> > > > the board .DTS file.  
> > >
> > > That's correct. But even for plug-in devices, those which are on
> > > non-discoverable buses need overlays to declare them, which causes a
> > > problem when the overlay application happens before the kernel is
> > > started.
> > >  
> >
> > Hum, I see.
> >
> > We worked on overlay usage on non-discoverable buses wired to a connector
> > and we did a talk about issues we are facing on at Plumber [0].
> >
> > You can also find our big picture in [1] and a last contribution introducing
> > export-symbols feature in [2]. export-symbols is also under discussion on
> > some other threads.
> >
> > Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> > an addon board to add devices on an i2c bus provided by a base board and
> > wired to an connector the addon board is connected to.
> >
> > Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> > addon board and used by the base board using also nexus node.
> >
> > We use a nexus node [4] (not presented at the Plumbers talk because the idea
> > came during 'out of talk' discussions in Plumbers) in order to allow our
> > addon board to use resources provided by the base board.
> >
> > In your case, if I understood, you are in the other direction but why not
> > using also a nexus node to decouple and translate resources in this other
> > direction ?
> >
> > Don't know if this idea can help but feel free to ask for some more
> > information if needed.  
> 
> Nexus nodes look interesting - I see them as adding a layer of
> abstraction such that, for example, boards can declare which of their
> specific resources performs a common function so that clients can
> treat them all the same. We do the same thing in a limited way by
> using common labels on nodes, but this goes much further.
> 
> In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
> dtb declares the connector node and the overlay fills in the content
> with references to its GPIO controller, PWM controller etc. However, I
> think the overlay would also have to be board specific because it's
> not possible to patch part of a property from an overlay, so you'd end
> up overwriting the GPIO number as well as the controller reference.
> 
> What is needed to make this work is the ability to cope with
> unresolved references in the base dtb, to be resolved as each overlay
> is applied, with runtime checking that each reference is resolved
> before it is used, all of which sounds like a nightmare. Plus, we
> really don't want to have to change the way all our camera and display
> overlays work on all Raspberry Pis just to accommodate somebody's idea
> of how RP1 should be handled.

Just to be clear, my comments were not there to tell you how RP1 should
work. I just proposed ideas without trying to force anything and I can
fully understand that ideas proposed don't feed your needs.

Sorry if my approach was misunderstood.

Best regards,
Hervé

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

On Thu, 13 Feb 2025, 21:06 Herve Codina, <herve.codina@bootlin.com> wrote:
>
> Hi Phil,
>
> On Thu, 13 Feb 2025 20:15:06 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>
> > Once more, with plain text, which I'd hoped the Android GMail client
> > would work out for itself.
> >
> > On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:
> > >
> > > Hi Phil,
> > >
> > > On Thu, 13 Feb 2025 17:57:37 +0000
> > > Phil Elwell <phil@raspberrypi.com> wrote:
> > >
> > > > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:
> > > > >
> > > > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > > > includes all these pieces?
> > > > > >
> > > > > > That depends on whether you count the Raspberry Pi 5 as a custom board.
> > > > >
> > > > > So you mean the Pi 5 board would itself make use of the resources the
> > > > > RP1 device has? They are not simply connected to headers for plugin
> > > > > boards, but used by the main board? Hence you want to describe them in
> > > > > the board .DTS file.
> > > >
> > > > That's correct. But even for plug-in devices, those which are on
> > > > non-discoverable buses need overlays to declare them, which causes a
> > > > problem when the overlay application happens before the kernel is
> > > > started.
> > > >
> > >
> > > Hum, I see.
> > >
> > > We worked on overlay usage on non-discoverable buses wired to a connector
> > > and we did a talk about issues we are facing on at Plumber [0].
> > >
> > > You can also find our big picture in [1] and a last contribution introducing
> > > export-symbols feature in [2]. export-symbols is also under discussion on
> > > some other threads.
> > >
> > > Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> > > an addon board to add devices on an i2c bus provided by a base board and
> > > wired to an connector the addon board is connected to.
> > >
> > > Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> > > addon board and used by the base board using also nexus node.
> > >
> > > We use a nexus node [4] (not presented at the Plumbers talk because the idea
> > > came during 'out of talk' discussions in Plumbers) in order to allow our
> > > addon board to use resources provided by the base board.
> > >
> > > In your case, if I understood, you are in the other direction but why not
> > > using also a nexus node to decouple and translate resources in this other
> > > direction ?
> > >
> > > Don't know if this idea can help but feel free to ask for some more
> > > information if needed.
> >
> > Nexus nodes look interesting - I see them as adding a layer of
> > abstraction such that, for example, boards can declare which of their
> > specific resources performs a common function so that clients can
> > treat them all the same. We do the same thing in a limited way by
> > using common labels on nodes, but this goes much further.
> >
> > In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
> > dtb declares the connector node and the overlay fills in the content
> > with references to its GPIO controller, PWM controller etc. However, I
> > think the overlay would also have to be board specific because it's
> > not possible to patch part of a property from an overlay, so you'd end
> > up overwriting the GPIO number as well as the controller reference.
> >
> > What is needed to make this work is the ability to cope with
> > unresolved references in the base dtb, to be resolved as each overlay
> > is applied, with runtime checking that each reference is resolved
> > before it is used, all of which sounds like a nightmare. Plus, we
> > really don't want to have to change the way all our camera and display
> > overlays work on all Raspberry Pis just to accommodate somebody's idea
> > of how RP1 should be handled.
>
> Just to be clear, my comments were not there to tell you how RP1 should
> work. I just proposed ideas without trying to force anything and I can
> fully understand that ideas proposed don't feed your needs.
>
> Sorry if my approach was misunderstood.

I feel I've been misunderstood - I appreciate your ideas.

Perhaps it would help if you could outline how you think we could
apply your suggestions?

Thanks,

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

Hi Phil,

On Thu, 13 Feb 2025 21:12:43 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> On Thu, 13 Feb 2025, 21:06 Herve Codina, <herve.codina@bootlin.com> wrote:
> >
> > Hi Phil,
> >
> > On Thu, 13 Feb 2025 20:15:06 +0000
> > Phil Elwell <phil@raspberrypi.com> wrote:
> >  
> > > Once more, with plain text, which I'd hoped the Android GMail client
> > > would work out for itself.
> > >
> > > On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:  
> > > >
> > > > Hi Phil,
> > > >
> > > > On Thu, 13 Feb 2025 17:57:37 +0000
> > > > Phil Elwell <phil@raspberrypi.com> wrote:
> > > >  
> > > > > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:  
> > > > > >  
> > > > > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > > > > includes all these pieces?  
> > > > > > >
> > > > > > > That depends on whether you count the Raspberry Pi 5 as a custom board.  
> > > > > >
> > > > > > So you mean the Pi 5 board would itself make use of the resources the
> > > > > > RP1 device has? They are not simply connected to headers for plugin
> > > > > > boards, but used by the main board? Hence you want to describe them in
> > > > > > the board .DTS file.  
> > > > >
> > > > > That's correct. But even for plug-in devices, those which are on
> > > > > non-discoverable buses need overlays to declare them, which causes a
> > > > > problem when the overlay application happens before the kernel is
> > > > > started.
> > > > >  
> > > >
> > > > Hum, I see.
> > > >
> > > > We worked on overlay usage on non-discoverable buses wired to a connector
> > > > and we did a talk about issues we are facing on at Plumber [0].
> > > >
> > > > You can also find our big picture in [1] and a last contribution introducing
> > > > export-symbols feature in [2]. export-symbols is also under discussion on
> > > > some other threads.
> > > >
> > > > Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> > > > an addon board to add devices on an i2c bus provided by a base board and
> > > > wired to an connector the addon board is connected to.
> > > >
> > > > Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> > > > addon board and used by the base board using also nexus node.
> > > >
> > > > We use a nexus node [4] (not presented at the Plumbers talk because the idea
> > > > came during 'out of talk' discussions in Plumbers) in order to allow our
> > > > addon board to use resources provided by the base board.
> > > >
> > > > In your case, if I understood, you are in the other direction but why not
> > > > using also a nexus node to decouple and translate resources in this other
> > > > direction ?
> > > >
> > > > Don't know if this idea can help but feel free to ask for some more
> > > > information if needed.  
> > >
> > > Nexus nodes look interesting - I see them as adding a layer of
> > > abstraction such that, for example, boards can declare which of their
> > > specific resources performs a common function so that clients can
> > > treat them all the same. We do the same thing in a limited way by
> > > using common labels on nodes, but this goes much further.
> > >
> > > In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
> > > dtb declares the connector node and the overlay fills in the content
> > > with references to its GPIO controller, PWM controller etc. However, I
> > > think the overlay would also have to be board specific because it's
> > > not possible to patch part of a property from an overlay, so you'd end
> > > up overwriting the GPIO number as well as the controller reference.
> > >
> > > What is needed to make this work is the ability to cope with
> > > unresolved references in the base dtb, to be resolved as each overlay
> > > is applied, with runtime checking that each reference is resolved
> > > before it is used, all of which sounds like a nightmare. Plus, we
> > > really don't want to have to change the way all our camera and display
> > > overlays work on all Raspberry Pis just to accommodate somebody's idea
> > > of how RP1 should be handled.  
> >
> > Just to be clear, my comments were not there to tell you how RP1 should
> > work. I just proposed ideas without trying to force anything and I can
> > fully understand that ideas proposed don't feed your needs.
> >
> > Sorry if my approach was misunderstood.  
> 
> I feel I've been misunderstood - I appreciate your ideas.
> 
> Perhaps it would help if you could outline how you think we could
> apply your suggestions?
> 

I was thinking about what your mentioned, i.e. the overlay fill the nexus node.
No sure to understand why the overlay should patch some properties.
Also where are the unresolved references in that case. The base DT refers to
the Nexus node.
The issue will probably be that the translation performed by the nexus node is
not available until the overlay is applied. The consumer will see errors other
than PROBE_DEFER when if probes while the overlay is not applied.

Also, the solution will lead to memory leak at runtime. Indeed, the overlay
add properties in an already existing node.
If the overlay is applied by the Kernel itself, this lead to memory leak when
the overlay is removed.
Indeed, an overlay can add/remove node without any issue but it cannot
add/remove properties to/from existing nodes.

In the case described here, the nexus node is already present in the DT and the
overlay add/remove properties to/from this existing node.

I haven't got any better idea for the moment in order to have this kind of
'reverse' Nexus node.

Best regards,
Hervé

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Hi Hervé,

On Mon, 17 Feb 2025 at 15:53, Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi Phil,
>
> On Thu, 13 Feb 2025 21:12:43 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>
> > On Thu, 13 Feb 2025, 21:06 Herve Codina, <herve.codina@bootlin.com> wrote:
> > >
> > > Hi Phil,
> > >
> > > On Thu, 13 Feb 2025 20:15:06 +0000
> > > Phil Elwell <phil@raspberrypi.com> wrote:
> > >
> > > > Once more, with plain text, which I'd hoped the Android GMail client
> > > > would work out for itself.
> > > >
> > > > On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:
> > > > >
> > > > > Hi Phil,
> > > > >
> > > > > On Thu, 13 Feb 2025 17:57:37 +0000
> > > > > Phil Elwell <phil@raspberrypi.com> wrote:
> > > > >
> > > > > > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:
> > > > > > >
> > > > > > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > > > > > includes all these pieces?
> > > > > > > >
> > > > > > > > That depends on whether you count the Raspberry Pi 5 as a custom board.
> > > > > > >
> > > > > > > So you mean the Pi 5 board would itself make use of the resources the
> > > > > > > RP1 device has? They are not simply connected to headers for plugin
> > > > > > > boards, but used by the main board? Hence you want to describe them in
> > > > > > > the board .DTS file.
> > > > > >
> > > > > > That's correct. But even for plug-in devices, those which are on
> > > > > > non-discoverable buses need overlays to declare them, which causes a
> > > > > > problem when the overlay application happens before the kernel is
> > > > > > started.
> > > > > >
> > > > >
> > > > > Hum, I see.
> > > > >
> > > > > We worked on overlay usage on non-discoverable buses wired to a connector
> > > > > and we did a talk about issues we are facing on at Plumber [0].
> > > > >
> > > > > You can also find our big picture in [1] and a last contribution introducing
> > > > > export-symbols feature in [2]. export-symbols is also under discussion on
> > > > > some other threads.
> > > > >
> > > > > Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> > > > > an addon board to add devices on an i2c bus provided by a base board and
> > > > > wired to an connector the addon board is connected to.
> > > > >
> > > > > Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> > > > > addon board and used by the base board using also nexus node.
> > > > >
> > > > > We use a nexus node [4] (not presented at the Plumbers talk because the idea
> > > > > came during 'out of talk' discussions in Plumbers) in order to allow our
> > > > > addon board to use resources provided by the base board.
> > > > >
> > > > > In your case, if I understood, you are in the other direction but why not
> > > > > using also a nexus node to decouple and translate resources in this other
> > > > > direction ?
> > > > >
> > > > > Don't know if this idea can help but feel free to ask for some more
> > > > > information if needed.
> > > >
> > > > Nexus nodes look interesting - I see them as adding a layer of
> > > > abstraction such that, for example, boards can declare which of their
> > > > specific resources performs a common function so that clients can
> > > > treat them all the same. We do the same thing in a limited way by
> > > > using common labels on nodes, but this goes much further.
> > > >
> > > > In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
> > > > dtb declares the connector node and the overlay fills in the content
> > > > with references to its GPIO controller, PWM controller etc. However, I
> > > > think the overlay would also have to be board specific because it's
> > > > not possible to patch part of a property from an overlay, so you'd end
> > > > up overwriting the GPIO number as well as the controller reference.
> > > >
> > > > What is needed to make this work is the ability to cope with
> > > > unresolved references in the base dtb, to be resolved as each overlay
> > > > is applied, with runtime checking that each reference is resolved
> > > > before it is used, all of which sounds like a nightmare. Plus, we
> > > > really don't want to have to change the way all our camera and display
> > > > overlays work on all Raspberry Pis just to accommodate somebody's idea
> > > > of how RP1 should be handled.
> > >
> > > Just to be clear, my comments were not there to tell you how RP1 should
> > > work. I just proposed ideas without trying to force anything and I can
> > > fully understand that ideas proposed don't feed your needs.
> > >
> > > Sorry if my approach was misunderstood.
> >
> > I feel I've been misunderstood - I appreciate your ideas.
> >
> > Perhaps it would help if you could outline how you think we could
> > apply your suggestions?
> >
>
> I was thinking about what your mentioned, i.e. the overlay fill the nexus node.
> No sure to understand why the overlay should patch some properties.
> Also where are the unresolved references in that case. The base DT refers to
> the Nexus node.
> The issue will probably be that the translation performed by the nexus node is
> not available until the overlay is applied. The consumer will see errors other
> than PROBE_DEFER when if probes while the overlay is not applied.

The job of the nexus node would be to translate a generic request for
a numbered resource to a specific request for an RP1 resource with
arbitrary properties. The arbitrary properties could be GPIO offsets,
which are board specific, while the node supplying the resource is
provided by the overlay. This means that an entry in the table,
described by a single property, could have contributions from the base
DT and the overlay, which is not possible since overlays overwrite
whole properties.

Perhaps that particular problem could be overcome by creating a
single-entry map, using the map-mask feature to pass through all of
the GPIO offset and flags to the parent, so that the whole table
becomes a proxy for RP1's GPIO controller. Is that what you had in
mind?

> Also, the solution will lead to memory leak at runtime. Indeed, the overlay
> add properties in an already existing node.
> If the overlay is applied by the Kernel itself, this lead to memory leak when
> the overlay is removed.
> Indeed, an overlay can add/remove node without any issue but it cannot
> add/remove properties to/from existing nodes.

Fortunately for me I'm not arguing _for_ the use of an overlay.

> In the case described here, the nexus node is already present in the DT and the
> overlay add/remove properties to/from this existing node.

I think I can see how that could be made to work for GPIOs. It looks
as though the GPIO subsystem is the only one making use of
of_parse_phandle_with_args_map. Interrupts seem to have an open-coded
equivalent, and iommus. What about I2C and PWM?

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Herve Codina]

On Mon, 17 Feb 2025 17:03:34 +0000
Phil Elwell <phil@raspberrypi.com> wrote:

> Hi Hervé,
> 
> On Mon, 17 Feb 2025 at 15:53, Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Hi Phil,
> >
> > On Thu, 13 Feb 2025 21:12:43 +0000
> > Phil Elwell <phil@raspberrypi.com> wrote:
> >  
> > > On Thu, 13 Feb 2025, 21:06 Herve Codina, <herve.codina@bootlin.com> wrote:  
> > > >
> > > > Hi Phil,
> > > >
> > > > On Thu, 13 Feb 2025 20:15:06 +0000
> > > > Phil Elwell <phil@raspberrypi.com> wrote:
> > > >  
> > > > > Once more, with plain text, which I'd hoped the Android GMail client
> > > > > would work out for itself.
> > > > >
> > > > > On Thu, 13 Feb 2025, 18:53 Herve Codina, <herve.codina@bootlin.com> wrote:  
> > > > > >
> > > > > > Hi Phil,
> > > > > >
> > > > > > On Thu, 13 Feb 2025 17:57:37 +0000
> > > > > > Phil Elwell <phil@raspberrypi.com> wrote:
> > > > > >  
> > > > > > > On Thu, 13 Feb 2025 at 17:45, Andrew Lunn <andrew@lunn.ch> wrote:  
> > > > > > > >  
> > > > > > > > > > Or do you mean a custom board, which has a CPU, RP1 and the button and
> > > > > > > > > > fan are directly on this custom board? You then want a board DTS which
> > > > > > > > > > includes all these pieces?  
> > > > > > > > >
> > > > > > > > > That depends on whether you count the Raspberry Pi 5 as a custom board.  
> > > > > > > >
> > > > > > > > So you mean the Pi 5 board would itself make use of the resources the
> > > > > > > > RP1 device has? They are not simply connected to headers for plugin
> > > > > > > > boards, but used by the main board? Hence you want to describe them in
> > > > > > > > the board .DTS file.  
> > > > > > >
> > > > > > > That's correct. But even for plug-in devices, those which are on
> > > > > > > non-discoverable buses need overlays to declare them, which causes a
> > > > > > > problem when the overlay application happens before the kernel is
> > > > > > > started.
> > > > > > >  
> > > > > >
> > > > > > Hum, I see.
> > > > > >
> > > > > > We worked on overlay usage on non-discoverable buses wired to a connector
> > > > > > and we did a talk about issues we are facing on at Plumber [0].
> > > > > >
> > > > > > You can also find our big picture in [1] and a last contribution introducing
> > > > > > export-symbols feature in [2]. export-symbols is also under discussion on
> > > > > > some other threads.
> > > > > >
> > > > > > Also, we proposed the i2c bus extensions feature [3] whose goal is to allow
> > > > > > an addon board to add devices on an i2c bus provided by a base board and
> > > > > > wired to an connector the addon board is connected to.
> > > > > >
> > > > > > Maybe in your case, you can decouple resources (gpio, pwm) provided by the
> > > > > > addon board and used by the base board using also nexus node.
> > > > > >
> > > > > > We use a nexus node [4] (not presented at the Plumbers talk because the idea
> > > > > > came during 'out of talk' discussions in Plumbers) in order to allow our
> > > > > > addon board to use resources provided by the base board.
> > > > > >
> > > > > > In your case, if I understood, you are in the other direction but why not
> > > > > > using also a nexus node to decouple and translate resources in this other
> > > > > > direction ?
> > > > > >
> > > > > > Don't know if this idea can help but feel free to ask for some more
> > > > > > information if needed.  
> > > > >
> > > > > Nexus nodes look interesting - I see them as adding a layer of
> > > > > abstraction such that, for example, boards can declare which of their
> > > > > specific resources performs a common function so that clients can
> > > > > treat them all the same. We do the same thing in a limited way by
> > > > > using common labels on nodes, but this goes much further.
> > > > >
> > > > > In the case of Pi 5 and RP1, I imagine you are proposing that the Pi 5
> > > > > dtb declares the connector node and the overlay fills in the content
> > > > > with references to its GPIO controller, PWM controller etc. However, I
> > > > > think the overlay would also have to be board specific because it's
> > > > > not possible to patch part of a property from an overlay, so you'd end
> > > > > up overwriting the GPIO number as well as the controller reference.
> > > > >
> > > > > What is needed to make this work is the ability to cope with
> > > > > unresolved references in the base dtb, to be resolved as each overlay
> > > > > is applied, with runtime checking that each reference is resolved
> > > > > before it is used, all of which sounds like a nightmare. Plus, we
> > > > > really don't want to have to change the way all our camera and display
> > > > > overlays work on all Raspberry Pis just to accommodate somebody's idea
> > > > > of how RP1 should be handled.  
> > > >
> > > > Just to be clear, my comments were not there to tell you how RP1 should
> > > > work. I just proposed ideas without trying to force anything and I can
> > > > fully understand that ideas proposed don't feed your needs.
> > > >
> > > > Sorry if my approach was misunderstood.  
> > >
> > > I feel I've been misunderstood - I appreciate your ideas.
> > >
> > > Perhaps it would help if you could outline how you think we could
> > > apply your suggestions?
> > >  
> >
> > I was thinking about what your mentioned, i.e. the overlay fill the nexus node.
> > No sure to understand why the overlay should patch some properties.
> > Also where are the unresolved references in that case. The base DT refers to
> > the Nexus node.
> > The issue will probably be that the translation performed by the nexus node is
> > not available until the overlay is applied. The consumer will see errors other
> > than PROBE_DEFER when if probes while the overlay is not applied.  
> 
> The job of the nexus node would be to translate a generic request for
> a numbered resource to a specific request for an RP1 resource with
> arbitrary properties. The arbitrary properties could be GPIO offsets,
> which are board specific, while the node supplying the resource is
> provided by the overlay. This means that an entry in the table,
> described by a single property, could have contributions from the base
> DT and the overlay, which is not possible since overlays overwrite
> whole properties.

Hum, I am a bit lost.
Some DT example (base and overlay) could help me to understand.

> 
> Perhaps that particular problem could be overcome by creating a
> single-entry map, using the map-mask feature to pass through all of
> the GPIO offset and flags to the parent, so that the whole table
> becomes a proxy for RP1's GPIO controller. Is that what you had in
> mind?
> 
> > Also, the solution will lead to memory leak at runtime. Indeed, the overlay
> > add properties in an already existing node.
> > If the overlay is applied by the Kernel itself, this lead to memory leak when
> > the overlay is removed.
> > Indeed, an overlay can add/remove node without any issue but it cannot
> > add/remove properties to/from existing nodes.  
> 
> Fortunately for me I'm not arguing _for_ the use of an overlay.
> 
> > In the case described here, the nexus node is already present in the DT and the
> > overlay add/remove properties to/from this existing node.  
> 
> I think I can see how that could be made to work for GPIOs. It looks
> as though the GPIO subsystem is the only one making use of
> of_parse_phandle_with_args_map. Interrupts seem to have an open-coded
> equivalent, and iommus. What about I2C and PWM?

Support for PWM has been recently accepted.
  https://lore.kernel.org/all/ufl4kwrjyp4zid4muvghefevqc6hk3zyvxnsu72fxd4f46fzg6@hufkci2dzjid/

For i2c, nexus node is not suitable.

Nexus node works well when resources are indexed (gpio line in a gpio chip
for instance). For bus controller there is no index.
I mean we never refer a i2c bus controller using <&i2c-ctrl 12>.

For i2c, I proposed i2c bus extension:
  https://lore.kernel.org/all/20250205173918.600037-1-herve.codina@bootlin.com/

Best regards,
Hervé

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

On Mon, 17 Feb 2025 at 17:56, Herve Codina <herve.codina@bootlin.com> wrote:
>
> On Mon, 17 Feb 2025 17:03:34 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>

<snip>

> > The job of the nexus node would be to translate a generic request for
> > a numbered resource to a specific request for an RP1 resource with
> > arbitrary properties. The arbitrary properties could be GPIO offsets,
> > which are board specific, while the node supplying the resource is
> > provided by the overlay. This means that an entry in the table,
> > described by a single property, could have contributions from the base
> > DT and the overlay, which is not possible since overlays overwrite
> > whole properties.
>
> Hum, I am a bit lost.
> Some DT example (base and overlay) could help me to understand.

I could, but I think it is becoming a distraction.

<snip>

> > I think I can see how that could be made to work for GPIOs. It looks
> > as though the GPIO subsystem is the only one making use of
> > of_parse_phandle_with_args_map. Interrupts seem to have an open-coded
> > equivalent, and iommus. What about I2C and PWM?
>
> Support for PWM has been recently accepted.
>   https://lore.kernel.org/all/ufl4kwrjyp4zid4muvghefevqc6hk3zyvxnsu72fxd4f46fzg6@hufkci2dzjid/
>
> For i2c, nexus node is not suitable.
>
> Nexus node works well when resources are indexed (gpio line in a gpio chip
> for instance). For bus controller there is no index.
> I mean we never refer a i2c bus controller using <&i2c-ctrl 12>.
>
> For i2c, I proposed i2c bus extension:
>   https://lore.kernel.org/all/20250205173918.600037-1-herve.codina@bootlin.com/

I don't see in principle why an address-cells of 0 should cause a
problem - it's a degenerate case, but it's still conceptually valid.

However, we seem to be having to invent a lot of new infrastructure -
some of it already supported by the kernel, some of it not - because
we have made the mistake of using a discoverable bus for a
point-to-point link on a PCB. I don't see how this is fundamentally
different to the USB-attached Ethernet controller on e.g. the Pi 3B.
Despite the fact that the Ethernet interface is discoverable, it has a
Device Tree declaration (see
arch/arm/boot/dts/broadcom/bcm283x-rpi-smsc9514.dtsi) in order that
the firmware can supply a MAC address. Yes, the RP1 DT declaration is
significantly larger, but size shouldn't matter for what seems to be
objections based on tenets.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Hi Hervé,

On Thu, 13 Feb 2025 at 16:14, Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi Phil,
>
> On Thu, 13 Feb 2025 15:18:45 +0000
> Phil Elwell <phil@raspberrypi.com> wrote:
>
> > Hi Andrea,
> >
> > The problem with this approach (loading an overlay from the RP1 PCIe
> > driver), and it's one that I have raised with you offline, is that
> > (unless anyone can prove otherwise) it becomes impossible to create a
> > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > declare something as simple as a button wired to an RP1 GPIO, or fan
> > connected to a PWM output?
>
> The driver could be improved in a second step.
> For instance, it could load the dtbo from user-space using request_firmare()
> instead of loading the embedded dtbo.
>
> >
> > If this is the preferred route to upstream adoption, I would prefer it
> > if rp1.dtso could be split in two - an rp1.dtsi similar to what we
> > have downstream, and an rp1.dtso that #includes it. In this way we can
> > keep the patching and duplication to a minimum.
>
> Indeed, having a rp1.dtsi avoid duplication but how the rp1.dtso in
> the the kernel sources could include user customization (button, fan, ...)
> without being modified ?
> At least we have to '#include <my_rp1_customizations.dtsi>'.
>
> Requesting the dtbo from user-space allows to let the user to create
> its own dtso without the need to modify the one in kernel sources.
>
> Does it make sense ?

I think I understand what you are saying, but at this point the RP1
overlay would no longer be an RP1 overlay - it would be an
RP1-and-everything-connected-to-it overlay, which is inherently
board-specific. Which user-space process do you think would be
responsible for loading this alternative overlay, choosing carefully
based on the platform it is running on? Doesn't that place quite a
burden on all the OS maintainers who up to now have just needed a
kernel and a bunch of dtb files?

If it is considered essential that the upstream Pi 5 dts file does not
include RP1 and its children, then Raspberry Pi are going to have to
walk a different path until we've seen how that can work. By splitting
rp1.dtso as I suggested, and perhaps providing an alternative helper
function that only applies the built-in overlay if the device node
doesn't already exist, we get to stay as close to upstream as
possible.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrea della Porta]

Hi,

On 16:27 Thu 13 Feb     , Phil Elwell wrote:
> Hi Hervé,
> 
> On Thu, 13 Feb 2025 at 16:14, Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > Hi Phil,
> >
> > On Thu, 13 Feb 2025 15:18:45 +0000
> > Phil Elwell <phil@raspberrypi.com> wrote:
> >
> > > Hi Andrea,
> > >
> > > The problem with this approach (loading an overlay from the RP1 PCIe
> > > driver), and it's one that I have raised with you offline, is that
> > > (unless anyone can prove otherwise) it becomes impossible to create a
> > > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > > declare something as simple as a button wired to an RP1 GPIO, or fan
> > > connected to a PWM output?
> >
> > The driver could be improved in a second step.
> > For instance, it could load the dtbo from user-space using request_firmare()
> > instead of loading the embedded dtbo.
> >
> > >
> > > If this is the preferred route to upstream adoption, I would prefer it
> > > if rp1.dtso could be split in two - an rp1.dtsi similar to what we
> > > have downstream, and an rp1.dtso that #includes it. In this way we can
> > > keep the patching and duplication to a minimum.
> >
> > Indeed, having a rp1.dtsi avoid duplication but how the rp1.dtso in
> > the the kernel sources could include user customization (button, fan, ...)
> > without being modified ?
> > At least we have to '#include <my_rp1_customizations.dtsi>'.
> >
> > Requesting the dtbo from user-space allows to let the user to create
> > its own dtso without the need to modify the one in kernel sources.
> >
> > Does it make sense ?
> 
> I think I understand what you are saying, but at this point the RP1
> overlay would no longer be an RP1 overlay - it would be an
> RP1-and-everything-connected-to-it overlay, which is inherently
> board-specific. Which user-space process do you think would be
> responsible for loading this alternative overlay, choosing carefully
> based on the platform it is running on? Doesn't that place quite a
> burden on all the OS maintainers who up to now have just needed a
> kernel and a bunch of dtb files?
> 
> If it is considered essential that the upstream Pi 5 dts file does not
> include RP1 and its children, then Raspberry Pi are going to have to
> walk a different path until we've seen how that can work. By splitting
> rp1.dtso as I suggested, and perhaps providing an alternative helper
> function that only applies the built-in overlay if the device node
> doesn't already exist, we get to stay as close to upstream as
> possible.
> 
> Phil

So, the problem is twofold: the first is due to the fact that downstream
expects the dtb to be fully declared at fw load time (I'll call that
*monolithic* dtb from now on), the second is about how to represent dependencies
between board dtb and rp1 overlay which arises only when using overlays instead
of a monolithic dtb.

The former issue must be solved first in order for the latter to even exists
(if we don't use overlay, the dependencies are fully exposed in the dtb since
the beginning), so I'll concentrate on the former for now.

There are 3 possible scenarios to be reconciled:


1 - MONOLITHIC DTB

This is the downstream case, where it's advisable to have only one dtb blob
containing everything (rp1 included) loaded by the fw. In this case the
resulting devicetree would looks like:

  axi {
    pcie@120000 {
      rp1_nexus {
        pci-ep-bus@1 {
             ...
        }
      }
    }
  }


2 - RP1 LOADED FROM OVERLAY BY THE FW

In this case the rp1 dt node is loaded from overlay directly by the fw and the 
resulting devicetree is exactly equal to the monolithic dtb scenario.
In order for that overlay to be loaded by fw, just add 'dtoverlay=rp1' in
'config.txt'.


3 - RP1 LOADED FROM OVERLAY AT RUNTIME

Here it's the rp1 driver that loads the overlay at runtime, which is the case
that this patchset originally proposed. The devicetree ends up like this:

  axi {
    pcie@120000 {
      pci@0,0 {
        dev@0,0 {
          pci-ep-bus@1 {
               ...
          }
        }
      }
    }
  }

and this is exepcially useful to cope with the case in which there's no DT
natively used, e.g. on ACPI systems.


In order for all those 3 mentioned scenatios to work, I propose the following
inclusion scheme for for the dts files (the arrow points to the includer):
                   
 
 rp1-pci.dtso         rp1.dtso
     ^                    ^
     |                    |
rp1-common.dtsi ----> rp1-nexus.dtsi ----> bcm2712-rpi-5-b-MONOLITHIC.dts
   
   
where those dts are defined as follows (omitting the internal properties for
clarity sake):


- rp1-common.dtsi ------- // definition of core rp1 and its peripherals, common
			  // for all cases

	pci_ep_bus: pci-ep-bus@1 
	{
		rp1_clocks { };

		rp1_gpio { };

		rp1_eth { };
	};

- rp1-pci.dtso ---------- // ovl linked in the rp1 driver code to be loaded at
			  // runtime from rp1 driver. Only for case 3

	/plugin/;
	fragment@0 {
                target-path="";
                __overlay__ {
			#include "rp1-common.dtsi"
		};
	}

- rp1-nexus.dtsi ------- // adapter to decouple rp1 ranges for non runtime-loaded
		         // overlay case (i.e. only for case 1 and 2)

	rp1_nexus {
		ranges = ...
		
		 #include "rp1-common.dtsi"
	};

- rp1.dtso ------------ // overlay to be loaded by fw (case 2)

	/plugin/;
	&pcie2 {
		#include "rp1-nexus.dtsi"
	};

- bcm2712-rpi-5-b-MONOLITHIC.dts --- // monolithic dtb to avoid any overlay use
				     // (case 1)

	/ {
		... all rpi5 board dts ...
		&pcie2 {
        		#include "rp1-nexus.dtsi"
		};
	};


with only minimal changes to the rp1 driver code, I can confirm that all those
scenarios can coexits and are working fine. Before processding with a new patchset
I'd like to have some thoughts about that, do you think this is a viable approach?

Many thanks,
Andrea

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Phil Elwell]

Hi Andrea,

On Mon, 10 Mar 2025 at 13:58, Andrea della Porta <andrea.porta@suse.com> wrote:
>
> Hi,
>
> On 16:27 Thu 13 Feb     , Phil Elwell wrote:
> > Hi Hervé,
> >
> > On Thu, 13 Feb 2025 at 16:14, Herve Codina <herve.codina@bootlin.com> wrote:
> > >
> > > Hi Phil,
> > >
> > > On Thu, 13 Feb 2025 15:18:45 +0000
> > > Phil Elwell <phil@raspberrypi.com> wrote:
> > >
> > > > Hi Andrea,
> > > >
> > > > The problem with this approach (loading an overlay from the RP1 PCIe
> > > > driver), and it's one that I have raised with you offline, is that
> > > > (unless anyone can prove otherwise) it becomes impossible to create a
> > > > Pi 5 DTS file which makes use of the RP1's resources. How do you
> > > > declare something as simple as a button wired to an RP1 GPIO, or fan
> > > > connected to a PWM output?
> > >
> > > The driver could be improved in a second step.
> > > For instance, it could load the dtbo from user-space using request_firmare()
> > > instead of loading the embedded dtbo.
> > >
> > > >
> > > > If this is the preferred route to upstream adoption, I would prefer it
> > > > if rp1.dtso could be split in two - an rp1.dtsi similar to what we
> > > > have downstream, and an rp1.dtso that #includes it. In this way we can
> > > > keep the patching and duplication to a minimum.
> > >
> > > Indeed, having a rp1.dtsi avoid duplication but how the rp1.dtso in
> > > the the kernel sources could include user customization (button, fan, ...)
> > > without being modified ?
> > > At least we have to '#include <my_rp1_customizations.dtsi>'.
> > >
> > > Requesting the dtbo from user-space allows to let the user to create
> > > its own dtso without the need to modify the one in kernel sources.
> > >
> > > Does it make sense ?
> >
> > I think I understand what you are saying, but at this point the RP1
> > overlay would no longer be an RP1 overlay - it would be an
> > RP1-and-everything-connected-to-it overlay, which is inherently
> > board-specific. Which user-space process do you think would be
> > responsible for loading this alternative overlay, choosing carefully
> > based on the platform it is running on? Doesn't that place quite a
> > burden on all the OS maintainers who up to now have just needed a
> > kernel and a bunch of dtb files?
> >
> > If it is considered essential that the upstream Pi 5 dts file does not
> > include RP1 and its children, then Raspberry Pi are going to have to
> > walk a different path until we've seen how that can work. By splitting
> > rp1.dtso as I suggested, and perhaps providing an alternative helper
> > function that only applies the built-in overlay if the device node
> > doesn't already exist, we get to stay as close to upstream as
> > possible.
> >
> > Phil
>
> So, the problem is twofold: the first is due to the fact that downstream
> expects the dtb to be fully declared at fw load time (I'll call that
> *monolithic* dtb from now on), the second is about how to represent dependencies
> between board dtb and rp1 overlay which arises only when using overlays instead
> of a monolithic dtb.
>
> The former issue must be solved first in order for the latter to even exists
> (if we don't use overlay, the dependencies are fully exposed in the dtb since
> the beginning), so I'll concentrate on the former for now.
>
> There are 3 possible scenarios to be reconciled:
>
>
> 1 - MONOLITHIC DTB
>
> This is the downstream case, where it's advisable to have only one dtb blob
> containing everything (rp1 included) loaded by the fw. In this case the
> resulting devicetree would looks like:
>
>   axi {
>     pcie@120000 {
>       rp1_nexus {
>         pci-ep-bus@1 {
>              ...
>         }
>       }
>     }
>   }

This is indeed our current DT usage for the Pi 5 family - a single DTB
describing each board - although I would like to make it clear that
overlays can then be applied on top of it describing the attached
peripherals.

> 2 - RP1 LOADED FROM OVERLAY BY THE FW
>
> In this case the rp1 dt node is loaded from overlay directly by the fw and the
> resulting devicetree is exactly equal to the monolithic dtb scenario.
> In order for that overlay to be loaded by fw, just add 'dtoverlay=rp1' in
> 'config.txt'.

This halfway house combines the advantages and disadvantages of
scenarios 1 and 3. Compared to scenario 3 it gains support for
applying overlays that make use of interfaces provided by RP1 - I2C,
SPI, UARTs etc. Compared to scenario 1 it loses the ability for the
base DTB to refer to elements of RP1, other than by replacing these
DTB elements with overlays that must be loaded after the RP1 overlay.
As such, we would see that as a backwards step and not use it.

> 3 - RP1 LOADED FROM OVERLAY AT RUNTIME
>
> Here it's the rp1 driver that loads the overlay at runtime, which is the case
> that this patchset originally proposed. The devicetree ends up like this:
>
>   axi {
>     pcie@120000 {
>       pci@0,0 {
>         dev@0,0 {
>           pci-ep-bus@1 {
>                ...
>           }
>         }
>       }
>     }
>   }
>
> and this is exepcially useful to cope with the case in which there's no DT
> natively used, e.g. on ACPI systems.
>
>
> In order for all those 3 mentioned scenatios to work, I propose the following
> inclusion scheme for for the dts files (the arrow points to the includer):
>
>
>  rp1-pci.dtso         rp1.dtso
>      ^                    ^
>      |                    |
> rp1-common.dtsi ----> rp1-nexus.dtsi ----> bcm2712-rpi-5-b-MONOLITHIC.dts
>
>
> where those dts are defined as follows (omitting the internal properties for
> clarity sake):
>
>
> - rp1-common.dtsi ------- // definition of core rp1 and its peripherals, common
>                           // for all cases
>
>         pci_ep_bus: pci-ep-bus@1
>         {
>                 rp1_clocks { };
>
>                 rp1_gpio { };
>
>                 rp1_eth { };
>         };
>
> - rp1-pci.dtso ---------- // ovl linked in the rp1 driver code to be loaded at
>                           // runtime from rp1 driver. Only for case 3
>
>         /plugin/;
>         fragment@0 {
>                 target-path="";
>                 __overlay__ {
>                         #include "rp1-common.dtsi"
>                 };
>         }
>
> - rp1-nexus.dtsi ------- // adapter to decouple rp1 ranges for non runtime-loaded
>                          // overlay case (i.e. only for case 1 and 2)
>
>         rp1_nexus {
>                 ranges = ...
>
>                  #include "rp1-common.dtsi"
>         };
>
> - rp1.dtso ------------ // overlay to be loaded by fw (case 2)
>
>         /plugin/;
>         &pcie2 {
>                 #include "rp1-nexus.dtsi"
>         };
>
> - bcm2712-rpi-5-b-MONOLITHIC.dts --- // monolithic dtb to avoid any overlay use
>                                      // (case 1)
>
>         / {
>                 ... all rpi5 board dts ...
>                 &pcie2 {
>                         #include "rp1-nexus.dtsi"
>                 };
>         };
>
>
> with only minimal changes to the rp1 driver code, I can confirm that all those
> scenarios can coexits and are working fine. Before processding with a new patchset
> I'd like to have some thoughts about that, do you think this is a viable approach?

Thank you for this - the creation of a core RP1 declaration that can
be tailored to multiple applications using different wrappers is
exactly what I had in mind. I agree that your partitioning scheme can
cater for the 3 usage scenarios, but for the reasons outlined above I
think scenario 2 is not useful to us, although it isn't impossible
that U-boot might see things differently; I see no harm in having it
supported, but I wouldn't want anyone to go to unnecessary effort to
make it work.

Phil

Re: [PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrea della Porta]

Hi Phil,

On 14:21 Mon 10 Mar     , Phil Elwell wrote:
> Hi Andrea,
> 

...

> 
> This is indeed our current DT usage for the Pi 5 family - a single DTB
> describing each board - although I would like to make it clear that
> overlays can then be applied on top of it describing the attached
> peripherals.

Indeed. Thanks for specifying that.

> 
> > 2 - RP1 LOADED FROM OVERLAY BY THE FW
> >
> > In this case the rp1 dt node is loaded from overlay directly by the fw and the
> > resulting devicetree is exactly equal to the monolithic dtb scenario.
> > In order for that overlay to be loaded by fw, just add 'dtoverlay=rp1' in
> > 'config.txt'.
> 
> This halfway house combines the advantages and disadvantages of
> scenarios 1 and 3. Compared to scenario 3 it gains support for
> applying overlays that make use of interfaces provided by RP1 - I2C,
> SPI, UARTs etc. Compared to scenario 1 it loses the ability for the
> base DTB to refer to elements of RP1, other than by replacing these
> DTB elements with overlays that must be loaded after the RP1 overlay.
> As such, we would see that as a backwards step and not use it.

This is in fact the second side of the problem I was mentioning: being
able to reference RP1 nodes from base tree (this of course arise only for
case 2 and 3, i.e. in case overlays are used). Some solutions to this have
been added by Herve (nexus nodes) and it seems promising, but I still
don't have a complete and tested solution.
My intention here is to verify if it is worth spending time on figuring
out a solution for the overlay case, since in case we only accept the
monolithic approach we won't use overlay for rp1 at all.

My idea is that we can still provide the dtbs for all three cases for the
user to choose from (so to potentially benefit from all advantages every
single solution provides), and refine the overlay case as we go, since we
can still fallback to the monolithic dtb in case we can't find a viable
solution to the across-overlay referencing issue.  But I'd like a confirm
from upstream too.
I think that this problem should really be addressed sooner or later (I'm
hinting at fpga hw, but lately we're seeing other appliances of this paradigm
arising more often), so maybe this is a good opportunity to lay the ground
for a proper solution that could be generalized for other situations too, but
again, not at the cost of slowing down rpi5 upstream efforts.

> 
> > 3 - RP1 LOADED FROM OVERLAY AT RUNTIME
> >
> > Here it's the rp1 driver that loads the overlay at runtime, which is the case
> > that this patchset originally proposed. The devicetree ends up like this:
> >
> >   axi {
> >     pcie@120000 {
> >       pci@0,0 {
> >         dev@0,0 {

...

> >         / {
> >                 ... all rpi5 board dts ...
> >                 &pcie2 {
> >                         #include "rp1-nexus.dtsi"
> >                 };
> >         };
> >
> >
> > with only minimal changes to the rp1 driver code, I can confirm that all those
> > scenarios can coexits and are working fine. Before processding with a new patchset
> > I'd like to have some thoughts about that, do you think this is a viable approach?
> 
> Thank you for this - the creation of a core RP1 declaration that can
> be tailored to multiple applications using different wrappers is
> exactly what I had in mind. I agree that your partitioning scheme can
> cater for the 3 usage scenarios, but for the reasons outlined above I
> think scenario 2 is not useful to us, although it isn't impossible
> that U-boot might see things differently; I see no harm in having it
> supported, but I wouldn't want anyone to go to unnecessary effort to
> make it work.

In all truth, this is in fact exactly your proposal (we discussed internally with
Dave also), just slightly re-arranged by me.
I agree with you about not wasting time, as I also mentioned above.

Many thanks,
Andrea

> 
> Phil

[PATCH v6 00/10] Add support for RaspberryPi RP1 PCI device using a DT overlay

[Andrea della Porta]

RP1 is an MFD chipset that acts as a south-bridge PCIe endpoint sporting
a pletora of subdevices (i.e.  Ethernet, USB host controller, I2C, PWM,
etc.) whose registers are all reachable starting from an offset from the
BAR address.  The main point here is that while the RP1 as an endpoint
itself is discoverable via usual PCI enumeraiton, the devices it contains
are not discoverable and must be declared e.g. via the devicetree.

This patchset is an attempt to provide a minimum infrastructure to allow
the RP1 chipset to be discovered and perpherals it contains to be added
from a devictree overlay loaded during RP1 PCI endpoint enumeration.
Followup patches should add support for the several peripherals contained
in RP1.

This work is based upon dowstream drivers code and the proposal from RH
et al. (see [1] and [2]). A similar approach is also pursued in [3].

The patches are ordered as follows:

-PATCHES 1 to 4: add binding schemas for clock, gpio and RP1 peripherals.
 They are needed to support the other peripherals, e.g. the ethernet mac
 depends on a clock generated by RP1 and the phy is reset though the
 on-board gpio controller.

-PATCH 5 and 6: add clock and gpio device drivers.

-PATCH 7: the devicetree overlay describing the RP1 chipset. Please
 note that this patch should be taken by the same maintainer that will
 also take patch 11, since txeieh dtso is compiled in as binary blob and is
 closely coupled to the driver.

-PATCH 8: this is the main patch to support RP1 chipset and peripherals
 enabling through dtb overlay. The dtso since is intimately coupled with
 the driver and will be linked in as binary blob in the driver obj.
 The real dtso is in devicetree folder while the dtso in driver folder is
 just a placeholder to include the real dtso.
 In this way it is possible to check the dtso against dt-bindings.
 The reason why drivers/misc has been selected as containing folder
 for this driver can be seen in [6], [7] and [8].

-PATCH 9: add the external clock node (used by RP1) to the main dts.

-PATCH 10: add the relevant kernel CONFIG_ options to defconfig.

This patchset is also a first attempt to be more agnostic wrt hardware
description standards such as OF devicetree and ACPI, where 'agnostic'
means "using DT in coexistence with ACPI", as been already promoted
by e.g. AL (see [4]). Although there's currently no evidence it will also
run out of the box on purely ACPI system, it is a first step towards
that direction.

Please note that albeit this patchset has no prerequisites in order to
be applied cleanly, it still depends on Stanimir's WIP patchset for BCM2712
PCIe controller (see [5]) in order to work at runtime.

Many thanks,
Andrea della Porta

Links:
- [1]: https://lpc.events/event/17/contributions/1421/attachments/1337/2680/LPC2023%20Non-discoverable%20devices%20in%20PCI.pdf
- [2]: https://lore.kernel.org/lkml/20230419231155.GA899497-robh@kernel.org/t/
- [3]: https://lore.kernel.org/all/20240808154658.247873-1-herve.codina@bootlin.com/#t
- [4]: https://lore.kernel.org/all/73e05c77-6d53-4aae-95ac-415456ff0ae4@lunn.ch/
- [5]: https://lore.kernel.org/all/20240626104544.14233-1-svarbanov@suse.de/
- [6]: https://lore.kernel.org/all/20240612140208.GC1504919@google.com/
- [7]: https://lore.kernel.org/all/83f7fa09-d0e6-4f36-a27d-cee08979be2a@app.fastmail.com/
- [8]: https://lore.kernel.org/all/2024081356-mutable-everyday-6f9d@gregkh/

CHANGES IN V6:

PATCH RELATED -------------------------------------------------

- patch 2: added: Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
- patch 3 and 4: added: 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'
- patch 8: removed a stale paragraph from git commit message regarding
  gpio renaming through configfs


RP1 MISC DRIVER -----------------------------------

- interrupts definitions moved as documentation from rp1_pci.c to
  pci1de4,1.yaml binding schema


Andrea della Porta (10):
  dt-bindings: clock: Add RaspberryPi RP1 clock bindings
  dt-bindings: pinctrl: Add RaspberryPi RP1 gpio/pinctrl/pinmux bindings
  dt-bindings: pci: Add common schema for devices accessible through PCI
    BARs
  dt-bindings: misc: Add device specific bindings for RaspberryPi RP1
  clk: rp1: Add support for clocks provided by RP1
  pinctrl: rp1: Implement RaspberryPi RP1 gpio support
  arm64: dts: rp1: Add support for RaspberryPi's RP1 device
  misc: rp1: RaspberryPi RP1 misc driver
  arm64: dts: bcm2712: Add external clock for RP1 chipset on Rpi5
  arm64: defconfig: Enable RP1 misc/clock/gpio drivers

 .../clock/raspberrypi,rp1-clocks.yaml         |   58 +
 .../devicetree/bindings/misc/pci1de4,1.yaml   |  135 ++
 .../devicetree/bindings/pci/pci-ep-bus.yaml   |   58 +
 .../pinctrl/raspberrypi,rp1-gpio.yaml         |  198 +++
 MAINTAINERS                                   |   14 +
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |    7 +
 arch/arm64/boot/dts/broadcom/rp1.dtso         |   58 +
 arch/arm64/configs/defconfig                  |    3 +
 drivers/clk/Kconfig                           |    9 +
 drivers/clk/Makefile                          |    1 +
 drivers/clk/clk-rp1.c                         | 1527 +++++++++++++++++
 drivers/misc/Kconfig                          |    1 +
 drivers/misc/Makefile                         |    1 +
 drivers/misc/rp1/Kconfig                      |   21 +
 drivers/misc/rp1/Makefile                     |    3 +
 drivers/misc/rp1/rp1-pci.dtso                 |    8 +
 drivers/misc/rp1/rp1_pci.c                    |  305 ++++
 drivers/misc/rp1/rp1_pci.h                    |   14 +
 drivers/pci/quirks.c                          |    1 +
 drivers/pinctrl/Kconfig                       |   11 +
 drivers/pinctrl/Makefile                      |    1 +
 drivers/pinctrl/pinctrl-rp1.c                 |  789 +++++++++
 .../clock/raspberrypi,rp1-clocks.h            |   61 +
 include/linux/pci_ids.h                       |    3 +
 24 files changed, 3287 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
 create mode 100644 Documentation/devicetree/bindings/misc/pci1de4,1.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/pci-ep-bus.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
 create mode 100644 arch/arm64/boot/dts/broadcom/rp1.dtso
 create mode 100644 drivers/clk/clk-rp1.c
 create mode 100644 drivers/misc/rp1/Kconfig
 create mode 100644 drivers/misc/rp1/Makefile
 create mode 100644 drivers/misc/rp1/rp1-pci.dtso
 create mode 100644 drivers/misc/rp1/rp1_pci.c
 create mode 100644 drivers/misc/rp1/rp1_pci.h
 create mode 100644 drivers/pinctrl/pinctrl-rp1.c
 create mode 100644 include/dt-bindings/clock/raspberrypi,rp1-clocks.h

-- 
2.35.3