| Message ID | 1483613000-537-1-git-send-email-horms+renesas@verge.net.au |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello! On 01/05/2017 01:43 PM, Simon Horman wrote: > From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > > "swiotlb buffer is full" errors occur after repeated initialisation of a > device - f.e. suspend/resume or ip link set up/down. This is because memory > mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit() > is not released. Resolve this problem by unmapping descriptors when > freeing rings. > > Note, ravb_tx_free() is moved but not otherwise modified by this patch. > > Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > [simon: reworked] > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > -- > v1 [Kazuya Mizuguchi] > > v2 [Simon Horman] > * As suggested by Sergei Shtylyov > - Use dma_mapping_error() and rx_desc->ds_cc when unmapping RX descriptors; > this is consistent with the way that they are mapped > - Use ravb_tx_free() to clear TX descriptors Not sure that was good idea (sorry)... ravb_tx_ring() only unmaps the transmitted buffers, while we need to unmap everything... > * Reduce scope of new local variable > --- > drivers/net/ethernet/renesas/ravb_main.c | 89 ++++++++++++++++++-------------- > 1 file changed, 51 insertions(+), 38 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 92d7692c840d..1797c48e3176 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -179,6 +179,44 @@ static struct mdiobb_ops bb_ops = { > .get_mdio_data = ravb_get_mdio_data, > }; > > +/* Free TX skb function for AVB-IP */ > +static int ravb_tx_free(struct net_device *ndev, int q) > +{ > + struct ravb_private *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &priv->stats[q]; > + struct ravb_tx_desc *desc; > + int free_num = 0; > + int entry; > + u32 size; > + > + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > + entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * > + NUM_TX_DESC); > + desc = &priv->tx_ring[q][entry]; > + if (desc->die_dt != DT_FEMPTY) Here, it stop once an untransmitted buffer is encountered... > + break; > + /* Descriptor type must be checked before all other reads */ > + dma_rmb(); > + size = le16_to_cpu(desc->ds_tagl) & TX_DS; > + /* Free the original skb. */ > + if (priv->tx_skb[q][entry / NUM_TX_DESC]) { > + dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr), > + size, DMA_TO_DEVICE); > + /* Last packet descriptor? */ > + if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { > + entry /= NUM_TX_DESC; > + dev_kfree_skb_any(priv->tx_skb[q][entry]); > + priv->tx_skb[q][entry] = NULL; > + stats->tx_packets++; > + } > + free_num++; > + } > + stats->tx_bytes += size; > + desc->die_dt = DT_EEMPTY; > + } > + return free_num; > +} > + > /* Free skb's and DMA buffers for Ethernet AVB */ > static void ravb_ring_free(struct net_device *ndev, int q) > { > @@ -207,6 +245,18 @@ static void ravb_ring_free(struct net_device *ndev, int q) > priv->tx_align[q] = NULL; > > if (priv->rx_ring[q]) { > + for (i = 0; i < priv->num_rx_ring[q]; i++) { > + struct ravb_ex_rx_desc *rx_desc = &priv->rx_ring[q][i]; > + > + if (!dma_mapping_error(ndev->dev.parent, > + rx_desc->dptr)) { You forgot le32_to_cpu() here, we can't use the raw descriptor fields. > + dma_unmap_single(ndev->dev.parent, > + le32_to_cpu(rx_desc->dptr), > + PKT_BUF_SZ, > + DMA_FROM_DEVICE); > + rx_desc->ds_cc = cpu_to_le16(0); You don't check it anyway, not sure what that buys... [...] MBR, Sergei
On Fri, Jan 06, 2017 at 10:02:36PM +0300, Sergei Shtylyov wrote: > Hello! > > On 01/05/2017 01:43 PM, Simon Horman wrote: > > >From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > > > >"swiotlb buffer is full" errors occur after repeated initialisation of a > >device - f.e. suspend/resume or ip link set up/down. This is because memory > >mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit() > >is not released. Resolve this problem by unmapping descriptors when > >freeing rings. > > > >Note, ravb_tx_free() is moved but not otherwise modified by this patch. > > > >Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > >[simon: reworked] > >Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > >-- > >v1 [Kazuya Mizuguchi] > > > >v2 [Simon Horman] > >* As suggested by Sergei Shtylyov > > - Use dma_mapping_error() and rx_desc->ds_cc when unmapping RX descriptors; > > this is consistent with the way that they are mapped > > - Use ravb_tx_free() to clear TX descriptors > > Not sure that was good idea (sorry)... ravb_tx_ring() only unmaps the > transmitted buffers, while we need to unmap everything... > > >* Reduce scope of new local variable > >--- > > drivers/net/ethernet/renesas/ravb_main.c | 89 ++++++++++++++++++-------------- > > 1 file changed, 51 insertions(+), 38 deletions(-) > > > >diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > >index 92d7692c840d..1797c48e3176 100644 > >--- a/drivers/net/ethernet/renesas/ravb_main.c > >+++ b/drivers/net/ethernet/renesas/ravb_main.c > >@@ -179,6 +179,44 @@ static struct mdiobb_ops bb_ops = { > > .get_mdio_data = ravb_get_mdio_data, > > }; > > > >+/* Free TX skb function for AVB-IP */ > >+static int ravb_tx_free(struct net_device *ndev, int q) > >+{ > >+ struct ravb_private *priv = netdev_priv(ndev); > >+ struct net_device_stats *stats = &priv->stats[q]; > >+ struct ravb_tx_desc *desc; > >+ int free_num = 0; > >+ int entry; > >+ u32 size; > >+ > >+ for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > >+ entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * > >+ NUM_TX_DESC); > >+ desc = &priv->tx_ring[q][entry]; > >+ if (desc->die_dt != DT_FEMPTY) > > Here, it stop once an untransmitted buffer is encountered... Yes, I see that now. I wonder if we should: a) paramatise ravb_tx_free() so it may either clear all transmitted buffers (current behaviour) or all buffers (new behaviour). b) provide a different version of this loop in ravb_ring_free() What are your thoughts? > >+ break; > >+ /* Descriptor type must be checked before all other reads */ > >+ dma_rmb(); > >+ size = le16_to_cpu(desc->ds_tagl) & TX_DS; > >+ /* Free the original skb. */ > >+ if (priv->tx_skb[q][entry / NUM_TX_DESC]) { > >+ dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr), > >+ size, DMA_TO_DEVICE); > >+ /* Last packet descriptor? */ > >+ if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { > >+ entry /= NUM_TX_DESC; > >+ dev_kfree_skb_any(priv->tx_skb[q][entry]); > >+ priv->tx_skb[q][entry] = NULL; > >+ stats->tx_packets++; > >+ } > >+ free_num++; > >+ } > >+ stats->tx_bytes += size; > >+ desc->die_dt = DT_EEMPTY; > >+ } > >+ return free_num; > >+} > >+ > > /* Free skb's and DMA buffers for Ethernet AVB */ > > static void ravb_ring_free(struct net_device *ndev, int q) > > { > >@@ -207,6 +245,18 @@ static void ravb_ring_free(struct net_device *ndev, int q) > > priv->tx_align[q] = NULL; > > > > if (priv->rx_ring[q]) { > >+ for (i = 0; i < priv->num_rx_ring[q]; i++) { > >+ struct ravb_ex_rx_desc *rx_desc = &priv->rx_ring[q][i]; > >+ > >+ if (!dma_mapping_error(ndev->dev.parent, > >+ rx_desc->dptr)) { > > You forgot le32_to_cpu() here, we can't use the raw descriptor fields. Thanks, I will fix that. > >+ dma_unmap_single(ndev->dev.parent, > >+ le32_to_cpu(rx_desc->dptr), > >+ PKT_BUF_SZ, > >+ DMA_FROM_DEVICE); > >+ rx_desc->ds_cc = cpu_to_le16(0); > > You don't check it anyway, not sure what that buys... Thanks, I will see about dropping that.
On 01/12/2017 12:11 PM, Simon Horman wrote: >>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>> >>> "swiotlb buffer is full" errors occur after repeated initialisation of a >>> device - f.e. suspend/resume or ip link set up/down. This is because memory >>> mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit() >>> is not released. Resolve this problem by unmapping descriptors when >>> freeing rings. >>> >>> Note, ravb_tx_free() is moved but not otherwise modified by this patch. >>> >>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> >>> [simon: reworked] >>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> >>> -- >>> v1 [Kazuya Mizuguchi] >>> >>> v2 [Simon Horman] >>> * As suggested by Sergei Shtylyov >>> - Use dma_mapping_error() and rx_desc->ds_cc when unmapping RX descriptors; >>> this is consistent with the way that they are mapped >>> - Use ravb_tx_free() to clear TX descriptors >> >> Not sure that was good idea (sorry)... ravb_tx_ring() only unmaps the >> transmitted buffers, while we need to unmap everything... >> >>> * Reduce scope of new local variable >>> --- >>> drivers/net/ethernet/renesas/ravb_main.c | 89 ++++++++++++++++++-------------- >>> 1 file changed, 51 insertions(+), 38 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >>> index 92d7692c840d..1797c48e3176 100644 >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>> @@ -179,6 +179,44 @@ static struct mdiobb_ops bb_ops = { >>> .get_mdio_data = ravb_get_mdio_data, >>> }; >>> >>> +/* Free TX skb function for AVB-IP */ >>> +static int ravb_tx_free(struct net_device *ndev, int q) >>> +{ >>> + struct ravb_private *priv = netdev_priv(ndev); >>> + struct net_device_stats *stats = &priv->stats[q]; >>> + struct ravb_tx_desc *desc; >>> + int free_num = 0; >>> + int entry; >>> + u32 size; >>> + >>> + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { >>> + entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * >>> + NUM_TX_DESC); >>> + desc = &priv->tx_ring[q][entry]; >>> + if (desc->die_dt != DT_FEMPTY) >> >> Here, it stop once an untransmitted buffer is encountered... > > Yes, I see that now. > > I wonder if we should: > > a) paramatise ravb_tx_free() so it may either clear all transmitted buffers > (current behaviour) or all buffers (new behaviour). > b) provide a different version of this loop in ravb_ring_free() > > What are your thoughts? I'm voting for (b). [...] MBR, Sergei
On Thu, Jan 12, 2017 at 03:03:05PM +0300, Sergei Shtylyov wrote: > On 01/12/2017 12:11 PM, Simon Horman wrote: ... > >> Here, it stop once an untransmitted buffer is encountered... > > > >Yes, I see that now. > > > >I wonder if we should: > > > >a) paramatise ravb_tx_free() so it may either clear all transmitted buffers > > (current behaviour) or all buffers (new behaviour). > >b) provide a different version of this loop in ravb_ring_free() > > > >What are your thoughts? > > I'm voting for (b). Ok, something like this? @@ -215,6 +225,30 @@ static void ravb_ring_free(struct net_device *ndev, int q) } if (priv->tx_ring[q]) { + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { + struct ravb_tx_desc *desc; + int entry; + + entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * + NUM_TX_DESC); + desc = &priv->tx_ring[q][entry]; + + /* Free the original skb. */ + if (priv->tx_skb[q][entry / NUM_TX_DESC]) { + u32 size = le16_to_cpu(desc->ds_tagl) & TX_DS; + + dma_unmap_single(ndev->dev.parent, + le32_to_cpu(desc->dptr), + size, DMA_TO_DEVICE); + /* Last packet descriptor? */ + if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { + entry /= NUM_TX_DESC; + dev_kfree_skb_any(priv->tx_skb[q][entry]); + priv->tx_skb[q][entry] = NULL; + } + } + } + ring_size = sizeof(struct ravb_tx_desc) * (priv->num_tx_ring[q] * NUM_TX_DESC + 1); dma_free_coherent(ndev->dev.parent, ring_size, priv->tx_ring[q],
Hi, On 12.01.2017 10:11, Simon Horman wrote: >>> + >>> + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { BTW: How can this work correctly when cur_tx wraps and dirty_tx is greater? Regards, Lino
On 01/12/2017 04:18 PM, Simon Horman wrote: > ... > >>>> Here, it stop once an untransmitted buffer is encountered... >>> >>> Yes, I see that now. >>> >>> I wonder if we should: >>> >>> a) paramatise ravb_tx_free() so it may either clear all transmitted buffers >>> (current behaviour) or all buffers (new behaviour). >>> b) provide a different version of this loop in ravb_ring_free() >>> >>> What are your thoughts? >> >> I'm voting for (b). > > Ok, something like this? > > @@ -215,6 +225,30 @@ static void ravb_ring_free(struct net_device *ndev, int q) > } > > if (priv->tx_ring[q]) { > + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { You're still copying the loop logic from ravb_tx_free() while we (I think) need a simple loop over all the descriptor ring. [...] MBR, Sergei
On 01/12/2017 04:23 PM, Lino Sanfilippo wrote: >>>> + >>>> + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > > BTW: How can this work correctly when cur_tx wraps and dirty_tx is greater? {cur|dirty}_tx never wrap. > Regards, > Lino MBR, Sergei
Hi, On 12.01.2017 17:37, Sergei Shtylyov wrote: > External Email > > ---------------------------------------------------------------------- > On 01/12/2017 04:23 PM, Lino Sanfilippo wrote: > >>>>> + >>>>> + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { >> >> BTW: How can this work correctly when cur_tx wraps and dirty_tx is greater? > > {cur|dirty}_tx never wrap. > Both values are 32 bit and AFAICS they are only incremented (and never reset or decremented). So what prevents them from wrapping every 2^32 processed tx descriptors? Am I missing something? Regards, Lino
On 01/12/2017 07:55 PM, Lino Sanfilippo wrote: >>>>>> + >>>>>> + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { >>> >>> BTW: How can this work correctly when cur_tx wraps and dirty_tx is greater? >> >> {cur|dirty}_tx never wrap. >> > > Both values are 32 bit and AFAICS they are only incremented (and never reset or > decremented). Yeah, sorry, O thought you mean the TX ring size wrapping. > So what prevents them from wrapping every 2^32 processed tx descriptors? Am I > missing > something? The math should still work all right IIUC. > Regards, > Lino MBR, Sergei
On Thu, Jan 12, 2017 at 07:33:51PM +0300, Sergei Shtylyov wrote: > On 01/12/2017 04:18 PM, Simon Horman wrote: > > >... > > > >>>> Here, it stop once an untransmitted buffer is encountered... > >>> > >>>Yes, I see that now. > >>> > >>>I wonder if we should: > >>> > >>>a) paramatise ravb_tx_free() so it may either clear all transmitted buffers > >>> (current behaviour) or all buffers (new behaviour). > >>>b) provide a different version of this loop in ravb_ring_free() > >>> > >>>What are your thoughts? > >> > >> I'm voting for (b). > > > >Ok, something like this? > > > >@@ -215,6 +225,30 @@ static void ravb_ring_free(struct net_device *ndev, int q) > > } > > > > if (priv->tx_ring[q]) { > >+ for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > > You're still copying the loop logic from ravb_tx_free() while we (I > think) need a simple loop over all the descriptor ring. I thought the above may be better as it seems to me that only descriptors in the range described above could need to be freed. But simpler may be better. And I also noticed several other problems with the code I posted yesterday afternoon. How about something like this? @@ -215,6 +225,17 @@ static void ravb_ring_free(struct net_device *ndev, int q) } if (priv->tx_ring[q]) { + for (i = 0; i < priv->num_tx_ring[q] * NUM_TX_DESC; i++) { + struct ravb_tx_desc *desc = &priv->tx_ring[q][i]; + + if (desc->die_dt != DT_EEMPTY) { + u32 size = le16_to_cpu(desc->ds_tagl) & TX_DS; + + dma_unmap_single(ndev->dev.parent, + le32_to_cpu(desc->dptr), + size, DMA_TO_DEVICE); + } + } ring_size = sizeof(struct ravb_tx_desc) * (priv->num_tx_ring[q] * NUM_TX_DESC + 1); dma_free_coherent(ndev->dev.parent, ring_size, priv->tx_ring[q],
Hello! On 01/12/2017 04:18 PM, Simon Horman wrote: > ... >>>> Here, it stop once an untransmitted buffer is encountered... >>> >>> Yes, I see that now. >>> >>> I wonder if we should: >>> >>> a) paramatise ravb_tx_free() so it may either clear all transmitted buffers >>> (current behaviour) or all buffers (new behaviour). >>> b) provide a different version of this loop in ravb_ring_free() >>> >>> What are your thoughts? >> >> I'm voting for (b). After looking at this issue for another time, I'll vote for (a). Sorry for back-and-forth on this matter -- I somehow thought we could do a better job by scanning all the TX ring... > Ok, something like this? > > @@ -215,6 +225,30 @@ static void ravb_ring_free(struct net_device *ndev, int q) > } > > if (priv->tx_ring[q]) { > + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > + struct ravb_tx_desc *desc; > + int entry; > + > + entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * > + NUM_TX_DESC); > + desc = &priv->tx_ring[q][entry]; > + > + /* Free the original skb. */ > + if (priv->tx_skb[q][entry / NUM_TX_DESC]) { > + u32 size = le16_to_cpu(desc->ds_tagl) & TX_DS; > + > + dma_unmap_single(ndev->dev.parent, > + le32_to_cpu(desc->dptr), > + size, DMA_TO_DEVICE); > + /* Last packet descriptor? */ > + if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { > + entry /= NUM_TX_DESC; > + dev_kfree_skb_any(priv->tx_skb[q][entry]); > + priv->tx_skb[q][entry] = NULL; > + } > + } > + } > + This is only different from ravb_tx_free() by not stopping on unfinished descriptor, so we must be good with adding an extra param to it... [...] MBR, Sergei
On Mon, Jan 16, 2017 at 11:41:51PM +0300, Sergei Shtylyov wrote: > Hello! > > On 01/12/2017 04:18 PM, Simon Horman wrote: > > >... > > >>>> Here, it stop once an untransmitted buffer is encountered... > >>> > >>>Yes, I see that now. > >>> > >>>I wonder if we should: > >>> > >>>a) paramatise ravb_tx_free() so it may either clear all transmitted buffers > >>> (current behaviour) or all buffers (new behaviour). > >>>b) provide a different version of this loop in ravb_ring_free() > >>> > >>>What are your thoughts? > >> > >> I'm voting for (b). > > After looking at this issue for another time, I'll vote for (a). Sorry > for back-and-forth on this matter -- I somehow thought we could do a better > job by scanning all the TX ring... No problem. I also have had several opinions on this over time. I'll see about respinning the patch. > > >Ok, something like this? > > > >@@ -215,6 +225,30 @@ static void ravb_ring_free(struct net_device *ndev, int q) > > } > > > > if (priv->tx_ring[q]) { > >+ for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { > >+ struct ravb_tx_desc *desc; > >+ int entry; > >+ > >+ entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * > >+ NUM_TX_DESC); > >+ desc = &priv->tx_ring[q][entry]; > >+ > >+ /* Free the original skb. */ > >+ if (priv->tx_skb[q][entry / NUM_TX_DESC]) { > >+ u32 size = le16_to_cpu(desc->ds_tagl) & TX_DS; > >+ > >+ dma_unmap_single(ndev->dev.parent, > >+ le32_to_cpu(desc->dptr), > >+ size, DMA_TO_DEVICE); > >+ /* Last packet descriptor? */ > >+ if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { > >+ entry /= NUM_TX_DESC; > >+ dev_kfree_skb_any(priv->tx_skb[q][entry]); > >+ priv->tx_skb[q][entry] = NULL; > >+ } > >+ } > >+ } > >+ > > This is only different from ravb_tx_free() by not stopping on unfinished > descriptor, so we must be good with adding an extra param to it... > > [...] > > MBR, Sergei >
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 92d7692c840d..1797c48e3176 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -179,6 +179,44 @@ static struct mdiobb_ops bb_ops = { .get_mdio_data = ravb_get_mdio_data, }; +/* Free TX skb function for AVB-IP */ +static int ravb_tx_free(struct net_device *ndev, int q) +{ + struct ravb_private *priv = netdev_priv(ndev); + struct net_device_stats *stats = &priv->stats[q]; + struct ravb_tx_desc *desc; + int free_num = 0; + int entry; + u32 size; + + for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { + entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * + NUM_TX_DESC); + desc = &priv->tx_ring[q][entry]; + if (desc->die_dt != DT_FEMPTY) + break; + /* Descriptor type must be checked before all other reads */ + dma_rmb(); + size = le16_to_cpu(desc->ds_tagl) & TX_DS; + /* Free the original skb. */ + if (priv->tx_skb[q][entry / NUM_TX_DESC]) { + dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr), + size, DMA_TO_DEVICE); + /* Last packet descriptor? */ + if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { + entry /= NUM_TX_DESC; + dev_kfree_skb_any(priv->tx_skb[q][entry]); + priv->tx_skb[q][entry] = NULL; + stats->tx_packets++; + } + free_num++; + } + stats->tx_bytes += size; + desc->die_dt = DT_EEMPTY; + } + return free_num; +} + /* Free skb's and DMA buffers for Ethernet AVB */ static void ravb_ring_free(struct net_device *ndev, int q) { @@ -207,6 +245,18 @@ static void ravb_ring_free(struct net_device *ndev, int q) priv->tx_align[q] = NULL; if (priv->rx_ring[q]) { + for (i = 0; i < priv->num_rx_ring[q]; i++) { + struct ravb_ex_rx_desc *rx_desc = &priv->rx_ring[q][i]; + + if (!dma_mapping_error(ndev->dev.parent, + rx_desc->dptr)) { + dma_unmap_single(ndev->dev.parent, + le32_to_cpu(rx_desc->dptr), + PKT_BUF_SZ, + DMA_FROM_DEVICE); + rx_desc->ds_cc = cpu_to_le16(0); + } + } ring_size = sizeof(struct ravb_ex_rx_desc) * (priv->num_rx_ring[q] + 1); dma_free_coherent(ndev->dev.parent, ring_size, priv->rx_ring[q], @@ -215,6 +265,7 @@ static void ravb_ring_free(struct net_device *ndev, int q) } if (priv->tx_ring[q]) { + ravb_tx_free(ndev, q); ring_size = sizeof(struct ravb_tx_desc) * (priv->num_tx_ring[q] * NUM_TX_DESC + 1); dma_free_coherent(ndev->dev.parent, ring_size, priv->tx_ring[q], @@ -431,44 +482,6 @@ static int ravb_dmac_init(struct net_device *ndev) return 0; } -/* Free TX skb function for AVB-IP */ -static int ravb_tx_free(struct net_device *ndev, int q) -{ - struct ravb_private *priv = netdev_priv(ndev); - struct net_device_stats *stats = &priv->stats[q]; - struct ravb_tx_desc *desc; - int free_num = 0; - int entry; - u32 size; - - for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) { - entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] * - NUM_TX_DESC); - desc = &priv->tx_ring[q][entry]; - if (desc->die_dt != DT_FEMPTY) - break; - /* Descriptor type must be checked before all other reads */ - dma_rmb(); - size = le16_to_cpu(desc->ds_tagl) & TX_DS; - /* Free the original skb. */ - if (priv->tx_skb[q][entry / NUM_TX_DESC]) { - dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr), - size, DMA_TO_DEVICE); - /* Last packet descriptor? */ - if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) { - entry /= NUM_TX_DESC; - dev_kfree_skb_any(priv->tx_skb[q][entry]); - priv->tx_skb[q][entry] = NULL; - stats->tx_packets++; - } - free_num++; - } - stats->tx_bytes += size; - desc->die_dt = DT_EEMPTY; - } - return free_num; -} - static void ravb_get_tx_tstamp(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev);