Hi Andrea,
On Mon, 10 Mar 2025 at 13:58, Andrea della Porta <andrea.porta@xxxxxxxx> 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@xxxxxxxxxxx> wrote:
> > >
> > > Hi Phil,
> > >
> > > On Thu, 13 Feb 2025 15:18:45 +0000
> > > Phil Elwell <phil@xxxxxxxxxxxxxxx> 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
