Message-ID: <fed57536-cd2c-bf9b-37bc-7a653685cbf2@gmail.com>
Date:   Wed, 29 Mar 2023 11:50:56 -0500
From:   Frank Rowand <frowand.list@...il.com>
To:     Clément Léger <clement.leger@...tlin.com>
Cc:     Rob Herring <robh@...nel.org>, Lizhi Hou <lizhi.hou@....com>,
        linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, helgaas@...nel.org, max.zhen@....com,
        sonal.santan@....com, larry.liu@....com, brian.xu@....com,
        stefano.stabellini@...inx.com, trix@...hat.com,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "Steen.Hegelund@...rochip.com" <Steen.Hegelund@...rochip.com>,
        "Horatiu.Vultur@...rochip.com" <Horatiu.Vultur@...rochip.com>,
        "Allan.Nielsen@...rochip.com" <Allan.Nielsen@...rochip.com>,
        Vincent Whitchurch <vincent.whitchurch@...s.com>
Subject: Re: [PATCH V7 0/3] Generate device tree node for pci devices

On 3/9/23 02:45, Clément Léger wrote:
> Le Wed, 8 Mar 2023 01:31:52 -0600,
> Frank Rowand <frowand.list@...il.com> a écrit :
> 
>> On 3/6/23 18:52, Rob Herring wrote:
>>> On Mon, Mar 6, 2023 at 3:24 PM Frank Rowand <frowand.list@...il.com> wrote:  
>>>>  
>>
>> < snip >
>>
>> Hi Rob,
>>
>> I am in no position to comment intelligently on your comments until I
>> understand the SoC on PCI card model I am asking to be described in
>> this subthread.
> 
> Hi Frank,
> 
> Rather than answering all of the assumptions that were made in the upper
> thread (that are probably doing a bit too much of inference), I will
> re-explain that from scratch.

Thanks!  The below answers a lot of my questions.
> 
> Our usecase involves the lan966x SoCs. These SoCs are mainly targeting
> networking application and offers multiple SFP and RGMII interfaces.
> This Soc can be used in two exclusive modes (at least for the intended
> usage):
> 
> SoC mode:
>    The device runs Linux by itself, on ARM64 cores included in the
>    SoC. This use-case of the lan966x is currently almost upstreamed,
>    using a traditional Device Tree representation of the lan996x HW
>    blocks [1] A number of drivers for the different IPs of the SoC have
>    already been merged in upstream Linux (see
>    arch/arm/boot/dts/lan966x.dtsi)
> 
> PCI mode:
>   The lan966x SoC is configured as a PCIe endpoint (PCI card),
>   connected to a separate platform that acts as the PCIe root complex.
>   In this case, all the IO memories that are exposed by the devices
>   embedded on this SoC are exposed through PCI BARs 0 & 1 and the ARM64
>   cores of the SoC are not used. Since this is a PCIe card, it can be
>   plugged on any platform, of any architecture supporting PCIe.
> 
> This work only focus on the *PCI mode* usage. In this mode, we have the
> following prerequisites:
> - Should work on all architectures (x86, ARM64, etc)
> - Should be self-contained in the driver

> - Should be able to reuse all existing platform drivers

I've said before (in different words) that using an existing platform
driver for hardware on a PCI card requires shims, which have been
strongly rejected by the Linux kernel.

