Re: [PATCH 1/4] net: phy: Add Edge-rate driver for Microsemi PHYs.

2016-09-08 Thread Raju Lakkaraju
Hi Andrew,

Thank you for review the code and valuable comments.
I accepted your review comments.
I too use the Device Tree for Edge-rate and MAC interface 
configuration.

Thanks,
Raju.

On Wed, Aug 24, 2016 at 02:59:34PM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> On Wed, Aug 24, 2016 at 12:20:03PM +, Raju Lakkaraju wrote:
> > From: Nagaraju Lakkaraju 
> >
> > Edge rate control support will be added for VSC 85xx Microsemi PHYs.
> 
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 2d24b28..8ec4c09 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -586,6 +586,8 @@ struct phy_driver {
> > void (*get_strings)(struct phy_device *dev, u8 *data);
> > void (*get_stats)(struct phy_device *dev,
> >   struct ethtool_stats *stats, u64 *data);
> > +   int (*phy_features_set)(struct phy_device *dev);
> > +   int (*phy_features_get)(struct phy_device *dev);
> >  };
> 
> Now we need the missing cover note what should be in 0/4.  What is the
> big picture? How are these two functions supposed to be used? Is there
> going to be a user space API via netlink? Should the MAC driver
> somehow call these functions? Are you going to extend the phylib with
> code to call these?
> 
> Those are all general questions for these two functions.
> 
> Now specifically for edge control, why did you decide not to use
> device tree? Both the micrel and renesas phy driver uses device tree
> for skew control. You need to explain why you need to do something
> different to other drivers.
> 
>   Thanks
> Andrew
> 
> 


Re: [PATCH 1/4] net: phy: Add Edge-rate driver for Microsemi PHYs.

2016-08-24 Thread Andrew Lunn
On Wed, Aug 24, 2016 at 12:20:03PM +, Raju Lakkaraju wrote:
> From: Nagaraju Lakkaraju 
> 
> Edge rate control support will be added for VSC 85xx Microsemi PHYs.

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2d24b28..8ec4c09 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -586,6 +586,8 @@ struct phy_driver {
> void (*get_strings)(struct phy_device *dev, u8 *data);
> void (*get_stats)(struct phy_device *dev,
>   struct ethtool_stats *stats, u64 *data);
> +   int (*phy_features_set)(struct phy_device *dev);
> +   int (*phy_features_get)(struct phy_device *dev);
>  };

Now we need the missing cover note what should be in 0/4.  What is the
big picture? How are these two functions supposed to be used? Is there
going to be a user space API via netlink? Should the MAC driver
somehow call these functions? Are you going to extend the phylib with
code to call these?

Those are all general questions for these two functions.

Now specifically for edge control, why did you decide not to use
device tree? Both the micrel and renesas phy driver uses device tree
for skew control. You need to explain why you need to do something
different to other drivers.

  Thanks
Andrew




[PATCH 1/4] net: phy: Add Edge-rate driver for Microsemi PHYs.

2016-08-24 Thread Raju Lakkaraju
From: Nagaraju Lakkaraju 

Edge rate control support will be added for VSC 85xx Microsemi PHYs.

Signed-off-by: Nagaraju Lakkaraju 
---
 drivers/net/phy/mscc.c | 109 +
 include/linux/mscc.h   |  34 +++
 include/linux/phy.h|   2 +
 3 files changed, 118 insertions(+), 27 deletions(-)
 create mode 100644 include/linux/mscc.h

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 6cc3036..963bf64 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -11,34 +11,9 @@
 #include 
 #include 
 #include 
+#include 

