Re: [RFC 10/10] net: sfp: add support for fwnode

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

Le Wed, 23 Feb 2022 14:39:27 +0100,
Hans de Goede <hdegoede@redhat.com> a écrit :

> > I think what we need is both approaches. We need a way for the SFP
> > driver (which is a platform_driver) to be used _without_ needing
> > descriptions in firmware. I think we have that for GPIOs, but for an
> > I2C bus, We have i2c_get_adapter() for I2C buses, but that needs the
> > bus number - we could either pass the i2c_adapter or the adapter
> > number through platform data to the SFP driver.
> > 
> > Or is there another solution to being able to reuse multi-driver
> > based infrastructure that we have developed based on DT descriptions
> > in situations such as an add-in PCI card?  
> 
> The use of software fwnode-s as proposed in this patch-set is another
> way to deal with this. There has been work to abstract ACPI vs
> of/dt firmware-nodes into a generic fwnode concept and software-nodes
> are a third way to define fwnode-s for "struct device" devices.
> 
> Software nodes currently are mainly used as so called secondary
> fwnodes which means they can e.g. add extra properties to cover
> for the firmware description missing some info (which at least
> on ACPI happens more often then we would like).
> 
> But a software-node can also be used as the primary fwnode for
> a device. So what this patch-set does is move the i2c of/dt
> enumeration code over to the fwnode abstraction (1). This allows
> the driver for the SPF card to attach a software fwnode to the
> device for the i2c-controller which describes the hotplug pins +
> any other always present hw in the same way as it would be done
> in a devicetree fwnode and then the existing of/dt based SPF
> code can be re-used as is.
> 
> At least that is my understanding of this patch-set.
> 
> Regards,
> 
> Hans

Hello Hans, your understanding is totally correct.

This PCIe device actually embeds much more than just a I2C controller.
I should have made that clearer in the cover letter, sorry for the
confusion. The PCIe card is actually using a lan9662x SoC which is
meant to be used as an ethernet switch with 4 ports (2 RJ45 and two
SFPS). In order to use this switch, the following drivers can be reused:
 - 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
 - lan966x-pci-mfd

All theses drivers are using of_* API and as such only works with a DT
description. One solution that did seems acceptable to me (although
not great)was to use mfd_cells and software_node description
as primary node.

Since I wanted to convert these to be software_node compatible, I had
to modify many subsystems (pinctrl, gpio, i2c, clocks, reset, etc).
This is why I stated in the cover letter that "This series is part of a
larger changeset that touches multiple subsystems". But clearly, it
lacks more context and namely the list of subsystems that needed to be
modify as well as the PCIe card type. I will modify this cover-letter to
add more informations.

So indeed, this series is targetting at using devices which uses a
software_node as a primary node and modifying subsystems to use the
fwnode API in order to make that compatible with these software nodes.
As you said, in order to avoid redefining the match tables and allow
device_get_match_data to work with software_node, the trick was to
reuse the of_table_id

However, I'm not totally happy with that as it seems we are doing what
was done with the "old" platform_data (a bit cleaner maybe since it
allows to reuse the driver with the fwnode API).

As Russell asked, I'm also really interested if someone has a solution
to reuse device-tree description (overlays ?) to describe such
hardware. However, the fact that CONFIG_OF isn't enabled on x86 config
seems a bit complicated on this side. This also requires to load a
device-tree overlay from the filesystem to describe the card, but that
might be something that could be made generic to allow other uses-cases.

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