Re: [PATCH] net: arc_emac: fix arc_emac_rx() error paths

[Alexander Kochetkov]

> 19 дек. 2017 г., в 18:22, David Miller <da...@davemloft.net> написал(а):
> 
> From: Alexander Kochetkov <al.koc...@gmail.com>
> Date: Fri, 15 Dec 2017 20:20:06 +0300
> 
>> arc_emac_rx() has some issues found by code review.
>> 
>> In case netdev_alloc_skb_ip_align() or dma_map_single() failure
>> rx fifo entry will not be returned to EMAC.
>> 
>> In case dma_map_single() failure previously allocated skb became
>> lost to driver. At the same time address of newly allocated skb
>> will not be provided to EMAC.
>> 
>> Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
> 
> This patch adds quite a few bugs, here is one which shows this is not
> functionally tested:

May be I don’t understand correctly, but I don’t see bug here.

Wrong dma mapping usage should immediately manifest itself (kernel
instability, koops and so on). The patch was tested on rk3188 and work fine for 
me.
Also I did simulations of netdev_alloc_skb_ip_align() and dma_map_single()
faults and can confirm that new error handling work.

Could someone else with ARC EMAC test the patch? I would be very grateful for 
the help.
Florian or Eric, can you test it on your hardware?

>> +
>> +/* unmap previosly mapped skb */
>> +dma_unmap_single(>dev, dma_unmap_addr(rx_buff, addr),
>> + dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
> 
> And then you unmap it.  "addr" is the new DMA mapping, not the old one.

The old mapping should be unmapped here. It refer to old skb what contains 
already
received packet. Because buffer doesn’t belong to EMAC anymore.
That old mapping point to old skb buffer. And netif_receive_skb() will be
called for old skb after that.

The new mapping «addr" will be unmapped then EMAC receive
new packet. Later by the code (after netif_receive_skb()) there are lines for
saving «addr» value:

rx_buff->skb = skb;
dma_unmap_addr_set(rx_buff, addr, addr);
dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE);

Regards,
Alexander.

[PATCH v2] net: arc_emac: fix arc_emac_rx() error paths

[Alexander Kochetkov]

arc_emac_rx() has some issues found by code review.

In case netdev_alloc_skb_ip_align() or dma_map_single() failure
rx fifo entry will not be returned to EMAC.

In case dma_map_single() failure previously allocated skb became
lost to driver. At the same time address of newly allocated skb
will not be provided to EMAC.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
Changes in v2:
- Rebased against stable linux-4.14.y branch

 drivers/net/ethernet/arc/emac_main.c |   53 --
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 363d909..bd277b0 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -212,39 +212,48 @@ static int arc_emac_rx(struct net_device *ndev, int 
budget)
continue;
}
 
