All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net: stmmac: only enable DMA interrupts when ready
@ 2022-02-24 11:38 ` Vincent Whitchurch
  0 siblings, 0 replies; 8+ messages in thread
From: Vincent Whitchurch @ 2022-02-24 11:38 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Coquelin
  Cc: kernel, Vincent Whitchurch, Lars Persson, Srinivas Kandagatla,
	netdev, linux-stm32, linux-arm-kernel, linux-kernel

In this driver's ->ndo_open() callback, it enables DMA interrupts,
starts the DMA channels, then requests interrupts with request_irq(),
and then finally enables napi.

If RX DMA interrupts are received before napi is enabled, no processing
is done because napi_schedule_prep() will return false.  If the network
has a lot of broadcast/multicast traffic, then the RX ring could fill up
completely before napi is enabled.  When this happens, no further RX
interrupts will be delivered, and the driver will fail to receive any
packets.

Fix this by only enabling DMA interrupts after all other initialization
is complete.

Fixes: 523f11b5d4fd72efb ("net: stmmac: move hardware setup for stmmac_open to new function")
Reported-by: Lars Persson <larper@axis.com>
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6708ca2aa4f7..43978558d6c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
 	stmmac_stop_tx(priv, priv->ioaddr, chan);
 }
 
+static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
+{
+	u32 rx_channels_count = priv->plat->rx_queues_to_use;
+	u32 tx_channels_count = priv->plat->tx_queues_to_use;
+	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
+	u32 chan;
+
+	for (chan = 0; chan < dma_csr_ch; chan++) {
+		struct stmmac_channel *ch = &priv->channel[chan];
+		unsigned long flags;
+
+		spin_lock_irqsave(&ch->lock, flags);
+		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+		spin_unlock_irqrestore(&ch->lock, flags);
+	}
+}
+
 /**
  * stmmac_start_all_dma - start all RX and TX DMA channels
  * @priv: driver private structure
@@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
 
 	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++)
+	for (chan = 0; chan < dma_csr_ch; chan++) {
 		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
+		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+	}
 
 	/* DMA RX Channel Configuration */
 	for (chan = 0; chan < rx_channels_count; chan++) {
@@ -3759,6 +3778,7 @@ static int stmmac_open(struct net_device *dev)
 
 	stmmac_enable_all_queues(priv);
 	netif_tx_start_all_queues(priv->dev);
+	stmmac_enable_all_dma_irq(priv);
 
 	return 0;
 
@@ -6508,8 +6528,10 @@ int stmmac_xdp_open(struct net_device *dev)
 	}
 
 	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++)
+	for (chan = 0; chan < dma_csr_ch; chan++) {
 		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
+		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+	}
 
 	/* Adjust Split header */
 	sph_en = (priv->hw->rx_csum > 0) && priv->sph;
@@ -6570,6 +6592,7 @@ int stmmac_xdp_open(struct net_device *dev)
 	stmmac_enable_all_queues(priv);
 	netif_carrier_on(dev);
 	netif_tx_start_all_queues(dev);
+	stmmac_enable_all_dma_irq(priv);
 
 	return 0;
 
@@ -7447,6 +7470,7 @@ int stmmac_resume(struct device *dev)
 	stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw);
 
 	stmmac_enable_all_queues(priv);
+	stmmac_enable_all_dma_irq(priv);
 
 	mutex_unlock(&priv->lock);
 	rtnl_unlock();
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2] net: stmmac: only enable DMA interrupts when ready
@ 2022-02-24 11:38 ` Vincent Whitchurch
  0 siblings, 0 replies; 8+ messages in thread
From: Vincent Whitchurch @ 2022-02-24 11:38 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Coquelin
  Cc: kernel, Vincent Whitchurch, Lars Persson, Srinivas Kandagatla,
	netdev, linux-stm32, linux-arm-kernel, linux-kernel

In this driver's ->ndo_open() callback, it enables DMA interrupts,
starts the DMA channels, then requests interrupts with request_irq(),
and then finally enables napi.

