[PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[Stas Sergeev]

Hello.

The following patches are needed when mvneta is used
in SGMII mode without MDIO.

[PATCH 1/6] restructure of_phy_register_fixed_link() for further modifications

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/of/of_mdio.c |   32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1bd4305..b3dc1e6 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -301,22 +301,26 @@ int of_phy_register_fixed_link(struct device_node *np)
                               "asym-pause");
         of_node_put(fixed_link_node);
         phy = fixed_phy_register(PHY_POLL, &status, np);
-        return IS_ERR(phy) ? PTR_ERR(phy) : 0;
-    }
-
-    /* Old binding */
-    fixed_link_prop = of_get_property(np, "fixed-link", &len);
-    if (fixed_link_prop && len == (5 * sizeof(__be32))) {
-        status.link = 1;
-        status.duplex = be32_to_cpu(fixed_link_prop[1]);
-        status.speed = be32_to_cpu(fixed_link_prop[2]);
-        status.pause = be32_to_cpu(fixed_link_prop[3]);
-        status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
-        phy = fixed_phy_register(PHY_POLL, &status, np);
-        return IS_ERR(phy) ? PTR_ERR(phy) : 0;
+        if (IS_ERR(phy))
+            return PTR_ERR(phy);
+    } else {
+        /* Old binding */
+        fixed_link_prop = of_get_property(np, "fixed-link", &len);
+        if (fixed_link_prop && len == (5 * sizeof(__be32))) {
+            status.link = 1;
+            status.duplex = be32_to_cpu(fixed_link_prop[1]);
+            status.speed = be32_to_cpu(fixed_link_prop[2]);
+            status.pause = be32_to_cpu(fixed_link_prop[3]);
+            status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
+            phy = fixed_phy_register(PHY_POLL, &status, np);
+            if (IS_ERR(phy))
+                return PTR_ERR(phy);
+        } else {
+            return -ENODEV;
+        }
     }
 
-    return -ENODEV;
+    return 0;
 }
 EXPORT_SYMBOL(of_phy_register_fixed_link);
 #endif
-- 
1.7.9.5

[PATCH 2/6] pass phy_device instead of net_device to fixed_phy link_update() function

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
Looking up phy_device pointer from net_device requires poking into
private data. This may be problematic if phy is handled separately
from net_device.
Pass phy_device pointer to link_update() function because looking
up net_device pointer from phy_device is trivial.

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/net/phy/fixed_phy.c |    7 +++----
 include/linux/phy_fixed.h   |    4 ++--
 net/dsa/slave.c             |    3 ++-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index a08a3c7..8078998 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -36,7 +36,7 @@ struct fixed_phy {
     u16 regs[MII_REGS_NUM];
     struct phy_device *phydev;
     struct fixed_phy_status status;
-    int (*link_update)(struct net_device *, struct fixed_phy_status *);
+    int (*link_update)(struct phy_device *, struct fixed_phy_status *);
     struct list_head node;
 };
 
@@ -139,8 +139,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int
phy_addr, int reg_num)
         if (fp->addr == phy_addr) {
             /* Issue callback if user registered it. */
             if (fp->link_update) {
-                fp->link_update(fp->phydev->attached_dev,
-                        &fp->status);
+                fp->link_update(fp->phydev, &fp->status);
                 fixed_phy_update_regs(fp);
             }
             return fp->regs[reg_num];
@@ -162,7 +161,7 @@ static int fixed_mdio_write(struct mii_bus *bus, int
phy_addr, int reg_num,
  * May be useful for PHY's that need to be software-driven.
  */
 int fixed_phy_set_link_update(struct phy_device *phydev,
-                  int (*link_update)(struct net_device *,
+                  int (*link_update)(struct phy_device *,
                          struct fixed_phy_status *))
 {
     struct fixed_mdio_bus *fmb = &platform_fmb;
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 7e75bfe..ac82e6d 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -19,7 +19,7 @@ extern struct phy_device *fixed_phy_register(unsigned
int irq,
                          struct device_node *np);
 extern void fixed_phy_del(int phy_addr);
 extern int fixed_phy_set_link_update(struct phy_device *phydev,
-            int (*link_update)(struct net_device *,
+            int (*link_update)(struct phy_device *,
                        struct fixed_phy_status *));
 #else
 static inline int fixed_phy_add(unsigned int irq, int phy_id,
@@ -38,7 +38,7 @@ static inline int fixed_phy_del(int phy_addr)
     return -ENODEV;
 }
 static inline int fixed_phy_set_link_update(struct phy_device *phydev,
-            int (*link_update)(struct net_device *,
+            int (*link_update)(struct phy_device *,
                        struct fixed_phy_status *))
 {
     return -ENODEV;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f23dead..86ac744 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -500,9 +500,10 @@ static void dsa_slave_adjust_link(struct net_device
*dev)
         phy_print_status(p->phy);
 }
 
-static int dsa_slave_fixed_link_update(struct net_device *dev,
+static int dsa_slave_fixed_link_update(struct phy_device *phy,
                        struct fixed_phy_status *status)
 {
+    struct net_device *dev = phy->attached_dev;
     struct dsa_slave_priv *p = netdev_priv(dev);
     struct dsa_switch *ds = p->parent;
 
-- 
1.7.9.5

[PATCH 3/6] fixed_phy: add fixed_phy_unregister()

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
A counterpart of fixed_phy_register().
Can be used on failure path.

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/net/phy/fixed_phy.c  |   11 +++++++++++
 drivers/net/phy/phy_device.c |    7 +++++++
 include/linux/phy.h          |    1 +
 include/linux/phy_fixed.h    |    4 ++++
 4 files changed, 23 insertions(+)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 8078998..c2e82a0 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -275,6 +275,17 @@ struct phy_device *fixed_phy_register(unsigned int irq,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_register);
 
+void fixed_phy_unregister(struct phy_device *phy)
+{
+    struct device_node *np = phy->dev.of_node;
+
+    fixed_phy_del(phy->addr);
+    phy_device_unregister(phy);
+    phy_device_free(phy);
+    of_node_put(np);
+}
+EXPORT_SYMBOL_GPL(fixed_phy_unregister);
+
 static int __init fixed_mdio_bus_init(void)
 {
     struct fixed_mdio_bus *fmb = &platform_fmb;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bdfe51f..8923c0d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -375,6 +375,13 @@ int phy_device_register(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_device_register);
 
+void phy_device_unregister(struct phy_device *phydev)
+{
+    phydev->bus->phy_map[phydev->addr] = NULL;
+    device_del(&phydev->dev);
+}
+EXPORT_SYMBOL(phy_device_unregister);
+
 /**
  * phy_find_first - finds the first PHY device on the bus
  * @bus: the target MII bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6858098..448dda3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -714,6 +714,7 @@ struct phy_device *phy_device_create(struct mii_bus
*bus, int addr, int phy_id,
                      struct phy_c45_device_ids *c45_ids);
 struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool
is_c45);
 int phy_device_register(struct phy_device *phy);
+void phy_device_unregister(struct phy_device *phydev);
 int phy_init_hw(struct phy_device *phydev);
 int phy_suspend(struct phy_device *phydev);
 int phy_resume(struct phy_device *phydev);
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index ac82e6d..bb7d17c 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -17,6 +17,7 @@ extern int fixed_phy_add(unsigned int irq, int phy_id,
 extern struct phy_device *fixed_phy_register(unsigned int irq,
                          struct fixed_phy_status *status,
                          struct device_node *np);
+extern void fixed_phy_unregister(struct phy_device *phy);
 extern void fixed_phy_del(int phy_addr);
 extern int fixed_phy_set_link_update(struct phy_device *phydev,
             int (*link_update)(struct phy_device *,
@@ -33,6 +34,9 @@ static inline struct phy_device
*fixed_phy_register(unsigned int irq,
 {
     return ERR_PTR(-ENODEV);
 }
+static inline void fixed_phy_unregister(struct phy_device *phy)
+{
+}
 static inline int fixed_phy_del(int phy_addr)
 {
     return -ENODEV;
-- 
1.7.9.5

[PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/of/of_mdio.c    |   72
+++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_mdio.h |   17 +++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index b3dc1e6..ade2426 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -251,6 +251,69 @@ struct phy_device *of_phy_attach(struct net_device
*dev,
 EXPORT_SYMBOL(of_phy_attach);
 
 #if defined(CONFIG_FIXED_PHY)
+struct fixed_link_data {
+    struct fixed_phy_status status;
+    struct fixed_phy_status changed;
+};
+
+static int of_phy_fixed_link_update(struct phy_device *phy,
+          struct fixed_phy_status *status)
+{
+    struct fixed_link_data *priv = phy->priv;
+
+    if (priv->changed.link) {
+        status->link = priv->status.link;
+        priv->changed.link = 0;
+    }
+    if (priv->changed.speed) {
+        status->speed = priv->status.speed;
+        priv->changed.speed = 0;
+    }
+    if (priv->changed.duplex) {
+        status->duplex = priv->status.duplex;
+        priv->changed.duplex = 0;
+    }
+    if (priv->changed.pause) {
+        status->pause = priv->status.pause;
+        priv->changed.pause = 0;
+    }
+    if (priv->changed.asym_pause) {
+        status->asym_pause = priv->status.asym_pause;
+        priv->changed.asym_pause = 0;
+    }
+    return 0;
+}
+
+int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
+{
+    struct fixed_link_data *priv = phy->priv;
+
+    priv->status.link = link;
+    priv->changed.link = 1;
+    return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_link);
+
+int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed)
+{
+    struct fixed_link_data *priv = phy->priv;
+
+    priv->status.speed = speed;
+    priv->changed.speed = 1;
+    return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_speed);
+
+int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex)
+{
+    struct fixed_link_data *priv = phy->priv;
+
+    priv->status.duplex = duplex;
+    priv->changed.duplex = 1;
+    return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_duplex);
+
 /*
  * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
  * support two DT bindings:
@@ -287,6 +350,7 @@ int of_phy_register_fixed_link(struct device_node *np)
     const __be32 *fixed_link_prop;
     int len;
     struct phy_device *phy;
+    struct fixed_link_data *priv;
 
     /* New binding */
     fixed_link_node = of_get_child_by_name(np, "fixed-link");
@@ -320,6 +384,14 @@ int of_phy_register_fixed_link(struct device_node *np)
         }
     }
 
+    priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+    if (!priv) {
+        fixed_phy_unregister(phy);
+        return -ENOMEM;
+    }
+    memset(&priv->changed, 0, sizeof(priv->changed));
+    phy->priv = priv;
+    fixed_phy_set_link_update(phy, of_phy_fixed_link_update);
     return 0;
 }
 EXPORT_SYMBOL(of_phy_register_fixed_link);
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index d449018..677adac 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -65,6 +65,9 @@ static inline struct mii_bus *of_mdio_find_bus(struct
device_node *mdio_np)
 #if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
 extern int of_phy_register_fixed_link(struct device_node *np);
 extern bool of_phy_is_fixed_link(struct device_node *np);
+extern int of_phy_fixed_link_set_link(struct phy_device *phy, int link);
+extern int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed);
+extern int of_phy_fixed_link_set_duplex(struct phy_device *phy, int
duplex);
 #else
 static inline int of_phy_register_fixed_link(struct device_node *np)
 {
@@ -74,6 +77,20 @@ static inline bool of_phy_is_fixed_link(struct
device_node *np)
 {
     return false;
 }
+static inline int of_phy_fixed_link_set_link(struct phy_device *phy,
int link)
+{
+    return -ENOSYS;
+}
+static inline int of_phy_fixed_link_set_speed(struct phy_device *phy,
+                          int speed)
+{
+    return -ENOSYS;
+}
+static inline int of_phy_fixed_link_set_duplex(struct phy_device *phy,
+                           int duplex)
+{
+    return -ENOSYS;
+}
 #endif
 
 
-- 
1.7.9.5

[PATCH 5/6] mvneta: implement SGMII-based in-band link status signaling

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
When MDIO bus is unavailable (common setup for SGMII), the in-band
signalling must be used to correctly track link status.

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/net/ethernet/marvell/mvneta.c |   94
+++++++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c
b/drivers/net/ethernet/marvell/mvneta.c
index 96208f1..44d2655 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -122,6 +122,7 @@
 #define      MVNETA_TX_INTR_MASK_ALL             (0xff << 0)
 #define      MVNETA_RX_INTR_MASK(nr_rxqs)        (((1 << nr_rxqs) - 1)
<< 8)
 #define      MVNETA_RX_INTR_MASK_ALL             (0xff << 8)
+#define      MVNETA_MISCINTR_INTR_MASK           BIT(31)
 
 #define MVNETA_INTR_OLD_CAUSE                    0x25a8
 #define MVNETA_INTR_OLD_MASK                     0x25ac
@@ -180,9 +181,11 @@
 #define MVNETA_GMAC_AUTONEG_CONFIG               0x2c0c
 #define      MVNETA_GMAC_FORCE_LINK_DOWN         BIT(0)
 #define      MVNETA_GMAC_FORCE_LINK_PASS         BIT(1)
+#define      MVNETA_GMAC_INBAND_AN_ENABLE        BIT(2)
 #define      MVNETA_GMAC_CONFIG_MII_SPEED        BIT(5)
 #define      MVNETA_GMAC_CONFIG_GMII_SPEED       BIT(6)
 #define      MVNETA_GMAC_AN_SPEED_EN             BIT(7)
+#define      MVNETA_GMAC_AN_FLOW_CTRL_EN         BIT(11)
 #define      MVNETA_GMAC_CONFIG_FULL_DUPLEX      BIT(12)
 #define      MVNETA_GMAC_AN_DUPLEX_EN            BIT(13)
 #define MVNETA_MIB_COUNTERS_BASE                 0x3080
@@ -304,6 +307,7 @@ struct mvneta_port {
     unsigned int link;
     unsigned int duplex;
     unsigned int speed;
+    int inband_status;
 };
 
 /* The mvneta_tx_desc and mvneta_rx_desc structures describe the
@@ -994,6 +998,17 @@ static void mvneta_defaults_set(struct mvneta_port *pp)
     val &= ~MVNETA_PHY_POLLING_ENABLE;
     mvreg_write(pp, MVNETA_UNIT_CONTROL, val);
 
+    if (pp->inband_status) {
+        val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
+        val &= ~(MVNETA_GMAC_FORCE_LINK_PASS |
+             MVNETA_GMAC_FORCE_LINK_DOWN);
+        val |= MVNETA_GMAC_INBAND_AN_ENABLE |
+               MVNETA_GMAC_AN_SPEED_EN |
+               MVNETA_GMAC_AN_FLOW_CTRL_EN |
+               MVNETA_GMAC_AN_DUPLEX_EN;
+        mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+    }
+
     mvneta_set_ucast_table(pp, -1);
     mvneta_set_special_mcast_table(pp, -1);
     mvneta_set_other_mcast_table(pp, -1);
@@ -2043,6 +2058,21 @@ static irqreturn_t mvneta_isr(int irq, void *dev_id)
     return IRQ_HANDLED;
 }
 
+static void mvneta_get_phy_status(struct mvneta_port *pp,
+                  struct fixed_phy_status *r_stat)
+{
+    u32 gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
+
+    r_stat->link = !!(gmac_stat & MVNETA_GMAC_LINK_UP);
+    if (gmac_stat & MVNETA_GMAC_SPEED_1000)
+        r_stat->speed = SPEED_1000;
+    else if (gmac_stat & MVNETA_GMAC_SPEED_100)
+        r_stat->speed = SPEED_100;
+    else
+        r_stat->speed = SPEED_10;
+    r_stat->duplex = !!(gmac_stat & MVNETA_GMAC_FULL_DUPLEX);
+}
+
 /* NAPI handler
  * Bits 0 - 7 of the causeRxTx register indicate that are transmitted
  * packets on the corresponding TXQ (Bit 0 is for TX queue 1).
@@ -2063,8 +2093,29 @@ static int mvneta_poll(struct napi_struct *napi,
int budget)
     }
 
     /* Read cause register */
-    cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE) &
-        (MVNETA_RX_INTR_MASK(rxq_number) |
MVNETA_TX_INTR_MASK(txq_number));
+    cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE);
+    if (cause_rx_tx & MVNETA_MISCINTR_INTR_MASK) {
+        u32 cause_misc = mvreg_read(pp, MVNETA_INTR_MISC_CAUSE);
+
+        mvreg_write(pp, MVNETA_INTR_MISC_CAUSE, 0);
+        if (pp->inband_status && (cause_misc &
+                 (MVNETA_CAUSE_PHY_STATUS_CHANGE |
+                  MVNETA_CAUSE_LINK_CHANGE |
+                  MVNETA_CAUSE_PSC_SYNC_CHANGE))) {
+            struct fixed_phy_status phy_stat;
+
+            mvneta_get_phy_status(pp, &phy_stat);
+            if (pp->link != phy_stat.link)
+                of_phy_fixed_link_set_link(pp->phy_dev,
+                               phy_stat.link);
+            if (pp->speed != phy_stat.speed)
+                of_phy_fixed_link_set_speed(pp->phy_dev,
+                                phy_stat.speed);
+            if (pp->duplex != phy_stat.duplex)
+                of_phy_fixed_link_set_duplex(pp->phy_dev,
+                                 phy_stat.duplex);
+        }
+    }
 
     /* Release Tx descriptors */
     if (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL) {
@@ -2109,7 +2160,9 @@ static int mvneta_poll(struct napi_struct *napi,
int budget)
         napi_complete(napi);
         local_irq_save(flags);
         mvreg_write(pp, MVNETA_INTR_NEW_MASK,
-                MVNETA_RX_INTR_MASK(rxq_number) |
MVNETA_TX_INTR_MASK(txq_number));
+                MVNETA_RX_INTR_MASK(rxq_number) |
+                MVNETA_TX_INTR_MASK(txq_number) |
+                MVNETA_MISCINTR_INTR_MASK);
         local_irq_restore(flags);
     }
 
@@ -2373,7 +2426,13 @@ static void mvneta_start_dev(struct mvneta_port *pp)
 
     /* Unmask interrupts */
     mvreg_write(pp, MVNETA_INTR_NEW_MASK,
-            MVNETA_RX_INTR_MASK(rxq_number) |
MVNETA_TX_INTR_MASK(txq_number));
+            MVNETA_RX_INTR_MASK(rxq_number) |
+            MVNETA_TX_INTR_MASK(txq_number) |
+            MVNETA_MISCINTR_INTR_MASK);
+    mvreg_write(pp, MVNETA_INTR_MISC_MASK,
+            MVNETA_CAUSE_PHY_STATUS_CHANGE |
+            MVNETA_CAUSE_LINK_CHANGE |
+            MVNETA_CAUSE_PSC_SYNC_CHANGE);
 
     phy_start(pp->phy_dev);
     netif_tx_start_all_queues(pp->dev);
@@ -2523,9 +2582,7 @@ static void mvneta_adjust_link(struct net_device
*ndev)
             val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
             val &= ~(MVNETA_GMAC_CONFIG_MII_SPEED |
                  MVNETA_GMAC_CONFIG_GMII_SPEED |
-                 MVNETA_GMAC_CONFIG_FULL_DUPLEX |
-                 MVNETA_GMAC_AN_SPEED_EN |
-                 MVNETA_GMAC_AN_DUPLEX_EN);
+                 MVNETA_GMAC_CONFIG_FULL_DUPLEX);
 
             if (phydev->duplex)
                 val |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
@@ -2554,12 +2611,24 @@ static void mvneta_adjust_link(struct net_device
*ndev)
 
     if (status_change) {
         if (phydev->link) {
-            u32 val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
-            val |= (MVNETA_GMAC_FORCE_LINK_PASS |
-                MVNETA_GMAC_FORCE_LINK_DOWN);
-            mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+            if (!pp->inband_status) {
+                u32 val = mvreg_read(pp,
+                          MVNETA_GMAC_AUTONEG_CONFIG);
+                val &= ~MVNETA_GMAC_FORCE_LINK_DOWN;
+                val |= MVNETA_GMAC_FORCE_LINK_PASS;
+                mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+                        val);
+            }
             mvneta_port_up(pp);
         } else {
+            if (!pp->inband_status) {
+                u32 val = mvreg_read(pp,
+                          MVNETA_GMAC_AUTONEG_CONFIG);
+                val &= ~MVNETA_GMAC_FORCE_LINK_PASS;
+                val |= MVNETA_GMAC_FORCE_LINK_DOWN;
+                mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+                        val);
+            }
             mvneta_port_down(pp);
         }
         phy_print_status(phydev);
@@ -2934,6 +3003,7 @@ static int mvneta_probe(struct platform_device *pdev)
     char hw_mac_addr[ETH_ALEN];
     const char *mac_from;
     int phy_mode;
+    int fixed_phy = 0;
     int err;
 
     /* Our multiqueue support is not complete, so for now, only
@@ -2967,6 +3037,7 @@ static int mvneta_probe(struct platform_device *pdev)
             dev_err(&pdev->dev, "cannot register fixed PHY\n");
             goto err_free_irq;
         }
+        fixed_phy = 1;
 
         /* In the case of a fixed PHY, the DT node associated
          * to the PHY is the Ethernet MAC DT node.
@@ -2990,6 +3061,7 @@ static int mvneta_probe(struct platform_device *pdev)
     pp = netdev_priv(dev);
     pp->phy_node = phy_node;
     pp->phy_interface = phy_mode;
+    pp->inband_status = (phy_mode == PHY_INTERFACE_MODE_SGMII) &&
fixed_phy;
 
     pp->clk = devm_clk_get(&pdev->dev, NULL);
     if (IS_ERR(pp->clk)) {
-- 
1.7.9.5

[PATCH 6/6] mvneta: port marvell's official in-band status enabling procedure

[Stas Sergeev]

-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
This is a back-port of official marvell's patch.
It changes nothing visible for me but looks reasonable.

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/net/ethernet/marvell/mvneta.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c
b/drivers/net/ethernet/marvell/mvneta.c
index 44d2655..5dc7416 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -100,6 +100,8 @@
 #define MVNETA_TXQ_CMD                           0x2448
 #define      MVNETA_TXQ_DISABLE_SHIFT            8
 #define      MVNETA_TXQ_ENABLE_MASK              0x000000ff
+#define MVNETA_GMAC_CLOCK_DIVIDER                0x24f4
+#define      MVNETA_GMAC_1MS_CLOCK_ENABLE        BIT(31)
 #define MVNETA_ACC_MODE                          0x2500
 #define MVNETA_CPU_MAP(cpu)                      (0x2540 + ((cpu) << 2))
 #define      MVNETA_CPU_RXQ_ACCESS_ALL_MASK      0x000000ff
@@ -166,6 +168,7 @@
 #define      MVNETA_GMAC_MAX_RX_SIZE_MASK        0x7ffc
 #define      MVNETA_GMAC0_PORT_ENABLE            BIT(0)
 #define MVNETA_GMAC_CTRL_2                       0x2c08
+#define      MVNETA_GMAC2_INBAND_AN_ENABLE       BIT(0)
 #define      MVNETA_GMAC2_PCS_ENABLE             BIT(3)
 #define      MVNETA_GMAC2_PORT_RGMII             BIT(4)
 #define      MVNETA_GMAC2_PORT_RESET             BIT(6)
@@ -1001,12 +1004,15 @@ static void mvneta_defaults_set(struct
mvneta_port *pp)
     if (pp->inband_status) {
         val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
         val &= ~(MVNETA_GMAC_FORCE_LINK_PASS |
-             MVNETA_GMAC_FORCE_LINK_DOWN);
+             MVNETA_GMAC_FORCE_LINK_DOWN |
+             MVNETA_GMAC_AN_FLOW_CTRL_EN);
         val |= MVNETA_GMAC_INBAND_AN_ENABLE |
                MVNETA_GMAC_AN_SPEED_EN |
-               MVNETA_GMAC_AN_FLOW_CTRL_EN |
                MVNETA_GMAC_AN_DUPLEX_EN;
         mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+        val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER);
+        val |= MVNETA_GMAC_1MS_CLOCK_ENABLE;
+        mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val);
     }
 
     mvneta_set_ucast_table(pp, -1);
@@ -2979,6 +2985,9 @@ static int mvneta_port_power_up(struct mvneta_port
*pp, int phy_mode)
         return -EINVAL;
     }
 
+    if (pp->inband_status)
+        ctrl |= MVNETA_GMAC2_INBAND_AN_ENABLE;
+
     /* Cancel Port Reset */
     ctrl &= ~MVNETA_GMAC2_PORT_RESET;
     mvreg_write(pp, MVNETA_GMAC_CTRL_2, ctrl);
-- 
1.7.9.5

Re: [PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[Florian Fainelli]

Hello,

On 26/03/15 08:56, Stas Sergeev wrote:
> Hello.
> 
> The following patches are needed when mvneta is used
> in SGMII mode without MDIO.

Would you mind using scripts/get_maintainer.pl such that the relevant
people who have touched this code in the past can be CC'd?

Your patches are barely readable, let alone that you have provided
little to explanation as to why you are making such invasive changes.
-- 
Florian

Re: [PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[Stas Sergeev]

26.03.2015 20:13, Florian Fainelli пишет:
> Hello,
>
> On 26/03/15 08:56, Stas Sergeev wrote:
>> Hello.
>>
>> The following patches are needed when mvneta is used
>> in SGMII mode without MDIO.
> Would you mind using scripts/get_maintainer.pl such that the relevant
> people who have touched this code in the past can be CC'd?
>
> Your patches are barely readable, let alone that you have provided
> little to explanation as to why you are making such invasive changes.
OK, will re-send tomorrow with more explanations and CCs.
Still not sure about "barely readable" though - I made them
through checkpatch.pl.

Re: [PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[Florian Fainelli]

On 26/03/15 10:23, Stas Sergeev wrote:
> 26.03.2015 20:13, Florian Fainelli пишет:
>> Hello,
>>
>> On 26/03/15 08:56, Stas Sergeev wrote:
>>> Hello.
>>>
>>> The following patches are needed when mvneta is used
>>> in SGMII mode without MDIO.
>> Would you mind using scripts/get_maintainer.pl such that the relevant
>> people who have touched this code in the past can be CC'd?
>>
>> Your patches are barely readable, let alone that you have provided
>> little to explanation as to why you are making such invasive changes.
> OK, will re-send tomorrow with more explanations and CCs.
> Still not sure about "barely readable" though - I made them
> through checkpatch.pl.

Your commit messages are very scarce, you need to explain more what you
are doing an why.
-- 
Florian

Re: [PATCH 0/6] mvneta: SGMII-based in-band link status signaling

[Stas Sergeev]

26.03.2015 20:29, Florian Fainelli пишет:
> On 26/03/15 10:23, Stas Sergeev wrote:
>> 26.03.2015 20:13, Florian Fainelli пишет:
>>> Hello,
>>>
>>> On 26/03/15 08:56, Stas Sergeev wrote:
>>>> Hello.
>>>>
>>>> The following patches are needed when mvneta is used
>>>> in SGMII mode without MDIO.
>>> Would you mind using scripts/get_maintainer.pl such that the relevant
>>> people who have touched this code in the past can be CC'd?
>>>
>>> Your patches are barely readable, let alone that you have provided
>>> little to explanation as to why you are making such invasive changes.
>> OK, will re-send tomorrow with more explanations and CCs.
>> Still not sure about "barely readable" though - I made them
>> through checkpatch.pl.
> Your commit messages are very scarce, you need to explain more what you
> are doing an why.
That's OK, will do, thanks.
You said that my patches and barely readable AND have
little explanation, so I thought you mean the code changes
itself are barely readable too.

Re: [PATCH 1/6] restructure of_phy_register_fixed_link() for further modifications

[Sergei Shtylyov]

Hello.

On 03/26/2015 06:58 PM, Stas Sergeev wrote:

> -=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-

    Hm, why? You could have put a proper change log here...
    Also, threading the patches one after the other is a bad idea, please post 
them all as a replies to the patchset summary (0/6).

> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
> ---
>   drivers/of/of_mdio.c |   32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)

> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 1bd4305..b3dc1e6 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -301,22 +301,26 @@ int of_phy_register_fixed_link(struct device_node *np)
>                                 "asym-pause");
>           of_node_put(fixed_link_node);
>           phy = fixed_phy_register(PHY_POLL, &status, np);
> -        return IS_ERR(phy) ? PTR_ERR(phy) : 0;
> -    }

    Unfortunately, your patch has been whitespace-damaged by your mailer, with 
all tabs converted to spaces.

WBR, Sergei

Re: [PATCH 1/6] restructure of_phy_register_fixed_link() for further modifications

[Stas Sergeev]

26.03.2015 22:01, Sergei Shtylyov пишет:
> Hello.
>
> On 03/26/2015 06:58 PM, Stas Sergeev wrote:
>
>> -=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
>
>    Hm, why?
Because I used some stupid thunderbird-patch-inline plugin that
adds that line:
http://www.floc.net/doc/git/contrib/thunderbird-patch-inline/README

> You could have put a proper change log here...
>    Also, threading the patches one after the other is a bad idea, 
> please post them all as a replies to the patchset summary (0/6).
Ok.

>    Unfortunately, your patch has been whitespace-damaged by your 
> mailer, with all tabs converted to spaces.
So the plugin didn't work as expected. :(

Re: [PATCH 2/6] pass phy_device instead of net_device to fixed_phy link_update() function

[David Miller]

From: Stas Sergeev <stsp@list.ru>
Date: Thu, 26 Mar 2015 19:00:01 +0300

> -=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-

Don't put this into youro commit messages, ever.

Re: [PATCH 2/6] pass phy_device instead of net_device to fixed_phy link_update() function

[Stas Sergeev]

27.03.2015 01:49, David Miller пишет:
> From: Stas Sergeev <stsp@list.ru>
> Date: Thu, 26 Mar 2015 19:00:01 +0300
>
>> -=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
> Don't put this into youro commit messages, ever.
I'll remove the silly thunderbird-patch-inline plugin (that adds this line)
and will try to convince thunderbird not to break indentation, oh...

Re: [PATCH 2/6] pass phy_device instead of net_device to fixed_phy link_update() function

[Thomas Petazzoni]

Dear Stas Sergeev,

On Fri, 27 Mar 2015 01:55:33 +0300, Stas Sergeev wrote:

> I'll remove the silly thunderbird-patch-inline plugin (that adds this line)
> and will try to convince thunderbird not to break indentation, oh...

Or you start using 'git send-email', which will do the right thing for
you.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

The following API is added:
- of_phy_fixed_link_set_link() allows to set link state (up/down)
- of_phy_fixed_link_set_speed() allows to set link speed
- of_phy_fixed_link_set_duplex() allows to enable/disable duplex

This API is needed when the MDIO-less link have some other means
of a status passing to MAC, for example with in-band data (SGMII).
MAC driver can then use that API to update the PHY status.

CC: Florian Fainelli <f.fainelli@gmail.com>
CC: Grant Likely <grant.likely@linaro.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: netdev@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/of/of_mdio.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_mdio.h |   17 +++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index b3dc1e6..ade2426 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -251,6 +251,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
 EXPORT_SYMBOL(of_phy_attach);

 #if defined(CONFIG_FIXED_PHY)
+struct fixed_link_data {
+	struct fixed_phy_status status;
+	struct fixed_phy_status changed;
+};
+
+static int of_phy_fixed_link_update(struct phy_device *phy,
+		  struct fixed_phy_status *status)
+{
+	struct fixed_link_data *priv = phy->priv;
+
+	if (priv->changed.link) {
+		status->link = priv->status.link;
+		priv->changed.link = 0;
+	}
+	if (priv->changed.speed) {
+		status->speed = priv->status.speed;
+		priv->changed.speed = 0;
+	}
+	if (priv->changed.duplex) {
+		status->duplex = priv->status.duplex;
+		priv->changed.duplex = 0;
+	}
+	if (priv->changed.pause) {
+		status->pause = priv->status.pause;
+		priv->changed.pause = 0;
+	}
+	if (priv->changed.asym_pause) {
+		status->asym_pause = priv->status.asym_pause;
+		priv->changed.asym_pause = 0;
+	}
+	return 0;
+}
+
+int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
+{
+	struct fixed_link_data *priv = phy->priv;
+
+	priv->status.link = link;
+	priv->changed.link = 1;
+	return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_link);
+
+int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed)
+{
+	struct fixed_link_data *priv = phy->priv;
+
+	priv->status.speed = speed;
+	priv->changed.speed = 1;
+	return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_speed);
+
+int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex)
+{
+	struct fixed_link_data *priv = phy->priv;
+
+	priv->status.duplex = duplex;
+	priv->changed.duplex = 1;
+	return 0;
+}
+EXPORT_SYMBOL(of_phy_fixed_link_set_duplex);
+
 /*
  * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
  * support two DT bindings:
@@ -287,6 +350,7 @@ int of_phy_register_fixed_link(struct device_node *np)
 	const __be32 *fixed_link_prop;
 	int len;
 	struct phy_device *phy;
+	struct fixed_link_data *priv;

 	/* New binding */
 	fixed_link_node = of_get_child_by_name(np, "fixed-link");
@@ -320,6 +384,14 @@ int of_phy_register_fixed_link(struct device_node *np)
 		}
 	}

+	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		fixed_phy_unregister(phy);
+		return -ENOMEM;
+	}
+	memset(&priv->changed, 0, sizeof(priv->changed));
+	phy->priv = priv;
+	fixed_phy_set_link_update(phy, of_phy_fixed_link_update);
 	return 0;
 }
 EXPORT_SYMBOL(of_phy_register_fixed_link);
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index d449018..677adac 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -65,6 +65,9 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 #if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
 extern int of_phy_register_fixed_link(struct device_node *np);
 extern bool of_phy_is_fixed_link(struct device_node *np);
+extern int of_phy_fixed_link_set_link(struct phy_device *phy, int link);
+extern int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed);
+extern int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex);
 #else
 static inline int of_phy_register_fixed_link(struct device_node *np)
 {
@@ -74,6 +77,20 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
 {
 	return false;
 }
+static inline int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
+{
+	return -ENOSYS;
+}
+static inline int of_phy_fixed_link_set_speed(struct phy_device *phy,
+					      int speed)
+{
+	return -ENOSYS;
+}
+static inline int of_phy_fixed_link_set_duplex(struct phy_device *phy,
+					       int duplex)
+{
+	return -ENOSYS;
+}
 #endif


-- 
1.7.9.5

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Florian Fainelli]

2015-03-27 6:37 GMT-07:00 Stas Sergeev <stsp-cmBhpYW9OiY@public.gmane.org>:
>
> The following API is added:
> - of_phy_fixed_link_set_link() allows to set link state (up/down)
> - of_phy_fixed_link_set_speed() allows to set link speed
> - of_phy_fixed_link_set_duplex() allows to enable/disable duplex
>
> This API is needed when the MDIO-less link have some other means
> of a status passing to MAC, for example with in-band data (SGMII).
> MAC driver can then use that API to update the PHY status.

I do not think any of these changes are required, if you look at
drivers/net/dsa/bcm_sf2.c, there is a fixed_link_update callback to
re-act to link up/down interrupts (but this could easily be extended
to speed/duplex as well), by directly modifying a fixed_phy_status
structure.

Can you try that approach in mvneta?