-   pktlen = info & LEN_MASK;
-   stats->rx_packets++;
-   stats->rx_bytes += pktlen;
-   skb = rx_buff->skb;
-   skb_put(skb, pktlen);
-   skb->dev = ndev;
-   skb->protocol = eth_type_trans(skb, ndev);
-
-   dma_unmap_single(>dev, dma_unmap_addr(rx_buff, addr),
-dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
-
-   /* Prepare the BD for next cycle */
-   rx_buff->skb = netdev_alloc_skb_ip_align(ndev,
-EMAC_BUFFER_SIZE);
-   if (unlikely(!rx_buff->skb)) {
+   /* Prepare the BD for next cycle. netif_receive_skb()
+* only if new skb was allocated and mapped to avoid holes
+* in the RX fifo.
+*/
+   skb = netdev_alloc_skb_ip_align(ndev, EMAC_BUFFER_SIZE);
+   if (unlikely(!skb)) {
+   if (net_ratelimit())
+   netdev_err(ndev, "cannot allocate skb\n");
+   /* Return ownership to EMAC */
+   rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
-   /* Because receive_skb is below, increment rx_dropped */
stats->rx_dropped++;
continue;
}
 
-   /* receive_skb only if new skb was allocated to avoid holes */
-   netif_receive_skb(skb);
-
-   addr = dma_map_single(>dev, (void *)rx_buff->skb->data,
+   addr = dma_map_single(>dev, (void *)skb->data,
  EMAC_BUFFER_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(>dev, addr)) {
if (net_ratelimit())
-   netdev_err(ndev, "cannot dma map\n");
-   dev_kfree_skb(rx_buff->skb);
+   netdev_err(ndev, "cannot map dma buffer\n");
+   dev_kfree_skb(skb);
+   /* Return ownership to EMAC */
+   rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
+   stats->rx_dropped++;
continue;
}
+
+   /* unmap previosly mapped skb */
+   dma_unmap_single(>dev, dma_unmap_addr(rx_buff, addr),
+dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
+
+   pktlen = info & LEN_MASK;
+   stats->rx_packets++;
+   stats->rx_bytes += pktlen;
+   skb_put(rx_buff->skb, pktlen);
+   rx_buff->skb->dev = ndev;
+   rx_buff->skb->protocol = eth_type_trans(rx_buff->skb, ndev);
+
+   netif_receive_skb(rx_buff->skb);
+
+   rx_buff->skb = skb;
dma_unmap_addr_set(rx_buff, addr, addr);
dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE);
 
-- 
1.7.9.5

[PATCH v2] net: arc_emac: restart stalled EMAC

[Alexander Kochetkov]

Under certain conditions EMAC stop reception of incoming packets and
continuously increment R_MISS register instead of saving data into
provided buffer. The commit implement workaround for such situation.
Then the stall detected EMAC will be restarted.

On device the stall looks like the device lost it's dynamic IP address.
ifconfig shows that interface error counter rapidly increments.
At the same time on the DHCP server we can see continues DHCP-requests
from device.

In real network stalls happen really rarely. To make them frequent the
broadcast storm[1] should be simulated. For simulation it is necessary
to make following connections:
1. connect radxarock to 1st port of switch
2. connect some PC to 2nd port of switch
3. connect two other free ports together using standard ethernet cable,
   in order to make a switching loop.

After that, is necessary to make a broadcast storm. For example, running on
PC 'ping' to some IP address triggers ARP-request storm. After some
time (~10sec), EMAC on rk3188 will stall.

Observed and tested on rk3188 radxarock.

[1] https://en.wikipedia.org/wiki/Broadcast_radiation

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---

Changes in v2:
- Rebased against stable linux-4.14.y branch

 drivers/net/ethernet/arc/emac.h  |2 +
 drivers/net/ethernet/arc/emac_main.c |  111 ++
 2 files changed, 113 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac.h b/drivers/net/ethernet/arc/emac.h
index 3c63b16..d9efbc8 100644
--- a/drivers/net/ethernet/arc/emac.h
+++ b/drivers/net/ethernet/arc/emac.h
@@ -159,6 +159,8 @@ struct arc_emac_priv {
unsigned int link;
unsigned int duplex;
unsigned int speed;
+
+   unsigned int rx_missed_errors;
 };
 
 /**
diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 3241af1..363d909 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -26,6 +26,8 @@
 
 #include "emac.h"
 
+static void arc_emac_restart(struct net_device *ndev);
+
 /**
  * arc_emac_tx_avail - Return the number of available slots in the tx ring.
  * @priv: Pointer to ARC EMAC private data structure.
@@ -259,6 +261,53 @@ static int arc_emac_rx(struct net_device *ndev, int budget)
 }
 
 /**
+ * arc_emac_rx_miss_handle - handle R_MISS register
+ * @ndev:  Pointer to the net_device structure.
+ */
+static void arc_emac_rx_miss_handle(struct net_device *ndev)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   struct net_device_stats *stats = >stats;
+   unsigned int miss;
+
+   miss = arc_reg_get(priv, R_MISS);
+   if (miss) {
+   stats->rx_errors += miss;
+   stats->rx_missed_errors += miss;
+   priv->rx_missed_errors += miss;
+   }
+}
+
+/**
+ * arc_emac_rx_stall_check - check RX stall
+ * @ndev:  Pointer to the net_device structure.
+ * @budget:How many BDs requested to process on 1 call.
+ * @work_done: How many BDs processed
+ *
+ * Under certain conditions EMAC stop reception of incoming packets and
+ * continuously increment R_MISS register instead of saving data into
+ * provided buffer. This function detect that condition and restart
+ * EMAC.
+ */
+static void arc_emac_rx_stall_check(struct net_device *ndev,
+   int budget, unsigned int work_done)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   struct arc_emac_bd *rxbd;
+
+   if (work_done)
+   priv->rx_missed_errors = 0;
+
+   if (priv->rx_missed_errors && budget) {
+   rxbd = >rxbd[priv->last_rx_bd];
+   if (le32_to_cpu(rxbd->info) & FOR_EMAC) {
+   arc_emac_restart(ndev);
+   priv->rx_missed_errors = 0;
+   }
+   }
+}
+
+/**
  * arc_emac_poll - NAPI poll handler.
  * @napi:  Pointer to napi_struct structure.
  * @budget:How many BDs to process on 1 call.
@@ -272,6 +321,7 @@ static int arc_emac_poll(struct napi_struct *napi, int 
budget)
unsigned int work_done;
 
arc_emac_tx_clean(ndev);
+   arc_emac_rx_miss_handle(ndev);
 
work_done = arc_emac_rx(ndev, budget);
if (work_done < budget) {
@@ -279,6 +329,8 @@ static int arc_emac_poll(struct napi_struct *napi, int 
budget)
arc_reg_or(priv, R_ENABLE, RXINT_MASK | TXINT_MASK);
}
 
+   arc_emac_rx_stall_check(ndev, budget, work_done);
+
return work_done;
 }
 
@@ -320,6 +372,8 @@ static irqreturn_t arc_emac_intr(int irq, void 
*dev_instance)
if (status & MSER_MASK) {
stats->rx_missed_errors += 0x100;
stats->rx_errors += 0x100;
+   priv->rx_missed_errors += 0x100;
+   napi_schedule(>napi);
}
 
   

[PATCH] net: arc_emac: fix arc_emac_rx() error paths

[Alexander Kochetkov]

arc_emac_rx() has some issues found by code review.

In case netdev_alloc_skb_ip_align() or dma_map_single() failure
rx fifo entry will not be returned to EMAC.

In case dma_map_single() failure previously allocated skb became
lost to driver. At the same time address of newly allocated skb
will not be provided to EMAC.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/net/ethernet/arc/emac_main.c |   53 --
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index b2e0051..0ea57fe 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -212,39 +212,48 @@ static int arc_emac_rx(struct net_device *ndev, int 
budget)
continue;
}
 
-   pktlen = info & LEN_MASK;
-   stats->rx_packets++;
-   stats->rx_bytes += pktlen;
-   skb = rx_buff->skb;
-   skb_put(skb, pktlen);
-   skb->dev = ndev;
-   skb->protocol = eth_type_trans(skb, ndev);
-
-   dma_unmap_single(>dev, dma_unmap_addr(rx_buff, addr),
-dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
-
-   /* Prepare the BD for next cycle */
-   rx_buff->skb = netdev_alloc_skb_ip_align(ndev,
-EMAC_BUFFER_SIZE);
-   if (unlikely(!rx_buff->skb)) {
+   /* Prepare the BD for next cycle. netif_receive_skb()
+* only if new skb was allocated and mapped to avoid holes
+* in the RX fifo.
+*/
+   skb = netdev_alloc_skb_ip_align(ndev, EMAC_BUFFER_SIZE);
+   if (unlikely(!skb)) {
+   if (net_ratelimit())
+   netdev_err(ndev, "cannot allocate skb\n");
+   /* Return ownership to EMAC */
+   rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
-   /* Because receive_skb is below, increment rx_dropped */
stats->rx_dropped++;
continue;
}
 
-   /* receive_skb only if new skb was allocated to avoid holes */
-   netif_receive_skb(skb);
-
-   addr = dma_map_single(>dev, (void *)rx_buff->skb->data,
+   addr = dma_map_single(>dev, (void *)skb->data,
  EMAC_BUFFER_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(>dev, addr)) {
if (net_ratelimit())
-   netdev_err(ndev, "cannot dma map\n");
-   dev_kfree_skb(rx_buff->skb);
+   netdev_err(ndev, "cannot map dma buffer\n");
+   dev_kfree_skb(skb);
+   /* Return ownership to EMAC */
+   rxbd->info = cpu_to_le32(FOR_EMAC | EMAC_BUFFER_SIZE);
stats->rx_errors++;
+   stats->rx_dropped++;
continue;
}
+
+   /* unmap previosly mapped skb */
+   dma_unmap_single(>dev, dma_unmap_addr(rx_buff, addr),
+dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
+
+   pktlen = info & LEN_MASK;
+   stats->rx_packets++;
+   stats->rx_bytes += pktlen;
+   skb_put(rx_buff->skb, pktlen);
+   rx_buff->skb->dev = ndev;
+   rx_buff->skb->protocol = eth_type_trans(rx_buff->skb, ndev);
+
+   netif_receive_skb(rx_buff->skb);
+
+   rx_buff->skb = skb;
dma_unmap_addr_set(rx_buff, addr, addr);
dma_unmap_len_set(rx_buff, len, EMAC_BUFFER_SIZE);
 
-- 
1.7.9.5

[PATCH] net: arc_emac: restart stalled EMAC

[Alexander Kochetkov]

Under certain conditions EMAC stop reception of incoming packets and
continuously increment R_MISS register instead of saving data into
provided buffer. The commit implement workaround for such situation.
Then the stall detected EMAC will be restarted.

On device the stall looks like the device lost it's dynamic IP address.
ifconfig shows that interface error counter rapidly increments.
At the same time on the DHCP server we can see continues DHCP-requests
from device.

In real network stalls happen really rarely. To make them frequent the
broadcast storm[1] should be simulated. For simulation it is necessary
to make following connections:
1. connect radxarock to 1st port of switch
2. connect some PC to 2nd port of switch
3. connect two other free ports together using standard ethernet cable,
   in order to make a switching loop.

After that, is necessary to make a broadcast storm. For example, running on
PC 'ping' to some IP address triggers ARP-request storm. After some
time (~10sec), EMAC on rk3188 will stall.

Observed and tested on rk3188 radxarock.

[1] https://en.wikipedia.org/wiki/Broadcast_radiation

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/net/ethernet/arc/emac.h  |2 +
 drivers/net/ethernet/arc/emac_main.c |  111 ++
 2 files changed, 113 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac.h b/drivers/net/ethernet/arc/emac.h
index e4feb71..1c7afc5 100644
--- a/drivers/net/ethernet/arc/emac.h
+++ b/drivers/net/ethernet/arc/emac.h
@@ -158,6 +158,8 @@ struct arc_emac_priv {
unsigned int link;
unsigned int duplex;
unsigned int speed;
+
+   unsigned int rx_missed_errors;
 };
 
 /**
diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 68de2f2..b2e0051 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -26,6 +26,8 @@
 
 #include "emac.h"
 
+static void arc_emac_restart(struct net_device *ndev);
+
 /**
  * arc_emac_tx_avail - Return the number of available slots in the tx ring.
  * @priv: Pointer to ARC EMAC private data structure.
@@ -259,6 +261,53 @@ static int arc_emac_rx(struct net_device *ndev, int budget)
 }
 
 /**
+ * arc_emac_rx_miss_handle - handle R_MISS register
+ * @ndev:  Pointer to the net_device structure.
+ */
+static void arc_emac_rx_miss_handle(struct net_device *ndev)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   struct net_device_stats *stats = >stats;
+   unsigned int miss;
+
+   miss = arc_reg_get(priv, R_MISS);
+   if (miss) {
+   stats->rx_errors += miss;
+   stats->rx_missed_errors += miss;
+   priv->rx_missed_errors += miss;
+   }
+}
+
+/**
+ * arc_emac_rx_stall_check - check RX stall
+ * @ndev:  Pointer to the net_device structure.
+ * @budget:How many BDs requested to process on 1 call.
+ * @work_done: How many BDs processed
+ *
+ * Under certain conditions EMAC stop reception of incoming packets and
+ * continuously increment R_MISS register instead of saving data into
+ * provided buffer. This function detect that condition and restart
+ * EMAC.
+ */
+static void arc_emac_rx_stall_check(struct net_device *ndev,
+   int budget, unsigned int work_done)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   struct arc_emac_bd *rxbd;
+
+   if (work_done)
+   priv->rx_missed_errors = 0;
+
+   if (priv->rx_missed_errors && budget) {
+   rxbd = >rxbd[priv->last_rx_bd];
+   if (le32_to_cpu(rxbd->info) & FOR_EMAC) {
+   arc_emac_restart(ndev);
+   priv->rx_missed_errors = 0;
+   }
+   }
+}
+
+/**
  * arc_emac_poll - NAPI poll handler.
  * @napi:  Pointer to napi_struct structure.
  * @budget:How many BDs to process on 1 call.
@@ -272,6 +321,7 @@ static int arc_emac_poll(struct napi_struct *napi, int 
budget)
unsigned int work_done;
 
arc_emac_tx_clean(ndev);
+   arc_emac_rx_miss_handle(ndev);
 
work_done = arc_emac_rx(ndev, budget);
if (work_done < budget) {
@@ -279,6 +329,8 @@ static int arc_emac_poll(struct napi_struct *napi, int 
budget)
arc_reg_or(priv, R_ENABLE, RXINT_MASK | TXINT_MASK);
}
 
+   arc_emac_rx_stall_check(ndev, budget, work_done);
+
return work_done;
 }
 
@@ -320,6 +372,8 @@ static irqreturn_t arc_emac_intr(int irq, void 
*dev_instance)
if (status & MSER_MASK) {
stats->rx_missed_errors += 0x100;
stats->rx_errors += 0x100;
+   priv->rx_missed_errors += 0x100;
+   napi_schedule(>napi);
}
 
if (status & RXCR_MASK) {
@@ -720,6 +774,63 @@ st

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hello David!

> 25 апр. 2017 г., в 17:36, David Miller  написал(а):
> 
> So... what are we doing here?
> 
> My understanding is that this should fix the same problem that commit
> 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> negotiation on startup") fixed and that this micrel commit should thus
> be reverted to improve MAC startup times which regressed.
> 
> Florian, any guidance?

Yes, this should be done.

I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: 
micrel: Disable auto
negotiation on startup») can be reverted, and he answered what it may do that
sometime this/next week.

Alexander.

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

> 21 апр. 2017 г., в 17:18, Roger Quadros  написал(а):
> 
> I think the following commit broke functionality with interrupt driven PHYs
> 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not 
> polling.")

Probably this one[1] broke, according to Alexandre’s commit[2].
And it was since Nov 16 2015. But it was hidden by some other commits.

For Roger problem became visible after 3c293f4e08b5 ("net: phy:
Trigger state machine on state change and not polling.»),

For my problem became visible after 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause»). As commit 529ed1275263 
removed SUPPORTED_Pause flag from PHY advertising property and
genphy_config_aneg() began to skip PHY auto-negotiation.

Alexander.

[1] Fixes: 321beec5047a (net: phy: Use interrupts when available in NOLINK 
state)
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=321beec5047af83db90c88114b7e664b156f49fe
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hi, Alexandre!

Just found that you've fixed similar problem for Micrel PHY:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66

Could you please test if my patch[1] fix your problem?

As reverting your patch will speedup MAC startup time for boards with Micrel PHY
on 3~5 sec (auto-negotiation time[2]). Could you check that also?

Regards,
Alexander.

[1] https://lkml.org/lkml/2017/4/20/357
[2] http://www.ieee802.org/3/af/public/jan02/brown_1_0102.pdf


> 20 апр. 2017 г., в 14:00, Alexander Kochetkov <al.koc...@gmail.com> 
> написал(а):
> 
> The Ethernet link on an interrupt driven PHY was not coming up if the Ethernet
> cable was plugged before the Ethernet interface was brought up.
> 
> The patch trigger PHY state machine to update link state if PHY was requested 
> to
> do auto-negotiation and auto-negotiation complete flag already set.
> 
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no 
> new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
> 
> Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
> Cc: stable <sta...@vger.kernel.org> # v4.9+
> ---
> drivers/net/phy/phy.c |   40 
> include/linux/phy.h   |1 +
> 2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..2d9975b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct 
> ifreq *ifr, int cmd)
> EXPORT_SYMBOL(phy_mii_ioctl);
> 
> /**
> - * phy_start_aneg - start auto-negotiation for this PHY device
> + * phy_start_aneg_priv - start auto-negotiation for this PHY device
>  * @phydev: the phy_device struct
> + * @sync: indicate whether we should wait for the workqueue cancelation
>  *
>  * Description: Sanitizes the settings (if we're not autonegotiating
>  *   them), and then calls the driver's config_aneg function.
>  *   If the PHYCONTROL Layer is operating, we change the state to
>  *   reflect the beginning of Auto-negotiation or forcing.
>  */
> -int phy_start_aneg(struct phy_device *phydev)
> +static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
> {
> + bool trigger = 0;
>   int err;
> 
>   mutex_lock(>lock);
> @@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
>   }
>   }
> 
> + /* Re-schedule a PHY state machine to check PHY status because
> +  * negotiation may already be done and aneg interrupt may not be
> +  * generated.
> +  */
> + if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
> + err = phy_aneg_done(phydev);
> + if (err > 0) {
> + trigger = true;
> + err = 0;
> + }
> + }
> +
> out_unlock:
>   mutex_unlock(>lock);
> +
> + if (trigger)
> + phy_trigger_machine(phydev, sync);
> +
>   return err;
> }
> +
> +/**
> + * phy_start_aneg - start auto-negotiation for this PHY device
> + * @phydev: the phy_device struct
> + *
> + * Description: Sanitizes the settings (if we're not autonegotiating
> + *   them), and then calls the driver's config_aneg function.
> + *   If the PHYCONTROL Layer is operating, we change the state to
> + *   reflect the beginning of Auto-negotiation or forcing.
> + */
> +int phy_start_aneg(struct phy_device *phydev)
> +{
> + return phy_start_aneg_priv(phydev, true);
> +}
> EXPORT_SYMBOL(phy_start_aneg);
> 
> /**
> @@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
>  *   state machine runs.
>  */
> 
> -static void phy_trigger_machine(struct phy_device *phydev, bool sync)
> +void phy_trigger_machine(struct phy_device *phydev, bool sync)
> {
>   if (sync)
>   cancel_delayed_work_sync(>state_queue);
> @@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
>   mutex_unlock(>lock);
> 
>   if (needs_aneg)
> - err = phy_start_aneg(phydev);
> + err = phy_start_aneg_priv(phydev, fal

[PATCH v2] net: arc_emac: switch to phy_start()/phy_stop()

[Alexander Kochetkov]

Currently driver use phy_start_aneg() in arc_emac_open() to bring
up PHY. But phy_start() function is more appropriate for this purposes.
Besides that it call phy_start_aneg() as part of PHY startup sequence
it also can correctly bring up PHY from error and suspended states.
So the patch replace phy_start_aneg() to phy_start().

Also the patch add call to phy_stop() to arc_emac_stop() to allow
the PHY device to be fully suspended when the interface is unused.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---

Changes in v2:
- Updated commit message to clarify changes

 drivers/net/ethernet/arc/emac_main.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index abc9f2a..188676d 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev)
/* Enable EMAC */
arc_reg_or(priv, R_CTRL, EN_MASK);
 
-   phy_start_aneg(ndev->phydev);
+   phy_start(ndev->phydev);
 
netif_start_queue(ndev);
 
@@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev)
napi_disable(>napi);
netif_stop_queue(ndev);
 
