[PATCH v4 1/3] net: emac: implement 802.1Q VLAN TX tagging support

[Christian Lamparter]

As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
VLAN:
 - Support for VLAN tag ID in compliance with IEEE 802.3ac.
 - VLAN tag insertion or replacement for transmit packets

This patch completes the missing code for the VLAN tx tagging
support, as the the EMAC_MR1_VLE was already enabled.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 32 
 drivers/net/ethernet/ibm/emac/core.h |  6 +-
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 760b2ad8e295..be560f9031f4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
 ndev->dev_addr[5]);
 
/* VLAN Tag Protocol ID */
-   out_be32(>vtpid, 0x8100);
+   out_be32(>vtpid, ETH_P_8021Q);
 
/* Receive mode register */
r = emac_iff2rmr(ndev);
@@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct 
emac_instance *dev, int len)
return NETDEV_TX_OK;
 }
 
+static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
+{
+   /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
+   skb_vlan_tag_present(skb)) {
+   struct emac_regs __iomem *p = dev->emacp;
+
+   /* update the VLAN TCI */
+   out_be32(>vtci, (u32)skb_vlan_tag_get(skb));
+
+   /* Insert VLAN tag */
+   return EMAC_TX_CTRL_IVT;
+   }
+   return 0;
+}
+
 /* Tx lock BH */
 static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device 
*ndev)
 {
@@ -1443,7 +1460,7 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, 
struct net_device *ndev)
int slot;
 
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb);
+   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
 
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
@@ -1518,7 +1535,7 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device 
*ndev)
goto stop_queue;
 
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   emac_tx_csum(dev, skb);
+   emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot;
 
/* skb data */
@@ -2891,7 +2908,8 @@ static int emac_init_config(struct emac_instance *dev)
if (of_device_is_compatible(np, "ibm,emac-apm821xx")) {
dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
  EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
- EMAC_FTR_460EX_PHY_CLK_FIX);
+ EMAC_FTR_460EX_PHY_CLK_FIX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX);
}
} else if (of_device_is_compatible(np, "ibm,emac4")) {
dev->features |= EMAC_FTR_EMAC4;
@@ -3148,6 +3166,12 @@ static int emac_probe(struct platform_device *ofdev)
 
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
+   ndev->vlan_features |= ndev->hw_features;
+   ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+   }
+
ndev->features |= ndev->hw_features | NETIF_F_RXCSUM;
}
ndev->watchdog_timeo = 5 * HZ;
diff --git a/drivers/net/ethernet/ibm/emac/core.h 
b/drivers/net/ethernet/ibm/emac/core.h
index 84caa4a3fc52..8d84d439168c 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -334,6 +334,8 @@ struct emac_instance {
  * APM821xx does not support Half Duplex mode
  */
 #define EMAC_FTR_APM821XX_NO_HALF_DUPLEX   0x1000
+/* EMAC can insert 802.1Q tag */
+#define EMAC_FTR_HAS_VLAN_CTAG_TX  0x2000
 
 /* Right now, we don't quite handle the always/possible masks on the
  * most optimal way as we don't have a way to say something like
@@ -363,7 +365,9 @@ enum {
EMAC_FTR_460EX_PHY_CLK_FIX |
EMAC_FTR_440EP_PHY_CLK_FIX |
EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
-   EMAC_FTR_APM821XX_NO_HALF_DUPLEX,
+   EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
+   EMAC_FTR_HAS_VLAN_CTAG_TX |
+   0,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
-- 
2.19.1

[PATCH v4 2/3] net: emac: implement TCP segmentation offload (TSO)

[Christian Lamparter]

This patch enables TSO(v4) hw feature for emac driver.
As atleast the APM82181's TCP/IP acceleration hardware
controller (TAH) provides TCP segmentation support in
the transmit path.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 112 ++-
 drivers/net/ethernet/ibm/emac/core.h |   7 ++
 drivers/net/ethernet/ibm/emac/emac.h |   7 ++
 drivers/net/ethernet/ibm/emac/tah.c  |  22 +-
 drivers/net/ethernet/ibm/emac/tah.h  |   2 +
 5 files changed, 148 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index be560f9031f4..80aafd7552aa 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1118,6 +1121,32 @@ static int emac_resize_rx_ring(struct emac_instance 
*dev, int new_mtu)
return ret;
 }
 
+/* Restriction applied for the segmentation size
+ * to use HW segmentation offload feature. the size
+ * of the segment must not be less than 168 bytes for
+ * DIX formatted segments, or 176 bytes for
+ * IEEE formatted segments. However based on actual
+ * tests any MTU less than 416 causes excessive retries
+ * due to TX FIFO underruns.
+ */
+const u32 tah_ss[TAH_NO_SSR] = { 1500, 1344, 1152, 960, 768, 416 };
+
+/* look-up matching segment size for the given mtu */
+static void emac_find_tso_ss_for_mtu(struct emac_instance *dev)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] <= dev->ndev->mtu)
+   break;
+   }
+   /* if no matching segment size is found, set the tso_ss_mtu_start
+* variable anyway. This will cause the emac_tx_tso to skip straight
+* to the software fallback.
+*/
+   dev->tso_ss_mtu_start = i;
+}
+
 /* Process ctx, rtnl_lock semaphore */
 static int emac_change_mtu(struct net_device *ndev, int new_mtu)
 {
@@ -1134,6 +1163,7 @@ static int emac_change_mtu(struct net_device *ndev, int 
new_mtu)
 
if (!ret) {
ndev->mtu = new_mtu;
+   emac_find_tso_ss_for_mtu(dev);
dev->rx_skb_size = emac_rx_skb_size(new_mtu);
dev->rx_sync_size = emac_rx_sync_size(new_mtu);
}
@@ -1410,6 +1440,33 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
 }
 
+static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
+  u16 *ctrl)
+{
+   if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) && skb_is_gso(skb) &&
+   !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
+   u32 seg_size = 0, i;
+
+   /* Get the MTU */
+   seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb) +
+  skb_network_header_len(skb);
+
+   for (i = dev->tso_ss_mtu_start; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] > seg_size)
+   continue;
+
+   *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
+   return 0;
+   }
+
+   /* none found fall back to software */
+   return -EINVAL;
+   }
+
+   *ctrl |= emac_tx_csum(dev, skb);
+   return 0;
+}
+
 static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
 {
struct emac_regs __iomem *p = dev->emacp;
@@ -1452,6 +1509,46 @@ static inline u16 emac_tx_vlan(struct emac_instance 
*dev, struct sk_buff *skb)
return 0;
 }
 
+static netdev_tx_t
+emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev);
+
+static int
+emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct sk_buff *segs, *curr;
+   unsigned int i, frag_slots;
+
+   /* make sure to not overflow the tx ring */
+   frag_slots = dev->tx_cnt;
+   for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+   struct skb_frag_struct *frag = _shinfo(skb)->frags[i];
+
+   frag_slots += mal_tx_chunks(skb_frag_size(frag));
+
+   if (frag_slots >= NUM_TX_BUFF)
+   return -ENOSPC;
+   };
+
+   segs = skb_gso_segment(skb, ndev->features &
+   ~(NETIF_F_TSO | NETIF_F_TSO6));
+   if (IS_ERR_OR_NULL(segs)) {
+   ++dev->estats.tx_dropped;
+   dev_kfree_skb_any(skb);
+   } else {
+   while (segs) {
+   curr = segs;
+   segs = curr->next;
+   curr->next = NULL;
+
+   emac_start_xmit_sg(curr, ndev);
+   }
+   dev_consume_skb_any(skb);
+   }
+
+   return 0;
+}
+
 /* Tx

[PATCH v4 3/3] net: emac: remove IBM_EMAC_RX_SKB_HEADROOM

[Christian Lamparter]

The EMAC driver had a custom IBM_EMAC_RX_SKB_HEADROOM
Kconfig option that reserved additional skb headroom for RX.
This patch removes the option and migrates the code
to use napi_alloc_skb() and netdev_alloc_skb_ip_align()
in its place.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/Kconfig | 12 --
 drivers/net/ethernet/ibm/emac/core.c  | 57 +++
 drivers/net/ethernet/ibm/emac/core.h  | 10 ++---
 3 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/Kconfig 
b/drivers/net/ethernet/ibm/emac/Kconfig
index 90d49191beb3..eacf7e141fdc 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -28,18 +28,6 @@ config IBM_EMAC_RX_COPY_THRESHOLD
depends on IBM_EMAC
default "256"
 
-config IBM_EMAC_RX_SKB_HEADROOM
-   int "Additional RX skb headroom (bytes)"
-   depends on IBM_EMAC
-   default "0"
-   help
- Additional receive skb headroom. Note, that driver
- will always reserve at least 2 bytes to make IP header
- aligned, so usually there is no need to add any additional
- headroom.
-
- If unsure, set to 0.
-
 config IBM_EMAC_DEBUG
bool "Debugging"
depends on IBM_EMAC
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 80aafd7552aa..266b6614125b 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1075,7 +1075,9 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
 
/* Second pass, allocate new skbs */
for (i = 0; i < NUM_RX_BUFF; ++i) {
-   struct sk_buff *skb = alloc_skb(rx_skb_size, GFP_ATOMIC);
+   struct sk_buff *skb;
+
+   skb = netdev_alloc_skb_ip_align(dev->ndev, rx_skb_size);
if (!skb) {
ret = -ENOMEM;
goto oom;
@@ -1084,7 +1086,6 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
BUG_ON(!dev->rx_skb[i]);
dev_kfree_skb(dev->rx_skb[i]);
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[i].data_ptr =
dma_map_single(>ofdev->dev, skb->data - 2, 
rx_sync_size,
   DMA_FROM_DEVICE) + 2;
@@ -1205,20 +1206,18 @@ static void emac_clean_rx_ring(struct emac_instance 
*dev)
}
 }
 
-static inline int emac_alloc_rx_skb(struct emac_instance *dev, int slot,
-   gfp_t flags)
+static inline int
+__emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
 {
-   struct sk_buff *skb = alloc_skb(dev->rx_skb_size, flags);
if (unlikely(!skb))
return -ENOMEM;
 
dev->rx_skb[slot] = skb;
dev->rx_desc[slot].data_len = 0;
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[slot].data_ptr =
-   dma_map_single(>ofdev->dev, skb->data - 2, dev->rx_sync_size,
-  DMA_FROM_DEVICE) + 2;
+   dma_map_single(>ofdev->dev, skb->data - NET_IP_ALIGN,
+  dev->rx_sync_size, DMA_FROM_DEVICE) + NET_IP_ALIGN;
wmb();
dev->rx_desc[slot].ctrl = MAL_RX_CTRL_EMPTY |
(slot == (NUM_RX_BUFF - 1) ? MAL_RX_CTRL_WRAP : 0);
@@ -1226,6 +1225,27 @@ static inline int emac_alloc_rx_skb(struct emac_instance 
*dev, int slot,
return 0;
 }
 
+static inline int
+emac_alloc_rx_skb(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = __netdev_alloc_skb_ip_align(dev->ndev, dev->rx_skb_size,
+ GFP_KERNEL);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
+static inline int
+emac_alloc_rx_skb_napi(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = napi_alloc_skb(>mal->napi, dev->rx_skb_size);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
 static void emac_print_link_status(struct emac_instance *dev)
 {
if (netif_carrier_ok(dev->ndev))
@@ -1256,7 +1276,7 @@ static int emac_open(struct net_device *ndev)
 
/* Allocate RX ring */
for (i = 0; i < NUM_RX_BUFF; ++i)
-   if (emac_alloc_rx_skb(dev, i, GFP_KERNEL)) {
+   if (emac_alloc_rx_skb(dev, i)) {
printk(KERN_ERR "%s: failed to allocate RX ring\n",
   ndev->name);
goto oom;
@@ -1779,8 +1799,9 @@ static inline void emac_recycle_rx_skb(struct 
emac_instance *dev, int slot,
DBG2(dev, "recycle %d %d" NL, slot, len);
 
if (len)
-   dma_map_single(>ofdev->dev, skb-

[PATCH v3 3/4] net: emac: remove IBM_EMAC_RX_SKB_HEADROOM

[Christian Lamparter]

The EMAC driver had a custom IBM_EMAC_RX_SKB_HEADROOM
Kconfig option that reserved additional skb headroom for RX.
This patch removes the option and migrates the code
to use napi_alloc_skb() and netdev_alloc_skb_ip_align()
in its place.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/Kconfig | 12 --
 drivers/net/ethernet/ibm/emac/core.c  | 57 +++
 drivers/net/ethernet/ibm/emac/core.h  | 10 ++---
 3 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/Kconfig 
b/drivers/net/ethernet/ibm/emac/Kconfig
index 90d49191beb3..eacf7e141fdc 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -28,18 +28,6 @@ config IBM_EMAC_RX_COPY_THRESHOLD
depends on IBM_EMAC
default "256"
 
-config IBM_EMAC_RX_SKB_HEADROOM
-   int "Additional RX skb headroom (bytes)"
-   depends on IBM_EMAC
-   default "0"
-   help
- Additional receive skb headroom. Note, that driver
- will always reserve at least 2 bytes to make IP header
- aligned, so usually there is no need to add any additional
- headroom.
-
- If unsure, set to 0.
-
 config IBM_EMAC_DEBUG
bool "Debugging"
depends on IBM_EMAC
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 80aafd7552aa..266b6614125b 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1075,7 +1075,9 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
 
/* Second pass, allocate new skbs */
for (i = 0; i < NUM_RX_BUFF; ++i) {
-   struct sk_buff *skb = alloc_skb(rx_skb_size, GFP_ATOMIC);
+   struct sk_buff *skb;
+
+   skb = netdev_alloc_skb_ip_align(dev->ndev, rx_skb_size);
if (!skb) {
ret = -ENOMEM;
goto oom;
@@ -1084,7 +1086,6 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
BUG_ON(!dev->rx_skb[i]);
dev_kfree_skb(dev->rx_skb[i]);
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[i].data_ptr =
dma_map_single(>ofdev->dev, skb->data - 2, 
rx_sync_size,
   DMA_FROM_DEVICE) + 2;
@@ -1205,20 +1206,18 @@ static void emac_clean_rx_ring(struct emac_instance 
*dev)
}
 }
 
-static inline int emac_alloc_rx_skb(struct emac_instance *dev, int slot,
-   gfp_t flags)
+static inline int
+__emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
 {
-   struct sk_buff *skb = alloc_skb(dev->rx_skb_size, flags);
if (unlikely(!skb))
return -ENOMEM;
 
dev->rx_skb[slot] = skb;
dev->rx_desc[slot].data_len = 0;
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[slot].data_ptr =
-   dma_map_single(>ofdev->dev, skb->data - 2, dev->rx_sync_size,
-  DMA_FROM_DEVICE) + 2;
+   dma_map_single(>ofdev->dev, skb->data - NET_IP_ALIGN,
+  dev->rx_sync_size, DMA_FROM_DEVICE) + NET_IP_ALIGN;
wmb();
dev->rx_desc[slot].ctrl = MAL_RX_CTRL_EMPTY |
(slot == (NUM_RX_BUFF - 1) ? MAL_RX_CTRL_WRAP : 0);
@@ -1226,6 +1225,27 @@ static inline int emac_alloc_rx_skb(struct emac_instance 
*dev, int slot,
return 0;
 }
 
+static inline int
+emac_alloc_rx_skb(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = __netdev_alloc_skb_ip_align(dev->ndev, dev->rx_skb_size,
+ GFP_KERNEL);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
+static inline int
+emac_alloc_rx_skb_napi(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = napi_alloc_skb(>mal->napi, dev->rx_skb_size);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
 static void emac_print_link_status(struct emac_instance *dev)
 {
if (netif_carrier_ok(dev->ndev))
@@ -1256,7 +1276,7 @@ static int emac_open(struct net_device *ndev)
 
/* Allocate RX ring */
for (i = 0; i < NUM_RX_BUFF; ++i)
-   if (emac_alloc_rx_skb(dev, i, GFP_KERNEL)) {
+   if (emac_alloc_rx_skb(dev, i)) {
printk(KERN_ERR "%s: failed to allocate RX ring\n",
   ndev->name);
goto oom;
@@ -1779,8 +1799,9 @@ static inline void emac_recycle_rx_skb(struct 
emac_instance *dev, int slot,
DBG2(dev, "recycle %d %d" NL, slot, len);
 
if (len)
-   dma_map_single(>ofdev->dev, skb-

[PATCH v3 2/4] net: emac: implement TCP segmentation offload (TSO)

[Christian Lamparter]

This patch enables TSO(v4) hw feature for emac driver.
As atleast the APM82181's TCP/IP acceleration hardware
controller (TAH) provides TCP segmentation support in
the transmit path.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 112 ++-
 drivers/net/ethernet/ibm/emac/core.h |   7 ++
 drivers/net/ethernet/ibm/emac/emac.h |   7 ++
 drivers/net/ethernet/ibm/emac/tah.c  |  22 +-
 drivers/net/ethernet/ibm/emac/tah.h  |   2 +
 5 files changed, 148 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index be560f9031f4..80aafd7552aa 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1118,6 +1121,32 @@ static int emac_resize_rx_ring(struct emac_instance 
*dev, int new_mtu)
return ret;
 }
 
+/* Restriction applied for the segmentation size
+ * to use HW segmentation offload feature. the size
+ * of the segment must not be less than 168 bytes for
+ * DIX formatted segments, or 176 bytes for
+ * IEEE formatted segments. However based on actual
+ * tests any MTU less than 416 causes excessive retries
+ * due to TX FIFO underruns.
+ */
+const u32 tah_ss[TAH_NO_SSR] = { 1500, 1344, 1152, 960, 768, 416 };
+
+/* look-up matching segment size for the given mtu */
+static void emac_find_tso_ss_for_mtu(struct emac_instance *dev)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] <= dev->ndev->mtu)
+   break;
+   }
+   /* if no matching segment size is found, set the tso_ss_mtu_start
+* variable anyway. This will cause the emac_tx_tso to skip straight
+* to the software fallback.
+*/
+   dev->tso_ss_mtu_start = i;
+}
+
 /* Process ctx, rtnl_lock semaphore */
 static int emac_change_mtu(struct net_device *ndev, int new_mtu)
 {
@@ -1134,6 +1163,7 @@ static int emac_change_mtu(struct net_device *ndev, int 
new_mtu)
 
if (!ret) {
ndev->mtu = new_mtu;
+   emac_find_tso_ss_for_mtu(dev);
dev->rx_skb_size = emac_rx_skb_size(new_mtu);
dev->rx_sync_size = emac_rx_sync_size(new_mtu);
}
@@ -1410,6 +1440,33 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
 }
 
+static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
+  u16 *ctrl)
+{
+   if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) && skb_is_gso(skb) &&
+   !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
+   u32 seg_size = 0, i;
+
+   /* Get the MTU */
+   seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb) +
+  skb_network_header_len(skb);
+
+   for (i = dev->tso_ss_mtu_start; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] > seg_size)
+   continue;
+
+   *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
+   return 0;
+   }
+
+   /* none found fall back to software */
+   return -EINVAL;
+   }
+
+   *ctrl |= emac_tx_csum(dev, skb);
+   return 0;
+}
+
 static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
 {
struct emac_regs __iomem *p = dev->emacp;
@@ -1452,6 +1509,46 @@ static inline u16 emac_tx_vlan(struct emac_instance 
*dev, struct sk_buff *skb)
return 0;
 }
 
+static netdev_tx_t
+emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev);
+
+static int
+emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct sk_buff *segs, *curr;
+   unsigned int i, frag_slots;
+
+   /* make sure to not overflow the tx ring */
+   frag_slots = dev->tx_cnt;
+   for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+   struct skb_frag_struct *frag = _shinfo(skb)->frags[i];
+
+   frag_slots += mal_tx_chunks(skb_frag_size(frag));
+
+   if (frag_slots >= NUM_TX_BUFF)
+   return -ENOSPC;
+   };
+
+   segs = skb_gso_segment(skb, ndev->features &
+   ~(NETIF_F_TSO | NETIF_F_TSO6));
+   if (IS_ERR_OR_NULL(segs)) {
+   ++dev->estats.tx_dropped;
+   dev_kfree_skb_any(skb);
+   } else {
+   while (segs) {
+   curr = segs;
+   segs = curr->next;
+   curr->next = NULL;
+
+   emac_start_xmit_sg(curr, ndev);
+   }
+   dev_consume_skb_any(skb);
+   }
+
+   return 0;
+}
+
 /* Tx

[PATCH v3 4/4] net: emac: add deprecation notice to emac custom phy users

[Christian Lamparter]

This patch starts the deprecation process of emac's small library of
supported phys by adding a message to inform all remaining users to
start looking into converting their platform's device-tree to PHYLIB.

EMAC's phy.c support is limited to mostly single ethernet transceivers:
CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.

And Linux has dedicated PHYLIB drivers for all but the BCM5248 which
can be supported by the generic phy driver.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/phy.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ibm/emac/phy.c 
b/drivers/net/ethernet/ibm/emac/phy.c
index aa070c063e48..143b4c688ee9 100644
--- a/drivers/net/ethernet/ibm/emac/phy.c
+++ b/drivers/net/ethernet/ibm/emac/phy.c
@@ -496,6 +496,7 @@ static struct mii_phy_def ar8035_phy_def = {
 };
 
 static struct mii_phy_def *mii_phy_table[] = {
+   /* DEPRECATED: Do not add any new PHY drivers to this list. */
_phy_def,
_phy_def,
_phy_def,
@@ -512,6 +513,9 @@ int emac_mii_phy_probe(struct mii_phy *phy, int address)
int i;
u32 id;
 
+   pr_info("EMAC's custom phy code has been deprecated.\n"
+   "Please convert your EMAC device to PHYLIB.\n");
+
phy->autoneg = AUTONEG_DISABLE;
phy->advertising = 0;
phy->address = address;
-- 
2.19.1

[PATCH v3 1/4] net: emac: implement 802.1Q VLAN TX tagging support

[Christian Lamparter]

As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
VLAN:
 - Support for VLAN tag ID in compliance with IEEE 802.3ac.
 - VLAN tag insertion or replacement for transmit packets

This patch completes the missing code for the VLAN tx tagging
support, as the the EMAC_MR1_VLE was already enabled.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 32 
 drivers/net/ethernet/ibm/emac/core.h |  6 +-
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 760b2ad8e295..be560f9031f4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
 ndev->dev_addr[5]);
 
/* VLAN Tag Protocol ID */
-   out_be32(>vtpid, 0x8100);
+   out_be32(>vtpid, ETH_P_8021Q);
 
/* Receive mode register */
r = emac_iff2rmr(ndev);
@@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct 
emac_instance *dev, int len)
return NETDEV_TX_OK;
 }
 
+static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
+{
+   /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
+   skb_vlan_tag_present(skb)) {
+   struct emac_regs __iomem *p = dev->emacp;
+
+   /* update the VLAN TCI */
+   out_be32(>vtci, (u32)skb_vlan_tag_get(skb));
+
+   /* Insert VLAN tag */
+   return EMAC_TX_CTRL_IVT;
+   }
+   return 0;
+}
+
 /* Tx lock BH */
 static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device 
*ndev)
 {
@@ -1443,7 +1460,7 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, 
struct net_device *ndev)
int slot;
 
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb);
+   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
 
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
@@ -1518,7 +1535,7 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device 
*ndev)
goto stop_queue;
 
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   emac_tx_csum(dev, skb);
+   emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot;
 
/* skb data */
@@ -2891,7 +2908,8 @@ static int emac_init_config(struct emac_instance *dev)
if (of_device_is_compatible(np, "ibm,emac-apm821xx")) {
dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
  EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
- EMAC_FTR_460EX_PHY_CLK_FIX);
+ EMAC_FTR_460EX_PHY_CLK_FIX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX);
}
} else if (of_device_is_compatible(np, "ibm,emac4")) {
dev->features |= EMAC_FTR_EMAC4;
@@ -3148,6 +3166,12 @@ static int emac_probe(struct platform_device *ofdev)
 
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
+   ndev->vlan_features |= ndev->hw_features;
+   ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+   }
+
ndev->features |= ndev->hw_features | NETIF_F_RXCSUM;
}
ndev->watchdog_timeo = 5 * HZ;
diff --git a/drivers/net/ethernet/ibm/emac/core.h 
b/drivers/net/ethernet/ibm/emac/core.h
index 84caa4a3fc52..8d84d439168c 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -334,6 +334,8 @@ struct emac_instance {
  * APM821xx does not support Half Duplex mode
  */
 #define EMAC_FTR_APM821XX_NO_HALF_DUPLEX   0x1000
+/* EMAC can insert 802.1Q tag */
+#define EMAC_FTR_HAS_VLAN_CTAG_TX  0x2000
 
 /* Right now, we don't quite handle the always/possible masks on the
  * most optimal way as we don't have a way to say something like
@@ -363,7 +365,9 @@ enum {
EMAC_FTR_460EX_PHY_CLK_FIX |
EMAC_FTR_440EP_PHY_CLK_FIX |
EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
-   EMAC_FTR_APM821XX_NO_HALF_DUPLEX,
+   EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
+   EMAC_FTR_HAS_VLAN_CTAG_TX |
+   0,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
-- 
2.19.1

[PATCH v2 1/4] net: emac: implement 802.1Q VLAN TX tagging support

[Christian Lamparter]

As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
VLAN:
 - Support for VLAN tag ID in compliance with IEEE 802.3ac.
 - VLAN tag insertion or replacement for transmit packets

This patch completes the missing code for the VLAN tx tagging
support, as the the EMAC_MR1_VLE was already enabled.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 32 
 drivers/net/ethernet/ibm/emac/core.h |  6 +-
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 760b2ad8e295..be560f9031f4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
 ndev->dev_addr[5]);
 
/* VLAN Tag Protocol ID */
-   out_be32(>vtpid, 0x8100);
+   out_be32(>vtpid, ETH_P_8021Q);
 
/* Receive mode register */
r = emac_iff2rmr(ndev);
@@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct 
emac_instance *dev, int len)
return NETDEV_TX_OK;
 }
 
+static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
+{
+   /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
+   skb_vlan_tag_present(skb)) {
+   struct emac_regs __iomem *p = dev->emacp;
+
+   /* update the VLAN TCI */
+   out_be32(>vtci, (u32)skb_vlan_tag_get(skb));
+
+   /* Insert VLAN tag */
+   return EMAC_TX_CTRL_IVT;
+   }
+   return 0;
+}
+
 /* Tx lock BH */
 static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device 
*ndev)
 {
@@ -1443,7 +1460,7 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, 
struct net_device *ndev)
int slot;
 
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb);
+   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
 
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
@@ -1518,7 +1535,7 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device 
*ndev)
goto stop_queue;
 
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   emac_tx_csum(dev, skb);
+   emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot;
 
/* skb data */
@@ -2891,7 +2908,8 @@ static int emac_init_config(struct emac_instance *dev)
if (of_device_is_compatible(np, "ibm,emac-apm821xx")) {
dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
  EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
- EMAC_FTR_460EX_PHY_CLK_FIX);
+ EMAC_FTR_460EX_PHY_CLK_FIX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX);
}
} else if (of_device_is_compatible(np, "ibm,emac4")) {
dev->features |= EMAC_FTR_EMAC4;
@@ -3148,6 +3166,12 @@ static int emac_probe(struct platform_device *ofdev)
 
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
+   ndev->vlan_features |= ndev->hw_features;
+   ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+   }
+
ndev->features |= ndev->hw_features | NETIF_F_RXCSUM;
}
ndev->watchdog_timeo = 5 * HZ;
diff --git a/drivers/net/ethernet/ibm/emac/core.h 
b/drivers/net/ethernet/ibm/emac/core.h
index 84caa4a3fc52..8d84d439168c 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -334,6 +334,8 @@ struct emac_instance {
  * APM821xx does not support Half Duplex mode
  */
 #define EMAC_FTR_APM821XX_NO_HALF_DUPLEX   0x1000
+/* EMAC can insert 802.1Q tag */
+#define EMAC_FTR_HAS_VLAN_CTAG_TX  0x2000
 
 /* Right now, we don't quite handle the always/possible masks on the
  * most optimal way as we don't have a way to say something like
@@ -363,7 +365,9 @@ enum {
EMAC_FTR_460EX_PHY_CLK_FIX |
EMAC_FTR_440EP_PHY_CLK_FIX |
EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
-   EMAC_FTR_APM821XX_NO_HALF_DUPLEX,
+   EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
+   EMAC_FTR_HAS_VLAN_CTAG_TX |
+   0,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
-- 
2.19.1

[PATCH v2 4/4] net: emac: add deprecation notice to emac custom phy users

[Christian Lamparter]

From: Christian Lamparter 

This patch starts the deprecation process of emac's small library of
supported phys by adding a message to inform all remaining users to
start looking into converting their platform's device-tree to PHYLIB.

EMAC's phy.c support is limited to mostly single ethernet transceivers:
CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.

And Linux has dedicated PHYLIB drivers for all but the BCM5248 which
can be supported by the generic phy driver.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/phy.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ibm/emac/phy.c 
b/drivers/net/ethernet/ibm/emac/phy.c
index aa070c063e48..143b4c688ee9 100644
--- a/drivers/net/ethernet/ibm/emac/phy.c
+++ b/drivers/net/ethernet/ibm/emac/phy.c
@@ -496,6 +496,7 @@ static struct mii_phy_def ar8035_phy_def = {
 };
 
 static struct mii_phy_def *mii_phy_table[] = {
+   /* DEPRECATED: Do not add any new PHY drivers to this list. */
_phy_def,
_phy_def,
_phy_def,
@@ -512,6 +513,9 @@ int emac_mii_phy_probe(struct mii_phy *phy, int address)
int i;
u32 id;
 
+   pr_info("EMAC's custom phy code has been deprecated.\n"
+   "Please convert your EMAC device to PHYLIB.\n");
+
phy->autoneg = AUTONEG_DISABLE;
phy->advertising = 0;
phy->address = address;
-- 
2.19.1

[PATCH v2 2/4] net: emac: implement TCP segmentation offload (TSO)

[Christian Lamparter]

This patch enables TSO(v4) hw feature for emac driver.
As atleast the APM82181's TCP/IP acceleration hardware
controller (TAH) provides TCP segmentation support in
the transmit path.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 113 ++-
 drivers/net/ethernet/ibm/emac/core.h |   7 ++
 drivers/net/ethernet/ibm/emac/emac.h |   7 ++
 drivers/net/ethernet/ibm/emac/tah.c  |  22 +-
 drivers/net/ethernet/ibm/emac/tah.h  |   2 +
 5 files changed, 148 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index be560f9031f4..b5c4b7d3057d 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1118,6 +1121,32 @@ static int emac_resize_rx_ring(struct emac_instance 
*dev, int new_mtu)
return ret;
 }
 
