On Sat, Feb 17, 2024 at 07:53:08PM +0000, Russell King (Oracle) wrote: > On Sat, Feb 17, 2024 at 08:41:11PM +0100, Christian Marangi wrote: > > Posting as RFC due to the massive change to a fundamental struct. > > > > While adding some PHY ID for Aquantia, I notice that there is a > > big problem with duplicating OPs with each PHY. > > > > The original idea to prevent this was to use mask on the PHY ID > > and identify PHY Family. Problem is that OEM started to use all > > kind of PHY ID and this is not doable, hence for PHY that have > > the same OPs, we have to duplicate all of them. > > > > This is present in Aquantia PHY, but is much more present in > > other PHY, especially in the BCM7XXX where they use a big macro > > for common PHYs. > > > > To reduce patch delta, I added the additional variable without > > adding tabs as this would have resulted in a massive patch. > > Also to have patch bisectable, this change has to be in one go > > hence I had to use this trick to reduce patch delta. > > > > Other solution to this problem were to introduce additional > > variables to phy_driver struct but that would have resulted > > in having 2 different way to do the same thing and that is not O.K. > > > > I took care to compile-test all the PHY, only exception is the unique > > RUST driver, where I still have to learn that funny language and > > I didn't had time to update it, so that is the only driver that > > I think require some fixup. > > > > I posted 2 example that would benefits from this change, but I can > > find much more in other PHY driver. > > Would it make more sense instead of this big churn, to instead > introduce into struct phy_driver: > > struct mdio_device_id *ids; > > which would then allow a phy_driver structure to be matched by > several device IDs? Yes that was an alternative idea, but is it good to then have 2 way to declare PHY ID? Also the name should be changed... Maybe an array of a struct PHY_ID, name that ends with a sentinel? > > We then would not need to touch any of the existing drivers initially, > and a later cleanup could be to identify those where all the ops are > the same for several phy_driver structures, and convert them over. We have many PHY that already have macro to define the same OPs and change only name PHY ID and mask. -- Ansuel
