Re: [PATCH net-next 5/5] net: phy: add support to get Master-Slave configuration

Hi Andrew,

Thank you for review comments.

The 06/14/2022 23:21, Andrew Lunn wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Tue, Jun 14, 2022 at 04:04:24PM +0530, Raju Lakkaraju wrote:

quoted

Implement reporting the Master-Slave configuration and state

Signed-off-by: Raju Lakkaraju <redacted>
---
 drivers/net/phy/mxl-gpy.c | 55 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 5ce1bf03bbd7..cf625ced4ec1 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c

@@ -27,11 +27,19 @@
 #define PHY_ID_GPY241BM              0x67C9DE80
 #define PHY_ID_GPY245B               0x67C9DEC0

+#define PHY_STD_GCTRL                0x09    /* Gbit ctrl */
+#define PHY_STD_GSTAT                0x0A    /* Gbit status */

#define MII_CTRL1000            0x09    /* 1000BASE-T control          */
#define MII_STAT1000            0x0a    /* 1000BASE-T status           */

from mii.h

I can use generic master-slave function.
I will remove these changes

quoted

 #define PHY_MIISTAT          0x18    /* MII state */
 #define PHY_IMASK            0x19    /* interrupt mask */
 #define PHY_ISTAT            0x1A    /* interrupt status */
 #define PHY_FWV                      0x1E    /* firmware version */

+#define PHY_STD_GCTRL_MS     BIT(11)
+#define PHY_STD_GCTRL_MSEN   BIT(12)
+
+#define PHY_STD_GSTAT_MSRES  BIT(14)
+#define PHY_STD_GSTAT_MSFAULT        BIT(15)

If the device is just following the standard, there should not be any
need to add defines, they should already exist. And if it does follow
the standard there are probably helpers you can use.

Accepted. 
Device following the standard. I will use generic master-slave function

quoted

 #define PHY_MIISTAT_SPD_MASK GENMASK(2, 0)
 #define PHY_MIISTAT_DPX              BIT(3)
 #define PHY_MIISTAT_LS               BIT(10)

@@ -160,6 +168,48 @@ static bool gpy_2500basex_chk(struct phy_device *phydev)
      return true;
 }

+static int gpy_master_slave_cfg_get(struct phy_device *phydev)
+{
+     int state;
+     int cfg;
+     int ret;
+
+     ret = phy_read(phydev, PHY_STD_GCTRL);
+     if (ret < 0) {
+             phydev_err(phydev, "Error: MDIO register access failed: %d\n",
+                        ret);
+             return ret;
+     }
+
+     if (ret & PHY_STD_GCTRL_MSEN)
+             if (ret & PHY_STD_GCTRL_MS)
+                     cfg = MASTER_SLAVE_CFG_MASTER_FORCE;
+             else
+                     cfg = MASTER_SLAVE_CFG_SLAVE_FORCE;
+     else
+             cfg = MASTER_SLAVE_CFG_MASTER_PREFERRED;
+
+     ret = phy_read(phydev, PHY_STD_GSTAT);
+     if (ret < 0) {
+             phydev_err(phydev, "Error: MDIO register access failed: %d\n",
+                        ret);
+             return ret;
+     }
+
+     if (ret & PHY_STD_GSTAT_MSFAULT)
+             state = MASTER_SLAVE_STATE_ERR;
+     else
+             if (ret & PHY_STD_GSTAT_MSRES)
+                     state = MASTER_SLAVE_STATE_MASTER;
+             else
+                     state = MASTER_SLAVE_STATE_SLAVE;
+
+     phydev->master_slave_get = cfg;
+     phydev->master_slave_state = state;
+
+     return 0;

Would genphy_read_master_slave() work?

Yes. 
I can use genphy_read_master_slave() function.

      Andrew

-- 

Thanks,
Raju