>
> CC: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> Signed-off-by: Stas Sergeev <stsp-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/of/of_mdio.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_mdio.h |   17 +++++++++++
>  2 files changed, 89 insertions(+)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index b3dc1e6..ade2426 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -251,6 +251,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
>  EXPORT_SYMBOL(of_phy_attach);
>
>  #if defined(CONFIG_FIXED_PHY)
> +struct fixed_link_data {
> +       struct fixed_phy_status status;
> +       struct fixed_phy_status changed;
> +};
> +
> +static int of_phy_fixed_link_update(struct phy_device *phy,
> +                 struct fixed_phy_status *status)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       if (priv->changed.link) {
> +               status->link = priv->status.link;
> +               priv->changed.link = 0;
> +       }
> +       if (priv->changed.speed) {
> +               status->speed = priv->status.speed;
> +               priv->changed.speed = 0;
> +       }
> +       if (priv->changed.duplex) {
> +               status->duplex = priv->status.duplex;
> +               priv->changed.duplex = 0;
> +       }
> +       if (priv->changed.pause) {
> +               status->pause = priv->status.pause;
> +               priv->changed.pause = 0;
> +       }
> +       if (priv->changed.asym_pause) {
> +               status->asym_pause = priv->status.asym_pause;
> +               priv->changed.asym_pause = 0;
> +       }
> +       return 0;
> +}
> +
> +int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.link = link;
> +       priv->changed.link = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_link);
> +
> +int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.speed = speed;
> +       priv->changed.speed = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_speed);
> +
> +int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.duplex = duplex;
> +       priv->changed.duplex = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_duplex);
> +
>  /*
>   * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
>   * support two DT bindings:
> @@ -287,6 +350,7 @@ int of_phy_register_fixed_link(struct device_node *np)
>         const __be32 *fixed_link_prop;
>         int len;
>         struct phy_device *phy;
> +       struct fixed_link_data *priv;
>
>         /* New binding */
>         fixed_link_node = of_get_child_by_name(np, "fixed-link");
> @@ -320,6 +384,14 @@ int of_phy_register_fixed_link(struct device_node *np)
>                 }
>         }
>
> +       priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv) {
> +               fixed_phy_unregister(phy);
> +               return -ENOMEM;
> +       }
> +       memset(&priv->changed, 0, sizeof(priv->changed));
> +       phy->priv = priv;
> +       fixed_phy_set_link_update(phy, of_phy_fixed_link_update);
>         return 0;
>  }
>  EXPORT_SYMBOL(of_phy_register_fixed_link);
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index d449018..677adac 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -65,6 +65,9 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
>  #if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
>  extern int of_phy_register_fixed_link(struct device_node *np);
>  extern bool of_phy_is_fixed_link(struct device_node *np);
> +extern int of_phy_fixed_link_set_link(struct phy_device *phy, int link);
> +extern int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed);
> +extern int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex);
>  #else
>  static inline int of_phy_register_fixed_link(struct device_node *np)
>  {
> @@ -74,6 +77,20 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
>  {
>         return false;
>  }
> +static inline int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
> +{
> +       return -ENOSYS;
> +}
> +static inline int of_phy_fixed_link_set_speed(struct phy_device *phy,
> +                                             int speed)
> +{
> +       return -ENOSYS;
> +}
> +static inline int of_phy_fixed_link_set_duplex(struct phy_device *phy,
> +                                              int duplex)
> +{
> +       return -ENOSYS;
> +}
>  #endif
>
>
> --
> 1.7.9.5



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Florian Fainelli]

2015-03-27 6:37 GMT-07:00 Stas Sergeev <stsp@list.ru>:
>
> The following API is added:
> - of_phy_fixed_link_set_link() allows to set link state (up/down)
> - of_phy_fixed_link_set_speed() allows to set link speed
> - of_phy_fixed_link_set_duplex() allows to enable/disable duplex
>
> This API is needed when the MDIO-less link have some other means
> of a status passing to MAC, for example with in-band data (SGMII).
> MAC driver can then use that API to update the PHY status.

I do not think any of these changes are required, if you look at
drivers/net/dsa/bcm_sf2.c, there is a fixed_link_update callback to
re-act to link up/down interrupts (but this could easily be extended
to speed/duplex as well), by directly modifying a fixed_phy_status
structure.

Can you try that approach in mvneta?

>
> CC: Florian Fainelli <f.fainelli@gmail.com>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: netdev@vger.kernel.org
> CC: devicetree@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
>
> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
> ---
>  drivers/of/of_mdio.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_mdio.h |   17 +++++++++++
>  2 files changed, 89 insertions(+)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index b3dc1e6..ade2426 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -251,6 +251,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
>  EXPORT_SYMBOL(of_phy_attach);
>
>  #if defined(CONFIG_FIXED_PHY)
> +struct fixed_link_data {
> +       struct fixed_phy_status status;
> +       struct fixed_phy_status changed;
> +};
> +
> +static int of_phy_fixed_link_update(struct phy_device *phy,
> +                 struct fixed_phy_status *status)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       if (priv->changed.link) {
> +               status->link = priv->status.link;
> +               priv->changed.link = 0;
> +       }
> +       if (priv->changed.speed) {
> +               status->speed = priv->status.speed;
> +               priv->changed.speed = 0;
> +       }
> +       if (priv->changed.duplex) {
> +               status->duplex = priv->status.duplex;
> +               priv->changed.duplex = 0;
> +       }
> +       if (priv->changed.pause) {
> +               status->pause = priv->status.pause;
> +               priv->changed.pause = 0;
> +       }
> +       if (priv->changed.asym_pause) {
> +               status->asym_pause = priv->status.asym_pause;
> +               priv->changed.asym_pause = 0;
> +       }
> +       return 0;
> +}
> +
> +int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.link = link;
> +       priv->changed.link = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_link);
> +
> +int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.speed = speed;
> +       priv->changed.speed = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_speed);
> +
> +int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex)
> +{
> +       struct fixed_link_data *priv = phy->priv;
> +
> +       priv->status.duplex = duplex;
> +       priv->changed.duplex = 1;
> +       return 0;
> +}
> +EXPORT_SYMBOL(of_phy_fixed_link_set_duplex);
> +
>  /*
>   * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
>   * support two DT bindings:
> @@ -287,6 +350,7 @@ int of_phy_register_fixed_link(struct device_node *np)
>         const __be32 *fixed_link_prop;
>         int len;
>         struct phy_device *phy;
> +       struct fixed_link_data *priv;
>
>         /* New binding */
>         fixed_link_node = of_get_child_by_name(np, "fixed-link");
> @@ -320,6 +384,14 @@ int of_phy_register_fixed_link(struct device_node *np)
>                 }
>         }
>
> +       priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv) {
> +               fixed_phy_unregister(phy);
> +               return -ENOMEM;
> +       }
> +       memset(&priv->changed, 0, sizeof(priv->changed));
> +       phy->priv = priv;
> +       fixed_phy_set_link_update(phy, of_phy_fixed_link_update);
>         return 0;
>  }
>  EXPORT_SYMBOL(of_phy_register_fixed_link);
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index d449018..677adac 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -65,6 +65,9 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
>  #if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
>  extern int of_phy_register_fixed_link(struct device_node *np);
>  extern bool of_phy_is_fixed_link(struct device_node *np);
> +extern int of_phy_fixed_link_set_link(struct phy_device *phy, int link);
> +extern int of_phy_fixed_link_set_speed(struct phy_device *phy, int speed);
> +extern int of_phy_fixed_link_set_duplex(struct phy_device *phy, int duplex);
>  #else
>  static inline int of_phy_register_fixed_link(struct device_node *np)
>  {
> @@ -74,6 +77,20 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
>  {
>         return false;
>  }
> +static inline int of_phy_fixed_link_set_link(struct phy_device *phy, int link)
> +{
> +       return -ENOSYS;
> +}
> +static inline int of_phy_fixed_link_set_speed(struct phy_device *phy,
> +                                             int speed)
> +{
> +       return -ENOSYS;
> +}
> +static inline int of_phy_fixed_link_set_duplex(struct phy_device *phy,
> +                                              int duplex)
> +{
> +       return -ENOSYS;
> +}
>  #endif
>
>
> --
> 1.7.9.5



-- 
Florian

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

27.03.2015 18:41, Florian Fainelli пишет:
> 2015-03-27 6:37 GMT-07:00 Stas Sergeev <stsp@list.ru>:
>>
>> The following API is added:
>> - of_phy_fixed_link_set_link() allows to set link state (up/down)
>> - of_phy_fixed_link_set_speed() allows to set link speed
>> - of_phy_fixed_link_set_duplex() allows to enable/disable duplex
>>
>> This API is needed when the MDIO-less link have some other means
>> of a status passing to MAC, for example with in-band data (SGMII).
>> MAC driver can then use that API to update the PHY status.
>
> I do not think any of these changes are required, if you look at
> drivers/net/dsa/bcm_sf2.c, there is a fixed_link_update callback to
> re-act to link up/down interrupts (but this could easily be extended
> to speed/duplex as well), by directly modifying a fixed_phy_status
> structure.
> 
> Can you try that approach in mvneta?
Just to make sure I properly understand what you want.
drivers/net/dsa/bcm_sf2.c registers the .fixed_link_update callback
of struct dsa_switch_driver, which is later called from net/dsa/slave.c's
dsa_slave_fixed_link_update().
Do you want mvneta to register a similar callback in of_mdio, instead
of adding an explicit state-updating functions? Something like
of_phy_fixed_link_set_update_callback()?
This will remove a few changes indeed, but perhaps not too much.
Please confirm if this is exactly what you want, and then I try.

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Florian Fainelli]