+   phy_stop(ndev->phydev);
+
/* Disable interrupts */
arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK);
 
-- 
1.7.9.5

[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

The Ethernet link on an interrupt driven PHY was not coming up if the Ethernet
cable was plugged before the Ethernet interface was brought up.

The patch trigger PHY state machine to update link state if PHY was requested to
do auto-negotiation and auto-negotiation complete flag already set.

During power-up cycle the PHY do auto-negotiation, generate interrupt and set
auto-negotiation complete flag. Interrupt is handled by PHY state machine but
doesn't update link state because PHY is in PHY_READY state. After some time
MAC bring up, start and request PHY to do auto-negotiation. If there are no new
settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
PHY continue to stay in auto-negotiation complete state and doesn't fire
interrupt. At the same time PHY state machine expect that PHY started
auto-negotiation and is waiting for interrupt from PHY and it won't get it.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
Cc: stable <sta...@vger.kernel.org> # v4.9+
---
 drivers/net/phy/phy.c |   40 
 include/linux/phy.h   |1 +
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7cc1b7d..2d9975b 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq 
*ifr, int cmd)
 EXPORT_SYMBOL(phy_mii_ioctl);
 
 /**
- * phy_start_aneg - start auto-negotiation for this PHY device
+ * phy_start_aneg_priv - start auto-negotiation for this PHY device
  * @phydev: the phy_device struct
+ * @sync: indicate whether we should wait for the workqueue cancelation
  *
  * Description: Sanitizes the settings (if we're not autonegotiating
  *   them), and then calls the driver's config_aneg function.
  *   If the PHYCONTROL Layer is operating, we change the state to
  *   reflect the beginning of Auto-negotiation or forcing.
  */
