diff mbox

[2/4] net: phy: Add MAC-IF driver for Microsemi PHYs.

Message ID 646450A91FAED74E85C6E9C4D6E936A145336694@avsrvexchmbx1.microsemi.net
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Raju Lakkaraju Aug. 24, 2016, 12:24 p.m. UTC 
From: Nagaraju Lakkaraju <Raju.Lakkaraju@microsemi.com>

MAC Interface support will be added for VSC 85xx Microsemi PHYs.

Signed-off-by: Nagaraju Lakkaraju <Raju.Lakkaraju@microsemi.com>
---
 drivers/net/phy/mscc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mscc.h   |  2 ++
 2 files changed, 74 insertions(+)

Comments

Andrew Lunn Aug. 24, 2016, 1:06 p.m. UTC | #1 
> +static int vsc85xx_mac_if_set(struct phy_device *phydev,
> +                             phy_interface_t   *interface)
> +{
> +       int rc;
> +       u16 reg_val;
> +
> +       mutex_lock(&phydev->lock);
> +       reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
> +       switch (*interface) {
> +       case PHY_INTERFACE_MODE_RGMII:
> +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> +               reg_val |= (MAC_IF_SELECTION_RGMII << MAC_IF_SELECTION_POS);
> +               break;
> +       case PHY_INTERFACE_MODE_RMII:
> +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> +               reg_val |= (MAC_IF_SELECTION_RMII << MAC_IF_SELECTION_POS);
> +               break;
> +       case PHY_INTERFACE_MODE_MII:
> +       case PHY_INTERFACE_MODE_GMII:
> +       default:
> +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> +               reg_val |= (MAC_IF_SELECTION_GMII << MAC_IF_SELECTION_POS);
> +               break;
> +       }
> +       rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
> +       if (rc != 0)
> +               goto out_unlock;
> +
> +       rc = vsc85xx_soft_reset(phydev);
> +
> +out_unlock:
> +       mutex_unlock(&phydev->lock);
> +
> +       return rc;
> +}

Again, you need to justify why you are doing something completely
different to all other phy drivers. You cannot you do what
m88e1121_config_aneg(), mv88e111_config_init(), dp83867_config_init()
does?

	Andrew
Raju Lakkaraju Sept. 8, 2016, 9:09 a.m. UTC | #2 
Hi Andrew,

Thank you for review the code and valuable comments.
I accepted your review comments.
I will re-send the code.

Thanks,
Raju.

On Wed, Aug 24, 2016 at 03:06:44PM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> > +static int vsc85xx_mac_if_set(struct phy_device *phydev,
> > +                             phy_interface_t   *interface)
> > +{
> > +       int rc;
> > +       u16 reg_val;
> > +
> > +       mutex_lock(&phydev->lock);
> > +       reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
> > +       switch (*interface) {
> > +       case PHY_INTERFACE_MODE_RGMII:
> > +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> > +               reg_val |= (MAC_IF_SELECTION_RGMII << MAC_IF_SELECTION_POS);
> > +               break;
> > +       case PHY_INTERFACE_MODE_RMII:
> > +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> > +               reg_val |= (MAC_IF_SELECTION_RMII << MAC_IF_SELECTION_POS);
> > +               break;
> > +       case PHY_INTERFACE_MODE_MII:
> > +       case PHY_INTERFACE_MODE_GMII:
> > +       default:
> > +               reg_val &= ~(MAC_IF_SELECTION_MASK);
> > +               reg_val |= (MAC_IF_SELECTION_GMII << MAC_IF_SELECTION_POS);
> > +               break;
> > +       }
> > +       rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
> > +       if (rc != 0)
> > +               goto out_unlock;
> > +
> > +       rc = vsc85xx_soft_reset(phydev);
> > +
> > +out_unlock:
> > +       mutex_unlock(&phydev->lock);
> > +
> > +       return rc;
> > +}
> 
> Again, you need to justify why you are doing something completely
> different to all other phy drivers. You cannot you do what
> m88e1121_config_aneg(), mv88e111_config_init(), dp83867_config_init()
> does?
> 
>         Andrew
diff mbox

Patch

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 963bf64..60d9d5f 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -84,6 +84,18 @@  static int vsc85xx_config_intr(struct phy_device *phydev)
        return rc;
 }