+/* Restriction applied for the segmentation size
+ * to use HW segmentation offload feature. the size
+ * of the segment must not be less than 168 bytes for
+ * DIX formatted segments, or 176 bytes for
+ * IEEE formatted segments. However based on actual
+ * tests any MTU less than 416 causes excessive retries
+ * due to TX FIFO underruns.
+ */
+const u32 tah_ss[TAH_NO_SSR] = { 1500, 1344, 1152, 960, 768, 416 };
+
+/* look-up matching segment size for the given mtu */
+static void emac_find_tso_ss_for_mtu(struct emac_instance *dev)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] <= dev->ndev->mtu)
+   break;
+   }
+   /* if no matching segment size is found, set the tso_ss_mtu_start
+* variable anyway. This will cause the emac_tx_tso to skip straight
+* to the software fallback.
+*/
+   dev->tso_ss_mtu_start = i;
+}
+
 /* Process ctx, rtnl_lock semaphore */
 static int emac_change_mtu(struct net_device *ndev, int new_mtu)
 {
@@ -1134,6 +1163,7 @@ static int emac_change_mtu(struct net_device *ndev, int 
new_mtu)
 
if (!ret) {
ndev->mtu = new_mtu;
+   emac_find_tso_ss_for_mtu(dev);
dev->rx_skb_size = emac_rx_skb_size(new_mtu);
dev->rx_sync_size = emac_rx_sync_size(new_mtu);
}
@@ -1410,6 +1440,33 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
 }
 
+static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
+  u16 *ctrl)
+{
+   if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) && skb_is_gso(skb) &&
+   !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
+   u32 seg_size = 0, i;
+
+   /* Get the MTU */
+   seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb) +
+  skb_network_header_len(skb);
+
+   for (i = dev->tso_ss_mtu_start; i < ARRAY_SIZE(tah_ss); i++) {
+   if (tah_ss[i] > seg_size)
+   continue;
+
+   *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
+   return 0;
+   }
+
+   /* none found fall back to software */
+   return -EINVAL;
+   }
+
+   *ctrl |= emac_tx_csum(dev, skb);
+   return 0;
+}
+
 static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
 {
struct emac_regs __iomem *p = dev->emacp;
@@ -1452,8 +1509,49 @@ static inline u16 emac_tx_vlan(struct emac_instance 
*dev, struct sk_buff *skb)
return 0;
 }
 
+static netdev_tx_t
+emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev);
+
+static netdev_tx_t
+emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct sk_buff *segs, *curr;
+   unsigned int i, frag_slots;
+
+   /* make sure to not overflow the tx ring */
+   frag_slots = dev->tx_cnt;
+   for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+   struct skb_frag_struct *frag = _shinfo(skb)->frags[i];
+
+   frag_slots += mal_tx_chunks(skb_frag_size(frag));
+
+   if (frag_slots >= NUM_TX_BUFF)
+   return NETDEV_TX_BUSY;
+   };
+
+   segs = skb_gso_segment(skb, ndev->features &
+   ~(NETIF_F_TSO | NETIF_F_TSO6));
+   if (IS_ERR_OR_NULL(segs)) {
+   ++dev->estats.tx_dropped;
+   dev_kfree_skb_any(skb);
+   } else {
+   while (segs) {
+   curr = segs;
+   segs = curr->next;
+   curr->next = NULL;
+
+   emac_start_xmit_sg(curr, ndev);
+   }
+   dev_consume_skb_any(skb);
+   }
+
+   return

[PATCH v2 3/4] net: emac: remove IBM_EMAC_RX_SKB_HEADROOM

[Christian Lamparter]

The EMAC driver had a custom IBM_EMAC_RX_SKB_HEADROOM
Kconfig option that reserved additional skb headroom for RX.
This patch removes the option and migrates the code
to use napi_alloc_skb() and netdev_alloc_skb_ip_align()
in its place.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/Kconfig | 12 --
 drivers/net/ethernet/ibm/emac/core.c  | 57 +++
 drivers/net/ethernet/ibm/emac/core.h  | 10 ++---
 3 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/Kconfig 
b/drivers/net/ethernet/ibm/emac/Kconfig
index 90d49191beb3..eacf7e141fdc 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -28,18 +28,6 @@ config IBM_EMAC_RX_COPY_THRESHOLD
depends on IBM_EMAC
default "256"
 
-config IBM_EMAC_RX_SKB_HEADROOM
-   int "Additional RX skb headroom (bytes)"
-   depends on IBM_EMAC
-   default "0"
-   help
- Additional receive skb headroom. Note, that driver
- will always reserve at least 2 bytes to make IP header
- aligned, so usually there is no need to add any additional
- headroom.
-
- If unsure, set to 0.
-
 config IBM_EMAC_DEBUG
bool "Debugging"
depends on IBM_EMAC
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index b5c4b7d3057d..388443e08674 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1075,7 +1075,9 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
 
/* Second pass, allocate new skbs */
for (i = 0; i < NUM_RX_BUFF; ++i) {
-   struct sk_buff *skb = alloc_skb(rx_skb_size, GFP_ATOMIC);
+   struct sk_buff *skb;
+
+   skb = netdev_alloc_skb_ip_align(dev->ndev, rx_skb_size);
if (!skb) {
ret = -ENOMEM;
goto oom;
@@ -1084,7 +1086,6 @@ static int emac_resize_rx_ring(struct emac_instance *dev, 
int new_mtu)
BUG_ON(!dev->rx_skb[i]);
dev_kfree_skb(dev->rx_skb[i]);
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[i].data_ptr =
dma_map_single(>ofdev->dev, skb->data - 2, 
rx_sync_size,
   DMA_FROM_DEVICE) + 2;
@@ -1205,20 +1206,18 @@ static void emac_clean_rx_ring(struct emac_instance 
*dev)
}
 }
 
-static inline int emac_alloc_rx_skb(struct emac_instance *dev, int slot,
-   gfp_t flags)
+static inline int
+__emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
 {
-   struct sk_buff *skb = alloc_skb(dev->rx_skb_size, flags);
if (unlikely(!skb))
return -ENOMEM;
 
dev->rx_skb[slot] = skb;
dev->rx_desc[slot].data_len = 0;
 
-   skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[slot].data_ptr =
-   dma_map_single(>ofdev->dev, skb->data - 2, dev->rx_sync_size,
-  DMA_FROM_DEVICE) + 2;
+   dma_map_single(>ofdev->dev, skb->data - NET_IP_ALIGN,
+  dev->rx_sync_size, DMA_FROM_DEVICE) + NET_IP_ALIGN;
wmb();
dev->rx_desc[slot].ctrl = MAL_RX_CTRL_EMPTY |
(slot == (NUM_RX_BUFF - 1) ? MAL_RX_CTRL_WRAP : 0);
@@ -1226,6 +1225,27 @@ static inline int emac_alloc_rx_skb(struct emac_instance 
*dev, int slot,
return 0;
 }
 
+static inline int
+emac_alloc_rx_skb(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = __netdev_alloc_skb_ip_align(dev->ndev, dev->rx_skb_size,
+ GFP_KERNEL);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
+static inline int
+emac_alloc_rx_skb_napi(struct emac_instance *dev, int slot)
+{
+   struct sk_buff *skb;
+
+   skb = napi_alloc_skb(>mal->napi, dev->rx_skb_size);
+
+   return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
 static void emac_print_link_status(struct emac_instance *dev)
 {
if (netif_carrier_ok(dev->ndev))
@@ -1256,7 +1276,7 @@ static int emac_open(struct net_device *ndev)
 
/* Allocate RX ring */
for (i = 0; i < NUM_RX_BUFF; ++i)
-   if (emac_alloc_rx_skb(dev, i, GFP_KERNEL)) {
+   if (emac_alloc_rx_skb(dev, i)) {
printk(KERN_ERR "%s: failed to allocate RX ring\n",
   ndev->name);
goto oom;
@@ -1778,8 +1798,9 @@ static inline void emac_recycle_rx_skb(struct 
emac_instance *dev, int slot,
DBG2(dev, "recycle %d %d" NL, slot, len);
 
if (len)
-   dma_map_single(>ofdev->dev, skb-

Re: [PATCH 1/2] net: emac: implement 802.1Q VLAN TX tagging support

[Christian Lamparter]

On Wednesday, October 17, 2018 10:09:10 PM CEST Florian Fainelli wrote:
> On 10/17/2018 01:08 PM, Florian Fainelli wrote:
> > On 10/17/2018 12:53 PM, Christian Lamparter wrote:
> >> As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
> >> VLAN:
> >>  - Support for VLAN tag ID in compliance with IEEE 802.3ac.
> >>  - VLAN tag insertion or replacement for transmit packets
> >>
> >> This patch completes the missing code for the VLAN tx tagging
> >> support, as the the EMAC_MR1_VLE was already enabled.
> >>
> >> Signed-off-by: Christian Lamparter 
> >> ---
> >>  drivers/net/ethernet/ibm/emac/core.c | 32 
> >>  drivers/net/ethernet/ibm/emac/core.h |  6 +-
> >>  2 files changed, 33 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/ibm/emac/core.c 
> >> b/drivers/net/ethernet/ibm/emac/core.c
> >> index 760b2ad8e295..be560f9031f4 100644
> >> --- a/drivers/net/ethernet/ibm/emac/core.c
> >> +++ b/drivers/net/ethernet/ibm/emac/core.c
> >> @@ -37,6 +37,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
> >> ndev->dev_addr[5]);
> >>  
> >>/* VLAN Tag Protocol ID */
> >> -  out_be32(>vtpid, 0x8100);
> >> +  out_be32(>vtpid, ETH_P_8021Q);
> >>  
> >>/* Receive mode register */
> >>r = emac_iff2rmr(ndev);
> >> @@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct 
> >> emac_instance *dev, int len)
> >>return NETDEV_TX_OK;
> >>  }
> >>  
> >> +static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff 
> >> *skb)
> >> +{
> >> +  /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
> >> +  if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
> >> +  skb_vlan_tag_present(skb)) {
> >> +  struct emac_regs __iomem *p = dev->emacp;
> >> +
> >> +  /* update the VLAN TCI */
> >> +  out_be32(>vtci, (u32)skb_vlan_tag_get(skb));
> > 
> > The only case where this is likely not going to be 0x8100/ETH_P_8021Q is
> > if you do 802.1ad (QinQ) and you decided to somehow offload the S-Tag
> > instead of the C-Tag.
> 
> Sorry, looks like I mixed up TCI and TPID here, this looks obviously
> correct ;)

Ok, I wasn't really sure what to write anyway ;).

The hardware documentation mentions that:
"Support for VLAN tag ID in compliance with IEEE Draft 802.3ac/D1.0 standard".
It's too old for offloading any fancy QinQ stuff :(.

Re: [PATCH 2/2] net: emac: implement TCP TSO

[Christian Lamparter]

Hello,

On Wednesday, October 17, 2018 10:09:44 PM CEST Florian Fainelli wrote:
> On 10/17/2018 12:53 PM, Christian Lamparter wrote:
> > This patch enables TSO(v4) hw feature for emac driver.
> > As atleast the APM82181's TCP/IP acceleration hardware
> > controller (TAH) provides TCP segmentation support in
> > the transmit path.
> > 
> > Signed-off-by: Christian Lamparter 
> > ---
> > diff --git a/drivers/net/ethernet/ibm/emac/core.c 
> > b/drivers/net/ethernet/ibm/emac/core.c
> > index be560f9031f4..49ffbd6e1707 100644
> > --- a/drivers/net/ethernet/ibm/emac/core.c
> > +++ b/drivers/net/ethernet/ibm/emac/core.c
> > @@ -1410,6 +1413,52 @@ static inline u16 emac_tx_csum(struct emac_instance 
> > *dev,
> > return 0;
> >  }
> >  
> > +const u32 tah_ss[TAH_NO_SSR] = { 9000, 4500, 1500, 1300, 576, 176 };
> > +
> > +static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
> > +  u16 *ctrl)
> > +{
> > +   if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) &&
> > +   skb_is_gso(skb) && !!(skb_shinfo(skb)->gso_type &
> > +   (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
> > +   u32 seg_size = 0, i;
> > +
> > +   /* Get the MTU */
> > +   seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb)
> > +   + skb_network_header_len(skb);
> > +
> > +   /* Restriction applied for the segmentation size
> > +* to use HW segmentation offload feature: the size
> > +* of the segment must not be less than 168 bytes for
> > +* DIX formatted segments, or 176 bytes for
> > +* IEEE formatted segments.
> > +*
> > +* I use value 176 to check for the segment size here
> > +* as it can cover both 2 conditions above.
> > +*/
> > +   if (seg_size < 176)
> > +   return -ENODEV;
> > +
> > +   /* Get the best suitable MTU */
> > +   for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
> > +   u32 curr_seg = tah_ss[i];
> > +
> > +   if (curr_seg > dev->ndev->mtu ||
> > +   curr_seg > seg_size)
> > +   continue;
> > +
> > +   *ctrl &= ~EMAC_TX_CTRL_TAH_CSUM;
> > +   *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
> > +   return 0;
> 
> This is something that you can possibly take out of your hot path and
> recalculate when the MTU actually changes?

Do you mean the curr_seg > dev->ndev->mtu check? Because from what I
know, the MSS can be manually set by a socket option (TCP_MAXSEG) on a
"per-socket" base.

(Altough, iperf warns that "Setting MSS is very buggy"... Which it is
as I see more way retries with a iperf run with a MSS less than
768-ish. Ideally, the change_mtu could update the TAH_SS register )

> [snip]
> 
> > +static netdev_tx_t emac_sw_tso(struct sk_buff *skb, struct net_device 
> > *ndev)
> > +{
> > +   struct emac_instance *dev = netdev_priv(ndev);
> > +   struct sk_buff *segs, *curr;
> > +
> > +   segs = skb_gso_segment(skb, ndev->features &
> > +   ~(NETIF_F_TSO | NETIF_F_TSO6));
> > +   if (IS_ERR_OR_NULL(segs)) {
> > +   goto drop;
> > +   } else {
> > +   while (segs) {
> > +   /* check for overflow */
> > +   if (dev->tx_cnt >= NUM_TX_BUFF) {
> > +   dev_kfree_skb_any(segs);
> > +   goto drop;
> > +   }
> 
> Would setting dev->max_gso_segs somehow help make sure the stack does
> not feed you oversized GSO'd skbs?
Ok, thanks's for that pointer. I'll look at dev->gso_max_segs and
dev->gso_max_size. The hardware documentation doesn't mention any
hard upper limit for how many segments it can do. What it does tell
is that it just needs an extra 512Bytes in the TX FIFO as a buffer
to store the header template and the calculated checksums and what
not.
 
But this should be no problem because that TX FIFO is 10 KiB. so even
the 9000 Jumbo frames should have plenty of "free real estate".

As for the "overflow" case:

There's a check in emac_start_xmit_sg() before emac_tx_tso() call that
does an *estimation* check and goes to "stop_queue" if it doesn't fit:

|/* Note, this is only an *estimation*, we can still run out of empty
| * slots because of the additional fragmentation

[PATCH 1/2] net: emac: implement 802.1Q VLAN TX tagging support

[Christian Lamparter]

As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
VLAN:
 - Support for VLAN tag ID in compliance with IEEE 802.3ac.
 - VLAN tag insertion or replacement for transmit packets

This patch completes the missing code for the VLAN tx tagging
support, as the the EMAC_MR1_VLE was already enabled.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 32 
 drivers/net/ethernet/ibm/emac/core.h |  6 +-
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 760b2ad8e295..be560f9031f4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
 ndev->dev_addr[5]);
 
/* VLAN Tag Protocol ID */
-   out_be32(>vtpid, 0x8100);
+   out_be32(>vtpid, ETH_P_8021Q);
 
/* Receive mode register */
r = emac_iff2rmr(ndev);
@@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct 
emac_instance *dev, int len)
return NETDEV_TX_OK;
 }
 
+static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
+{
+   /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
+   skb_vlan_tag_present(skb)) {
+   struct emac_regs __iomem *p = dev->emacp;
+
+   /* update the VLAN TCI */
+   out_be32(>vtci, (u32)skb_vlan_tag_get(skb));
+
+   /* Insert VLAN tag */
+   return EMAC_TX_CTRL_IVT;
+   }
+   return 0;
+}
+
 /* Tx lock BH */
 static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device 
*ndev)
 {
@@ -1443,7 +1460,7 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, 
struct net_device *ndev)
int slot;
 
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb);
+   MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
 
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
@@ -1518,7 +1535,7 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device 
*ndev)
goto stop_queue;
 
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
-   emac_tx_csum(dev, skb);
+   emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot;
 
/* skb data */
@@ -2891,7 +2908,8 @@ static int emac_init_config(struct emac_instance *dev)
if (of_device_is_compatible(np, "ibm,emac-apm821xx")) {
dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
  EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
- EMAC_FTR_460EX_PHY_CLK_FIX);
+ EMAC_FTR_460EX_PHY_CLK_FIX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX);
}
} else if (of_device_is_compatible(np, "ibm,emac4")) {
dev->features |= EMAC_FTR_EMAC4;
@@ -3148,6 +3166,12 @@ static int emac_probe(struct platform_device *ofdev)
 
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+
+   if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
+   ndev->vlan_features |= ndev->hw_features;
+   ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+   }
+
ndev->features |= ndev->hw_features | NETIF_F_RXCSUM;
}
ndev->watchdog_timeo = 5 * HZ;
diff --git a/drivers/net/ethernet/ibm/emac/core.h 
b/drivers/net/ethernet/ibm/emac/core.h
index 84caa4a3fc52..8d84d439168c 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -334,6 +334,8 @@ struct emac_instance {
  * APM821xx does not support Half Duplex mode
  */
 #define EMAC_FTR_APM821XX_NO_HALF_DUPLEX   0x1000
+/* EMAC can insert 802.1Q tag */
+#define EMAC_FTR_HAS_VLAN_CTAG_TX  0x2000
 
 /* Right now, we don't quite handle the always/possible masks on the
  * most optimal way as we don't have a way to say something like
@@ -363,7 +365,9 @@ enum {
EMAC_FTR_460EX_PHY_CLK_FIX |
EMAC_FTR_440EP_PHY_CLK_FIX |
EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
-   EMAC_FTR_APM821XX_NO_HALF_DUPLEX,
+   EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
+   EMAC_FTR_HAS_VLAN_CTAG_TX |
+   0,
 };
 
 static inline int emac_has_feature(struct emac_instance *dev,
-- 
2.19.1

[PATCH 2/2] net: emac: implement TCP TSO

[Christian Lamparter]

This patch enables TSO(v4) hw feature for emac driver.
As atleast the APM82181's TCP/IP acceleration hardware
controller (TAH) provides TCP segmentation support in
the transmit path.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 101 ++-
 drivers/net/ethernet/ibm/emac/core.h |   4 ++
 drivers/net/ethernet/ibm/emac/emac.h |   7 ++
 drivers/net/ethernet/ibm/emac/tah.c  |  20 ++
 drivers/net/ethernet/ibm/emac/tah.h  |   2 +
 5 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index be560f9031f4..49ffbd6e1707 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1410,6 +1413,52 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
 }
 
+const u32 tah_ss[TAH_NO_SSR] = { 9000, 4500, 1500, 1300, 576, 176 };
+
+static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
+  u16 *ctrl)
+{
+   if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) &&
+   skb_is_gso(skb) && !!(skb_shinfo(skb)->gso_type &
+   (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
+   u32 seg_size = 0, i;
+
+   /* Get the MTU */
+   seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb)
+   + skb_network_header_len(skb);
+
+   /* Restriction applied for the segmentation size
+* to use HW segmentation offload feature: the size
+* of the segment must not be less than 168 bytes for
+* DIX formatted segments, or 176 bytes for
+* IEEE formatted segments.
+*
+* I use value 176 to check for the segment size here
+* as it can cover both 2 conditions above.
+*/
+   if (seg_size < 176)
+   return -ENODEV;
+
+   /* Get the best suitable MTU */
+   for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+   u32 curr_seg = tah_ss[i];
+
+   if (curr_seg > dev->ndev->mtu ||
+   curr_seg > seg_size)
+   continue;
+
+   *ctrl &= ~EMAC_TX_CTRL_TAH_CSUM;
+   *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
+   return 0;
+   }
+
+   /* none found fall back to software */
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
 {
struct emac_regs __iomem *p = dev->emacp;
@@ -1452,8 +1501,46 @@ static inline u16 emac_tx_vlan(struct emac_instance 
*dev, struct sk_buff *skb)
return 0;
 }
 
+static netdev_tx_t
+emac_start_xmit(struct sk_buff *skb, struct net_device *ndev);
+
+static netdev_tx_t emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct sk_buff *segs, *curr;
+
+   segs = skb_gso_segment(skb, ndev->features &
+   ~(NETIF_F_TSO | NETIF_F_TSO6));
+   if (IS_ERR_OR_NULL(segs)) {
+   goto drop;
+   } else {
+   while (segs) {
+   /* check for overflow */
+   if (dev->tx_cnt >= NUM_TX_BUFF) {
+   dev_kfree_skb_any(segs);
+   goto drop;
+   }
+
+   curr = segs;
+   segs = curr->next;
+   curr->next = NULL;
+
+   emac_start_xmit(curr, ndev);
+   }
+   dev_consume_skb_any(skb);
+   }
+
+   return NETDEV_TX_OK;
+
+drop:
+   ++dev->estats.tx_dropped;
+   dev_kfree_skb_any(skb);
+   return NETDEV_TX_OK;
+}
+
 /* Tx lock BH */
-static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device 
*ndev)
+static netdev_tx_t
+emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
struct emac_instance *dev = netdev_priv(ndev);
unsigned int len = skb->len;
@@ -1462,6 +1549,9 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, 
struct net_device *ndev)
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
 
+   if (emac_tx_tso(dev, skb, ))
+   return emac_sw_tso(skb, ndev);
+
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
dev->tx_slot = 0;
@@ -1536,6 +1626,9 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_de

[PATCH] net: emac: fix fixed-link setup for the RTL8363SB switch

[Christian Lamparter]

On the Netgear WNDAP620, the emac ethernet isn't receiving nor
xmitting any frames from/to the RTL8363SB (identifies itself
as a RTL8367RB).

This is caused by the emac hardware not knowing the forced link
parameters for speed, duplex, pause, etc.

This begs the question, how this was working on the original
driver code, when it was necessary to set the phy_address and
phy_map to 0x. But I guess without access to the old
PPC405/440/460 hardware, it's not possible to know.

Signed-off-by: Christian Lamparter 
---
 drivers/net/ethernet/ibm/emac/core.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 354c0982847b..3b398ebdb5e6 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2677,12 +2677,17 @@ static int emac_init_phy(struct emac_instance *dev)
if (of_phy_is_fixed_link(np)) {
int res = emac_dt_mdio_probe(dev);
 
-   if (!res) {
-   res = of_phy_register_fixed_link(np);
-   if (res)
-   mdiobus_unregister(dev->mii_bus);
+   if (res)
+   return res;
+
+   res = of_phy_register_fixed_link(np);
+   dev->phy_dev = of_phy_find_device(np);
+   if (res || !dev->phy_dev) {
+   mdiobus_unregister(dev->mii_bus);
+   return res ? res : -EINVAL;
}
-   return res;
+   emac_adjust_link(dev->ndev);
+   put_device(>phy_dev->mdio.dev);
}
return 0;
}
-- 
2.19.0.rc2

Re: [PATCH 2/2] net/ibm/emac: wrong bit is used for STA control register write

[Christian Lamparter]

On Monday, January 22, 2018 8:01:46 PM CET Ivan Mikhaylov wrote:
> >Something looks wrong here?! The commit message talks about bit 18, 19 and 
> >20.
> >However, 0x0800, 0x1000, 0x2000 and are like bit 11, 12 and 13? Furthermore,
> >what about the EMAC_STACR_STAC_MASK? shouldn't it be 0x1800 now (or delete it
> >since it doesn't look like it's used anywhere?).
> Christian, nope, it's all fine there, it's big endian.
Ok Thanks. I think I found the relevant info on Wikipedia:


"Little-endian CPUs usually employ "LSB 0" bit numbering, however both bit
numbering conventions can be seen in big-endian machines. Some architectures
like SPARC and Motorola 68000 use "LSB 0" bit numbering, while S/390, PowerPC
and PA-RISC use "MSB 0"."

But as far as the kernel (code) is concerned, all architectures (big or
little endian) have the same BIT marco:

|#define BIT(nr)(1UL << (nr))
So if someone tries to #define EMAC_STACR_STAC_WRITE BIT(18) it would be
0x4 instead. This is where the confusion is coming from. Can you please
at least mention this somewhere that all the bits in the commit message are
in "MSB 0" format? It's confusing enough as it is ;).

> This commit related only
> to write operation, I'll check MASK and see what I can do with that.
Well, the MASK is not used and it now looks odd. So you might as well
delete it. Maybe as well as all the unused EMACX_STACR_STAC_IND_* macros?

Thanks,
Christian

Re: [PATCH 2/2] net/ibm/emac: wrong bit is used for STA control register write

[Christian Lamparter]

On Monday, January 22, 2018 5:00:38 PM CET Ivan Mikhaylov wrote:
> STA control register has areas of mode and opcodes for opeations. 18 bit is
> using for mode selection, where 0 is old MIO/MDIO access method and 1 is
> indirect access mode. 19-20 bits are using for setting up read/write
> operation(STA opcodes). In current state 'read' is set into old MIO/MDIO mode
> with 19 bit and write operation is set into 18 bit which is mode selection,
> not a write operation. To correlate write with read we set it into 20 bit.
> 
> Signed-off-by: Ivan Mikhaylov 
> ---
>  drivers/net/ethernet/ibm/emac/emac.h |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/emac.h 
> b/drivers/net/ethernet/ibm/emac/emac.h
> index d0a0e3b..c26d263 100644
> --- a/drivers/net/ethernet/ibm/emac/emac.h
> +++ b/drivers/net/ethernet/ibm/emac/emac.h
> @@ -244,7 +244,7 @@ struct emac_regs {
>  #define EMAC_STACR_PHYE  0x4000
>  #define EMAC_STACR_STAC_MASK 0x3000
>  #define EMAC_STACR_STAC_READ 0x1000
> -#define EMAC_STACR_STAC_WRITE0x2000
> +#define EMAC_STACR_STAC_WRITE0x0800
>  #define EMAC_STACR_OPBC_MASK 0x0C00
>  #define EMAC_STACR_OPBC_50   0x
>  #define EMAC_STACR_OPBC_66   0x0400
> 

Something looks wrong here?! The commit message talks about bit 18, 19 and 20.
However, 0x0800, 0x1000, 0x2000 and are like bit 11, 12 and 13? Furthermore,
what about the EMAC_STACR_STAC_MASK? shouldn't it be 0x1800 now (or delete it
since it doesn't look like it's used anywhere?).

