* [PATCH] net: macb: free resources on failure path of at91ether_open()
@ 2020-06-24 7:26 Claudiu Beznea
2020-06-24 10:04 ` Claudiu.Beznea
0 siblings, 1 reply; 2+ messages in thread
From: Claudiu Beznea @ 2020-06-24 7:26 UTC (permalink / raw)
To: nicolas.ferre, davem, kuba
Cc: antoine.tenart, netdev, linux-kernel, Claudiu Beznea
DMA buffers were not freed on failure path of at91ether_open().
Along with changes for freeing the DMA buffers the enable/disable
interrupt instructions were moved to at91ether_start()/at91ether_stop()
functions and the operations on at91ether_stop() were done in
their reverse order (compared with how is done in at91ether_start()):
before this patch the operation order on interface open path
was as follows:
1/ alloc DMA buffers
2/ enable tx, rx
3/ enable interrupts
and the order on interface close path was as follows:
1/ disable tx, rx
2/ disable interrupts
3/ free dma buffers.
Fixes: 7897b071ac3b ("net: macb: convert to phylink")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 116 +++++++++++++++++++------------
1 file changed, 73 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 5705359a3612..52582e8ed90e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3762,15 +3762,9 @@ static int macb_init(struct platform_device *pdev)
static struct sifive_fu540_macb_mgmt *mgmt;
-/* Initialize and start the Receiver and Transmit subsystems */
-static int at91ether_start(struct net_device *dev)
+static int at91ether_alloc_coherent(struct macb *lp)
{
- struct macb *lp = netdev_priv(dev);
struct macb_queue *q = &lp->queues[0];
- struct macb_dma_desc *desc;
- dma_addr_t addr;
- u32 ctl;
- int i;
q->rx_ring = dma_alloc_coherent(&lp->pdev->dev,
(AT91ETHER_MAX_RX_DESCR *
@@ -3792,6 +3786,43 @@ static int at91ether_start(struct net_device *dev)
return -ENOMEM;
}
+ return 0;
+}
+
+static void at91ether_free_coherent(struct macb *lp)
+{
+ struct macb_queue *q = &lp->queues[0];
+
+ if (q->rx_ring) {
+ dma_free_coherent(&lp->pdev->dev,
+ AT91ETHER_MAX_RX_DESCR *
+ macb_dma_desc_get_size(lp),
+ q->rx_ring, q->rx_ring_dma);
+ q->rx_ring = NULL;
+ }
+
+ if (q->rx_buffers) {
+ dma_free_coherent(&lp->pdev->dev,
+ AT91ETHER_MAX_RX_DESCR *
+ AT91ETHER_MAX_RBUFF_SZ,
+ q->rx_buffers, q->rx_buffers_dma);
+ q->rx_buffers = NULL;
+ }
+}
+
+/* Initialize and start the Receiver and Transmit subsystems */
+static int at91ether_start(struct macb *lp)
+{
+ struct macb_queue *q = &lp->queues[0];
+ struct macb_dma_desc *desc;
+ dma_addr_t addr;
+ u32 ctl;
+ int i, ret;
+
+ ret = at91ether_alloc_coherent(lp);
+ if (ret)
+ return ret;
+
addr = q->rx_buffers_dma;
for (i = 0; i < AT91ETHER_MAX_RX_DESCR; i++) {
desc = macb_rx_desc(q, i);
@@ -3813,9 +3844,39 @@ static int at91ether_start(struct net_device *dev)
ctl = macb_readl(lp, NCR);
macb_writel(lp, NCR, ctl | MACB_BIT(RE) | MACB_BIT(TE));
+ /* Enable MAC interrupts */
+ macb_writel(lp, IER, MACB_BIT(RCOMP) |
+ MACB_BIT(RXUBR) |
+ MACB_BIT(ISR_TUND) |
+ MACB_BIT(ISR_RLE) |
+ MACB_BIT(TCOMP) |
+ MACB_BIT(ISR_ROVR) |
+ MACB_BIT(HRESP));
+
return 0;
}
+static void at91ether_stop(struct macb *lp)
+{
+ u32 ctl;
+
+ /* Disable MAC interrupts */
+ macb_writel(lp, IDR, MACB_BIT(RCOMP) |
+ MACB_BIT(RXUBR) |
+ MACB_BIT(ISR_TUND) |
+ MACB_BIT(ISR_RLE) |
+ MACB_BIT(TCOMP) |
+ MACB_BIT(ISR_ROVR) |
+ MACB_BIT(HRESP));
+
+ /* Disable Receiver and Transmitter */
+ ctl = macb_readl(lp, NCR);
+ macb_writel(lp, NCR, ctl & ~(MACB_BIT(TE) | MACB_BIT(RE)));
+
+ /* Free resources. */
+ at91ether_free_coherent(lp);
+}
+
/* Open the ethernet interface */
static int at91ether_open(struct net_device *dev)
{
@@ -3835,27 +3896,20 @@ static int at91ether_open(struct net_device *dev)
macb_set_hwaddr(lp);
- ret = at91ether_start(dev);
+ ret = at91ether_start(lp);
if (ret)
goto pm_exit;
- /* Enable MAC interrupts */
- macb_writel(lp, IER, MACB_BIT(RCOMP) |
- MACB_BIT(RXUBR) |
- MACB_BIT(ISR_TUND) |
- MACB_BIT(ISR_RLE) |
- MACB_BIT(TCOMP) |
- MACB_BIT(ISR_ROVR) |
- MACB_BIT(HRESP));
-
ret = macb_phylink_connect(lp);
if (ret)
- goto pm_exit;
+ goto stop;
netif_start_queue(dev);
return 0;
+stop:
+ at91ether_stop(lp);
pm_exit:
pm_runtime_put_sync(&lp->pdev->dev);
return ret;
@@ -3865,37 +3919,13 @@ static int at91ether_open(struct net_device *dev)
static int at91ether_close(struct net_device *dev)
{
struct macb *lp = netdev_priv(dev);
- struct macb_queue *q = &lp->queues[0];
- u32 ctl;
-
- /* Disable Receiver and Transmitter */
- ctl = macb_readl(lp, NCR);
- macb_writel(lp, NCR, ctl & ~(MACB_BIT(TE) | MACB_BIT(RE)));
-
- /* Disable MAC interrupts */
- macb_writel(lp, IDR, MACB_BIT(RCOMP) |
- MACB_BIT(RXUBR) |
- MACB_BIT(ISR_TUND) |
- MACB_BIT(ISR_RLE) |
- MACB_BIT(TCOMP) |
- MACB_BIT(ISR_ROVR) |
- MACB_BIT(HRESP));
netif_stop_queue(dev);
phylink_stop(lp->phylink);
phylink_disconnect_phy(lp->phylink);
- dma_free_coherent(&lp->pdev->dev,
- AT91ETHER_MAX_RX_DESCR *
- macb_dma_desc_get_size(lp),
- q->rx_ring, q->rx_ring_dma);
- q->rx_ring = NULL;
-
- dma_free_coherent(&lp->pdev->dev,
- AT91ETHER_MAX_RX_DESCR * AT91ETHER_MAX_RBUFF_SZ,
- q->rx_buffers, q->rx_buffers_dma);
- q->rx_buffers = NULL;
+ at91ether_stop(lp);
return pm_runtime_put(&lp->pdev->dev);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] net: macb: free resources on failure path of at91ether_open()
2020-06-24 7:26 [PATCH] net: macb: free resources on failure path of at91ether_open() Claudiu Beznea
@ 2020-06-24 10:04 ` Claudiu.Beznea
0 siblings, 0 replies; 2+ messages in thread
From: Claudiu.Beznea @ 2020-06-24 10:04 UTC (permalink / raw)
To: Nicolas.Ferre, davem, kuba; +Cc: antoine.tenart, netdev, linux-kernel
Please ignore this one!
I'll send a v2.
On 24.06.2020 10:26, Claudiu Beznea wrote:
> DMA buffers were not freed on failure path of at91ether_open().
> Along with changes for freeing the DMA buffers the enable/disable
> interrupt instructions were moved to at91ether_start()/at91ether_stop()
> functions and the operations on at91ether_stop() were done in
> their reverse order (compared with how is done in at91ether_start()):
> before this patch the operation order on interface open path
> was as follows:
> 1/ alloc DMA buffers
> 2/ enable tx, rx
> 3/ enable interrupts
> and the order on interface close path was as follows:
> 1/ disable tx, rx
> 2/ disable interrupts
> 3/ free dma buffers.
>
> Fixes: 7897b071ac3b ("net: macb: convert to phylink")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 116 +++++++++++++++++++------------
> 1 file changed, 73 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 5705359a3612..52582e8ed90e 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3762,15 +3762,9 @@ static int macb_init(struct platform_device *pdev)
>
> static struct sifive_fu540_macb_mgmt *mgmt;
>
> -/* Initialize and start the Receiver and Transmit subsystems */
> -static int at91ether_start(struct net_device *dev)
> +static int at91ether_alloc_coherent(struct macb *lp)
> {
> - struct macb *lp = netdev_priv(dev);
> struct macb_queue *q = &lp->queues[0];
> - struct macb_dma_desc *desc;
> - dma_addr_t addr;
> - u32 ctl;
> - int i;
>
> q->rx_ring = dma_alloc_coherent(&lp->pdev->dev,
> (AT91ETHER_MAX_RX_DESCR *
> @@ -3792,6 +3786,43 @@ static int at91ether_start(struct net_device *dev)
> return -ENOMEM;
> }
>
> + return 0;
> +}
> +
> +static void at91ether_free_coherent(struct macb *lp)
> +{
> + struct macb_queue *q = &lp->queues[0];
> +
> + if (q->rx_ring) {
> + dma_free_coherent(&lp->pdev->dev,
> + AT91ETHER_MAX_RX_DESCR *
> + macb_dma_desc_get_size(lp),
> + q->rx_ring, q->rx_ring_dma);
> + q->rx_ring = NULL;
> + }
> +
> + if (q->rx_buffers) {
> + dma_free_coherent(&lp->pdev->dev,
> + AT91ETHER_MAX_RX_DESCR *
> + AT91ETHER_MAX_RBUFF_SZ,
> + q->rx_buffers, q->rx_buffers_dma);
> + q->rx_buffers = NULL;
> + }
> +}
> +
> +/* Initialize and start the Receiver and Transmit subsystems */
> +static int at91ether_start(struct macb *lp)
> +{
> + struct macb_queue *q = &lp->queues[0];
> + struct macb_dma_desc *desc;
> + dma_addr_t addr;
> + u32 ctl;
> + int i, ret;
> +
> + ret = at91ether_alloc_coherent(lp);
> + if (ret)
> + return ret;
> +
> addr = q->rx_buffers_dma;
> for (i = 0; i < AT91ETHER_MAX_RX_DESCR; i++) {
> desc = macb_rx_desc(q, i);
> @@ -3813,9 +3844,39 @@ static int at91ether_start(struct net_device *dev)
> ctl = macb_readl(lp, NCR);
> macb_writel(lp, NCR, ctl | MACB_BIT(RE) | MACB_BIT(TE));
>
> + /* Enable MAC interrupts */
> + macb_writel(lp, IER, MACB_BIT(RCOMP) |
> + MACB_BIT(RXUBR) |
> + MACB_BIT(ISR_TUND) |
> + MACB_BIT(ISR_RLE) |
> + MACB_BIT(TCOMP) |
> + MACB_BIT(ISR_ROVR) |
> + MACB_BIT(HRESP));
> +
> return 0;
> }
>
> +static void at91ether_stop(struct macb *lp)
> +{
> + u32 ctl;
> +
> + /* Disable MAC interrupts */
> + macb_writel(lp, IDR, MACB_BIT(RCOMP) |
> + MACB_BIT(RXUBR) |
> + MACB_BIT(ISR_TUND) |
> + MACB_BIT(ISR_RLE) |
> + MACB_BIT(TCOMP) |
> + MACB_BIT(ISR_ROVR) |
> + MACB_BIT(HRESP));
> +
> + /* Disable Receiver and Transmitter */
> + ctl = macb_readl(lp, NCR);
> + macb_writel(lp, NCR, ctl & ~(MACB_BIT(TE) | MACB_BIT(RE)));
> +
> + /* Free resources. */
> + at91ether_free_coherent(lp);
> +}
> +
> /* Open the ethernet interface */
> static int at91ether_open(struct net_device *dev)
> {
> @@ -3835,27 +3896,20 @@ static int at91ether_open(struct net_device *dev)
>
> macb_set_hwaddr(lp);
>
> - ret = at91ether_start(dev);
> + ret = at91ether_start(lp);
> if (ret)
> goto pm_exit;
>
> - /* Enable MAC interrupts */
> - macb_writel(lp, IER, MACB_BIT(RCOMP) |
> - MACB_BIT(RXUBR) |
> - MACB_BIT(ISR_TUND) |
> - MACB_BIT(ISR_RLE) |
> - MACB_BIT(TCOMP) |
> - MACB_BIT(ISR_ROVR) |
> - MACB_BIT(HRESP));
> -
> ret = macb_phylink_connect(lp);
> if (ret)
> - goto pm_exit;
> + goto stop;
>
> netif_start_queue(dev);
>
> return 0;
>
> +stop:
> + at91ether_stop(lp);
> pm_exit:
> pm_runtime_put_sync(&lp->pdev->dev);
> return ret;
> @@ -3865,37 +3919,13 @@ static int at91ether_open(struct net_device *dev)
> static int at91ether_close(struct net_device *dev)
> {
> struct macb *lp = netdev_priv(dev);
> - struct macb_queue *q = &lp->queues[0];
> - u32 ctl;
> -
> - /* Disable Receiver and Transmitter */
> - ctl = macb_readl(lp, NCR);
> - macb_writel(lp, NCR, ctl & ~(MACB_BIT(TE) | MACB_BIT(RE)));
> -
> - /* Disable MAC interrupts */
> - macb_writel(lp, IDR, MACB_BIT(RCOMP) |
> - MACB_BIT(RXUBR) |
> - MACB_BIT(ISR_TUND) |
> - MACB_BIT(ISR_RLE) |
> - MACB_BIT(TCOMP) |
> - MACB_BIT(ISR_ROVR) |
> - MACB_BIT(HRESP));
>
> netif_stop_queue(dev);
>
> phylink_stop(lp->phylink);
> phylink_disconnect_phy(lp->phylink);
>
> - dma_free_coherent(&lp->pdev->dev,
> - AT91ETHER_MAX_RX_DESCR *
> - macb_dma_desc_get_size(lp),
> - q->rx_ring, q->rx_ring_dma);
> - q->rx_ring = NULL;
> -
> - dma_free_coherent(&lp->pdev->dev,
> - AT91ETHER_MAX_RX_DESCR * AT91ETHER_MAX_RBUFF_SZ,
> - q->rx_buffers, q->rx_buffers_dma);
> - q->rx_buffers = NULL;
> + at91ether_stop(lp);
>
> return pm_runtime_put(&lp->pdev->dev);
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-06-24 10:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 7:26 [PATCH] net: macb: free resources on failure path of at91ether_open() Claudiu Beznea
2020-06-24 10:04 ` Claudiu.Beznea
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.