* [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-24 3:02 UTC (permalink / raw)
To: kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel
In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
when this allocation fails, ENOMEM is returned. However, 'pdev_data'
and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
platform_device_alloc() returns NULL, we should also free 'pdev_data'
and 'glue' before wl1271_probe() ends to prevent leaking memory.
Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should
free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is
error.
Further, we should free 'glue->core', 'pdev_data' and 'glue' when this
function normally ends to prevent leaking memory.
Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index 62ce54a..ea0ec26 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -480,7 +480,7 @@ static int wl1271_probe(struct spi_device *spi)
struct wl12xx_spi_glue *glue;
struct wlcore_platdev_data *pdev_data;
struct resource res[1];
- int ret;
+ int ret = -ENOMEM;
pdev_data = devm_kzalloc(&spi->dev, sizeof(*pdev_data), GFP_KERNEL);
if (!pdev_data)
@@ -491,7 +491,8 @@ static int wl1271_probe(struct spi_device *spi)
glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&spi->dev, "can't allocate glue\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free1;
}
glue->dev = &spi->dev;
@@ -503,31 +504,35 @@ static int wl1271_probe(struct spi_device *spi)
spi->bits_per_word = 32;
glue->reg = devm_regulator_get(&spi->dev, "vwlan");
- if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
+ if (PTR_ERR(glue->reg) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out_free2;
+ }
if (IS_ERR(glue->reg)) {
dev_err(glue->dev, "can't get regulator\n");
- return PTR_ERR(glue->reg);
+ ret = PTR_ERR(glue->reg);
+ goto out_free2;
}
ret = wlcore_probe_of(spi, glue, pdev_data);
if (ret) {
dev_err(glue->dev,
"can't get device tree parameters (%d)\n", ret);
- return ret;
+ goto out_free2;
}
ret = spi_setup(spi);
if (ret < 0) {
dev_err(glue->dev, "spi_setup failed\n");
- return ret;
+ goto out_free2;
}
glue->core = platform_device_alloc(pdev_data->family->name,
PLATFORM_DEVID_AUTO);
if (!glue->core) {
dev_err(glue->dev, "can't allocate platform_device\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free2;
}
glue->core->dev.parent = &spi->dev;
@@ -557,10 +562,14 @@ static int wl1271_probe(struct spi_device *spi)
goto out_dev_put;
}
- return 0;
+ ret = 0;
out_dev_put:
platform_device_put(glue->core);
+out_free2:
+ devm_kfree(&func->dev, glue);
+out_free1:
+ devm_kfree(&func->dev, pdev_data);
return ret;
}
---
^ permalink raw reply related
* [PATCH v2] wlcore: sdio: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-24 2:43 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87d0k9b4hm.fsf@kamboji.qca.qualcomm.com>
In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
when this allocation fails, ENOMEM is returned. However, 'pdev_data'
and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
platform_device_alloc() returns NULL, we should also free 'pdev_data'
and 'glue' before wl1271_probe() ends to prevent leaking memory.
Similarly, we should free 'pdev_data' when 'glue' is NULL. And we
should free 'pdev_data' and 'glue' when 'ret' is error.
Further, we shoulf free 'glue->dev', 'pdev_data' and 'glue' when this
function normally ends to prevent memory leaking.
Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index 4d4b0770..9110891 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -298,8 +298,10 @@ static int wl1271_probe(struct sdio_func *func,
pdev_data->if_ops = &sdio_ops;
glue = devm_kzalloc(&func->dev, sizeof(*glue), GFP_KERNEL);
- if (!glue)
- return -ENOMEM;
+ if (!glue) {
+ ret = -ENOMEM;
+ goto out_free1;
+ }
glue->dev = &func->dev;
@@ -311,7 +313,7 @@ static int wl1271_probe(struct sdio_func *func,
ret = wlcore_probe_of(&func->dev, &irq, &wakeirq, pdev_data);
if (ret)
- goto out;
+ goto out_free2;
/* if sdio can keep power while host is suspended, enable wow */
mmcflags = sdio_get_host_pm_caps(func);
@@ -340,7 +342,7 @@ static int wl1271_probe(struct sdio_func *func,
if (!glue->core) {
dev_err(glue->dev, "can't allocate platform_device");
ret = -ENOMEM;
- goto out;
+ goto out_free2;
}
glue->core->dev.parent = &func->dev;
@@ -380,12 +382,17 @@ static int wl1271_probe(struct sdio_func *func,
dev_err(glue->dev, "can't add platform device\n");
goto out_dev_put;
}
- return 0;
+ ret = 0;
out_dev_put:
platform_device_put(glue->core);
-out:
+out_free2:
+ devm_kfree(&func->dev, glue);
+
+out_free1:
+ devm_kfree(&func->dev, pdev_data);
+
return ret;
}
---
^ permalink raw reply related
* Re: [PATCH] ath10k: add missing error handling
From: Brian Norris @ 2019-05-23 23:18 UTC (permalink / raw)
To: Claire Chang; +Cc: Kalle Valo, ath10k, linux-wireless, Wen Gong, Erik Stromdahl
In-Reply-To: <CA+ASDXMaKpMWnLnKxeft-8eKfpM6qGDsmEzvh290JCCjeRRtxQ@mail.gmail.com>
On Thu, May 23, 2019 at 9:42 AM Brian Norris <briannorris@chromium.org> wrote:
> IIUC, you have basically the same failure case a few lines up, where
> ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there?
Oh, I see Erik Stromdahl already got that one:
ath10k: sdio: add missing error check
https://patchwork.kernel.org/patch/10906009/
^ permalink raw reply
* Re: [PATCH v2 2/2] qtnfmac: add support for Topaz chipsets
From: Igor Mitsyanko @ 2019-05-23 19:26 UTC (permalink / raw)
To: Jonas Gorski, Sergey Matyukevich
Cc: linux-wireless@vger.kernel.org, Andrey Shevchenko
In-Reply-To: <CAOiHx=nBWr4GNh61WV+SAY-++Z6es-HX3_pd70DB_N33bVK1tw@mail.gmail.com>
>
> A bit late of a review/question, but how does one obtain one of these
> files? There's nothing in linux-firmware, and I see only one aborted
> attempt for adding fmac_qsr10g.img from 2016, but none for the others.
> Searching for these filenames also didn't reveal any external
> locations.
>
>
> Regards
> Jonas
>
Hi Jonas, we're working towards a second attempt to get those accepted
to linux-firmware (fmac_qsr1000.img binary first) admittedly it takes us
a long time to do that. The main obstacle for us as developers is that
the binary contains 3-d party GPL code so we have to work with other
departments to satisfy all submission requirements (provide sources,
proper licensing/attribution etc). From a failed fmac_qsr10g.img attempt
it was clear that simply providing an contact email is not enough.
We're planning a second attempt to submit firmware binary patch
relatively soon, for now we expect device will boot from flash.
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Ajay.Kathat @ 2019-05-23 16:45 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <87sgt5cl8q.fsf@kamboji.qca.qualcomm.com>
Hi Kalle,
On 5/23/2019 7:43 PM, Kalle Valo wrote:
> <Ajay.Kathat@microchip.com> writes:
>
>> Hi Kalle,
>>
>> On 3/21/2019 6:24 PM, Kalle Valo wrote:
>>>
>>> <Ajay.Kathat@microchip.com> writes:
>>>
>>>> Hi Kalle/Johannes,
>>>>
>>>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>>>
>>>>> This patch series is to review and move wilc1000 driver out of staging.
>>>>> Implemented the initial received review comments[1] and submitting the
>>>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>>>
>>>>> Below are the major items fixed in recent cleanup:
>>>>> - remove use of shadow buffer to keep scan result.
>>>>> - remove internal messaging flow to handle cfg80211_ops.
>>>>> - make use of cfg80211 provided API.
>>>>> - use structure for packing firmware commands.
>>>>> - make use of kernel provided API and macros.
>>>>> - remove unnecessary logs messages.
>>>>>
>>>>> Pending action item:
>>>>> - dynamically add/remove p2p interface.
>>>>>
>>>>> This item will take some time, we are planning to take it up after
>>>>> mainline.
>>>>>
>>>>> We hope it can be move out staging in v5.1.
>>>>> Please review and confirm if it is good to move out.
>>>>
>>>> Do you have any update for this series. Please provide your inputs for
>>>> next step.
>>>
>>> rtw88 is taking priority in the new drivers "queue" so I doubt I can
>>> take a look at this in the next few weeks.
>>>
>>
>> Is there any update for wilc1000 driver review. Please let me know your
>> inputs.
>
> So is the driver in good shape now? I really do not want to use a lot of
> time reviewing it just to find out that there's a lot of work to do. For
> example, has someone else reviewed it?
>
The first series for complete driver review was submitted earlier [1].
It was reviewed by Johannes and he looked into driver integration with
the cfg80211 stack part. We worked on the review comments and submitted
the changes to staging.
We need further review to identify if there is any blocker to move to
mainline.
[1]. https://www.spinics.net/lists/linux-wireless/msg177878.html
Regards,
Ajay
^ permalink raw reply
* Re: [PATCH] ath10k: add missing error handling
From: Brian Norris @ 2019-05-23 16:42 UTC (permalink / raw)
To: Claire Chang; +Cc: Kalle Valo, ath10k, linux-wireless, Wen Gong
In-Reply-To: <20190523071534.254611-1-tientzu@chromium.org>
On Thu, May 23, 2019 at 12:15 AM Claire Chang <tientzu@chromium.org> wrote:
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -607,6 +607,10 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
> full_len,
> last_in_bundle,
> last_in_bundle);
> + if (ret) {
IIUC, you have basically the same failure case a few lines up, where
ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there?
This (including the error label to which it's jumping) looks fine to me though:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> + ath10k_warn(ar, "alloc_rx_pkt error %d\n", ret);
> + goto err;
> + }
> }
>
> ar_sdio->n_rx_pkts = i;
^ permalink raw reply
* [PATCH v2 5.2] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Nathan Chancellor @ 2019-05-23 15:30 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Arnd Bergmann,
Nick Desaulniers, clang-built-linux, Nathan Chancellor
In-Reply-To: <20190502151548.11143-1-natechancellor@gmail.com>
When building with -Wuninitialized, Clang warns:
drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
is uninitialized when used here [-Wuninitialized]
put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
^~~~
drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
variable 'data' to silence this warning
u8 *data;
^
= NULL
1 warning generated.
Using Clang's suggestion of initializing data to NULL wouldn't work out
because data will be dereferenced by put_unaligned_le32. Use kzalloc to
properly initialize data, which matches a couple of other places in this
driver.
Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
Link: https://github.com/ClangBuiltLinux/linux/issues/464
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Use RSI_9116_REG_SIZE instead of sizeof(u32) for kzalloc thanks to
review from Arnd.
drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index f9c67ed473d1..b42cd50b837e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
u32 addr;
u8 *data;
+ data = kzalloc(RSI_9116_REG_SIZE, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE,
"Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
@@ -944,7 +948,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to hold TA threads\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_SOFT_RST_CLR, data);
@@ -954,7 +958,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to get TA out of reset\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_PC_ZERO, data);
@@ -964,7 +968,8 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to Reset TA PC value\n");
- return -EINVAL;
+ status = -EINVAL;
+ goto err;
}
put_unaligned_le32(TA_RELEASE_THREAD_VALUE, data);
@@ -974,17 +979,19 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to release TA threads\n");
- return status;
+ goto err;
}
status = rsi_sdio_master_access_msword(adapter, MISC_CFG_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "***** TA Reset done *****\n");
- return 0;
+err:
+ kfree(data);
+ return status;
}
static struct rsi_host_intf_ops sdio_host_intf_ops = {
--
2.22.0.rc1
^ permalink raw reply related
* Re: [PATCH] iw: print HE capabilities
From: Sven Eckelmann @ 2019-05-23 15:22 UTC (permalink / raw)
To: John Crispin; +Cc: Johannes Berg, linux-wireless, Shashidhar Lakkavalli
In-Reply-To: <20190520105416.27185-1-john@phrozen.org>
[-- Attachment #1: Type: text/plain, Size: 568 bytes --]
On Monday, 20 May 2019 12:54:16 CEST John Crispin wrote:
> + char *iftypes[NUM_NL80211_IFTYPES] = {
> + "Unspec", "Adhoc", "Station", "AP", "AP/VLAN", "WDS", "Monitor"
> + "Mesh", "P2P/Client", "P2P/Go", "P2P/Device", "OCB", "NAN",
> + };
Noticed during tests with hwsim that this list is slightly incorrect - because
the "," is missing after "Monitor". Thus you will now have an entry
"MonitorMesh" instead of two entries "Monitor" + "Mesh". So everything after
NL80211_IFTYPE_MESH_POINT is off by one.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] sdio: Fix a memory leaking bug in wl1271_probe()
From: Kalle Valo @ 2019-05-23 15:00 UTC (permalink / raw)
To: Gen Zhang; +Cc: eyalreizer, netdev, linux-kernel, linux-wireless
In-Reply-To: <20190523144425.GA26766@zhanggen-UX430UQ>
+ linux-wireless
Gen Zhang <blackgod016574@gmail.com> writes:
> In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
> when this allocation fails, ENOMEM is returned. However, 'pdev_data'
> and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
> platform_device_alloc() returns NULL, we should also free 'pdev_data'
> and 'glue' before wl1271_probe() ends to prevent leaking memory.
>
> Similarly, we should free 'pdev_data' when 'glue' is NULL. And we
> should free 'pdev_data' and 'glue' when 'ret' is error.
>
> Further, we shoulf free 'glue->dev', 'pdev_data' and 'glue' when this
> function normally ends to prevent memory leaking.
>
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> ---
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index 4d4b0770..232ce5f 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
You need to CC linux-wireless, otherwise patchwork won't see the patch
and it will not be applied. Also you should use prefix "wlcore:".
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Kalle Valo @ 2019-05-23 14:13 UTC (permalink / raw)
To: Ajay.Kathat
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <2ba91fc2-af3a-b235-5cb0-2ba06444ea30@microchip.com>
<Ajay.Kathat@microchip.com> writes:
> Hi Kalle,
>
> On 3/21/2019 6:24 PM, Kalle Valo wrote:
>>
>> <Ajay.Kathat@microchip.com> writes:
>>
>>> Hi Kalle/Johannes,
>>>
>>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>>
>>>> This patch series is to review and move wilc1000 driver out of staging.
>>>> Implemented the initial received review comments[1] and submitting the
>>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>>
>>>> Below are the major items fixed in recent cleanup:
>>>> - remove use of shadow buffer to keep scan result.
>>>> - remove internal messaging flow to handle cfg80211_ops.
>>>> - make use of cfg80211 provided API.
>>>> - use structure for packing firmware commands.
>>>> - make use of kernel provided API and macros.
>>>> - remove unnecessary logs messages.
>>>>
>>>> Pending action item:
>>>> - dynamically add/remove p2p interface.
>>>>
>>>> This item will take some time, we are planning to take it up after
>>>> mainline.
>>>>
>>>> We hope it can be move out staging in v5.1.
>>>> Please review and confirm if it is good to move out.
>>>
>>> Do you have any update for this series. Please provide your inputs for
>>> next step.
>>
>> rtw88 is taking priority in the new drivers "queue" so I doubt I can
>> take a look at this in the next few weeks.
>>
>
> Is there any update for wilc1000 driver review. Please let me know your
> inputs.
So is the driver in good shape now? I really do not want to use a lot of
time reviewing it just to find out that there's a lot of work to do. For
example, has someone else reviewed it?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Ajay.Kathat @ 2019-05-23 13:32 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <871s30pevc.fsf@codeaurora.org>
Hi Kalle,
On 3/21/2019 6:24 PM, Kalle Valo wrote:
>
> <Ajay.Kathat@microchip.com> writes:
>
>> Hi Kalle/Johannes,
>>
>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>
>>> This patch series is to review and move wilc1000 driver out of staging.
>>> Implemented the initial received review comments[1] and submitting the
>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>
>>> Below are the major items fixed in recent cleanup:
>>> - remove use of shadow buffer to keep scan result.
>>> - remove internal messaging flow to handle cfg80211_ops.
>>> - make use of cfg80211 provided API.
>>> - use structure for packing firmware commands.
>>> - make use of kernel provided API and macros.
>>> - remove unnecessary logs messages.
>>>
>>> Pending action item:
>>> - dynamically add/remove p2p interface.
>>>
>>> This item will take some time, we are planning to take it up after
>>> mainline.
>>>
>>> We hope it can be move out staging in v5.1.
>>> Please review and confirm if it is good to move out.
>>
>> Do you have any update for this series. Please provide your inputs for
>> next step.
>
> rtw88 is taking priority in the new drivers "queue" so I doubt I can
> take a look at this in the next few weeks.
>
Is there any update for wilc1000 driver review. Please let me know your
inputs.
Regards,
Ajay
^ permalink raw reply
* Re: [PATCH 1/7] brcm80211: switch common header files to using SPDX license identifier
From: Arend Van Spriel @ 2019-05-23 10:53 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <871s0pedih.fsf@kamboji.qca.qualcomm.com>
On May 23, 2019 11:17:22 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>> Or do you want me to resend the whole series without patch 3?
>
> If the series applies without patch 3 (and I assume it does) no need to
> resend the whole series.
Thanks, Kalle
I assume the same.
Regards,
Arend
^ permalink raw reply
* [PATCH] iw: add HE support to station dump call
From: John Crispin @ 2019-05-23 10:50 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John Crispin, Shashidhar Lakkavalli
Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
station.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/station.c b/station.c
index 0454f87..aaad079 100644
--- a/station.c
+++ b/station.c
@@ -243,6 +243,18 @@ void parse_bitrate(struct nlattr *bitrate_attr, char *buf, int buflen)
if (rinfo[NL80211_RATE_INFO_VHT_NSS])
pos += snprintf(pos, buflen - (pos - buf),
" VHT-NSS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_VHT_NSS]));
+ if (rinfo[NL80211_RATE_INFO_HE_MCS])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-MCS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_MCS]));
+ if (rinfo[NL80211_RATE_INFO_HE_NSS])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-NSS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_NSS]));
+ if (rinfo[NL80211_RATE_INFO_HE_GI])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-GI %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_GI]));
+ if (rinfo[NL80211_RATE_INFO_HE_DCM])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-DCM %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_DCM]));
}
static char *get_chain_signal(struct nlattr *attr_list)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/7] brcm80211: switch common header files to using SPDX license identifier
From: Kalle Valo @ 2019-05-23 9:17 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <f4d582e8-3d65-959f-78bd-88b7c47131a5@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On 5/20/2019 6:54 PM, Kalle Valo wrote:
>> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 5/16/2019 10:57 PM, Arend Van Spriel wrote:
>>>> On 5/16/2019 2:04 PM, Arend van Spriel wrote:
>>>>> With ISC license text in place under the LICENSES folder switch
>>>>> to using the SPDX license identifier to refer to the ISC license.
>>>>
>>>> Hi Kalle,
>>>>
>>>> Given the feedback on checkpatch (or spdxcheck) failures let me
>>>> respin this series.
>>>
>>> Actually let's *NOT* respin and leave this series as is and ignore the
>>> warning for the header files as Thomas wrote in his response: " So we
>>> can fixup the documentation and allow // style for headers as well.".
>>
>> What about patch 3, should I drop that patch? Wasn't the conclusion that
>> you need separately change led.c?
>
> Okay. Let's do that.
Dropped it now.
> Or do you want me to resend the whole series without patch 3?
If the series applies without patch 3 (and I assume it does) no need to
resend the whole series.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 3/7] brcmsmac: switch source files to using SPDX license identifier
From: Kalle Valo @ 2019-05-23 9:16 UTC (permalink / raw)
To: Arend van Spriel
Cc: linux-wireless, Arend van Spriel, Thomas Gleixner,
Greg Kroah-Hartman
In-Reply-To: <1558008251-13692-4-git-send-email-arend.vanspriel@broadcom.com>
Arend van Spriel <arend.vanspriel@broadcom.com> wrote:
> With ISC license text in place under the LICENSES folder switch
> to using the SPDX license identifier to refer to the ISC license.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Dropped per discussion
Patch set to Changes Requested.
--
https://patchwork.kernel.org/patch/10946471/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] wireless: fix intel typos in code comments
From: Kalle Valo @ 2019-05-23 9:14 UTC (permalink / raw)
To: Weitao Hou; +Cc: stas.yakovlev, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190519033114.20271-1-houweitaoo@gmail.com>
Weitao Hou <houweitaoo@gmail.com> writes:
> fix lengh to length
>
> Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
> ---
> drivers/net/wireless/intel/ipw2x00/ipw2200.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Correct prefix is "ipw2x00:".
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] net: fix typos in code comments
From: Kalle Valo @ 2019-05-23 9:12 UTC (permalink / raw)
To: Weitao Hou
Cc: nbd, lorenzo.bianconi83, ryder.lee, royluo, davem, matthias.bgg,
linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel
In-Reply-To: <20190519030923.18343-1-houweitaoo@gmail.com>
Weitao Hou <houweitaoo@gmail.com> writes:
> fix lenght to length
>
> Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
> ---
> drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c | 2 +-
Please use correct prefix "mt76:":
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Kalle Valo @ 2019-05-23 8:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nathan Chancellor, Nick Desaulniers, Amitkumar Karwar,
Siva Rebbagondla, linux-wireless, Networking, LKML,
clang-built-linux
In-Reply-To: <CAK8P3a001V5qQo4vGfpugtmrnFfUNeP_q4KY-YS7rP_L91HY1A@mail.gmail.com>
Arnd Bergmann <arnd@arndb.de> writes:
>> > @@ -937,7 +937,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> > }
>> >
>> > rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
>> > - put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
>> > + put_unaligned_le32(TA_HOLD_THREAD_VALUE, &data);
>> > addr = TA_HOLD_THREAD_REG | RSI_SD_REQUEST_MASTER;
>> > status = rsi_sdio_write_register_multiple(adapter, addr,
>> > (u8 *)&data,
>
> This is clearly not ok, as put_unaligned_le32() stores four bytes, and
> the local variable is only one byte!
>
> Also, sdio does use DMA for transfers, so the variable has to be
> dynamically allocated. I think your original patch was correct.
> The only change I'd possibly make would be to use
> RSI_9116_REG_SIZE instead of sizeof(u32).
Good point. Nathan please fix this and submit v2.
>> Did any of the maintainers have any comments on what the correct
>> solution is here to resolve this warning? It is one of the few left
>> before we can turn on -Wuninitialized for the whole kernel.
>
> I would argue that this should not stop us from turning it on, as the
> warning is for a clear bug in the code that absolutely needs to be
> fixed, rather than a false-positive.
I can queue v2 for v5.2, just remind me by adding "[PATCH v2 5.2]" to
the subject.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v3] ath10k: add support for firmware crash recovery on SDIO chip
From: Claire Chang @ 2019-05-23 8:53 UTC (permalink / raw)
To: Wen Gong; +Cc: ath10k, open list:NETWORKING DRIVERS (WIRELESS)
In-Reply-To: <1558506776-19702-1-git-send-email-wgong@codeaurora.org>
Tested-by: Claire Chang <tientzu@chromium.org>
^ permalink raw reply
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Kalle Valo @ 2019-05-23 8:52 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, Amitkumar Karwar, Siva Rebbagondla,
linux-wireless, netdev, LKML, clang-built-linux
In-Reply-To: <20190523015415.GA17819@archlinux-epyc>
Nathan Chancellor <natechancellor@gmail.com> writes:
> On Thu, May 02, 2019 at 08:17:18PM -0700, Nathan Chancellor wrote:
>> On Thu, May 02, 2019 at 11:18:01AM -0700, Nick Desaulniers wrote:
>> > On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
>> > <natechancellor@gmail.com> wrote:
>> > >
>> > > When building with -Wuninitialized, Clang warns:
>> > >
>> > > drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
>> > > is uninitialized when used here [-Wuninitialized]
>> > > put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
>> > > ^~~~
>> > > drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
>> > > variable 'data' to silence this warning
>> > > u8 *data;
>> > > ^
>> > > = NULL
>> > > 1 warning generated.
>> > >
>> > > Using Clang's suggestion of initializing data to NULL wouldn't work out
>> > > because data will be dereferenced by put_unaligned_le32. Use kzalloc to
>> > > properly initialize data, which matches a couple of other places in this
>> > > driver.
>> > >
>> > > Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
>> > > Link: https://github.com/ClangBuiltLinux/linux/issues/464
>> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> > > ---
>> > > drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
>> > > 1 file changed, 14 insertions(+), 7 deletions(-)
>> > >
>> > > diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> > > index f9c67ed473d1..b35728564c7b 100644
>> > > --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> > > +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> > > @@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> > > u32 addr;
>> > > u8 *data;
>> > >
>> > > + data = kzalloc(sizeof(u32), GFP_KERNEL);
>> >
>> > Something fishy is going on here. We allocate 4 B but declare data as
>> > a u8* (pointer to individual bytes)? In general, dynamically
>> > allocating that few bytes is a code smell; either you meant to just
>> > use the stack, or this memory's lifetime extends past the lifetime of
>> > this stackframe, at which point you probably just meant to stack
>> > allocate space in a higher parent frame and pass this preallocated
>> > memory down to the child frame to get filled in.
>> >
>> > Reading through this code, I don't think that the memory is meant to
>> > outlive the stack frame. Is there a reason why we can't just declare
>> > data as:
>> >
>> > u8 data [4];
>>
>> data was __le32 in rsi_reset_chip() before commit f700546682a6 ("rsi:
>> fix nommu_map_sg overflow kernel panic").
>>
>> I wonder if this would be okay for this function:
>>
>> -------------------------------------------------
>>
>> diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> index f9c67ed473d1..0330c50ab99c 100644
>> --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
>> @@ -927,7 +927,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> {
>> int status;
>> u32 addr;
>> - u8 *data;
>> + u8 data;
>>
>> status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
>> if (status < 0) {
>> @@ -937,7 +937,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> }
>>
>> rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
>> - put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
>> + put_unaligned_le32(TA_HOLD_THREAD_VALUE, &data);
>> addr = TA_HOLD_THREAD_REG | RSI_SD_REQUEST_MASTER;
>> status = rsi_sdio_write_register_multiple(adapter, addr,
>> (u8 *)&data,
>> @@ -947,7 +947,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> return status;
>> }
>>
>> - put_unaligned_le32(TA_SOFT_RST_CLR, data);
>> + put_unaligned_le32(TA_SOFT_RST_CLR, &data);
>> addr = TA_SOFT_RESET_REG | RSI_SD_REQUEST_MASTER;
>> status = rsi_sdio_write_register_multiple(adapter, addr,
>> (u8 *)&data,
>> @@ -957,7 +957,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> return status;
>> }
>>
>> - put_unaligned_le32(TA_PC_ZERO, data);
>> + put_unaligned_le32(TA_PC_ZERO, &data);
>> addr = TA_TH0_PC_REG | RSI_SD_REQUEST_MASTER;
>> status = rsi_sdio_write_register_multiple(adapter, addr,
>> (u8 *)&data,
>> @@ -967,7 +967,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
>> return -EINVAL;
>> }
>>
>> - put_unaligned_le32(TA_RELEASE_THREAD_VALUE, data);
>> + put_unaligned_le32(TA_RELEASE_THREAD_VALUE, &data);
>> addr = TA_RELEASE_THREAD_REG | RSI_SD_REQUEST_MASTER;
>> status = rsi_sdio_write_register_multiple(adapter, addr,
>> (u8 *)&data,
>>
>>
>> >
>> > then use ARRAY_SIZE(data) or RSI_9116_REG_SIZE in rsi_reset_chip(),
>> > getting rid of the kzalloc/kfree?
>> >
>> > (Sorry, I hate when a simple fixup becomes a "hey let's rewrite all
>> > this code" thus becoming "that guy.")
>>
>> If we aren't actually improving the code, then why bother? :)
>>
>> Thank you for the review!
>
> Did any of the maintainers have any comments on what the correct
> solution is here to resolve this warning? It is one of the few left
> before we can turn on -Wuninitialized for the whole kernel.
I don't have any strong opinion, but as the commit log says that
kzalloc() is also used in similar cases in the same driver I would happy
to take this patch as is.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Arnd Bergmann @ 2019-05-23 8:51 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nick Desaulniers, Amitkumar Karwar, Siva Rebbagondla, Kalle Valo,
linux-wireless, Networking, LKML, clang-built-linux
In-Reply-To: <20190523015415.GA17819@archlinux-epyc>
On Thu, May 23, 2019 at 3:54 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, May 02, 2019 at 08:17:18PM -0700, Nathan Chancellor wrote:
> > On Thu, May 02, 2019 at 11:18:01AM -0700, Nick Desaulniers wrote:
> > > On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
> > > u8 data [4];
> >
> > data was __le32 in rsi_reset_chip() before commit f700546682a6 ("rsi:
> > fix nommu_map_sg overflow kernel panic").
> >
> > I wonder if this would be okay for this function:
> >
> > -------------------------------------------------
> >
> > diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > index f9c67ed473d1..0330c50ab99c 100644
> > --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> > @@ -927,7 +927,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
> > {
> > int status;
> > u32 addr;
> > - u8 *data;
> > + u8 data;
> >
> > status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
> > if (status < 0) {
> > @@ -937,7 +937,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
> > }
> >
> > rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
> > - put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
> > + put_unaligned_le32(TA_HOLD_THREAD_VALUE, &data);
> > addr = TA_HOLD_THREAD_REG | RSI_SD_REQUEST_MASTER;
> > status = rsi_sdio_write_register_multiple(adapter, addr,
> > (u8 *)&data,
This is clearly not ok, as put_unaligned_le32() stores four bytes, and
the local variable is only one byte!
Also, sdio does use DMA for transfers, so the variable has to be
dynamically allocated. I think your original patch was correct.
The only change I'd possibly make would be to use
RSI_9116_REG_SIZE instead of sizeof(u32).
> Did any of the maintainers have any comments on what the correct
> solution is here to resolve this warning? It is one of the few left
> before we can turn on -Wuninitialized for the whole kernel.
I would argue that this should not stop us from turning it on, as the
warning is for a clear bug in the code that absolutely needs to be
fixed, rather than a false-positive.
Arnd
^ permalink raw reply
* [PATCH V2] mac80211: fix rate reporting inside cfg80211_calculate_bitrate_he()
From: John Crispin @ 2019-05-23 8:27 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John Crispin, Shashidhar Lakkavalli
The reported rate is not scaled down correctly. After applying this patch,
the function will behave just like the v/ht equivalents.
Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
net/wireless/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 75899b62bdc9..5a03f38788e7 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1237,7 +1237,7 @@ static u32 cfg80211_calculate_bitrate_he(struct rate_info *rate)
if (rate->he_dcm)
result /= 2;
- return result;
+ return result / 10000;
}
u32 cfg80211_calculate_bitrate(struct rate_info *rate)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] mac80211: fix rate reporting inside cfg80211_calculate_bitrate_he()
From: John Crispin @ 2019-05-23 8:27 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Shashidhar Lakkavalli
In-Reply-To: <20190523080332.29173-1-john@phrozen.org>
On 23/05/2019 10:03, John Crispin wrote:
> The reported rate is not scaled done correctly. After applying this patch, the
> function will behave just like the v/ht equivalents.
s/done/down/ ... sorry aspell obviously did not catch that one :-)
John
> Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
> Signed-off-by: John Crispin <john@phrozen.org>
> ---
> net/wireless/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 75899b62bdc9..5a03f38788e7 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -1237,7 +1237,7 @@ static u32 cfg80211_calculate_bitrate_he(struct rate_info *rate)
> if (rate->he_dcm)
> result /= 2;
>
> - return result;
> + return result / 10000;
> }
>
> u32 cfg80211_calculate_bitrate(struct rate_info *rate)
^ permalink raw reply
* [PATCH] mac80211: fix rate reporting inside cfg80211_calculate_bitrate_he()
From: John Crispin @ 2019-05-23 8:03 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John Crispin, Shashidhar Lakkavalli
The reported rate is not scaled done correctly. After applying this patch, the
function will behave just like the v/ht equivalents.
Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
net/wireless/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 75899b62bdc9..5a03f38788e7 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1237,7 +1237,7 @@ static u32 cfg80211_calculate_bitrate_he(struct rate_info *rate)
if (rate->he_dcm)
result /= 2;
- return result;
+ return result / 10000;
}
u32 cfg80211_calculate_bitrate(struct rate_info *rate)
--
2.20.1
^ permalink raw reply related
* Re: ath10k QCA9377 firmware crashes and fails to recover
From: Kalle Valo @ 2019-05-23 7:35 UTC (permalink / raw)
To: Daniel Drake; +Cc: ath10k, linux-wireless, Endless Linux Upstreaming Team
In-Reply-To: <CAD8Lp45wxQ3vL_ttq-yKYDxscjn2KyJVCx_vJBCn+u8Yc5QtOQ@mail.gmail.com>
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?
--
Kalle Valo
^ 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