-enum rgmii_rx_clock_delay {
-   RGMII_RX_CLK_DELAY_0_2_NS = 0,
-   RGMII_RX_CLK_DELAY_0_8_NS = 1,
-   RGMII_RX_CLK_DELAY_1_1_NS = 2,
-   RGMII_RX_CLK_DELAY_1_7_NS = 3,
-   RGMII_RX_CLK_DELAY_2_0_NS = 4,
-   RGMII_RX_CLK_DELAY_2_3_NS = 5,
-   RGMII_RX_CLK_DELAY_2_6_NS = 6,
-   RGMII_RX_CLK_DELAY_3_4_NS = 7
-};
-
-#define MII_VSC85XX_INT_MASK  25
-#define MII_VSC85XX_INT_MASK_MASK 0xa000
-#define MII_VSC85XX_INT_STATUS26
-
-#define MSCC_EXT_PAGE_ACCESS  31
-#define MSCC_PHY_PAGE_STANDARD0x /* Standard registers */
-#define MSCC_PHY_PAGE_EXTENDED_2  0x0002 /* Extended reg - page 2 */
-
-/* Extended Page 2 Registers */
-#define MSCC_PHY_RGMII_CNTL   20
-#define RGMII_RX_CLK_DELAY_MASK   0x0070
-#define RGMII_RX_CLK_DELAY_POS4
-
-/* Microsemi PHY ID's */
-#define PHY_ID_VSC85310x00070570
-#define PHY_ID_VSC85410x00070770
+#include "mscc_reg.h"

 static int vsc85xx_phy_page_set(struct phy_device *phydev, u8 page)
 {
@@ -109,6 +84,82 @@ static int vsc85xx_config_intr(struct phy_device *phydev)
return rc;
 }

+static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev,
+ u8 *rate)
+{
+   int rc;
+   u16 reg_val;
+   u8  edge_rate = *rate;
+
+   mutex_lock(>lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
+   if (rc != 0)
+   goto out_unlock;
+   reg_val = phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
+   reg_val &= ~(EDGE_RATE_CNTL_MASK);
+   reg_val |= (edge_rate << EDGE_RATE_CNTL_POS);
+   phy_write(phydev, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(>lock);
+
+   return rc;
+}
+
+static int vsc85xx_edge_rate_cntl_get(struct phy_device *phydev,
+ u8 *rate)
+{
+   int rc;
+   u16 reg_val;
+
+   mutex_lock(>lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
+   if (rc != 0)
+   goto out_unlock;
+   reg_val = phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
+   reg_val &= EDGE_RATE_CNTL_MASK;
+   *rate = reg_val >> EDGE_RATE_CNTL_POS;
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(>lock);
+
+   return rc;
+}
+
+static int vsc85xx_features_set(struct phy_device *phydev)
+{
+   int rc = 0;
+   struct phy_features_t *ftrs = (struct phy_features_t *)phydev->priv;
+
+   switch (ftrs->cmd) {
+   case PHY_EDGE_RATE_CONTROL:
+   rc = vsc85xx_edge_rate_cntl_set(phydev, >rate);
+   break;
+   default:
+   break;
+   }
+
+   return rc;
+}
+
+static int vsc85xx_features_get(struct phy_device *phydev)
+{
+   int rc = 0;
+   struct phy_features_t *ftrs = (struct phy_features_t *)phydev->priv;
+
+   switch (ftrs->cmd) {
+   case PHY_EDGE_RATE_CONTROL:
+   rc = vsc85xx_edge_rate_cntl_get(phydev, >rate);
+   break;
+   default:
+   break;
+   }
+
+   return rc;
+}
+
 /* Microsemi VSC85xx PHYs */
 static struct phy_driver vsc85xx_driver[] = {
 {
@@ -126,6 +177,8 @@ static struct phy_driver vsc85xx_driver[] = {
.config_intr= _config_intr,
.suspend= _suspend,
.resume = _resume,
+   .phy_features_set = _features_set,
+   .phy_features_get = _features_get,
 },
 {
.phy_id = PHY_ID_VSC8541,
@@ -142,6 +195,8 @@ static struct phy_driver vsc85xx_driver[] = {
.config_intr= _config_intr,
.suspend= _suspend,
.resume = _resume,
+   .phy_features_set = _features_set,
+   .phy_features_get = _features_get,
 }

 };
diff --git a/include/linux/mscc.h b/include/linux/mscc.h
new file mode 100644
index 000..b80a2ac
--- /dev/null
+++ b/include/linux/mscc.h
@@ -0,0 +1,34 @@
+/*
+ * Driver for Microsemi VSC85xx PHYs
+ *
+ * Author: Nagaraju Lakkaraju
+ * License: Dual MIT/GPL
+ * Copyright (c) 2016 Microsemi Corporation
+ */
+
+#ifndef