2015-03-27 9:07 GMT-07:00 Stas Sergeev <stsp@list.ru>:
> 27.03.2015 18:41, Florian Fainelli пишет:
>> 2015-03-27 6:37 GMT-07:00 Stas Sergeev <stsp@list.ru>:
>>>
>>> The following API is added:
>>> - of_phy_fixed_link_set_link() allows to set link state (up/down)
>>> - of_phy_fixed_link_set_speed() allows to set link speed
>>> - of_phy_fixed_link_set_duplex() allows to enable/disable duplex
>>>
>>> This API is needed when the MDIO-less link have some other means
>>> of a status passing to MAC, for example with in-band data (SGMII).
>>> MAC driver can then use that API to update the PHY status.
>>
>> I do not think any of these changes are required, if you look at
>> drivers/net/dsa/bcm_sf2.c, there is a fixed_link_update callback to
>> re-act to link up/down interrupts (but this could easily be extended
>> to speed/duplex as well), by directly modifying a fixed_phy_status
>> structure.
>>
>> Can you try that approach in mvneta?
> Just to make sure I properly understand what you want.
> drivers/net/dsa/bcm_sf2.c registers the .fixed_link_update callback
> of struct dsa_switch_driver, which is later called from net/dsa/slave.c's
> dsa_slave_fixed_link_update().

Right, that's because there is a two layer model, first in DSA, then
in the switch driver.

> Do you want mvneta to register a similar callback in of_mdio, instead
> of adding an explicit state-updating functions? Something like
> of_phy_fixed_link_set_update_callback()?

You don't need an of_phy_fixed_link_set_update callback, you just need
to provide a fixed_link_update callback in mvneta, that you register,
if and only if you are binding to a fixed PHY which is also SGMII (you
can use a phy-mode property to tell that). So essentially, do the same
thing as net/dsa/slave.c, checking that a PHY is fixed, and if so,
register a fixed_link_update callback, within mvneta, and this
callback updates fixed_phy_status, ala drivers/net/dsa/bcm_sf2.c to
adjust the link parameters.

> This will remove a few changes indeed, but perhaps not too much.
> Please confirm if this is exactly what you want, and then I try.

Let me know if this is clearer now, if not, I can certainly cook a
patch which does what I am suggesting. Thanks!
-- 
Florian

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

27.03.2015 19:21, Florian Fainelli пишет:
>> Do you want mvneta to register a similar callback in of_mdio, instead
>> of adding an explicit state-updating functions? Something like
>> of_phy_fixed_link_set_update_callback()?
> You don't need an of_phy_fixed_link_set_update callback, you just need
> to provide a fixed_link_update callback in mvneta, that you register,
That approach I in fact considered initially, as the simplest one,
and even had a patch. But I disliked the fact that then mvneta will
exploit the knowledge of the fact that of_phy_register_fixed_link()
uses a fixed_phy driver. What if the implementation will later change?
Also what makes me uncomfortable is that since of_phy_register_fixed_link()
doesn't even return the struct phy_device pointer, mvneta will have
to get around that and use for example of_phy_find_device(), or register
the callback later, after of_phy_connect(). dsa/slave.c does of_phy_connect()
initially, together with fixed link registration, so it gets around the
problem. But mvneta registers the fixed_link in .probe callback, and
does of_phy_connect() in .open callback.
This all made me to drop that idea despite the simplicity.

>> This will remove a few changes indeed, but perhaps not too much.
>> Please confirm if this is exactly what you want, and then I try.
> Let me know if this is clearer now, if not, I can certainly cook a
> patch which does what I am suggesting. Thanks!
I can do that too, because I already did.
Let me know if the above concerns are not important, and I'll
restore my initial patch.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

27.03.2015 19:21, Florian Fainelli пишет:
>> Do you want mvneta to register a similar callback in of_mdio, instead
>> of adding an explicit state-updating functions? Something like
>> of_phy_fixed_link_set_update_callback()?
> You don't need an of_phy_fixed_link_set_update callback, you just need
> to provide a fixed_link_update callback in mvneta, that you register,
That approach I in fact considered initially, as the simplest one,
and even had a patch. But I disliked the fact that then mvneta will
exploit the knowledge of the fact that of_phy_register_fixed_link()
uses a fixed_phy driver. What if the implementation will later change?
Also what makes me uncomfortable is that since of_phy_register_fixed_link()
doesn't even return the struct phy_device pointer, mvneta will have
to get around that and use for example of_phy_find_device(), or register
the callback later, after of_phy_connect(). dsa/slave.c does of_phy_connect()
initially, together with fixed link registration, so it gets around the
problem. But mvneta registers the fixed_link in .probe callback, and
does of_phy_connect() in .open callback.
This all made me to drop that idea despite the simplicity.

>> This will remove a few changes indeed, but perhaps not too much.
>> Please confirm if this is exactly what you want, and then I try.
> Let me know if this is clearer now, if not, I can certainly cook a
> patch which does what I am suggesting. Thanks!
I can do that too, because I already did.
Let me know if the above concerns are not important, and I'll
restore my initial patch.

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Florian Fainelli]

On 27/03/15 09:39, Stas Sergeev wrote:
> 27.03.2015 19:21, Florian Fainelli пишет:
>>> Do you want mvneta to register a similar callback in of_mdio, instead
>>> of adding an explicit state-updating functions? Something like
>>> of_phy_fixed_link_set_update_callback()?
>> You don't need an of_phy_fixed_link_set_update callback, you just need
>> to provide a fixed_link_update callback in mvneta, that you register,
> That approach I in fact considered initially, as the simplest one,
> and even had a patch. But I disliked the fact that then mvneta will
> exploit the knowledge of the fact that of_phy_register_fixed_link()
> uses a fixed_phy driver. What if the implementation will later change?

There is no reason why it should change later, that's the entire purpose
of why we can tell whether it is a fixed PHY or a regular MDIO-managed
PHY, and drivers rely on that for their operations.

> Also what makes me uncomfortable is that since of_phy_register_fixed_link()
> doesn't even return the struct phy_device pointer, mvneta will have
> to get around that and use for example of_phy_find_device(), or register
> the callback later, after of_phy_connect()

Ok, you could either make of_phy_register_fixed_link() return a
phy_device, or as you suggest resolve the phy_device from the
device_node later, your call.

. dsa/slave.c does of_phy_connect()
> initially, together with fixed link registration, so it gets around the
> problem. But mvneta registers the fixed_link in .probe callback, and
> does of_phy_connect() in .open callback.
> This all made me to drop that idea despite the simplicity.

Yet that's still the cleanest/less invasive approach imho.

> 
>>> This will remove a few changes indeed, but perhaps not too much.
>>> Please confirm if this is exactly what you want, and then I try.
>> Let me know if this is clearer now, if not, I can certainly cook a
>> patch which does what I am suggesting. Thanks!
> I can do that too, because I already did.
> Let me know if the above concerns are not important, and I'll
> restore my initial patch.
> 

I think your concerns are valid, but I don't think there is going to be
any problem with the approach I suggested because there is a contract
that the fixed PHYs and regular PHYs need to
-- 
Florian

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

27.03.2015 20:15, Florian Fainelli пишет:
> On 27/03/15 09:39, Stas Sergeev wrote:
>> 27.03.2015 19:21, Florian Fainelli пишет:
>>>> Do you want mvneta to register a similar callback in of_mdio, instead
>>>> of adding an explicit state-updating functions? Something like
>>>> of_phy_fixed_link_set_update_callback()?
>>> You don't need an of_phy_fixed_link_set_update callback, you just need
>>> to provide a fixed_link_update callback in mvneta, that you register,
>> That approach I in fact considered initially, as the simplest one,
>> and even had a patch. But I disliked the fact that then mvneta will
>> exploit the knowledge of the fact that of_phy_register_fixed_link()
>> uses a fixed_phy driver. What if the implementation will later change?
> 
> There is no reason why it should change later, that's the entire purpose
> of why we can tell whether it is a fixed PHY or a regular MDIO-managed
> PHY, and drivers rely on that for their operations.
I don't think anything is exported to the outside of of_mdio.
It uses fixed_phy driver completely internally. Assuming that we
can pass the created phy_device to the fixed_phy API directly, look
like a bit of layering violation to me. But it is not that big as to
worry about. :)
I don't think many drivers rely on that - perhaps only dsa_slave.
So I'll add the second one.

> Ok, you could either make of_phy_register_fixed_link() return a
> phy_device,
OK, if you think that's right - I'll do.
I was under an impression that this was intentional to not return
the phy.

> I think your concerns are valid, but I don't think there is going to be
> any problem with the approach I suggested because there is a contract
> that the fixed PHYs and regular PHYs need to
OK, so I'll do that on Monday then.

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