If RX DMA interrupts are received before napi is enabled, no processing
is done because napi_schedule_prep() will return false.  If the network
has a lot of broadcast/multicast traffic, then the RX ring could fill up
completely before napi is enabled.  When this happens, no further RX
interrupts will be delivered, and the driver will fail to receive any
packets.

Fix this by only enabling DMA interrupts after all other initialization
is complete.

Fixes: 523f11b5d4fd72efb ("net: stmmac: move hardware setup for stmmac_open to new function")
Reported-by: Lars Persson <larper@axis.com>
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6708ca2aa4f7..43978558d6c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
 	stmmac_stop_tx(priv, priv->ioaddr, chan);
 }
 
+static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
+{
+	u32 rx_channels_count = priv->plat->rx_queues_to_use;
+	u32 tx_channels_count = priv->plat->tx_queues_to_use;
+	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
+	u32 chan;
+
+	for (chan = 0; chan < dma_csr_ch; chan++) {
+		struct stmmac_channel *ch = &priv->channel[chan];
+		unsigned long flags;
+
+		spin_lock_irqsave(&ch->lock, flags);
+		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+		spin_unlock_irqrestore(&ch->lock, flags);
+	}
+}
+
 /**
  * stmmac_start_all_dma - start all RX and TX DMA channels
  * @priv: driver private structure
@@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
 
 	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++)
+	for (chan = 0; chan < dma_csr_ch; chan++) {
 		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
+		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+	}
 
 	/* DMA RX Channel Configuration */
 	for (chan = 0; chan < rx_channels_count; chan++) {
@@ -3759,6 +3778,7 @@ static int stmmac_open(struct net_device *dev)
 
 	stmmac_enable_all_queues(priv);
 	netif_tx_start_all_queues(priv->dev);
+	stmmac_enable_all_dma_irq(priv);
 
 	return 0;
 
@@ -6508,8 +6528,10 @@ int stmmac_xdp_open(struct net_device *dev)
 	}
 
 	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++)
+	for (chan = 0; chan < dma_csr_ch; chan++) {
 		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
+		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+	}
 
 	/* Adjust Split header */
 	sph_en = (priv->hw->rx_csum > 0) && priv->sph;
@@ -6570,6 +6592,7 @@ int stmmac_xdp_open(struct net_device *dev)
 	stmmac_enable_all_queues(priv);
 	netif_carrier_on(dev);
 	netif_tx_start_all_queues(dev);
+	stmmac_enable_all_dma_irq(priv);
 
 	return 0;
 
@@ -7447,6 +7470,7 @@ int stmmac_resume(struct device *dev)
 	stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw);
 
 	stmmac_enable_all_queues(priv);
+	stmmac_enable_all_dma_irq(priv);
 
 	mutex_unlock(&priv->lock);
 	rtnl_unlock();
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
  2022-02-24 11:38 ` Vincent Whitchurch
@ 2022-02-24 12:53   ` Denis Kirjanov
  -1 siblings, 0 replies; 8+ messages in thread
From: Denis Kirjanov @ 2022-02-24 12:53 UTC (permalink / raw)
  To: Vincent Whitchurch, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S. Miller, Jakub Kicinski, Maxime Coquelin
  Cc: kernel, Lars Persson, Srinivas Kandagatla, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel



2/24/22 14:38, Vincent Whitchurch пишет:
> In this driver's ->ndo_open() callback, it enables DMA interrupts,
> starts the DMA channels, then requests interrupts with request_irq(),
> and then finally enables napi.
> 
> If RX DMA interrupts are received before napi is enabled, no processing
> is done because napi_schedule_prep() will return false.  If the network
> has a lot of broadcast/multicast traffic, then the RX ring could fill up
> completely before napi is enabled.  When this happens, no further RX
> interrupts will be delivered, and the driver will fail to receive any
> packets.
> 
> Fix this by only enabling DMA interrupts after all other initialization
> is complete.
> 
> Fixes: 523f11b5d4fd72efb ("net: stmmac: move hardware setup for stmmac_open to new function")
> Reported-by: Lars Persson <larper@axis.com>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6708ca2aa4f7..43978558d6c0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
>   	stmmac_stop_tx(priv, priv->ioaddr, chan);
>   }
>   
> +static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
> +{
> +	u32 rx_channels_count = priv->plat->rx_queues_to_use;
> +	u32 tx_channels_count = priv->plat->tx_queues_to_use;
> +	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
> +	u32 chan;
> +
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
> +		struct stmmac_channel *ch = &priv->channel[chan];
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&ch->lock, flags);
> +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +		spin_unlock_irqrestore(&ch->lock, flags);
> +	}
> +}
> +
>   /**
>    * stmmac_start_all_dma - start all RX and TX DMA channels
>    * @priv: driver private structure
> @@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>   		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
>   
>   	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++)
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
>   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
Did you miss to take a channel lock?
> +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +	}
>   
>   	/* DMA RX Channel Configuration */
>   	for (chan = 0; chan < rx_channels_count; chan++) {
> @@ -3759,6 +3778,7 @@ static int stmmac_open(struct net_device *dev)
>   
>   	stmmac_enable_all_queues(priv);
>   	netif_tx_start_all_queues(priv->dev);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	return 0;
>   
> @@ -6508,8 +6528,10 @@ int stmmac_xdp_open(struct net_device *dev)
>   	}
>   
>   	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++)
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
>   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +	}
>   
>   	/* Adjust Split header */
>   	sph_en = (priv->hw->rx_csum > 0) && priv->sph;
> @@ -6570,6 +6592,7 @@ int stmmac_xdp_open(struct net_device *dev)
>   	stmmac_enable_all_queues(priv);
>   	netif_carrier_on(dev);
>   	netif_tx_start_all_queues(dev);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	return 0;
>   
> @@ -7447,6 +7470,7 @@ int stmmac_resume(struct device *dev)
>   	stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw);
>   
>   	stmmac_enable_all_queues(priv);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	mutex_unlock(&priv->lock);
>   	rtnl_unlock();

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
@ 2022-02-24 12:53   ` Denis Kirjanov
  0 siblings, 0 replies; 8+ messages in thread
From: Denis Kirjanov @ 2022-02-24 12:53 UTC (permalink / raw)
  To: Vincent Whitchurch, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S. Miller, Jakub Kicinski, Maxime Coquelin
  Cc: kernel, Lars Persson, Srinivas Kandagatla, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel



2/24/22 14:38, Vincent Whitchurch пишет:
> In this driver's ->ndo_open() callback, it enables DMA interrupts,
> starts the DMA channels, then requests interrupts with request_irq(),
> and then finally enables napi.
> 
> If RX DMA interrupts are received before napi is enabled, no processing
> is done because napi_schedule_prep() will return false.  If the network
> has a lot of broadcast/multicast traffic, then the RX ring could fill up
> completely before napi is enabled.  When this happens, no further RX
> interrupts will be delivered, and the driver will fail to receive any
> packets.
> 
> Fix this by only enabling DMA interrupts after all other initialization
> is complete.
> 
> Fixes: 523f11b5d4fd72efb ("net: stmmac: move hardware setup for stmmac_open to new function")
> Reported-by: Lars Persson <larper@axis.com>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
>   .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6708ca2aa4f7..43978558d6c0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
>   	stmmac_stop_tx(priv, priv->ioaddr, chan);
>   }
>   
> +static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
> +{
> +	u32 rx_channels_count = priv->plat->rx_queues_to_use;
> +	u32 tx_channels_count = priv->plat->tx_queues_to_use;
> +	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
> +	u32 chan;
> +
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
> +		struct stmmac_channel *ch = &priv->channel[chan];
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&ch->lock, flags);
> +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +		spin_unlock_irqrestore(&ch->lock, flags);
> +	}
> +}
> +
>   /**
>    * stmmac_start_all_dma - start all RX and TX DMA channels
>    * @priv: driver private structure
> @@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>   		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
>   
>   	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++)
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
>   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
Did you miss to take a channel lock?
> +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +	}
>   
>   	/* DMA RX Channel Configuration */
>   	for (chan = 0; chan < rx_channels_count; chan++) {
> @@ -3759,6 +3778,7 @@ static int stmmac_open(struct net_device *dev)
>   
>   	stmmac_enable_all_queues(priv);
>   	netif_tx_start_all_queues(priv->dev);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	return 0;
>   
> @@ -6508,8 +6528,10 @@ int stmmac_xdp_open(struct net_device *dev)
>   	}
>   
>   	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++)
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
>   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +	}
>   
>   	/* Adjust Split header */
>   	sph_en = (priv->hw->rx_csum > 0) && priv->sph;
> @@ -6570,6 +6592,7 @@ int stmmac_xdp_open(struct net_device *dev)
>   	stmmac_enable_all_queues(priv);
>   	netif_carrier_on(dev);
>   	netif_tx_start_all_queues(dev);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	return 0;
>   
> @@ -7447,6 +7470,7 @@ int stmmac_resume(struct device *dev)
>   	stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw);
>   
>   	stmmac_enable_all_queues(priv);
> +	stmmac_enable_all_dma_irq(priv);
>   
>   	mutex_unlock(&priv->lock);
>   	rtnl_unlock();

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
  2022-02-24 12:53   ` Denis Kirjanov
