devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Raspberry Pi5 - RP1 driver - RFC
@ 2024-06-11 15:39 Andrea della Porta
  2024-06-11 19:05 ` Stefan Wahren
  2024-06-12 14:02 ` Lee Jones
  0 siblings, 2 replies; 5+ messages in thread
From: Andrea della Porta @ 2024-06-11 15:39 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Broadcom internal kernel review list,
	Stefan Wahren, devicetree, linux-rpi-kernel, linux-arm-kernel,
	linux-kernel, Bjorn Helgaas, linux-pci, Dave Ertman, Lizhi Hou,
	clement.leger

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/20230419231155.GA899497-robh@kernel.org/t/
- [5]: https://lore.kernel.org/lkml/Y862WTT03%2FJxXUG8@kroah.com/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Raspberry Pi5 - RP1 driver - RFC
  2024-06-11 15:39 Raspberry Pi5 - RP1 driver - RFC Andrea della Porta
@ 2024-06-11 19:05 ` Stefan Wahren
  2024-06-11 20:12   ` Andrew Lunn
  2024-06-12 16:10   ` Jeremy Linton
  2024-06-12 14:02 ` Lee Jones
  1 sibling, 2 replies; 5+ messages in thread
From: Stefan Wahren @ 2024-06-11 19:05 UTC (permalink / raw)
  To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Broadcom internal kernel review list,
	devicetree, linux-rpi-kernel, linux-arm-kernel, linux-kernel,
	Bjorn Helgaas, linux-pci, Dave Ertman, Lizhi Hou, clement.leger,
	Jeremy Linton

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/20230419231155.GA899497-robh@kernel.org/t/
> - [5]: https://lore.kernel.org/lkml/Y862WTT03%2FJxXUG8@kroah.com/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Raspberry Pi5 - RP1 driver - RFC
  2024-06-11 19:05 ` Stefan Wahren
@ 2024-06-11 20:12   ` Andrew Lunn
  2024-06-12 16:10   ` Jeremy Linton
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2024-06-11 20:12 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, Broadcom internal kernel review list,
	devicetree, linux-rpi-kernel, linux-arm-kernel, linux-kernel,
	Bjorn Helgaas, linux-pci, Dave Ertman, Lizhi Hou, clement.leger,
	Jeremy Linton

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/20240527161450.326615-1-herve.codina@bootlin.com/

ACPI and DT should not be considered mutually exclusive.

     Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Raspberry Pi5 - RP1 driver - RFC
  2024-06-11 15:39 Raspberry Pi5 - RP1 driver - RFC Andrea della Porta
  2024-06-11 19:05 ` Stefan Wahren
@ 2024-06-12 14:02 ` Lee Jones
  1 sibling, 0 replies; 5+ messages in thread
From: Lee Jones @ 2024-06-12 14:02 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Broadcom internal kernel review list, Stefan Wahren, devicetree,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, Bjorn Helgaas,
	linux-pci, Dave Ertman, Lizhi Hou, clement.leger

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 [李琼斯]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Raspberry Pi5 - RP1 driver - RFC
  2024-06-11 19:05 ` Stefan Wahren
  2024-06-11 20:12   ` Andrew Lunn
@ 2024-06-12 16:10   ` Jeremy Linton
  1 sibling, 0 replies; 5+ messages in thread
From: Jeremy Linton @ 2024-06-12 16:10 UTC (permalink / raw)
  To: Stefan Wahren, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli,
	Broadcom internal kernel review list, devicetree,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, Bjorn Helgaas,
	linux-pci, Dave Ertman, Lizhi Hou, clement.leger

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/20230419231155.GA899497-robh@kernel.org/t/
>> - [5]: https://lore.kernel.org/lkml/Y862WTT03%2FJxXUG8@kroah.com/
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-12 16:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-11 15:39 Raspberry Pi5 - RP1 driver - RFC Andrea della Porta
2024-06-11 19:05 ` Stefan Wahren
2024-06-11 20:12   ` Andrew Lunn
2024-06-12 16:10   ` Jeremy Linton
2024-06-12 14:02 ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).