Re: [RFC 00/10] add support for fwnode in i2c mux system and sfp

["Clément Léger" <clement.leger@bootlin.com>]

Le Thu, 24 Feb 2022 15:40:40 +0100,
Clément Léger <clement.leger@bootlin.com> a écrit :

> Hi,
> 
> As stated at the beginning of the cover letter, the PCIe card I'm
> working on uses a lan9662 SoC. This card is meant to be used an
> ethernet switch with 2 x RJ45 ports and 2 x 10G SFPs. The lan966x SoCs
> can be used in two different ways:
> 
>  - It can run Linux by itself, on ARM64 cores included in the SoC. This
>    use-case of the lan966x is currently being 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.
> 
>  - It can be used as a PCIe endpoint, connected to a separate platform
>    that acts as the PCIe root complex. In this case, all the devices
>    that are embedded on this SoC are exposed through PCIe BARs 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.
> 
> The goal of this effort is to enable this second use-case, while
> allowing the re-use of the existing drivers for the different devices
> part of the SoC.
> 
> Following a first round of discussion, here are some clarifications on
> what problem this series is trying to solve and what are the possible
> choices to support this use-case.
> 
> Here is the list of devices that are exposed and needed to make this
> card work as an ethernet switch:
>  - lan966x-switch
>  - reset-microchip-sparx5
>  - lan966x_serdes
>  - reset-microchip-lan966x-phy
>  - mdio-mscc-miim
>  - pinctrl-lan966x
>  - atmel-flexcom
>  - i2c-at91
>  - i2c-mux
>  - i2c-mux-pinctrl
>  - sfp
>  - clk-lan966x
> 
> All the devices on this card are "self-contained" and do not require
> cross-links with devices that are on the host (except to demux IRQ but
> this is something easy to do). These drivers already exists and are
> using of_* API to register controllers, get properties and so on.
> 
> The challenge we're trying to solve is how can the PCI driver for this
> card re-use the existing drivers, and using which hardware
> representation to instantiate all those drivers.
> 
> Although this series only contained the modifications for the I2C
> subsystem all the subsystems that are used or needed by the previously
> listed driver have also been modified to have support for fwnode. This
> includes the following subsystems:
> - reset
> - clk
> - pinctrl
> - syscon
> - gpio
> - pinctrl
> - phy
> - mdio
> - i2c
> 
> The first feedback on this series does not seems to reach a consensus
> (to say the least) on how to do it cleanly so here is a recap of the
> possible solutions, either brought by this series or mentioned by
> contributors:
> 
> 1) Describe the card statically using swnode
> 
> This is the approach that was taken by this series. The devices are
> described using the MFD subsystem with mfd_cells. These cells are
> attached with a swnode which will be used as a primary node in place of
> ACPI or OF description. This means that the device description
> (properties and references) is conveyed entirely in the swnode. In order
> to make these swnode usable with existing OF based subsystems, the
> fwnode API can be used in needed subsystems.
> 
> Pros:
>  - Self-contained in the driver.
>  - Will work on all platforms no matter the firmware description.
>  - Makes the subsystems less OF-centric.
> 
> Cons:
>  - Modifications are required in subsystems to support fwnode
>    (mitigated by the fact it makes to subsystems less OF-centric).
>  - swnode are not meant to be used entirely as primary nodes.
>  - Specifications for both ACPI and OF must be handled if using fwnode
>    API.
> 
> 2) Use SSDT overlays
> 
> Andy mentioned that SSDT overlays could be used. This overlay should
> match the exact configuration that is used (ie correct PCIe bus/port
> etc). It requires the user to write/modify/compile a .asl file and load
> it using either EFI vars, custom initrd or via configfs. The existing
> drivers would also need more modifications to work with ACPI. Some of
> them might even be harder (if not possible) to use since there is no
> ACPI support for the subsystems they are using .
> 
> Pros:
>  - Can't really find any for this one
> 
> Cons:
>  - Not all needed subsystems have appropriate ACPI bindings/support
>    (reset, clk, pinctrl, syscon).
>  - Difficult to setup for the user (modify/compile/load .aml file).
>  - Not portable between machines, as the SSDT overlay need to be
>    different depending on how the PCI device is connected to the
>    platform.
> 
> 3) Use device-tree overlays
> 
> This solution was proposed by Andrew and could potentially allows to
> keep all the existing device-tree infrastructure and helpers. A
> device-tree overlay could be loaded by the driver and applied using
> of_overlay_fdt_apply(). There is some glue to make this work but it
> could potentially be possible. Mark have raised some warnings about
> using such device-tree overlays on an ACPI enabled platform.
> 
> Pros:
>  - Reuse all the existing OF infrastructure, no modifications at all on
>    drivers and subsystems.
>  - Could potentially lead to designing a generic driver for PCI devices
>    that uses a composition of other drivers.
> 
> Cons:
>  - Might not the best idea to mix it with ACPI.
>  - Needs CONFIG_OF, which typically isn't enabled today on most x86
>    platforms.
>  - Loading DT overlays on non-DT platforms is not currently working. It
>    can be addressed, but it's not necessarily immediate.
> 
> My preferred solutions would be swnode or device-tree overlays but
> since there to is no consensus on how to add this support, how
> can we go on with this series ?
> 
> Thanks,
> 
> [1]
> https://lore.kernel.org/linux-arm-kernel/20220210123704.477826-1-michael@walle.cc/
> 

Does anybody have some other advices or recommendation regarding
this RFC ? It would be nice to have more feedback on the solution that
might e preferred to support this use-case.

Thanks,


-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com