| Message ID | 20210818150943.1630199-1-festevam@denx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: Do not call dma_alloc_coherent() for SDIO and USB | expand |
On August 18, 2021 5:11:10 PM Fabio Estevam <festevam@denx.de> wrote: > When running the "hostapd" application on a i.MX7-based board with > an ath10k device connected via SDIO, the following warning is seen: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 489 at kernel/dma/mapping.c:427 dma_alloc_attrs+0xd0/0x114 > Modules linked in: ath10k_sdio ath10k_core ath > CPU: 0 PID: 489 Comm: hostapd Not tainted 5.10.48-stable-standard #1 > Hardware name: Freescale i.MX7 Dual (Device Tree) > [<c0111378>] (unwind_backtrace) from [<c010bc04>] (show_stack+0x10/0x14) > [<c010bc04>] (show_stack) from [<c0e26094>] (dump_stack+0xdc/0x104) > [<c0e26094>] (dump_stack) from [<c0125574>] (__warn+0xd8/0x114) > [<c0125574>] (__warn) from [<c0e20ecc>] (warn_slowpath_fmt+0x60/0xbc) > [<c0e20ecc>] (warn_slowpath_fmt) from [<c01b9eac>] (dma_alloc_attrs+0xd0/0x114) > [<c01b9eac>] (dma_alloc_attrs) from [<bf01373c>] > (ath10k_add_interface+0x2f0/0x1094 [ath10k_core]) > [<bf01373c>] (ath10k_add_interface [ath10k_core]) from [<c0d94470>] > (drv_add_interface+0x88/0x2fc) > > As explained by Christoph Hellwig: > > "Looking at the ath10k code ar->dev is set by ath10k_core_create, which > has multiple callers. > > For ath10k_pci_probe it is a pci_dev, which should always have a > dma_mask. > > For ath10k_ahb_probe is a device tree probed platform_device, > which should have a dma mask. > > For ath10k_sdio_probe it is a sdio_func, which from my understanding is > a virtual device can't do DMA itself. > > For ath10k_snoc_probe it is a platform device with an explicit > dma_set_mask_and_coherent and above so the dma_mask is set. > > For ath10k_usb_probe it is an usb device which can't do USB." > > Fix the problem by not calling dma_alloc_coherent() when the device > is not DMA capable, such as SDIO and USB. > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Hi, > > I am not certain about the proper commit to include in the Fixes tag. > > Any suggestions? > > Thanks > > drivers/net/wireless/ath/ath10k/mac.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/mac.c > b/drivers/net/wireless/ath/ath10k/mac.c > index c272b290fa73..e85c3f107d2e 100644 > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -5576,15 +5576,17 @@ static int ath10k_add_interface(struct ieee80211_hw > *hw, > if (vif->type == NL80211_IFTYPE_ADHOC || > vif->type == NL80211_IFTYPE_MESH_POINT || > vif->type == NL80211_IFTYPE_AP) { > - arvif->beacon_buf = dma_alloc_coherent(ar->dev, > - IEEE80211_MAX_FRAME_LEN, > - &arvif->beacon_paddr, > - GFP_ATOMIC); > - if (!arvif->beacon_buf) { > - ret = -ENOMEM; > - ath10k_warn(ar, "failed to allocate beacon buffer: %d\n", > - ret); > - goto err; > + if (!(ar->hif.bus == ATH10K_BUS_SDIO) && !(ar->hif.bus == ATH10K_BUS_USB)) { Does this mean you can not really setup a beaconing interface type for SDIO and USB? Regards, Arend
>> Fix the problem by not calling dma_alloc_coherent() when the device >> is not DMA capable, such as SDIO and USB. >> ath10k calls dma_alloc_coherent multiple places including ath10k_htt_rx_alloc. Do SDIO and USB not use such data path function at all? Thanks, Peter
Hi Peter, On Wed, Aug 18, 2021 at 2:35 PM Peter Oh <peter.oh@eero.com> wrote: > > > >> Fix the problem by not calling dma_alloc_coherent() when the device > >> is not DMA capable, such as SDIO and USB. > >> > ath10k calls dma_alloc_coherent multiple places including > ath10k_htt_rx_alloc. > > Do SDIO and USB not use such data path function at all? Good point! Maybe I am not just hitting these paths on my simple tests. Kalle, How should we handle this? Thanks
Hi Arend, On Wed, Aug 18, 2021 at 1:51 PM Arend van Spriel <aspriel@gmail.com> wrote: > Does this mean you can not really setup a beaconing interface type for SDIO > and USB? As per the debug message below: ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vdev create %d (add interface) type %d subtype %d bcnmode %s\n", arvif->vdev_id, arvif->vdev_type, arvif->vdev_subtype, arvif->beacon_buf ? "single-buf" : "per-skb"); If arvif->beacon_buf is NULL then it will be a "per-skb" beacon mode. Thanks
Hi Peter, On Wed, Aug 18, 2021 at 2:35 PM Peter Oh <peter.oh@eero.com> wrote: > > > >> Fix the problem by not calling dma_alloc_coherent() when the device > >> is not DMA capable, such as SDIO and USB. > >> > ath10k calls dma_alloc_coherent multiple places including > ath10k_htt_rx_alloc. > > Do SDIO and USB not use such data path function at all? Now that I look closer: no, SDIO and USB do not reach the dma_alloc_coherent() inside ath10k_htt_rx_alloc. drivers/net/wireless/ath/ath10k/usb.c and drivers/net/wireless/ath/ath10k/sdio.c both set: bus_params.dev_type = ATH10K_DEV_TYPE_HL; ath10k_htt_rx_alloc() has: if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) return 0; So it returns early in the USB and SDIO cases and does not call dma_alloc_coherent(). Thanks
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index c272b290fa73..e85c3f107d2e 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -5576,15 +5576,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw, if (vif->type == NL80211_IFTYPE_ADHOC || vif->type == NL80211_IFTYPE_MESH_POINT || vif->type == NL80211_IFTYPE_AP) { - arvif->beacon_buf = dma_alloc_coherent(ar->dev, - IEEE80211_MAX_FRAME_LEN, - &arvif->beacon_paddr, - GFP_ATOMIC); - if (!arvif->beacon_buf) { - ret = -ENOMEM; - ath10k_warn(ar, "failed to allocate beacon buffer: %d\n", - ret); - goto err; + if (!(ar->hif.bus == ATH10K_BUS_SDIO) && !(ar->hif.bus == ATH10K_BUS_USB)) { + arvif->beacon_buf = dma_alloc_coherent(ar->dev, + IEEE80211_MAX_FRAME_LEN, + &arvif->beacon_paddr, + GFP_ATOMIC); + if (!arvif->beacon_buf) { + ret = -ENOMEM; + ath10k_warn(ar, "failed to allocate beacon buffer: %d\n", + ret); + goto err; + } } } if (test_bit(ATH10K_FLAG_HW_CRYPTO_DISABLED, &ar->dev_flags))
When running the "hostapd" application on a i.MX7-based board with an ath10k device connected via SDIO, the following warning is seen: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 489 at kernel/dma/mapping.c:427 dma_alloc_attrs+0xd0/0x114 Modules linked in: ath10k_sdio ath10k_core ath CPU: 0 PID: 489 Comm: hostapd Not tainted 5.10.48-stable-standard #1 Hardware name: Freescale i.MX7 Dual (Device Tree) [<c0111378>] (unwind_backtrace) from [<c010bc04>] (show_stack+0x10/0x14) [<c010bc04>] (show_stack) from [<c0e26094>] (dump_stack+0xdc/0x104) [<c0e26094>] (dump_stack) from [<c0125574>] (__warn+0xd8/0x114) [<c0125574>] (__warn) from [<c0e20ecc>] (warn_slowpath_fmt+0x60/0xbc) [<c0e20ecc>] (warn_slowpath_fmt) from [<c01b9eac>] (dma_alloc_attrs+0xd0/0x114) [<c01b9eac>] (dma_alloc_attrs) from [<bf01373c>] (ath10k_add_interface+0x2f0/0x1094 [ath10k_core]) [<bf01373c>] (ath10k_add_interface [ath10k_core]) from [<c0d94470>] (drv_add_interface+0x88/0x2fc) As explained by Christoph Hellwig: "Looking at the ath10k code ar->dev is set by ath10k_core_create, which has multiple callers. For ath10k_pci_probe it is a pci_dev, which should always have a dma_mask. For ath10k_ahb_probe is a device tree probed platform_device, which should have a dma mask. For ath10k_sdio_probe it is a sdio_func, which from my understanding is a virtual device can't do DMA itself. For ath10k_snoc_probe it is a platform device with an explicit dma_set_mask_and_coherent and above so the dma_mask is set. For ath10k_usb_probe it is an usb device which can't do USB." Fix the problem by not calling dma_alloc_coherent() when the device is not DMA capable, such as SDIO and USB. Signed-off-by: Fabio Estevam <festevam@denx.de> --- Hi, I am not certain about the proper commit to include in the Fixes tag. Any suggestions? Thanks drivers/net/wireless/ath/ath10k/mac.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)