27.03.2015 20:15, Florian Fainelli пишет:
> I think your concerns are valid, but I don't think there is going to be
> any problem with the approach I suggested because there is a contract
> that the fixed PHYs and regular PHYs need to
Hello Florian.

As promised, today I tried to resurrect my first implementation
and do things as you suggested: install the link_update callback
for mvneta privately.
I feel SGMII setup is very common and deserves the separate API,
not the per-driver handling, but in any case, I'd like to show
the implementation first, then discuss.

Unfortunately, it didn't work quite right.
The problem is that mvneta calls phy_disconnect() on .ndo_stop
callback. After that, phy->attached_dev becomes NULL, and so the
link_update callback gets called with net_dev==NULL! And crashs.
Of course I can easily work around that, but IMHO its a bug -
the one that actually gets fixed by the patches I posted previously.
They were changing the callback to receive phy_device instead of
net_device, and so NULL will never be passed.
I am attaching the new patch so that you can take a look and decide.
If you still think its fine, even despite the NULL passing w/a, then
I'll mail it with the proper boilerplate. But if you agree that
passing NULL to link_update is a bug, then maybe you'll decide to
get the whole surgery thing. :)


-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
When MDIO bus is unavailable (common setup for SGMII), the in-band
signaling must be used to correctly track link state.
This patch enables the in-band status delivery for links state changes, namely:
- link up/down
- link speed
- duplex full/half
fixed_phy link_update() callback is used to update status.

Note: SGMII setup is so common that I think it deserves a separate API
rather than the per-driver handling. There is an alternative implementation
that adds such API:
https://lkml.org/lkml/2015/3/27/346

CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
CC: Florian Fainelli <f.fainelli@gmail.com>
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
 drivers/net/ethernet/marvell/mvneta.c |   97 +++++++++++++++++++++++++++++----
 1 file changed, 86 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 96208f1..5b9cc0a 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -100,6 +100,8 @@
 #define MVNETA_TXQ_CMD                           0x2448
 #define      MVNETA_TXQ_DISABLE_SHIFT            8
 #define      MVNETA_TXQ_ENABLE_MASK              0x000000ff
+#define MVNETA_GMAC_CLOCK_DIVIDER                0x24f4
+#define      MVNETA_GMAC_1MS_CLOCK_ENABLE        BIT(31)
 #define MVNETA_ACC_MODE                          0x2500
 #define MVNETA_CPU_MAP(cpu)                      (0x2540 + ((cpu) << 2))
 #define      MVNETA_CPU_RXQ_ACCESS_ALL_MASK      0x000000ff
@@ -122,6 +124,7 @@
 #define      MVNETA_TX_INTR_MASK_ALL             (0xff << 0)
 #define      MVNETA_RX_INTR_MASK(nr_rxqs)        (((1 << nr_rxqs) - 1) << 8)
 #define      MVNETA_RX_INTR_MASK_ALL             (0xff << 8)
+#define      MVNETA_MISCINTR_INTR_MASK           BIT(31)

 #define MVNETA_INTR_OLD_CAUSE                    0x25a8
 #define MVNETA_INTR_OLD_MASK                     0x25ac
@@ -165,6 +168,7 @@
 #define      MVNETA_GMAC_MAX_RX_SIZE_MASK        0x7ffc
 #define      MVNETA_GMAC0_PORT_ENABLE            BIT(0)
 #define MVNETA_GMAC_CTRL_2                       0x2c08
+#define      MVNETA_GMAC2_INBAND_AN_ENABLE       BIT(0)
 #define      MVNETA_GMAC2_PCS_ENABLE             BIT(3)
 #define      MVNETA_GMAC2_PORT_RGMII             BIT(4)
 #define      MVNETA_GMAC2_PORT_RESET             BIT(6)
@@ -180,9 +184,11 @@
 #define MVNETA_GMAC_AUTONEG_CONFIG               0x2c0c
 #define      MVNETA_GMAC_FORCE_LINK_DOWN         BIT(0)
 #define      MVNETA_GMAC_FORCE_LINK_PASS         BIT(1)
+#define      MVNETA_GMAC_INBAND_AN_ENABLE        BIT(2)
 #define      MVNETA_GMAC_CONFIG_MII_SPEED        BIT(5)
 #define      MVNETA_GMAC_CONFIG_GMII_SPEED       BIT(6)
 #define      MVNETA_GMAC_AN_SPEED_EN             BIT(7)
+#define      MVNETA_GMAC_AN_FLOW_CTRL_EN         BIT(11)
 #define      MVNETA_GMAC_CONFIG_FULL_DUPLEX      BIT(12)
 #define      MVNETA_GMAC_AN_DUPLEX_EN            BIT(13)
 #define MVNETA_MIB_COUNTERS_BASE                 0x3080
@@ -304,6 +310,7 @@ struct mvneta_port {
 	unsigned int link;
 	unsigned int duplex;
 	unsigned int speed;
+	int inband_status;
 };

 /* The mvneta_tx_desc and mvneta_rx_desc structures describe the
@@ -994,6 +1001,20 @@ static void mvneta_defaults_set(struct mvneta_port *pp)
 	val &= ~MVNETA_PHY_POLLING_ENABLE;
 	mvreg_write(pp, MVNETA_UNIT_CONTROL, val);

+	if (pp->inband_status) {
+		val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
+		val &= ~(MVNETA_GMAC_FORCE_LINK_PASS |
+			 MVNETA_GMAC_FORCE_LINK_DOWN |
+			 MVNETA_GMAC_AN_FLOW_CTRL_EN);
+		val |= MVNETA_GMAC_INBAND_AN_ENABLE |
+		       MVNETA_GMAC_AN_SPEED_EN |
+		       MVNETA_GMAC_AN_DUPLEX_EN;
+		mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+		val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER);
+		val |= MVNETA_GMAC_1MS_CLOCK_ENABLE;
+		mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val);
+	}
+
 	mvneta_set_ucast_table(pp, -1);
 	mvneta_set_special_mcast_table(pp, -1);
 	mvneta_set_other_mcast_table(pp, -1);
@@ -2043,6 +2064,29 @@ static irqreturn_t mvneta_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }

+static int mvneta_fixed_link_update(struct net_device *dev,
+				  struct fixed_phy_status *status)
+{
+	struct mvneta_port *pp;
+	u32 gmac_stat;
+
+	if (!dev) {
+		/* we have a bug here */
+		return -EINVAL;
+	}
+	pp = netdev_priv(dev);
+	gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
+	status->link = !!(gmac_stat & MVNETA_GMAC_LINK_UP);
+	if (gmac_stat & MVNETA_GMAC_SPEED_1000)
+		status->speed = SPEED_1000;
+	else if (gmac_stat & MVNETA_GMAC_SPEED_100)
+		status->speed = SPEED_100;
+	else
+		status->speed = SPEED_10;
+	status->duplex = !!(gmac_stat & MVNETA_GMAC_FULL_DUPLEX);
+	return 0;
+}
+
 /* NAPI handler
  * Bits 0 - 7 of the causeRxTx register indicate that are transmitted
  * packets on the corresponding TXQ (Bit 0 is for TX queue 1).
@@ -2063,8 +2107,12 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
 	}

 	/* Read cause register */
-	cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE) &
-		(MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+	cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE);
+	if (cause_rx_tx & MVNETA_MISCINTR_INTR_MASK) {
+		/* since phylib uses polling, not much to do here...
+		 * Any way to tell phylib to do the poll NOW? */
+		mvreg_write(pp, MVNETA_INTR_MISC_CAUSE, 0);
+	}

 	/* Release Tx descriptors */
 	if (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL) {
@@ -2109,7 +2157,9 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
 		napi_complete(napi);
 		local_irq_save(flags);
 		mvreg_write(pp, MVNETA_INTR_NEW_MASK,
-			    MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+			    MVNETA_RX_INTR_MASK(rxq_number) |
+			    MVNETA_TX_INTR_MASK(txq_number) |
+			    MVNETA_MISCINTR_INTR_MASK);
 		local_irq_restore(flags);
 	}

@@ -2373,7 +2423,13 @@ static void mvneta_start_dev(struct mvneta_port *pp)

 	/* Unmask interrupts */
 	mvreg_write(pp, MVNETA_INTR_NEW_MASK,
-		    MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+		    MVNETA_RX_INTR_MASK(rxq_number) |
+		    MVNETA_TX_INTR_MASK(txq_number) |
+		    MVNETA_MISCINTR_INTR_MASK);
+	mvreg_write(pp, MVNETA_INTR_MISC_MASK,
+		    MVNETA_CAUSE_PHY_STATUS_CHANGE |
+		    MVNETA_CAUSE_LINK_CHANGE |
+		    MVNETA_CAUSE_PSC_SYNC_CHANGE);

 	phy_start(pp->phy_dev);
 	netif_tx_start_all_queues(pp->dev);
