On Mon, Aug 16, 2021 at 2:15 PM Andrew Lunn <andrew@xxxxxxx> wrote: > > On Mon, Aug 16, 2021 at 01:43:19PM -0700, Saravana Kannan wrote: > > On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@xxxxxxx> wrote: > > > > > > Hi Saravana > > > > > > > Hi Andrew, > > > > > > > > > > > Also there > > > > are so many phy related properties that my head is spinning. Is there a > > > > "phy" property (which is different from "phys") that treated exactly as > > > > "phy-handle"? > > > > > > Sorry, i don't understand your question. > > > > Sorry. I was just saying I understand the "phy-handle" DT property > > (seems specific to ethernet PHY) and "phys" DT property (seems to be > > for generic PHYs -- used mostly by display and USB?). But I noticed > > there's yet another "phy" DT property which I'm not sure I understand. > > It seems to be used by display and ethernet and seems to be a > > deprecated property. If you can explain that DT property in the > > context of networking and how to interpret it as a human, that'd be > > nice. > > Ah, i think i understand: > > Documentation/devicetree/bindings/net/ethernet-controller.yaml > > phy: > $ref: "#/properties/phy-handle" > deprecated: true > > So it is used the same as phy-handle. I doubt there are many examples > of it, it has been deprecated a long time. Maybe look in the powerpc > dts files? > > > > > + /* > > > > + * Device tree nodes pointed to by phy-handle never have struct devices > > > > + * created for them even if they have a "compatible" property. So > > > > + * return the parent node pointer. > > > > + */ > > > > > > We have a classic bus with devices on it. The bus master is registers > > > with linux using one of the mdiobus_register() calls. That then > > > enumerates the bus, looking at the 32 possible address on the bus, > > > using mdiobus_scan. It then gets a little complex, due to > > > history. > > > > > > Originally, the only thing you could have on an MDIO bus was a > > > PHY. But devices on MDIO busses are more generic, and Linux gained > > > support for Ethernet switches on an MDIO bus, and there are a few > > > other sort device. So to keep the PHY API untouched, but to add these > > > extra devices, we added the generic struct mdio_device which > > > represents any sort of device on an MDIO bus. This has a struct device > > > embedded in it. > > > > > > When we scan the bus and find a PHY, a struct phy_device is created, > > > which has an embedded struct mdio_device. The struct device in that is > > > then registered with the driver core. > > > > > > So a phy-handle does point to a device, but you need to do an object > > > orientated style look at the base class to find it. > > > > Thanks for the detailed explanation. I didn't notice a phy_device had > > an mdio_device inside it. Makes sense. I think my comment is not > > worded accurately and it really should be: > > > > Device tree nodes pointed to by phy-handle (even if they have a > > "compatible" property) will never have struct devices probed and bound > > to a driver through the driver framework. It's the parent node/device > > that gets bound to a driver and initializes the PHY. So return the > > parent node pointer instead. > > > > Does this sound right? As opposed to PHYs the other generic mdio > > devices seem to actually have drivers that'll bind to them through the > > driver framework. > > That sounds wrong. The MDIO bus master is a linux device and has a > driver. Same as an I2C bus master, or an SPI bus master, or a USB > host. All these busses have devices on them, same as an MDIO bus. The > devices on the bus are found and registered with the driver > framework. The driver framework, with some help from the mdio bus > class, with then find the correct driver of the device, and probe > it. During probe, it gets initialized by the PHY driver. > > So for me, the parent of a PHY would be the MDIO bus master, and the > bus master is not driving the PHY, in the same way an I2C bus master > does not drive the tmp100 temperature sensor on an i2c bus. > > But maybe i don't understand your terminology here? > > Maybe this will help: > > root@370rd:/sys/class/mdio_bus# ls -l > total 0 > lrwxrwxrwx 1 root root 0 Jan 2 2021 '!soc!internal-regs!mdio@72004!switch@10!mdio' -> '../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii/f1072004.mdio-mii:10/mdio_bus/!soc!internal-regs!mdio@72004!switch@10!mdio' > lrwxrwxrwx 1 root root 0 Jan 2 2021 f1072004.mdio-mii -> ../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii > lrwxrwxrwx 1 root root 0 Jan 2 2021 fixed-0 -> '../../devices/platform/Fixed MDIO bus.0/mdio_bus/fixed-0' > > So there are three MDIO bus masters. > > Going into f1072004.mdio-mii, we see there are two PHYs on this bus: > > root@370rd:/sys/class/mdio_bus/f1072004.mdio-mii# ls -l > total 0 > lrwxrwxrwx 1 root root 0 Aug 16 21:03 device -> ../../../f1072004.mdio > drwxr-xr-x 5 root root 0 Jan 2 2021 f1072004.mdio-mii:00 > drwxr-xr-x 6 root root 0 Jan 2 2021 f1072004.mdio-mii:10 > lrwxrwxrwx 1 root root 0 Aug 16 21:03 of_node -> ../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004 > drwxr-xr-x 2 root root 0 Aug 16 21:03 power > drwxr-xr-x 2 root root 0 Aug 16 21:03 statistics > lrwxrwxrwx 1 root root 0 Jan 2 2021 subsystem -> ../../../../../../../class/mdio_bus > -rw-r--r-- 1 root root 4096 Jan 2 2021 uevent > -r--r--r-- 1 root root 4096 Aug 16 21:03 waiting_for_supplier > > and going into one of the PHYs f1072004.mdio-mii:00 > > lrwxrwxrwx 1 root root 0 Aug 16 20:54 attached_dev -> ../../../../f1070000.ethernet/net/eth0 > lrwxrwxrwx 1 root root 0 Aug 16 20:54 driver -> '../../../../../../../../bus/mdio_bus/drivers/Marvell 88E1510' > drwxr-xr-x 3 root root 0 Jan 2 2021 hwmon > lrwxrwxrwx 1 root root 0 Aug 16 20:54 of_node -> ../../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004/ethernet-phy@0 > -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_dev_flags > -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_has_fixups > -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_id > -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_interface > drwxr-xr-x 2 root root 0 Aug 16 20:54 power > drwxr-xr-x 2 root root 0 Aug 16 20:54 statistics > lrwxrwxrwx 1 root root 0 Jan 2 2021 subsystem -> ../../../../../../../../bus/mdio_bus > -rw-r--r-- 1 root root 4096 Jan 2 2021 uevent > > The phy-handle in the MAC node points to ethernet-phy@0. Thanks! Looks like I got confused a bit based on some misconceptions I got when working on unrelated patches. I read through the code and now understand how this works. I'll fix up this patch and send out a v2 (it actually makes the patch simpler). On an unrelated note, I'm still a bit confused on what's going on in get_phy_device() when one PHY C45 device has multiple IDs found by get_phy_c45_ids(). Based on the comments, it looks like each of those IDs might be separate devices? Or am I misunderstanding the comment? If they are separate devices, how is one phy_device representing them all? -Saravana
