On Wed, Nov 22, 2023 at 07:32:22PM +0100, Christian Marangi wrote: > Sooooo.... Sorry if I insist but I would really love to have something > ""stable"" to move this further. (the changes are easy enough so it's > really a matter of finding a good DT structure) > > Maybe a good idea would be summarize the concern and see what solution > was proposed: Sorry, I didn't follow the entire discussion. I hope I'm not too far off with my understanding of your problems. I think you are hitting some of the same points I have hit with DSA. The PHY package could be considered an SoC with lots of peripherals on it, for which you'd want separate drivers. Just like a DSA switch would. I don't think it's exactly phylib's place to deal with that, just like it's not DSA's place to deal with complex SoCs, just with the switching IP (like the Ethernet PHY IP for phylib). https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/ Why does the ethernet-phy-package DT binding attempt to be so grand and generic? I would expect the 180 degree opposite. Make it have a _single_ compatible of "qcom,qca807x" (but don't use an "x" wildcard, do specify the full package name). Make it have a "reg" property which is the base MDIO address of the package. Write an mdio_device driver that probes on that. The PHY core already knows that if a child on the MDIO bus has a compatible string of the normal form (not like "ethernet-phy-id004d.d0b2"), then it's looking at an mdio_device. Make the OF node of the package have an "mdio" child with its own compatible string, which represents the place where PHYs are. The driver for the "mdio" child has a very simple implementation of the mii_bus ops, which just calls into the device parent (it can assume a certain parent implementation and private data structures). Lateral to the "mdio" child node of the "qcom,qca807x" package node, you could put any other device tree properties that you want. Make the mdio_device driver for "qcom,qca807x" use shared code if you want - but keep the device tree structure hardware-specific. Look at the compatible strings that e.g. the drivers/mfd/simple-mfd-i2c.c driver probes on. You could always change the driver implementation for a certain compatible string, but you'll be stuck with the ultra-generic compatible = "ethernet-phy-package", which has the problems that you mention. > > Concern list: > 1. ethernet-phy-package MUST be placed in mdio node (not in ethernet, > the example was wrong anyway) and MUST have an addr > > Current example doesn't have an addr. I would prefer this way but > no problem in changing this. > > Solution: > - Add reg to the ethernet-phy-package node with the base address of > the PHY package (base address = the first PHY address of the > package) Correct, what I'm saying is compatible with this. > > We will have a PHY node with the same address of the PHY package > node. Each PHY node in the PHY package node will have reg set to > the REAL address in the mdio bus. If the real PHY IPs are children of the package rather than on the same level with it, I don't think this will be a problem. I wonder if some address translation could be done with the "ranges" device tree property. I've never seen this with MDIO though. > 4. Not finding a correct place to put PHY package info. > > I'm still convinced the mdio node is the correct place. > - PHY package are PHY in bundle so they are actual PHY > - We already have in the mdio node special handling (every DSA switch > use custom compatible and PHY ID is not used to probe them > normally) > - Node this way won't be treated as PHY as they won't match the PHY > node name pattern and also won't have the compatible pattern for > PHY. > > Solution: > - ethernet-phy-package node is OK given a reg is defined. I agree that it should sit under the MDIO node. I disagree with the idea of a standardized binding for PHY packages.
