Re: [PATCH RFC 4/7] net: phy: add support for probing MMDs >= 8 for devices-in-package

[Jeremy Linton]

Hi,

On 5/26/20 12:14 PM, Russell King - ARM Linux admin wrote:
On Tue, May 26, 2020 at 03:31:16PM +0100, Russell King wrote:
Add support for probing MMDs above 7 for a valid devices-in-package
specifier.
Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
---
  drivers/net/phy/phy_device.c | 39 ++++++++++++++++++++++++++++++++++--
  include/linux/phy.h          |  2 ++
  2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0d6b6ca66216..fa9164ac0f3d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -659,6 +659,28 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
int addr, u32 phy_id,
  }
  EXPORT_SYMBOL(phy_device_create);
  
+/* phy_c45_probe_present - checks to see if a MMD is present in the package
+ * @bus: the target MII bus
+ * @prtad: PHY package address on the MII bus
+ * @devad: PHY device (MMD) address
+ *
+ * Read the MDIO_STAT2 register, and check whether a device is responding
+ * at this address.
+ *
+ * Returns: negative error number on bus access error, zero if no device
+ * is responding, or positive if a device is present.
+ */
+static int phy_c45_probe_present(struct mii_bus *bus, int prtad, int devad)
+{
+       int stat2;
+
+       stat2 = mdiobus_c45_read(bus, prtad, devad, MDIO_STAT2);
+       if (stat2 < 0)
+               return stat2;
+
+       return (stat2 & MDIO_STAT2_DEVPRST) == MDIO_STAT2_DEVPRST_VAL;
+}
+
  /* get_phy_c45_devs_in_pkg - reads a MMD's devices in package registers.
   * @bus: the target MII bus
   * @addr: PHY address on the MII bus
@@ -709,12 +731,25 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, 
u32 *phy_id,
  {
        const int num_ids = ARRAY_SIZE(c45_ids->device_ids);
        u32 *devs = &c45_ids->devices_in_package;
-       int i, phy_reg;
+       int i, ret, phy_reg;
  
  	/* Find first non-zero Devices In package. Device zero is reserved
         * for 802.3 c45 complied PHYs, so don't probe it at first.
         */
-       for (i = 1; i < num_ids && *devs == 0; i++) {
+       for (i = 1; i < MDIO_MMD_NUM && *devs == 0; i++) {
+               if (i >= 8) {
+                       /* Only probe for the devices-in-package if there
+                        * is a PHY reporting as present here; this avoids
+                        * picking up on PHYs that implement non-IEEE802.3
+                        * compliant register spaces.
+                        */
+                       ret = phy_c45_probe_present(bus, addr, i);
+                       if (ret < 0)
+                               return -EIO;
+
+                       if (!ret)
+                               continue;
+               }

A second look at 802.3, this can't be done for all MMDs (which becomes
visible when I look at the results from the 88x3310.)  Only MMDs 1, 2,
3, 4, 5, 30 and 31 are defined to have this register with the "Device
Present" bit pair.


I'm not sure it helps, but my thought process following some of the 
discussion last night was:

something to the effect:

        for (i = 1; i < MDIO_MMD_NUM && *devs == 0; i++) {
+               if (i & RESERVED_MMDS)
+                       continue;

where RESERVED_MMDS was a hardcoded bitfield matching the IEEE reserved 
MMDs. or maybe a "IGNORE_MMDS" which also includes BIT0 and other MMDs 
the code doesn't understand.