On Tue, Apr 9, 2024 at 4:42 PM Andrew Lunn <andrew@xxxxxxx> 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?