@@ -2523,9 +2579,7 @@ static void mvneta_adjust_link(struct net_device *ndev)
 			val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
 			val &= ~(MVNETA_GMAC_CONFIG_MII_SPEED |
 				 MVNETA_GMAC_CONFIG_GMII_SPEED |
-				 MVNETA_GMAC_CONFIG_FULL_DUPLEX |
-				 MVNETA_GMAC_AN_SPEED_EN |
-				 MVNETA_GMAC_AN_DUPLEX_EN);
+				 MVNETA_GMAC_CONFIG_FULL_DUPLEX);

 			if (phydev->duplex)
 				val |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
@@ -2554,12 +2608,24 @@ static void mvneta_adjust_link(struct net_device *ndev)

 	if (status_change) {
 		if (phydev->link) {
-			u32 val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
-			val |= (MVNETA_GMAC_FORCE_LINK_PASS |
-				MVNETA_GMAC_FORCE_LINK_DOWN);
-			mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+			if (!pp->inband_status) {
+				u32 val = mvreg_read(pp,
+						  MVNETA_GMAC_AUTONEG_CONFIG);
+				val &= ~MVNETA_GMAC_FORCE_LINK_DOWN;
+				val |= MVNETA_GMAC_FORCE_LINK_PASS;
+				mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+					    val);
+			}
 			mvneta_port_up(pp);
 		} else {
+			if (!pp->inband_status) {
+				u32 val = mvreg_read(pp,
+						  MVNETA_GMAC_AUTONEG_CONFIG);
+				val &= ~MVNETA_GMAC_FORCE_LINK_PASS;
+				val |= MVNETA_GMAC_FORCE_LINK_DOWN;
+				mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+					    val);
+			}
 			mvneta_port_down(pp);
 		}
 		phy_print_status(phydev);
@@ -2910,6 +2976,9 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 		return -EINVAL;
 	}

+	if (pp->inband_status)
+		ctrl |= MVNETA_GMAC2_INBAND_AN_ENABLE;
+
 	/* Cancel Port Reset */
 	ctrl &= ~MVNETA_GMAC2_PORT_RESET;
 	mvreg_write(pp, MVNETA_GMAC_CTRL_2, ctrl);
@@ -2934,6 +3003,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	char hw_mac_addr[ETH_ALEN];
 	const char *mac_from;
 	int phy_mode;
+	int fixed_phy = 0;
 	int err;

 	/* Our multiqueue support is not complete, so for now, only
@@ -2956,6 +3026,7 @@ static int mvneta_probe(struct platform_device *pdev)

 	phy_node = of_parse_phandle(dn, "phy", 0);
 	if (!phy_node) {
+		struct phy_device *phy;
 		if (!of_phy_is_fixed_link(dn)) {
 			dev_err(&pdev->dev, "no PHY specified\n");
 			err = -ENODEV;
@@ -2967,11 +3038,14 @@ static int mvneta_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "cannot register fixed PHY\n");
 			goto err_free_irq;
 		}
+		fixed_phy = 1;

 		/* In the case of a fixed PHY, the DT node associated
 		 * to the PHY is the Ethernet MAC DT node.
 		 */
 		phy_node = of_node_get(dn);
+		phy = of_phy_find_device(dn);
+		fixed_phy_set_link_update(phy, mvneta_fixed_link_update);
 	}

 	phy_mode = of_get_phy_mode(dn);
@@ -2990,6 +3064,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	pp = netdev_priv(dev);
 	pp->phy_node = phy_node;
 	pp->phy_interface = phy_mode;
+	pp->inband_status = (phy_mode == PHY_INTERFACE_MODE_SGMII) && fixed_phy;

 	pp->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pp->clk)) {
-- 
1.7.9.5

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Florian Fainelli]

2015-03-30 7:39 GMT-07:00 Stas Sergeev <stsp@list.ru>:
> 27.03.2015 20:15, Florian Fainelli пишет:
>> I think your concerns are valid, but I don't think there is going to be
>> any problem with the approach I suggested because there is a contract
>> that the fixed PHYs and regular PHYs need to
> Hello Florian.
>
> As promised, today I tried to resurrect my first implementation
> and do things as you suggested: install the link_update callback
> for mvneta privately.
> I feel SGMII setup is very common and deserves the separate API,
> not the per-driver handling, but in any case, I'd like to show
> the implementation first, then discuss.
>
> Unfortunately, it didn't work quite right.
> The problem is that mvneta calls phy_disconnect() on .ndo_stop
> callback. After that, phy->attached_dev becomes NULL, and so the
> link_update callback gets called with net_dev==NULL! And crashs.
> Of course I can easily work around that, but IMHO its a bug -
> the one that actually gets fixed by the patches I posted previously.

I actually submitted some patches a while ago that allow you to
unregister the fixed_link_update callback before in case you need to,
precisely for that. Since this is specific to dealing with a fixed
PHY, it is the driver responsibility to know that is has registered a
fixed_link_update callback and then unregister it by passing a NULL
callback as the new callback.

So yes, it is a bug in the sense that it is not transparently handled,
but at the same time, the PHY library has no way to know whether a
fixed_link_update callback is being invoked since it is not poking
into the fixed PHY driver.


> They were changing the callback to receive phy_device instead of
> net_device, and so NULL will never be passed.

Ok, but then you may still poll a PHY which has no network device
attached to it, so that leads to worse drivers which might not be
doing the right thing wrt. power management or other things.

> I am attaching the new patch so that you can take a look and decide.
> If you still think its fine, even despite the NULL passing w/a, then
> I'll mail it with the proper boilerplate. But if you agree that
> passing NULL to link_update is a bug, then maybe you'll decide to
> get the whole surgery thing. :)

[snip]

> @@ -304,6 +310,7 @@ struct mvneta_port {
>         unsigned int link;
>         unsigned int duplex;
>         unsigned int speed;
> +       int inband_status;

Since you are essentially using this variable as a boolean to indicate
whether in-band status should be queried or not, maybe you should name
that "needs_inband_status" or "wants_inband_status", inband_status
would suggest that this is the data-structure holding all the in-band
status information, but that's pure nitpicking.
--
Florian

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

30.03.2015 19:06, Florian Fainelli пишет:
> So yes, it is a bug in the sense that it is not transparently handled,
> but at the same time, the PHY library has no way to know whether a
> fixed_link_update callback is being invoked since it is not poking
> into the fixed PHY driver.
Maybe then it would be better to have an API in fixed_phy.c itself
to change the state? No one will then care about the callback at all.
For example, currently in my new patch, when I receive the interrupt
about phy status change, I can't do anything: I can't change the state
or force the phylib to do a callback right now. So, instead of changing
the state upon interrupt, I need instead to ask HW the current state on
every poll, which is likely not effective in some regards. By having the
API that would be solved, as well as the detached device problem.

>> @@ -304,6 +310,7 @@ struct mvneta_port {
>>         unsigned int link;
>>         unsigned int duplex;
>>         unsigned int speed;
>> +       int inband_status;
> Since you are essentially using this variable as a boolean to indicate
> whether in-band status should be queried or not, maybe you should name
> that "needs_inband_status" or "wants_inband_status", inband_status
That's fine, will go for "use_inband_status:1" tomorrow.

Re: [PATCH 4/6] of: add API for changing parameters of fixed link

[Stas Sergeev]

30.03.2015 19:06, Florian Fainelli пишет:
> 2015-03-30 7:39 GMT-07:00 Stas Sergeev <stsp@list.ru>:
>> 27.03.2015 20:15, Florian Fainelli пишет:
>>> I think your concerns are valid, but I don't think there is going to be
>>> any problem with the approach I suggested because there is a contract
>>> that the fixed PHYs and regular PHYs need to
>> Hello Florian.
>>
>> As promised, today I tried to resurrect my first implementation
>> and do things as you suggested: install the link_update callback
>> for mvneta privately.
>> I feel SGMII setup is very common and deserves the separate API,
>> not the per-driver handling, but in any case, I'd like to show
>> the implementation first, then discuss.
>>
>> Unfortunately, it didn't work quite right.
>> The problem is that mvneta calls phy_disconnect() on .ndo_stop
>> callback. After that, phy->attached_dev becomes NULL, and so the
>> link_update callback gets called with net_dev==NULL! And crashs.
>> Of course I can easily work around that, but IMHO its a bug -
>> the one that actually gets fixed by the patches I posted previously.
> 
> I actually submitted some patches a while ago that allow you to
> unregister the fixed_link_update callback before in case you need to,
> precisely for that. Since this is specific to dealing with a fixed
> PHY, it is the driver responsibility to know that is has registered a
> fixed_link_update callback and then unregister it by passing a NULL
> callback as the new callback.
Today I posted the patch that does exactly that.
But I already see the problem:
---
[   10.535424] mvneta f1030000.ethernet eth1: Link is Up - Unsupported (update phy.c)/Half - flow control off
---

Seems like if I register a callback after of_phy_connect()
(and unregister before disconnect), there is a small window
between connect and setting the callback. If someone at that
time will query phy, he'll get an undefined status (speed).
And before of_phy_connect() the phy_device pointer is not known
to register callback earlier. Of course I can see the possible
work-arounds, but I wonder if something better can be done
than using of_phy_find_device() and related magic right before
of_phy_connect() just for that.