[PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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)) {
+			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))
-- 
2.25.1

[PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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)) {
+			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))
-- 
2.25.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Arend van Spriel]

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

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Arend van Spriel]

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



_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Peter Oh]

>> 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

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Peter Oh]

>> 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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

Re: [PATCH] ath10k: Do not call dma_alloc_coherent() for SDIO and USB

[Fabio Estevam]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k