Re: [PATCH v2 15/19] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Friday, January 12, 2018 7:39:50 PM CET Dan Williams wrote:
> On Fri, Jan 12, 2018 at 6:42 AM, Christian Lamparter <chunk...@gmail.com> 
> wrote:
> > On Friday, January 12, 2018 1:47:46 AM CET Dan Williams wrote:
> >> Static analysis reports that 'queue' may be a user controlled value that
> >> is used as a data dependency to read from the 'ar9170_qmap' array. In
> >> order to avoid potential leaks of kernel memory values, block
> >> speculative execution of the instruction stream that could issue reads
> >> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> >> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> >> 'ar->edcf' array.
> >>
> >> Based on an original patch by Elena Reshetova.
> >>
> >> Cc: Christian Lamparter <chunk...@googlemail.com>
> >> Cc: Kalle Valo <kv...@codeaurora.org>
> >> Cc: linux-wirel...@vger.kernel.org
> >> Cc: netdev@vger.kernel.org
> >> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> >> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> >> ---
> > This patch (and p54, cw1200) look like the same patch?!
> > Can you tell me what happend to:
> >
> > On Saturday, January 6, 2018 5:34:03 PM CET Dan Williams wrote:
> >> On Sat, Jan 6, 2018 at 6:23 AM, Christian Lamparter <chunk...@gmail.com> 
> >> wrote:
> >> > And Furthermore a invalid queue (param->ac) would cause a crash in
> >> > this line in mac80211 before it even reaches the driver [1]:
> >> > |   sdata->tx_conf[params->ac] = p;
> >> > |   
> >> > |   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
> >> > |^^ (this is a wrapper for the *_op_conf_tx)
> >> >
> >> > I don't think these chin-up exercises are needed.
> >>
> >> Quite the contrary, you've identified a better place in the call stack
> >> to sanitize the input and disable speculation. Then we can kill the
> >> whole class of the wireless driver reports at once it seems.
> > <https://www.spinics.net/lists/netdev/msg476353.html>
> 
> I didn't see where ac is being validated against the driver specific
> 'queues' value in that earlier patch.
The link to the check is right there in the earlier post. It's in 
parse_txq_params():
<https://github.com/torvalds/linux/blob/master/net/wireless/nl80211.c#L2070>
|   if (txq_params->ac >= NL80211_NUM_ACS)
|   return -EINVAL;

NL80211_NUM_ACS is 4
<http://elixir.free-electrons.com/linux/v4.15-rc7/source/include/uapi/linux/nl80211.h#L3748>

This check was added ever since mac80211's ieee80211_set_txq_params():
| sdata->tx_conf[params->ac] = p;

For cw1200: the driver just sets the dev->queue to 4.
In carl9170 dev->queues is set to __AR9170_NUM_TXQ and
p54 uses P54_QUEUE_AC_NUM.

Both __AR9170_NUM_TXQ and P54_QUEUE_AC_NUM are 4.
And this is not going to change since all drivers
have to follow mac80211's queue API:
<https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/queues>

Some background:
In the old days (linux 2.6 and early 3.x), the parse_txq_params() function did
not verify the "queue" value. That's why these drivers had to do it.

Here's the relevant code from 2.6.39:
<http://elixir.free-electrons.com/linux/v2.6.39/source/net/wireless/nl80211.c#L879>
You'll notice that the check is missing there.
Here's mac80211's ieee80211_set_txq_params for reference:
<http://elixir.free-electrons.com/linux/v2.6.39/source/net/mac80211/cfg.c#L1197>

However over time, the check in the driver has become redundant.

> > Anyway, I think there's an easy way to solve this: remove the
> > "if (queue < ar->hw->queues)" check altogether. It's no longer needed
> > anymore as the "queue" value is validated long before the driver code
> > gets called.
> > 
> > And from my understanding, this will fix the "In this case
> > the value of 'ar9170_qmap[queue]' is immediately reused as an index to
> > the 'ar->edcf' array." gripe your tool complains about.
> >
> > This is here's a quick test-case for carl9170.:
> > ---
> > diff --git a/drivers/net/wireless/ath/carl9170/main.c 
> > b/drivers/net/wireless/ath/carl9170/main.c
> > index 988c8857d78c..2d3afb15bb62 100644
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1387,13 +1387,8 @@ static int carl9170_op_conf_tx(struct ieee80211_hw 
> > *hw,
> > int r

Re: [PATCH v2 15/19] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Friday, January 12, 2018 1:47:46 AM CET Dan Williams wrote:
> Static analysis reports that 'queue' may be a user controlled value that
> is used as a data dependency to read from the 'ar9170_qmap' array. In
> order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue reads
> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> 'ar->edcf' array.
> 
> Based on an original patch by Elena Reshetova.
> 
> Cc: Christian Lamparter <chunk...@googlemail.com>
> Cc: Kalle Valo <kv...@codeaurora.org>
> Cc: linux-wirel...@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
This patch (and p54, cw1200) look like the same patch?! 
Can you tell me what happend to:

On Saturday, January 6, 2018 5:34:03 PM CET Dan Williams wrote:
> On Sat, Jan 6, 2018 at 6:23 AM, Christian Lamparter <chunk...@gmail.com> 
> wrote:
> > And Furthermore a invalid queue (param->ac) would cause a crash in
> > this line in mac80211 before it even reaches the driver [1]:
> > |   sdata->tx_conf[params->ac] = p;
> > |   
> > |   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
> > |^^ (this is a wrapper for the *_op_conf_tx)
> >
> > I don't think these chin-up exercises are needed.
> 
> Quite the contrary, you've identified a better place in the call stack
> to sanitize the input and disable speculation. Then we can kill the
> whole class of the wireless driver reports at once it seems.
<https://www.spinics.net/lists/netdev/msg476353.html>

Anyway, I think there's an easy way to solve this: remove the 
"if (queue < ar->hw->queues)" check altogether. It's no longer needed
anymore as the "queue" value is validated long before the driver code
gets called. And from my understanding, this will fix the "In this case
the value of 'ar9170_qmap[queue]' is immediately reused as an index to
the 'ar->edcf' array." gripe your tool complains about.

This is here's a quick test-case for carl9170.:
---
diff --git a/drivers/net/wireless/ath/carl9170/main.c 
b/drivers/net/wireless/ath/carl9170/main.c
index 988c8857d78c..2d3afb15bb62 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1387,13 +1387,8 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw,
int ret;
 
mutex_lock(>mutex);
-   if (queue < ar->hw->queues) {
-   memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
-   ret = carl9170_set_qos(ar);
-   } else {
-   ret = -EINVAL;
-   }
-
+   memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
+   ret = carl9170_set_qos(ar);
mutex_unlock(>mutex);
return ret;
 }
---
What does your tool say about this? 

(If necessary, the "queue" value could be even sanitized with a
queue %= ARRAY_SIZE(ar9170_qmap); before the mutex_lock.)

Re: [PATCH 08/18] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Saturday, January 6, 2018 4:06:21 PM CET Alan Cox wrote:
> > The only way a user can set this in any meaningful way would be via
> > a NL80211_CMD_SET_WIPHY netlink message. However, the value will get
> > vetted there by cfg80211's parse_txq_params [0]. This is long before
> 
> Far more than a couple of hundred instructions ?
Well, the user would have to send a netlink message each time. So
cfg80211 can parse it (this is where the initial "if queue >= 4 " check
happen). So the CPU would have to continue through and into 
rdev_set_txq_params() to get to mac80211's ieee80211_set_txq_params().
Then pass through that before gets to the driver's op_tx_conf. Once
there the driver code aquires a mutex_lock too before it gets to
check the queue value again.

Is this enough and how would the mutex_lock fit in there? Or can
the CPU skip past this as well? 
> The problem is that the processor will speculate that the parameter
> is valid and continue on that basis until the speculation resolves
> or it hits some other limit that stops it speculating further.
> That can be quite a distance on a modern x86 processor, and for all
> we know could be even more on some of the other CPUs involved.
 
> > it reaches any of the *_op_conf_tx functions.
> > 
> > And Furthermore a invalid queue (param->ac) would cause a crash in 
> > this line in mac80211 before it even reaches the driver [1]:
> > |   sdata->tx_conf[params->ac] = p;
> > |   
> 
> Firstly it might not because the address you get as a result could be
> valid kernel memory. In fact your attackers wants it to be valid kernel
> memory since they are trying to find the value of a piece of that memory.
> 
> Secondly the processor is doing this speculatively so it won't fault. It
> will eventually decide it went the wrong way and throw all the
> speculative work away - leaving footprints. It won't fault unless the
> speculative resolves that was the real path the code took.
> 
> If it's not a performance critical path then it's better to be safe.
Thank you for reading the canary too.

Christian

Re: [PATCH 08/18] carl9170: prevent bounds-check bypass via speculative execution

[Christian Lamparter]