-int phy_start_aneg(struct phy_device *phydev)
+static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
 {
+   bool trigger = 0;
int err;
 
mutex_lock(>lock);
@@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
}
}
 
+   /* Re-schedule a PHY state machine to check PHY status because
+* negotiation may already be done and aneg interrupt may not be
+* generated.
+*/
+   if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
+   err = phy_aneg_done(phydev);
+   if (err > 0) {
+   trigger = true;
+   err = 0;
+   }
+   }
+
 out_unlock:
mutex_unlock(>lock);
+
+   if (trigger)
+   phy_trigger_machine(phydev, sync);
+
return err;
 }
+
+/**
+ * phy_start_aneg - start auto-negotiation for this PHY device
+ * @phydev: the phy_device struct
+ *
+ * Description: Sanitizes the settings (if we're not autonegotiating
+ *   them), and then calls the driver's config_aneg function.
+ *   If the PHYCONTROL Layer is operating, we change the state to
+ *   reflect the beginning of Auto-negotiation or forcing.
+ */
+int phy_start_aneg(struct phy_device *phydev)
+{
+   return phy_start_aneg_priv(phydev, true);
+}
 EXPORT_SYMBOL(phy_start_aneg);
 
 /**
@@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
  *   state machine runs.
  */
 
-static void phy_trigger_machine(struct phy_device *phydev, bool sync)
+void phy_trigger_machine(struct phy_device *phydev, bool sync)
 {
if (sync)
cancel_delayed_work_sync(>state_queue);
@@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
mutex_unlock(>lock);
 
if (needs_aneg)
-   err = phy_start_aneg(phydev);
+   err = phy_start_aneg_priv(phydev, false);
else if (do_suspend)
phy_suspend(phydev);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7fc1105..b19ae66 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -840,6 +840,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int 
n,
 void phy_mac_interrupt(struct phy_device *phydev, int new_link);
 void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
+void phy_trigger_machine(struct phy_device *phydev, bool sync);
 int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_ethtool_ksettings_get(struct phy_device *phydev,
-- 
1.7.9.5

[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hello Florian, Roger and Madalin!

This is slightly modified version of previous patch[1].

It take into account that phy_start_aneg() may be called from
differend places not only from PHY state machine. So the patch
use phy_trigger_machine() to schedule link state update
correctly.

I borrow some text from Roger's commit message.

Added 'Cc: stable <sta...@vger.kernel.org> tag'
because the problem exist on 4.9 and 4.10 branches.

Don't know what commit brake the code, so I don't add
Fixes tag.

Roger, could you please test this one patch?

Madalin, I found, that you tested Roger’s patch[2] and this
one patch will get upstream instead of patch[2]. Could you
please test it?

Regards,
Alexander.

[1] http://patchwork.ozlabs.org/patch/743773/
[2] https://lkml.org/lkml/2017/3/30/517

Alexander Kochetkov (1):
  net: phy: fix auto-negotiation stall due to unavailable interrupt

 drivers/net/phy/phy.c |   40 
 include/linux/phy.h   |1 +
 2 files changed, 37 insertions(+), 4 deletions(-)

-- 
1.7.9.5

Re: [PATCH] net: arc_emac: switch to phy_start()/phy_stop()

[Alexander Kochetkov]

> 20 апр. 2017 г., в 0:08, Florian Fainelli <f.faine...@gmail.com> написал(а):
> 
> This looks fine. If you wanted to go further, you could move the
> phy_connect(), phy_disconnect() calls down to the arc_emac_open()
> respectively arc_emac_stop() as this would also allow the PHY device to
> be fully suspended when the interface is unused.


I’ve checked patch phy_connect() is called from arc_emac_open() and
phy_disconnect() is called from arc_emac_stop().

So, I’ve made mistake in the commit message.

Thank you for review.

> 
> 19 апр. 2017 г., в 21:22, Sergei Shtylyov 
> <sergei.shtyl...@cogentembedded.com> написал(а):
> 
> On 04/19/2017 05:29 PM, Alexander Kochetkov wrote:
> 
>> The patch replace phy_start_aneg() with phy_start(). phy_start() call
> 
>   Replaces.
> 
>> phy_start_aneg() as a part of startup sequence and allow recover from
>> error (PHY_HALTED) state.
>> 
>> Also added call phy_stop() to arc_emac_remove() to stop PHY state machine
> 
>   To arc_emac_stop() maybe?
> 

Sergei, thanks for spell and gramma checking.

Regards,
Alexander.

Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

> 19 апр. 2017 г., в 19:32, Florian Fainelli  написал(а):
> 
> http://patchwork.ozlabs.org/patch/743773/
> 
> Roger can you also test Alexander's patch?

If MAC use phy_start_aneg() instead of phy_start() my patch will not work as
expected. Roger, if patch don’t work for you please check what MAC bring up PHY 
using
phy_start():

http://patchwork.ozlabs.org/patch/752308/

Is it correct to start PHY inside MAC probe using phy_start_aneg()? Or 
phy_start() must be used?

And probably this tags should be added for my patch:
Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not 
polling.")
Cc: stable  # v4.9+

Because I bisected to commit 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause») that looks pretty good.

Also, there is another issue I found. link_timeout doesn’t work for interrupt 
driven PHY.
It is possible to implement timer to handle this case.
Florian, what do you think? Should this be fixed?

Alexander.

[PATCH] net: arc_emac: switch to phy_start()/phy_stop()

[Alexander Kochetkov]

The patch replace phy_start_aneg() with phy_start(). phy_start() call
phy_start_aneg() as a part of startup sequence and allow recover from
error (PHY_HALTED) state.

Also added call phy_stop() to arc_emac_remove() to stop PHY state machine
when MAC is down.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/net/ethernet/arc/emac_main.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index abc9f2a..188676d 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev)
/* Enable EMAC */
arc_reg_or(priv, R_CTRL, EN_MASK);
 
-   phy_start_aneg(ndev->phydev);
+   phy_start(ndev->phydev);
 
netif_start_queue(ndev);
 
@@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev)
napi_disable(>napi);
netif_stop_queue(ndev);
 
