Message-ID: <CAKgT0UcVnhgmXNU2FGcy6hbzUQZwNBZw0EKbFF3DsKDc8r452A@mail.gmail.com>
Date: Wed, 10 Apr 2024 08:56:31 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Jakub Kicinski <kuba@...nel.org>, pabeni@...hat.com, 
	John Fastabend <john.fastabend@...il.com>, Alexander Lobakin <aleksander.lobakin@...el.com>, 
	Florian Fainelli <f.fainelli@...il.com>, Daniel Borkmann <daniel@...earbox.net>, 
	Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org, bhelgaas@...gle.com, 
	linux-pci@...r.kernel.org, Alexander Duyck <alexanderduyck@...com>, 
	Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [net-next PATCH 00/15] eth: fbnic: Add network driver for Meta
 Platforms Host Network Interface

On Tue, Apr 9, 2024 at 4:42 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > What is less clear to me is what do we do about uAPI / core changes.
>
> I would differentiate between core change and core additions. If there
> is very limited firmware on this device, i assume Linux is managing
> the SFP cage, and to some extend the PCS. Extending the core to handle
> these at higher speeds than currently supported would be one such core
> addition. I've no problem with this. And i doubt it will be a single
> NIC using such additions for too long. It looks like ClearFog CX LX2
> could make use of such extensions as well, and there are probably
> other boards and devices, maybe the Zynq 7000?

The driver on this device doesn't have full access over the PHY.
Basically we control everything from the PCS north, and the firmware
controls everything from the PMA south as the physical connection is
MUXed between 4 slices. So this means the firmware also controls all
the I2C and the QSFP and EEPROM. The main reason for this is that
those blocks are shared resources between the slices, as such the
firmware acts as the arbitrator for 4 slices and the BMC.

> > Is my reading correct? Does anyone have an opinion on whether we should
> > try to dig more into this question prior to merging the driver, and
> > set some ground rules? Or proceed and learn by doing?
>
> I'm not too keen on keeping potentially shareable code in the driver
> just because of UEFI. It has long been the norm that you should not
> have wrappers so you can reuse code in different OSes. And UEFI is
> just another OS. So i really would like to see a Linux I2C bus master
> driver, a linux GPIO driver if appropriate, and using phylink, just as
> i've pushed wangxun to do that, and to some extend nvidia with their
> GPIO controller embedded in their NIC. The nice thing is, the
> developers for wangxun has mostly solved all this for a PCIe device,
> so their code can be copied.

Well when you are a one man driver development team sharing code
definitely makes one's life much easier when you have to maintain
multiple drivers. That said, I will see what I can do to comply with
your requests while hopefully not increasing my maintenance burden too
much. If nothing else I might just write myself a kernel compatibility
shim in UEFI so I can reuse the code that way.

The driver has no access to I2C or GPIO. The QSFP and EEPROM are
shared so I don't have access directly from the driver and I will have
to send any messages over the Host/FW mailbox to change any of that.

> Do we need to set some ground rules? No. I can give similar feedback
> as i gave the wangxun developers, if Linux subsystems are not used
> appropriately.
>
>        Andrew

Yeah, I kind of assume that will always be the case. Rules only mean
something if they are enforced anway. Although having the rules
documented does help in terms of making them known to all those
involved.

Anyway, I have already incorporated most of the feedback from the
other maintainers. The two requests you had were a bit more difficult
as they are in areas I am not entirely familiar with so I thought I
would check in with you before I dive into them.

One request I recall you having was to look at using the LED framework
for the LEDs. I think I have enough info to chase that down and get it
resolved for v2. However if you have some examples you would prefer I
follow I can look into that.

As far as the PCS block does it matter if I expose what the actual
underlying IP is or isn't? My request for info on what I can disclose
seems to be moving at the speed of bureaucracy so I don't know how
long it would take if I were to try to write something truly generic.
That said if I just put together something that referred to this as
pcs-fbnic for now, would that work for making this acceptable for
upstream?

Powered by blists - more mailing lists