On Mon, Nov 20, 2023 at 09:44:58PM +0100, Andrew Lunn wrote: > On Mon, Nov 20, 2023 at 02:50:30PM +0100, Christian Marangi wrote: > > Document ethernet PHY package nodes used to describe PHY shipped in > > bundle of 4-5 PHY. These particular PHY require specific PHY in the > > package for global onfiguration of the PHY package. > > > > Example are PHY package that have some regs only in one PHY of the > > package and will affect every other PHY in the package, for example > > related to PHY interface mode calibration or global PHY mode selection. > > I think you are being overly narrow here. The 'global' registers could > be spread over multiple addresses. Particularly for a C22 PHY. I > suppose they could even be in a N+1 address space, where there is no > PHY at all. > > Where the global registers are is specific to a PHY package > vendor/model. The PHY driver should know this. All the PHY driver > needs to know is some sort of base offset. PHY0 in this package is > using address X. It can then use relative addressing from this base to > access the global registers for this package. Yes that would also work but adds extra fragile code in PHY driver. An idea might be define PHY package node with a reg that is the base addr... and if we really want every PHY in the PHY package node is an offset of the base addr. > > > It's also possible to specify the property phy-mode to specify that the > > PHY package sets a global PHY interface mode and every PHY of the > > package requires to have the same PHY interface mode. > > I don't think it is what simple. See the QCA8084 for example. 3 of the > 4 PHYs must use QXGMII. The fourth PHY can also use QXGMII but it can > be multiplexed to a different PMA and use 1000BaseX, SGMII or > 2500BaseX. Yes that is totally a problem but I think it can only be handled with some validation in the PHY driver... I assume probe_once would validate the modes? > > I do think we need somewhere to put package properties. But i don't > think phy-mode is such a property. At the moment, i don't have a good > example of a package property. > And this is the main problem with this thing... Find a good way to define them that everyone is OK with. Another idea might be introduce to each PHY a property that point to the PHY package node (phandle) with all the info... But where to place that??? Outside mdio node? That would be confusing... This is why I like this subnode way. I know it deviates a bit from the normal way of defining small node in the mdio node one for each PHY. > > +examples: > > + - | > > + ethernet { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ethernet-phy-package { > > + compatible = "ethernet-phy-package"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > You have the PHYs within the Ethernet node. This is allowed by DT, for > historic reasons. However, i don't remember the last time a patch was > submitted that actually used this method. Now a days, PHYs are on an > MDIO bus, and they are children of that bus in the DT representation. > However you represent the package needs to work with MDIO busses. > Using the ethernet node was an oversight and actually this is defined as a subnode in the mdio node. A real DT that use this is (ipq807x): &mdio { status = "okay"; pinctrl-0 = <&mdio_pins>; pinctrl-names = "default"; reset-gpios = <&tlmm 37 GPIO_ACTIVE_LOW>; ethernet-phy-package { compatible = "ethernet-phy-package"; phy-mode = "psgmii"; global-phys = <&qca8075_4>, <&qca8075_psgmii>; global-phy-names = "combo", "analog_psgmii"; qca8075_0: ethernet-phy@0 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <0>; }; qca8075_1: ethernet-phy@1 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <1>; }; qca8075_2: ethernet-phy@2 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <2>; }; qca8075_3: ethernet-phy@3 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <3>; }; qca8075_4: ethernet-phy@4 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <4>; }; qca8075_psgmii: ethernet-phy@5 { compatible = "ethernet-phy-ieee802.3-c22"; reg = <5>; }; }; qca8081: ethernet-phy@28 { compatible = "ethernet-phy-id004d.d101"; reg = <28>; reset-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>; }; aqr113c: ethernet-phy@8 { compatible = "ethernet-phy-ieee802.3-c45"; reg = <8>; reset-gpios = <&tlmm 63 GPIO_ACTIVE_LOW>; }; }; -- Ansuel