+   phy_stop(ndev->phydev);
+
/* Disable interrupts */
arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK);
 
-- 
1.7.9.5

Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Just found similar problem fixed in another PHY. See commit 99f81afc139c
("phy: micrel: Disable auto negotiation on startup»)

> 19 апр. 2017 г., в 16:46, Alexander Kochetkov <al.koc...@gmail.com> 
> написал(а):
> 
> The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
> interrupts. During power-up cycle the PHY do auto-negotiation, generate
> interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
> state machine but doesn't update link because PHY is in PHY_READY state.
> After some time MAC bring up and connect with PHY. It start PHY using
> phy_start(). During startup PHY change state to PHY_AN but doesn't
> set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
> because there no new to advertising. As a result, state machine wait for
> interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
> already set the patch schedule check link without waiting interrupt.
> In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
> flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
> on auto-negotiation interrupt.
> 
> Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
> ---
> drivers/net/phy/phy.c |   12 
> 1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..da8f03d 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
>   if (phydev->irq == PHY_POLL)
>   queue_delayed_work(system_power_efficient_wq, 
> >state_queue,
>  PHY_STATE_TIME * HZ);
> +
> + /* Re-schedule a PHY state machine to check PHY status because
> +  * negotiation already done and aneg interrupt may not be generated.
> +  */
> + if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
> + err = phy_aneg_done(phydev);
> + if (err > 0)
> + queue_delayed_work(system_power_efficient_wq,
> +>state_queue, 0);
> + if (err < 0)
> + phy_error(phydev);
> + }
> }
> 
> /**
> -- 
> 1.7.9.5
> 

[PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
interrupts. During power-up cycle the PHY do auto-negotiation, generate
interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
state machine but doesn't update link because PHY is in PHY_READY state.
After some time MAC bring up and connect with PHY. It start PHY using
phy_start(). During startup PHY change state to PHY_AN but doesn't
set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
because there no new to advertising. As a result, state machine wait for
interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
already set the patch schedule check link without waiting interrupt.
In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
on auto-negotiation interrupt.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
 drivers/net/phy/phy.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7cc1b7d..da8f03d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
if (phydev->irq == PHY_POLL)
queue_delayed_work(system_power_efficient_wq, 
>state_queue,
   PHY_STATE_TIME * HZ);
+
+   /* Re-schedule a PHY state machine to check PHY status because
+* negotiation already done and aneg interrupt may not be generated.
+*/
+   if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
+   err = phy_aneg_done(phydev);
+   if (err > 0)
+   queue_delayed_work(system_power_efficient_wq,
+  >state_queue, 0);
+   if (err < 0)
+   phy_error(phydev);
+   }
 }
 
 /**
-- 
1.7.9.5

[PATCHv2 2/2] net: arc_emac: don't pass multicast packets to kernel in non-multicast mode

[Alexander Kochetkov]

The patch disable capturing multicast packets when multicast mode
disabled for ethernet ('ifconfig eth0 -multicast'). In that case
no multicast packet will be passed to kernel.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
Changes in v2:
Add Signed-off-by text.
Removed deleted line from patch.

 drivers/net/ethernet/arc/emac_main.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 2e4ee86..be865b4 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -460,7 +460,7 @@ static void arc_emac_set_rx_mode(struct net_device *ndev)
if (ndev->flags & IFF_ALLMULTI) {
arc_reg_set(priv, R_LAFL, ~0);
arc_reg_set(priv, R_LAFH, ~0);
-   } else {
+   } else if (ndev->flags & IFF_MULTICAST) {
struct netdev_hw_addr *ha;
unsigned int filter[2] = { 0, 0 };
int bit;
@@ -472,6 +472,9 @@ static void arc_emac_set_rx_mode(struct net_device *ndev)
 
arc_reg_set(priv, R_LAFL, filter[0]);
arc_reg_set(priv, R_LAFH, filter[1]);
+   } else {
+   arc_reg_set(priv, R_LAFL, 0);
+   arc_reg_set(priv, R_LAFH, 0);
}
}
 }
-- 
1.7.9.5

[PATCHv2 1/2] net: arc_emac: annonce IFF_MULTICAST support

[Alexander Kochetkov]

Multicast support was implemented by commit 775dd682e2b0ec7
('arc_emac: implement promiscuous mode and multicast filtering').

It can be enabled explicity using 'ifconfig eth0 multicast'.
The patch is needed in order to remove explicit configuration
as most devices has multicast mode enabled by default.

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>
---
Changes in v2:
Add Signed-off-by text.

 drivers/net/ethernet/arc/emac_main.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index b0da969..2e4ee86 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -764,8 +764,6 @@ int arc_emac_probe(struct net_device *ndev, int interface)
ndev->netdev_ops = _emac_netdev_ops;
ndev->ethtool_ops = _emac_ethtool_ops;
ndev->watchdog_timeo = TX_TIMEOUT;
-   /* FIXME :: no multicast support yet */
-   ndev->flags &= ~IFF_MULTICAST;
 
