Hi,
I'm on the verge of reworking the RP1 driver from downstream in order for it to be
in good shape for upstream inclusion.
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 is an RFC about the correct approach to use
in integrating the driver and registering the subdevices.
--- CURRENT DOWNSTREAM APPROACH ---
The DTS shows something like this (see [1] and [2]):
pcie {
compatible = "brcm,bcm2712-pcie";
#address-cells = <0x03>;
#size-cells = <0x02>;
ranges = <0x2000000 0x00 0x00 0x1f 0x00 0x00 0xfffffffc>;
...
rp1 {
compatible = "simple-bus";
#address-cells = <0x02>;
#size-cells = <0x02>;
ranges = <0xc0 0x40000000 0x2000000 0x00 0x00 0x00 0x400000>;
...
serial@34000 {
compatible = "arm,pl011-axi";
reg = <0xc0 0x40034000 0x00 0x100>;
...
};
};
};
The PCI bar address here is at CPU physical address 0x1f00000000 and the RP1 driver
probe function calls of_platform_populate() on the 'rp1' node to register the platform
drivers for each subdevices (e.g. in the above example: 'serial@34000').
Pros:
- quite straightforward to implement
- RP1's dts resides in the directory it should (possibly but not necessarily) belong to,
e.g. somewhere under arch/*/boot/dts/...
Cons:
- the board dts must manually override 'pcie' ranges (in this case 0x1f00000000)
depending on the BAR address value, while it should be retrieved by reading the PCI
config register instead
- the probe() function retieves a reference to 'rp1' node via of_find_node_by_name(NULL,
"rp1"), harcoding the node name. This is not desirable since the node name is then set
in stone, or otherwise if the node name needs to be changed it must be changed either
in the dts *and* in driver code.
--- ROB HERRING, LIZHI HOU (et al.) PROPOSED APPROACH ---
A proposal (see [3]) presented at LPC advise to create a PCI bridge DT node ('pcie') leveraging
the current OF dynamic infrastructure, adding a dtb overlay ('rpi1' node in the example
above) on top of it during probe and rearranging the 'ranges' mapping dynamically.
This sounds like the correct approach and is somewhat used in at least a couple of drivers
(namely for Alveo U50 card and MicroChip LAN9662 SoC) but AFAIK none of them made their way
to mainline, leaving some doubts about the applicability of this paradigm.
Pros:
- no need to provide the BAR address manually since it will be discovered and automatically
amended into the 'ranges' property
- no harcoded reference to 'rp1' node since the endpoint node will be reparented to the 'pcie'
node automatically
- the core OF dynamic infrastructure on which to base these changes are basically already in
mainline (see [4] for discussions)
Cons (albeit I'd consider them minor ones):
- CONFIG_PCI_DYNAMIC_OF_NODES must be enabled
- the dtb should probably reside somewhere near the driver source code, e.g. in drivers/mfd/...
--- AUXILIARY BUS APPROACH ---
The thread in [5] seems to advise to use the auxiliary bus for this kind of devices. In this
case, here's the drawbacks:
- as stated in kernel docs for aux bus (cit.) "need a mechanism to connect and provide access
to a shared object allocated by the auxiliary_device’s registering driver...", and again
(cit.) "A key requirement for utilizing the auxiliary bus is that there is no dependency on
a physical bus...These individual devices split from the core cannot live on the platform
bus as they are not physical devices that are controlled by DT/ACPI". Those statements are
of course at the opposite of how RP1 behaves
- subdevices drivers may need rework in order to cope with the auxiliary bus, while we need to
use the already existing drivers without modifications
so, for all of the above (and probably other cons I'm not aware of right now), the auxiliary bus
does not seems feasible to be used, but I'm mentioning it just because of the discussionin in [5]
that let me wonder whether I may be missing something relevant here.
CONCLUSIONS
All in all, I'd say Rob's approach should be the way to go, any thoughts about it will be
greatly appreciated.
Many thanks,
Andrea della Porta
Link:
- [1]: https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/broadcom/rp1.dtsi
- [2]: https://github.com/raspberrypi/linux/blob/rpi-6.6.y/drivers/mfd/rp1.c
- [3]: https://lpc.events/event/17/contributions/1421/attachments/1337/2680/LPC2023%20Non-discoverable%20devices%20in%20PCI.pdf
- [4]: https://lore.kernel.org/lkml/[email protected]/t/
- [5]: https://lore.kernel.org/lkml/Y862WTT03%[email protected]/
Hi Andrea,
i added Jeremy, because AFAIK he was deeply involved in ACPI
implementation of the RPi 4.
Am 11.06.24 um 17:39 schrieb Andrea della Porta:
> Hi,
> I'm on the verge of reworking the RP1 driver from downstream in order for it to be
> in good shape for upstream inclusion.
> 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 is an RFC about the correct approach to use
> in integrating the driver and registering the subdevices.
>
I cannot provide much input into the technical discussion, but i would
prefer an approach which works good with DT and ACPI.
Best regards
Stefan
>
> Link:
> - [1]: https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/broadcom/rp1.dtsi
> - [2]: https://github.com/raspberrypi/linux/blob/rpi-6.6.y/drivers/mfd/rp1.c
> - [3]: https://lpc.events/event/17/contributions/1421/attachments/1337/2680/LPC2023%20Non-discoverable%20devices%20in%20PCI.pdf
> - [4]: https://lore.kernel.org/lkml/[email protected]/t/
> - [5]: https://lore.kernel.org/lkml/Y862WTT03%[email protected]/
On Tue, Jun 11, 2024 at 09:05:24PM +0200, Stefan Wahren wrote:
> Hi Andrea,
>
> i added Jeremy, because AFAIK he was deeply involved in ACPI
> implementation of the RPi 4.
>
> Am 11.06.24 um 17:39 schrieb Andrea della Porta:
> > Hi,
> > I'm on the verge of reworking the RP1 driver from downstream in order for it to be
> > in good shape for upstream inclusion.
> > 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 is an RFC about the correct approach to use
> > in integrating the driver and registering the subdevices.
> >
> I cannot provide much input into the technical discussion, but i would
> prefer an approach which works good with DT and ACPI.
There is a small and slowly growing interest in using DT overlays on
ACPI systems. It makes a lot of sense when you have an already working
set of drivers based on DT, and then need to make them work on ACPI
systems.
The Microchip LAN996x is an ARM SoC with lots of peripherals and an
Ethernet switch. There is a full DT description of it and
drivers. However, it also has a PCIe interface which allows access to
all the peripherals and the Ethernet switch. Bootlin are adding
patches to allow any host with a PCIe bus use all the existing drivers
and a DT overlay to glue it all together.
https://patchwork.kernel.org/project/linux-pci/cover/[email protected]/
ACPI and DT should not be considered mutually exclusive.
Andrew
On Tue, 11 Jun 2024, Andrea della Porta wrote:
> Hi,
> I'm on the verge of reworking the RP1 driver from downstream in order for it to be
> in good shape for upstream inclusion.
> 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.
It's less of an MFD and more of an SoC.
Please refrain from implemented entire SoCs in drivers/mfd.
Take a look in drivers/soc and drivers/platform.
--
Lee Jones [李琼斯]
Hi,
On 6/11/24 14:05, Stefan Wahren wrote:
> Hi Andrea,
>
> i added Jeremy, because AFAIK he was deeply involved in ACPI
> implementation of the RPi 4.
I'm not sure what to add here, the RPi4 work was done as an example of
using firmware standards to boot multiple OSs with a single
boot/firmware interface. Which means ACPI.
Alternatively, the PCIe/SMCCC might be able to make this device look
more regular, by putting everything on separate PCIe functions.
OTOH, I don't think this device is particularly special, except maybe to
the extent that it doubles down on ideas regarded as best left in the
1990's. The kernel documents how to handle these cases with ACPI _ADR().
A PCI device can create the _ADR nodes by injecting an SSDT into the
ACPI namespace via the PCI option ROMs if the platform firmware doesn't
provide them. Should it though? If I were doing it I might be tempted to
configure the root port in early firmware and hide it from the OS,
claiming instead a bunch of platform devices.
IMHO, DT/Linux platforms should probably do something similar to _ADR()
for consistency rather than requiring the EP driver to get involved.
Further, mixing DT's into a possible ACPI platform is really the worst
of both. Even worse if it requires further distro
dracut/initrd/grub/etc one off hacking or polluting the initrd or ESP of
non RPi platforms to handle the overlay.
So, a custom EP/bus driver option solves the problem on linux for both
DT and ACPI implementations if the device type/offsets are hard coded
into it. And presumably if there is a follow on device, it would use
multiple PCIe functions to avoid all these problems, the ones around
securing the platform with an IOMMU, enabling VFIO, and everything else
one gets for "free" with a proper PCIe EP.
PS:
The PCIe/SMCC API could probably make all these devices appear as PCIe
functions avoiding the need for a monolithic bus or DT/ACPI description
to handle it. But that will likely break the second this device is
plugged into something with an SMMU (this platform doesn't have one,
correct?), and of course if would require all the firmware configured
BAR mappings to remain static, which isn't a problem if its presented as
an integrated endpoint. If someone is interested in doing it that way
then we should talk.
>
> Am 11.06.24 um 17:39 schrieb Andrea della Porta:
>> Hi,
>> I'm on the verge of reworking the RP1 driver from downstream in order
>> for it to be
>> in good shape for upstream inclusion.
>> 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 is an RFC about the correct
>> approach to use
>> in integrating the driver and registering the subdevices.
>>
> I cannot provide much input into the technical discussion, but i would
> prefer an approach which works good with DT and ACPI.
>
> Best regards
> Stefan
>>
>> Link:
>> - [1]:
>> https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/broadcom/rp1.dtsi
>> - [2]:
>> https://github.com/raspberrypi/linux/blob/rpi-6.6.y/drivers/mfd/rp1.c
>> - [3]:
>> https://lpc.events/event/17/contributions/1421/attachments/1337/2680/LPC2023%20Non-discoverable%20devices%20in%20PCI.pdf
>> - [4]:
>> https://lore.kernel.org/lkml/[email protected]/t/
>> - [5]: https://lore.kernel.org/lkml/Y862WTT03%[email protected]/
>