> 
> In PCI mode, the card runs a firmware (not that it matters at all by
> the way) which configure the card in PCI mode at boot time. In this
> mode, it exposes a single PCI physical function associated with
> vendor/product 0x1055/0x9660. This is not a multi-function PCI device !
> This means that all the IO memories (peripheral memories, device
> memories, registers, whatever you call them) are accessible using
> standard readl()/writel() on the BARs that have been remapped. For
> instance (not accurate), in the BAR 0, we will have this kind of memory
> map:
> 
>            BAR0
>    0x0 ┌───────────┐
>        │           │
>        ├───────────┤
>        │   Clock   │
>        │ controller│
>        ├───────────┤
>        │           │
>        ├───────────┤
>        │   I2C     │
>        │ controller│
>        ├───────────┤
>        │           │
>        ├───────────┤
>        │   MDIO    │
>        │ Controller│
>        ├───────────┤
>        │           │
>        ├───────────┤
>        │  Switch   │
>        │ Controller│
>        ├───────────┤
>        │           │
>        │   ...     │
>           
> 
> It also exposes either a single interrupt via the legacy interrupt
> (which can then be demuxed by reading the SoC internal interrupt
> controller registers), or multiple interrupts using MSI interrupts.
> 
> As stated before, all these peripherals are already supported in SoC
> mode and thus, there are aleready existing platform drivers for each of
> them. For more information about the devices that are exposed please
> see link [1] which is the device-tree overlay used to describe the
> lan9662 card.
> 
> In order to use the ethernet switch, we must configure everything that
> lies around this ethernet controller, here are a few amongst all of
> them:
> - MDIO bus
> - I2C controller for SFP modules access
> - Clock controller
> - Ethernet controller
> - Syscon
> 
> Since all the platform drivers already exist for these devices, we
> want to reuse them. Multiple solutions were thought of (fwnode, mfd,
> ACPI, device-tree) and eventually ruled out for some of them and efforts
> were made to try to tackle that (using fwnode [2], device-tree [3])
> 

> One way to do so is to use a device-tree overlay description that is
> loaded dynamically on the PCI device OF node. This can be done using the
> various device-tree series series that have been proposed (included
> this one). On systems that do not provide a device-tree of_root, create
> an empty of_root node (see [4]). Then during PCI enumeration, create
> device-tree node matching the PCI tree that was enumerated (See [5]).
> This is needed since the PCI card can be plugged on whatever port the
> user wants and thus it can not be statically described using a fixed
> "target-path" property in the overlay.

I understand that this is a use case and a desire to implement a
solution for the use case.  But this is a very non-standard model.
The proposal exposes a bunch of hardware beyond the pci interface
in a non-pci method.

No, just no.  Respect the pci interface boundary and do not drag
devicetree into an effort to pierce and straddle that boundary
(by adding information about the card, beyond the PCI controller,
into the system devicetree).  Information about dynamically
discoverable hardware does not belong in the devicetree.

-Frank

> 
> Finally, to glue everything together, we add a PCI driver for the
> VID/PID of the PCI card (See [6]). This driver is responsible of adding
> the "ranges" property in the device-tree PCI node to remap the child
> nodes "reg" property to the PCI memory map. This is needed because the
> PCI memory addresses differ between platform, enumeration order and so
> on.Finally, the driver will load the device-tree overlay (See [1]) to
> the PCI device-tree node. Eventually, a call to
> of_platform_default_populate() will probe the nodes and platform
> drivers.
> 
> I hope this will help you understanding what is going on here. In the
> meantime, I'm also trying to obtain public documentation about the
> lan966x SoC.
> 
> [1]
> https://github.com/clementleger/linux/blob/bf9b4ef803d86c4ae59a4ca195a4152b0d5c3cea/drivers/mfd/lan966x_pci.dts
> [2]
> https://lore.kernel.org/netdev/YhPSkz8+BIcdb72R@smile.fi.intel.com/T/
> [3]
> https://lore.kernel.org/lkml/20220427094502.456111-1-clement.leger@bootlin.com/
> [4]
> https://lore.kernel.org/lkml/20230223213418.891942-1-frowand.list@gmail.com/
> [5]
> https://lore.kernel.org/lkml/1674183732-5157-1-git-send-email-lizhi.hou@amd.com/
> [6]
> https://github.com/clementleger/linux/blob/bf9b4ef803d86c4ae59a4ca195a4152b0d5c3cea/drivers/mfd/lan966x_pci_of.c
> 

Powered by blists - more mailing lists