@ 2022-02-24 13:40     ` Vincent Whitchurch
  -1 siblings, 0 replies; 8+ messages in thread
From: Vincent Whitchurch @ 2022-02-24 13:40 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Coquelin, kernel,
	Lars Persson, Srinivas Kandagatla, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

On Thu, Feb 24, 2022 at 01:53:33PM +0100, Denis Kirjanov wrote:
> 2/24/22 14:38, Vincent Whitchurch пишет:
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6708ca2aa4f7..43978558d6c0 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
> >   	stmmac_stop_tx(priv, priv->ioaddr, chan);
> >   }
> >   
> > +static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
> > +{
> > +	u32 rx_channels_count = priv->plat->rx_queues_to_use;
> > +	u32 tx_channels_count = priv->plat->tx_queues_to_use;
> > +	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
> > +	u32 chan;
> > +
> > +	for (chan = 0; chan < dma_csr_ch; chan++) {
> > +		struct stmmac_channel *ch = &priv->channel[chan];
> > +		unsigned long flags;
> > +
> > +		spin_lock_irqsave(&ch->lock, flags);
> > +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> > +		spin_unlock_irqrestore(&ch->lock, flags);
> > +	}
> > +}
> > +
> >   /**
> >    * stmmac_start_all_dma - start all RX and TX DMA channels
> >    * @priv: driver private structure
> > @@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> >   		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
> >   
> >   	/* DMA CSR Channel configuration */
> > -	for (chan = 0; chan < dma_csr_ch; chan++)
> > +	for (chan = 0; chan < dma_csr_ch; chan++) {
> >   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> Did you miss to take a channel lock?

I didn't add it on purpose.  At this point during initialization there
is no-one who can race with the register write in
stmmac_disable_dma_irq().  The call to stmmac_init_chan() (in the
existing code) writes the same register without the lock. 

> > +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> > +	}
> >   

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
@ 2022-02-24 13:40     ` Vincent Whitchurch
  0 siblings, 0 replies; 8+ messages in thread
From: Vincent Whitchurch @ 2022-02-24 13:40 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Coquelin, kernel,
	Lars Persson, Srinivas Kandagatla, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

