On Fri, Aug 23, 2024 at 11:31 AM Andrea della Porta
[off-list ref] wrote:

Hi Stefan,

On 12:23 Fri 23 Aug     , Stefan Wahren wrote:

quoted

Hi Andrea,

Am 23.08.24 um 11:44 schrieb Andrea della Porta:

quoted

Hi Stefan,

On 18:20 Wed 21 Aug     , Stefan Wahren wrote:

quoted

Hi Andrea,

Am 20.08.24 um 16:36 schrieb Andrea della Porta:

quoted

The RaspberryPi RP1 is ia PCI multi function device containing
peripherals ranging from Ethernet to USB controller, I2C, SPI
and others.

sorry, i cannot provide you a code review, but just some comments. multi
function device suggests "mfd" subsystem or at least "soc" . I won't
recommend misc driver here.

It's true that RP1 can be called an MFD but the reason for not placing
it in mfd subsystem are twofold:

- these discussions are quite clear about this matter: please see [1]
   and [2]
- the current driver use no mfd API at all

This RP1 driver is not currently addressing any aspect of ARM core in the
SoC so I would say it should not stay in drivers/soc / either, as also
condifirmed by [2] again and [3] (note that Microchip LAN966x is a very
close fit to what we have here on RP1).

thanks i was aware of these discussions. A pointer to them or at least a
short statement in the cover letter would be great.

Sure, consider it done.

quoted

quoted

quoted

quoted

Implement a bare minimum driver to operate the RP1, leveraging
actual OF based driver implementations for the on-borad peripherals
by loading a devicetree overlay during driver probe.

Can you please explain why this should be a DT overlay? The RP1 is
assembled on the Raspberry Pi 5 PCB. DT overlays are typically for loose
connections like displays or HATs. I think a DTSI just for the RP1 would
fit better and is easier to read.

The dtsi solution you proposed is the one adopted downstream. It has its
benefits of course, but there's more.
With the overlay approach we can achieve more generic and agnostic approach
to managing this chipset, being that it is a PCI endpoint and could be
possibly be reused in other hw implementations. I believe a similar
reasoning could be applied to Bootlin's Microchip LAN966x patchset as
well, and they also choose to approach the dtb overlay.

Could please add this point in the commit message. Doesn't introduce

Ack.

quoted

(maintainence) issues in case U-Boot needs a RP1 driver, too?

Good point. Right now u-boot does not support RP1 nor PCIe (which is a
prerequisite for RP1 to work) on Rpi5 and I'm quite sure that it will be
so in the near future. Of course I cannot guarantee this will be the case
far away in time.

Since u-boot is lacking support for RP1 we cannot really produce some test
results to check the compatibility versus kernel dtb overlay but we can
speculate a little bit about it. AFAIK u-boot would probably place the rp1
node directly under its pcie@12000 node in DT while the dtb overlay will use
dynamically created PCI endpoint node (dev@0) as parent for rp1 node.

u-boot could do that and it would not be following the 25+ year old
PCI bus bindings. Some things may be argued about as "Linux bindings",
but that isn't one of them.

Rob