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

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: "Hans de Goede" <hdegoede@redhat.com>,
	"Clément Léger" <clement.leger@bootlin.com>,
	"Daniel Scally" <djrscally@gmail.com>,
	"Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	"Wolfram Sang" <wsa@kernel.org>, "Peter Rosin" <peda@axentia.se>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-i2c@vger.kernel.org, netdev@vger.kernel.org,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Subject: Re: [RFC 10/10] net: sfp: add support for fwnode
Date: Wed, 23 Feb 2022 16:37:05 +0200	[thread overview]
Message-ID: <YhZGkZNAaXNPBRbf@smile.fi.intel.com> (raw)
In-Reply-To: <YhYriwvHJKjrDQRf@shell.armlinux.org.uk>

On Wed, Feb 23, 2022 at 12:41:47PM +0000, Russell King (Oracle) wrote:
> On Wed, Feb 23, 2022 at 01:02:23PM +0100, Hans de Goede wrote:
> > On 2/23/22 12:22, Andy Shevchenko wrote:
> > > On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote:
> > >> Le Mon, 21 Feb 2022 19:57:39 +0200,
> > >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :
> > >>> On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote:
> > >>>> Add support to retrieve a i2c bus in sfp with a fwnode. This support
> > >>>> is using the fwnode API which also works with device-tree and ACPI.
> > >>>> For this purpose, the device-tree and ACPI code handling the i2c
> > >>>> adapter retrieval was factorized with the new code. This also allows
> > >>>> i2c devices using a software_node description to be used by sfp code.  
> > >>>
> > >>> If I'm not mistaken this patch can even go separately right now, since all used
> > >>> APIs are already available.
> > >>
> > >> This patches uses fwnode_find_i2c_adapter_by_node() which is introduced
> > >> by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can
> > >> probably be contributed both in a separate series.
> > > 
> > > I summon Hans into the discussion since I remember he recently refactored
> > > a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big
> > > picture approach with this series based on his ACPI experience.
> > 
> > If I understand this series correctly then this is about a PCI-E card
> > which has an I2C controller on the card and behind that I2C-controller
> > there are a couple if I2C muxes + I2C clients.
> 
> That is what I gathered as well.
> 
> > Assuming I did understand the above correctly. One alternative would be
> > to simply manually instantiate the I2C muxes + clients using
> > i2c_new_client_device(). But I'm not sure if i2c_new_client_device()
> > will work for the muxes without adding some software_nodes which
> > brings us back to something like this patch-set.
> 
> That assumes that an I2C device is always present, which is not always
> the case - there are hot-pluggable devices on I2C buses.
> 
> Specifically, this series includes pluggable SFP modules, which fall
> into this category of "hot-pluggable I2C devices" - spanning several
> bus addresses (0x50, 0x51, 0x56). 0x50 is EEPROM like, but not quite
> as the top 128 bytes is paged and sometimes buggy in terms of access
> behaviour. 0x51 contains a bunch of monitoring and other controls
> for the module which again can be paged. At 0x56, there may possibly
> be some kind of device that translates I2C accesses to MDIO accesses
> to access a PHY onboard.
> 
> Consequently, the SFP driver and MDIO translation layer wants access to
> the I2C bus, rather than a device.
> 
> Now, before ARM was converted to DT, we had ways to cope with
> non-firmware described setups like this by using platform devices and
> platform data. Much of that ended up deprecated, because - hey - DT
> is great and more modern and the old way is disgusting and we want to
> get rid of it.
> 
> However, that approach locks us into describing stuff in firmware,
> which is unsuitable when something like this comes along.

Looks like this is a way to reinvent what FPGA should cope with already.
And if I remember correctly the discussions about PCIe FPGAs (from 2016,
though) the idea is that FPGA should have provided a firmware description
with itself. I.o.w. If we are talking about "run-time configurable"
devices they should provide a way to bring their description to the
system.

The currently available way to do it is to get this from EEPROM / ROM
specified on the hardware side in form of DT and ACPI blobs (representing
overlays). Then the only part that is missed (at least for ACPI case) is
to dynamically insert that based on the PCI BDF of the corresponding
PCI bridge.

TL;DR: In my opinion such hardware must bring the description with itself
in case it uses non-enumerable busses, such as SPI, I²C.

I dunno what was the last development in this area for FPGAs cases.

-- 
With Best Regards,
Andy Shevchenko