On Thu, Feb 24, 2022 at 01:53:33PM +0100, Denis Kirjanov wrote:
> 2/24/22 14:38, Vincent Whitchurch пишет:
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6708ca2aa4f7..43978558d6c0 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -2260,6 +2260,23 @@ static void stmmac_stop_tx_dma(struct stmmac_priv *priv, u32 chan)
> >   	stmmac_stop_tx(priv, priv->ioaddr, chan);
> >   }
> >   
> > +static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
> > +{
> > +	u32 rx_channels_count = priv->plat->rx_queues_to_use;
> > +	u32 tx_channels_count = priv->plat->tx_queues_to_use;
> > +	u32 dma_csr_ch = max(rx_channels_count, tx_channels_count);
> > +	u32 chan;
> > +
> > +	for (chan = 0; chan < dma_csr_ch; chan++) {
> > +		struct stmmac_channel *ch = &priv->channel[chan];
> > +		unsigned long flags;
> > +
> > +		spin_lock_irqsave(&ch->lock, flags);
> > +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> > +		spin_unlock_irqrestore(&ch->lock, flags);
> > +	}
> > +}
> > +
> >   /**
> >    * stmmac_start_all_dma - start all RX and TX DMA channels
> >    * @priv: driver private structure
> > @@ -2902,8 +2919,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> >   		stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
> >   
> >   	/* DMA CSR Channel configuration */
> > -	for (chan = 0; chan < dma_csr_ch; chan++)
> > +	for (chan = 0; chan < dma_csr_ch; chan++) {
> >   		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> Did you miss to take a channel lock?

I didn't add it on purpose.  At this point during initialization there
is no-one who can race with the register write in
stmmac_disable_dma_irq().  The call to stmmac_init_chan() (in the
existing code) writes the same register without the lock. 

> > +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> > +	}
> >   

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
  2022-02-24 11:38 ` Vincent Whitchurch
@ 2022-02-25 10:50   ` patchwork-bot+netdevbpf
  -1 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-25 10:50 UTC (permalink / raw)
  To: Vincent Whitchurch
  Cc: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba,
	mcoquelin.stm32, kernel, larper, srinivas.kandagatla, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 24 Feb 2022 12:38:29 +0100 you wrote:
> In this driver's ->ndo_open() callback, it enables DMA interrupts,
> starts the DMA channels, then requests interrupts with request_irq(),
> and then finally enables napi.
> 
> If RX DMA interrupts are received before napi is enabled, no processing
> is done because napi_schedule_prep() will return false.  If the network
> has a lot of broadcast/multicast traffic, then the RX ring could fill up
> completely before napi is enabled.  When this happens, no further RX
> interrupts will be delivered, and the driver will fail to receive any
> packets.
> 
> [...]

Here is the summary with links:
  - [v2] net: stmmac: only enable DMA interrupts when ready
    https://git.kernel.org/netdev/net/c/087a7b944c5d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: stmmac: only enable DMA interrupts when ready
@ 2022-02-25 10:50   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-25 10:50 UTC (permalink / raw)
  To: Vincent Whitchurch
  Cc: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba,
	mcoquelin.stm32, kernel, larper, srinivas.kandagatla, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 24 Feb 2022 12:38:29 +0100 you wrote:
> In this driver's ->ndo_open() callback, it enables DMA interrupts,
> starts the DMA channels, then requests interrupts with request_irq(),
> and then finally enables napi.
> 
> If RX DMA interrupts are received before napi is enabled, no processing
> is done because napi_schedule_prep() will return false.  If the network
> has a lot of broadcast/multicast traffic, then the RX ring could fill up
> completely before napi is enabled.  When this happens, no further RX
> interrupts will be delivered, and the driver will fail to receive any
> packets.
> 
> [...]

Here is the summary with links:
  - [v2] net: stmmac: only enable DMA interrupts when ready
    https://git.kernel.org/netdev/net/c/087a7b944c5d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-02-25 10:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 11:38 [PATCH v2] net: stmmac: only enable DMA interrupts when ready Vincent Whitchurch
2022-02-24 11:38 ` Vincent Whitchurch
2022-02-24 12:53 ` Denis Kirjanov
2022-02-24 12:53   ` Denis Kirjanov
2022-02-24 13:40   ` Vincent Whitchurch
2022-02-24 13:40     ` Vincent Whitchurch
2022-02-25 10:50 ` patchwork-bot+netdevbpf
2022-02-25 10:50   ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.