priv = netdev_priv(ndev);
priv->dev = dev;
-- 
1.7.9.5

[PATCH 1/2] net: arc_emac: annonce IFF_MULTICAST support

[Alexander Kochetkov]

Multicast support was implemented by commit 775dd682e2b0ec7
('arc_emac: implement promiscuous mode and multicast filtering').

It can be enabled explicity using 'ifconfig eth0 multicast'.
The patch is needed in order to remove explicit configuration
as most devices has multicast mode enabled by default.
---
 drivers/net/ethernet/arc/emac_main.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index b0da969..2e4ee86 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -764,8 +764,6 @@ int arc_emac_probe(struct net_device *ndev, int interface)
ndev->netdev_ops = _emac_netdev_ops;
ndev->ethtool_ops = _emac_ethtool_ops;
ndev->watchdog_timeo = TX_TIMEOUT;
-   /* FIXME :: no multicast support yet */
-   ndev->flags &= ~IFF_MULTICAST;
 
priv = netdev_priv(ndev);
priv->dev = dev;
-- 
1.7.9.5

[PATCH 2/2] net: arc_emac: don't pass multicast packets to kernel in non-multicast mode

[Alexander Kochetkov]

The patch disable capturing multicast packets when multicast mode
disabled for ethernet ('ifconfig eth0 -multicast'). In that case
no multicast packet will be passed to kernel.
---
 drivers/net/ethernet/arc/emac_main.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 2e4ee86..30ad833 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -460,7 +460,7 @@ static void arc_emac_set_rx_mode(struct net_device *ndev)
