| Message ID | 20220812154820.2225457-1-saproj@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: moxa: inherit DMA masks to make dma_map_single() work | expand |
On Fri, Aug 12, 2022 at 06:48:20PM +0300, Sergei Antonov wrote: > dma_map_single() calls fail in moxart_mac_setup_desc_ring() and > moxart_mac_start_xmit() which leads to an incessant output of this: > > [ 16.043925] moxart-ethernet 92000000.mac eth0: DMA mapping error > [ 16.050957] moxart-ethernet 92000000.mac eth0: DMA mapping error > [ 16.058229] moxart-ethernet 92000000.mac eth0: DMA mapping error > > To make dma_map_single() work, inherit DMA masks from the platform device. > > Signed-off-by: Sergei Antonov <saproj@gmail.com> > CC: Jakub Kicinski <kuba@kernel.org> > CC: Pavel Skripkin <paskripkin@gmail.com> > CC: David S. Miller <davem@davemloft.net> > CC: Guobin Huang <huangguobin4@huawei.com> > CC: Paolo Abeni <pabeni@redhat.com> > --- > drivers/net/ethernet/moxa/moxart_ether.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c > index a3214a762e4b..de99211d85c2 100644 > --- a/drivers/net/ethernet/moxa/moxart_ether.c > +++ b/drivers/net/ethernet/moxa/moxart_ether.c > @@ -537,6 +537,10 @@ static int moxart_mac_probe(struct platform_device *pdev) > ndev->priv_flags |= IFF_UNICAST_FLT; > ndev->irq = irq; > > + /* Inherit the DMA masks from the platform device */ > + ndev->dev.dma_mask = p_dev->dma_mask; > + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask; There is only one other ethernet driver which does this. What you see much more often is: alacritech/slicoss.c: paddr = dma_map_single(&sdev->pdev->dev, skb->data, maplen, neterion/s2io.c: dma_map_single(&sp->pdev->dev, ba->ba_1, dlink/dl2k.c: cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data, micrel/ksz884x.c: dma_buf->dma = dma_map_single(&hw_priv->pdev->dev, skb->data, Andrew
On Fri, 12 Aug 2022 at 19:13, Andrew Lunn <andrew@lunn.ch> wrote: > > + /* Inherit the DMA masks from the platform device */ > > + ndev->dev.dma_mask = p_dev->dma_mask; > > + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask; > > There is only one other ethernet driver which does this. What you see > much more often is: > > alacritech/slicoss.c: paddr = dma_map_single(&sdev->pdev->dev, skb->data, maplen, > neterion/s2io.c: dma_map_single(&sp->pdev->dev, ba->ba_1, > dlink/dl2k.c: cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data, > micrel/ksz884x.c: dma_buf->dma = dma_map_single(&hw_priv->pdev->dev, skb->data, Also works. Do you recommend to create a v2 of the patch?
On Fri, Aug 12, 2022 at 07:35:43PM +0300, Sergei Antonov wrote: > On Fri, 12 Aug 2022 at 19:13, Andrew Lunn <andrew@lunn.ch> wrote: > > > + /* Inherit the DMA masks from the platform device */ > > > + ndev->dev.dma_mask = p_dev->dma_mask; > > > + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask; > > > > There is only one other ethernet driver which does this. What you see > > much more often is: > > > > alacritech/slicoss.c: paddr = dma_map_single(&sdev->pdev->dev, skb->data, maplen, > > neterion/s2io.c: dma_map_single(&sp->pdev->dev, ba->ba_1, > > dlink/dl2k.c: cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data, > > micrel/ksz884x.c: dma_buf->dma = dma_map_single(&hw_priv->pdev->dev, skb->data, > > Also works. Do you recommend to create a v2 of the patch? Yes please. It makes things easier to maintain if every driver does the same thing. Andrew
On 8/12/22 09:38, Andrew Lunn wrote: > On Fri, Aug 12, 2022 at 07:35:43PM +0300, Sergei Antonov wrote: >> On Fri, 12 Aug 2022 at 19:13, Andrew Lunn <andrew@lunn.ch> wrote: >>>> + /* Inherit the DMA masks from the platform device */ >>>> + ndev->dev.dma_mask = p_dev->dma_mask; >>>> + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask; >>> >>> There is only one other ethernet driver which does this. What you see >>> much more often is: >>> >>> alacritech/slicoss.c: paddr = dma_map_single(&sdev->pdev->dev, skb->data, maplen, >>> neterion/s2io.c: dma_map_single(&sp->pdev->dev, ba->ba_1, >>> dlink/dl2k.c: cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data, >>> micrel/ksz884x.c: dma_buf->dma = dma_map_single(&hw_priv->pdev->dev, skb->data, >> >> Also works. Do you recommend to create a v2 of the patch? > > Yes please. It makes things easier to maintain if every driver does > the same thing. Yes this is a common pattern to store a device reference pointing to &pdev->dev into your network device private structure fetched via netdev_priv(). Alternatively, we could sort of try to settle on a common pattern where we utilize &dev->parent->dev thanks to having called SET_NETDEV_DEV(), that might be more universal?
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c index a3214a762e4b..de99211d85c2 100644 --- a/drivers/net/ethernet/moxa/moxart_ether.c +++ b/drivers/net/ethernet/moxa/moxart_ether.c @@ -537,6 +537,10 @@ static int moxart_mac_probe(struct platform_device *pdev) ndev->priv_flags |= IFF_UNICAST_FLT; ndev->irq = irq; + /* Inherit the DMA masks from the platform device */ + ndev->dev.dma_mask = p_dev->dma_mask; + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask; + SET_NETDEV_DEV(ndev, &pdev->dev); ret = register_netdev(ndev);
dma_map_single() calls fail in moxart_mac_setup_desc_ring() and moxart_mac_start_xmit() which leads to an incessant output of this: [ 16.043925] moxart-ethernet 92000000.mac eth0: DMA mapping error [ 16.050957] moxart-ethernet 92000000.mac eth0: DMA mapping error [ 16.058229] moxart-ethernet 92000000.mac eth0: DMA mapping error To make dma_map_single() work, inherit DMA masks from the platform device. Signed-off-by: Sergei Antonov <saproj@gmail.com> CC: Jakub Kicinski <kuba@kernel.org> CC: Pavel Skripkin <paskripkin@gmail.com> CC: David S. Miller <davem@davemloft.net> CC: Guobin Huang <huangguobin4@huawei.com> CC: Paolo Abeni <pabeni@redhat.com> --- drivers/net/ethernet/moxa/moxart_ether.c | 4 ++++ 1 file changed, 4 insertions(+)