+static int vsc85xx_soft_reset(struct phy_device *phydev)
+{
+       int rc;
+       u16 reg_val;
+
+       reg_val = phy_read(phydev, MII_BMCR);
+       reg_val |= BMCR_RESET;
+       rc = phy_write(phydev, MII_BMCR, reg_val);
+
+       return rc;
+}
+
 static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev,
                                      u8     *rate)
 {
@@ -128,6 +140,60 @@  out_unlock:
        return rc;
 }

+static int vsc85xx_mac_if_set(struct phy_device *phydev,
+                             phy_interface_t   *interface)
+{
+       int rc;
+       u16 reg_val;
+
+       mutex_lock(&phydev->lock);
+       reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
+       switch (*interface) {
+       case PHY_INTERFACE_MODE_RGMII:
+               reg_val &= ~(MAC_IF_SELECTION_MASK);
+               reg_val |= (MAC_IF_SELECTION_RGMII << MAC_IF_SELECTION_POS);
+               break;
+       case PHY_INTERFACE_MODE_RMII:
+               reg_val &= ~(MAC_IF_SELECTION_MASK);
+               reg_val |= (MAC_IF_SELECTION_RMII << MAC_IF_SELECTION_POS);
+               break;
+       case PHY_INTERFACE_MODE_MII:
+       case PHY_INTERFACE_MODE_GMII:
+       default:
+               reg_val &= ~(MAC_IF_SELECTION_MASK);
+               reg_val |= (MAC_IF_SELECTION_GMII << MAC_IF_SELECTION_POS);
+               break;
+       }
+       rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
+       if (rc != 0)
+               goto out_unlock;
+
+       rc = vsc85xx_soft_reset(phydev);
+
+out_unlock:
+       mutex_unlock(&phydev->lock);
+
+       return rc;
+}
+
+static int vsc85xx_mac_if_get(struct phy_device *phydev,
+                             phy_interface_t   *interface)
+{
+       int rc = 0;
+       u16 reg_val;
+
+       reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
+       reg_val = ((reg_val & MAC_IF_SELECTION_MASK) >> MAC_IF_SELECTION_POS);
+       if (reg_val == MAC_IF_SELECTION_RGMII)
+               *interface = PHY_INTERFACE_MODE_RGMII;
+       else if (reg_val == MAC_IF_SELECTION_RMII)
+               *interface = PHY_INTERFACE_MODE_RMII;
+       else
+               *interface = PHY_INTERFACE_MODE_GMII;
+
+       return rc;
+}
+
 static int vsc85xx_features_set(struct phy_device *phydev)
 {
        int rc = 0;
@@ -137,6 +203,9 @@  static int vsc85xx_features_set(struct phy_device *phydev)
        case PHY_EDGE_RATE_CONTROL:
                rc = vsc85xx_edge_rate_cntl_set(phydev, &ftrs->rate);
                break;
+       case PHY_MAC_IF:
+               rc = vsc85xx_mac_if_set(phydev, &ftrs->mac_if);
+               break;
        default:
                break;
        }
@@ -153,6 +222,9 @@  static int vsc85xx_features_get(struct phy_device *phydev)
        case PHY_EDGE_RATE_CONTROL:
                rc = vsc85xx_edge_rate_cntl_get(phydev, &ftrs->rate);
                break;
+       case PHY_MAC_IF:
+               rc = vsc85xx_mac_if_get(phydev, &ftrs->mac_if);
+               break;
        default:
                break;
        }
diff --git a/include/linux/mscc.h b/include/linux/mscc.h
index b80a2ac..dcfd0ae 100644
--- a/include/linux/mscc.h
+++ b/include/linux/mscc.h
@@ -11,6 +11,7 @@ 

 enum phy_features {
        PHY_EDGE_RATE_CONTROL = 0,
+       PHY_MAC_IF            = 1,
        PHY_SUPPORTED_FEATURES_MAX
 };

@@ -28,6 +29,7 @@  enum rgmii_rx_clock_delay {
 struct phy_features_t {
        enum phy_features cmd;           /* PHY Supported Features */
        u8   rate;                       /* Edge rate control */
+       phy_interface_t mac_if;          /* MAC interface config */
 };

 #endif /*  __LINUX_MSCC_H */