if (ndev->flags & IFF_ALLMULTI) {
arc_reg_set(priv, R_LAFL, ~0);
arc_reg_set(priv, R_LAFH, ~0);
-   } else {
+   } else if (ndev->flags & IFF_MULTICAST) {
struct netdev_hw_addr *ha;
unsigned int filter[2] = { 0, 0 };
int bit;
@@ -469,9 +469,11 @@ static void arc_emac_set_rx_mode(struct net_device *ndev)
bit = ether_crc_le(ETH_ALEN, ha->addr) >> 26;
filter[bit >> 5] |= 1 << (bit & 31);
}
-
arc_reg_set(priv, R_LAFL, filter[0]);
arc_reg_set(priv, R_LAFH, filter[1]);
+   } else {
+   arc_reg_set(priv, R_LAFL, 0);
+   arc_reg_set(priv, R_LAFH, 0);
}
}
 }
-- 
1.7.9.5

[PATCH 2/3] net: arc_emac: reset txbd_curr and txbd_dirty pointers to zero

[Alexander Kochetkov]

EMAC reset internal tx ring pointer to zero at statup.
txbd_curr and txbd_dirty can be different from zero.
That cause ethernet transfer hang (no packets transmitted).

In order to reproduce, run on device:
ifconfig eth0 down
ifconfig eth0 up

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>

---
CC: sta...@vger.kernel.org # 3.18.x-
---
 drivers/net/ethernet/arc/emac_main.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 63a63e3..4f6e5be 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -447,6 +447,9 @@ static int arc_emac_open(struct net_device *ndev)
*last_rx_bd = (*last_rx_bd + 1) % RX_BD_NUM;
}
 
+   priv->txbd_curr = 0;
+   priv->txbd_dirty = 0;
+
/* Clean Tx BD's */
memset(priv->txbd, 0, TX_RING_SZ);
 
-- 
1.7.9.5

[PATCH 1/3] net: arc_emac: fix koops caused by sk_buff free

[Alexander Kochetkov]

 c0027568
[   34.522176] df40: ef027340 c003ef48 ef027300  ef027340 c003edd4 
 
[   34.530367] df60:  c003c37c ff7f 0001  ef027340 
 00030003
[   34.538559] df80: ef05df80 ef05df80   ef05df90 ef05df90 
ef05dfac ef027300
[   34.546750] dfa0: c003c2a4   c000f578   
 
[   34.554939] dfc0:       
 
[   34.563129] dfe0:     0013  
 dfff7fff
