* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-11 6:05 UTC (permalink / raw)
To: Larry Finger
Cc: Christoph Hellwig, Aaro Koskinen, Christian Zigotzky,
Michael Ellerman, linux-kernel, linux-wireless, linuxppc-dev
In-Reply-To: <153c13f5-a829-1eab-a3c5-fecfb84127ff@lwfinger.net>
On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>>> return -EIO;
>>>
>>> For b43legacy, dev->dma_mask is 0xc265684800000000.
>>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
>>> the routine returns -EIO.
>>>
>>> For b43, dev->dma_mask is 0xc265684800000001,
>>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
>>> the routine returns 0.
>>
>> I don't fully understand what values the above map to. Can you send
>> me your actual debugging patch as well?
>
> I do not understand why the if statement returns true as neither of the
> values is zero. After seeing the x86 output shown below, I also do not
> understand all the trailing zeros.
>
> My entire patch is attached. That output came from this section:
What might be confusing in your output is that dev->dma_mask is a pointer,
and we are setting it in dma_set_mask. That is before we only check
if the pointer is set, and later we override it. Of course this doesn't
actually explain the failure. But what is even more strange to me
is that you get a return value from dma_supported() that isn't 0 or 1,
as that function is supposed to return a boolean, and I really can't see
how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
or 1. Does the output change if you use the correct printk specifiers?
i.e. with a debug patch like this:
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..9e5b30b12b10 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
int dma_direct_supported(struct device *dev, u64 mask)
{
u64 min_mask;
+ bool ret;
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
@@ -391,7 +392,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
- return mask >= __phys_to_dma(dev, min_mask);
+ ret = (mask >= __phys_to_dma(dev, min_mask));
+ if (!ret)
+ dev_info(dev,
+ "%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
+ __func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
+ return ret;
}
size_t dma_direct_max_mapping_size(struct device *dev)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..6c57ccdee2ae 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
int dma_set_mask(struct device *dev, u64 mask)
{
- if (!dev->dma_mask || !dma_supported(dev, mask))
+ if (!dev->dma_mask) {
+ dev_info(dev, "no DMA mask set!\n");
return -EIO;
+ }
+ if (!dma_supported(dev, mask)) {
+ printk("DMA not supported\n");
+ return -EIO;
+ }
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
^ permalink raw reply related
* Re: ath10k QCA9377 firmware crashes and fails to recover
From: Daniel Drake @ 2019-06-11 6:01 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k, linux-wireless, Endless Linux Upstreaming Team
In-Reply-To: <87h89lei7e.fsf@kamboji.qca.qualcomm.com>
Hi Kalle,
On Thu, May 23, 2019 at 3:36 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Daniel Drake <drake@endlessm.com> writes:
>
> > We are experiencing failures with QCA9377 wifi, using Linux 4.18 and
> > Linux 5.0 with the latest firmware version:
> >
> > ath10k_pci 0000:02:00.0: firmware crashed! (guid
> > 54a4649a-1240-4459-9442-9d498c49de79)
> > ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
> > 0x003821ff sub 1a3b:2b31
> > ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
> > ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00002-QCATFSWPZ-5
> > api 5 features ignore-otp crc32 c3e0d04f
>
> Is this a regression? For example, have you tried older firmware
> versions?
Sorry for the delayed response, as we were testing old versions.
It doesn't seem to be a regression, at least we tested:
Linux 5.0 / latest firmware API 6
ath10k_pci 0000:02:00.0: firmware crashed! (guid
697a3b62-bf3a-4953-bf3d-058eb3b828ff)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.2.1-00021-QCARMSWP-1 api
6 features wowlan,ignore-otp crc32 42e41877
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.56 wmi-op 4 htt-op 3 cal otp
max-sta 32 raw 0 hwcrypto 1
Linux 4.18 / latest firmware API 5
ath10k_pci 0000:02:00.0: firmware crashed! (guid
54a4649a-1240-4459-9442-9d498c49de79)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00002-QCATFSWPZ-5
api 5 features ignore-otp crc32 c3e0d04f
Linux 4.15 / older firmware
ath10k_pci 0000:02:00.0: firmware crashed! (guid
7e1505fa-49e1-4fab-a7c5-a2352f1a47f6)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00267-1 api 5
features ignore-otp crc32 79cea2c7
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.1 wmi-op 4 htt-op 3 cal otp max-sta
32 raw 0 hwcrypto 1
Linux 4.13 / same older firmware
ath10k_pci 0000:02:00.0: firmware crashed! (uuid
701e7d5e-b405-408c-ae27-7de285c38c8f)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00267-1 api 5
features ignore-otp crc32 79cea2c7
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.1 wmi-op 4 htt-op 3 cal otp max-sta
32 raw 0 hwcrypto 1
Any further suggestions?
Thanks
Daniel
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-11 5:56 UTC (permalink / raw)
To: Larry Finger, Aaro Koskinen, Christoph Hellwig,
Christian Zigotzky, Michael Ellerman
Cc: linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <3ed1ccfe-d7ca-11b9-17b3-303d1ae1bb0f@lwfinger.net>
On Mon, 2019-06-10 at 13:44 -0500, Larry Finger wrote:
> On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> >
> > > Please try the attached patch. I'm not really pleased with it and I will
> > > continue to determine why the fallback to a 30-bit mask fails, but at least this
> > > one works for me.
> >
> > Your patch only makes sense if the device is indeed capable of
> > addressing 31-bits.
> >
> > So either the driver is buggy and asks for a too small mask in which
> > case your patch is ok, or it's not and you're just going to cause all
> > sort of interesting random problems including possible memory
> > corruption.
>
> Of course the driver may be buggy, but it asks for the correct mask.
>
> This particular device is not capable of handling 32-bit DMA. The driver detects
> the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32
> until 5.1. As Christoph said, it should always be possible to use fewer bits
> than the maximum.
No, I don't think it *worked* on ppc32 before Christoph patch. I think
it "mostly sort-of worked" :-)
The reason I'm saying that is if your system has more than 1GB of RAM,
then you'll have chunks of memory that the device simply cannot
address.
Before Christoph patches, we had no ZONE_DMA or ZONE_DMA32 covering the
30-bit limited space, so any memory allocation could in theory land
above 30-bits, causing all sort of horrible things to happen with that
driver.
The reason I think it sort-of-mostly-worked is that to get more than
1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
buffers aren't allocated in Highmem.... so you got lucky.
That said, there is such as thing as no-copy send on network, so I
wouldn't be surprised if some things would still have failed, just not
frequent enough for you to notice.
> Similar devices that are new enough to use b43 rather than b43legacy work with
> new kernels; however, they have and use 32-bit DMA.
Cheres,
Ben.
^ permalink raw reply
* Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: H Buus @ 2019-06-11 4:56 UTC (permalink / raw)
To: Larry Finger, Michael Büsch, Kalle Valo; +Cc: linux-wireless
In-Reply-To: <de857b70-fbc2-9c29-b31e-d544a33c8ced@lwfinger.net>
On 6/10/2019 3:16 PM, Larry Finger wrote:
> On 6/10/19 1:49 PM, Michael Büsch wrote:
>> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
>> The warning serves no purpose. So let's just remove it.
>>
>> Reported-by: H Buus <ubuntu@hbuus.com>
>> Signed-off-by: Michael Büsch <m@bues.ch>
>>
>> ---
>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
>
> Larry
Works for me. Thanks!
>>
>> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
>> index e809dae4c470..66a76fd83248 100644
>> --- a/drivers/ssb/driver_gpio.c
>> +++ b/drivers/ssb/driver_gpio.c
>> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
>> return ssb_gpio_chipco_init(bus);
>> else if (ssb_extif_available(&bus->extif))
>> return ssb_gpio_extif_init(bus);
>> - else
>> - WARN_ON(1);
>> -
>> return -1;
>> }
>> @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
>> ssb_extif_available(&bus->extif)) {
>> gpiochip_remove(&bus->gpio);
>> return 0;
>> - } else {
>> - WARN_ON(1);
>> }
>> -
>> return -1;
>> }
>>
>
^ permalink raw reply
* [PATCH] cfg80211: fix memory leak of wiphy device name
From: Eric Biggers @ 2019-06-10 20:02 UTC (permalink / raw)
To: linux-wireless, Johannes Berg
Cc: gregkh, linux-kernel, rafael, syzkaller-bugs
In-Reply-To: <00000000000026f98d058a0944ed@google.com>
From: Eric Biggers <ebiggers@google.com>
In wiphy_new_nm(), if an error occurs after dev_set_name() and
device_initialize() have already been called, it's necessary to call
put_device() (via wiphy_free()) to avoid a memory leak.
Reported-by: syzbot+7fddca22578bc67c3fe4@syzkaller.appspotmail.com
Fixes: 1f87f7d3a3b4 ("cfg80211: add rfkill support")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
net/wireless/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 037816163e70d..458f5e0906875 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -514,7 +514,7 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
&rdev->rfkill_ops, rdev);
if (!rdev->rfkill) {
- kfree(rdev);
+ wiphy_free(&rdev->wiphy);
return NULL;
}
--
2.22.0.rc2.383.gf4fbbf30c2-goog
^ permalink raw reply related
* Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: Larry Finger @ 2019-06-10 19:16 UTC (permalink / raw)
To: Michael Büsch, Kalle Valo; +Cc: H Buus, linux-wireless
In-Reply-To: <20190610204927.2de21c9a@wiggum>
On 6/10/19 1:49 PM, Michael Büsch wrote:
> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> The warning serves no purpose. So let's just remove it.
>
> Reported-by: H Buus <ubuntu@hbuus.com>
> Signed-off-by: Michael Büsch <m@bues.ch>
>
> ---
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Larry
>
> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
> index e809dae4c470..66a76fd83248 100644
> --- a/drivers/ssb/driver_gpio.c
> +++ b/drivers/ssb/driver_gpio.c
> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
> return ssb_gpio_chipco_init(bus);
> else if (ssb_extif_available(&bus->extif))
> return ssb_gpio_extif_init(bus);
> - else
> - WARN_ON(1);
> -
> return -1;
> }
>
> @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
> ssb_extif_available(&bus->extif)) {
> gpiochip_remove(&bus->gpio);
> return 0;
> - } else {
> - WARN_ON(1);
> }
> -
> return -1;
> }
>
^ permalink raw reply
* [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: Michael Büsch @ 2019-06-10 18:49 UTC (permalink / raw)
To: Kalle Valo; +Cc: H Buus, Larry Finger, linux-wireless
In-Reply-To: <946c86bf-7e90-a981-b9fc-757adb98adfa@hbuus.com>
[-- Attachment #1: Type: text/plain, Size: 840 bytes --]
The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
The warning serves no purpose. So let's just remove it.
Reported-by: H Buus <ubuntu@hbuus.com>
Signed-off-by: Michael Büsch <m@bues.ch>
---
diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
index e809dae4c470..66a76fd83248 100644
--- a/drivers/ssb/driver_gpio.c
+++ b/drivers/ssb/driver_gpio.c
@@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
return ssb_gpio_chipco_init(bus);
else if (ssb_extif_available(&bus->extif))
return ssb_gpio_extif_init(bus);
- else
- WARN_ON(1);
-
return -1;
}
@@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
ssb_extif_available(&bus->extif)) {
gpiochip_remove(&bus->gpio);
return 0;
- } else {
- WARN_ON(1);
}
-
return -1;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-10 18:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Aaro Koskinen, Christoph Hellwig,
Christian Zigotzky, Michael Ellerman
Cc: linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <7697a9d10777b28ae79fdffdde6d0985555f6310.camel@kernel.crashing.org>
On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
>
>> Please try the attached patch. I'm not really pleased with it and I will
>> continue to determine why the fallback to a 30-bit mask fails, but at least this
>> one works for me.
>
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
>
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.
Of course the driver may be buggy, but it asks for the correct mask.
This particular device is not capable of handling 32-bit DMA. The driver detects
the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32
until 5.1. As Christoph said, it should always be possible to use fewer bits
than the maximum.
Similar devices that are new enough to use b43 rather than b43legacy work with
new kernels; however, they have and use 32-bit DMA.
Larry
^ permalink raw reply
* Re: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Doug Anderson @ 2019-06-10 16:50 UTC (permalink / raw)
To: Hunter, Adrian
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel,
brcm80211-dev-list.pdl@broadcom.com,
linux-rockchip@lists.infradead.org, Double Lo,
briannorris@chromium.org, linux-wireless@vger.kernel.org,
Naveen Gupta, Madhan Mohan R, mka@chromium.org, Wright Feng,
Chi-Hsien Lin, netdev@vger.kernel.org,
brcm80211-dev-list@cypress.com, Franky Lin,
linux-kernel@vger.kernel.org, Hante Meuleman, YueHaibing,
David S. Miller
In-Reply-To: <363DA0ED52042842948283D2FC38E4649C52F8A0@IRSMSX106.ger.corp.intel.com>
Hi,
On Mon, Jun 10, 2019 at 1:56 AM Hunter, Adrian <adrian.hunter@intel.com> wrote:
>
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > @@ -16,6 +16,7 @@
> > #include <linux/mmc/sdio_ids.h>
> > #include <linux/mmc/sdio_func.h>
> > #include <linux/mmc/card.h>
> > +#include <linux/mmc/core.h>
>
> SDIO function drivers should not really include linux/mmc/core.h
> (Also don't know why linux/mmc/card.h is included)
OK, so I guess you're requesting an extra level of "sdio_" wrappers
for all the functions I need to call. I don't think the wrappers buy
us a ton other than to abstract things a little bit and make it look
prettier. :-) ...but certainly I can code that up if that's what
everyone wants.
Just to make sure, I looked in "drivers/net/wireless/" and I do see
quite a few instances of "mmc_" functions being used. That doesn't
mean all these instances are correct but it does appear to be
commonplace. Selected examples:
drivers/net/wireless/ath/ath10k/sdio.c:
ret = mmc_hw_reset(ar_sdio->func->card->host);
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:
mmc_set_data_timeout(md, func->card);
mmc_wait_for_req(func->card->host, mr);
drivers/net/wireless/marvell/mwifiex/sdio.c:
mmc_hw_reset(func->card->host);
drivers/net/wireless/rsi/rsi_91x_sdio.c:
err = mmc_wait_for_cmd(host, &cmd, 3);
...anyway, I'll give it a few days and if nobody else chimes in then
I'll assume you indeed want "sdio_" wrappers for things and I'll post
a v4. If patch #1 happens to land in the meantime then I won't
object. ;-)
-Doug
^ permalink raw reply
* Re: [PATCH][next] qtnfmac: Use struct_size() in kzalloc()
From: Sergey Matyukevich @ 2019-06-10 16:14 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich, Kalle Valo,
David S. Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190607191745.GA19120@embeddedor>
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct ieee80211_regdomain {
> ...
> struct ieee80211_reg_rule reg_rules[];
> };
>
> instance = kzalloc(sizeof(*mac->rd) +
> sizeof(struct ieee80211_reg_rule) *
> count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, reg_rules, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Hi Gustavo,
Thanks for the patch !
Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Regards,
Sergey
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-10 16:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Aaro Koskinen, Christian Zigotzky, Michael Ellerman, linux-kernel,
linux-wireless, linuxppc-dev
In-Reply-To: <20190610081825.GA16534@lst.de>
[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]
On 6/10/19 3:18 AM, Christoph Hellwig wrote:
> On Sat, Jun 08, 2019 at 04:52:24PM -0500, Larry Finger wrote:
>> On 6/7/19 12:29 PM, Christoph Hellwig wrote:
>>> I don't think we should work around this in the driver, we need to fix
>>> it in the core. I'm curious why my previous patch didn't work. Can
>>> you throw in a few printks what failed? I.e. did dma_direct_supported
>>> return false? Did the actual allocation fail?
>>
>> Routine dma_direct_supported() returns true.
>>
>> The failure is in routine dma_set_mask() in the following if test:
>>
>> if (!dev->dma_mask || !dma_supported(dev, mask))
>> return -EIO;
>>
>> For b43legacy, dev->dma_mask is 0xc265684800000000.
>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
>> the routine returns -EIO.
>>
>> For b43, dev->dma_mask is 0xc265684800000001,
>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
>> the routine returns 0.
>
> I don't fully understand what values the above map to. Can you send
> me your actual debugging patch as well?
I do not understand why the if statement returns true as neither of the values
is zero. After seeing the x86 output shown below, I also do not understand all
the trailing zeros.
My entire patch is attached. That output came from this section:
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask,
dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
On a 32-bit x86 computer with 1GB of RAM, that same output was
For b43legacy, dev->dma_mask is 0x01f4029044.
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0x3fffffff, and
the routine returns 0. 30-bit DMA works.
For b43, dev->dma_mask is 0x01f4029044,
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0xffffffff, and
the routine also returns 0. This card supports 32-bit DMA.
Larry
[-- Attachment #2: b43legacy_tests --]
[-- Type: text/plain, Size: 4133 bytes --]
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2..7a367ce 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ extern void copy_user_page(void *to, void *from, unsigned long vaddr,
#endif /* __ASSEMBLY__ */
#include <asm/slice.h>
+#if 1 /* XXX: pmac? dynamic discovery? */
+#define ARCH_ZONE_DMA_BITS 30
+#else
#define ARCH_ZONE_DMA_BITS 31
+#endif
#endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 09231ef..761d951 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -20,6 +20,8 @@
*/
static inline bool dma_iommu_alloc_bypass(struct device *dev)
{
+ pr_info("dev->archdata.iommu_bypass %d, !iommu_fixed_is_weak %d\n",
+ dev->archdata.iommu_bypass, !iommu_fixed_is_weak)
return dev->archdata.iommu_bypass && !iommu_fixed_is_weak &&
dma_direct_supported(dev, dev->coherent_dma_mask);
}
@@ -27,6 +29,8 @@ static inline bool dma_iommu_alloc_bypass(struct device *dev)
static inline bool dma_iommu_map_bypass(struct device *dev,
unsigned long attrs)
{
+ pr_info("(attrs & DMA_ATTR_WEAK_ORDERING) %d\n",
+ (attrs & DMA_ATTR_WEAK_ORDERING));
return dev->archdata.iommu_bypass &&
(!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index cba2913..2540d3b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -248,7 +248,8 @@ void __init paging_init(void)
(long int)((top_of_ram - total_ram) >> 20));
#ifdef CONFIG_ZONE_DMA
- max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
+ max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
+ ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
#endif
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
#ifdef CONFIG_HIGHMEM
diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 806406a..e0270da 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1053,6 +1053,7 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f4..c625ffc 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -794,6 +794,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e..b716e62 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -391,6 +391,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
+ pr_info("min_mask 0x%x. max_pfn 0x%x, __phys_to_dma 0x%x, mask 0x%x\n", min_mask,
+ max_pfn, __phys_to_dma(dev, min_mask), mask);
return mask >= __phys_to_dma(dev, min_mask);
}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask, dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
^ permalink raw reply related
* Re: [PATCH v2] carl9170: fix misuse of device driver API
From: Alan Stern @ 2019-06-10 14:12 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, linux-usb, Kalle Valo
In-Reply-To: <20190608144947.744-3-chunkeey@gmail.com>
On Sat, 8 Jun 2019, Christian Lamparter wrote:
> This patch follows Alan Stern's recent patch:
> "p54: Fix race between disconnect and firmware loading"
>
> that overhauled carl9170 buggy firmware loading and driver
> unbinding procedures.
>
> Since the carl9170 code was adapted from p54 it uses the
> same functions and is likely to have the same problem, but
> it's just that the syzbot hasn't reproduce them (yet).
>
> a summary from the changes (copied from the p54 patch):
> * Call usb_driver_release_interface() rather than
> device_release_driver().
>
> * Lock udev (the interface's parent) before unbinding the
> driver instead of locking udev->parent.
>
> * During the firmware loading process, take a reference
> to the USB interface instead of the USB device.
>
> * Don't take an unnecessary reference to the device during
> probe (and then don't drop it during disconnect).
>
> and
>
> * Make sure to prevent use-after-free bugs by explicitly
> setting the driver context to NULL after signaling the
> completion.
>
> Cc: <stable@vger.kernel.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> v2: Alan Stern's comments
> - fixed possible use-after-free
> ---
Acked-by: Alan Stern <stern@rowland.harvard.edu>
> drivers/net/wireless/ath/carl9170/usb.c | 39 +++++++++++--------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
> index e7c3f3b8457d..99f1897a775d 100644
> --- a/drivers/net/wireless/ath/carl9170/usb.c
> +++ b/drivers/net/wireless/ath/carl9170/usb.c
> @@ -128,6 +128,8 @@ static const struct usb_device_id carl9170_usb_ids[] = {
> };
> MODULE_DEVICE_TABLE(usb, carl9170_usb_ids);
>
> +static struct usb_driver carl9170_driver;
> +
> static void carl9170_usb_submit_data_urb(struct ar9170 *ar)
> {
> struct urb *urb;
> @@ -966,32 +968,28 @@ static int carl9170_usb_init_device(struct ar9170 *ar)
>
> static void carl9170_usb_firmware_failed(struct ar9170 *ar)
> {
> - struct device *parent = ar->udev->dev.parent;
> - struct usb_device *udev;
> -
> - /*
> - * Store a copy of the usb_device pointer locally.
> - * This is because device_release_driver initiates
> - * carl9170_usb_disconnect, which in turn frees our
> - * driver context (ar).
> + /* Store a copies of the usb_interface and usb_device pointer locally.
> + * This is because release_driver initiates carl9170_usb_disconnect,
> + * which in turn frees our driver context (ar).
> */
> - udev = ar->udev;
> + struct usb_interface *intf = ar->intf;
> + struct usb_device *udev = ar->udev;
>
> complete(&ar->fw_load_wait);
> + /* at this point 'ar' could be already freed. Don't use it anymore */
> + ar = NULL;
>
> /* unbind anything failed */
> - if (parent)
> - device_lock(parent);
> -
> - device_release_driver(&udev->dev);
> - if (parent)
> - device_unlock(parent);
> + usb_lock_device(udev);
> + usb_driver_release_interface(&carl9170_driver, intf);
> + usb_unlock_device(udev);
>
> - usb_put_dev(udev);
> + usb_put_intf(intf);
> }
>
> static void carl9170_usb_firmware_finish(struct ar9170 *ar)
> {
> + struct usb_interface *intf = ar->intf;
> int err;
>
> err = carl9170_parse_firmware(ar);
> @@ -1009,7 +1007,7 @@ static void carl9170_usb_firmware_finish(struct ar9170 *ar)
> goto err_unrx;
>
> complete(&ar->fw_load_wait);
> - usb_put_dev(ar->udev);
> + usb_put_intf(intf);
> return;
>
> err_unrx:
> @@ -1052,7 +1050,6 @@ static int carl9170_usb_probe(struct usb_interface *intf,
> return PTR_ERR(ar);
>
> udev = interface_to_usbdev(intf);
> - usb_get_dev(udev);
> ar->udev = udev;
> ar->intf = intf;
> ar->features = id->driver_info;
> @@ -1094,15 +1091,14 @@ static int carl9170_usb_probe(struct usb_interface *intf,
> atomic_set(&ar->rx_anch_urbs, 0);
> atomic_set(&ar->rx_pool_urbs, 0);
>
> - usb_get_dev(ar->udev);
> + usb_get_intf(intf);
>
> carl9170_set_state(ar, CARL9170_STOPPED);
>
> err = request_firmware_nowait(THIS_MODULE, 1, CARL9170FW_NAME,
> &ar->udev->dev, GFP_KERNEL, ar, carl9170_usb_firmware_step2);
> if (err) {
> - usb_put_dev(udev);
> - usb_put_dev(udev);
> + usb_put_intf(intf);
> carl9170_free(ar);
> }
> return err;
> @@ -1131,7 +1127,6 @@ static void carl9170_usb_disconnect(struct usb_interface *intf)
>
> carl9170_release_firmware(ar);
> carl9170_free(ar);
> - usb_put_dev(udev);
> }
>
> #ifdef CONFIG_PM
>
^ permalink raw reply
* Re: [PATCH] carl9170: fix enum compare splat
From: Christian Lamparter @ 2019-06-10 11:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <87pnnlncll.fsf@codeaurora.org>
On Monday, June 10, 2019 9:06:30 AM CEST Kalle Valo wrote:
> Christian Lamparter <chunkeey@gmail.com> writes:
>
> > This patch fixes a noisy warning triggered by -Wenum-compare
> >
> > |main.c:1390:31: warning: comparison between ‘enum nl80211_ac’
> > | and ‘enum ar9170_txq’ [-Wenum-compare]
> > | BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> > | ^
> > | [...]
> >
> > This is a little bit unfortunate, since the number of queues
> > (hence NL80211_NUM_ACS) is a constant based on the IEEE 802.11
> > (much like IEEE80211_NUM_ACS) and __AR9170_NUM_TXQ is more or
> > less defined by the AR9170 hardware.
>
> Is the warning enabled by default? TBH I'm not seeing how useful this
> warning is for kernel development.
It is included in the "-Wall" (which is coming from "KBUILD_CFLAGS"
in the main Makefile).
I tried debian's gcc starting from 4.6 to the lastest 8.3. They all
complain about it in various degrees.
https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Warning-Options.html#Warning-Options
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1387,7 +1387,7 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw,
> > int ret;
> >
> > BUILD_BUG_ON(ARRAY_SIZE(ar9170_qmap) != __AR9170_NUM_TXQ);
> > - BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> > + BUILD_BUG_ON((size_t)NL80211_NUM_ACS > (size_t)__AR9170_NUM_TXQ);
>
> IMHO this just makes the code worse. Does it make sense to workaround
> (stupid) compiler warnings like this?
True. What's worse: This isn't really code. The BUILD_BUG_ON Macro is there
to guard but it's getting compiled away. I could also just drop it.
^ permalink raw reply
* Re: [PATCH v2 2/7] net/mac80211: move WEP handling to ARC4 library interface
From: Ard Biesheuvel @ 2019-06-10 10:58 UTC (permalink / raw)
To: Johannes Berg, Herbert Xu
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Eric Biggers, <linux-wireless@vger.kernel.org>,
John W. Linville
In-Reply-To: <107dc7707e6c9d0110aa5535bd5baf4f6db7f6a5.camel@sipsolutions.net>
On Sun, 9 Jun 2019 at 22:09, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Hi Ard,
>
> In general, I have no objections to this.
>
> However, with this
>
> > - select CRYPTO_ARC4
> > + select CRYPTO_LIB_ARC4
>
> and this
>
> > case WLAN_CIPHER_SUITE_WEP40:
> > case WLAN_CIPHER_SUITE_TKIP:
> > case WLAN_CIPHER_SUITE_WEP104:
> > - if (IS_ERR(local->wep_tx_tfm))
> > - return -EINVAL;
> > - break;
>
> there's one quirk that I worry about. Does this mean WEP is now *always*
> available/enabled?
>
> I had to dig in history a bit, but vaguely remembered this commit:
>
> commit 3473187d2459a078e00e5fac8aafc30af69c57fa
> Author: John W. Linville <linville@tuxdriver.com>
> Date: Wed Jul 7 15:07:49 2010 -0400
>
> mac80211: remove wep dependency
>
>
> Since you create the new CRYPTO_LIB_ARC4 in patch 1, I wonder if
> something is broken? I can't really seem to figure out how WEP is
> disallowed in FIPS mode to start with though.
>
>
> (This also is the reason for all the code that removes WEP/TKIP from the
> list of permitted cipher suites when ARC4 isn't available...)
>
Good point. And in the future, there may be other reasons besides FIPS
compliance to turn off WEP support, so it makes sense to retain this
logic.
I am still curious whether FIPS compliance actually requires us to do
this, or whether that code is simply there to work around the lack of
RC4 in the crypto API when FIPS support happens to be enabled.
^ permalink raw reply
* Re: [RFC PATCH v4] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-06-10 10:29 UTC (permalink / raw)
To: Jes Sorensen
Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
Linux Upstreaming Team
In-Reply-To: <CAB4CAwf3Mi2iuR7nAj1U4EoyU5ZnvY9xoLrv7QT2X-tc_1ex3g@mail.gmail.com>
On Wed, Jun 5, 2019 at 10:17 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> On Tue, Jun 4, 2019 at 3:21 AM Jes Sorensen <jes.sorensen@gmail.com> wrote:
> >
> > On 5/31/19 5:12 AM, Chris Chiu wrote:
> > > We have 3 laptops which connect the wifi by the same RTL8723BU.
> > > The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
> > > They have the same problem with the in-kernel rtl8xxxu driver, the
> > > iperf (as a client to an ethernet-connected server) gets ~1Mbps.
> > > Nevertheless, the signal strength is reported as around -40dBm,
> > > which is quite good. From the wireshark capture, the tx rate for each
> > > data and qos data packet is only 1Mbps. Compare to the Realtek driver
> > > at https://github.com/lwfinger/rtl8723bu, the same iperf test gets
> > > ~12Mbps or better. The signal strength is reported similarly around
> > > -40dBm. That's why we want to improve.
> > >
> > > After reading the source code of the rtl8xxxu driver and Realtek's, the
> > > major difference is that Realtek's driver has a watchdog which will keep
> > > monitoring the signal quality and updating the rate mask just like the
> > > rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
> > > And this kind of watchdog also exists in rtlwifi driver of some specific
> > > chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
> > > the same member function named dm_watchdog and will invoke the
> > > corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
> > > mask.
> > >
> > > With this commit, the tx rate of each data and qos data packet will
> > > be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> > > to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> > > the lowest rate from the rate mask and explains why the tx rate of
> > > data and qos data is always lowest 1Mbps because the default rate mask
> > > passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> > > and MCS rate. However, with Realtek's driver, the tx rate observed from
> > > wireshark under the same condition is almost 65Mbps or 72Mbps.
> > >
> > > I believe the firmware of RTL8723BU may need fix. And I think we
> > > can still bring in the dm_watchdog as rtlwifi to improve from the
> > > driver side. Please leave precious comments for my commits and
> > > suggest what I can do better. Or suggest if there's any better idea
> > > to fix this. Thanks.
> > >
> > > Signed-off-by: Chris Chiu <chiu@endlessm.com>
> >
> > I am really pleased to see you're investigating some of these issues,
> > since I've been pretty swamped and not had time to work on this driver
> > for a long time.
> >
> > The firmware should allow for two rate modes, either firmware handled or
> > controlled by the driver. Ideally we would want the driver to handle it,
> > but I never was able to make that work reliable.
> >
> > This fix should at least improve the situation, and it may explain some
> > of the performance issues with the 8192eu as well?
> >
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > index 8828baf26e7b..216f603827a8 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > @@ -1195,6 +1195,44 @@ struct rtl8723bu_c2h {
> > >
> > > struct rtl8xxxu_fileops;
> > >
> > > +/*mlme related.*/
> > > +enum wireless_mode {
> > > + WIRELESS_MODE_UNKNOWN = 0,
> > > + /* Sub-Element */
> > > + WIRELESS_MODE_B = BIT(0),
> > > + WIRELESS_MODE_G = BIT(1),
> > > + WIRELESS_MODE_A = BIT(2),
> > > + WIRELESS_MODE_N_24G = BIT(3),
> > > + WIRELESS_MODE_N_5G = BIT(4),
> > > + WIRELESS_AUTO = BIT(5),
> > > + WIRELESS_MODE_AC = BIT(6),
> > > + WIRELESS_MODE_MAX = 0x7F,
> > > +};
> > > +
> > > +/* from rtlwifi/wifi.h */
> > > +enum ratr_table_mode_new {
> > > + RATEID_IDX_BGN_40M_2SS = 0,
> > > + RATEID_IDX_BGN_40M_1SS = 1,
> > > + RATEID_IDX_BGN_20M_2SS_BN = 2,
> > > + RATEID_IDX_BGN_20M_1SS_BN = 3,
> > > + RATEID_IDX_GN_N2SS = 4,
> > > + RATEID_IDX_GN_N1SS = 5,
> > > + RATEID_IDX_BG = 6,
> > > + RATEID_IDX_G = 7,
> > > + RATEID_IDX_B = 8,
> > > + RATEID_IDX_VHT_2SS = 9,
> > > + RATEID_IDX_VHT_1SS = 10,
> > > + RATEID_IDX_MIX1 = 11,
> > > + RATEID_IDX_MIX2 = 12,
> > > + RATEID_IDX_VHT_3SS = 13,
> > > + RATEID_IDX_BGN_3SS = 14,
> > > +};
> > > +
> > > +#define RTL8XXXU_RATR_STA_INIT 0
> > > +#define RTL8XXXU_RATR_STA_HIGH 1
> > > +#define RTL8XXXU_RATR_STA_MID 2
> > > +#define RTL8XXXU_RATR_STA_LOW 3
> > > +
> >
> > > extern struct rtl8xxxu_fileops rtl8192cu_fops;
> > > extern struct rtl8xxxu_fileops rtl8192eu_fops;
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > index 26b674aca125..2071ab9fd001 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > @@ -1645,6 +1645,148 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
> > > rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
> > > }
> > >
> > > +static u8 rtl8723b_signal_to_rssi(int signal)
> > > +{
> > > + if (signal < -95)
> > > + signal = -95;
> > > + return (u8)(signal + 95);
> > > +}
> >
> > Could you make this more generic so it can be used by the other sub-drivers?
> >
> Sure. I'll do that.
>
> > > +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> > > + int signal, struct ieee80211_sta *sta)
> > > +{
> > > + struct ieee80211_hw *hw = priv->hw;
> > > + u16 wireless_mode;
> > > + u8 rssi_level, ratr_idx;
> > > + u8 txbw_40mhz;
> > > + u8 rssi, rssi_thresh_high, rssi_thresh_low;
> > > +
> > > + rssi_level = priv->rssi_level;
> > > + rssi = rtl8723b_signal_to_rssi(signal);
> > > + txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40) ? 1 : 0;
> > > +
> > > + switch (rssi_level) {
> > > + case RTL8XXXU_RATR_STA_HIGH:
> > > + rssi_thresh_high = 50;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + case RTL8XXXU_RATR_STA_MID:
> > > + rssi_thresh_high = 55;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + case RTL8XXXU_RATR_STA_LOW:
> > > + rssi_thresh_high = 60;
> > > + rssi_thresh_low = 25;
> > > + break;
> > > + default:
> > > + rssi_thresh_high = 50;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + }
> >
> > Can we make this use defined values with some explanation rather than
> > hard coded values?
> >
>
> I also thought about this. So I refer to the same refresh_rateadaotive_mask
> in rtlwifi/rtl8192se/dm.c, rtlwifi/rtl8723ae/dm.c, and rtl8188ee...etc. They
> don't give a better explanation. And I also don't know if these values can be
> generally applied to other subdrivers or specifically for 8723b series, for
> example, the rtl8192se use different values for the threshold. It maybe due
> to different noise floor for different chip? I'm not sure. I took these values
> from vendor driver and rtl8188ee. I can simply use defined values to replace
> but I have to admit it's hard to find a good explanation.
>
> > > + if (rssi > rssi_thresh_high)
> > > + rssi_level = RTL8XXXU_RATR_STA_HIGH;
> > > + else if (rssi > rssi_thresh_low)
> > > + rssi_level = RTL8XXXU_RATR_STA_MID;
> > > + else
> > > + rssi_level = RTL8XXXU_RATR_STA_LOW;
> > > +
> > > + if (rssi_level != priv->rssi_level) {
> > > + int sgi = 0;
> > > + u32 rate_bitmap = 0;
> > > +
> > > + rcu_read_lock();
> > > + rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > > + (sta->ht_cap.mcs.rx_mask[1] << 20);
> > > + if (sta->ht_cap.cap &
> > > + (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> > > + sgi = 1;
> > > + rcu_read_unlock();
> > > +
> > > + wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> > > + switch (wireless_mode) {
> > > + case WIRELESS_MODE_B:
> > > + ratr_idx = RATEID_IDX_B;
> > > + if (rate_bitmap & 0x0000000c)
> > > + rate_bitmap &= 0x0000000d;
> > > + else
> > > + rate_bitmap &= 0x0000000f;
> > > + break;
> > > + case WIRELESS_MODE_A:
> > > + case WIRELESS_MODE_G:
> > > + ratr_idx = RATEID_IDX_G;
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > > + rate_bitmap &= 0x00000f00;
> > > + else
> > > + rate_bitmap &= 0x00000ff0;
> > > + break;
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_G):
> > > + ratr_idx = RATEID_IDX_BG;
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > > + rate_bitmap &= 0x00000f00;
> > > + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > > + rate_bitmap &= 0x00000ff0;
> > > + else
> > > + rate_bitmap &= 0x00000ff5;
> > > + break;
> >
> > It would be nice as well to get all these masks into generic names.
> >
>
> I also take these mask values from the update_hal_rate_mask of the
> vendor driver and other realtek drivers under rtlwifi. I thought about to
> define the lower 12 bits like RTL8XXXU_BG_RATE_MASK, 13~20 bits
> as RTL8XXXU_MCS0_7_RATE_MASK. But it's still hard to express
> all the combinations here. So I just leave it as it is. I can try to add
> explanations for the rate mapping of each bit. It would be a lot easier.
>
> > > + case WIRELESS_MODE_N_24G:
> > > + case WIRELESS_MODE_N_5G:
> > > + case (WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > > + case (WIRELESS_MODE_A | WIRELESS_MODE_N_5G):
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_GN_N2SS;
> > > + else
> > > + ratr_idx = RATEID_IDX_GN_N1SS;
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_N_24G):
> > > + if (txbw_40mhz) {
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > > + else
> > > + ratr_idx = RATEID_IDX_BGN_40M_1SS;
> > > + } else {
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_BGN_20M_2SS_BN;
> > > + else
> > > + ratr_idx = RATEID_IDX_BGN_20M_1SS_BN;
> > > + }
> > > +
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > > + rate_bitmap &= 0x0f8f0000;
> > > + } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > > + rate_bitmap &= 0x0f8ff000;
> > > + } else {
> > > + if (txbw_40mhz)
> > > + rate_bitmap &= 0x0f8ff015;
> > > + else
> > > + rate_bitmap &= 0x0f8ff005;
> > > + }
> > > + } else {
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > > + rate_bitmap &= 0x000f0000;
> > > + } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > > + rate_bitmap &= 0x000ff000;
> > > + } else {
> > > + if (txbw_40mhz)
> > > + rate_bitmap &= 0x000ff015;
> > > + else
> > > + rate_bitmap &= 0x000ff005;
> > > + }
> > > + }
> > > + break;
> > > + default:
> > > + ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > > + rate_bitmap &= 0x0fffffff;
> > > + break;
> > > + }
> > > +
> > > + priv->rssi_level = rssi_level;
> > > + priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi);
> > > + }
> > > +}
> > > +
> >
> > In general I think all of this should be fairly generic and the other
> > subdrivers should be able to benefit from it?
> >
> >
> I agree. Mabe separates the rssi level judgement function to be chip specific,
> and move the whole refresh_rate_mask thing generic?
>
> > > struct rtl8xxxu_fileops rtl8723bu_fops = {
> > > .parse_efuse = rtl8723bu_parse_efuse,
> > > .load_firmware = rtl8723bu_load_firmware,
> > > @@ -1665,6 +1807,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
> > > .usb_quirks = rtl8xxxu_gen2_usb_quirks,
> > > .set_tx_power = rtl8723b_set_tx_power,
> > > .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> > > + .refresh_rate_mask = rtl8723b_refresh_rate_mask,
> > > .report_connect = rtl8xxxu_gen2_report_connect,
> > > .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> > > .writeN_block_size = 1024,
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > index 039e5ca9d2e4..be322402ca01 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > @@ -4311,7 +4311,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
> > > rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> > > }
> > >
> > > -void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> > > +void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > + u32 ramask, u8 rateid, int sgi)
> > > {
> > > struct h2c_cmd h2c;
> > >
> > > @@ -4331,7 +4332,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> > > }
> > >
> > > void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > - u32 ramask, int sgi)
> > > + u32 ramask, u8 rateid, int sgi)
> > > {
> > > struct h2c_cmd h2c;
> > > u8 bw = 0;
> > > @@ -4345,7 +4346,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
> > >
> > > h2c.ramask.arg = 0x80;
> > > - h2c.b_macid_cfg.data1 = 0;
> > > + h2c.b_macid_cfg.data1 = rateid;
> > > if (sgi)
> > > h2c.b_macid_cfg.data1 |= BIT(7);
> > >
> > > @@ -4485,6 +4486,40 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
> > > rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
> > > }
> > >
> > > +u16
> > > +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> > > +{
> > > + u16 network_type = WIRELESS_MODE_UNKNOWN;
> > > + u32 rate_mask;
> > > +
> > > + rate_mask = (sta->supp_rates[0] & 0xfff) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 20);
> > > +
> > > + if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> > > + if (sta->vht_cap.vht_supported)
> > > + network_type = WIRELESS_MODE_AC;
> > > + else if (sta->ht_cap.ht_supported)
> > > + network_type = WIRELESS_MODE_N_5G;
> > > +
> > > + network_type |= WIRELESS_MODE_A;
> > > + } else {
> > > + if (sta->vht_cap.vht_supported)
> > > + network_type = WIRELESS_MODE_AC;
> > > + else if (sta->ht_cap.ht_supported)
> > > + network_type = WIRELESS_MODE_N_24G;
> > > +
> > > + if (sta->supp_rates[0] <= 0xf)
> > > + network_type |= WIRELESS_MODE_B;
> > > + else if (sta->supp_rates[0] & 0xf)
> > > + network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> > > + else
> > > + network_type |= WIRELESS_MODE_G;
> > > + }
> > > +
> > > + return network_type;
> > > +}
> >
> > I always hated the wireless_mode nonsense in the realtek driver, but
> > maybe we cannot avoid it :(
> >
> > Cheers,
> > Jes
Jes, look forward to any comments or suggestions from you. I would re-write a
patch with generic refresh_rate_mask for all rtl8xxxu series chips in
short time.
Chris
^ permalink raw reply
* [PATCH v3] ath10k: Enable MSA region dump support for WCN3990
From: Govind Singh @ 2019-06-10 9:17 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Govind Singh
MSA memory region caries the hw descriptors information.
Dump MSA region in core dump as this is very helpful in debugging
hw issues.
Testing: Tested on WCN3990 HW
Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
Changes from v2:
- Rebased on top of 38faed150438 ath10k: perform crash dump collection in workqueue.
- Removed redundant msa permission call.
---
drivers/net/wireless/ath/ath10k/coredump.c | 21 +++++++
drivers/net/wireless/ath/ath10k/coredump.h | 1 +
drivers/net/wireless/ath/ath10k/qmi.c | 4 ++
drivers/net/wireless/ath/ath10k/snoc.c | 66 ++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/snoc.h | 1 +
5 files changed, 93 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/coredump.c b/drivers/net/wireless/ath/ath10k/coredump.c
index aa04fbf146e0..0ec690b49fb1 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -962,6 +962,19 @@ static const struct ath10k_mem_region qca4019_hw10_mem_regions[] = {
},
};
+static const struct ath10k_mem_region wcn399x_hw10_mem_regions[] = {
+ {
+ /* MSA region start is not fixed, hence it is assigned at runtime */
+ .type = ATH10K_MEM_REGION_TYPE_MSA,
+ .len = 0x100000,
+ .name = "DRAM",
+ .section_table = {
+ .sections = NULL,
+ .size = 0,
+ },
+ },
+};
+
static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
{
.hw_id = QCA6174_HW_1_0_VERSION,
@@ -1059,6 +1072,14 @@ static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
.size = ARRAY_SIZE(qca4019_hw10_mem_regions),
},
},
+ {
+ .hw_id = WCN3990_HW_1_0_DEV_VERSION,
+ .hw_rev = ATH10K_HW_WCN3990,
+ .region_table = {
+ .regions = wcn399x_hw10_mem_regions,
+ .size = ARRAY_SIZE(wcn399x_hw10_mem_regions),
+ },
+ },
};
static u32 ath10k_coredump_get_ramdump_size(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/coredump.h b/drivers/net/wireless/ath/ath10k/coredump.h
index 5dac653e1649..9802e90483f4 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.h
+++ b/drivers/net/wireless/ath/ath10k/coredump.h
@@ -126,6 +126,7 @@ enum ath10k_mem_region_type {
ATH10K_MEM_REGION_TYPE_IRAM2 = 5,
ATH10K_MEM_REGION_TYPE_IOSRAM = 6,
ATH10K_MEM_REGION_TYPE_IOREG = 7,
+ ATH10K_MEM_REGION_TYPE_MSA = 8,
};
/* Define a section of the region which should be copied. As not all parts
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index ba8f5a8f83d1..8eb0f0f0d3a7 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -817,9 +817,13 @@ ath10k_qmi_driver_event_post(struct ath10k_qmi *qmi,
static void ath10k_qmi_event_server_exit(struct ath10k_qmi *qmi)
{
struct ath10k *ar = qmi->ar;
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
ath10k_qmi_remove_msa_permission(qmi);
ath10k_core_free_board_files(ar);
+ if (!test_bit(ATH10K_SNOC_FLAG_UNREGISTERING, &ar_snoc->flags))
+ ath10k_snoc_fw_crashed_dump(ar);
+
ath10k_snoc_fw_indication(ar, ATH10K_QMI_EVENT_FW_DOWN_IND);
ath10k_dbg(ar, ATH10K_DBG_QMI, "wifi fw qmi service disconnected\n");
}
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 0be12996beba..ecc0f884b123 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -24,6 +24,7 @@
#include <linux/regulator/consumer.h>
#include "ce.h"
+#include "coredump.h"
#include "debug.h"
#include "hif.h"
#include "htc.h"
@@ -1586,6 +1587,71 @@ static int ath10k_hw_power_off(struct ath10k *ar)
return ret;
}
+static void ath10k_msa_dump_memory(struct ath10k *ar,
+ struct ath10k_fw_crash_data *crash_data)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ const struct ath10k_hw_mem_layout *mem_layout;
+ const struct ath10k_mem_region *current_region;
+ struct ath10k_dump_ram_data_hdr *hdr;
+ size_t buf_len;
+ u8 *buf;
+
+ if (!crash_data && !crash_data->ramdump_buf)
+ return;
+
+ mem_layout = ath10k_coredump_get_mem_layout(ar);
+ if (!mem_layout)
+ return;
+
+ current_region = &mem_layout->region_table.regions[0];
+
+ buf = crash_data->ramdump_buf;
+ buf_len = crash_data->ramdump_buf_len;
+ memset(buf, 0, buf_len);
+
+ /* Reserve space for the header. */
+ hdr = (void *)buf;
+ buf += sizeof(*hdr);
+ buf_len -= sizeof(*hdr);
+
+ hdr->region_type = cpu_to_le32(current_region->type);
+ hdr->start = cpu_to_le32(ar_snoc->qmi->msa_va);
+ hdr->length = cpu_to_le32(ar_snoc->qmi->msa_mem_size);
+
+ if (current_region->len < ar_snoc->qmi->msa_mem_size) {
+ memcpy(buf, ar_snoc->qmi->msa_va, current_region->len);
+ ath10k_warn(ar, "msa dump length is less than msa size %x, %x\n",
+ current_region->len, ar_snoc->qmi->msa_mem_size);
+ } else {
+ memcpy(buf, ar_snoc->qmi->msa_va, ar_snoc->qmi->msa_mem_size);
+ }
+}
+
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar)
+{
+ struct ath10k_fw_crash_data *crash_data;
+ char guid[UUID_STRING_LEN + 1];
+
+ mutex_lock(&ar->dump_mutex);
+
+ spin_lock_bh(&ar->data_lock);
+ ar->stats.fw_crash_counter++;
+ spin_unlock_bh(&ar->data_lock);
+
+ crash_data = ath10k_coredump_new(ar);
+
+ if (crash_data)
+ scnprintf(guid, sizeof(guid), "%pUl", &crash_data->guid);
+ else
+ scnprintf(guid, sizeof(guid), "n/a");
+
+ ath10k_err(ar, "firmware crashed! (guid %s)\n", guid);
+ ath10k_print_driver_info(ar);
+ ath10k_msa_dump_memory(ar, crash_data);
+ mutex_unlock(&ar->dump_mutex);
+}
+
static const struct of_device_id ath10k_snoc_dt_match[] = {
{ .compatible = "qcom,wcn3990-wifi",
.data = &drv_priv,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 25383de8f17d..6d28a6290a94 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -101,5 +101,6 @@ static inline struct ath10k_snoc *ath10k_snoc_priv(struct ath10k *ar)
}
int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type);
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar);
#endif /* _SNOC_H_ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* RE: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Hunter, Adrian @ 2019-06-10 8:56 UTC (permalink / raw)
To: Douglas Anderson, Ulf Hansson, Kalle Valo, Arend van Spriel
Cc: brcm80211-dev-list.pdl@broadcom.com,
linux-rockchip@lists.infradead.org, Double Lo,
briannorris@chromium.org, linux-wireless@vger.kernel.org,
Naveen Gupta, Madhan Mohan R, mka@chromium.org, Wright Feng,
Chi-Hsien Lin, netdev@vger.kernel.org,
brcm80211-dev-list@cypress.com, Franky Lin,
linux-kernel@vger.kernel.org, Madhan Mohan R, Hante Meuleman,
YueHaibing, David S. Miller
In-Reply-To: <20190607223716.119277-4-dianders@chromium.org>
> -----Original Message-----
> From: Douglas Anderson [mailto:dianders@chromium.org]
> Sent: Saturday, June 8, 2019 1:37 AM
> To: Ulf Hansson <ulf.hansson@linaro.org>; Kalle Valo
> <kvalo@codeaurora.org>; Hunter, Adrian <adrian.hunter@intel.com>; Arend
> van Spriel <arend.vanspriel@broadcom.com>
> Cc: brcm80211-dev-list.pdl@broadcom.com; linux-
> rockchip@lists.infradead.org; Double Lo <double.lo@cypress.com>;
> briannorris@chromium.org; linux-wireless@vger.kernel.org; Naveen Gupta
> <naveen.gupta@cypress.com>; Madhan Mohan R
> <madhanmohan.r@cypress.com>; mka@chromium.org; Wright Feng
> <wright.feng@cypress.com>; Chi-Hsien Lin <chi-hsien.lin@cypress.com>;
> netdev@vger.kernel.org; brcm80211-dev-list@cypress.com; Douglas
> Anderson <dianders@chromium.org>; Franky Lin
> <franky.lin@broadcom.com>; linux-kernel@vger.kernel.org; Madhan Mohan
> R <MadhanMohan.R@cypress.com>; Hante Meuleman
> <hante.meuleman@broadcom.com>; YueHaibing
> <yuehaibing@huawei.com>; David S. Miller <davem@davemloft.net>
> Subject: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around
> commands expected to fail
>
> There are certain cases, notably when transitioning between sleep and active
> state, when Broadcom SDIO WiFi cards will produce errors on the SDIO bus.
> This is evident from the source code where you can see that we try
> commands in a loop until we either get success or we've tried too many
> times. The comment in the code reinforces this by saying "just one write
> attempt may fail"
>
> Unfortunately these failures sometimes end up causing an "-EILSEQ"
> back to the core which triggers a retuning of the SDIO card and that blocks all
> traffic to the card until it's done.
>
> Let's disable retuning around the commands we expect might fail.
>
> Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v3:
> - Expect errors for all of brcmf_sdio_kso_control() (Adrian).
>
> Changes in v2: None
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 4a750838d8cd..4040aae1f9ed 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -16,6 +16,7 @@
> #include <linux/mmc/sdio_ids.h>
> #include <linux/mmc/sdio_func.h>
> #include <linux/mmc/card.h>
> +#include <linux/mmc/core.h>
SDIO function drivers should not really include linux/mmc/core.h
(Also don't know why linux/mmc/card.h is included)
> #include <linux/semaphore.h>
> #include <linux/firmware.h>
> #include <linux/module.h>
> @@ -667,6 +668,8 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool
> on)
>
> brcmf_dbg(TRACE, "Enter: on=%d\n", on);
>
> + mmc_expect_errors_begin(bus->sdiodev->func1->card->host);
> +
> wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
> /* 1st KSO write goes to AOS wake up core if device is asleep */
> brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
> wr_val, &err); @@ -727,6 +730,8 @@ brcmf_sdio_kso_control(struct
> brcmf_sdio *bus, bool on)
> if (try_cnt > MAX_KSO_ATTEMPTS)
> brcmf_err("max tries: rd_val=0x%x err=%d\n", rd_val, err);
>
> + mmc_expect_errors_end(bus->sdiodev->func1->card->host);
> +
> return err;
> }
>
> --
> 2.22.0.rc2.383.gf4fbbf30c2-goog
^ permalink raw reply
* Re: [PATCH] wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ
From: Tony Lindgren @ 2019-06-10 8:30 UTC (permalink / raw)
To: Kalle Valo
Cc: Eugeniu Rosca, Geert Uytterhoeven, Simon Horman, David S. Miller,
Greg Kroah-Hartman, Randy Dunlap, Ulf Hansson, John Stultz,
linux-wireless, netdev, linux-kernel, Spyridon Papageorgiou,
Joshua Frkuska, George G . Davis, Andrey Gusakov, Linux-Renesas,
Eugeniu Rosca, eyalr
In-Reply-To: <87tvcxncuq.fsf@codeaurora.org>
Hi,
* Kalle Valo <kvalo@codeaurora.org> [190610 07:01]:
> Eugeniu Rosca <erosca@de.adit-jv.com> writes:
>
> > The wl1837mod datasheet [1] says about the WL_IRQ pin:
> >
> > ---8<---
> > SDIO available, interrupt out. Active high. [..]
> > Set to rising edge (active high) on powerup.
> > ---8<---
> >
> > That's the reason of seeing the interrupt configured as:
> > - IRQ_TYPE_EDGE_RISING on HiKey 960/970
> > - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms
> >
> > We assert that all those platforms have the WL_IRQ pin connected
> > to the SoC _directly_ (confirmed on HiKey 970 [2]).
> >
> > That's not the case for R-Car Kingfisher extension target, which carries
> > a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present
> > between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively
> > reversing the requirement quoted from [1]. IOW, in Kingfisher DTS
> > configuration we would need to use IRQ_TYPE_EDGE_FALLING or
> > IRQ_TYPE_LEVEL_LOW.
> >
> > Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq:
> > support platform dependent interrupt types") made a special case out
> > of these interrupt types. After this commit, it is impossible to provide
> > an IRQ configuration via DTS which would describe an inverter present
> > between the WL18* chip and the SoC, generating the need for workarounds
> > like [3].
> >
> > Create a boolean OF property, called "invert-irq" to specify that
> > the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter.
> >
> > This solution has been successfully tested on R-Car H3ULCB-KF-M06 using
> > the DTS configuration [4] combined with the "invert-irq" property.
> >
> > [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf
> > [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/
> > [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch
> > [4] https://patchwork.kernel.org/patch/10895879/
> > ("arm64: dts: ulcb-kf: Add support for TI WL1837")
> >
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
>
> Tony&Eyal, do you agree with this?
Yeah if there's some hardware between the WLAN device and the SoC
inverting the interrupt, I don't think we have clear a way to deal
with it short of setting up a separate irqchip that does the
translation.
But in some cases we also do not want to invert the interrupt, so
I think this property should take IRQ_TYPE_EDGE_RISING and
IRQ_TYPE_EDGE_RISING values to override the setting for
the WLAN end of the hardware?
Let's wait a bit longer for comments from Eyal too.
Regards,
Tony
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-10 8:18 UTC (permalink / raw)
To: Larry Finger
Cc: Christoph Hellwig, Aaro Koskinen, Christian Zigotzky,
Michael Ellerman, linux-kernel, linux-wireless, linuxppc-dev
In-Reply-To: <30000803-3772-3edf-f4a9-55122d504f3f@lwfinger.net>
On Sat, Jun 08, 2019 at 04:52:24PM -0500, Larry Finger wrote:
> On 6/7/19 12:29 PM, Christoph Hellwig wrote:
>> I don't think we should work around this in the driver, we need to fix
>> it in the core. I'm curious why my previous patch didn't work. Can
>> you throw in a few printks what failed? I.e. did dma_direct_supported
>> return false? Did the actual allocation fail?
>
> Routine dma_direct_supported() returns true.
>
> The failure is in routine dma_set_mask() in the following if test:
>
> if (!dev->dma_mask || !dma_supported(dev, mask))
> return -EIO;
>
> For b43legacy, dev->dma_mask is 0xc265684800000000.
> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
> the routine returns -EIO.
>
> For b43, dev->dma_mask is 0xc265684800000001,
> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
> the routine returns 0.
I don't fully understand what values the above map to. Can you send
me your actual debugging patch as well?
^ permalink raw reply
* Re: [PATCH] mt76: mt7615: add support for per-chain signal strength reporting
From: Ryder Lee @ 2019-06-10 7:12 UTC (permalink / raw)
To: Sebastian Gottschall
Cc: Sean Wang, Chih-Min Chen, YF Luo, linux-wireless, linux-kernel,
Yiwei Chung, linux-mediatek, Roy Luo, Lorenzo Bianconi,
Felix Fietkau
In-Reply-To: <64662021-8e5a-91b5-9afb-3c9005564d19@newmedia-net.de>
On Mon, 2019-06-10 at 06:47 +0200, Sebastian Gottschall wrote:
> okay. curious is, that my variant works with sane results too.
> i will test your variant and check the results
>
> Sebastian
Please don't top post as it's hard to track the thread.
More specifically, IBRSSI is obtained from packet's L-STF portion and
MTK HW PD (packet detection) will take it as a reference. (with
variation more or less)
As for RCPI which is calculated from packet's data portion. The other
MTK chipsets may use IBRSSI as their baseband couldn't report RCPI.
Ryder
> Am 10.06.2019 um 06:22 schrieb Ryder Lee:
> > On Mon, 2019-06-10 at 10:09 +0800, Ryder Lee wrote:
> >> On Sun, 2019-06-09 at 16:44 +0200, Sebastian Gottschall wrote:
> >>> according to my findings
> >>>
> >>> MT_RXV4_RCPI1 is part of rx descriptor 4 and not 3
> >>> so it must be rxdg4 = rxd[4] etc.
> >> RXV start from 1 in the code.
> >>
> >> That is: RXV1 <-> rxdg0, RXV2 <-> rxdg1 ...so RXV4 <-> rxdg3
> >>
> >>> however rxdg3 contains MT_RXV3_IB_RSSIRX which can be used for signal calculation.
> >>> i already wrote a similar code for this driver which i sended to felix a long time ago.
> >>> my variant looks like
> >>> status->signal = (FIELD_GET(MT_RXV3_IB_RSSIRX, rxdg3) - 220) / 2;
> >>> status->chain_signal[0] = (FIELD_GET(MT_RXV4_RCPI0, rxdg4) - 220) / 2;
> >>> status->chain_signal[1] = (FIELD_GET(MT_RXV4_RCPI1, rxdg4) - 220) / 2;
> >>> status->chain_signal[2] = (FIELD_GET(MT_RXV4_RCPI2, rxdg4) - 220) / 2;
> >>> status->chain_signal[3] = (FIELD_GET(MT_RXV4_RCPI3, rxdg4) - 220) / 2;
> > mt7615 actually doesn't use in-band RSSI for signal calculation, but it
> > occurs to me that i should modify the code to compare per-chain's
> > signal. Something like this:
> >
> > status->chain_signal[0] = to_rssi(MT_RXV4_RCPI0, rxdg3);
> > status->chain_signal[1] = to_rssi(MT_RXV4_RCPI1, rxdg3);
> > status->chain_signal[2] = to_rssi(MT_RXV4_RCPI2, rxdg3);
> > status->chain_signal[3] = to_rssi(MT_RXV4_RCPI3, rxdg3);
> > status->signal = status->chain_signal[0];
> >
> > switch (status->chains) {
> > case 0xf:
> > status->signal = max(status->signal,
> > status->chain_signal[3]);
> > case 0x7:
> > status->signal = max(status->signal,
> > status->chain_signal[2]);
> > case 0x3:
> > status->signal = max(status->signal,
> > status->chain_signal[1]);
> > break;
> > default:
> > break;
> > }
> >
> >
> > I could send a v2 or you can take care of that.
> >
> > Ryder
> >
> >
^ permalink raw reply
* Re: [PATCH] carl9170: fix enum compare splat
From: Kalle Valo @ 2019-06-10 7:06 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless
In-Reply-To: <20190608144947.744-1-chunkeey@gmail.com>
Christian Lamparter <chunkeey@gmail.com> writes:
> This patch fixes a noisy warning triggered by -Wenum-compare
>
> |main.c:1390:31: warning: comparison between ‘enum nl80211_ac’
> | and ‘enum ar9170_txq’ [-Wenum-compare]
> | BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> | ^
> | [...]
>
> This is a little bit unfortunate, since the number of queues
> (hence NL80211_NUM_ACS) is a constant based on the IEEE 802.11
> (much like IEEE80211_NUM_ACS) and __AR9170_NUM_TXQ is more or
> less defined by the AR9170 hardware.
Is the warning enabled by default? TBH I'm not seeing how useful this
warning is for kernel development.
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -1387,7 +1387,7 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw,
> int ret;
>
> BUILD_BUG_ON(ARRAY_SIZE(ar9170_qmap) != __AR9170_NUM_TXQ);
> - BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> + BUILD_BUG_ON((size_t)NL80211_NUM_ACS > (size_t)__AR9170_NUM_TXQ);
IMHO this just makes the code worse. Does it make sense to workaround
(stupid) compiler warnings like this?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ
From: Kalle Valo @ 2019-06-10 7:01 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: Geert Uytterhoeven, Simon Horman, David S. Miller,
Greg Kroah-Hartman, Randy Dunlap, Tony Lindgren, Ulf Hansson,
John Stultz, linux-wireless, netdev, linux-kernel,
Spyridon Papageorgiou, Joshua Frkuska, George G . Davis,
Andrey Gusakov, Linux-Renesas, Eugeniu Rosca, eyalr
In-Reply-To: <20190607172958.20745-1-erosca@de.adit-jv.com>
Eugeniu Rosca <erosca@de.adit-jv.com> writes:
> The wl1837mod datasheet [1] says about the WL_IRQ pin:
>
> ---8<---
> SDIO available, interrupt out. Active high. [..]
> Set to rising edge (active high) on powerup.
> ---8<---
>
> That's the reason of seeing the interrupt configured as:
> - IRQ_TYPE_EDGE_RISING on HiKey 960/970
> - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms
>
> We assert that all those platforms have the WL_IRQ pin connected
> to the SoC _directly_ (confirmed on HiKey 970 [2]).
>
> That's not the case for R-Car Kingfisher extension target, which carries
> a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present
> between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively
> reversing the requirement quoted from [1]. IOW, in Kingfisher DTS
> configuration we would need to use IRQ_TYPE_EDGE_FALLING or
> IRQ_TYPE_LEVEL_LOW.
>
> Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq:
> support platform dependent interrupt types") made a special case out
> of these interrupt types. After this commit, it is impossible to provide
> an IRQ configuration via DTS which would describe an inverter present
> between the WL18* chip and the SoC, generating the need for workarounds
> like [3].
>
> Create a boolean OF property, called "invert-irq" to specify that
> the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter.
>
> This solution has been successfully tested on R-Car H3ULCB-KF-M06 using
> the DTS configuration [4] combined with the "invert-irq" property.
>
> [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf
> [2] https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/
> [3] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch
> [4] https://patchwork.kernel.org/patch/10895879/
> ("arm64: dts: ulcb-kf: Add support for TI WL1837")
>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Tony&Eyal, do you agree with this?
--
Kalle Valo
^ permalink raw reply
* Re: Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?
From: Michael Büsch @ 2019-06-10 6:51 UTC (permalink / raw)
To: H Buus; +Cc: Larry Finger, Kalle Valo, Michael Chan, linux-wireless, netdev
In-Reply-To: <a7c07ad7-1ca2-c16d-4082-6ddc9325a20d@hbuus.com>
[-- Attachment #1: Type: text/plain, Size: 323 bytes --]
On Mon, 10 Jun 2019 01:40:17 -0400
H Buus <ubuntu@hbuus.com> wrote:
> Unless I get lucky and figure
> out what commit is making newer kernels unstable on this laptop.
You can use 'git bisect' to quickly find a commit that breaks something.
I'll prepare a patch to get rid of the warning asap.
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: iwlwifi module crash
From: Emmanuel Grumbach @ 2019-06-10 6:50 UTC (permalink / raw)
To: Balakrishnan Balasubramanian; +Cc: linux-wireless
In-Reply-To: <2696773.yqXG4m880n@zadesk>
On Fri, Jun 7, 2019 at 2:41 PM Balakrishnan Balasubramanian
<linux-wireless-list@balki.me> wrote:
>
> > This is because the device is removed from the PCI bus. Nothing from
> > iwlwifi side can be done.
>
> I am sure the device is not physically disturbed. If that was the case, should it not stay down when restarting the system?
Not necessarily. The disturbance may impact ASPM or something alike.
>
> > If that happens upon suspend / resume, I know there are been fixes in
> > PCI bus driver.
>
> To my knowledge I have disabled all power/suspend features and I don't see releated logs in journal except the below. Not sure if relevant.
>
> Jun 03 21:33:14 zadesk kernel: wlan0: Limiting TX power to 14 (17 - 3) dBm as advertised by d4:5d:df:25:ee:90
>
> Is there a way to restart the module safely without restarting the system?
echo 1 > /sys/module/iwlwifi/devices/0000\:02\:00.0/remove
echo 1 > /sys/bus/pci/rescan
>
> Regards,
> Bala
>
>
> On Friday, June 7, 2019 5:25:41 AM EDT Emmanuel Grumbach wrote:
> > On Fri, Jun 7, 2019 at 5:22 AM Balakrishnan Balasubramanian
> >
> > <linux-wireless-list@balki.me> wrote:
> > > I am using iwd demon for wifi. Once a while I loose connectivity.
> > > Restarting the demon does not help. But once I restart the system, it
> > > starts working fine. Attaching stack trace from journal.
> >
> > This is because the device is removed from the PCI bus. Nothing from
> > iwlwifi side can be done.
> > If that happens upon suspend / resume, I know there are been fixes in
> > PCI bus driver. If not, check that the device sits correctly in its
> > socket.
> >
> > > Regards,
> > > Bala
> > >
> > >
> > > ---------- Forwarded message ----------
> > > From: Denis Kenzior <denkenz@gmail.com>
> > > To: Balakrishnan Balasubramanian <iwd-lists@balki.me>, iwd@lists.01.org
> > > Cc:
> > > Bcc:
> > > Date: Thu, 06 Jun 2019 18:07:40 -0500
> > > Subject: Re: iwd crashes randomly
> > > Hi Bala,
> > >
> > > On 06/06/2019 06:00 PM, Balakrishnan Balasubramanian wrote:
> > > > Sometimes after a week and sometimes after two days. Once crashed,
> > > > restarting the service does not help. Had to restart the computer.
> > > > Attaching stack trace from journal.
> > >
> > > That implies that your kernel is crashing, not iwd. The attached log
> > > shows a kernel stack trace somewhere inside iwlwifi module. I would
> > > post this trace to linux-wireless@vger.kernel.org.
> > >
> > > If you have an associated iwd backtrace, then certainly post this here,
> > > but if the kernel module is crashing, there isn't much we can do.
> > >
> > > Regards,
> > > -Denis
>
>
>
>
^ permalink raw reply
* How to debug/show status of intel wifi card?
From: Tetsuji Rai @ 2019-06-10 6:22 UTC (permalink / raw)
To: linux-wireless
Hi all,
I installed kernel-5.2-rc4 and found my intel wireless-ac 7260 card
stops working a moment (some seconds or minutes) after some browsing on
chrome or receiving emails on thunderbird or any other file transfer.
After turning the wifi off and on again, it starts working again, a
short while. It doesn't happen with the stable kernel 5.1.8.
I filed it bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=203851
but I still can't grasp the status of my wifi card while it's locked.
ifconfig, iwconfig, ethtool, trace-cmd show nothing abnormal even when
the wifi card is locked. They say my wifi card detects radio signal
correctly, transfer mode is correct.
So will you tell me how to see what happens with my wifi card? It
should be something like a bug of the new kernel.
Thanks in advance!
-Tetsuji Rai
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox