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

From: Clément Léger <[email-masked]>
Date: Wed, 23 Feb 2022 18:59:27 +0100

Le Wed, 23 Feb 2022 17:41:30 +0000,
Mark Brown <[email-masked]> a écrit :

> On Wed, Feb 23, 2022 at 05:24:37PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 23, 2022 at 04:11:50PM +0100, Clément Léger wrote:  
> > > Le Wed, 23 Feb 2022 16:46:45 +0200,
> > > Andy Shevchenko <[email-masked]> a écrit :  
> 
> > > > And here is the problem. We have a few different resource providers
> > > > (a.k.a. firmware interfaces) which we need to cope with.  
> 
> > > Understood that but does adding fwnode support means it should work
> > > as-is with both DT and ACPI ? ACPI code is still in place and only the
> > > of part was converted. But maybe you expect the fwnode prot to be
> > > conformant with ACPI.  
> 
> > Not only me, I believe Mark also was against using pure DT approach on
> > ACPI enabled platforms.  
> 
> I'm not 100% clear on the context here (I did dig about a bit in the
> thread on lore but it looks like there's some extra context here) but in
> general I don't think there's any enthusiasm for trying to mix different
> firmware interfaces on a single system.  Certainly in the case of ACPI
> and DT they have substantial differences in system model and trying to
> paper over those cracks and integrate the two is a route to trouble.
> This doesn't look like it's trying to use a DT on an ACPI system though?

Ideally no, but it is a possibility mentionned by Andrew, use DT
overlays on an ACPI system. This series did not took this way (yet).
Andrew mentionned that it could potentially be done but judging by your
comment, i'm not sure you agree with that.

> 
> There's been some discussion on how to handle loadable descriptions for
> things like FPGA but I don't recall it ever having got anywhere concrete
> - I could have missed something.  Those are dynamic cases which are more
> trouble though.  For something that's a PCI card it's not clear that we
> can't just statically instanitate the devices from kernel code, that was
> how the MFD subsystem started off although it's now primarily applied to
> other applications.  That looks to be what's going on here?

Yes, in this series, I used the MFD susbsytems with mfd_cells. These
cells are attached with a swnode. Then, needed subsystems are
modified to use the fwnode API to be able to use them with
devices that have a swnode as a primary node.

> 
> There were separately some issues with people trying to create
> completely swnode based enumeration mechanisms for things that required
> totally independent code for handling swnodes which seemed very
> concerning but it's not clear to me if that's what's going on here.

The card is described entirely using swnode that in a MFD PCI
driver, everything is described statically. The "enumeration" is static
since all the devices are described in the driver and registered using
mfd_add_device() at probe time. Thus, I don't think it adds an
enumeration mechanism like you mention but I may be wrong.

> 
> > > As I said in the cover-letter, this approach is the only one that I did
> > > found acceptable without being tied to some firmware description. If you
> > > have another more portable approach, I'm ok with that. But this
> > > solution should ideally work with pinctrl, gpio, clk, reset, phy, i2c,
> > > i2c-mux without rewriting half of the code. And also allows to easily
> > > swap the PCIe card to other slots/computer without having to modify the
> > > description.  
> 
> > My proposal is to use overlays that card provides with itself.
> > These are supported mechanisms by Linux kernel.  
> 
> We have code for DT overlays in the kernel but it's not generically
> available.  There's issues with binding onto the platform device tree,
> though they're less of a problem with something like this where it seems
> to be a separate card with no cross links.

Indeed, the card does not have crosslinks with other devices and thus
it might be a solution to use a device-tree overlay (loaded from the
filesystem). But  I'm not sure if it's a good idea to do that on
a ACPI enabled platform.

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