[   34.571360] [] (__dev_kfree_skb_irq) from [] 
(arc_emac_poll+0x130/0x474)
[   34.579840] [] (arc_emac_poll) from [] 
(net_rx_action+0xdc/0x28c)
[   34.587712] [] (net_rx_action) from [] 
(__do_softirq+0xcc/0x1f8)
[   34.595482] [] (__do_softirq) from [] 
(run_ksoftirqd+0x2c/0x50)
[   34.603168] [] (run_ksoftirqd) from [] 
(smpboot_thread_fn+0x174/0x18c)
[   34.611466] [] (smpboot_thread_fn) from [] 
(kthread+0xd8/0xec)
[   34.619075] [] (kthread) from [] 
(ret_from_fork+0x14/0x3c)
[   34.626317] Code: e8bd8010 e3a0 e12fff1e e92d4010 (e59030a4)
[   34.632572] ---[ end trace cca5a3d86a82249a ]---

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>

---
CC: sta...@vger.kernel.org # 3.18.x-
---
 drivers/net/ethernet/arc/emac_main.c |9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index abe1eab..63a63e3 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -163,7 +163,7 @@ static void arc_emac_tx_clean(struct net_device *ndev)
struct sk_buff *skb = tx_buff->skb;
unsigned int info = le32_to_cpu(txbd->info);
 
-   if ((info & FOR_EMAC) || !txbd->data)
+   if ((info & FOR_EMAC) || !txbd->data || !skb)
break;
 
if (unlikely(info & (DROP | DEFR | LTCL | UFLO))) {
@@ -191,6 +191,7 @@ static void arc_emac_tx_clean(struct net_device *ndev)
 
txbd->data = 0;
txbd->info = 0;
+   tx_buff->skb = NULL;
 
*txbd_dirty = (*txbd_dirty + 1) % TX_BD_NUM;
}
@@ -610,7 +611,6 @@ static int arc_emac_tx(struct sk_buff *skb, struct 
net_device *ndev)
dma_unmap_addr_set(>tx_buff[*txbd_curr], addr, addr);
dma_unmap_len_set(>tx_buff[*txbd_curr], len, len);
 
-   priv->tx_buff[*txbd_curr].skb = skb;
priv->txbd[*txbd_curr].data = cpu_to_le32(addr);
 
/* Make sure pointer to data buffer is set */
@@ -620,6 +620,11 @@ static int arc_emac_tx(struct sk_buff *skb, struct 
net_device *ndev)
 
*info = cpu_to_le32(FOR_EMAC | FIRST_OR_LAST_MASK | len);
 
+   /* Make sure info word is set */
+   wmb();
+
+   priv->tx_buff[*txbd_curr].skb = skb;
+
/* Increment index to point to the next BD */
*txbd_curr = (*txbd_curr + 1) % TX_BD_NUM;
 
-- 
1.7.9.5

[PATCH 3/3] net: arc_emac: fix sk_buff leak

[Alexander Kochetkov]

EMAC could be disabled, while there is some sb_buff
in use. That buffers got lost for linux.

In order to reproduce run on device during active ethernet work:
ifconfig eth0 down

Signed-off-by: Alexander Kochetkov <al.koc...@gmail.com>

---
CC: sta...@vger.kernel.org # 3.18.x-
---
 drivers/net/ethernet/arc/emac_main.c |   62 ++
 1 file changed, 62 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 4f6e5be..6446af1 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -518,6 +518,64 @@ static void arc_emac_set_rx_mode(struct net_device *ndev)
 }
 
 /**
+ * arc_free_tx_queue - free skb from tx queue
+ * @ndev:  Pointer to the network device.
+ *
+ * This function must be called while EMAC disable
+ */
+static void arc_free_tx_queue(struct net_device *ndev)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   unsigned int i;
+
+   for (i = 0; i < TX_BD_NUM; i++) {
+   struct arc_emac_bd *txbd = >txbd[i];
+   struct buffer_state *tx_buff = >tx_buff[i];
+
+   if (tx_buff->skb) {
+   dma_unmap_single(>dev, dma_unmap_addr(tx_buff, 
addr),
+dma_unmap_len(tx_buff, len), 
DMA_TO_DEVICE);
+
+   /* return the sk_buff to system */
+   dev_kfree_skb_irq(tx_buff->skb);
+   }
+
+   txbd->info = 0;
+   txbd->data = 0;
+   tx_buff->skb = NULL;
+   }
+}
+
+/**
+ * arc_free_rx_queue - free skb from rx queue
+ * @ndev:  Pointer to the network device.
+ *
+ * This function must be called while EMAC disable
+ */
+static void arc_free_rx_queue(struct net_device *ndev)
+{
+   struct arc_emac_priv *priv = netdev_priv(ndev);
+   unsigned int i;
+
+   for (i = 0; i < RX_BD_NUM; i++) {
+   struct arc_emac_bd *rxbd = >rxbd[i];
+   struct buffer_state *rx_buff = >rx_buff[i];
+
+   if (rx_buff->skb) {
+   dma_unmap_single(>dev, dma_unmap_addr(rx_buff, 
addr),
+   dma_unmap_len(rx_buff, len), 
DMA_FROM_DEVICE);
+
+   /* return the sk_buff to system */
+   dev_kfree_skb_irq(rx_buff->skb);
+   }
+
+   rxbd->info = 0;
+   rxbd->data = 0;
+   rx_buff->skb = NULL;
+   }
+}
+
+/**
  * arc_emac_stop - Close the network device.
  * @ndev:  Pointer to the network device.
  *
@@ -538,6 +596,10 @@ static int arc_emac_stop(struct net_device *ndev)
/* Disable EMAC */
arc_reg_clr(priv, R_CTRL, EN_MASK);
 
+   /* Return the sk_buff to system */
+   arc_free_tx_queue(ndev);
+   arc_free_rx_queue(ndev);
+
return 0;
 }
 
-- 
1.7.9.5

Fixes for rockchip EMAC

[Alexander Kochetkov]

Hello!

Here is a set of 3 patches what fix koops, memory leak and
rockchip EMAC hang. Tested on radxarock lite.

[PATCH 1/3] net: arc_emac: fix koops caused by sk_buff free
[PATCH 2/3] net: arc_emac: reset txbd_curr and txbd_dirty pointers
[PATCH 3/3] net: arc_emac: fix sk_buff leak