On Saturday, January 6, 2018 2:10:37 AM CET Dan Williams wrote:
> Static analysis reports that 'queue' may be a user controlled value that
> is used as a data dependency to read from the 'ar9170_qmap' array. In
> order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue reads
> based on an invalid result of 'ar9170_qmap[queue]'. In this case the
> value of 'ar9170_qmap[queue]' is immediately reused as an index to the
> 'ar->edcf' array.
> 
> Based on an original patch by Elena Reshetova.
> 
> Cc: Christian Lamparter <chunk...@googlemail.com>
> Cc: Kalle Valo <kv...@codeaurora.org>
> Cc: linux-wirel...@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
>  drivers/net/wireless/ath/carl9170/main.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/main.c 
> b/drivers/net/wireless/ath/carl9170/main.c
> index 988c8857d78c..0ff34cbe2b62 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "hw.h"
> @@ -1384,11 +1385,12 @@ static int carl9170_op_conf_tx(struct ieee80211_hw 
> *hw,
>  const struct ieee80211_tx_queue_params *param)
>  {
>   struct ar9170 *ar = hw->priv;
> + const u8 *elem;
>   int ret;
>  
>   mutex_lock(>mutex);
> - if (queue < ar->hw->queues) {
> - memcpy(>edcf[ar9170_qmap[queue]], param, sizeof(*param));
> + if ((elem = nospec_array_ptr(ar9170_qmap, queue, ar->hw->queues))) {
> + memcpy(>edcf[*elem], param, sizeof(*param));
>   ret = carl9170_set_qos(ar);
>   } else {
>   ret = -EINVAL;
> 
> 
About the "queue" in carl9170_op_conf_tx, p54_conf_tx and cw1200_conf_tx:

The only way a user can set this in any meaningful way would be via
a NL80211_CMD_SET_WIPHY netlink message. However, the value will get
vetted there by cfg80211's parse_txq_params [0]. This is long before
it reaches any of the *_op_conf_tx functions.

And Furthermore a invalid queue (param->ac) would cause a crash in 
this line in mac80211 before it even reaches the driver [1]:
|   sdata->tx_conf[params->ac] = p;
|   
|   if (drv_conf_tx(local, sdata, >>>> params->ac <<<<, )) {
|^^ (this is a wrapper for the *_op_conf_tx)

I don't think these chin-up exercises are needed.

[0] <https://github.com/torvalds/linux/blob/master/net/wireless/nl80211.c#L2070>
[1] <https://github.com/torvalds/linux/blob/master/net/mac80211/cfg.c#L2157>

[PATCH v3 2/3] net: ibm: emac: replace custom PHY_MODE_* macros

[Christian Lamparter]

The ibm_emac driver predates the PHY_INTERFACE_MODE_*
enums by a few years.

And while the driver has been retrofitted to use the PHYLIB,
the old definitions have stuck around to this day.

This patch replaces all occurences of PHY_MODE_* with
the respective equivalent PHY_INTERFACE_MODE_* enum.
And finally, it purges the old macros for good.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>

---
Extra change in zmii.c to make checkpatch happy.
---
 drivers/net/ethernet/ibm/emac/core.c  | 20 +-
 drivers/net/ethernet/ibm/emac/emac.h  | 13 
 drivers/net/ethernet/ibm/emac/phy.c   | 10 -
 drivers/net/ethernet/ibm/emac/rgmii.c | 20 +-
 drivers/net/ethernet/ibm/emac/zmii.c  | 38 +--
 5 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 7feff2450ed6..27f592da7cfc 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -199,18 +199,18 @@ static void __emac_set_multicast_list(struct 
emac_instance *dev);
 
 static inline int emac_phy_supports_gige(int phy_mode)
 {
-   return  phy_mode == PHY_MODE_GMII ||
-   phy_mode == PHY_MODE_RGMII ||
-   phy_mode == PHY_MODE_SGMII ||
-   phy_mode == PHY_MODE_TBI ||
-   phy_mode == PHY_MODE_RTBI;
+   return  phy_mode == PHY_INTERFACE_MODE_GMII ||
+   phy_mode == PHY_INTERFACE_MODE_RGMII ||
+   phy_mode == PHY_INTERFACE_MODE_SGMII ||
+   phy_mode == PHY_INTERFACE_MODE_TBI ||
+   phy_mode == PHY_INTERFACE_MODE_RTBI;
 }
 
 static inline int emac_phy_gpcs(int phy_mode)
 {
-   return  phy_mode == PHY_MODE_SGMII ||
-   phy_mode == PHY_MODE_TBI ||
-   phy_mode == PHY_MODE_RTBI;
+   return  phy_mode == PHY_INTERFACE_MODE_SGMII ||
+   phy_mode == PHY_INTERFACE_MODE_TBI ||
+   phy_mode == PHY_INTERFACE_MODE_RTBI;
 }
 
 static inline void emac_tx_enable(struct emac_instance *dev)
@@ -2865,7 +2865,7 @@ static int emac_init_config(struct emac_instance *dev)
/* PHY mode needs some decoding */
dev->phy_mode = of_get_phy_mode(np);
if (dev->phy_mode < 0)
-   dev->phy_mode = PHY_MODE_NA;
+   dev->phy_mode = PHY_INTERFACE_MODE_NA;
 
/* Check EMAC version */
if (of_device_is_compatible(np, "ibm,emac4sync")) {
@@ -3168,7 +3168,7 @@ static int emac_probe(struct platform_device *ofdev)
printk(KERN_INFO "%s: EMAC-%d %pOF, MAC %pM\n",
   ndev->name, dev->cell_index, np, ndev->dev_addr);
 
-   if (dev->phy_mode == PHY_MODE_SGMII)
+   if (dev->phy_mode == PHY_INTERFACE_MODE_SGMII)
printk(KERN_NOTICE "%s: in SGMII mode\n", ndev->name);
 
if (dev->phy.address >= 0)
diff --git a/drivers/net/ethernet/ibm/emac/emac.h 
b/drivers/net/ethernet/ibm/emac/emac.h
index 5afcc27ceebb..bc14dcf27b6b 100644
--- a/drivers/net/ethernet/ibm/emac/emac.h
+++ b/drivers/net/ethernet/ibm/emac/emac.h
@@ -104,19 +104,6 @@ struct emac_regs {
} u1;
 };
 
-/*
- * PHY mode settings (EMAC <-> ZMII/RGMII bridge <-> PHY)
- */
-#define PHY_MODE_NAPHY_INTERFACE_MODE_NA
-#define PHY_MODE_MII   PHY_INTERFACE_MODE_MII
-#define PHY_MODE_RMII  PHY_INTERFACE_MODE_RMII
-#define PHY_MODE_SMII  PHY_INTERFACE_MODE_SMII
-#define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
-#define PHY_MODE_TBI   PHY_INTERFACE_MODE_TBI
-#define PHY_MODE_GMII  PHY_INTERFACE_MODE_GMII
-#define PHY_MODE_RTBI  PHY_INTERFACE_MODE_RTBI
-#define PHY_MODE_SGMII PHY_INTERFACE_MODE_SGMII
-
 /* EMACx_MR0 */
 #define EMAC_MR0_RXI   0x8000
 #define EMAC_MR0_TXI   0x4000
diff --git a/drivers/net/ethernet/ibm/emac/phy.c 
b/drivers/net/ethernet/ibm/emac/phy.c
index 874949faa424..71ee13c34192 100644
--- a/drivers/net/ethernet/ibm/emac/phy.c
+++ b/drivers/net/ethernet/ibm/emac/phy.c
@@ -96,7 +96,7 @@ int emac_mii_reset_gpcs(struct mii_phy *phy)
if ((val & BMCR_ISOLATE) && limit > 0)
gpcs_phy_write(phy, MII_BMCR, val & ~BMCR_ISOLATE);
 
-   if (limit > 0 && phy->mode == PHY_MODE_SGMII) {
+   if (limit > 0 && phy->mode == PHY_INTERFACE_MODE_SGMII) {
/* Configure GPCS interface to recommended setting for SGMII */
gpcs_phy_write(phy, 0x04, 0x8120); /* AsymPause, FDX */
gpcs_phy_write(phy, 0x07, 0x2801); /* msg_pg, toggle */
@@ -313,16 +313,16 @@ static int cis8201_init(struct mii_phy *phy)
epcr &= ~EPCR_MODE_MASK;
 
switch (phy->mode) {
-   case PHY_MODE_TBI:
+   case PHY_INTERFACE_MODE_TBI:
epcr |= EPCR_TBI_MODE;
break;
-   case PHY_MODE_RTBI:
+   case PHY_INTERFA

[PATCH v3 3/3] net: ibm: emac: support RGMII-[RX|TX]ID phymode

[Christian Lamparter]

The RGMII spec allows compliance for devices that implement an internal
delay on TXC and/or RXC inside the transmitter. This patch adds the
necessary RGMII_[RX|TX]ID mode code to handle such PHYs with the
emac driver.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>

---
v3: - replace PHY_MODE_* with PHY_INTERFACE_MODE_*
- replace rgmii_mode_name() with phy_modes[]

v2: - utilize phy_interface_mode_is_rgmii()
---
 drivers/net/ethernet/ibm/emac/core.c  | 4 ++--
 drivers/net/ethernet/ibm/emac/rgmii.c | 7 +--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 27f592da7cfc..71ddad13baf4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -199,8 +199,8 @@ static void __emac_set_multicast_list(struct emac_instance 
*dev);
 
 static inline int emac_phy_supports_gige(int phy_mode)
 {
-   return  phy_mode == PHY_INTERFACE_MODE_GMII ||
-   phy_mode == PHY_INTERFACE_MODE_RGMII ||
+   return  phy_interface_mode_is_rgmii(phy_mode) ||
+   phy_mode == PHY_INTERFACE_MODE_GMII ||
phy_mode == PHY_INTERFACE_MODE_SGMII ||
phy_mode == PHY_INTERFACE_MODE_TBI ||
phy_mode == PHY_INTERFACE_MODE_RTBI;
diff --git a/drivers/net/ethernet/ibm/emac/rgmii.c 
b/drivers/net/ethernet/ibm/emac/rgmii.c
index 5db225831e81..00f5999de3cf 100644
--- a/drivers/net/ethernet/ibm/emac/rgmii.c
+++ b/drivers/net/ethernet/ibm/emac/rgmii.c
@@ -52,9 +52,9 @@
 /* RGMII bridge supports only GMII/TBI and RGMII/RTBI PHYs */
 static inline int rgmii_valid_mode(int phy_mode)
 {
-   return  phy_mode == PHY_INTERFACE_MODE_GMII ||
+   return  phy_interface_mode_is_rgmii(phy_mode) ||
+   phy_mode == PHY_INTERFACE_MODE_GMII ||
phy_mode == PHY_INTERFACE_MODE_MII ||
-   phy_mode == PHY_INTERFACE_MODE_RGMII ||
phy_mode == PHY_INTERFACE_MODE_TBI ||
phy_mode == PHY_INTERFACE_MODE_RTBI;
 }
@@ -63,6 +63,9 @@ static inline u32 rgmii_mode_mask(int mode, int input)
 {
switch (mode) {
case PHY_INTERFACE_MODE_RGMII:
+   case PHY_INTERFACE_MODE_RGMII_ID:
+   case PHY_INTERFACE_MODE_RGMII_RXID:
+   case PHY_INTERFACE_MODE_RGMII_TXID:
return RGMII_FER_RGMII(input);
case PHY_INTERFACE_MODE_TBI:
return RGMII_FER_TBI(input);
-- 
2.15.1

[PATCH v3 1/3] net: ibm: emac: replace custom rgmii_mode_name with phy_modes

[Christian Lamparter]

phy_modes() in the common phy.h already defines the same phy mode
names in lower case. The deleted rgmii_mode_name() is used only
in one place and for a "notice-level" printk. Hence, it will not
be missed.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/ethernet/ibm/emac/rgmii.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/rgmii.c 
b/drivers/net/ethernet/ibm/emac/rgmii.c
index c4a1ac38bba8..9a1c06f2471d 100644
--- a/drivers/net/ethernet/ibm/emac/rgmii.c
+++ b/drivers/net/ethernet/ibm/emac/rgmii.c
@@ -59,24 +59,6 @@ static inline int rgmii_valid_mode(int phy_mode)
phy_mode == PHY_MODE_RTBI;
 }
 
-static inline const char *rgmii_mode_name(int mode)
-{
-   switch (mode) {
-   case PHY_MODE_RGMII:
-   return "RGMII";
-   case PHY_MODE_TBI:
-   return "TBI";
-   case PHY_MODE_GMII:
-   return "GMII";
-   case PHY_MODE_MII:
-   return "MII";
-   case PHY_MODE_RTBI:
-   return "RTBI";
-   default:
-   BUG();
-   }
-}
-
 static inline u32 rgmii_mode_mask(int mode, int input)
 {
switch (mode) {
@@ -115,7 +97,7 @@ int rgmii_attach(struct platform_device *ofdev, int input, 
int mode)
out_be32(>fer, in_be32(>fer) | rgmii_mode_mask(mode, input));
 
printk(KERN_NOTICE "%pOF: input %d in %s mode\n",
-  ofdev->dev.of_node, input, rgmii_mode_name(mode));
+  ofdev->dev.of_node, input, phy_modes(mode));
 
++dev->users;
 
-- 
2.15.1

Re: [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode

[Christian Lamparter]

On Wednesday, December 20, 2017 3:10:46 PM CET David Miller wrote:
> From: Christian Lamparter <chunk...@gmail.com>
> Date: Wed, 20 Dec 2017 17:02:01 +0100
> 
> > diff --git a/drivers/net/ethernet/ibm/emac/emac.h 
> > b/drivers/net/ethernet/ibm/emac/emac.h
> > index 5afcc27ceebb..8c6d2af7281b 100644
> > --- a/drivers/net/ethernet/ibm/emac/emac.h
> > +++ b/drivers/net/ethernet/ibm/emac/emac.h
> > @@ -112,6 +112,9 @@ struct emac_regs {
> >  #define PHY_MODE_RMII  PHY_INTERFACE_MODE_RMII
> >  #define PHY_MODE_SMII  PHY_INTERFACE_MODE_SMII
> >  #define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
> > +#define PHY_MODE_RGMII_ID  PHY_INTERFACE_MODE_RGMII_ID
> > +#define PHY_MODE_RGMII_RXIDPHY_INTERFACE_MODE_RGMII_RXID
> > +#define PHY_MODE_RGMII_TXIDPHY_INTERFACE_MODE_RGMII_TXID
> >  #define PHY_MODE_TBI   PHY_INTERFACE_MODE_TBI
> >  #define PHY_MODE_GMII  PHY_INTERFACE_MODE_GMII
> >  #define PHY_MODE_RTBI  PHY_INTERFACE_MODE_RTBI
> 
> Why does this driver do all of this CPP macro aliasing?
Ah, well the emac driver is almost as old as dirt ;).
I added Benjamin Herrenschmidt since he seems to be the
original author. maybe he can provide some valuable insights
in this archaeological dig.

I don't know when the ibm_emac driver was added, but it does
predate the shared PHY_INTERFACE_MODE_ macros by a few years.

For example 2.6.11 had the driver and the defines.:
<http://elixir.free-electrons.com/linux/v2.6.11/source/drivers/net/ibm_emac/ibm_emac_phy.h#L41>
But there's no PHY_INTERFACE_MODE_* yet.

Fast forward to 2011:
The patch <https://patchwork.kernel.org/patch/945642/> wedded the
PHY_MODE_* macros to the PHY_INTERFACE_MODE_ enums. But this was
not a complete convertion.

So, as far as I can tell, these definitions have been there since
the beginning.
> 
> It's really cruddy because anyone who now reads this code:
> 
> >  static inline int rgmii_valid_mode(int phy_mode)
> >  {
> > -   return  phy_mode == PHY_MODE_GMII ||
> > +   return  phy_interface_mode_is_rgmii(phy_mode) ||
> > +   phy_mode == PHY_MODE_GMII ||
> > phy_mode == PHY_MODE_MII ||
> > -   phy_mode == PHY_MODE_RGMII ||
> > phy_mode == PHY_MODE_TBI ||
> > phy_mode == PHY_MODE_RTBI;
> >  }
> 
> will read this and say "Oh the function tests against these weird
> PHY_MODE_* aliases, but the helper function phy_interface_mode_is_rgmii()
> tests against PHY_INTERFACE_MODE_*, what is going on?"
> 
> I hate to do this to you, but please get rid of these confusing and
> completely unnecessary PHY_MODE_* CPP aliases first, and just use the
> proper PHY_INTERFACE_MODE_* values consistently.

Yeah, I can do that. no problem. 

Question is, should I also replace the rgmii_mode_name() with phy_modes() too?

The only user of rgmii_mode_name() is this notice printk in rgmii_attach():
<http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/ibm/emac/rgmii.c#L117>

Regards,
Christian

[PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode

[Christian Lamparter]

The RGMII spec allows compliance for devices that implement an internal
delay on TXC and/or RXC inside the transmitter. This patch adds the
necessary RGMII_[RX|TX]ID mode code to handle such PHYs with the
emac driver.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>

---
v2: - utilize phy_interface_mode_is_rgmii()
---
 drivers/net/ethernet/ibm/emac/core.c  |  4 ++--
 drivers/net/ethernet/ibm/emac/emac.h  |  3 +++
 drivers/net/ethernet/ibm/emac/rgmii.c | 10 --
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 7feff2450ed6..043e72e28bba 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -199,8 +199,8 @@ static void __emac_set_multicast_list(struct emac_instance 
*dev);
 
 static inline int emac_phy_supports_gige(int phy_mode)
 {
-   return  phy_mode == PHY_MODE_GMII ||
-   phy_mode == PHY_MODE_RGMII ||
+   return  phy_interface_mode_is_rgmii(phy_mode) ||
+   phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_SGMII ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
diff --git a/drivers/net/ethernet/ibm/emac/emac.h 
b/drivers/net/ethernet/ibm/emac/emac.h
index 5afcc27ceebb..8c6d2af7281b 100644
--- a/drivers/net/ethernet/ibm/emac/emac.h
+++ b/drivers/net/ethernet/ibm/emac/emac.h
@@ -112,6 +112,9 @@ struct emac_regs {
 #define PHY_MODE_RMII  PHY_INTERFACE_MODE_RMII
 #define PHY_MODE_SMII  PHY_INTERFACE_MODE_SMII
 #define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
+#define PHY_MODE_RGMII_ID  PHY_INTERFACE_MODE_RGMII_ID
+#define PHY_MODE_RGMII_RXIDPHY_INTERFACE_MODE_RGMII_RXID
+#define PHY_MODE_RGMII_TXIDPHY_INTERFACE_MODE_RGMII_TXID
 #define PHY_MODE_TBI   PHY_INTERFACE_MODE_TBI
 #define PHY_MODE_GMII  PHY_INTERFACE_MODE_GMII
 #define PHY_MODE_RTBI  PHY_INTERFACE_MODE_RTBI
diff --git a/drivers/net/ethernet/ibm/emac/rgmii.c 
b/drivers/net/ethernet/ibm/emac/rgmii.c
index c4a1ac38bba8..124b0473d2b7 100644
--- a/drivers/net/ethernet/ibm/emac/rgmii.c
+++ b/drivers/net/ethernet/ibm/emac/rgmii.c
@@ -52,9 +52,9 @@
 /* RGMII bridge supports only GMII/TBI and RGMII/RTBI PHYs */
 static inline int rgmii_valid_mode(int phy_mode)
 {
-   return  phy_mode == PHY_MODE_GMII ||
+   return  phy_interface_mode_is_rgmii(phy_mode) ||
+   phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_MII ||
-   phy_mode == PHY_MODE_RGMII ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
 }
@@ -63,6 +63,9 @@ static inline const char *rgmii_mode_name(int mode)
 {
switch (mode) {
case PHY_MODE_RGMII:
+   case PHY_MODE_RGMII_ID:
+   case PHY_MODE_RGMII_RXID:
+   case PHY_MODE_RGMII_TXID:
return "RGMII";
case PHY_MODE_TBI:
return "TBI";
@@ -81,6 +84,9 @@ static inline u32 rgmii_mode_mask(int mode, int input)
 {
switch (mode) {
case PHY_MODE_RGMII:
+   case PHY_MODE_RGMII_ID:
+   case PHY_MODE_RGMII_RXID:
+   case PHY_MODE_RGMII_TXID:
return RGMII_FER_RGMII(input);
case PHY_MODE_TBI:
return RGMII_FER_TBI(input);
-- 
2.15.1

[PATCH] net: ibm: emac: support RGMII-[RX|TX]ID phymode

[Christian Lamparter]

The RGMII spec allows compliance for devices that implement an internal
delay on TXC and/or RXC inside the transmitter. This patch adds the
necessary RGMII_[RX|TX]ID mode code to handle such PHYs with the
emac driver.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/net/ethernet/ibm/emac/core.c  | 3 +++
 drivers/net/ethernet/ibm/emac/emac.h  | 3 +++
 drivers/net/ethernet/ibm/emac/rgmii.c | 9 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 7feff2450ed6..820173bee168 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -201,6 +201,9 @@ static inline int emac_phy_supports_gige(int phy_mode)
 {
return  phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_RGMII ||
+   phy_mode == PHY_MODE_RGMII_ID ||
+   phy_mode == PHY_MODE_RGMII_RXID ||
+   phy_mode == PHY_MODE_RGMII_TXID ||
phy_mode == PHY_MODE_SGMII ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
diff --git a/drivers/net/ethernet/ibm/emac/emac.h 
b/drivers/net/ethernet/ibm/emac/emac.h
index 5afcc27ceebb..8c6d2af7281b 100644
--- a/drivers/net/ethernet/ibm/emac/emac.h
+++ b/drivers/net/ethernet/ibm/emac/emac.h
@@ -112,6 +112,9 @@ struct emac_regs {
 #define PHY_MODE_RMII  PHY_INTERFACE_MODE_RMII
 #define PHY_MODE_SMII  PHY_INTERFACE_MODE_SMII
 #define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
+#define PHY_MODE_RGMII_ID  PHY_INTERFACE_MODE_RGMII_ID
+#define PHY_MODE_RGMII_RXIDPHY_INTERFACE_MODE_RGMII_RXID
+#define PHY_MODE_RGMII_TXIDPHY_INTERFACE_MODE_RGMII_TXID
 #define PHY_MODE_TBI   PHY_INTERFACE_MODE_TBI
 #define PHY_MODE_GMII  PHY_INTERFACE_MODE_GMII
 #define PHY_MODE_RTBI  PHY_INTERFACE_MODE_RTBI
diff --git a/drivers/net/ethernet/ibm/emac/rgmii.c 
b/drivers/net/ethernet/ibm/emac/rgmii.c
index c4a1ac38bba8..7963adffbb1c 100644
--- a/drivers/net/ethernet/ibm/emac/rgmii.c
+++ b/drivers/net/ethernet/ibm/emac/rgmii.c
@@ -55,6 +55,9 @@ static inline int rgmii_valid_mode(int phy_mode)
return  phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_MII ||
phy_mode == PHY_MODE_RGMII ||
+   phy_mode == PHY_MODE_RGMII_ID ||
+   phy_mode == PHY_MODE_RGMII_RXID ||
+   phy_mode == PHY_MODE_RGMII_TXID ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
 }
@@ -63,6 +66,9 @@ static inline const char *rgmii_mode_name(int mode)
 {
switch (mode) {
case PHY_MODE_RGMII:
+   case PHY_MODE_RGMII_ID:
+   case PHY_MODE_RGMII_RXID:
+   case PHY_MODE_RGMII_TXID:
return "RGMII";
case PHY_MODE_TBI:
return "TBI";
@@ -81,6 +87,9 @@ static inline u32 rgmii_mode_mask(int mode, int input)
 {
switch (mode) {
case PHY_MODE_RGMII:
+   case PHY_MODE_RGMII_ID:
+   case PHY_MODE_RGMII_RXID:
+   case PHY_MODE_RGMII_TXID:
return RGMII_FER_RGMII(input);
case PHY_MODE_TBI:
return RGMII_FER_TBI(input);
-- 
2.15.1

Re: [PATCH v2] p54: don't unregister leds when they are not initialized

[Christian Lamparter]

On Tuesday, September 26, 2017 5:11:33 PM CEST Andrey Konovalov wrote:
> ieee80211_register_hw() in p54_register_common() may fail and leds won't
> get initialized. Currently p54_unregister_common() doesn't check that and
> always calls p54_unregister_leds(). The fix is to check priv->registered
> flag before calling p54_unregister_leds().
> 
> Found by syzkaller.
> 
> [...]
>  process_scheduled_works kernel/workqueue.c:2179
>  worker_thread+0xb2b/0x1850 kernel/workqueue.c:2255
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> 
> Signed-off-by: Andrey Konovalov <andreyk...@google.com>
Cc: sta...@vger.kernel.org
Acked-by: Christian Lamparter <chunk...@googlemail.com>

Thanks for making the patch too!

[RESEND] Re: usb/net/p54: trying to register non-static key in p54_unregister_leds

[Christian Lamparter]

This got rejected by gmail once. Let's see if it works now.

On Thursday, September 21, 2017 8:22:45 PM CEST Andrey Konovalov wrote:
> On Wed, Sep 20, 2017 at 9:55 PM, Johannes Berg
> <johan...@sipsolutions.net> wrote:
> > On Wed, 2017-09-20 at 21:27 +0200, Christian Lamparter wrote:
> >
> >> It seems this is caused as a result of:
> >> -> lock_map_acquire(>lockdep_map);
> >>   lock_map_release(>lockdep_map);
> >>
> >> in flush_work() [0]
> >
> > Agree.
> >
> >> This was added by:
> >>
> >>   commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
> >>   Author: Stephen Boyd <sb...@codeaurora.org>
> >>   Date:   Fri Apr 20 17:28:50 2012 -0700
> >>
> >>   workqueue: Catch more locking problems with flush_work()
> >
> > Yes, but that doesn't matter.
> >
> >> Looking at the Stephen's patch, it's clear that it was made
> >> with "static DECLARE_WORK(work, my_work)" in mind. However
> >> p54's led_work is "per-device", hence it is stored in the
> >> devices context p54_common, which is dynamically allocated.
> >> So, maybe revert Stephen's patch?
> >
> > I disagree - as the lockdep warning says:
> >
> >> > INFO: trying to register non-static key.
> >> > the code is fine but needs lockdep annotation.
> >> > turning off the locking correctness validator.
> >
> > What it needs is to actually correctly go through initializing the work
> > at least once.
> >
> > Without more information, I can't really say what's going on, but I
> > assume that something is failing and p54_unregister_leds() is getting
> > invoked without p54_init_leds() having been invoked, so essentially
> > it's trying to flush a work that was never initialized?
> >
> > INIT_DELAYED_WORK() does, after all, initialize the lockdep map
> > properly via __INIT_WORK().

Ok, thanks. This does indeed explain it.

But this also begs the question: Is this really working then?
>From what I can tell, if CONFIG_LOCKDEP is not set then there's no BUG
no WARN, no other splat or any other odd system behaviour. Does
[cancel | flush]_[delayed_]work[_sync] really "just work" by *accident*,
as long the delayed_work | work_struct is zeroed out? 
And should it work in the future as well?

> Since I'm able to reproduce this, please let me know if you need me to
> collect some debug traces to help with the triage.

Do you want to take a shot at making a patch too? At a quick glance, it
should be enough to move the [#ifdef CONFIG_P54_LEDS ... #endif] block
in p54_unregister_common() into the if (priv->registered) { block
(preferably before the ieee80211_unregister_hw(dev).

Regards,
Christian

Re: usb/net/p54: trying to register non-static key in p54_unregister_leds

[Christian Lamparter]

On Wednesday, September 20, 2017 8:37:08 PM CEST Andrey Konovalov wrote:
> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
> 
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 1 PID: 1404 Comm: kworker/1:1 Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80-dirty #205
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  __dump_stack lib/dump_stack.c:16
>  dump_stack+0x292/0x395 lib/dump_stack.c:52
>  register_lock_class+0x6c4/0x1a00 kernel/locking/lockdep.c:769
>  __lock_acquire+0x27e/0x4550 kernel/locking/lockdep.c:3385
>  lock_acquire+0x259/0x620 kernel/locking/lockdep.c:4002
>  flush_work+0xf0/0x8c0 kernel/workqueue.c:2886
>  __cancel_work_timer+0x51d/0x870 kernel/workqueue.c:2961
>  cancel_delayed_work_sync+0x1f/0x30 kernel/workqueue.c:3081
>  p54_unregister_leds+0x6c/0xc0 drivers/net/wireless/intersil/p54/led.c:160
>  p54_unregister_common+0x3d/0xb0 drivers/net/wireless/intersil/p54/main.c:856
>  p54u_disconnect+0x86/0x120 drivers/net/wireless/intersil/p54/p54usb.c:1073
>  usb_unbind_interface+0x21c/0xa90 drivers/usb/core/driver.c:423
>  __device_release_driver drivers/base/dd.c:861
>  device_release_driver_internal+0x4f4/0x5c0 drivers/base/dd.c:893
>  device_release_driver+0x1e/0x30 drivers/base/dd.c:918
>  bus_remove_device+0x2f4/0x4b0 drivers/base/bus.c:565
>  device_del+0x5c4/0xab0 drivers/base/core.c:1985
>  usb_disable_device+0x1e9/0x680 drivers/usb/core/message.c:1170
>  usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124
>  hub_port_connect drivers/usb/core/hub.c:4754
>  hub_port_connect_change drivers/usb/core/hub.c:5009
>  port_event drivers/usb/core/hub.c:5115
>  hub_event+0x1318/0x3740 drivers/usb/core/hub.c:5195
>  process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
>  process_scheduled_works kernel/workqueue.c:2179
>  worker_thread+0xb2b/0x1850 kernel/workqueue.c:2255
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> 

It seems this is caused as a result of:
-> lock_map_acquire(>lockdep_map);
lock_map_release(>lockdep_map);

in flush_work() [0]

This was added by:

commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
Author: Stephen Boyd 
Date:   Fri Apr 20 17:28:50 2012 -0700

workqueue: Catch more locking problems with flush_work()

Looking at the Stephen's patch, it's clear that it was made
with "static DECLARE_WORK(work, my_work)" in mind. However
p54's led_work is "per-device", hence it is stored in the
devices context p54_common, which is dynamically allocated.
So, maybe revert Stephen's patch?

[0] 

[PATCH] net: emac: Fix napi poll list corruption

[Christian Lamparter]

This patch is pretty much a carbon copy of
commit 3079c652141f ("caif: Fix napi poll list corruption")
with "caif" replaced by "emac".

The commit d75b1ade567f ("net: less interrupt masking in NAPI")
breaks emac.

It is now required that if the entire budget is consumed when poll
returns, the napi poll_list must remain empty.  However, like some
other drivers emac tries to do a last-ditch check and if there is
more work it will call napi_reschedule and then immediately process
some of this new work.  Should the entire budget be consumed while
processing such new work then we will violate the new caller
contract.

This patch fixes this by not touching any work when we reschedule
in emac.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/mal.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/mal.c 
b/drivers/net/ethernet/ibm/emac/mal.c
index 2c74baa2398a..fff09dcf9e34 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -402,7 +402,7 @@ static int mal_poll(struct napi_struct *napi, int budget)
unsigned long flags;
 
MAL_DBG2(mal, "poll(%d)" NL, budget);
- again:
+
/* Process TX skbs */
list_for_each(l, >poll_list) {
struct mal_commac *mc =
@@ -451,7 +451,6 @@ static int mal_poll(struct napi_struct *napi, int budget)
spin_lock_irqsave(>lock, flags);
mal_disable_eob_irq(mal);
spin_unlock_irqrestore(>lock, flags);
-   goto again;
}
mc->ops->poll_tx(mc->dev);
}
-- 
2.14.1

Re: hung task in mac80211

[Christian Lamparter]

On Wednesday, September 6, 2017 1:57:47 PM CEST Matteo Croce wrote:
> Hi,
> 
> I have an hung task on vanilla 4.13 kernel which I haven't on 4.12.
> The problem is present both on my AP and on my notebook,
> so it seems it affects AP and STA mode as well.
> The generated messages are:
> 
> INFO: task kworker/u16:6:120 blocked for more than 120 seconds.
>   Not tainted 4.13.0 #57
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> kworker/u16:6   D0   120  2 0x
> Workqueue: phy0 ieee80211_ba_session_work [mac80211]
> Call Trace:
>  ? __schedule+0x174/0x5b0
>  ? schedule+0x31/0x80
>  ? schedule_preempt_disabled+0x9/0x10
>  ? __mutex_lock.isra.2+0x163/0x480
>  ? select_task_rq_fair+0xb9f/0xc60
>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>  ? __ieee80211_start_rx_ba_session+0x135/0x4d0 [mac80211]
>  ? try_to_wake_up+0x1f1/0x340
>  ? update_curr+0x88/0xd0
>  ? ieee80211_ba_session_work+0x148/0x230 [mac80211]
> 
> I did a bisect and the offending commit is:
> 
> commit 699cb58c8a52ff39bf659bff7971893ebe111bf2
> Author: Johannes Berg 
> Date:   Tue May 30 16:34:46 2017 +0200
> 
> mac80211: manage RX BA session offload without SKB queue

I looked at this briefly:

ieee80211_ba_session_work acquires:
mutex_lock(>ampdu_mlme.mtx) @


But it now also calls
__ieee80211_start_rx_ba_session() @
http://elixir.free-electrons.com/linux/v4.13/source/net/mac80211/ht.c#L336

which also wants to hold mutex_lock(>ampdu_mlme.mtx) in:
http://elixir.free-electrons.com/linux/v4.13/source/net/mac80211/agg-rx.c#L314

I guess this is where it deadlocks?

Regards,
Christian

Re: [PATCH] net: ibm: emac: Fix some error handling path in 'emac_probe()'

[Christian Lamparter]

On Saturday, August 19, 2017 1:07:57 AM CEST Christophe JAILLET wrote:
> If 'irq_of_parse_and_map()' or 'of_address_to_resource()' fail, 'err' is
> known to be 0 at this point.
> So return -ENODEV instead in the first case and propagate the error
> returned by 'of_address_to_resource()' in the 2nd case.
> 
> While at it, turn a 'err != 0' test into an equivalent 'err' to be more
> consistent.
> 
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/net/ethernet/ibm/emac/core.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/core.c 
> b/drivers/net/ethernet/ibm/emac/core.c
> index 95135d20458f..1af56a97fb47 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> [...]
>   /* Map EMAC regs */
> - if (of_address_to_resource(np, 0, >rsrc_regs)) {
> + err = of_address_to_resource(np, 0, >rsrc_regs);
> + if (err) {
>   printk(KERN_ERR "%pOF: Can't get registers address\n", np);
>   goto err_irq_unmap;
>   }
>  // TODO : request_mem_region
>  dev->emacp = ioremap(dev->rsrc_regs.start,
>   resource_size(>rsrc_regs));
>  ...

If you want to go for 101%: you could get rid of this block
altogether by doing: 
dev->emacp = of_iomap(np, 0);

Note1:
This will also make the rsrc_regs variable in the emac_instance
struct redundant. So simply remove it from the core.h.

Note2: if you want to go for 110%, you could replace this with
platform_get_resource() and devm_ioremap_resource() (if you
are interested, take a look at devm_ioremap_resource's kdoc
it has an example).

Thanks,
Christian

[PATCH v2 1/2] net: emac: fix reset timeout with AR8035 phy

[Christian Lamparter]

This patch fixes a problem where the AR8035 PHY can't be
detected on an Cisco Meraki MR24, if the ethernet cable is
not connected on boot.

Russell Senior provided steps to reproduce the issue:
|Disconnect ethernet cable, apply power, wait until device has booted,
|plug in ethernet, check for interfaces, no eth0 is listed.
|
|This appears to be a problem during probing of the AR8035 Phy chip.
|When ethernet has no link, the phy detection fails, and eth0 is not
|created. Plugging ethernet later has no effect, because there is no
|interface as far as the kernel is concerned. The relevant part of
|the boot log looks like this:
|this is the failing case:
|
|[0.876611] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode
|[0.882532] /plb/opb/ethernet@ef600c00: reset timeout
|[0.888546] /plb/opb/ethernet@ef600c00: can't find PHY!
|and the succeeding case:
|
|[0.876672] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode
|[0.883952] eth0: EMAC-0 /plb/opb/ethernet@ef600c00, MAC 00:01:..
|[0.890822] eth0: found Atheros 8035 Gigabit Ethernet PHY (0x01)

Based on the comment and the commit message of
commit 23fbb5a87c56 ("emac: Fix EMAC soft reset on 460EX/GT").
This is because the AR8035 PHY doesn't provide the TX Clock,
if the ethernet cable is not attached. This causes the reset
to timeout and the PHY detection code in emac_init_phy() is
unable to detect the AR8035 PHY. As a result, the emac driver
bails out early and the user left with no ethernet.

In order to stay compatible with existing configurations, the driver
tries the current reset approach at first. Only if the first attempt
timed out, it does perform one more retry with the clock temporarily
switched to the internal source for just the duration of the reset.

LEDE-Bug: #687 <https://bugs.lede-project.org/index.php?do=details_id=687>

Cc: Chris Blake <chrisrblak...@gmail.com>
Reported-by: Russell Senior <russ...@personaltelco.net>
Fixes: 23fbb5a87c56e98 ("emac: Fix EMAC soft reset on 460EX/GT")
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
v1 -> v2:
- made it clear, that the clock source is only switched
  temporarily.
- fixed missing goto label, if !CONFIG_PPC_DCR_NATIVE
---
 drivers/net/ethernet/ibm/emac/core.c | 26 ++
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 508923f39ccf..b6e871bfb659 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -343,6 +343,7 @@ static int emac_reset(struct emac_instance *dev)
 {
struct emac_regs __iomem *p = dev->emacp;
int n = 20;
+   bool __maybe_unused try_internal_clock = false;
 
DBG(dev, "reset" NL);
 
@@ -355,6 +356,7 @@ static int emac_reset(struct emac_instance *dev)
}
 
 #ifdef CONFIG_PPC_DCR_NATIVE
+do_retry:
/*
 * PPC460EX/GT Embedded Processor Advanced User's Manual
 * section 28.10.1 Mode Register 0 (EMACx_MR0) states:
@@ -362,10 +364,19 @@ static int emac_reset(struct emac_instance *dev)
 * of the EMAC. If none is present, select the internal clock
 * (SDR0_ETH_CFG[EMACx_PHY_CLK] = 1).
 * After a soft reset, select the external clock.
+*
+* The AR8035-A PHY Meraki MR24 does not provide a TX Clk if the
+* ethernet cable is not attached. This causes the reset to timeout
+* and the PHY detection code in emac_init_phy() is unable to
+* communicate and detect the AR8035-A PHY. As a result, the emac
+* driver bails out early and the user has no ethernet.
+* In order to stay compatible with existing configurations, the
+* driver will temporarily switch to the internal clock, after
+* the first reset fails.
 */
if (emac_has_feature(dev, EMAC_FTR_460EX_PHY_CLK_FIX)) {
-   if (dev->phy_address == 0x &&
-   dev->phy_map == 0x) {
+   if (try_internal_clock || (dev->phy_address == 0x &&
+  dev->phy_map == 0x)) {
/* No PHY: select internal loop clock before reset */
dcri_clrset(SDR0, SDR0_ETH_CFG,
0, SDR0_ETH_CFG_ECS << dev->cell_index);
@@ -383,8 +394,15 @@ static int emac_reset(struct emac_instance *dev)
 
 #ifdef CONFIG_PPC_DCR_NATIVE
if (emac_has_feature(dev, EMAC_FTR_460EX_PHY_CLK_FIX)) {
-   if (dev->phy_address == 0x &&
-   dev->phy_map == 0x) {
+   if (!n && !try_internal_clock) {
+   /* first attempt has timed out. */
+   n = 20;
+   try_internal_clock = true;
+

[PATCH v2 2/2] net: emac: fix and unify emac_mdio functions

[Christian Lamparter]

emac_mdio_read_link() was not copying the requested phy settings
back into the emac driver's own phy api. This has caused a link
speed mismatch issue for the AR8035 as the emac driver kept
trying to connect with 10/100MBps on a 1GBit/s link.

This patch also unifies shared code between emac_setup_aneg()
and emac_mdio_setup_forced(). And furthermore it removes
a chunk of emac_mdio_init_phy(), that was copying the same
data into itself.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 41 
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index b6e871bfb659..259e69a52ec5 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2478,20 +2478,24 @@ static int emac_mii_bus_reset(struct mii_bus *bus)
return emac_reset(dev);
 }
 
+static int emac_mdio_phy_start_aneg(struct mii_phy *phy,
+   struct phy_device *phy_dev)
+{
+   phy_dev->autoneg = phy->autoneg;
+   phy_dev->speed = phy->speed;
+   phy_dev->duplex = phy->duplex;
+   phy_dev->advertising = phy->advertising;
+   return phy_start_aneg(phy_dev);
+}
+
 static int emac_mdio_setup_aneg(struct mii_phy *phy, u32 advertise)
 {
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
 
-   dev->phy.autoneg = AUTONEG_ENABLE;
-   dev->phy.speed = SPEED_1000;
-   dev->phy.duplex = DUPLEX_FULL;
-   dev->phy.advertising = advertise;
phy->autoneg = AUTONEG_ENABLE;
-   phy->speed = dev->phy.speed;
-   phy->duplex = dev->phy.duplex;
phy->advertising = advertise;
-   return phy_start_aneg(dev->phy_dev);
+   return emac_mdio_phy_start_aneg(phy, dev->phy_dev);
 }
 
 static int emac_mdio_setup_forced(struct mii_phy *phy, int speed, int fd)
@@ -2499,13 +2503,10 @@ static int emac_mdio_setup_forced(struct mii_phy *phy, 
int speed, int fd)
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
 
-   dev->phy.autoneg =  AUTONEG_DISABLE;
-   dev->phy.speed = speed;
-   dev->phy.duplex = fd;
phy->autoneg = AUTONEG_DISABLE;
phy->speed = speed;
phy->duplex = fd;
-   return phy_start_aneg(dev->phy_dev);
+   return emac_mdio_phy_start_aneg(phy, dev->phy_dev);
 }
 
 static int emac_mdio_poll_link(struct mii_phy *phy)
@@ -2527,16 +2528,17 @@ static int emac_mdio_read_link(struct mii_phy *phy)
 {
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
+   struct phy_device *phy_dev = dev->phy_dev;
int res;
 
-   res = phy_read_status(dev->phy_dev);
+   res = phy_read_status(phy_dev);
if (res)
return res;
 
-   dev->phy.speed = phy->speed;
-   dev->phy.duplex = phy->duplex;
-   dev->phy.pause = phy->pause;
-   dev->phy.asym_pause = phy->asym_pause;
+   phy->speed = phy_dev->speed;
+   phy->duplex = phy_dev->duplex;
+   phy->pause = phy_dev->pause;
+   phy->asym_pause = phy_dev->asym_pause;
return 0;
 }
 
@@ -2546,13 +2548,6 @@ static int emac_mdio_init_phy(struct mii_phy *phy)
struct emac_instance *dev = netdev_priv(ndev);
 
phy_start(dev->phy_dev);
-   dev->phy.autoneg = phy->autoneg;
-   dev->phy.speed = phy->speed;
-   dev->phy.duplex = phy->duplex;
-   dev->phy.advertising = phy->advertising;
-   dev->phy.pause = phy->pause;
-   dev->phy.asym_pause = phy->asym_pause;
-
return phy_init_hw(dev->phy_dev);
 }
 
-- 
2.11.0

Re: [PATCH v1 2/2] net: emac: fix and unify emac_mdio functions

[Christian Lamparter]

On Monday, June 5, 2017 11:43:33 PM CEST Andrew Lunn wrote:
> On Mon, Jun 05, 2017 at 10:49:40PM +0200, Christian Lamparter wrote:
> > emac_mdio_read_link() was not copying the requested phy settings
> > back into the emac driver's own phy api. This has caused a link
> > speed mismatch issue for the AR8035 as the emac driver kept
> > trying to connect with 10/100MBps on a 1GBit/s link.

> In the long run, you might want to remove the emac phy drivers. 
> Linux has PHYLIB drivers for all but the bcm5248.

Hello Andrew

Back in February I added PHYLIB support to emac.
<https://www.spinics.net/lists/netdev/msg421734.html> ;)

I could add a separate patch that adds a oneliner message like:
pr_info("EMAC supports PHYLIB. Please convert your device to it.\n");
at the right place (emac_mii_phy_probe) to let people know about it.

Is that Ok? If not, please tell me what the appropriate deprecation 
notice should look like.

About the MR24:
I do have a patchset to convert the MR24 to use PHYLIB's at803x as well.
<https://github.com/chunkeey/apm82181-lede/commit/82c50b7f4fca68ce905d4a7a3559700635bf227f>
But because of AT8035 "special tx/rx delay" requirements, this
will need a patched at803x.c driver as well.

Regards,
Christian

Re: [PATCH v1 1/2] net: emac: fix reset timeout with AR8035 phy

[Christian Lamparter]

On Monday, June 5, 2017 11:26:17 PM CEST Andrew Lunn wrote:
> > In order to stay compatible with existing configurations, the
> > driver will try the normal reset first and only falls back to
> > to the internal clock, after the first reset fails. If the
> > second reset fails as well, it will give up as before.
> 
> Hi Christian
> 
> This gets things probed correctly. But should you swap back to the PHY
> clock when the PHY declares the link up? Is there code already to do
> this?
> 
>   Andrew
> 
Oh, sorry. I omitted this from the commit message. But the proposed 
emac_reset() code  switched to the internal clock only after the first attempt
has failed AND only for the duration of the reset.

If the reset succeeds or the reset times out, the clock is always switched 
back to the external clock.

Thanks,
Christian

[PATCH v1 1/2] net: emac: fix reset timeout with AR8035 phy

[Christian Lamparter]

This patch fixes a problem where the AR8035 PHY can't be
detected on an Cisco Meraki MR24, if the ethernet cable is
not connected on boot.

Russell Senior provided steps to reproduce the issue:
|Disconnect ethernet cable, apply power, wait until device has booted,
|plug in ethernet, check for interfaces, no eth0 is listed.
|
|This appears to be a problem during probing of the AR8035 Phy chip.
|When ethernet has no link, the phy detection fails, and eth0 is not
|created. Plugging ethernet later has no effect, because there is no
|interface as far as the kernel is concerned. The relevant part of
|the boot log looks like this:
|this is the failing case:
|
|[0.876611] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode
|[0.882532] /plb/opb/ethernet@ef600c00: reset timeout
|[0.888546] /plb/opb/ethernet@ef600c00: can't find PHY!
|and the succeeding case:
|
|[0.876672] /plb/opb/emac-rgmii@ef601500: input 0 in RGMII mode
|[0.883952] eth0: EMAC-0 /plb/opb/ethernet@ef600c00, MAC 00:01:..
|[0.890822] eth0: found Atheros 8035 Gigabit Ethernet PHY (0x01)

Based on the comment and the commit message of
commit 23fbb5a87c56 ("emac: Fix EMAC soft reset on 460EX/GT").
This is because the AR8035 PHY of the Meraki MR24 does not
provide the TX Clk, if the ethernet cable is not attached.
This causes the reset to timeout and the PHY detection code
in emac_init_phy() is unable to detect the AR8035 PHY.
As a result, the emac driver bails out early and the user
has no ethernet.

In order to stay compatible with existing configurations, the
driver will try the normal reset first and only falls back to
to the internal clock, after the first reset fails. If the
second reset fails as well, it will give up as before.

LEDE-Bug: #687 <https://bugs.lede-project.org/index.php?do=details_id=687>

Cc: Chris Blake <chrisrblak...@gmail.com>
Reported-by: Russell Senior <russ...@personaltelco.net>
Fixes: 23fbb5a87c56e98 ("emac: Fix EMAC soft reset on 460EX/GT")
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 508923f39ccf..18af1116fa1d 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -343,6 +343,7 @@ static int emac_reset(struct emac_instance *dev)
 {
struct emac_regs __iomem *p = dev->emacp;
int n = 20;
+   bool try_internal_clock = false;
 
DBG(dev, "reset" NL);
 
@@ -355,6 +356,7 @@ static int emac_reset(struct emac_instance *dev)
}
 
 #ifdef CONFIG_PPC_DCR_NATIVE
+do_retry:
/*
 * PPC460EX/GT Embedded Processor Advanced User's Manual
 * section 28.10.1 Mode Register 0 (EMACx_MR0) states:
@@ -362,10 +364,19 @@ static int emac_reset(struct emac_instance *dev)
 * of the EMAC. If none is present, select the internal clock
 * (SDR0_ETH_CFG[EMACx_PHY_CLK] = 1).
 * After a soft reset, select the external clock.
+*
+* The AR8035-A PHY Meraki MR24 does not provide a TX Clk if the
+* ethernet cable is not attached. This causes the reset to timeout
+* and the PHY detection code in emac_init_phy() is unable to
+* communicate and detect the AR8035-A PHY. As a result, the emac
+* driver bails out early and the user has no ethernet.
+* In order to stay compatible with existing configurations, the
+* driver will fall back to and switch to the internal clock, after
+* the first reset fails.
 */
if (emac_has_feature(dev, EMAC_FTR_460EX_PHY_CLK_FIX)) {
-   if (dev->phy_address == 0x &&
-   dev->phy_map == 0x) {
+   if (try_internal_clock || (dev->phy_address == 0x &&
+  dev->phy_map == 0x)) {
/* No PHY: select internal loop clock before reset */
dcri_clrset(SDR0, SDR0_ETH_CFG,
0, SDR0_ETH_CFG_ECS << dev->cell_index);
@@ -383,8 +394,8 @@ static int emac_reset(struct emac_instance *dev)
 
 #ifdef CONFIG_PPC_DCR_NATIVE
if (emac_has_feature(dev, EMAC_FTR_460EX_PHY_CLK_FIX)) {
-   if (dev->phy_address == 0x &&
-   dev->phy_map == 0x) {
+   if (try_internal_clock || (dev->phy_address == 0x &&
+  dev->phy_map == 0x)) {
/* No PHY: restore external clock source after reset */
dcri_clrset(SDR0, SDR0_ETH_CFG,
SDR0_ETH_CFG_ECS << dev->cell_index, 0);
@@ -396,9 +407,16 

[PATCH v1 2/2] net: emac: fix and unify emac_mdio functions

[Christian Lamparter]

emac_mdio_read_link() was not copying the requested phy settings
back into the emac driver's own phy api. This has caused a link
speed mismatch issue for the AR8035 as the emac driver kept
trying to connect with 10/100MBps on a 1GBit/s link.

This patch also unifies shared code between emac_setup_aneg()
and emac_mdio_setup_forced(). And furthermore it removes
a chunk of emac_mdio_init_phy(), that was copying the same
data into itself.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 41 
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 18af1116fa1d..8cfb148cfdb0 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2478,20 +2478,24 @@ static int emac_mii_bus_reset(struct mii_bus *bus)
return emac_reset(dev);
 }
 
+static int emac_mdio_phy_start_aneg(struct mii_phy *phy,
+   struct phy_device *phy_dev)
+{
+   phy_dev->autoneg = phy->autoneg;
+   phy_dev->speed = phy->speed;
+   phy_dev->duplex = phy->duplex;
+   phy_dev->advertising = phy->advertising;
+   return phy_start_aneg(phy_dev);
+}
+
 static int emac_mdio_setup_aneg(struct mii_phy *phy, u32 advertise)
 {
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
 
-   dev->phy.autoneg = AUTONEG_ENABLE;
-   dev->phy.speed = SPEED_1000;
-   dev->phy.duplex = DUPLEX_FULL;
-   dev->phy.advertising = advertise;
phy->autoneg = AUTONEG_ENABLE;
-   phy->speed = dev->phy.speed;
-   phy->duplex = dev->phy.duplex;
phy->advertising = advertise;
-   return phy_start_aneg(dev->phy_dev);
+   return emac_mdio_phy_start_aneg(phy, dev->phy_dev);
 }
 
 static int emac_mdio_setup_forced(struct mii_phy *phy, int speed, int fd)
@@ -2499,13 +2503,10 @@ static int emac_mdio_setup_forced(struct mii_phy *phy, 
int speed, int fd)
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
 
-   dev->phy.autoneg =  AUTONEG_DISABLE;
-   dev->phy.speed = speed;
-   dev->phy.duplex = fd;
phy->autoneg = AUTONEG_DISABLE;
phy->speed = speed;
phy->duplex = fd;
-   return phy_start_aneg(dev->phy_dev);
+   return emac_mdio_phy_start_aneg(phy, dev->phy_dev);
 }
 
 static int emac_mdio_poll_link(struct mii_phy *phy)
@@ -2527,16 +2528,17 @@ static int emac_mdio_read_link(struct mii_phy *phy)
 {
struct net_device *ndev = phy->dev;
struct emac_instance *dev = netdev_priv(ndev);
+   struct phy_device *phy_dev = dev->phy_dev;
int res;
 
-   res = phy_read_status(dev->phy_dev);
+   res = phy_read_status(phy_dev);
if (res)
return res;
 
-   dev->phy.speed = phy->speed;
-   dev->phy.duplex = phy->duplex;
-   dev->phy.pause = phy->pause;
-   dev->phy.asym_pause = phy->asym_pause;
+   phy->speed = phy_dev->speed;
+   phy->duplex = phy_dev->duplex;
+   phy->pause = phy_dev->pause;
+   phy->asym_pause = phy_dev->asym_pause;
return 0;
 }
 
@@ -2546,13 +2548,6 @@ static int emac_mdio_init_phy(struct mii_phy *phy)
struct emac_instance *dev = netdev_priv(ndev);
 
phy_start(dev->phy_dev);
-   dev->phy.autoneg = phy->autoneg;
-   dev->phy.speed = phy->speed;
-   dev->phy.duplex = phy->duplex;
-   dev->phy.advertising = phy->advertising;
-   dev->phy.pause = phy->pause;
-   dev->phy.asym_pause = phy->asym_pause;
-
return phy_init_hw(dev->phy_dev);
 }
 
-- 
2.11.0

Re: [PATCH] p54: add null pointer check before releasing socket buffer

[Christian Lamparter]

On Monday, April 10, 2017 1:54:14 PM CEST Myungho Jung wrote:
> On Mon, Apr 10, 2017 at 02:12:54PM +0200, Christian Lamparter wrote:
> > On Sunday, April 9, 2017 10:23:20 PM CEST Myungho Jung wrote:
> > > Kernel panic is caused by trying to dereference null pointer. Check if
> > > the pointer is null before freeing space.
> >  [...]
> > As for adding if (!skb) checks. I think kfree, kfree_skb, dev_kfree_skb 
> > (aka consume_skb) all check for null pointers already. So the logical
> > thing to do would be to make dev_kfree_skb_irq (which would also fix
> > dev_kfree_skb_any) consistent with kfree, kfree_skb, dev_kfree_skb and
> > add the check there.
> > > ---
> > >  drivers/net/wireless/intersil/p54/txrx.c | 8 ++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/intersil/p54/txrx.c 
> > > b/drivers/net/wireless/intersil/p54/txrx.c
> > > index 1af7da0..8956061 100644
> > > --- a/drivers/net/wireless/intersil/p54/txrx.c
> > > +++ b/drivers/net/wireless/intersil/p54/txrx.c
> > > @@ -503,7 +503,9 @@ static void p54_rx_eeprom_readback(struct p54_common 
> > > *priv,
> > >  
> > >   priv->eeprom = NULL;
> > >   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> > > - dev_kfree_skb_any(tmp);
> > > + if (unlikely(!tmp))
> > > + dev_kfree_skb_any(tmp);
> > > +
> > >   complete(>eeprom_comp);
> > >  }
> > >  
> > > @@ -597,7 +599,9 @@ static void p54_rx_stats(struct p54_common *priv, 
> > > struct sk_buff *skb)
> > >   }
> > >  
> > >   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> > > - dev_kfree_skb_any(tmp);
> > > + if (unlikely(!tmp))
> > > + dev_kfree_skb_any(tmp);
> > > +
> > >   complete(>stat_comp);
> > >  }
> > >  
> > > 
> > 
> > 
> [...] I'm not sure it actually caused kernel panic but guessed from
> a bug report [https://bugzilla.kernel.org/show_bug.cgi?id=195289].
Thank you for bringing this to my attention. Reading the bugreport, it
does sound like there's a bigger issue with the USB Ports. I'll see if
this can be fixed. But it does sound like a hardware issue at this 
point.

> And correct fix will be like this:
>   if (likely(tmp))
>   dev_kfree_skb_any(tmp);
> 
> But, like you said, I think null pointer should be checked in
> dev_kfree_skb_irq although already checking before calling it in many
> other places. I'll try another patch. Thank you for your advice.

Well, the patch could be as simple as this:
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 7869ae3837ca..44f7d5a1c67c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum 
skb_free_reason reason)
 {
unsigned long flags;
 
+   if (!skb)
+   return;
+
if (likely(atomic_read(>users) == 1)) {
smp_rmb();
atomic_set(>users, 0);
---

The question is: would David or Eric support the change. Any comments,
what's the prefered solution? Just patch __dev_kfree_skb_irq to make
it consistent with *kfree*, or patch the driver? I'm fine either way,
but I would prefere patching __dev_kfree_skb_irq.

Regards,
Christian

Re: [PATCH] p54: add null pointer check before releasing socket buffer

[Christian Lamparter]

(Added linux-wireless, since this is a wireless driver)

On Sunday, April 9, 2017 10:23:20 PM CEST Myungho Jung wrote:
> Kernel panic is caused by trying to dereference null pointer. Check if
> the pointer is null before freeing space.
Do you have the kernel panic somewhere?
I think you have an even bigger problem: You see, in order to get EEPROM
readback and rx_stats feedback you need to sent a request to the firmware
and if the response's req_id cookies don't match, you end up filling up 
the very limited device address space.

As for adding if (!skb) checks. I think kfree, kfree_skb, dev_kfree_skb 
(aka consume_skb) all check for null pointers already. So the logical
thing to do would be to make dev_kfree_skb_irq (which would also fix
dev_kfree_skb_any) consistent with kfree, kfree_skb, dev_kfree_skb and
add the check there.

> Signed-off-by: Myungho Jung 
> ---
>  drivers/net/wireless/intersil/p54/txrx.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c 
> b/drivers/net/wireless/intersil/p54/txrx.c
> index 1af7da0..8956061 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -503,7 +503,9 @@ static void p54_rx_eeprom_readback(struct p54_common 
> *priv,
>  
>   priv->eeprom = NULL;
>   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> - dev_kfree_skb_any(tmp);
> + if (unlikely(!tmp))
> + dev_kfree_skb_any(tmp);
> +
>   complete(>eeprom_comp);
>  }
>  
> @@ -597,7 +599,9 @@ static void p54_rx_stats(struct p54_common *priv, struct 
> sk_buff *skb)
>   }
>  
>   tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
> - dev_kfree_skb_any(tmp);
> + if (unlikely(!tmp))
> + dev_kfree_skb_any(tmp);
> +
>   complete(>stat_comp);
>  }
>  
> 

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 6:41:59 PM CEST Andrew Lunn wrote:
> > Oh, in that case you should probably go "all out" and ask on the 
> > LKML to remove all of the ath9k and ath10k ahb work. From what I
> > know all the "users" are running some sort of OpenWRT/LEDE or a 
> > derivative. This is because Atheros/QCA provided a SDK based on
> > OpenWRT.
> > 
> > Alban has been trying to convert the platform to device-tree
> > and add them to the mainline for a while now:
> >  
> > https://patchwork.kernel.org/patch/6514551/
> > 
> > So, you are questioning this work as well.
> 
> Not at all. Ralph Sennhauser has been doing a great job of getting all
> the Marvell devices into Mainline, and i help as much as i can, being
> one of the Marvell SoC Maintainers.
> 
> I'm just saying, get a few boards which require these facilities into
> the mainline, and then you have a much stronger base to argue from.

I was arguing not to deprecate "qca,no-eeprom" property.

based on this quote from Linus' :
|if a new interface is truly more flexible, then it should be able
|to implement the old interface with no changes, so that drivers
|shouldn't need to be changed/upgraded.

what stronger point to do you want?

Thanks,
Christian

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 5:18:37 PM CEST Andrew Lunn wrote:
> > But LEDE/OpenWRT rely on the firmware loading API more than ever and 
> > currently there is not a replacement for it.
> 
> 
> 
> 
> > I looked into 10-ath9k-eeprom [0] of LEDE's AR71XX target and I noticed
> > that quite a few devices patch the MACs of the wifi.
> > If you look at the code for the Airtight C-55 and C-60, Meraki MR18,
> > Meraki Z1, you'll notice that each one has to add a fixed value (+1,
> > +2, ...) to the extraced MAC-Address. So how would you replicate this,
> > with "nvmem-cell-names = address" without some sort of 
> > nvmem-provider-processor?
> 
> ...
>  
> > https://github.com/lede-project/source/blob/master/target/linux/ar71xx/files/arch/mips/ath79/dev-eth.c#L1204
> > 
> > and grep lists the following devices:
> > mach-dgl-5500-a1.c, mach-dhp-1565-a1.c, mach-dir-505-a1.c, mach-dir-615-c1.c
> > mach-dir-615-i1.c, mach-dir-825-b1.c, mach-dir-825-c1.c, mach-tew-673gru.c
> > mach-tew-712br.c, mach-tew-732br.c, mach-tew-823dru.c
> 
> I would say a big part of the problem is that all of these use cases
> are outside of mainline. Why should mainline support something which
> is not actually used in mainline.
> 
> So i would suggest your first step is to bring some of these devices
> into mainline. Once in mainline, it becomes a mainline issue, and
> people will help get it solved.
>
Oh, in that case you should probably go "all out" and ask on the 
LKML to remove all of the ath9k and ath10k ahb work. From what I
know all the "users" are running some sort of OpenWRT/LEDE or a 
derivative. This is because Atheros/QCA provided a SDK based on
OpenWRT.

Alban has been trying to convert the platform to device-tree
and add them to the mainline for a while now:
 
https://patchwork.kernel.org/patch/6514551/

So, you are questioning this work as well.

Thanks,
Christian

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Tuesday, March 28, 2017 10:44:41 AM CEST Alban wrote:
> On Mon, 27 Mar 2017 18:11:15 +0200
> Christian Lamparter <chunk...@googlemail.com> wrote:
> 
> > On Monday, March 13, 2017 10:05:09 PM CEST Alban wrote:
> > > The current binding only cover PCI devices so extend it for SoC devices.
> > > 
> > > Most SoC platforms use an MTD partition for the calibration data
> > > instead of an EEPROM. The qca,no-eeprom property was added to allow
> > > loading the EEPROM content using firmware loading. This new binding
> > > replace this hack with NVMEM cells, so we also mark the qca,no-eeprom
> > > property as deprecated in case anyone ever used it.  
> > 
> > Please don't mark "qca,no-eeprom" as deprecated then.
> > If some devices geniously need to rely on userspace for extracting 
> > and processing the calibration data, it should be stay a
> > optional properties.
> 
> Deprecated just mean that it shouldn't be used for new devices. 
> But as it is not used by any board, misuse the firmware loading API and
> firmware loader user helper are deprecated in udev, I find we could also
> just drop it.
But LEDE/OpenWRT rely on the firmware loading API more than ever and 
currently there is not a replacement for it. From what I know, Luis 
tried to replace it with his sysdata approach:

<https://lkml.org/lkml/2016/12/16/204>

however, this was disliked by Greg KH and Linus for the following reasons.
<https://lkml.org/lkml/2016/6/16/995>:

|So I absolutely abhor "changes for changes sake".
|
|If the existing code works for existing drivers, let them keep it.
|
|And if a new interface is truly more flexible, then it should be able
|to implement the old interface with no changes, so that drivers
|shouldn't need to be changed/upgraded.
|
|Then, drivers that actually _want_ new features, or that can take
|advantage of new interfaces to actually make things *simpler*, can
|choose to make those changes. But those changes should have real
|advantages.
|[...]


your nvmem approach would need to be as universal and
powerful as the "qca,no-eeprom" + userspace solutions
in order to deprecate it.
 
> > For example: A device that can't do easily without "qca,no-eeprom" is
> > the AVM FRITZ!WLAN Repeater 300E. For this device, the caldata 
> > is stored in the flash, however for whatever reason the vendor
> > choose to "reverse" it. (like completely back to front, not byteswapped
> > or something). So an extra "unreversing step" is required. So, it would
> > require some sort of a special nvmem-provider-processor as an 
> > alternative.
> 
> Or just handle this special eeprom format in the ath9k driver. I doubt
> that this case is so common that it would justify adding a whole new
> layer to nvmem.
Well, you'll have to deal with it in nvmem, if you want it to deprecate
"qca,no-eeprom".

I looked into 10-ath9k-eeprom [0] of LEDE's AR71XX target and I noticed
that quite a few devices patch the MACs of the wifi.
If you look at the code for the Airtight C-55 and C-60, Meraki MR18,
Meraki Z1, you'll notice that each one has to add a fixed value (+1,
+2, ...) to the extraced MAC-Address. So how would you replicate this,
with "nvmem-cell-names = address" without some sort of 
nvmem-provider-processor?

Also, there's another usecase of a nvmem-provider-processor. 
For example, one could be convert all the different types of
ascii-macs (Either strings like "00:11:22:33:44:55", 
"00.11.22.33..." or "00112233..." ) to their binary representation.
For AR71XX, this is mostly done by ath79_parse_ascii_mac:

https://github.com/lede-project/source/blob/master/target/linux/ar71xx/files/arch/mips/ath79/dev-eth.c#L1204

and grep lists the following devices:
mach-dgl-5500-a1.c, mach-dhp-1565-a1.c, mach-dir-505-a1.c, mach-dir-615-c1.c
mach-dir-615-i1.c, mach-dir-825-b1.c, mach-dir-825-c1.c, mach-tew-673gru.c
mach-tew-712br.c, mach-tew-732br.c, mach-tew-823dru.c

I did a quick check: All of them use the extracted MACs for ath9k
and/or ethernet.

Note: I think Ralink/MediaTek will have the same issues.

> > >  Optional properties:
> > > +- mac-address: See ethernet.txt in the parent directory
> > > +- local-mac-address: See ethernet.txt in the parent directory
> > > [...]
> > > +
> > > +Deprecated properties:
> > >  - qca,no-eeprom: Indicates that there is no physical EEPROM
> > > connected to the ath9k wireless chip (in this case the calibration /
> > >   EEPROM data will be loaded from userspace
> > > using the kernel firmware loader).
> > > -- mac-address: See ethernet.txt in the parent directory
> > > -- local-mac-addres

Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices

[Christian Lamparter]

On Monday, March 13, 2017 10:05:09 PM CEST Alban wrote:
> The current binding only cover PCI devices so extend it for SoC devices.
> 
> Most SoC platforms use an MTD partition for the calibration data
> instead of an EEPROM. The qca,no-eeprom property was added to allow
> loading the EEPROM content using firmware loading. This new binding
> replace this hack with NVMEM cells, so we also mark the qca,no-eeprom
> property as deprecated in case anyone ever used it.

Please don't mark "qca,no-eeprom" as deprecated then.
If some devices geniously need to rely on userspace for extracting 
and processing the calibration data, it should be stay a
optional properties.

For example: A device that can't do easily without "qca,no-eeprom" is
the AVM FRITZ!WLAN Repeater 300E. For this device, the caldata 
is stored in the flash, however for whatever reason the vendor
choose to "reverse" it. (like completely back to front, not byteswapped
or something). So an extra "unreversing step" is required. So, it would
require some sort of a special nvmem-provider-processor as an 
alternative.

> Signed-off-by: Alban 
> ---
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt 
> b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> index b7396c8..61f5f6d 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -27,16 +27,34 @@ Required properties:
>   - 0034 for AR9462
>   - 0036 for AR9565
>   - 0037 for AR9485
> + For SoC devices the compatible should be "qca,-wmac"
> + and one of the following fallbacks:
> + - "qca,ar9100-wmac"
> + - "qca,ar9330-wmac"
> + - "qca,ar9340-wmac"
> + - "qca,qca9550-wmac"
> + - "qca,qca9530-wmac"
>  - reg: Address and length of the register set for the device.
>  
> +Required properties for SoC devices:
> +- interrupt-parent: phandle of the parent interrupt controller.
> +- interrupts: Interrupt specifier for the controllers interrupt.
> +
>  Optional properties:
> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +- clock-names: has to be "ref"
> +- clocks: phandle of the reference clock
> +- resets: phandle of the reset line
> +- nvmem-cell-names: has to be "eeprom" and/or "address"
> +- nvmem-cells: phandle to the eeprom nvmem cell and/or to the mac address
> + nvmem cell.
> +
> +Deprecated properties:
>  - qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>   ath9k wireless chip (in this case the calibration /
>   EEPROM data will be loaded from userspace using the
>   kernel firmware loader).
> -- mac-address: See ethernet.txt in the parent directory
> -- local-mac-address: See ethernet.txt in the parent directory
> -
It sounds like you want to deprecate mac-address and local-mac-address as well.
If so you sould add this to the commit as well. From my point of view, people
mostly flat-out patched the eeprom-image if they wanted to set the mac-address.
However, this was an extra step, if nvmem does away with it, I'm completely
fine with deprecating these properties.

Thanks,
Christian

Re: [PATCH 3/7] ath9k: Add support for reading the EEPROM data using the nvmem API

[Christian Lamparter]

On Thursday, March 23, 2017 3:43:28 PM CET Alban wrote:
> On Tue, 14 Mar 2017 00:53:55 +0100
> Christian Lamparter <chunk...@googlemail.com> wrote:
> 
> > On Monday, March 13, 2017 10:05:11 PM CET Alban wrote:
> > > Currently SoC platforms use a firmware request to get the EEPROM data.
> > > This is mostly a hack and rely on using a user-helper scripts which is
> > > deprecated. A nicer alternative is to use the nvmem API which was
> > > designed for this kind of task.
> > > 
> > > Furthermore we let CONFIG_ATH9K_AHB select CONFIG_NVMEM as such
> > > devices will generally use this method for loading the EEPROM data.
> > > 
> > > Signed-off-by: Alban <al...@free.fr>
> > > ---
> > > @@ -654,6 +656,25 @@ static int ath9k_init_softc(u16 devid, struct 
> > > ath_softc *sc,
> > >   if (ret)
> > >   return ret;
> > >  
> > > + /* If the EEPROM hasn't been retrieved via firmware request
> > > +  * use the nvmem API insted.
> > > +  */
> > > + if (!ah->eeprom_blob) {
> > > + struct nvmem_cell *eeprom_cell;
> > > +
> > > + eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
> > > + if (!IS_ERR(eeprom_cell)) {
> > > + ah->eeprom_data = nvmem_cell_read(
> > > + eeprom_cell, >eeprom_size);
> > > + nvmem_cell_put(eeprom_cell);
> > > +
> > > + if (IS_ERR(ah->eeprom_data)) {
> > > + dev_err(ah->dev, "failed to read eeprom");
> > > + return PTR_ERR(ah->eeprom_data);
> > > + }
> > > + }
> > > + }
> > > +
> > >   if (ath9k_led_active_high != -1)
> > >   ah->config.led_active_high = ath9k_led_active_high == 1;
> > >
> > Are you sure this works with AR93XX SoCs that have the calibration data
> > in the OTP?
> 
> I only tested this on an ar9132 platform, so it might well be that a
> few things are missing for the newer SoC. However this shouldn't break
> anything existing as a platform needs to define an nvmem cell on the
> athk9 device for this code to be used a all.

Yes, I looked at it.
+   eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
+   if (!IS_ERR(eeprom_cell)) {
+   ...
+   }
So if there's a error with the "eeprom" nvmem property,
(i.e.: it's not present) the driver should works as before.

Thanks,
Christian

Re: [PATCH 3/7] ath9k: Add support for reading the EEPROM data using the nvmem API

[Christian Lamparter]

On Monday, March 13, 2017 10:05:11 PM CET Alban wrote:
> Currently SoC platforms use a firmware request to get the EEPROM data.
> This is mostly a hack and rely on using a user-helper scripts which is
> deprecated. A nicer alternative is to use the nvmem API which was
> designed for this kind of task.
> 
> Furthermore we let CONFIG_ATH9K_AHB select CONFIG_NVMEM as such
> devices will generally use this method for loading the EEPROM data.
> 
> Signed-off-by: Alban 
> ---
> @@ -654,6 +656,25 @@ static int ath9k_init_softc(u16 devid, struct ath_softc 
> *sc,
>   if (ret)
>   return ret;
>  
> + /* If the EEPROM hasn't been retrieved via firmware request
> +  * use the nvmem API insted.
> +  */
> + if (!ah->eeprom_blob) {
> + struct nvmem_cell *eeprom_cell;
> +
> + eeprom_cell = nvmem_cell_get(ah->dev, "eeprom");
> + if (!IS_ERR(eeprom_cell)) {
> + ah->eeprom_data = nvmem_cell_read(
> + eeprom_cell, >eeprom_size);
> + nvmem_cell_put(eeprom_cell);
> +
> + if (IS_ERR(ah->eeprom_data)) {
> + dev_err(ah->dev, "failed to read eeprom");
> + return PTR_ERR(ah->eeprom_data);
> + }
> + }
> + }
> +
>   if (ath9k_led_active_high != -1)
>   ah->config.led_active_high = ath9k_led_active_high == 1;
>  
Are you sure this works with AR93XX SoCs that have the calibration data
in the OTP?

I've added Chris to the CC, since he has a HiveAP 121 that has this
configuration, so he can test, whenever this is a problem or not.

But from what I can tell, devices with the calibration data in the
OTP would fail now. This is because the OTP is done by ath9k_hw_init()
which hasn't run yet (it's a bit further down the road in the same
function though). 

Note: the OTP doesn't store the whole caldata. Just a few parts.
A temporary "eeprom" gets constructed with the help of the 
ar9300_eep_templates in ar9003_eeprom.c's
ar9300_eeprom_restore_internal(). But from what I don't think, 
that the eeprom_blob is constructed/set by the functions in
ar9003_eeprom.

I think this will require an additional property like
qca,calibration-in-otp. What's your opinion?

Thanks,
Christian

[PATCH] net: ibm: emac: fix regression caused by emac_dt_phy_probe()

[Christian Lamparter]

Julian Margetson reported a panic on his SAM460EX with Kernel 4.11-rc1:
| Unable to handle kernel paging request for data at address 0x0014
| Oops: Kernel access of bad area, sig: 11 [#1]
| PREEMPT
| Canyonlands
| Modules linked in:
| CPU: 0 PID: 1 Comm: swapper Not tainted [...]
| task: ea838000 task.stack: ea836000
| NIP: c0599f5c LR: c0599dd8 CTR: 
| REGS: ea837c80 TRAP: 0300   Not tainted [...]
| MSR: 00029000 <CE,EE,ME>
|  CR: 24371242  XER: 2000
| DEAR: 0014 ESR: 
| GPR00: c0599ce8 ea837d30 ea838000 c0e52dcc c0d56ffb [...]
| NIP [c0599f5c] emac_probe+0xfb4/0x1304
| LR [c0599dd8] emac_probe+0xe30/0x1304
| Call Trace:
| [ea837d30] [c0599ce8] emac_probe+0xd40/0x1304 (unreliable)
| [ea837d80] [c0533504] platform_drv_probe+0x48/0x90
| [ea837da0] [c0531c14] driver_probe_device+0x15c/0x2c4
| [ea837dd0] [c0531e04] __driver_attach+0x88/0xb0
| ---[ end trace ... ]---

The problem is caused by emac_dt_phy_probe() returing success (0)
for existing device-trees configurations that do not specify a
"phy-handle" property. This caused the code to skip the existing
phy probe and setup. Which led to essential phy related
data-structures being uninitialized.

This patch also removes the unused variable in emac_dt_phy_connect().

Fixes: a577ca6badb5261d ("net: emac: add support for device-tree based PHY 
discovery and setup")
Reported-by: Julian Margetson <runa...@candw.ms>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 275c2e2349ad..c44036d5761a 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2589,8 +2589,6 @@ static int emac_dt_mdio_probe(struct emac_instance *dev)
 static int emac_dt_phy_connect(struct emac_instance *dev,
   struct device_node *phy_handle)
 {
-   int res;
-
dev->phy.def = devm_kzalloc(>ofdev->dev, sizeof(*dev->phy.def),
GFP_KERNEL);
if (!dev->phy.def)
@@ -2617,7 +2615,7 @@ static int emac_dt_phy_probe(struct emac_instance *dev)
 {
struct device_node *np = dev->ofdev->dev.of_node;
struct device_node *phy_handle;
-   int res = 0;
+   int res = 1;
 
phy_handle = of_parse_phandle(np, "phy-handle", 0);
 
@@ -2714,13 +2712,24 @@ static int emac_init_phy(struct emac_instance *dev)
if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII)) {
int res = emac_dt_phy_probe(dev);
 
-   mutex_unlock(_phy_map_lock);
-   if (!res)
+   switch (res) {
+   case 1:
+   /* No phy-handle property configured.
+* Continue with the existing phy probe
+* and setup code.
+*/
+   break;
+
+   case 0:
+   mutex_unlock(_phy_map_lock);
goto init_phy;
 
-   dev_err(>ofdev->dev, "failed to attach dt phy (%d).\n",
-   res);
-   return res;
+   default:
+   mutex_unlock(_phy_map_lock);
+   dev_err(>ofdev->dev, "failed to attach dt phy 
(%d).\n",
+   res);
+   return res;
+   }
}
 
if (dev->phy_address != 0x)
-- 
2.11.0

[PATCH v1.2] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

This patch adds documentation for a new "phy-handle" property,
"fixed-link" and "mdio" sub-node. These allows the enumeration
of PHYs which are supported by the phy library under drivers/net/phy.

The EMAC ethernet controller in IBM and AMCC 4xx chips is
currently stuck with a few privately defined phy
implementations. It has no support for PHYs which
are supported by the generic phylib.

Acked-by: Rob Herring <r...@kernel.org>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
Fixed: phy node - so it conforms to phy.txt.
---
---
 .../devicetree/bindings/powerpc/4xx/emac.txt   | 61 +-
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
index 712baf6c3e24..2fa861378294 100644
--- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
+++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
@@ -71,6 +71,9 @@
  For Axon it can be absent, though my current driver
  doesn't handle phy-address yet so for now, keep
  0x00ff in it.
+- phy-handle   : Used to describe configurations where a external PHY
+ is used. Please refer to:
+ Documentation/devicetree/bindings/net/ethernet.txt
 - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
  operations (if absent the value is the same as
  rx-fifo-size).  For Axon, either absent or 2048.
@@ -81,8 +84,22 @@
  offload, phandle of the TAH device node.
 - tah-channel   : 1 cell, optional. If appropriate, channel used on the
  TAH engine.
+- fixed-link   : Fixed-link subnode describing a link to a non-MDIO
+ managed entity. See
+ Documentation/devicetree/bindings/net/fixed-link.txt
+ for details.
+- mdio subnode : When the EMAC has a phy connected to its local
+ mdio, which us supported by the kernel's network
+ PHY library in drivers/net/phy, there must be device
+ tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
 
-Example:
+ For PHY definitions: Please refer to
+ Documentation/devicetree/bindings/net/phy.txt and
+ Documentation/devicetree/bindings/net/ethernet.txt
+
+Examples:
 
EMAC0: ethernet@4800 {
device_type = "network";
@@ -104,6 +121,47 @@
zmii-channel = <0>;
};
 
+   EMAC1: ethernet@ef600c00 {
+   device_type = "network";
+   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
+   interrupt-parent = <>;
+   interrupts = <0 1>;
+   #interrupt-cells = <1>;
+   #address-cells = <0>;
+   #size-cells = <0>;
+   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
+1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
+   reg = <0xef600c00 0x00c4>;
+   local-mac-address = []; /* Filled in by U-Boot */
+   mal-device = <>;
+   mal-tx-channel = <0>;
+   mal-rx-channel = <0>;
+   cell-index = <0>;
+   max-frame-size = <9000>;
+   rx-fifo-size = <16384>;
+   tx-fifo-size = <2048>;
+   fifo-entry-size = <10>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   phy-map = <0x>;
+   rgmii-device = <>;
+   rgmii-channel = <0>;
+   tah-device = <>;
+   tah-channel = <0>;
+   has-inverted-stacr-oc;
+   has-new-stacr-staopc;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <0>;
+   };
+   };
+
+
   ii) McMAL node
 
 Required properties:
@@ -145,4 +203,3 @@
 - revision   : as provided by the RGMII new version register if
   available.
   For Axon: 0x012a
-
-- 
2.11.0

[PATCH v1.1] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

This patch adds documentation for a new "phy-handle" property,
"fixed-link" and "mdio" sub-node. These allows the enumeration
of PHYs which are supported by the phy library under drivers/net/phy.

The EMAC ethernet controller in IBM and AMCC 4xx chips is
currently stuck with a few privately defined phy
implementations. It has no support for PHYs which
are supported by the generic phylib.

Acked-by: Rob Herring <r...@kernel.org>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
Resent - no changes.
---
 .../devicetree/bindings/powerpc/4xx/emac.txt   | 64 +-
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
index 712baf6c3e24..1893b4c4d93b 100644
--- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
+++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
@@ -71,6 +71,9 @@
  For Axon it can be absent, though my current driver
  doesn't handle phy-address yet so for now, keep
  0x00ff in it.
+- phy-handle   : Used to describe configurations where a external PHY
+ is used. Please refer to:
+ Documentation/devicetree/bindings/net/ethernet.txt
 - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
  operations (if absent the value is the same as
  rx-fifo-size).  For Axon, either absent or 2048.
@@ -81,8 +84,22 @@
  offload, phandle of the TAH device node.
 - tah-channel   : 1 cell, optional. If appropriate, channel used on the
  TAH engine.
+- fixed-link   : Fixed-link subnode describing a link to a non-MDIO
+ managed entity. See
+ Documentation/devicetree/bindings/net/fixed-link.txt
+ for details.
+- mdio subnode : When the EMAC has a phy connected to its local
+ mdio, which us supported by the kernel's network
+ PHY library in drivers/net/phy, there must be device
+ tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
 
-Example:
+ For PHY definitions: Please refer to
+ Documentation/devicetree/bindings/net/phy.txt and
+ Documentation/devicetree/bindings/net/ethernet.txt
+
+Examples:
 
EMAC0: ethernet@4800 {
device_type = "network";
@@ -104,6 +121,50 @@
zmii-channel = <0>;
};
 
+   EMAC1: ethernet@ef600c00 {
+   device_type = "network";
+   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
+   interrupt-parent = <>;
+   interrupts = <0 1>;
+   #interrupt-cells = <1>;
+   #address-cells = <0>;
+   #size-cells = <0>;
+   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
+1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
+   reg = <0xef600c00 0x00c4>;
+   local-mac-address = []; /* Filled in by U-Boot */
+   mal-device = <>;
+   mal-tx-channel = <0>;
+   mal-rx-channel = <0>;
+   cell-index = <0>;
+   max-frame-size = <9000>;
+   rx-fifo-size = <16384>;
+   tx-fifo-size = <2048>;
+   fifo-entry-size = <10>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   phy-map = <0x>;
+   rgmii-device = <>;
+   rgmii-channel = <0>;
+   tah-device = <>;
+   tah-channel = <0>;
+   has-inverted-stacr-oc;
+   has-new-stacr-staopc;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   device_type = "ethernet-phy";
+   reg = <0>;
+
+   qca,ar8327-initvals = <
+   0x0010 0x4000>;
+   };
+   };
+
+
   ii) McMAL node
 
 Required properties:
@@ -145,4 +206,3 @@
 - revision   : as provided by the RGMII new version register if
   available.
   For Axon: 0x012a
-
-- 
2.11.0

[PATCH v2] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

This patch adds documentation for a new "phy-handle" property,
"fixed-link" and "mdio" sub-node. These allows the enumeration
of PHYs which are supported by the phy library under drivers/net/phy.

The EMAC ethernet controller in IBM and AMCC 4xx chips is
currently stuck with a few privately defined phy
implementations. It has no support for PHYs which
are supported by the generic phylib.

Acked-by: Rob Herring <r...@kernel.org>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
---
 .../devicetree/bindings/powerpc/4xx/emac.txt   | 62 +-
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
index 712baf6c3e24..44b842b6ca15 100644
--- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
+++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
@@ -71,6 +71,9 @@
  For Axon it can be absent, though my current driver
  doesn't handle phy-address yet so for now, keep
  0x00ff in it.
+- phy-handle   : Used to describe configurations where a external PHY
+ is used. Please refer to:
+ Documentation/devicetree/bindings/net/ethernet.txt
 - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
  operations (if absent the value is the same as
  rx-fifo-size).  For Axon, either absent or 2048.
@@ -81,8 +84,22 @@
  offload, phandle of the TAH device node.
 - tah-channel   : 1 cell, optional. If appropriate, channel used on the
  TAH engine.
+- fixed-link   : Fixed-link subnode describing a link to a non-MDIO
+ managed entity. See
+ Documentation/devicetree/bindings/net/fixed-link.txt
+ for details.
+- mdio subnode : When the EMAC has a phy connected to its local
+ mdio, which us supported by the kernel's network
+ PHY library in drivers/net/phy, there must be device
+ tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
 
-Example:
+ For PHY definitions: Please refer to
+ Documentation/devicetree/bindings/net/phy.txt and
+ Documentation/devicetree/bindings/net/ethernet.txt
+
+Examples:
 
EMAC0: ethernet@4800 {
device_type = "network";
@@ -104,6 +121,48 @@
zmii-channel = <0>;
};
 
+   EMAC1: ethernet@ef600c00 {
+   device_type = "network";
+   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
+   interrupt-parent = <>;
+   interrupts = <0 1>;
+   #interrupt-cells = <1>;
+   #address-cells = <0>;
+   #size-cells = <0>;
+   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
+1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
+   reg = <0xef600c00 0x00c4>;
+   local-mac-address = []; /* Filled in by U-Boot */
+   mal-device = <>;
+   mal-tx-channel = <0>;
+   mal-rx-channel = <0>;
+   cell-index = <0>;
+   max-frame-size = <9000>;
+   rx-fifo-size = <16384>;
+   tx-fifo-size = <2048>;
+   fifo-entry-size = <10>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   phy-map = <0x>;
+   rgmii-device = <>;
+   rgmii-channel = <0>;
+   tah-device = <>;
+   tah-channel = <0>;
+   has-inverted-stacr-oc;
+   has-new-stacr-staopc;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <0>;
+   };
+   };
+   };
+
+
   ii) McMAL node
 
 Required properties:
@@ -145,4 +204,3 @@
 - revision   : as provided by the RGMII new version register if
   available.
   For Axon: 0x012a
-
-- 
2.11.0

Re: [PATCH v1.1] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

On Wednesday, February 22, 2017 3:37:35 PM CET David Miller wrote:
> From: Christian Lamparter <chunk...@googlemail.com>
> Date: Mon, 20 Feb 2017 20:10:58 +0100
> 
> > This patch adds glue-code that allows the EMAC driver to interface
> > with the existing dt-supported PHYs in drivers/net/phy.
> > 
> > Because currently, the emac driver maintains a small library of
> > supported phys for in a private phy.c file located in the drivers
> > directory.
> > 
> > The support is limited to mostly single ethernet transceiver like the:
> > CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.
> > 
> > However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
> > have a 5-port switch (AR8327N) attached to the EMAC. The switch chip
> > is supported by the qca8k mdio driver, which uses the generic phy
> > library. Another reason is that PHYLIB also supports the BCM54610,
> > which was used for the Western Digital My Book Live.
> > 
> > This will now also make EMAC select PHYLIB.
> > 
> > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> 
> Applied, thanks.
> 
Thanks David.

I noticed that the DT Documentation patch:
"[v1,1/2] dt: emac: document device-tree based phy discovery and setup"
is still pending with "Changes Requested":
<https://patchwork.ozlabs.org/patch/729658/>

I think this is because of Florian's comment on patch:
"[v1,2/2] net: emac: add support for device-tree based PHY discovery and setup"
If so, can you please queue this documentation update patch for -next?
(I haven't received any comments or complains. If necessary, I can also
resent it.)

Thanks,
Christian

[PATCH v1.1] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

This patch adds glue-code that allows the EMAC driver to interface
with the existing dt-supported PHYs in drivers/net/phy.

Because currently, the emac driver maintains a small library of
supported phys for in a private phy.c file located in the drivers
directory.

The support is limited to mostly single ethernet transceiver like the:
CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.

However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
have a 5-port switch (AR8327N) attached to the EMAC. The switch chip
is supported by the qca8k mdio driver, which uses the generic phy
library. Another reason is that PHYLIB also supports the BCM54610,
which was used for the Western Digital My Book Live.

This will now also make EMAC select PHYLIB.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
RFC->v1:
- added PHYLIB (fixes kbuild-bot error)
- used the correct version (the working one from LEDE).
- added fixed-link DT support. (This fixes a XXX)
- Added WA for the MX60(W): -EREMOTEIO/-ETIMEDOUT/...
  errors from mdio_read are converted to 0x.
v1->v1.1:
- removed phy_flags
---
 drivers/net/ethernet/ibm/emac/Kconfig |   1 +
 drivers/net/ethernet/ibm/emac/core.c  | 254 +-
 drivers/net/ethernet/ibm/emac/core.h  |   4 +
 3 files changed, 252 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/Kconfig 
b/drivers/net/ethernet/ibm/emac/Kconfig
index 3f44a30e0615..90d49191beb3 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -2,6 +2,7 @@ config IBM_EMAC
tristate "IBM EMAC Ethernet support"
depends on PPC_DCR
select CRC32
+   select PHYLIB
help
  This driver supports the IBM EMAC family of Ethernet controllers
  typically found on 4xx embedded PowerPC chips, but also on the
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 5909615c27f7..c3806679dfa3 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -2410,6 +2411,219 @@ static int emac_read_uint_prop(struct device_node *np, 
const char *name,
return 0;
 }
 
+static void emac_adjust_link(struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct phy_device *phy = dev->phy_dev;
+
+   dev->phy.autoneg = phy->autoneg;
+   dev->phy.speed = phy->speed;
+   dev->phy.duplex = phy->duplex;
+   dev->phy.pause = phy->pause;
+   dev->phy.asym_pause = phy->asym_pause;
+   dev->phy.advertising = phy->advertising;
+}
+
+static int emac_mii_bus_read(struct mii_bus *bus, int addr, int regnum)
+{
+   int ret = emac_mdio_read(bus->priv, addr, regnum);
+   /* This is a workaround for powered down ports/phys.
+* In the wild, this was seen on the Cisco Meraki MX60(W).
+* This hardware disables ports as part of the handoff
+* procedure. Accessing the ports will lead to errors
+* (-ETIMEDOUT, -EREMOTEIO) that do more harm than good.
+*/
+   return ret < 0 ? 0x : ret;
+}
+
+static int emac_mii_bus_write(struct mii_bus *bus, int addr,
+ int regnum, u16 val)
+{
+   emac_mdio_write(bus->priv, addr, regnum, val);
+   return 0;
+}
+
+static int emac_mii_bus_reset(struct mii_bus *bus)
+{
+   struct emac_instance *dev = netdev_priv(bus->priv);
+
+   return emac_reset(dev);
+}
+
+static int emac_mdio_setup_aneg(struct mii_phy *phy, u32 advertise)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   dev->phy.autoneg = AUTONEG_ENABLE;
+   dev->phy.speed = SPEED_1000;
+   dev->phy.duplex = DUPLEX_FULL;
+   dev->phy.advertising = advertise;
+   phy->autoneg = AUTONEG_ENABLE;
+   phy->speed = dev->phy.speed;
+   phy->duplex = dev->phy.duplex;
+   phy->advertising = advertise;
+   return phy_start_aneg(dev->phy_dev);
+}
+
+static int emac_mdio_setup_forced(struct mii_phy *phy, int speed, int fd)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   dev->phy.autoneg =  AUTONEG_DISABLE;
+   dev->phy.speed = speed;
+   dev->phy.duplex = fd;
+   phy->autoneg = AUTONEG_DISABLE;
+   phy->speed = speed;
+   phy->duplex = fd;
+   return phy_start_aneg(dev->phy_dev);
+}
+
+static int emac_mdio_poll_link(struct mii_phy *phy)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+   int res;
+
+   res = phy_read_status(dev->phy_dev);
+   if (res) {
+   dev_e

[PATCH v1 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

This patch adds glue-code that allows the EMAC driver to interface
with the existing dt-supported PHYs in drivers/net/phy.

Because currently, the emac driver maintains a small library of
supported phys for in a private phy.c file located in the drivers
directory.

The support is limited to mostly single ethernet transceiver like the:
CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.

However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
have a 5-port switch (AR8327N) attached to the EMAC. The switch chip
is supported by the qca8k mdio driver, which uses the generic phy
library. Another reason is that PHYLIB also supports the BCM54610,
which was used for the Western Digital My Book Live.

This will now also make EMAC select PHYLIB.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
RFC->v1:
- added PHYLIB (fixes kbuild-bot error)
- used the correct version (the working one from LEDE).
- added fixed-link DT support. (This fixes a XXX)
- Added WA for the MX60(W): -EREMOTEIO/-ETIMEDOUT/...
  errors from mdio_read are converted to 0x.
---
 drivers/net/ethernet/ibm/emac/Kconfig |   1 +
 drivers/net/ethernet/ibm/emac/core.c  | 260 +-
 drivers/net/ethernet/ibm/emac/core.h  |   4 +
 3 files changed, 258 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/Kconfig 
b/drivers/net/ethernet/ibm/emac/Kconfig
index 3f44a30e0615..90d49191beb3 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -2,6 +2,7 @@ config IBM_EMAC
tristate "IBM EMAC Ethernet support"
depends on PPC_DCR
select CRC32
+   select PHYLIB
help
  This driver supports the IBM EMAC family of Ethernet controllers
  typically found on 4xx embedded PowerPC chips, but also on the
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 5909615c27f7..11d97971fb28 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -2410,6 +2411,225 @@ static int emac_read_uint_prop(struct device_node *np, 
const char *name,
return 0;
 }
 
+static void emac_adjust_link(struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct phy_device *phy = dev->phy_dev;
+
+   dev->phy.autoneg = phy->autoneg;
+   dev->phy.speed = phy->speed;
+   dev->phy.duplex = phy->duplex;
+   dev->phy.pause = phy->pause;
+   dev->phy.asym_pause = phy->asym_pause;
+   dev->phy.advertising = phy->advertising;
+}
+
+static int emac_mii_bus_read(struct mii_bus *bus, int addr, int regnum)
+{
+   int ret = emac_mdio_read(bus->priv, addr, regnum);
+   /* This is a workaround for powered down ports/phys.
+* In the wild, this was seen on the Cisco Meraki MX60(W).
+* This hardware disables ports as part of the handoff
+* procedure. Accessing the ports will lead to errors
+* (-ETIMEDOUT, -EREMOTEIO) that do more harm than good.
+*/
+   return ret < 0 ? 0x : ret;
+}
+
+static int emac_mii_bus_write(struct mii_bus *bus, int addr,
+ int regnum, u16 val)
+{
+   emac_mdio_write(bus->priv, addr, regnum, val);
+   return 0;
+}
+
+static int emac_mii_bus_reset(struct mii_bus *bus)
+{
+   struct emac_instance *dev = netdev_priv(bus->priv);
+
+   return emac_reset(dev);
+}
+
+static int emac_mdio_setup_aneg(struct mii_phy *phy, u32 advertise)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   dev->phy.autoneg = AUTONEG_ENABLE;
+   dev->phy.speed = SPEED_1000;
+   dev->phy.duplex = DUPLEX_FULL;
+   dev->phy.advertising = advertise;
+   phy->autoneg = AUTONEG_ENABLE;
+   phy->speed = dev->phy.speed;
+   phy->duplex = dev->phy.duplex;
+   phy->advertising = advertise;
+   return phy_start_aneg(dev->phy_dev);
+}
+
+static int emac_mdio_setup_forced(struct mii_phy *phy, int speed, int fd)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   dev->phy.autoneg =  AUTONEG_DISABLE;
+   dev->phy.speed = speed;
+   dev->phy.duplex = fd;
+   phy->autoneg = AUTONEG_DISABLE;
+   phy->speed = speed;
+   phy->duplex = fd;
+   return phy_start_aneg(dev->phy_dev);
+}
+
+static int emac_mdio_poll_link(struct mii_phy *phy)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+   int res;
+
+   res = phy_read_status(dev->phy_dev);
+   if (res) {
+   dev_err(>ofdev->dev, "

[PATCH v1 1/2] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

This patch adds documentation for a new "phy-handle" property,
"fixed-link" and "mdio" sub-node. These allows the enumeration
of PHYs which are supported by the phy library under drivers/net/phy.

The EMAC ethernet controller in IBM and AMCC 4xx chips is
currently stuck with a few privately defined phy
implementations. It has no support for PHYs which
are supported by the generic phylib.

Acked-by: Rob Herring <r...@kernel.org>
Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
I rewrote the "phy-handle" property in order to make the
links to net/ethernet.txt consistent with the ones in the mdio
subnode. I've also added the "fixed-link" subnode, which I
copied from net/dsa.txt. I hope this doesn't invalidate Rob's
ACK from the RFC (If it does, please tell me, otherwise I
assume everything is still OK).
---
 .../devicetree/bindings/powerpc/4xx/emac.txt   | 64 +-
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
index 712baf6c3e24..1893b4c4d93b 100644
--- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
+++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
@@ -71,6 +71,9 @@
  For Axon it can be absent, though my current driver
  doesn't handle phy-address yet so for now, keep
  0x00ff in it.
+- phy-handle   : Used to describe configurations where a external PHY
+ is used. Please refer to:
+ Documentation/devicetree/bindings/net/ethernet.txt
 - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
  operations (if absent the value is the same as
  rx-fifo-size).  For Axon, either absent or 2048.
@@ -81,8 +84,22 @@
  offload, phandle of the TAH device node.
 - tah-channel   : 1 cell, optional. If appropriate, channel used on the
  TAH engine.
+- fixed-link   : Fixed-link subnode describing a link to a non-MDIO
+ managed entity. See
+ Documentation/devicetree/bindings/net/fixed-link.txt
+ for details.
+- mdio subnode : When the EMAC has a phy connected to its local
+ mdio, which us supported by the kernel's network
+ PHY library in drivers/net/phy, there must be device
+ tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
 
-Example:
+ For PHY definitions: Please refer to
+ Documentation/devicetree/bindings/net/phy.txt and
+ Documentation/devicetree/bindings/net/ethernet.txt
+
+Examples:
 
EMAC0: ethernet@4800 {
device_type = "network";
@@ -104,6 +121,50 @@
zmii-channel = <0>;
};
 
+   EMAC1: ethernet@ef600c00 {
+   device_type = "network";
+   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
+   interrupt-parent = <>;
+   interrupts = <0 1>;
+   #interrupt-cells = <1>;
+   #address-cells = <0>;
+   #size-cells = <0>;
+   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
+1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
+   reg = <0xef600c00 0x00c4>;
+   local-mac-address = []; /* Filled in by U-Boot */
+   mal-device = <>;
+   mal-tx-channel = <0>;
+   mal-rx-channel = <0>;
+   cell-index = <0>;
+   max-frame-size = <9000>;
+   rx-fifo-size = <16384>;
+   tx-fifo-size = <2048>;
+   fifo-entry-size = <10>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   phy-map = <0x>;
+   rgmii-device = <>;
+   rgmii-channel = <0>;
+   tah-device = <>;
+   tah-channel = <0>;
+   has-inverted-stacr-oc;
+   has-new-stacr-staopc;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   device_type = "ethernet-phy";
+   reg = <0>;
+
+

Re: [PATCH][CFT] Saner error handling in skb_copy_datagram_iter() et.al.

[Christian Lamparter]

On Saturday, February 18, 2017 12:02:14 AM CET Al Viro wrote:
> On Fri, Feb 17, 2017 at 05:03:15PM +, Al Viro wrote:
> > On Fri, Feb 17, 2017 at 10:54:20AM -0500, David Miller wrote:
> > > From: Al Viro <v...@zeniv.linux.org.uk>
> > > Date: Tue, 14 Feb 2017 01:33:06 +
> > > 
> > > > OK...  Remaining interesting question is whether it adds a noticable
> > > > overhead.  Could somebody try it on assorted benchmarks and see if
> > > > it slows the things down?  The patch in question follows:
> > > 
> > > That's about a 40 byte copy onto the stack for each invocation of this
> > > thing.  You can benchmark all you want, but it's clear that this is
> > > non-trivial amount of work and will take some operations from fitting
> > > in the cache to not doing so for sure.
> > 
> > In principle, that could be reduced a bit (32 bytes - ->type is never
> > changed, so we don't need to restore it), but that's not much of 
> > improvement...
> 
> Actually, I've a better solution.  Namely, analogue of iov_iter_advance()
> for going backwards.  The restriction is that you should never unroll
> further than where you've initially started *or* have the iovec, etc.
> array modified under you.  For iovec/kvec/bio_vec it's trivial, for pipe -
> a bit more convoluted, but still doable.  Then net/core/datagram.c stuff
> could simply use iov_iter_unroll() in case of error - all we need to keep
> track of is how much had we copied and that's easy to do.
> 
> The patch below is completely untested, but if it works it should avoid
> buggering the fast paths at all, still giving the same semantics re
> reverting ->msg_iter both on EINVAL and EFAULT.  Comments?
I've tested it. It also fixes the corruption issue I can reproduce
with my setup.:
# tc qdisc add dev eth0 root netem corrupt 0.1%
(and the dlbug code)

So, for what's worth:
Tested-by: Christian Lamparter <chunk...@googlemail.com>

> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 804e34c6f981..237965348bc2 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -39,7 +39,10 @@ struct iov_iter {
>   };
>   union {
>   unsigned long nr_segs;
> - int idx;
> + struct {
> + int idx;
> + int start_idx;
> + };
>   };
>  };
>  
> @@ -81,6 +84,7 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long 
> nr_segs, size_t to);
>  size_t iov_iter_copy_from_user_atomic(struct page *page,
>   struct iov_iter *i, unsigned long offset, size_t bytes);
>  void iov_iter_advance(struct iov_iter *i, size_t bytes);
> +void iov_iter_unroll(struct iov_iter *i, size_t bytes);
>  int iov_iter_fault_in_readable(struct iov_iter *i, size_t bytes);
>  size_t iov_iter_single_seg_count(const struct iov_iter *i);
>  size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index e68604ae3ced..afdaca37f5c9 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -786,6 +786,68 @@ void iov_iter_advance(struct iov_iter *i, size_t size)
>  }
>  EXPORT_SYMBOL(iov_iter_advance);
>  
> +void iov_iter_unroll(struct iov_iter *i, size_t unroll)
> +{
> + if (!unroll)
> + return;
> + i->count += unroll;
> + if (unlikely(i->type & ITER_PIPE)) {
> + struct pipe_inode_info *pipe = i->pipe;
> + int idx = i->idx;
> + size_t off = i->iov_offset;
> + while (1) {
> + size_t n = off - pipe->bufs[idx].offset;
> + if (unroll < n) {
> + off -= (n - unroll);
> + break;
> + }
> + unroll -= n;
> + if (!unroll && idx == i->start_idx) {
> + off = 0;
> + break;
> + }
> + if (!idx--)
> + idx = pipe->buffers - 1;
> + off = pipe->bufs[idx].offset + pipe->bufs[idx].len;
> + }
> + i->iov_offset = off;
> + i->idx = idx;
> + pipe_truncate(i);
> + return;
> + }
> + if (unroll <= i->iov_offset) {
> + i->iov_offset -= unroll;
> + return;
> + }
> + unroll -= i->iov_offset;
> + if (i->type & ITER_BVEC) {
> + const struct bio_vec *bvec = i->bvec;
> + while (1) {
> +   

Re: [RFC 1/2] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

On Sunday, February 5, 2017 2:33:44 PM CET Florian Fainelli wrote:
> Le 02/05/17 à 14:25, Christian Lamparter a écrit :
> > This patch adds documentation for a new "phy-handler" property
> > and "mdio" sub-node. These allows the enumeration of PHYs which
> > are supported by the phy library under drivers/net/phy.
> > 
> > The EMAC ethernet controller in IBM and AMCC 4xx chips is
> > currently stuck with a few privately defined phy
> > implementations. It has no support for PHYs which
> > are supported by the generic phylib.
> > 
> > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> > ---
> >  .../devicetree/bindings/powerpc/4xx/emac.txt   | 60 
> > +-
> >  1 file changed, 58 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
> > b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> > index 712baf6c3e24..0572d053c35a 100644
> > --- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> > +++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> > @@ -71,6 +71,8 @@
> >   For Axon it can be absent, though my current driver
> >   doesn't handle phy-address yet so for now, keep
> >   0x00ff in it.
> > +- phy-handle   : See net/ethernet.txt file; used to describe
> > + configurations where a external PHY is used.
> >  - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
> >   operations (if absent the value is the same as
> >   rx-fifo-size).  For Axon, either absent or 2048.
> > @@ -82,7 +84,18 @@
> >  - tah-channel   : 1 cell, optional. If appropriate, channel used 
> > on the
> >   TAH engine.
> >  
> > -Example:
> > +- mdio subnode : When the EMAC has a phy connected to its local
> > + mdio, which us supported by the kernel's network
> > + PHY library in drivers/net/phy, there must be device
> > + tree subnode with the following required properties:
> > +   - #address-cells: Must be <1>.
> > +   - #size-cells: Must be <0>.
> > +
> > + For each phy on the mdio bus, there must be a node
> > + with the following fields:
> > +   - reg: phy id used to communicate to phy.
> > +   - device_type: Must be "ethernet-phy".
> 
> Just provide a reference to
> Documentation/devicetree/bindings/net/phy.txt and
> Documentation/devicetree/bindings/net/ethernet.txt here. device_type is
> not required.

Yes, I added a reference there.
> 
> > +Examples:
> >  
> > EMAC0: ethernet@4800 {
> > device_type = "network";
> > @@ -104,6 +117,50 @@
> > zmii-channel = <0>;
> > };
> >  
> > +   EMAC1: ethernet@ef600c00 {
> > +   device_type = "network";
> > +   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
> > +   interrupt-parent = <>;
> > +   interrupts = <0 1>;
> > +   #interrupt-cells = <1>;
> > +   #address-cells = <0>;
> > +   #size-cells = <0>;
> > +   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
> > +1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
> > +   reg = <0xef600c00 0x00c4>;
> > +   local-mac-address = []; /* Filled in by U-Boot */
> > +   mal-device = <>;
> > +   mal-tx-channel = <0>;
> > +   mal-rx-channel = <0>;
> > +   cell-index = <0>;
> > +   max-frame-size = <9000>;
> > +   rx-fifo-size = <16384>;
> > +   tx-fifo-size = <2048>;
> > +   fifo-entry-size = <10>;
> > +   phy-mode = "rgmii";
> > +   phy-map = <0x>;
> 
> If you have a proper mdio subnode, this property becomes irrelevant and
> should be unused.
This is emac.c doing. It defaults to 0x... if the property is absent.
<http://lxr.free-electrons.com/source/drivers/net/ethernet/ibm/emac/core.c#L2578>
> > +   phy-handle = <>;
> > +   rgmii-device = <>;
> > +   rgmii-channel = <0>;
>

Re: [RFC 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

On Wednesday, February 15, 2017 3:23:08 PM CET Andrew Lunn wrote:
> > > > Is the PHY just powered down by chance (BMCR_PWRDN set?) and resetting
> > > > it implicitly clears the power down that seems to be what is going on.
> > > 
> > > Yes, the PHY is just in the BMCR_PDOWN state. I can do the same
> > > on the WNDR4700, by messing with u-boot:
> 
> Hi Christian
> 
> What happens if you list the PHYs in the device tree, with their PHY
> ID. That should avoid it looking for the ID and getting 0x
> back. It should just probe the correct PHY driver. If the first thing
> the drivers probe function does it reset the power down bit, it might
> work.

I just tested it. And it didn't work (Same result/error).

It fails because the PHYs on the dsa slave-bus are not detected via the
device tree method. See 


|315 if (!ds->slave_mii_bus && ds->ops->phy_read) {
|316 ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
|317 if (!ds->slave_mii_bus)
|318 return -ENOMEM;
|319 
|320 dsa_slave_mii_bus_init(ds);
|321 
|322 err = mdiobus_register(ds->slave_mii_bus);
|323 if (err < 0)
|324 return err;
|325 }

You see that dsa2 just calls mdiobus_register() which will do the
PHY discovery with mdiobus_scan() 

which relys on the values from MII_PHYSID1 and MII_PHYSID2.

In order to get it work, this mdiobus_register would need to be
replaced with of_mdiobus_register().

---
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 737be6470c7f..5a90ec81040f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "dsa_priv.h"
 
 static LIST_HEAD(dsa_switch_trees);
@@ -288,7 +289,8 @@ static void dsa_user_port_unapply(struct dsa_port *port, 
u32 index,
}
 }
 
-static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
+static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds,
+   struct device_node *ports)
 {
struct dsa_port *port;
u32 index;
@@ -322,7 +324,10 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, 
struct dsa_switch *ds)
 
dsa_slave_mii_bus_init(ds);
 
-   err = mdiobus_register(ds->slave_mii_bus);
+   if (!ports)
+   err = mdiobus_register(ds->slave_mii_bus);
+   else
+   err = of_mdiobus_register(ds->slave_mii_bus, ports);
if (err < 0)
return err;
}
@@ -383,7 +388,8 @@ static void dsa_ds_unapply(struct dsa_switch_tree *dst, 
struct dsa_switch *ds)
dsa_switch_unregister_notifier(ds);
 }
 
-static int dsa_dst_apply(struct dsa_switch_tree *dst)
+static int dsa_dst_apply(struct dsa_switch_tree *dst,
+struct device_node *ports)
 {
struct dsa_switch *ds;
u32 index;
@@ -394,7 +400,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
if (!ds)
continue;
 
-   err = dsa_ds_apply(dst, ds);
+   err = dsa_ds_apply(dst, ds, ports);
if (err)
return err;
}
@@ -649,7 +655,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, 
struct device *dev)
struct dsa_chip_data *pdata = dev->platform_data;
struct device_node *np = dev->of_node;
struct dsa_switch_tree *dst;
-   struct device_node *ports;
+   struct device_node *ports = NULL;
u32 tree, index;
int i, err;
 
@@ -722,7 +728,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, 
struct device *dev)
goto out_del_dst;
}
 
-   err = dsa_dst_apply(dst);
+   err = dsa_dst_apply(dst, ports);
if (err) {
dsa_dst_unapply(dst);
goto out_del_dst;
---

With this patch applied, the device discovery works as intended:


| [4.514106] Generic PHY 4ef600c00.ethern:00: attached PHY driver [Generic 
PHY] (mii_bus:phy_addr=4ef600c00.ethern:00, irq=-1)
|[4.525975] Generic PHY dsa-0.0:01: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:01, irq=-1)
|[4.536269] Generic PHY dsa-0.0:02: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:02, irq=-1)
|[4.546562] Generic PHY dsa-0.0:03: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:03, irq=-1)
|[4.556887] Generic PHY dsa-0.0:04: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:04, irq=-1)

>From what I can tell, the PHY works. Although it will ping-pong for a bit:
|[  170.463823] qca8k 4ef600c00.ethern:10 lan1: Link is Down
|[  172.511860] qca8k 4ef600c00.ethern:10 lan1: Link is Up - 1Gbps/Full - flow 
control rx/tx
|[  174.559823] qca8k 

Re: [RFC 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

On Tuesday, February 14, 2017 12:38:42 AM CET Christian Lamparter wrote:
> > >>> diff --git a/drivers/net/ethernet/ibm/emac/core.c 
> > >>> b/drivers/net/ethernet/ibm/emac/core.c
> > >>> --- a/drivers/net/ethernet/ibm/emac/core.c
> > >>> +++ b/drivers/net/ethernet/ibm/emac/core.c
> > >>> @@ -2420,6 +2421,179 @@ static int emac_read_uint_prop(struct 
> > >>> device_node *np, const char *name,
> > >>> [...] 
> > >>> +static int emac_mii_bus_reset(struct mii_bus *bus)
> > >>> +{
> > >>> +   struct emac_instance *dev = netdev_priv(bus->priv);
> > >>> +
> > >>> +   emac_mii_reset_phy(>phy);
> > >>
> > >> This seems wrong, emac_mii_reset_phy() does a BMCR software reset, which
> > >> PHYLIB is already going to do (phy_init_hw), yet you do this here at the
> > >> MDIO bus level towards a specify PHY, whereas this should be affecting
> > >> the MDIO bus itself (and/or *all* PHY child devices for quirks).
> > > Ah, this is a good point. The emac driver has a emac_reset() function
> > > that does disable and enabled the phy clocks. That said, this is already
> > > done by the emac driver during init too. So if I added it, the bus is
> > > reset twice (since it doesn't hurt - I added it back).
> > > 
> > > The emac_mii_phy_reset() was added because of the Meraki MX60(W).
> > > This is because Cisco's bootloader disables the switch port 
> > > (probably to prevent WAN<->LAN leakage during boot)
> > > 
> > > [bootlog from the MX60(W)]
> > > |Disabling port 0
> > > |Disabling port 1
> > > |Disabling port 2
> > > |Disabling port 3
> > > |ENET Speed is 1000 Mbps - FULL duplex connection (EMAC0)
> > > 
> > > Without emac_mii_reset_phy(), the mdiobus_scan() function, which
> > > is called by mdiobus_register will fail with -ENODEV.
> > > | /plb/opb/ethernet@ef600c00: failed to attach dt phy (-19).
> > > This is because get_phy_id() will "mostly read mostly Fs" and abort.
> > 
> > Is the PHY just powered down by chance (BMCR_PWRDN set?) and resetting
> > it implicitly clears the power down that seems to be what is going on.
> 
> Yes, the PHY is just in the BMCR_PDOWN state. I can do the same
> on the WNDR4700, by messing with u-boot:
> 
> | => mii write 0 0 0x0800
> | => mii dump
> | 0. () -- PHY control register --
> |  (8000:8000) 0.15= 1reset
> |  (4000:4000) 0.14= 1loopback
> |  (2040:2040) 0. 6,13 =   b11speed selection = ??? Mbps
> |  (1000:1000) 0.12= 1A/N enable
> |  (0800:0800) 0.11= 1power-down
> |  (0400:0400) 0.10= 1isolate
> |  (0200:0200) 0. 9= 1restart A/N
> |  (0100:0100) 0. 8= 1duplex = full
> |  (0080:0080) 0. 7= 1collision test enable
> |  (003f:003f) 0. 5- 0 =63(reserved)
> 
> On the Meraki, the port disabled by the bootloader. 
> The reset is still needed.
> 
> > Keep in mind that MDIO address 16 is the switch's pseudo PHY address
> > here, so if you are telling PHYLIB to probe for that address and you
> > don't get the expected MII_PHYSID1/2 value in return, that usually means
> > that there was a PHY fixup registered to intercept these reads and make
> > us return the switch's unique identifier. Reading from the switch's
> > pseudo PHY at address 16 registers 2/3 (MII_PHYSID1/2) is not guaranteed
> > to return the switch's unique identifer.
> > 
> > With a MDIO device driver this won't happen because you will be probed
> > by address, and you can read any switch register you want to and from
> > there move on with the initialization.
> > 
> > > 
> > > 
> > > With emac_mii_reset_phy() in place, it gets detected:
> > > | switch0: Atheros AR8327 rev. 4 switch registered on emac_mdio
> > > 
> > > Furthermore, this is probably not the only device which need it.
> > > Currently, emac's own phy.c code does call emac_mii_reset_phy() 
> > > as well as part of its probe procedure.
> > > <http://lxr.free-electrons.com/source/drivers/net/ethernet/ibm/emac/phy.c#L522>
> > > 
> > > Ideally, we would like to reset only the ports which are registered in 
> > > the DT.
> > 
> > Which you would get for free if you did extend qca8k to support the
> > 8327, because qca8k does implicitly tell the DSA layer to register a
> > dsa_slave_mii_bus which will probe 

Re: [RFC 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

On Saturday, February 11, 2017 3:07:04 PM CET Florian Fainelli wrote:
> Le 02/11/17 à 14:45, Christian Lamparter a écrit :
> > On Sunday, February 5, 2017 2:44:54 PM CET Florian Fainelli wrote:
> >> Le 02/05/17 à 14:25, Christian Lamparter a écrit :
> >>> From: Christian Lamparter <chunk...@gmail.com>
> >>>
> >>> This patch adds glue-code that allows the EMAC driver to interface
> >>> with the existing dt-supported PHYs in drivers/net/phy.
> >>>
> >>> Because currently, the emac driver maintains a small library of
> >>> supported phys for in a private phy.c file located in the drivers
> >>> directory.
> >>>
> >>> The support is limited to mostly single ethernet transceiver like the:
> >>> CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.
> >>> However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
> >>> have a 5-port switch (QCA8327N) attached to the MDIO of the EMAC.
> >>> The switch chip has already a proper phy-driver (qca8k) that uses
> >>> the generic phy library.
> >>
> >> Technically, it's a mdio_device in the upstream kernel that registers a
> >> switch with DSA (and a PHY device in the OpenWrt/LEDE downstream
> >> kernel). If your goal is to specifically support that device you should
> >> consider making the EMAC interface with a fixed link PHY to properly
> >> initialize the EMAC <=> CPU port of the switch link, and then declare
> >> the qca8k device as a child MDIO device (not a PHY), similar to what is
> >> done in arch/arm/boot/dts/vf610-zii-dev-rev-b.dts for instance.
> > 
> > Ok. I looked what was going on here. As you explained: qca8k is indeed 
> > the wrong driver. We do use the ar8216 with swconfig interface.
> 
> Can you look into adding support for the 8216 into
> drivers/net/dsa/qca8k.c? You don't necessarily need to use QCA tags
> (using DSA_PROTO_NONE works too) and this would be a good way to know
> what could be missing in that driver, you'd also get per-port network
> devices, which could all be driving their built-in PHYs (so ethtool and
> friends work as expected).
Oh, I don't have any devices with an AR8216. Both the Meraki MX60(W) and
the WNDR4700 have the AR8327. I only mentioned AR8216, because that's
the driver module in OpenWRT/LEDE which supports the AR8327 [0], [1].

As for emac and AR8327N: It will come right up, once the QCA8337-only guard 
is removed from qca8k.c [2]. [QCA8K_ID_QCA8337 is 0x13, AR8327N is 0x12]:
|954if (id != QCA8K_ID_QCA8337)
|955return -ENODEV;

[4.250097] libphy: emac_mdio: probed
[4.253789] mdio_bus 4ef600c00.ethern:10: mdio_device_register
[4.346679] eth0: EMAC-0 /plb/opb/ethernet@ef600c00, MAC 10:0d:7f:4e:20:6e
[...]
[4.425333] DSA: switch 0 0 parsed
[4.428751] DSA: tree 0 parsed
[4.496094] libphy: dsa slave smi: probed
[4.513056] Generic PHY 4ef600c00.ethern:00: attached PHY driver [Generic 
PHY] (mii_bus:phy_addr=4ef600c00.ethern:00, irq=-1)
[4.524916] Generic PHY dsa-0.0:01: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:01, irq=-1)
[4.535219] Generic PHY dsa-0.0:02: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:02, irq=-1)
[4.545504] Generic PHY dsa-0.0:03: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:03, irq=-1)
[4.555815] Generic PHY dsa-0.0:04: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=dsa-0.0:04, irq=-1)

# ip link
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP 
mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
3: lan4@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue switchid 
 state UP mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
4: lan3@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop switchid  state 
DOWN mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
5: lan2@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop switchid  state 
DOWN mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
6: lan1@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop switchid  state 
DOWN mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
7: wan@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop switchid  state 
DOWN mode DEFAULT group default qlen 1000
link/ether 10:0d:7f:4e:20:6e brd ff:ff:ff:ff:ff:ff
 
> > As for this patch. Currently the apm821xx target in LEDE has two supported
> > routers, on AP and one NAS.
> > 
> > Both routers: The Netgear WN

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Sunday, February 12, 2017 5:42:18 AM CET Al Viro wrote:
> On Sat, Feb 11, 2017 at 08:37:06PM +0100, Christian Lamparter wrote:
> 
> > I think if you follow through with this argument. You have the problem of:
> > How to handle EFAULT from skb_copy_datagram_* (and all it's "wrappers")?
> > 
> > Because on one hand, the iovec could be partially bad. I remember that 
> > the application could do the following shenanigans during recvmsg: 
> >  - mprotect() could've flipped page read-only and back to read-write.
> >  - Or truncate() could've shortened the mmapped file,
> >  - etc.
> > 
> > In this case the error should be propagated back to the userspace.
> > 
> > But OTOH, it could just be a temporary failure (*) and restoring the
> > iovec and trying again is needed.
> 
> No.  You can't _rely_ upon -EFAULT being repeated, but it's not something
> you would expect to retry your way out of.
> 
> The sane semantics is
>   * fail read/recvmsg (with EFAULT) if it's a datagram socket
>   * fail if it's a stream socket and nothing has been read by
> that point
>   * a short read if something has been already read.
> 
> > Is this a correct/complete assessment of the problem at hand? Or did
> > I make a mistake / wrong assumption in there?
> 
> > I'm looking at:
> > <http://lxr.free-electrons.com/source/net/ipv4/tcp_input.c#L4668>
> > <http://lxr.free-electrons.com/source/net/ipv4/tcp_input.c#L5232>
> > <http://lxr.free-electrons.com/source/net/ipv4/tcp_input.c#L5465>
> > 
> > >From what I can see, the tcp functions tcp_data_queue(),
> > tcp_copy_to_iovec() and tcp_rcv_established() would need to be
> > extended to handle EFAULT. Because if the iovec is restored
> > and the application did something bad (mprotect(), truncate(),
> >  ...), this code would sort of loop?
> 
> tcp_v4_do_rcv() has every right to copy nothing whatsoever - it's a fastpath
> and when e.g. it's called in context of another thread or when skb isn't the
> next fragment expected it won't bother with tcp_copy_to_iovec() at all.
> Failure to copy anything in there is just fine, as long as you don't end
> up buggering tp->ucopy state (in particular, tp->ucopy.msg->msg_iter).
> 
> > If this is the case: How many retries do we want, before we can
> > say it is a permament failure (and abort)?
> 
> We don't want any.  What happens is that this path won't copy anything and
> when that skb gets to
> err = skb_copy_datagram_msg(skb, offset, msg, used);
> if (err) {
> /* Exception. Bailout! */
> if (!copied)
> copied = -EFAULT;
> break;
> }
> in tcp_recvmsg() we'll get our short read.
> 
> Again, the trouble is not with tcp_v4_do_rcv() failing to copy something -
> it's failing to copy and ending up with iov_iter advanced that might be
> a problem.  E.g. tp->ucopy.len getting out of sync with 
> tp->ucopy.msg->msg_iter,
> etc.
> 
> Short read on fault is fine.  So's full copy if somebody had been flipping
> memprotect() and slow path ends up catching the moment when the buffer is
> writable.  Both outcomes are fine.  Having the same memprotect() flipping
> leave ->msg_iter more than one would expect by tp->ucopy.len and everything
> back with copy_to_user working again, OTOH, might confuse tcp_input.c.
> 
Ok, thank you for sticking around. As for the patch: I've tested it with
the dlbug program from <https://lkml.org/lkml/2016/7/26/25> (modified to
pull from a local server) and the netem corruption policy as described
in <https://lkml.org/lkml/2016/8/3/181>. 
It works as expected. I did not get a single corruption with the patch
applied. Without the patch: every try had corruptions in random places. 

Tested-by: Christian Lamparter <chunk...@googlemail.com>

Regards,
Christian

Re: [RFC 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

Hello,

I'm sorry for the delay.

On Sunday, February 5, 2017 2:44:54 PM CET Florian Fainelli wrote:
> Le 02/05/17 à 14:25, Christian Lamparter a écrit :
> > From: Christian Lamparter <chunk...@gmail.com>
> > 
> > This patch adds glue-code that allows the EMAC driver to interface
> > with the existing dt-supported PHYs in drivers/net/phy.
> > 
> > Because currently, the emac driver maintains a small library of
> > supported phys for in a private phy.c file located in the drivers
> > directory.
> > 
> > The support is limited to mostly single ethernet transceiver like the:
> > CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.
> > However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
> > have a 5-port switch (QCA8327N) attached to the MDIO of the EMAC.
> > The switch chip has already a proper phy-driver (qca8k) that uses
> > the generic phy library.
> 
> Technically, it's a mdio_device in the upstream kernel that registers a
> switch with DSA (and a PHY device in the OpenWrt/LEDE downstream
> kernel). If your goal is to specifically support that device you should
> consider making the EMAC interface with a fixed link PHY to properly
> initialize the EMAC <=> CPU port of the switch link, and then declare
> the qca8k device as a child MDIO device (not a PHY), similar to what is
> done in arch/arm/boot/dts/vf610-zii-dev-rev-b.dts for instance.

Ok. I looked what was going on here. As you explained: qca8k is indeed 
the wrong driver. We do use the ar8216 with swconfig interface.

As for this patch. Currently the apm821xx target in LEDE has two supported
routers, on AP and one NAS.

Both routers: The Netgear WNDR4700 and the Cisco MX60(W) use the AR8327N.

The AP: The Cisco Meraki MR24 has a AR8035 PHY. There's the at803x. driver,
but David Miller was nice enough to merge this patch [0]. This patch added
support for it in in emac's phy.c, however it also limits it to the MR24.

The NAS: Western Digital My Book Live (Uno and Duo) have a Broadcom PHY
BCM54610 (it is detected as a BCM50610 PHY with a better version of this
patch). There's a proper phy driver in the kernel for it too (broadcom.c).
However, emac is limited to its own generic phy driver for this device.

Before I can answer the comments, I would like to deal with 
the kbuild-test-robot. It discovered the following issues:

|   drivers/built-in.o: In function `emac_mdio_cleanup.isra.2':
|>> core.c:(.text+0x70464): undefined reference to `mdiobus_free'
|>> core.c:(.text+0x70494): undefined reference to `mdiobus_unregister'
|   core.c:(.text+0x704a0): undefined reference to `mdiobus_free'
|   drivers/built-in.o: In function `emac_remove':
|>> core.c:(.text+0x70500): undefined reference to `phy_disconnect'

All these symbols are defined in include/linux/phy.h though.
So, shouldn't there be some stubs for those functions in the
header in case CONFIG_PHYLIB is not defined.
Is this a simple oversight, or is there more to it?
(I can add them if necessary. Or is someone looking for "easy" work?)

> > Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
> > ---
> >  drivers/net/ethernet/ibm/emac/core.c | 188 
> > +++
> >  drivers/net/ethernet/ibm/emac/core.h |   4 +
> >  2 files changed, 192 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/ibm/emac/core.c 
> > b/drivers/net/ethernet/ibm/emac/core.c
> > index 6ead2335a169..ea9234cdb227 100644
> > --- a/drivers/net/ethernet/ibm/emac/core.c
> > +++ b/drivers/net/ethernet/ibm/emac/core.c
> > @@ -42,6 +42,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include 
> > @@ -2420,6 +2421,179 @@ static int emac_read_uint_prop(struct device_node 
> > *np, const char *name,
> > return 0;
> >  }
> >  
> > +static void emac_adjust_link(struct net_device *ndev)
> > +{
> > +   struct emac_instance *dev = netdev_priv(ndev);
> > +   struct phy_device *phy = dev->phy_dev;
> > +
> > +   mutex_lock(>link_lock);
> > +   dev->phy.autoneg = phy->autoneg;
> > +   dev->phy.speed = phy->speed;
> > +   dev->phy.duplex = phy->duplex;
> > +   dev->phy.pause = phy->pause;
> > +   dev->phy.asym_pause = phy->asym_pause;
> > +   dev->phy.advertising = phy->advertising;
> > +   mutex_unlock(>link_lock);
> 
> PHYLIB already executes grabbing the phy device's mutex, is this really
> needed here?
Yes, this is a bug. I accidently sent a very old version.
(In fact, the LEDE patch had it already fixed[1].)

> > +}
> > +
> > +static int emac_mii_bus_read(struct mii_bus *bus, 

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Friday, February 10, 2017 9:45:13 PM CET Al Viro wrote:
> On Tue, Aug 09, 2016 at 03:58:36PM +0100, Al Viro wrote:
> 
> > Actually returning to the original behaviour would be "restore ->msg_iter
> > if we tried skb_copy_and_csum_datagram() and failed for any reason".  Which
> > would be bloody inconsistent wrt EFAULT, since the other branch (chunk
> > large enough to cover the entire recvmsg()) will copy as much as it can
> > and (in old kernel) drain iovec or (on the current one) leave iov_iter
> > advance unreverted.
> 
> To resurrect the old thread: the problem is still there.  Namely, csum
> mismatch on packet should leave the iterator as it had been.  That much
> is clear; the question is what should be done on EFAULT halfway through.
Thanks for being very persistent with this. The original problem report 
was just about the data corruption issue. I think everyone involved agrees
that restoring the iterator for cases where the checksum check failed
is definitely the right action. (And of course: It is high time that the
data corruption issue gets fixed).

However, it's as you have said earlier about -EFAULT:
"[...] That's why I hadn't simply ACKed the proposed patch; it very much smells
like we have something bogus with EFAULT handling in the whole area."

Because from the explanations that: (*) "-EFAULT can happen at any point, with
zero warning before you get actual page fault when copying the data and
have handle_mm_fault() return VM_FAULT_ERROR. "

I think if you follow through with this argument. You have the problem of:
How to handle EFAULT from skb_copy_datagram_* (and all it's "wrappers")?

Because on one hand, the iovec could be partially bad. I remember that 
the application could do the following shenanigans during recvmsg: 
 - mprotect() could've flipped page read-only and back to read-write.
 - Or truncate() could've shortened the mmapped file,
 - etc.

In this case the error should be propagated back to the userspace.

But OTOH, it could just be a temporary failure (*) and restoring the
iovec and trying again is needed.

Is this a correct/complete assessment of the problem at hand? Or did
I make a mistake / wrong assumption in there?

> Semantics of both csum and non-csum skb_copy_datagram_msg() variants in
> EFAULT case is an interesting question.  None of that family report
  support?
> partial copy; it's full or -EFAULT.  So for the sake of basic sanity
> it would be better to leave iterator in the original state when that
> kind of thing happens.  On the other hand, quite a few callers don't
> care about the state of iterator after that and I wonder if the overhead
> would be sensitive.  OTTH, the overhead in question is "save 5 words into
> local variable and don't use it in the normal case" - in the code that
> copies an skb worth of data.
> 
> AFAICS, the following gives consistent (and minimally surprising) semantics,
> as well as fixing the outright bug with iov_iter left advanced in case of csum
> errors.  Comments?

I'm looking at:




>From what I can see, the tcp functions tcp_data_queue(),
tcp_copy_to_iovec() and tcp_rcv_established() would need to be
extended to handle EFAULT. Because if the iovec is restored
and the application did something bad (mprotect(), truncate(),
 ...), this code would sort of loop?

If this is the case: How many retries do we want, before we can
say it is a permament failure (and abort)?

Regards,
Christian

[...]
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index f1adddc1c5ac..ee8d962373af 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3060,8 +3060,17 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, 
> unsigned flags, int noblock,
> int *err);
>  unsigned int datagram_poll(struct file *file, struct socket *sock,
>  struct poll_table_struct *wait);
> -int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
> +int __skb_copy_datagram_iter(const struct sk_buff *from, int offset,
>  struct iov_iter *to, int size);
> +static inline int skb_copy_datagram_iter(const struct sk_buff *from, int 
> offset,
> + struct iov_iter *to, int size)
> +{
> + struct iov_iter saved = *to;
> + int res = __skb_copy_datagram_iter(from, offset, to, size);
> + if (unlikely(res))
> + *to = saved;
> + return res;
> +}
>  static inline int skb_copy_datagram_msg(const struct sk_buff *from, int 
> offset,
>   struct msghdr *msg, int size)
>  {
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index ea633342ab0d..33ff2046dda1 100644
> --- a/net/core/datagram.c
> +++ 

[RFC 1/2] dt: emac: document device-tree based phy discovery and setup

[Christian Lamparter]

This patch adds documentation for a new "phy-handler" property
and "mdio" sub-node. These allows the enumeration of PHYs which
are supported by the phy library under drivers/net/phy.

The EMAC ethernet controller in IBM and AMCC 4xx chips is
currently stuck with a few privately defined phy
implementations. It has no support for PHYs which
are supported by the generic phylib.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 .../devicetree/bindings/powerpc/4xx/emac.txt   | 60 +-
 1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt 
b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
index 712baf6c3e24..0572d053c35a 100644
--- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
+++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
@@ -71,6 +71,8 @@
  For Axon it can be absent, though my current driver
  doesn't handle phy-address yet so for now, keep
  0x00ff in it.
+- phy-handle   : See net/ethernet.txt file; used to describe
+ configurations where a external PHY is used.
 - rx-fifo-size-gige : 1 cell, Rx fifo size in bytes for 1000 Mb/sec
  operations (if absent the value is the same as
  rx-fifo-size).  For Axon, either absent or 2048.
@@ -82,7 +84,18 @@
 - tah-channel   : 1 cell, optional. If appropriate, channel used on the
  TAH engine.
 
-Example:
+- mdio subnode : When the EMAC has a phy connected to its local
+ mdio, which us supported by the kernel's network
+ PHY library in drivers/net/phy, there must be device
+ tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+
+ For each phy on the mdio bus, there must be a node
+ with the following fields:
+   - reg: phy id used to communicate to phy.
+   - device_type: Must be "ethernet-phy".
+Examples:
 
EMAC0: ethernet@4800 {
device_type = "network";
@@ -104,6 +117,50 @@
zmii-channel = <0>;
};
 
+   EMAC1: ethernet@ef600c00 {
+   device_type = "network";
+   compatible = "ibm,emac-apm821xx", "ibm,emac4sync";
+   interrupt-parent = <>;
+   interrupts = <0 1>;
+   #interrupt-cells = <1>;
+   #address-cells = <0>;
+   #size-cells = <0>;
+   interrupt-map = <0  0x10 IRQ_TYPE_LEVEL_HIGH /* Status */
+1  0x14 IRQ_TYPE_LEVEL_HIGH /* Wake */>;
+   reg = <0xef600c00 0x00c4>;
+   local-mac-address = []; /* Filled in by U-Boot */
+   mal-device = <>;
+   mal-tx-channel = <0>;
+   mal-rx-channel = <0>;
+   cell-index = <0>;
+   max-frame-size = <9000>;
+   rx-fifo-size = <16384>;
+   tx-fifo-size = <2048>;
+   fifo-entry-size = <10>;
+   phy-mode = "rgmii";
+   phy-map = <0x>;
+   phy-handle = <>;
+   rgmii-device = <>;
+   rgmii-channel = <0>;
+   tah-device = <>;
+   tah-channel = <0>;
+   has-inverted-stacr-oc;
+   has-new-stacr-staopc;
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   device_type = "ethernet-phy";
+   reg = <0>;
+
+   qca,ar8327-initvals = <
+   0x0010 0x4000>;
+   };
+   };
+
+
   ii) McMAL node
 
 Required properties:
@@ -145,4 +202,3 @@
 - revision   : as provided by the RGMII new version register if
   available.
   For Axon: 0x012a
-
-- 
2.11.0

[RFC 2/2] net: emac: add support for device-tree based PHY discovery and setup

[Christian Lamparter]

From: Christian Lamparter <chunk...@gmail.com>

This patch adds glue-code that allows the EMAC driver to interface
with the existing dt-supported PHYs in drivers/net/phy.

Because currently, the emac driver maintains a small library of
supported phys for in a private phy.c file located in the drivers
directory.

The support is limited to mostly single ethernet transceiver like the:
CIS8201, BCM5248, ET1011C, Marvell 88E and 88E1112, AR8035.
However, routers like the Netgear WNDR4700 and Cisco Meraki MX60(W)
have a 5-port switch (QCA8327N) attached to the MDIO of the EMAC.
The switch chip has already a proper phy-driver (qca8k) that uses
the generic phy library.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 188 +++
 drivers/net/ethernet/ibm/emac/core.h |   4 +
 2 files changed, 192 insertions(+)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 6ead2335a169..ea9234cdb227 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -2420,6 +2421,179 @@ static int emac_read_uint_prop(struct device_node *np, 
const char *name,
return 0;
 }
 
+static void emac_adjust_link(struct net_device *ndev)
+{
+   struct emac_instance *dev = netdev_priv(ndev);
+   struct phy_device *phy = dev->phy_dev;
+
+   mutex_lock(>link_lock);
+   dev->phy.autoneg = phy->autoneg;
+   dev->phy.speed = phy->speed;
+   dev->phy.duplex = phy->duplex;
+   dev->phy.pause = phy->pause;
+   dev->phy.asym_pause = phy->asym_pause;
+   dev->phy.advertising = phy->advertising;
+   mutex_unlock(>link_lock);
+}
+
+static int emac_mii_bus_read(struct mii_bus *bus, int addr, int regnum)
+{
+   return emac_mdio_read(bus->priv, addr, regnum);
+}
+
+static int emac_mii_bus_write(struct mii_bus *bus, int addr, int regnum,
+ u16 val)
+{
+   emac_mdio_write(bus->priv, addr, regnum, val);
+   return 0;
+}
+
+static int emac_mii_bus_reset(struct mii_bus *bus)
+{
+   struct emac_instance *dev = netdev_priv(bus->priv);
+
+   emac_mii_reset_phy(>phy);
+   return 0;
+}
+
+static int emac_mdio_probe(struct emac_instance *dev)
+{
+   struct device_node *mii_np;
+   struct mii_bus *bus;
+   int res;
+
+   bus = mdiobus_alloc();
+   if (!bus)
+   return -ENOMEM;
+
+   mii_np = of_get_child_by_name(dev->ofdev->dev.of_node, "mdio");
+   if (!mii_np) {
+   dev_err(>ndev->dev, "no mdio definition found.");
+   return -ENODEV;
+   }
+
+   if (!of_device_is_available(mii_np))
+   return 0;
+
+   bus->priv = dev->ndev;
+   bus->parent = dev->ndev->dev.parent;
+   bus->name = "emac_mdio";
+   bus->read = _mii_bus_read;
+   bus->write = _mii_bus_write;
+   bus->reset = _mii_bus_reset;
+
+   snprintf(bus->id, MII_BUS_ID_SIZE, "%s", bus->name);
+
+   res = of_mdiobus_register(bus, mii_np);
+   if (res) {
+   dev_err(>ndev->dev, "cannot register MDIO bus %s\n",
+   bus->name);
+   mdiobus_free(bus);
+   }
+
+   dev->mii_bus = bus;
+   return res;
+}
+
+static void emac_mdio_cleanup(struct emac_instance *dev)
+{
+   if (dev->mii_bus) {
+   if (dev->mii_bus->state == MDIOBUS_REGISTERED)
+   mdiobus_unregister(dev->mii_bus);
+   mdiobus_free(dev->mii_bus);
+   dev->mii_bus = NULL;
+   kfree(dev->phy.def);
+   }
+}
+
+static int stub_setup_aneg(struct mii_phy *phy, u32 advertise)
+{
+   return 0;
+}
+
+static int stub_setup_forced(struct mii_phy *phy, int speed, int fd)
+{
+   return 0;
+}
+
+static int stub_poll_link(struct mii_phy *phy)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   return dev->opened;
+}
+
+static int stub_read_link(struct mii_phy *phy)
+{
+   struct net_device *ndev = phy->dev;
+   struct emac_instance *dev = netdev_priv(ndev);
+
+   phy_start(dev->phy_dev);
+   return 0;
+}
+
+static const struct mii_phy_ops emac_stub_phy_ops = {
+   .setup_aneg = stub_setup_aneg,
+   .setup_forced   = stub_setup_forced,
+   .poll_link  = stub_poll_link,
+   .read_link  = stub_read_link,
+};
+
+static int emac_probe_dt_phy(struct emac_instance *dev)
+{
+   struct device_node *np = dev->ofdev->dev.of_node;
+   struct device_node *phy_handle;
+   struct net_device *ndev = dev->ndev;
+   int res;
+
+   phy_handle = of_pars

Re: [PATCH] p54: memset(0) whole array

[Christian Lamparter]

On Friday, October 14, 2016 11:23:09 AM CEST Jiri Slaby wrote:
> gcc 7 complains:
> drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
> drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with 
> length equal to number of elements without multiplication by element size 
> [-Wmemset-elt-size]
> 
> Fix that by passing the correct size to memset.
> 
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
> Cc: Christian Lamparter <chunk...@googlemail.com>
> Cc: Kalle Valo <kv...@codeaurora.org>
> Cc: linux-wirel...@vger.kernel.org
> Cc: netdev@vger.kernel.org
Acked-by: Christian Lamparter <chunk...@googlemail.com>
> ---
>  drivers/net/wireless/intersil/p54/fwio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/fwio.c 
> b/drivers/net/wireless/intersil/p54/fwio.c
> index 257a9eadd595..4ac6764f4897 100644
> --- a/drivers/net/wireless/intersil/p54/fwio.c
> +++ b/drivers/net/wireless/intersil/p54/fwio.c
> @@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u16 mode, u16 dwell)
>  
>   entry += sizeof(__le16);
>   chan->pa_points_per_curve = 8;
> - memset(chan->curve_data, 0, sizeof(*chan->curve_data));
> + memset(chan->curve_data, 0, sizeof(chan->curve_data));
>   memcpy(chan->curve_data, entry,
>  sizeof(struct p54_pa_curve_data_sample) *
>  min((u8)8, curve_data->points_per_channel));
> 

Re: UDP wierdness around skb_copy_and_csum_datagram_msg()

[Christian Lamparter]

On Friday, September 30, 2016 10:35:25 AM CEST Jay Smith wrote:
> Christian Lamparter writes:
> > On Wednesday, September 28, 2016 7:20:39 PM CEST Jay Smith wrote:
> >> Actually, on a little more searching of this list's archives, I think
> >> that this discussion:  https://patchwork.kernel.org/patch/9260733/ is
> >> about exactly the same issue I've found, except from the TCP side. I'm
> >> cc'ing a few of the participants from that discussion.
> >> 
> >> So is the patch proposed there (copying and restoring the entire
> >> iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
> >> fix?
> >
> > From Alan's post:
> >
> > "My ugly patch fixes this in the most obvious way: make a local copy of
> > msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy
> > it back if the checksum is bad, just before goto csum_error;"
> >
> > IMHO this meant that the patch is a proof of concept for his problem.
> 
> It's also the simplest thing that fixes all of the relevant cases (udp4,
> udp6, tcp4). 
Al Viro noted that tcp needed more work here (tp->ucopy.len and friends).
Trouble is, that this discussion (between Alan, David, me and Eric) was 
offlist at that point... And it doesn't look like anyone involved
wants to repeat what they have already said/written. 

(Al Viro, are you there?)

> [...]
> > As for fixing the issue: I'm happy to test and review patches. 
> > The trouble is that nobody seem to be able to produce them...
> 
> Sorry -- is the trouble you're talking about here that no-one's produced
> a patch, or that we don't have a reproduction of the problem?  I don't
> think either is true.
Reproduction wasn't the issue. Back in August, I posted method 
which used the network emulator (CONFIG_NET_SCH_NETEM) to reproduce
it easily and almost anywhere [0].

(i.e.: run this on the server/router)
# tc qdisc add dev eth0 root netem corrupt 0.1%
 
Alan also wrote a userspace tool that had a select/noselect switch
which would lead to different outcomes... etc. However, in the end
Alan could fix his issue by switching to CCMP(AES WLAN cipher), 
which he reported fixed his corruption issue.

So sadly yes, what I meant was that the patches are missing in action.

Regards,
Christian

[0] <https://lkml.org/lkml/2016/8/3/181>

Re: UDP wierdness around skb_copy_and_csum_datagram_msg()

[Christian Lamparter]

On Wednesday, September 28, 2016 7:20:39 PM CEST Jay Smith wrote:
> Actually, on a little more searching of this list's archives, I think
> that this discussion:  https://patchwork.kernel.org/patch/9260733/ is
> about exactly the same issue I've found, except from the TCP side. I'm
> cc'ing a few of the participants from that discussion.
> 
> So is the patch proposed there (copying and restoring the entire
> iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
> fix?

>From Alan's post:

"My ugly patch fixes this in the most obvious way: make a local copy of
msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy
it back if the checksum is bad, just before goto csum_error;"

IMHO this meant that the patch is a proof of concept for his problem.

> If not, would an alternate one that concealed the save-and-restore logic
> inside iov_iter.c be more acceptable? I'd be happy to produce whatever's
> needed, or yield to someone with stronger feelings about where it should
> go...
Al Viro identified more inconsistencies within the error-paths that deal
with EFAULT in the whole area (in and around skb_copy_and_csum_datagram()).

As far as I can tell the original discussion about the data corruption
issue went off on a tangent and it is stuck in figuring out "How to handle
the errors in tcp_copy_to_iovec()".

As for fixing the issue: I'm happy to test and review patches. 
The trouble is that nobody seem to be able to produce them...

Regards,
Christian

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Wednesday, August 3, 2016 3:49:26 AM CEST Alan Curry wrote:
> Al Viro wrote:
> > 
> > Which just might mean that we have *three* issues here -
> > (1) buggered __copy_to_user_inatomic() (and friends) on some sparcs
> > (2) your ssl-only corruption
> > (3) Alan's x86_64 corruption on plain TCP read - no ssl *or* sparc
> > anywhere, and no multi-segment recvmsg().  Which would strongly argue in
> > favour of some kind of copy_page_to_iter() breakage triggered when handling
> > a fragmented skb, as in (1).  Except that I don't see anything similar in
> > x86_64 uaccess primitives...
> > 
> 
> I think I've solved (3) at least...
> 
> Using the twin weapons of printk and stubbornness, I have built a working
> theory of the bug. I haven't traced it all the way through, so my explanation
> may be partly wrong. I do have a patch that eliminates the symptom in all my
> tests though. Here's what happens:
> 
> A corrupted packet somehow arrives in skb_copy_and_csum_datagram_msg().
> During downloads at reasonably high speed, about 0.1% of my incoming
> packets are bad. Probably because the access point is that suspicious
> Comcast thing.

Thanks for being very persistent with this. I think I'm able to reproduce
this now (on any hardware... like r8169 ethernet) as long as the following 
"traffic policy" is enacted on the HTTP - Server: 

# tc qdisc add dev eth0 root netem corrupt 0.1%

(This needs the "Network Emulation" Sched CONFIG_NET_SCH_NETEM [0].)

With your tool (changed to point to my apache local server). I'm seeing 
corruptions in the "noselect" case. Running it in "select" mode however
and the resulting files have no corruptions.

About AR9170 corruption issues: I know of one report that the AR9170's
Encryption Engine can cause corruptions [1]. In this case outgoing
data was corrupted which lead to deauths/disassocs since the AP was
basically sending out multicast deauths/disassocs with bad addresses.
However, "nohwcrypt" should have made a difference there since the
software decryption would discard the faulty package due the message
integrety checks.

Another source for corruptions could be the USB-PHY (FUSB200) in the
AR9170 [2]. I know it's causing problems for the ath9k_htc. However
not everyone is affected.

One thing I noticed in your previous post is that you "might" not have
draft-802.11n enabled. Do you see any "disabling HT/VHT due to WEP/TKIP use."
in your dmesg logs? If so, check if you can force your AP to use WPA2
with CCMP/AES only.

Regards,
Christian

[0] 
[1] 
[2] 

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

On Tuesday, July 26, 2016 4:57:03 AM CEST Alan Curry wrote:
> Al Viro wrote:
> > On Sun, Jul 24, 2016 at 07:45:13PM +0200, Christian Lamparter wrote:
> > 
> > > > The symptom is that downloaded files (http, ftp, and probably other
> > > > protocols) have small corrupted segments (about 1-2 kilobytes long) in
> > > > random locations. Only downloads that sustain a high speed for at least 
> > > > a
> > > > few seconds are corrupted. Anything small enough to be received in less
> > > > than about 5 seconds is not affected.
> > 
> > Can that sucker be reproduced with netcat?  That would eliminate all issues
> > with multi-iovec recvmsg(2), narrowing the things down quite bit.
> 
> netcat seems to be immune. Comparing strace results, I didn't see any
> recvmsg() calls in the other programs that have had the problem, but there
> is an interesting difference: netcat calls select() to wait for the socket
> to be ready for reading, where my other test programs just call read() and
> let it block until ready.
> 
> So I wrote a small test program to isolate that difference. It downloads
> a file using only read() and write() and a hardcoded HTTP request. It has
> a select mode (main loop alternates read() and select() on the TCP socket)
> and a noselect mode (main loop just read()s the TCP socket).
> 
> The program is included at the bottom of this message.
> 
> I ran it several times in both modes and got corruption if and only if the
> noselect mode was used.
> 
> > 
> > Another thing (and if that works, it's *NOT* a proper fix - it would be
> > papering over the problem, but at least it would show where to look for
> > it) - try (on top of mainline) the following delta:
> > 
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> 
> Will try that patch soon. Meanwhile, here's my test:
> 
> /* Demonstration program "dlbug".
>Usage: dlbug select > outfile
>   or
>   dlbug noselect > outfile
>outfile will contain the full HTTP response. Edit out the HTTP headers
>and what's left should be a valid gzip if the download worked. */
> [...]
Thanks, I gave the program a try with my WNDA3100 and a WN821N v2 devices.
I did not see any corruptions in any of the tests though. Can you tell me
something about your wireless network too? I would like to know what router
and firmware are you using? Also important: what's your wireless configuration?
(WPA?, CCMP or TKIP? HT40, HT20 or Legacy rates? ...)

Probably the quickest and easiest way to get that information is by running
the following commands as root, when you are connected to your wifi network
and post the results:
# iw dev wlan0 link
# iw dev wlan0 scan dump

(You can of course remove your device's MACs, but please do it consistently).

Regards,
Christian

Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)

[Christian Lamparter]

Hello,

I added Al Viro to the CC (probably not necessary...)

On Sunday, July 24, 2016 3:35:14 AM CEST Alan Curry wrote:
> [1.] One line summary of the problem:
> network data corruption (bisected to e5a4b0bb803b)
> 
> [2.] Full description of the problem/report:
> Note: although my bisect ended at a commit from before 3.19, I have the
> same symptom in all newer kernels I've tried, up to 4.6.4.
> 
> The commit was:
> 
> >commit e5a4b0bb803b39a36478451eae53a880d2663d5b
> >Author: Al Viro 
> >Date:   Mon Nov 24 18:17:55 2014 -0500
> >
> >switch memcpy_to_msg() and skb_copy{,_and_csum}_datagram_msg() to 
> > primitives
> 
> The symptom is that downloaded files (http, ftp, and probably other
> protocols) have small corrupted segments (about 1-2 kilobytes long) in
> random locations. Only downloads that sustain a high speed for at least a
> few seconds are corrupted. Anything small enough to be received in less
> than about 5 seconds is not affected.
> 
> If I download the same file twice in a row, the corruption is in different
> places in each copy.
> 
> If I try to do a git clone, it fails a few seconds into the "Receiving
> objects" stage with a deflate error.

Thanks for the detailed bug-report. I looked around the web to see if it
was already reported or not. If found that this issue was reported before:
[0], [1] and [2] by the same person (CC'ed). One difference is that the 
reporter had this issue with rsync on multiple SPARC systems. I ran a
git grep on a 4.7.0-rc7+ (wt-2016-07-21-15-g97bd3b0). But it didn't find
any patches directly referencing the commit. I'm not sure if this issue
has been fixed by now or not. I would greatly appreciate any comment
about this from the "people of netdev" (Al Viro? Alex Mcwhirter?).

As for carl9170: I'm not sure what the driver or firmware can do about
this at this time. You can try to disable the hardware crypto by setting
nohwcrypt via the module option. However, this might not do anything at all.

> [3.] Keywords: networking, carl9170
> 
> [4.] Kernel information
> [4.1.] Kernel version (from /proc/version):
> Multiple versions are known to be affected, from 3.19 to 4.6.4
> 
> [4.2.] Kernel .config file:
> For testing I built with make x86_64_defconfig followed by enabling the
> carl9170 driver, which adds these lines:
> CONFIG_ATH_COMMON=m
> CONFIG_ATH_CARDS=m
> CONFIG_CARL9170=m
> CONFIG_CARL9170_LEDS=y
> CONFIG_CARL9170_WPC=y
> 
> [5.] Most recent kernel version which did not have the bug:
> That would be the predecessor of e5a4b0bb803b39a36478451eae53a880d2663d5b
> which is v3.18-rc6-1620-g17836394e578
> 
> [6.] no Oops
> 
> [7.] A small shell script or example program which triggers the
>  problem (if possible)
> 
> This command fails reliably for me when running an affected kernel:
> 
> git clone git://git.kernel.org/pub/scm/git/git.git
> 
> (I'm including all the standard format stuff suggested by REPORTING-BUGS,
> but I think you can skip from here to section 8.7 without missing anything
> relevant)
Yes, I removed it for the most part. If anyone is interested in the details:
Here's a link to the original post @LKML [3].

> 
> [8.] Environment
> [8.1.] Software (add the output of the ver_linux script here)
> 
> Mostly Debian 8.5 stable packages here.
> 
> [8.3.] Module information (from /proc/modules):
> 
> When I tested with the x86_64_defconfig + carl9170 kernel, there were
> hardly any modules built, and I reproduced the problem after booting with
> init=/bin/sh, so no unnecessary modules were loaded. Currently running a
> normal 4.6.4 kernel which is showing the bug.
> 
> [...]
> [8.7.] Other information that might be relevant to the problem
>(please look in /proc and include all information that you
>think to be relevant):
> 
> lsusb identifies my network device as:
> 
> Bus 005 Device 004: ID 0cf3:1002 Atheros Communications, Inc. TP-Link 
> TL-WN821N v2 802.11n [Atheros AR9170]
> 
> I have version 1.9.9 of carl9170-1.fw in /lib/firmware
Just one additional question: Is the TL-WN821N connected to a USB3 port?

Regards,
Christian

[0] 
[1] 
[2] 
[3] 

Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char

[Christian Lamparter]

On Wednesday, July 20, 2016 5:06:27 PM CEST Xose Vazquez Perez wrote:
> Arnd Bergmann  wrote:
> 
> > rtlwifi, but I found the older r8712u device to work fine with
> > the staging/rtl8712 driver.
> 
> A replacement for "staging/rtl8712", with MAC80211 support, is
> available at: https://github.com/chunkeey/rtl8192su
> 
> Also a fullmac/cfg80211 driver(r92su) is available at the
> same repository.
> 
Yes, it has its problems. The rtl8712/r92su isn't really
a fullmac device. While the MLME (scan, probe, auth, assoc) is
done by the firmware. The 802.11 frame creation (from 802.3)
frames needs to be done by the driver. The rtl8712 firmware
however has its fair share of issues. Like: no support for
(tx) fragmentation and ERP is a mystery too. monitor 
mode is barely working and limited to 20Mhz wide channels.
There's also limited tx injection support and of course
stability issues (just like with the staging/rtl8712 driver
or FreeBSD's rsu driver).

The rtl8192su driver (based on rtlwifi) in the same repository
has proper fragmentation support. But it uses the firmware from
the windows/mac os x driver, which is similar to rtl8192se softmac
firmware in design. Getting it to work properly in station mode
however either needs "some magic" or help from Realtek's USB group...

[PATCH] drivers: net: emac: add Atheros AR8035 phy initialization code

[Christian Lamparter]

This patch adds the phy initialization code for Qualcomm
Atheros AR8035 phy. This configuration is found in the
Cisco Meraki MR24.

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/net/ethernet/ibm/emac/phy.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/ibm/emac/phy.c 
b/drivers/net/ethernet/ibm/emac/phy.c
index d3b9d10..5b88cc6 100644
--- a/drivers/net/ethernet/ibm/emac/phy.c
+++ b/drivers/net/ethernet/ibm/emac/phy.c
@@ -470,12 +470,38 @@ static struct mii_phy_def m88e1112_phy_def = {
.ops= _phy_ops,
 };
 
+static int ar8035_init(struct mii_phy *phy)
+{
+   phy_write(phy, 0x1d, 0x5); /* Address debug register 5 */
+   phy_write(phy, 0x1e, 0x2d47); /* Value copied from u-boot */
+   phy_write(phy, 0x1d, 0xb);/* Address hib ctrl */
+   phy_write(phy, 0x1e, 0xbc20); /* Value copied from u-boot */
+
+   return 0;
+}
+
+static struct mii_phy_ops ar8035_phy_ops = {
+   .init   = ar8035_init,
+   .setup_aneg = genmii_setup_aneg,
+   .setup_forced   = genmii_setup_forced,
+   .poll_link  = genmii_poll_link,
+   .read_link  = genmii_read_link,
+};
+
+static struct mii_phy_def ar8035_phy_def = {
+   .phy_id = 0x004dd070,
+   .phy_id_mask= 0xfff0,
+   .name   = "Atheros 8035 Gigabit Ethernet",
+   .ops= _phy_ops,
+};
+
 static struct mii_phy_def *mii_phy_table[] = {
_phy_def,
_phy_def,
_phy_def,
_phy_def,
_phy_def,
+   _phy_def,
_phy_def,
NULL
 };
-- 
2.8.1

[RESEND] Re: updating carl9170-1.fw in linux-firmware.git

[Christian Lamparter]

On Wednesday, April 20, 2016 10:59:44 AM Kalle Valo wrote:
> Christian Lamparter <chunk...@googlemail.com> writes:
> 
> > On Monday, April 18, 2016 07:42:05 PM Kalle Valo wrote:
> >> Christian Lamparter <chunk...@googlemail.com> writes:
> >> 
> >> > On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> >> >
> >> >> Why even mention anything about a "special firmware" as the firmware is
> >> >> already available from linux-firmware.git? 
> >> >
> >> > Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
> >> > but that failed.
> >> > <http://comments.gmane.org/gmane.linux.kernel.wireless.general/114639>
> >> 
> >> Rick's comment makes sense to me, better just to provide the latest
> >> version. No need to unnecessary confuse the users. And if someone really
> >> wants to use an older version that she can retrieve it from the git
> >> history.
> >
> > Part of the fun here is that firmware is GPLv2. The linux-firmware.git has
> > to point to or add the firmware source to their tree. They have added every
> > single source file to it instead of "packaging" it in a tar.bz2/gz/xz
> > like you normally do for release sources.
> >
> > If you want to read more about it:
> > <http://www.spinics.net/lists/linux-wireless/msg101868.html>
> 
> Yeah, that's more work. I get that. But I'm still not understanding
> what's the actual problem which prevents us from updating carl9170
> firmware in linux-firmware.
I'm not sure, but why not ask? I've added the cc'ed Linux Firmware
Maintainers. So for those people reading the fw list:

What would it take to update the carl9170-1.fw firmware file in your
repository to the latest version?

Who has to sent the firmware update. Does it have to be the person who
sent the first request? (Xose)? The maintainer of the firmware (me)?
someone from Qualcomm Atheros? Or someone else (specific)? (the 
firmware is licensed as GPLv2 - in theory anyone should be able to
do that)

How should the firmware source update be handled? Currently the latest
.tar.xz of the firmware has ~130kb. The formated patches from 1.9.6 to
latest are about ~100kb (182 individual patches).

How does linux-firmware handle new binary firmware images and new 
sources? What if carl9170fw-2.bin is added. Do we need another
source directory for this in the current tree then? Because 
carl9170fw-1.bin will still be needed for backwards compatibility
so we basically need to duplicate parts of the source?

Also, how's the situation with ath9k_htc? The 1.4.0 image contains
some GPLv2 code as well? So, why is there no source in the tree, but 
just the link to it? Because, I would like to do basically the same
for carl9170fw and just add a link to the carl9170fw repository and
save everyone this source update "song and dance".

Regards,
Christian

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 06:47:44 PM Xose Vazquez Perez wrote:
> Christian Lamparter wrote:
> 
> > Sure, but this could be a different patch then. I think Intel devices 
> > (iwlwifi, iwlegacy and ipw2x00) have a similar text about "download
> > firmware from this device from our homepage here" too. So if we want,
> > we can remove them altogether?
> 
> linux-firmware.git does not contain firmware for all drivers. _At least_
> zd1211rw [1], atmel [2] and ipw2x00 [3] are out of the tree.

Yeah, don't forget p54. It needs out-of-tree firmwares as well.

But what about iwlegacy? Isn't that another candidate that would fit both
requirements (no recent firmware update, firmware in linux-firmware.git) ?

> [1] http://sf.net/projects/zd1211/files/
> [2] 
> http://web.archive.org/web/20121016132320/http://at76c503a.berlios.de/fw_dl.html
> [3] http://ipw2100.sf.net/firmware.php http://ipw2200.sf.net/firmware.php

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 07:42:05 PM Kalle Valo wrote:
> Christian Lamparter <chunk...@googlemail.com> writes:
> 
> > On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> >> Lauri Kasanen <c...@gmx.com> writes:
> >> 
> >> > --- a/drivers/net/wireless/ath/carl9170/Kconfig
> >> > +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> >> > @@ -5,12 +5,10 @@ config CARL9170
> >> >  select FW_LOADER
> >> >  select CRC32
> >> >  help
> >> > -  This is another driver for the Atheros "otus" 802.11n USB 
> >> > devices.
> >> > +  This is the mainline driver for the Atheros "otus" 802.11n 
> >> > USB devices.
> >> >  
> >> > -  This driver provides more features than the original,
> >> > -  but it needs a special firmware (carl9170-1.fw) to do that.
> >> > -
> >> > -  The firmware can be downloaded from our wiki here:
> >> > +  It needs a special firmware (carl9170-1.fw), which can be 
> >> > downloaded
> >> > +  from our wiki here:
> >> ><http://wireless.kernel.org/en/users/Drivers/carl9170>
> >> 
> >> Why even mention anything about a "special firmware" as the firmware is
> >> already available from linux-firmware.git? 
> >
> > Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
> > but that failed.
> > <http://comments.gmane.org/gmane.linux.kernel.wireless.general/114639>
> 
> Rick's comment makes sense to me, better just to provide the latest
> version. No need to unnecessary confuse the users. And if someone really
> wants to use an older version that she can retrieve it from the git
> history.
Part of the fun here is that firmware is GPLv2. The linux-firmware.git has
to point to or add the firmware source to their tree. They have added every
single source file to it instead of "packaging" it in a tar.bz2/gz/xz
like you normally do for release sources.

If you want to read more about it:
<http://www.spinics.net/lists/linux-wireless/msg101868.html>

Re: [PATCH v3] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 07:07:39 PM Lauri Kasanen wrote:
> The previous text was confusing, leading readers to think this
> driver was a duplicate, and so didn't need to be enabled.
> 
> After the removal of the older staging driver, this is the only
> driver in mainline for these devices.
> 
> Signed-off-by: Lauri Kasanen 
> ---
> v3: Remove all firmware mentions.
> 
>  drivers/net/wireless/ath/carl9170/Kconfig | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> b/drivers/net/wireless/ath/carl9170/Kconfig
> index 1a796e5..ca1ae09 100644
> --- a/drivers/net/wireless/ath/carl9170/Kconfig
> +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -5,13 +5,7 @@ config CARL9170
>   select FW_LOADER
>   select CRC32
>   help
> -   This is another driver for the Atheros "otus" 802.11n USB devices.
> -
> -   This driver provides more features than the original,
> -   but it needs a special firmware (carl9170-1.fw) to do that.
> -
> -   The firmware can be downloaded from our wiki here:
> -   
Please do keep the wiki link.

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Monday, April 18, 2016 06:45:09 PM Kalle Valo wrote:
> Lauri Kasanen  writes:
> 
> > The previous text was confusing, leading readers to think this
> > driver was a duplicate, and so didn't need to be enabled.
> >
> > After the removal of the older staging driver, this is the only
> > driver in mainline for these devices.
> >
> > Signed-off-by: Lauri Kasanen 
> > ---
> > v2: Remove the mention of the previous driver, suggested by Christian.
> >
> >  drivers/net/wireless/ath/carl9170/Kconfig | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> > b/drivers/net/wireless/ath/carl9170/Kconfig
> > index 1a796e5..2e34bae 100644
> > --- a/drivers/net/wireless/ath/carl9170/Kconfig
> > +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> > @@ -5,12 +5,10 @@ config CARL9170
> > select FW_LOADER
> > select CRC32
> > help
> > - This is another driver for the Atheros "otus" 802.11n USB devices.
> > + This is the mainline driver for the Atheros "otus" 802.11n USB 
> > devices.
> >  
> > - This driver provides more features than the original,
> > - but it needs a special firmware (carl9170-1.fw) to do that.
> > -
> > - The firmware can be downloaded from our wiki here:
> > + It needs a special firmware (carl9170-1.fw), which can be downloaded
> > + from our wiki here:
> >   
> 
> Why even mention anything about a "special firmware" as the firmware is
> already available from linux-firmware.git? 
Yes and no. 1.9.6 is in linux-firmware.git. I've tried to add 1.9.9 too
but that failed.


> That's default location for all firmware images and I think most, if not all,
> distros should have it available. So wouldn't it be better not to mention 
> anything about firmware at all?
Sure, but this could be a different patch then. I think Intel devices 
(iwlwifi, iwlegacy and ipw2x00) have a similar text about "download
firmware from this device from our homepage here" too. So if we want,
we can remove them altogether?

Regards,
Christian

Re: [PATCH v2] carl9170: Clarify kconfig text

[Christian Lamparter]

On Saturday, April 16, 2016 05:18:56 PM Lauri Kasanen wrote:
> The previous text was confusing, leading readers to think this
> driver was a duplicate, and so didn't need to be enabled.
> 
> After the removal of the older staging driver, this is the only
> driver in mainline for these devices.
> 
> Signed-off-by: Lauri Kasanen <c...@gmx.com>
Acked-by: Christian Lamparter <chunk...@googlemail.com>

> ---
> v2: Remove the mention of the previous driver, suggested by Christian.

Thanks!
 
>  drivers/net/wireless/ath/carl9170/Kconfig | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/Kconfig 
> b/drivers/net/wireless/ath/carl9170/Kconfig
> index 1a796e5..2e34bae 100644
> --- a/drivers/net/wireless/ath/carl9170/Kconfig
> +++ b/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -5,12 +5,10 @@ config CARL9170
>   select FW_LOADER
>   select CRC32
>   help
> -   This is another driver for the Atheros "otus" 802.11n USB devices.
> +   This is the mainline driver for the Atheros "otus" 802.11n USB 
> devices.
>  
> -   This driver provides more features than the original,
> -   but it needs a special firmware (carl9170-1.fw) to do that.
> -
> -   The firmware can be downloaded from our wiki here:
> +   It needs a special firmware (carl9170-1.fw), which can be downloaded
> +   from our wiki here:
> <http://wireless.kernel.org/en/users/Drivers/carl9170>
>  
> If you choose to build a module, it'll be called carl9170.
>