Re: [PATCH 01/12] soc: qcom: add firmware name helper
Dmitry Baryshkov writes: > On Tue, 21 May 2024 at 12:52, wrote: >> >> On 21/05/2024 11:45, Dmitry Baryshkov wrote: >> > Qualcomm platforms have different sets of the firmware files, which >> > differ from platform to platform (and from board to board, due to the >> > embedded signatures). Rather than listing all the firmware files, >> > including full paths, in the DT, provide a way to determine firmware >> > path based on the root DT node compatible. >> >> Ok this looks quite over-engineered but necessary to handle the legacy, >> but I really think we should add a way to look for a board-specific path >> first and fallback to those SoC specific paths. > > Again, CONFIG_FW_LOADER_USER_HELPER => delays. To me this also looks like very over-engineered, can you elaborate more why this is needed? Concrete examples would help to understand better. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] tracing: Inform kmemleak of saved_cmdlines allocation
Steven Rostedt writes:
> From: "Steven Rostedt (Google)"
>
> The allocation of the struct saved_cmdlines_buffer structure changed from:
>
> s = kmalloc(sizeof(*s), GFP_KERNEL);
> s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
>
> to:
>
> orig_size = sizeof(*s) + val * TASK_COMM_LEN;
> order = get_order(orig_size);
> size = 1 << (order + PAGE_SHIFT);
> page = alloc_pages(GFP_KERNEL, order);
> if (!page)
> return NULL;
>
> s = page_address(page);
> memset(s, 0, sizeof(*s));
>
> s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
>
> Where that s->saved_cmdlines allocation looks to be a dangling allocation
> to kmemleak. That's because kmemleak only keeps track of kmalloc()
> allocations. For allocations that use page_alloc() directly, the kmemleak
> needs to be explicitly informed about it.
>
> Add kmemleak_alloc() and kmemleak_free() around the page allocation so
> that it doesn't give the following false positive:
>
> unreferenced object 0x8881010c8000 (size 32760):
> comm "swapper", pid 0, jiffies 4294667296
> hex dump (first 32 bytes):
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> backtrace (crc ae6ec1b9):
> [] kmemleak_alloc+0x45/0x80
> [] __kmalloc_large_node+0x10d/0x190
> [] __kmalloc+0x3b1/0x4c0
> [] allocate_cmdlines_buffer+0x113/0x230
> [] tracer_alloc_buffers.isra.0+0x124/0x460
> [] early_trace_init+0x14/0xa0
> [] start_kernel+0x12e/0x3c0
> [] x86_64_start_reservations+0x18/0x30
> [] x86_64_start_kernel+0x7b/0x80
> [] secondary_startup_64_no_verify+0x15e/0x16b
>
> Link: https://lore.kernel.org/linux-trace-kernel/87r0hfnr9r@kernel.org/
>
> Fixes: 44dc5c41b5b1 ("tracing: Fix wasted memory in saved_cmdlines logic")
> Reported-by: Kalle Valo
> Tested-by: Kalle Valo
> Signed-off-by: Steven Rostedt (Google)
Applies cleanly to v6.8-rc4 and I don't see the leak anymore, thank you
for fixing it so quickly!
Tested-by: Kalle Valo
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [regression] tracing: kmemleak warning in allocate_cmdlines_buffer()
Steven Rostedt writes:
> On Wed, 14 Feb 2024 14:50:56 +0200
> Kalle Valo wrote:
>
>> Hi Steven,
>>
>> I upgraded our ath11k test setup to v6.8-rc4 and noticed a new kmemleak
>> warning in the log:
>
> Thanks for the report.
>
>>
>> unreferenced object 0x8881010c8000 (size 32760):
>> comm "swapper", pid 0, jiffies 4294667296
>> hex dump (first 32 bytes):
>> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> backtrace (crc ae6ec1b9):
>> [] kmemleak_alloc+0x45/0x80
>> [] __kmalloc_large_node+0x10d/0x190
>> [] __kmalloc+0x3b1/0x4c0
>> [] allocate_cmdlines_buffer+0x113/0x230
>> [] tracer_alloc_buffers.isra.0+0x124/0x460
>> [] early_trace_init+0x14/0xa0
>> [] start_kernel+0x12e/0x3c0
>> [] x86_64_start_reservations+0x18/0x30
>> [] x86_64_start_kernel+0x7b/0x80
>> [] secondary_startup_64_no_verify+0x15e/0x16b
>>
>> I don't see this warning in v6.8-rc3 and also reverting commit
>> 44dc5c41b5b1 ("tracing: Fix wasted memory in saved_cmdlines logic")
>> makes the warning go away. Let me know if you need more info or help
>> with testing, I see the warning every time so it's easy to reproduce.
>>
>
> Hmm, I changed the code a bit and I wonder if this is a false positive?
>
> Instead of allocating the structure via kmalloc() I now use it as part of a
> page.
>
> That is, the old code had:
>
> s = kmalloc(sizeof(*s), GFP_KERNEL);
> s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
>
> Where as the new code has:
>
> orig_size = sizeof(*s) + val * TASK_COMM_LEN;
> order = get_order(orig_size);
> size = 1 << (order + PAGE_SHIFT);
> page = alloc_pages(GFP_KERNEL, order);
> if (!page)
> return NULL;
>
> s = page_address(page);
> memset(s, 0, sizeof(*s));
>
> s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
>
>
> Does kmemleak handle structures that are assigned to alloc_pages()
> allocations? I don't think it does.
>
> I think we need to inform kmemleak about this. Does the following patch fix
> this for you?
It does, thank you!
Tested-by: Kalle Valo
> diff --git a/kernel/trace/trace_sched_switch.c
> b/kernel/trace/trace_sched_switch.c
> index e4fbcc3bede5..de4182224ea2 100644
> --- a/kernel/trace/trace_sched_switch.c
> +++ b/kernel/trace/trace_sched_switch.c
Although the patch didn't apply for me as in my tree the functions are
in kernel/trace/trace.c. I don't know what happened so as a quick hack I
just manually added the three lines to my version of trace.c. Let me
know if there's a git tree or branch you would like me to test, I can do
that easily.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[regression] tracing: kmemleak warning in allocate_cmdlines_buffer()
Hi Steven,
I upgraded our ath11k test setup to v6.8-rc4 and noticed a new kmemleak
warning in the log:
unreferenced object 0x8881010c8000 (size 32760):
comm "swapper", pid 0, jiffies 4294667296
hex dump (first 32 bytes):
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
backtrace (crc ae6ec1b9):
[] kmemleak_alloc+0x45/0x80
[] __kmalloc_large_node+0x10d/0x190
[] __kmalloc+0x3b1/0x4c0
[] allocate_cmdlines_buffer+0x113/0x230
[] tracer_alloc_buffers.isra.0+0x124/0x460
[] early_trace_init+0x14/0xa0
[] start_kernel+0x12e/0x3c0
[] x86_64_start_reservations+0x18/0x30
[] x86_64_start_kernel+0x7b/0x80
[] secondary_startup_64_no_verify+0x15e/0x16b
I don't see this warning in v6.8-rc3 and also reverting commit
44dc5c41b5b1 ("tracing: Fix wasted memory in saved_cmdlines logic")
makes the warning go away. Let me know if you need more info or help
with testing, I see the warning every time so it's easy to reproduce.
Kalle
Re: 回复：general protection fault in ath9k_wmi_event_tasklet
Toke Høiland-Jørgensen writes:
> "Ubisectech Sirius" writes:
>
>>>Hmm, so from eyeballing the code in question, this looks like it is
>>>another initialisation race along the lines of the one fixed in commit:
>>>8b3046abc99e ("ath9k_htc: fix NULL pointer dereference at
>>> ath9k_htc_tx_get_packet()")
>>>Could you please test the patch below and see if you can still reproduce
>>>this issue with that applied?
>> Hello.
>> I can not reproduce the issue on the Linux with the patch applied
>> Ubisectech Sirius Team
>
> Great, thank you for testing! I'll send a proper patch. How would you
> like to be credited with reporting? Just as 'Ubisectech Sirius
> ' ?
Ubisectech, please CC linux-wireless on any wireless issues and don't
use HTML format in emails. More info in the wiki below.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wifi: ath11k: Defer on rproc_get failure
Luca Weiss wrote:
> If we already have gotten the rproc_handle (meaning the "qcom,rproc"
> property is defined in the devicetree), it's a valid state that the
> remoteproc module hasn't probed yet so we should defer probing instead
> of just failing to probe.
>
> This resolves a race condition when the ath11k driver probes and fails
> before the wpss remoteproc driver has probed, like the following:
>
> [6.232360] ath11k 17a10040.wifi: failed to get rproc
> [6.232366] ath11k 17a10040.wifi: failed to get rproc: -22
> [6.232478] ath11k: probe of 17a10040.wifi failed with error -22
>...
> [6.252415] remoteproc remoteproc2: 8a0.remoteproc is available
> [6.252776] remoteproc remoteproc2: powering up 8a0.remoteproc
> [6.252781] remoteproc remoteproc2: Booting fw image
> qcom/qcm6490/fairphone5/wpss.mdt, size 7188
>
> So, defer the probe if we hit that so we can retry later once the wpss
> remoteproc is available.
>
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-01264-QCAMSLSWPLZ-1.37886.3
>
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Luca Weiss
> Signed-off-by: Kalle Valo
Patch applied to ath-next branch of ath.git, thanks.
2a3ec40b98b4 wifi: ath11k: Defer on rproc_get failure
--
https://patchwork.kernel.org/project/linux-wireless/patch/20231027-ath11k-rproc-defer-v1-1-f6b6a812c...@fairphone.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wifi: ath11k: Defer on rproc_get failure
"Luca Weiss" writes:
> On Fri Oct 27, 2023 at 10:25 AM CEST, Kalle Valo wrote:
>
>> Luca Weiss writes:
>>
>> > If we already have gotten the rproc_handle (meaning the "qcom,rproc"
>> > property is defined in the devicetree), it's a valid state that the
>> > remoteproc module hasn't probed yet so we should defer probing instead
>> > of just failing to probe.
>> >
>> > This resolves a race condition when the ath11k driver probes and fails
>> > before the wpss remoteproc driver has probed, like the following:
>> >
>> > [6.232360] ath11k 17a10040.wifi: failed to get rproc
>> > [6.232366] ath11k 17a10040.wifi: failed to get rproc: -22
>> > [6.232478] ath11k: probe of 17a10040.wifi failed with error -22
>> >...
>> > [6.252415] remoteproc remoteproc2: 8a0.remoteproc is available
>> > [6.252776] remoteproc remoteproc2: powering up 8a0.remoteproc
>> > [6.252781] remoteproc remoteproc2: Booting fw image
>> > qcom/qcm6490/fairphone5/wpss.mdt, size 7188
>> >
>> > So, defer the probe if we hit that so we can retry later once the wpss
>> > remoteproc is available.
>> >
>> > Signed-off-by: Luca Weiss
>>
>> Did you test this on a real device? If yes, what ath11k hardware and firmware
>> did you use? We use Tested-on tag to document that:
>>
>> https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag
>
> Hi,
>
> Yes I tested this on qcm6490-fairphone-fp5 including some extra patches
> for wpss-pas remoteproc support (nothing special, just adding it to the
> existing PAS driver) and wifi enablement in dts.
Nice, do you have a link to the patches or a git tree which has
everything? And how difficult would it be for me to run vanilla
kernel.org kernel (no vendor kernels or anything like that) on Fairphone
5? Any documentation available for that?
I'm asking because I don't have a test setup for WCN6750 right now. It
would be awesome if I could use Fairphone for testing :)
> I built this line from info from the dmesg, hope it's okay:
>
> Tested-on: wcn6750 hw1.0 AHB WLAN.MSL.1.0.1-01264-QCAMSLSWPLZ-1.37886.3
Thanks, I added that to the commit message.
> And thinking about it, a Fixes tag would also be appropriate for this
> patch. The code was moved to a different file in commit ba929d6fe31a
> ("ath11k: Remove rproc references from common core layer") but I think
> this tag should be correct.
>
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Ok, I added that as well.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi
"Luca Weiss" writes:
>> >> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> >> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> >> > > @@ -713,3 +713,7 @@ {
>> >> > > firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>> >> > > status = "okay";
>> >> > > };
>> >> > > +
>> >> > > + {
>> >> > > + status = "okay";
>> >> > qcom,ath11k-calibration-variant?
>> >>
>> >> What value would I put there for my device? Based on existing usages
>> >> (mostly for ath10k) I'd say "Fairphone_5"?
>> >
>> > I think this is fine.
>>
>> From style point of view I would prefer lower case and dashes, for
>> example "fairphone-5" but I'm just nitpicking, uppercase and underscores
>> work fine as well.
>
> I really don't mind, but I used "Fairphone_5" in v2 now, but I can
> change it for v3 if that happens if you wish.
Nah, no need to resend. That's fine.
But in the future please try to CC the ath11k list for patches like
this, easier to follow what's happening.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi
(adding ath11k list)
Dmitry Baryshkov writes:
> [Added Kalle to the CC list]
>
> On Tue, 31 Oct 2023 at 12:31, Luca Weiss wrote:
>>
>> On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
>> > On 27.10.2023 16:20, Luca Weiss wrote:
>> > > Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
>> > >
>> > > Signed-off-by: Luca Weiss
>> > > ---
>> > > arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4
>> > > 1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > index d65eef30091b..e7e20f73cbe6 100644
>> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > @@ -713,3 +713,7 @@ {
>> > > firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>> > > status = "okay";
>> > > };
>> > > +
>> > > + {
>> > > + status = "okay";
>> > qcom,ath11k-calibration-variant?
>>
>> What value would I put there for my device? Based on existing usages
>> (mostly for ath10k) I'd say "Fairphone_5"?
>
> I think this is fine.
>From style point of view I would prefer lower case and dashes, for
example "fairphone-5" but I'm just nitpicking, uppercase and underscores
work fine as well.
If you have different SKUs or similar which need different ath11k board
files being more specific like "fairphone-5-eu" and "fairphone-5-us" is
one option. But I'm sure Luca knows best what is needed for Fairphone,
just throwing out ideas here.
>> And you mean I should add this property in dts before even looking into
>> the firmware/calibration side of it?
>
> From my experience some (most?) of the device manufacturers do the
> wrong thing here. They do not program a sensible board_id, leaving it
> as 0xff or some other semi-random value. The calibration variant is
> the only way for the kernel to distinguish between such poor devices.
>
> The kernel will do a smart thing though. If the device-specific
> calibration data is not present, it will try to fall back to the
> generic data.
You are correct, just to be specific it's ath11k which will choose which
board file to use. I recommend always setting
qcom,ath11k-calibration-variant in DTS if you can.
Back in the day I have tried to push for the firmware team to improve
the board file selection but no success. So the only practical solution
we have is qcom,ath11k-calibration-variant in DTS.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wifi: ath11k: Defer on rproc_get failure
Luca Weiss writes: > If we already have gotten the rproc_handle (meaning the "qcom,rproc" > property is defined in the devicetree), it's a valid state that the > remoteproc module hasn't probed yet so we should defer probing instead > of just failing to probe. > > This resolves a race condition when the ath11k driver probes and fails > before the wpss remoteproc driver has probed, like the following: > > [6.232360] ath11k 17a10040.wifi: failed to get rproc > [6.232366] ath11k 17a10040.wifi: failed to get rproc: -22 > [6.232478] ath11k: probe of 17a10040.wifi failed with error -22 >... > [6.252415] remoteproc remoteproc2: 8a0.remoteproc is available > [6.252776] remoteproc remoteproc2: powering up 8a0.remoteproc > [6.252781] remoteproc remoteproc2: Booting fw image > qcom/qcm6490/fairphone5/wpss.mdt, size 7188 > > So, defer the probe if we hit that so we can retry later once the wpss > remoteproc is available. > > Signed-off-by: Luca Weiss Did you test this on a real device? If yes, what ath11k hardware and firmware did you use? We use Tested-on tag to document that: https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag I can add that in the pending branch if you provide the info. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 09/10] wireless: hostap: remove unused ioctl function
Arnd Bergmann writes: > From: Arnd Bergmann > > The ioctl handler has no actual callers in the kernel and is useless. > All the functionality should be reachable through the regualar interfaces. > > Signed-off-by: Arnd Bergmann In the title we prefer "wifi:" over "wireless:" but that's nitpicking. I assume this goes via a net tree so: Acked-by: Kalle Valo Let me know if I should take this to wireless-next instead. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 08/10] wireless: atmel: remove unused ioctl function
Arnd Bergmann writes: > From: Arnd Bergmann > > This function has no callers, and for the past 20 years, the request_firmware > interface has been in place instead of the custom firmware loader. > > Signed-off-by: Arnd Bergmann Yuck, good riddance. In the title we prefer "wifi:" over "wireless:" but that's nitpicking. I assume this goes via a net tree so: Acked-by: Kalle Valo Let me know if I should take this to wireless-next instead. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wl1251: Fix possible buffer overflow in wl1251_cmd_scan
Lee Gibson writes:
> Function wl1251_cmd_scan calls memcpy without checking the length.
> A user could control that length and trigger a buffer overflow.
> Fix by checking the length is within the maximum allowed size.
>
> Signed-off-by: Lee Gibson
Please fix the commit log, the user cannot control this length as
cfg80211 checks it before handling it to wl1251. Unless I'm missing
something.
> ---
> drivers/net/wireless/ti/wl1251/cmd.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wl1251/cmd.c
> b/drivers/net/wireless/ti/wl1251/cmd.c
> index 498c8db2eb48..e4d028a53d91 100644
> --- a/drivers/net/wireless/ti/wl1251/cmd.c
> +++ b/drivers/net/wireless/ti/wl1251/cmd.c
> @@ -455,8 +455,11 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t
> ssid_len,
> }
>
> cmd->params.ssid_len = ssid_len;
If you are checking the length, you should also check ssid_len here.
> - if (ssid)
> - memcpy(cmd->params.ssid, ssid, ssid_len);
> + if (ssid) {
> + int len = min_t(int, ssid_len, IEEE80211_MAX_SSID_LEN);
> +
> + memcpy(cmd->params.ssid, ssid, len);
> + }
Please use clamp_val().
Also another (and IMHO better) way to cleanup this is to provide a
pointer to struct cfg80211_ssid, which makes it clear that the length
can be trusted and not length checking is not needed. So something like
this:
int wl1251_cmd_scan(struct wl1251 *wl, const struct cfg80211_ssid *ssid,
struct ieee80211_channel *channels[],
unsigned int n_channels, unsigned int n_probes)
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] bcma: remove unused function
Jiapeng Chong wrote: > Fix the following clang warning: > > drivers/bcma/driver_mips.c:55:20: warning: unused function > 'mips_write32' [-Wunused-function]. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Patch applied to wireless-drivers-next.git, thanks. fa84df705260 bcma: remove unused function -- https://patchwork.kernel.org/project/linux-wireless/patch/1618382354-866-1-git-send-email-jiapeng.ch...@linux.alibaba.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wil6210: wmi: Remove useless code
Jiapeng Chong wrote: > Fix the following whitescan warning: > > An unsigned value can never be less than 0. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Patch applied to wireless-drivers-next.git, thanks. 5e6087559e85 wil6210: wmi: Remove useless code -- https://patchwork.kernel.org/project/linux-wireless/patch/1617788766-91433-1-git-send-email-jiapeng.ch...@linux.alibaba.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] carl9170: remove get_tid_h
Christophe JAILLET wrote: > 'get_tid_h()' is the same as 'ieee80211_get_tid()'. > So this function can be removed to save a few lines of code. > > Signed-off-by: Christophe JAILLET > Acked-by: Christian Lamparter Patch applied to wireless-drivers-next.git, thanks. cf366b154704 carl9170: remove get_tid_h -- https://patchwork.kernel.org/project/linux-wireless/patch/68efad7a597159e22771d37fc8b4a8a613866d60.1617399010.git.christophe.jail...@wanadoo.fr/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: mwl8k: Fix a double Free in mwl8k_probe_hw
Lv Yunlong wrote:
> In mwl8k_probe_hw, hw->priv->txq is freed at the first time by
> dma_free_coherent() in the call chain:
> if(!priv->ap_fw)->mwl8k_init_txqs(hw)->mwl8k_txq_init(hw, i).
>
> Then in err_free_queues of mwl8k_probe_hw, hw->priv->txq is freed
> at the second time by mwl8k_txq_deinit(hw, i)->dma_free_coherent().
>
> My patch set txq->txd to NULL after the first free to avoid the
> double free.
>
> Fixes: a66098daacee2 ("mwl8k: Marvell TOPDOG wireless driver")
> Signed-off-by: Lv Yunlong
Patch applied to wireless-drivers-next.git, thanks.
a8e083ee8e2a mwl8k: Fix a double Free in mwl8k_probe_hw
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210402182627.4256-1-lyl2...@mail.ustc.edu.cn/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [1/4] brcmfmac: Remove duplicate struct declaration
Wan Jiabing wrote: > struct brcmf_bus is declared twice. One has been declared > at 37th line. Remove the duplicate. > > Signed-off-by: Wan Jiabing 2 patches applied to wireless-drivers-next.git, thanks. d663bc3317c9 brcmfmac: Remove duplicate struct declaration 444a9af68b5c wilc1000: Remove duplicate struct declaration -- https://patchwork.kernel.org/project/linux-wireless/patch/20210331023557.2804128-2-wanjiab...@vivo.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [2/2] wl3501: fix typo of 'Networks' in comment
Eric Lin wrote: > Signed-off-by: Eric Lin > Reported-by: Gustavo A. R. Silva Patch applied to wireless-drivers-next.git, thanks. 7f50ddc5d4fe wl3501: fix typo of 'Networks' in comment -- https://patchwork.kernel.org/project/linux-wireless/patch/20210331010418.1632816-2-dslin1...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: libertas: struct lbs_private is declared duplicately
Wan Jiabing wrote: > struct lbs_private has been declared at 22nd line. > Remove the duplicate. > > Signed-off-by: Wan Jiabing Patch applied to wireless-drivers-next.git, thanks. d3240418a662 libertas: struct lbs_private is declared duplicately -- https://patchwork.kernel.org/project/linux-wireless/patch/20210325064154.854245-1-wanjiab...@vivo.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] brcmfmac: A typo fix
Bhaskar Chowdhury wrote: > s/revsion/revision/ > > Signed-off-by: Bhaskar Chowdhury Patch applied to wireless-drivers-next.git, thanks. 705b5cfab183 brcmfmac: A typo fix -- https://patchwork.kernel.org/project/linux-wireless/patch/20210323043657.1466296-1-unixbhas...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] rsi: fix comment syntax in file headers
Aditya Srivastava wrote: > The opening comment mark '/**' is used for highlighting the beginning of > kernel-doc comments. > There are some files in drivers/net/wireless/rsi which follow this syntax > in their file headers, i.e. start with '/**' like comments, which causes > unexpected warnings from kernel-doc. > > E.g., running scripts/kernel-doc -none on drivers/net/wireless/rsi/rsi_coex.h > causes this warning: > "warning: wrong kernel-doc identifier on line: > * Copyright (c) 2018 Redpine Signals Inc." > > Similarly for other files too. > > Provide a simple fix by replacing such occurrences with general comment > format, i.e., "/*", to prevent kernel-doc from parsing it. > > Signed-off-by: Aditya Srivastava > Reviewed-by: Randy Dunlap Patch applied to wireless-drivers-next.git, thanks. 3051946056c3 rsi: fix comment syntax in file headers -- https://patchwork.kernel.org/project/linux-wireless/patch/20210315173259.8757-1-yashsri...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] hostap: Fix memleak in prism2_config
Dinghao Liu wrote: > When prism2_hw_config() fails, we just return an error code > without any resource release, which may lead to memleak. > > Signed-off-by: Dinghao Liu Nobody reviewed this, so dropping the patch. Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210329085246.24586-1-dinghao@zju.edu.cn/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] brcmsmac: fix shift on 4 bit masked value
Colin King wrote:
> From: Colin Ian King
>
> The calculation of offtune_val seems incorrect, the u16 value in
> pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then
> shifted 8 places right so that always ends up as a zero result. I
> believe the intended shift was 4 bits to the right. Fix this.
>
> [Note: not tested, I don't have the H/W]
>
> Addresses-Coverity: ("Operands don't affect result")
> Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers")
> Signed-off-by: Colin Ian King
I think this needs review from someone familiar with the hardware.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210318164513.19600-1-colin.k...@canonical.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices
Pascal Terjan wrote: > Based on 2001:3319 and 2357:0109 which I used to test the fix and > 0bda:818b and 2357:0108 for which I found efuse dumps online. > > == 2357:0109 == > === Before === > Vendor: Realtek > Product: \x03802.11n NI > Serial: > === After === > Vendor: Realtek > Product: 802.11n NIC > Serial not available. > > == 2001:3319 == > === Before === > Vendor: Realtek > Product: Wireless N > Serial: no USB Adap > === After === > Vendor: Realtek > Product: Wireless N Nano USB Adapter > Serial not available. > > Signed-off-by: Pascal Terjan Can someone review this, please? Patch set to Deferred. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210323193617.3748164-1-pter...@google.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rsi: fix error return code of rsi_load_9116_firmware()
Jia-Ju Bai wrote: > When kmemdup() returns NULL to ta_firmware, no error return code of > rsi_load_9116_firmware() is assigned. > To fix this bug, status is assigned with -ENOMEM in this case. > > Reported-by: TOTE Robot > Signed-off-by: Jia-Ju Bai Someone needs to review this. Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210307083445.21322-1-baijiaju1...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] ti: wlcore: fix error return code of wl1271_cmd_build_ps_poll()
Jia-Ju Bai wrote: > When ieee80211_pspoll_get() returns NULL to skb, no error return code of > wl1271_cmd_build_ps_poll() is assigned. > To fix this bug, ret is assigned with -ENOMEM in this case. > > Reported-by: TOTE Robot > Signed-off-by: Jia-Ju Bai Someone needs to review this. Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210307082906.20950-1-baijiaju1...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] ti: wlcore: fix error return code of wl1271_suspend()
Jia-Ju Bai wrote: > When wl is NULL, no error return code of wl1271_suspend() is assigned. > To fix this bug, ret is assigned with -EINVAL in this case. > > Reported-by: TOTE Robot > Signed-off-by: Jia-Ju Bai Someone needs to review this. Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210306143600.19676-1-baijiaju1...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] marvell: libertas_tf: fix error return code of if_usb_prog_firmware()
Jia-Ju Bai wrote: > When check_fwfile_format() fails, no error return code of > if_usb_prog_firmware() is assigned. > To fix this bug, ret is assigned with -EINVAL as error return code. > > Reported-by: TOTE Robot > Signed-off-by: Jia-Ju Bai Someone needs to review this. Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210305033115.6015-1-baijiaju1...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next] airo: work around stack usage warning
Arnd Bergmann wrote: > From: Arnd Bergmann > > gcc-11 with KASAN on 32-bit arm produces a warning about a function > that needs a lot of stack space: > > drivers/net/wireless/cisco/airo.c: In function 'setup_card.constprop': > drivers/net/wireless/cisco/airo.c:3960:1: error: the frame size of 1512 bytes > is larger than 1400 bytes [-Werror=frame-larger-than=] > > Most of this is from a single large structure that could be dynamically > allocated or moved into the per-device structure. However, as the callers > all seem to have a fairly well bounded call chain, the easiest change > is to pull out the part of the function that needs the large variables > into a separate function and mark that as noinline_for_stack. This does > not reduce the total stack usage, but it gets rid of the warning and > requires minimal changes otherwise. > > Signed-off-by: Arnd Bergmann Patch applied to wireless-drivers-next.git, thanks. 7909a590eba6 airo: work around stack usage warning -- https://patchwork.kernel.org/project/linux-wireless/patch/20210323131634.2669455-1-a...@kernel.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next] wlcore: fix overlapping snprintf arguments in debugfs
Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> gcc complains about undefined behavior in calling snprintf()
> with the same buffer as input and output:
>
> drivers/net/wireless/ti/wl18xx/debugfs.c: In function
> 'diversity_num_of_packets_per_ant_read':
> drivers/net/wireless/ti/wl18xx/../wlcore/debugfs.h:86:3: error: 'snprintf'
> argument 4 overlaps destination object 'buf' [-Werror=restrict]
>86 | snprintf(buf, sizeof(buf), "%s[%d] = %d\n", \
> | ^~
>87 | buf, i, stats->sub.name[i]); \
> | ~~~
> drivers/net/wireless/ti/wl18xx/debugfs.c:24:2: note: in expansion of macro
> 'DEBUGFS_FWSTATS_FILE_ARRAY'
>24 | DEBUGFS_FWSTATS_FILE_ARRAY(a, b, c, wl18xx_acx_statistics)
> | ^~
> drivers/net/wireless/ti/wl18xx/debugfs.c:159:1: note: in expansion of macro
> 'WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY'
> 159 | WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY(diversity, num_of_packets_per_ant,
>
> There are probably other ways of handling the debugfs file, without
> using on-stack buffers, but a simple workaround here is to remember the
> current position in the buffer and just keep printing in there.
>
> Fixes: bcca1bbdd412 ("wlcore: add debugfs macro to help print fw statistics
> arrays")
> Signed-off-by: Arnd Bergmann
Patch applied to wireless-drivers-next.git, thanks.
7b0e2c4f6be3 wlcore: fix overlapping snprintf arguments in debugfs
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210323125723.1961432-1-a...@kernel.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next 4/5] libertas: avoid -Wempty-body warning
Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Building without mesh supports shows a couple of warnings with
> 'make W=1':
>
> drivers/net/wireless/marvell/libertas/main.c: In function 'lbs_start_card':
> drivers/net/wireless/marvell/libertas/main.c:1068:37: error: suggest braces
> around empty body in an 'if' statement [-Werror=empty-body]
> 1068 | lbs_start_mesh(priv);
>
> Change the macros to use the usual "do { } while (0)" instead to shut up
> the warnings and make the code a litte more robust.
>
> Signed-off-by: Arnd Bergmann
Patch applied to wireless-drivers-next.git, thanks.
01414f8882f9 libertas: avoid -Wempty-body warning
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210322104343.948660-4-a...@kernel.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtl8xxxu: Simplify locking of a skb list accesses
Christophe JAILLET wrote: > The 'c2hcmd_lock' spinlock is only used to protect some __skb_queue_tail() > and __skb_dequeue() calls. > Use the lock provided in the skb itself and call skb_queue_tail() and > skb_dequeue(). These functions already include the correct locking. > > Signed-off-by: Christophe JAILLET Patch applied to wireless-drivers-next.git, thanks. 431eb49e87ed rtl8xxxu: Simplify locking of a skb list accesses -- https://patchwork.kernel.org/project/linux-wireless/patch/8bcec6429615aeb498482dc7e1955ce09b456585.1617613700.git.christophe.jail...@wanadoo.fr/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] mwifiex: Remove unneeded variable: "ret"
zuoqil...@163.com wrote: > From: zuoqilin > > Remove unneeded variable: "ret" > > Signed-off-by: zuoqilin Patch applied to wireless-drivers-next.git, thanks. c81852a48e13 mwifiex: Remove unneeded variable: "ret" -- https://patchwork.kernel.org/project/linux-wireless/patch/20210317063353.1055-1-zuoqil...@163.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH RESEND][next] rtl8xxxu: Fix fall-through warnings for Clang
"Gustavo A. R. Silva" wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix > multiple warnings by replacing /* fall through */ comments with > the new pseudo-keyword macro fallthrough; instead of letting the > code fall through to the next case. > > Notice that Clang doesn't recognize /* fall through */ comments as > implicit fall-through markings. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva Patch applied to wireless-drivers-next.git, thanks. bf3365a856a1 rtl8xxxu: Fix fall-through warnings for Clang -- https://patchwork.kernel.org/project/linux-wireless/patch/20210305094850.GA141221@embeddedor/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] mwifiex: don't print SSID to logs
Brian Norris wrote: > There are a few reasons not to dump SSIDs as-is in kernel logs: > > 1) they're not guaranteed to be any particular text encoding (UTF-8, >ASCII, ...) in general > 2) it's somewhat redundant; the BSSID should be enough to uniquely >identify the AP/STA to which we're connecting > 3) BSSIDs have an easily-recognized format, whereas SSIDs do not (they >are free-form) > 4) other common drivers (e.g., everything based on mac80211) get along >just fine by only including BSSIDs when logging state transitions > > Additional notes on reason #3: this is important for the > privacy-conscious, especially when providing tools that convey > kernel logs on behalf of a user -- e.g., when reporting bugs. So for > example, it's easy to automatically filter logs for MAC addresses, but > it's much harder to filter SSIDs out of unstructured text. > > Signed-off-by: Brian Norris Patch applied to wireless-drivers-next.git, thanks. d23a96220353 mwifiex: don't print SSID to logs -- https://patchwork.kernel.org/project/linux-wireless/patch/20210225024454.4106485-1-briannor...@chromium.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next] rtlwifi: rtl8192de: Use DEFINE_SPINLOCK() for spinlock
Huang Guobin wrote: > From: Guobin Huang > > spinlock can be initialized automatically with DEFINE_SPINLOCK() > rather than explicitly calling spin_lock_init(). > > Reported-by: Hulk Robot > Signed-off-by: Guobin Huang Patch applied to wireless-drivers-next.git, thanks. e9642be26a37 rtlwifi: rtl8192de: Use DEFINE_SPINLOCK() for spinlock -- https://patchwork.kernel.org/project/linux-wireless/patch/1617711406-49649-1-git-send-email-huangguob...@huawei.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] qtnfmac: remove meaningless goto statement and labels
samirweng1979 wrote: > From: wengjianfeng > > some function's label meaningless, the label statement follows > the goto statement, no other statements, so just remove it. > > Reported-by: kernel test robot > Signed-off-by: wengjianfeng Patch applied to wireless-drivers-next.git, thanks. fb98734f7936 qtnfmac: remove meaningless goto statement and labels -- https://patchwork.kernel.org/project/linux-wireless/patch/20210406025206.4924-1-samirweng1...@163.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtlwifi: Simplify locking of a skb list accesses
Christophe JAILLET wrote: > The 'c2hcmd_lock' spinlock is only used to protect some __skb_queue_tail() > and __skb_dequeue() calls. > Use the lock provided in the skb itself and call skb_queue_tail() and > skb_dequeue(). These functions already include the correct locking. > > Signed-off-by: Christophe JAILLET > Acked-by: Larry Finger Patch applied to wireless-drivers-next.git, thanks. 1186006adee9 rtlwifi: Simplify locking of a skb list accesses -- https://patchwork.kernel.org/project/linux-wireless/patch/99cf8894fd52202cb7ce2ec6e3200eef400bc071.1617609346.git.christophe.jail...@wanadoo.fr/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtlwifi: remove rtl_get_tid_h
Christophe JAILLET wrote: > 'rtl_get_tid_h()' is the same as 'ieee80211_get_tid()'. > So this function can be removed to save a line of code. > > Signed-off-by: Christophe JAILLET Patch applied to wireless-drivers-next.git, thanks. 987e9bcdd0b7 rtlwifi: remove rtl_get_tid_h -- https://patchwork.kernel.org/project/linux-wireless/patch/db340a67a95c119e4f9ba8fa99aea1c73d0dcfc9.1617383263.git.christophe.jail...@wanadoo.fr/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtlwifi: rtl8188ee: remove redundant assignment of variable rtlpriv->btcoexist.reg_bt_sco
Yang Li wrote: > Assigning value "3" to "rtlpriv->btcoexist.reg_bt_sco" here, but that > stored value is overwritten before it can be used. > > Coverity reports this problem as > CWE563: A value assigned to a variable is never used. > drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c: > rtl8188ee_bt_reg_init > > Reported-by: Abaci Robot > Signed-off-by: Yang Li Patch applied to wireless-drivers-next.git, thanks. 8e04a06530c6 rtlwifi: rtl8188ee: remove redundant assignment of variable rtlpriv->btcoexist.reg_bt_sco -- https://patchwork.kernel.org/project/linux-wireless/patch/1617182023-110950-1-git-send-email-yang@linux.alibaba.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtlwifi: remove redundant assignment to variable err
Colin King wrote:
> From: Colin Ian King
>
> Variable err is assigned -ENODEV followed by an error return path
> via label error_out that does not access the variable and returns
> with the -ENODEV error return code. The assignment to err is
> redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
Patch applied to wireless-drivers-next.git, thanks.
87431bc1f0f6 rtlwifi: remove redundant assignment to variable err
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210327230014.25554-1-colin.k...@canonical.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtlwifi: Few mundane typo fixes
Bhaskar Chowdhury wrote: > s/resovle/resolve/ > s/broadcase/broadcast/ > s/sytem/system/ > > Signed-off-by: Bhaskar Chowdhury > Acked-by: Randy Dunlap Patch applied to wireless-drivers-next.git, thanks. 2377b1c49d48 rtlwifi: Few mundane typo fixes -- https://patchwork.kernel.org/project/linux-wireless/patch/20210320194426.21621-1-unixbhas...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] qtnfmac: remove meaningless labels
samirweng1979 wrote: > From: wengjianfeng > > some function's label meaningless, the return statement follows > the goto statement, so just remove it. > > Signed-off-by: wengjianfeng > Reviewed-by: Sergey Matyukevich Patch applied to wireless-drivers-next.git, thanks. a221d0afbf39 qtnfmac: remove meaningless labels -- https://patchwork.kernel.org/project/linux-wireless/patch/20210223065754.34392-1-samirweng1...@163.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] cw1200: Remove unused function pointer typedef wsm_*
Chen Lin wrote: > From: Chen Lin > > Remove the 'wsm_*' typedef as it is not used. > > Signed-off-by: Chen Lin Patch applied to wireless-drivers-next.git, thanks. 9dc5fdc8c4f8 cw1200: Remove unused function pointer typedef wsm_* -- https://patchwork.kernel.org/project/linux-wireless/patch/1613449833-4910-1-git-send-email-chen45464...@163.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] cw1200: Remove unused function pointer typedef cw1200_wsm_handler
Chen Lin wrote: > From: Chen Lin > > Remove the 'cw1200_wsm_handler' typedef as it is not used. > > Signed-off-by: Chen Lin Patch applied to wireless-drivers-next.git, thanks. 1c22233a745e cw1200: Remove unused function pointer typedef cw1200_wsm_handler -- https://patchwork.kernel.org/project/linux-wireless/patch/1613446918-4532-1-git-send-email-chen45464...@163.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] qtnfmac: Fix possible buffer overflow in qtnf_event_handle_external_auth
Lee Gibson wrote: > Function qtnf_event_handle_external_auth calls memcpy without > checking the length. > A user could control that length and trigger a buffer overflow. > Fix by checking the length is within the maximum allowed size. > > Signed-off-by: Lee Gibson Please use clamp_val() instead, that's preferred over min_t(). Patch set to Changes Requested. -- https://patchwork.kernel.org/project/linux-wireless/patch/20210317121706.389058-1-lee...@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] mac80211_hwsim: indicate support for 60GHz channels
Ramon Fontes writes: > Advertise 60GHz channels to mac80211. SoB missing: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#signed-off-by_missing -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2 0/2][next] wl3501_cs: Fix out-of-bounds warnings
"Gustavo A. R. Silva" writes: > Friendly ping: could somebody give us some feedback or take > this series, please? First patch 2 comment needs to be resolved. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/2] dt-binding: bcm43xx-fmac: add optional brcm,ccode-map
Shawn Guo writes:
> On Sun, Apr 11, 2021 at 10:57:54AM +0300, Kalle Valo wrote:
>> Shawn Guo writes:
>>
>> > Add optional brcm,ccode-map property to support translation from ISO3166
>> > country code to brcmfmac firmware country code and revision.
>> >
>> > Signed-off-by: Shawn Guo
>> > ---
>> > .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 7 +++
>> > 1 file changed, 7 insertions(+)
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> > index cffb2d6876e3..a65ac4384c04 100644
>> > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> > @@ -15,6 +15,12 @@ Optional properties:
>> >When not specified the device will use in-band SDIO interrupts.
>> > - interrupt-names : name of the out-of-band interrupt, which must be set
>> >to "host-wake".
>> > + - brcm,ccode-map : multiple strings for translating ISO3166 country code
>> > to
>> > + brcmfmac firmware country code and revision. Each string must be in
>> > + format "AA-BB-num" where:
>> > +AA is the ISO3166 country code which must be 2 characters.
>> > +BB is the firmware country code which must be 2 characters.
>> > +num is the revision number which must fit into signed integer.
>> >
>> > Example:
>> >
>> > @@ -34,5 +40,6 @@ mmc3: mmc@1c12000 {
>> >interrupt-parent = <>;
>> >interrupts = <10 8>; /* PH10 / EINT10 */
>> >interrupt-names = "host-wake";
>> > + brcm,ccode-map = "JP-JP-78", "US-Q2-86";
>>
>> The commit log does not answer "Why?". Why this needs to be in device
>> tree and, for example, not hard coded in the driver?
>
> Thanks for the comment, Kalle. Actually, this is something I need some
> input from driver maintainers. I can see this country code mapping
> table is chipset specific, and can be hard coded in driver per chip id
> and revision. But on the other hand, it makes some sense to have this
> table in device tree, as the country code that need to be supported
> could be a device specific configuration.
Could be? Does such a use case exist at the moment or are just guessing
future needs?
>From what I have learned so far I think this kind of data should be in
the driver, but of course I might be missing something.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next 3/5] iwlegacy: avoid -Wempty-body warning
Arnd Bergmann wrote: > From: Arnd Bergmann > > There are a couple of warnings in this driver when building with W=1: > > drivers/net/wireless/intel/iwlegacy/common.c: In function 'il_power_set_mode': > drivers/net/wireless/intel/iwlegacy/common.c:1195:60: error: suggest braces > around empty body in an 'if' statement [-Werror=empty-body] > 1195 | il->chain_noise_data.state); > |^ > drivers/net/wireless/intel/iwlegacy/common.c: In function 'il_do_scan_abort': > drivers/net/wireless/intel/iwlegacy/common.c:1343:57: error: suggest braces > around empty body in an 'else' statement [-Werror=empty-body] > > Change the empty debug macros to no_printk(), which avoids the > warnings and adds useful format string checks. > > Signed-off-by: Arnd Bergmann > Acked-by: Stanislaw Gruszka Patch applied to wireless-drivers-next.git, thanks. fa9f5d0e0b45 iwlegacy: avoid -Wempty-body warning -- https://patchwork.kernel.org/project/linux-wireless/patch/20210322104343.948660-3-a...@kernel.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] mt7601u: fix always true expression
Colin King wrote:
> From: Colin Ian King
>
> Currently the expression ~nic_conf1 is always true because nic_conf1
> is a u16 and according to 6.5.3.3 of the C standard the ~ operator
> promotes the u16 to an integer before flipping all the bits. Thus
> the top 16 bits of the integer result are all set so the expression
> is always true. If the intention was to flip all the bits of nic_conf1
> then casting the integer result back to a u16 is a suitabel fix.
>
> Interestingly static analyzers seem to thing a bitwise ! should be
> used instead of ~ for this scenario, so I think the original intent
> of the expression may need some extra consideration.
>
> Addresses-Coverity: ("Logical vs. bitwise operator")
> Fixes: c869f77d6abb ("add mt7601u driver")
> Signed-off-by: Colin Ian King
> Acked-by: Jakub Kicinski
Patch applied to wireless-drivers-next.git, thanks.
87fce88658ba mt7601u: fix always true expression
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210225183241.1002129-1-colin.k...@canonical.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/2] dt-binding: bcm43xx-fmac: add optional brcm,ccode-map
Shawn Guo writes:
> Add optional brcm,ccode-map property to support translation from ISO3166
> country code to brcmfmac firmware country code and revision.
>
> Signed-off-by: Shawn Guo
> ---
> .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index cffb2d6876e3..a65ac4384c04 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -15,6 +15,12 @@ Optional properties:
> When not specified the device will use in-band SDIO interrupts.
> - interrupt-names : name of the out-of-band interrupt, which must be set
> to "host-wake".
> + - brcm,ccode-map : multiple strings for translating ISO3166 country code to
> + brcmfmac firmware country code and revision. Each string must be in
> + format "AA-BB-num" where:
> + AA is the ISO3166 country code which must be 2 characters.
> + BB is the firmware country code which must be 2 characters.
> + num is the revision number which must fit into signed integer.
>
> Example:
>
> @@ -34,5 +40,6 @@ mmc3: mmc@1c12000 {
> interrupt-parent = <>;
> interrupts = <10 8>; /* PH10 / EINT10 */
> interrupt-names = "host-wake";
> + brcm,ccode-map = "JP-JP-78", "US-Q2-86";
The commit log does not answer "Why?". Why this needs to be in device
tree and, for example, not hard coded in the driver?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v5 08/24] wfx: add bus_sdio.c
Ulf Hansson writes:
>> If I follow what has been done in other drivers I would write something
>> like:
>>
>> static int wfx_sdio_suspend(struct device *dev)
>> {
>> struct sdio_func *func = dev_to_sdio_func(dev);
>> struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
>>
>> config_reg_write_bits(bus->core, CFG_IRQ_ENABLE_DATA, 0);
>> // Necessary to keep device firmware in RAM
>> return sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
>
> This will tell the mmc/sdio core to keep the SDIO card powered on
> during system suspend. Thus, it doesn't need to re-initialize it at
> system resume - and the firmware should not need to be re-programmed.
>
> On the other hand, if you don't plan to support system wakeups, it
> would probably be better to power off the card, to avoid wasting
> energy while the system is suspended. I assume that means you need to
> re-program the firmware as well. Normally, it's these kinds of things
> that need to be managed from a ->resume() callback.
Many mac80211 drivers do so that the device is powered off during
interface down (ifconfig wlan0 down), and as mac80211 does interface
down automatically during suspend, suspend then works without extra
handlers.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] hostap: Fix memleak in prism2_config
Dinghao Liu writes: > When prism2_hw_config() fails, we just return an error code > without any resource release, which may lead to memleak. > > Signed-off-by: Dinghao Liu > --- > drivers/net/wireless/intersil/hostap/hostap_cs.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/intersil/hostap/hostap_cs.c > b/drivers/net/wireless/intersil/hostap/hostap_cs.c > index ec7db2badc40..7dc16ab50ad6 100644 > --- a/drivers/net/wireless/intersil/hostap/hostap_cs.c > +++ b/drivers/net/wireless/intersil/hostap/hostap_cs.c > @@ -536,10 +536,10 @@ static int prism2_config(struct pcmcia_device *link) > sandisk_enable_wireless(dev); > > ret = prism2_hw_config(dev, 1); > - if (!ret) > - ret = hostap_hw_ready(dev); > + if (ret) > + goto failed; > > - return ret; > + return hostap_hw_ready(dev);; Two semicolons. But I'm not sure about this, can someone provide a Reviewed-by tag? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] drivers: net: wireless: struct lbs_private is declared duplicately
Wan Jiabing writes: > struct lbs_private has been declared at 22nd line. > Remove the duplicate. > > Signed-off-by: Wan Jiabing > --- > drivers/net/wireless/marvell/libertas/decl.h | 1 - > 1 file changed, 1 deletion(-) The prefix should be "libertas:", I can fix that during commit. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next] wlcore: fix overlapping snprintf arguments in debugfs
Arnd Bergmann writes:
> From: Arnd Bergmann
>
> gcc complains about undefined behavior in calling snprintf()
> with the same buffer as input and output:
>
> drivers/net/wireless/ti/wl18xx/debugfs.c: In function
> 'diversity_num_of_packets_per_ant_read':
> drivers/net/wireless/ti/wl18xx/../wlcore/debugfs.h:86:3: error:
> 'snprintf' argument 4 overlaps destination object 'buf'
> [-Werror=restrict]
>86 | snprintf(buf, sizeof(buf), "%s[%d] = %d\n", \
> | ^~
>87 | buf, i, stats->sub.name[i]); \
> | ~~~
> drivers/net/wireless/ti/wl18xx/debugfs.c:24:2: note: in expansion of
> macro 'DEBUGFS_FWSTATS_FILE_ARRAY'
>24 | DEBUGFS_FWSTATS_FILE_ARRAY(a, b, c, wl18xx_acx_statistics)
> | ^~
> drivers/net/wireless/ti/wl18xx/debugfs.c:159:1: note: in expansion of macro
> 'WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY'
> 159 | WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY(diversity, num_of_packets_per_ant,
>
> There are probably other ways of handling the debugfs file, without
> using on-stack buffers, but a simple workaround here is to remember the
> current position in the buffer and just keep printing in there.
>
> Fixes: bcca1bbdd412 ("wlcore: add debugfs macro to help print fw statistics
> arrays")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/net/wireless/ti/wlcore/boot.c| 13 -
> drivers/net/wireless/ti/wlcore/debugfs.h | 7 ---
This should go to wireless-drivers-next, not net-next.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] brcmsmac: fix shift on 4 bit masked value
Colin King writes:
> From: Colin Ian King
>
> The calculation of offtune_val seems incorrect, the u16 value in
> pi->tx_rx_cal_radio_saveregs[2] is being masked with 0xf0 and then
> shifted 8 places right so that always ends up as a zero result. I
> believe the intended shift was 4 bits to the right. Fix this.
>
> [Note: not tested, I don't have the H/W]
>
> Addresses-Coverity: ("Operands don't affect result")
> Fixes: 5b435de0d786 ("net: wireless: add brcm80211 drivers")
> Signed-off-by: Colin Ian King
Can someone ack this?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net-next] mt76: mt7921: remove unneeded semicolon
Qiheng Lin writes: > Eliminate the following coccicheck warning: > drivers/net/wireless/mediatek/mt76/mt7921/mac.c:1402:2-3: Unneeded semicolon > > Signed-off-by: Qiheng Lin mt76 patches go to the mt76 tree maintained by Felix, not to net-next. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: rtlwifi/rtl8192cu AP mode broken with PS STA
"Maciej S. Szmigiero" writes: > On 29.03.2021 00:54, Maciej S. Szmigiero wrote: >> Hi, >> >> It looks like rtlwifi/rtl8192cu AP mode is broken when a STA is using PS, >> since the driver does not update its beacon to account for TIM changes, >> so a station that is sleeping will never learn that it has packets >> buffered at the AP. >> >> Looking at the code, the rtl8192cu driver implements neither the set_tim() >> callback, nor does it explicitly update beacon data periodically, so it >> has no way to learn that it had changed. >> >> This results in the AP mode being virtually unusable with STAs that do >> PS and don't allow for it to be disabled (IoT devices, mobile phones, >> etc.). >> >> I think the easiest fix here would be to implement set_tim() for example >> the way rt2x00 driver does: queue a work or schedule a tasklet to update >> the beacon data on the device. > > Are there any plans to fix this? > The driver is listed as maintained by Ping-Ke. Yeah, power save is hard and I'm not surprised that there are drivers with broken power save mode support. If there's no fix available we should stop supporting AP mode in the driver. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
Jiri Kosina wrote: > From: Jiri Kosina > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs > disabled (e.g. from LED core). We can't enable BHs in such a situation. > > Turn the unconditional BH-enable/BH-disable code into > hardirq-disable/conditional-enable. > > This fixes the warning below. > > WARNING: CPU: 1 PID: 1139 at kernel/softirq.c:178 > __local_bh_enable_ip+0xa5/0xf0 > CPU: 1 PID: 1139 Comm: NetworkManager Not tainted > 5.12.0-rc1-4-gb4ded168af79 #7 > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017 > RIP: 0010:__local_bh_enable_ip+0xa5/0xf0 > Code: f7 69 e8 ee 23 14 00 fb 66 0f 1f 44 00 00 65 8b 05 f0 f4 f7 69 85 c0 > 74 3f 48 83 c4 08 5b c3 65 8b 05 9b fe f7 69 85 c0 75 8e <0f> 0b eb 8a 48 89 > 3c 24 e8 4e 20 14 00 48 8b 3c 24 eb 91 e8 13 4e > RSP: 0018:afd580b13298 EFLAGS: 00010046 > RAX: RBX: 0201 RCX: > RDX: 0003 RSI: 0201 RDI: c1272389 > RBP: 96517ae4c018 R08: 0001 R09: > R10: afd580b13178 R11: 0001 R12: 96517b06 > R13: R14: 8000 R15: 0001 > FS: 7fc604ebefc0() GS:96526748() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 55fb3fef13b2 CR3: 000109112004 CR4: 003706e0 > Call Trace: > ? _raw_spin_unlock_bh+0x1f/0x30 > iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi] > iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi] > iwl_trans_send_cmd+0x88/0x170 [iwlwifi] > ? lock_acquire+0x277/0x3d0 > iwl_mvm_send_cmd+0x32/0x80 [iwlmvm] > iwl_mvm_led_set+0xc2/0xe0 [iwlmvm] > ? led_trigger_event+0x46/0x70 > led_trigger_event+0x46/0x70 > ieee80211_do_open+0x5c5/0xa20 [mac80211] > ieee80211_open+0x67/0x90 [mac80211] > __dev_open+0xd4/0x150 > __dev_change_flags+0x19e/0x1f0 > dev_change_flags+0x23/0x60 > do_setlink+0x30d/0x1230 > ? lock_is_held_type+0xb4/0x120 > ? __nla_validate_parse.part.7+0x57/0xcb0 > ? __lock_acquire+0x2e1/0x1a50 > __rtnl_newlink+0x560/0x910 > ? __lock_acquire+0x2e1/0x1a50 > ? __lock_acquire+0x2e1/0x1a50 > ? lock_acquire+0x277/0x3d0 > ? sock_def_readable+0x5/0x290 > ? lock_is_held_type+0xb4/0x120 > ? find_held_lock+0x2d/0x90 > ? sock_def_readable+0xb3/0x290 > ? lock_release+0x166/0x2a0 > ? lock_is_held_type+0x90/0x120 > rtnl_newlink+0x47/0x70 > rtnetlink_rcv_msg+0x25c/0x470 > ? netlink_deliver_tap+0x97/0x3e0 > ? validate_linkmsg+0x350/0x350 > netlink_rcv_skb+0x50/0x100 > netlink_unicast+0x1b2/0x280 > netlink_sendmsg+0x336/0x450 > sock_sendmsg+0x5b/0x60 > sys_sendmsg+0x1ed/0x250 > ? copy_msghdr_from_user+0x5c/0x90 > ___sys_sendmsg+0x88/0xd0 > ? lock_is_held_type+0xb4/0x120 > ? find_held_lock+0x2d/0x90 > ? lock_release+0x166/0x2a0 > ? __fget_files+0xfe/0x1d0 > ? __sys_sendmsg+0x5e/0xa0 > __sys_sendmsg+0x5e/0xa0 > ? lockdep_hardirqs_on_prepare+0xd9/0x170 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > RIP: 0033:0x7fc605c9572d > Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 da ee ff ff 8b 54 24 1c > 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff > 77 33 44 89 c7 48 89 44 24 08 e8 2e ef ff ff 48 > RSP: 002b:7fffc83789f0 EFLAGS: 0293 ORIG_RAX: 002e > RAX: ffda RBX: 55ef468570c0 RCX: 7fc605c9572d > RDX: RSI: 7fffc8378a30 RDI: 000c > RBP: 0010 R08: R09: > R10: R11: 0293 R12: > R13: 7fffc8378b80 R14: 7fffc8378b7c R15: > irq event stamp: 170785 > hardirqs last enabled at (170783): [] > __local_bh_enable_ip+0x82/0xf0 > hardirqs last disabled at (170784): [] > _raw_read_lock_irqsave+0x8d/0x90 > softirqs last enabled at (170782): [] > iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi] > softirqs last disabled at (170785): [] > iwl_pcie_enqueue_hcmd+0x116/0xa00 [iwlwifi] > > Signed-off-by: Jiri Kosina > Tested-by: Sedat Dilek # LLVM/Clang v12.0.0-rc3 > Acked-by: Luca Coelho Patch applied to wireless-drivers.git, thanks. 2800aadc18a6 iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd() -- https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.yfh.7.76.2103021125430.12...@cbobk.fhfr.pm/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 00/10] rsi: fix comment syntax in file headers
Lukas Bulwahn writes: > On Sun, Mar 14, 2021 at 9:18 PM Aditya Srivastava > wrote: >> >> The opening comment mark '/**' is used for highlighting the beginning of >> kernel-doc comments. >> There are files in drivers/net/wireless/rsi which follow this syntax in >> their file headers, i.e. start with '/**' like comments, which causes >> unexpected warnings from kernel-doc. >> >> E.g., running scripts/kernel-doc -none on drivers/net/wireless/rsi/rsi_coex.h >> causes this warning: >> "warning: wrong kernel-doc identifier on line: >> * Copyright (c) 2018 Redpine Signals Inc." >> >> Similarly for other files too. >> >> Provide a simple fix by replacing the kernel-doc like comment syntax with >> general format, i.e. "/*", to prevent kernel-doc from parsing it. >> > > Aditya, thanks for starting to clean up the repository following your > investigation on kernel-doc warnings. > > The changes to all those files look sound. > > However I think these ten patches are really just _one change_, and > hence, all can be put into a single commit. I agree, this is one logical change to a single driver so one patch will suffice. I think for cleanup changes like this one patch per driver is a good approach. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: wilc1000: write value to WILC_INTR2_ENABLE register
Marcus Folkesson wrote:
> Write the value instead of reading it twice.
>
> Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
> Signed-off-by: Marcus Folkesson
Patch applied to wireless-drivers-next.git, thanks.
e21b6e5a5462 wilc1000: write value to WILC_INTR2_ENABLE register
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210224163706.519658-1-marcus.folkes...@gmail.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
Luca Coelho writes: > On Sat, 2021-03-13 at 16:43 +0100, Jiri Kosina wrote: >> On Sat, 13 Mar 2021, Kalle Valo wrote: >> >> > > > > From: Jiri Kosina >> > > > > >> > > > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard >> > > > > IRQs >> > > > > disabled (e.g. from LED core). We can't enable BHs in such a >> > > > > situation. >> > > > > >> > > > > Turn the unconditional BH-enable/BH-disable code into >> > > > > hardirq-disable/conditional-enable. >> > > > > >> > > > > This fixes the warning below. >> > > > >> > > > Hi, >> > > > >> > > > friendly ping on this one ... >> > > >> > > Luca, >> > > >> > > Johannes is telling me that he merged this patch internally, but I have >> > > no >> > > idea what is happening to it ... ? >> > > >> > > The reported splat is a clear bug, so it should be fixed one way or the >> > > other. >> > >> > Should I take this to wireless-drivers? >> >> I can't speak for the maintainers, but as far as I am concerned, it >> definitely is a 5.12 material, as it fixes real scheduling bug. > > Yes, please take this to w-d. We have a similar patch internally, but > there's a backlog and it will take me some time to get to it. I'll > resolve eventual conflicts when time comes. Ok, can I have your ack for patchwork? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
Jiri Kosina writes: > On Mon, 8 Mar 2021, Jiri Kosina wrote: > >> > From: Jiri Kosina >> > >> > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs >> > disabled (e.g. from LED core). We can't enable BHs in such a situation. >> > >> > Turn the unconditional BH-enable/BH-disable code into >> > hardirq-disable/conditional-enable. >> > >> > This fixes the warning below. >> >> Hi, >> >> friendly ping on this one ... > > Luca, > > Johannes is telling me that he merged this patch internally, but I have no > idea what is happening to it ... ? > > The reported splat is a clear bug, so it should be fixed one way or the > other. Should I take this to wireless-drivers? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH net] wireless/nl80211: fix wdev_id may be used uninitialized
Jarod Wilson writes: > Build currently fails with -Werror=maybe-uninitialized set: > > net/wireless/nl80211.c: In function '__cfg80211_wdev_from_attrs': > net/wireless/nl80211.c:124:44: error: 'wdev_id' may be used > uninitialized in this function [-Werror=maybe-uninitialized] Really, build fails? Is -Werror enabled by default now? I hope not. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH RESEND][next] rtl8xxxu: Fix fall-through warnings for Clang
Kees Cook writes: > On Fri, Mar 05, 2021 at 03:40:33PM +0200, Kalle Valo wrote: >> "Gustavo A. R. Silva" writes: >> >> > In preparation to enable -Wimplicit-fallthrough for Clang, fix >> > multiple warnings by replacing /* fall through */ comments with >> > the new pseudo-keyword macro fallthrough; instead of letting the >> > code fall through to the next case. >> > >> > Notice that Clang doesn't recognize /* fall through */ comments as >> > implicit fall-through markings. >> > >> > Link: https://github.com/KSPP/linux/issues/115 >> > Signed-off-by: Gustavo A. R. Silva >> >> It's not cool that you ignore the comments you got in [1], then after a >> while mark the patch as "RESEND" and not even include a changelog why it >> was resent. >> >> [1] >> https://patchwork.kernel.org/project/linux-wireless/patch/d522f387b2d0dde774785c7169c1f25aa529989d.1605896060.git.gustavo...@kernel.org/ > > Hm, this conversation looks like a miscommunication, mainly? I see > Gustavo, as requested by many others[1], replacing the fallthrough > comments with the "fallthrough" statement. (This is more than just a > "Clang doesn't parse comments" issue.) v1 was clearly rejected by Jes, so sending a new version without any changelog or comments is not the way to go. The changelog shoud at least have had "v1 was rejected but I'm resending this again because " or something like that to make it clear what's happening. > This could be a tree-wide patch and not bother you, but Greg KH has > generally advised us to send these changes broken out. Anyway, this > change still needs to land, so what would be the preferred path? I think > Gustavo could just carry it for Linus to merge without bothering you if > that'd be preferred? I agree with Greg. Please don't do cleanups like this via another tree as that just creates more work due to conflicts between the trees, which is a lot more annoying to deal with than applying few patches. But when submitting patches please follow the rules, don't cut corners. Jes, I don't like 'fallthrough' either and prefer the original comment, but the ship has sailed on this one. Maybe we should just take it? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v3] ath10k: skip the wait for completion to recovery in shutdown path
Youghandhar Chintala wrote: > Currently in the shutdown callback we wait for recovery to complete > before freeing up the resources. This results in additional two seconds > delay during the shutdown and thereby increase the shutdown time. > > As an attempt to take less time during shutdown, remove the wait for > recovery completion in the shutdown callback and added an API to freeing > the reosurces in which they were common for shutdown and removing > the module. > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1 > > Signed-off-by: Youghandhar Chintala > Signed-off-by: Kalle Valo Patch applied to ath-next branch of ath.git, thanks. 018e3fa8e7ff ath10k: skip the wait for completion to recovery in shutdown path -- https://patchwork.kernel.org/project/linux-wireless/patch/20210223142908.23374-1-yough...@codeaurora.org/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [patch 10/14] ath9k: Use tasklet_disable_in_atomic()
Thomas Gleixner writes: > From: Sebastian Andrzej Siewior > > All callers of ath9k_beacon_ensure_primary_slot() are preemptible / > acquire a mutex except for this callchain: > > spin_lock_bh(>sc_pcu_lock); > ath_complete_reset() > -> ath9k_calculate_summary_state() > -> ath9k_beacon_ensure_primary_slot() > > It's unclear how that can be distangled, so use tasklet_disable_in_atomic() > for now. This allows tasklet_disable() to become sleepable once the > remaining atomic users are cleaned up. > > Signed-off-by: Sebastian Andrzej Siewior > Signed-off-by: Thomas Gleixner > Cc: ath9k-de...@qca.qualcomm.com > Cc: Kalle Valo > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: linux-wirel...@vger.kernel.org > Cc: net...@vger.kernel.org > --- > drivers/net/wireless/ath/ath9k/beacon.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I assume this goes via some other tree: Acked-by: Kalle Valo -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH RESEND][next] rtl8xxxu: Fix fall-through warnings for Clang
"Gustavo A. R. Silva" writes: > In preparation to enable -Wimplicit-fallthrough for Clang, fix > multiple warnings by replacing /* fall through */ comments with > the new pseudo-keyword macro fallthrough; instead of letting the > code fall through to the next case. > > Notice that Clang doesn't recognize /* fall through */ comments as > implicit fall-through markings. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva It's not cool that you ignore the comments you got in [1], then after a while mark the patch as "RESEND" and not even include a changelog why it was resent. [1] https://patchwork.kernel.org/project/linux-wireless/patch/d522f387b2d0dde774785c7169c1f25aa529989d.1605896060.git.gustavo...@kernel.org/ -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
Jiri Kosina writes: > On Wed, 3 Mar 2021, Kalle Valo wrote: > >> Patch applied to wireless-drivers.git, thanks. > > Thanks, but ... > >> 295d4cd82b01 iwlwifi: don't call netif_napi_add() with rxq->lock >> held (was Re: Lockdep warning in iwl_pcie_rx_handle()) > > ... i believe you want to drop the "(was ...") part from the patch > subject. Too late now, it's already applied and pull request sent. Why was it there in the first place? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
Jiri Kosina wrote:
> From: Jiri Kosina
>
> We can't call netif_napi_add() with rxq-lock held, as there is a potential
> for deadlock as spotted by lockdep (see below). rxq->lock is not
> protecting anything over the netif_napi_add() codepath anyway, so let's
> drop it just before calling into NAPI.
>
>
> WARNING: possible irq lock inversion dependency detected
> 5.12.0-rc1-2-gbada49429032 #5 Not tainted
>
> irq/136-iwlwifi/565 just changed the state of lock:
> 89f28433b0b0 (>lock){+.-.}-{2:2}, at: iwl_pcie_rx_handle+0x7f/0x960
> [iwlwifi]
> but this lock took another, SOFTIRQ-unsafe lock in the past:
> (napi_hash_lock){+.+.}-{2:2}
>
> and interrupts could create inverse lock ordering between them.
>
> other info that might help us debug this:
> Possible interrupt unsafe locking scenario:
>
> CPU0CPU1
>
>lock(napi_hash_lock);
> local_irq_disable();
> lock(>lock);
> lock(napi_hash_lock);
>
> lock(>lock);
>
> *** DEADLOCK ***
>
> 1 lock held by irq/136-iwlwifi/565:
> #0: 89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at:
> iwl_pcie_irq_handler+0x5/0xb30
>
> the shortest dependencies between 2nd lock and 1st lock:
> -> (napi_hash_lock){+.+.}-{2:2} {
> HARDIRQ-ON-W at:
>lock_acquire+0x277/0x3d0
>_raw_spin_lock+0x2c/0x40
>netif_napi_add+0x14b/0x270
>e1000_probe+0x2fe/0xee0 [e1000e]
>local_pci_probe+0x42/0x90
>pci_device_probe+0x10b/0x1c0
>really_probe+0xef/0x4b0
>driver_probe_device+0xde/0x150
>device_driver_attach+0x4f/0x60
>__driver_attach+0x9c/0x140
>bus_for_each_dev+0x79/0xc0
>bus_add_driver+0x18d/0x220
>driver_register+0x5b/0xf0
>do_one_initcall+0x5b/0x300
>do_init_module+0x5b/0x21c
>load_module+0x1dae/0x22c0
>__do_sys_finit_module+0xad/0x110
>do_syscall_64+0x33/0x80
>entry_SYSCALL_64_after_hwframe+0x44/0xae
> SOFTIRQ-ON-W at:
>lock_acquire+0x277/0x3d0
>_raw_spin_lock+0x2c/0x40
>netif_napi_add+0x14b/0x270
>e1000_probe+0x2fe/0xee0 [e1000e]
>local_pci_probe+0x42/0x90
>pci_device_probe+0x10b/0x1c0
>really_probe+0xef/0x4b0
>driver_probe_device+0xde/0x150
>device_driver_attach+0x4f/0x60
>__driver_attach+0x9c/0x140
>bus_for_each_dev+0x79/0xc0
>bus_add_driver+0x18d/0x220
>driver_register+0x5b/0xf0
>do_one_initcall+0x5b/0x300
>do_init_module+0x5b/0x21c
>load_module+0x1dae/0x22c0
>__do_sys_finit_module+0xad/0x110
>do_syscall_64+0x33/0x80
>entry_SYSCALL_64_after_hwframe+0x44/0xae
> INITIAL USE at:
> lock_acquire+0x277/0x3d0
> _raw_spin_lock+0x2c/0x40
> netif_napi_add+0x14b/0x270
> e1000_probe+0x2fe/0xee0 [e1000e]
> local_pci_probe+0x42/0x90
> pci_device_probe+0x10b/0x1c0
> really_probe+0xef/0x4b0
> driver_probe_device+0xde/0x150
> device_driver_attach+0x4f/0x60
> __driver_attach+0x9c/0x140
> bus_for_each_dev+0x79/0xc0
> bus_add_driver+0x18d/0x220
> driver_register+0x5b/0xf0
> do_one_initcall+0x5b/0x300
> do_init_module+0x5b/0x21c
> load_module+0x1dae/0x22c0
> __do_sys_finit_module+0xad/0x110
> do_syscall_64+0x33/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>}
>... key at: [] napi_hash_lock+0x18/0x40
>... acquired at:
> _raw_spin_lock+0x2c/0x40
> netif_napi_add+0x14b/0x270
> _iwl_pcie_rx_init+0x1f4/0x710 [iwlwifi]
> iwl_pcie_rx_init+0x1b/0x3b0 [iwlwifi]
> iwl_trans_pcie_start_fw+0x2ac/0x6a0 [iwlwifi]
> iwl_mvm_load_ucode_wait_alive+0x116/0x460 [iwlmvm]
> iwl_run_init_mvm_ucode+0xa4/0x3a0 [iwlmvm]
>
Re: [PATCH] iwlwifi: fix ARCH=i386 compilation warnings
Pierre-Louis Bossart wrote:
> An unsigned long variable should rely on '%lu' format strings, not '%zd'
>
> Fixes: a1a6a4cf49ece ("iwlwifi: pnvm: implement reading PNVM from UEFI")
> Signed-off-by: Pierre-Louis Bossart
> Acked-by: Luca Coelho
Patch applied to wireless-drivers.git, thanks.
436b265671d6 iwlwifi: fix ARCH=i386 compilation warnings
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210302011640.1276636-1-pierre-louis.boss...@linux.intel.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
"Coelho, Luciano" writes:
> On Tue, 2021-03-02 at 11:34 +0100, Jiri Kosina wrote:
>> From: Jiri Kosina
>>
>> We can't call netif_napi_add() with rxq-lock held, as there is a potential
>> for deadlock as spotted by lockdep (see below). rxq->lock is not
>> protecting anything over the netif_napi_add() codepath anyway, so let's
>> drop it just before calling into NAPI.
>>
>>
>> WARNING: possible irq lock inversion dependency detected
>> 5.12.0-rc1-2-gbada49429032 #5 Not tainted
>>
>> irq/136-iwlwifi/565 just changed the state of lock:
>> 89f28433b0b0 (>lock){+.-.}-{2:2}, at:
>> iwl_pcie_rx_handle+0x7f/0x960 [iwlwifi]
>> but this lock took another, SOFTIRQ-unsafe lock in the past:
>> (napi_hash_lock){+.+.}-{2:2}
>>
>> and interrupts could create inverse lock ordering between them.
>>
>> other info that might help us debug this:
>> Possible interrupt unsafe locking scenario:
>>
>> CPU0CPU1
>>
>> lock(napi_hash_lock);
>> local_irq_disable();
>> lock(>lock);
>> lock(napi_hash_lock);
>>
>> lock(>lock);
>>
>> *** DEADLOCK ***
>>
>> 1 lock held by irq/136-iwlwifi/565:
>> #0: 89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at:
>> iwl_pcie_irq_handler+0x5/0xb30
>>
>> the shortest dependencies between 2nd lock and 1st lock:
>> -> (napi_hash_lock){+.+.}-{2:2} {
>> HARDIRQ-ON-W at:
>> lock_acquire+0x277/0x3d0
>> _raw_spin_lock+0x2c/0x40
>> netif_napi_add+0x14b/0x270
>> e1000_probe+0x2fe/0xee0 [e1000e]
>> local_pci_probe+0x42/0x90
>> pci_device_probe+0x10b/0x1c0
>> really_probe+0xef/0x4b0
>> driver_probe_device+0xde/0x150
>> device_driver_attach+0x4f/0x60
>> __driver_attach+0x9c/0x140
>> bus_for_each_dev+0x79/0xc0
>> bus_add_driver+0x18d/0x220
>> driver_register+0x5b/0xf0
>> do_one_initcall+0x5b/0x300
>> do_init_module+0x5b/0x21c
>> load_module+0x1dae/0x22c0
>> __do_sys_finit_module+0xad/0x110
>> do_syscall_64+0x33/0x80
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>> SOFTIRQ-ON-W at:
>> lock_acquire+0x277/0x3d0
>> _raw_spin_lock+0x2c/0x40
>> netif_napi_add+0x14b/0x270
>> e1000_probe+0x2fe/0xee0 [e1000e]
>> local_pci_probe+0x42/0x90
>> pci_device_probe+0x10b/0x1c0
>> really_probe+0xef/0x4b0
>> driver_probe_device+0xde/0x150
>> device_driver_attach+0x4f/0x60
>> __driver_attach+0x9c/0x140
>> bus_for_each_dev+0x79/0xc0
>> bus_add_driver+0x18d/0x220
>> driver_register+0x5b/0xf0
>> do_one_initcall+0x5b/0x300
>> do_init_module+0x5b/0x21c
>> load_module+0x1dae/0x22c0
>> __do_sys_finit_module+0xad/0x110
>> do_syscall_64+0x33/0x80
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>> INITIAL USE at:
>> lock_acquire+0x277/0x3d0
>> _raw_spin_lock+0x2c/0x40
>> netif_napi_add+0x14b/0x270
>> e1000_probe+0x2fe/0xee0 [e1000e]
>> local_pci_probe+0x42/0x90
>> pci_device_probe+0x10b/0x1c0
>> really_probe+0xef/0x4b0
>> driver_probe_device+0xde/0x150
>> device_driver_attach+0x4f/0x60
>> __driver_attach+0x9c/0x140
>> bus_for_each_dev+0x79/0xc0
>> bus_add_driver+0x18d/0x220
>> driver_register+0x5b/0xf0
>> do_one_initcall+0x5b/0x300
>> do_init_module+0x5b/0x21c
>> load_module+0x1dae/0x22c0
>> __do_sys_finit_module+0xad/0x110
>> do_syscall_64+0x33/0x80
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>> }
>> ... key at: [] napi_hash_lock+0x18/0x40
>> ... acquired at:
>> _raw_spin_lock+0x2c/0x40
>> netif_napi_add+0x14b/0x270
>> _iwl_pcie_rx_init+0x1f4/0x710 [iwlwifi]
>> iwl_pcie_rx_init+0x1b/0x3b0 [iwlwifi]
>>
Re: [PATCH] iwlwifi: fix ARCH=i386 compilation warnings
"Coelho, Luciano" writes:
> On Tue, 2021-03-02 at 07:58 +0200, Kalle Valo wrote:
>> Pierre-Louis Bossart writes:
>>
>> > An unsigned long variable should rely on '%lu' format strings, not '%zd'
>> >
>> > Fixes: a1a6a4cf49ece ("iwlwifi: pnvm: implement reading PNVM from UEFI")
>> > Signed-off-by: Pierre-Louis Bossart
>> > ---
>> > warnings found with v5.12-rc1 and next-20210301
>>
>> Luca, can I take this to wireless-drivers?
>
> Yes, please.
>
> Acked-by: Luca Coelho
Thansk. I don't see this in patchwork yet (I guess vger is slow again)
so I cannot assign it to me at the moment, will do it later.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] iwlwifi: fix ARCH=i386 compilation warnings
Pierre-Louis Bossart writes:
> An unsigned long variable should rely on '%lu' format strings, not '%zd'
>
> Fixes: a1a6a4cf49ece ("iwlwifi: pnvm: implement reading PNVM from UEFI")
> Signed-off-by: Pierre-Louis Bossart
> ---
> warnings found with v5.12-rc1 and next-20210301
Luca, can I take this to wireless-drivers?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support
Bhaumik Bhatt writes: > On 2021-03-01 03:14 AM, Kalle Valo wrote: >> + ath11k list >> >> Manivannan Sadhasivam writes: >> >>> On Fri, Feb 26, 2021 at 04:12:49PM +0530, Gokul Sriram Palanisamy >>> wrote: >>>> On platforms with two or more identical mhi >>>> devices, qmi service will run with identical >>>> qrtr-node-id. Because of this identical ID, >>>> host qrtr-lookup cannot register more than one >>>> qmi service with identical node ID. Ultimately, >>>> only one qmi service will be avilable for the >>>> underlying drivers to communicate with. >>>> >>>> On QCN9000, it implements a unique qrtr-node-id >>>> and qmi instance ID using a unique instance ID >>>> written to a debug register from host driver >>>> soon after SBL is loaded. >>>> >>>> This change generates a unique instance ID from >>>> PCIe domain number and bus number, writes to the >>>> given debug register just after SBL is loaded so >>>> that it is available for FW when the QMI service >>>> is spawned. >>>> >>>> sample: >>>> root@OpenWrt:/# qrtr-lookup >>>> Service Version Instance Node Port >>>>15 108 1 Test service >>>>69 188 2 ATH10k WLAN firmware service >>>>15 10 24 1 Test service >>>>69 1 24 24 2 ATH10k WLAN firmware service >>>> >>>> Here 8 and 24 on column 3 (QMI Instance ID) >>>> and 4 (QRTR Node ID) are the node IDs that >>>> is unique per mhi device. >>>> >>>> Signed-off-by: Gokul Sriram Palanisamy >>>> --- >>>> drivers/bus/mhi/core/boot.c | 14 ++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/bus/mhi/core/boot.c >>>> b/drivers/bus/mhi/core/boot.c >>>> index c2546bf..5e5dad5 100644 >>>> --- a/drivers/bus/mhi/core/boot.c >>>> +++ b/drivers/bus/mhi/core/boot.c >>>> @@ -16,8 +16,12 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> #include "internal.h" >>>> >>>> +#define QRTR_INSTANCE_MASK0x00FF >>>> +#define QRTR_INSTANCE_SHIFT 0 >>>> + >>>> /* Setup RDDM vector table for RDDM transfer and program RXVEC */ >>>> void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, >>>> struct image_info *img_info) >>>> @@ -391,6 +395,9 @@ void mhi_fw_load_handler(struct mhi_controller >>>> *mhi_cntrl) >>>>const struct firmware *firmware = NULL; >>>>struct image_info *image_info; >>>>struct device *dev = _cntrl->mhi_dev->dev; >>>> + struct pci_dev *pci_dev = to_pci_dev(mhi_cntrl->cntrl_dev); >>>> + struct pci_bus *bus = pci_dev->bus; >>>> + uint32_t instance; >>>>const char *fw_name; >>>>void *buf; >>>>dma_addr_t dma_addr; >>>> @@ -466,6 +473,13 @@ void mhi_fw_load_handler(struct >>>> mhi_controller *mhi_cntrl) >>>>return; >>>>} >>>> >>>> + instance = ((pci_domain_nr(bus) & 0xF) << 4) | (bus->number & 0xF); >>>> + instance &= QRTR_INSTANCE_MASK; >>>> + >>>> + mhi_write_reg_field(mhi_cntrl, mhi_cntrl->bhi, >>>> + BHI_ERRDBG2, QRTR_INSTANCE_MASK, >>>> + QRTR_INSTANCE_SHIFT, instance); >>> >>> You cannot not do this in MHI stack. Why can't you do this in the >>> MHI controller >>> specific to QCN9000? And btw, is QCN9000 supported in mainline? >> >> I'm not sure what QCN9000 means but I'm guessing it's QCN9074. We have >> initial QCN9074 support in ath11k but there are some issues still so >> it's not enabled by default (yet): >> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next=4e80946197a83a6115e308334618449b77696d6a >> >> And I suspect we have this same qrtr issue with any ath11k PCI device, >> including QCA6390, so this is not a QCN9074 specific problem. >> >> BTW Gokul, please always CC the ath11k list when submitting patches >> which are related to ath11k. > > QRTR sits on top of MHI so shouldn't this be handled outside of MHI > after MHI is operational? We cannot allow PCI code in MHI core driver > but this can be handled pre or post MHI power-up in whatever way you > desire that does not have to directly involve MHI. Sure, makes sense. I was just replying to Mani's question about status of QCN9000 upstream support. So should we handle this within ath11k, is that the right approach? I also suspect that for QCN9074 and QCA6390 we have to do this a bit differently, so it would be easier to handle the differences between devices (and firmware versions) inside ath11k. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] bus: mhi: core: Add unique qrtr node id support
+ ath11k list Manivannan Sadhasivam writes: > On Fri, Feb 26, 2021 at 04:12:49PM +0530, Gokul Sriram Palanisamy wrote: >> On platforms with two or more identical mhi >> devices, qmi service will run with identical >> qrtr-node-id. Because of this identical ID, >> host qrtr-lookup cannot register more than one >> qmi service with identical node ID. Ultimately, >> only one qmi service will be avilable for the >> underlying drivers to communicate with. >> >> On QCN9000, it implements a unique qrtr-node-id >> and qmi instance ID using a unique instance ID >> written to a debug register from host driver >> soon after SBL is loaded. >> >> This change generates a unique instance ID from >> PCIe domain number and bus number, writes to the >> given debug register just after SBL is loaded so >> that it is available for FW when the QMI service >> is spawned. >> >> sample: >> root@OpenWrt:/# qrtr-lookup >> Service Version Instance Node Port >>15 108 1 Test service >>69 188 2 ATH10k WLAN firmware service >>15 10 24 1 Test service >>69 1 24 24 2 ATH10k WLAN firmware service >> >> Here 8 and 24 on column 3 (QMI Instance ID) >> and 4 (QRTR Node ID) are the node IDs that >> is unique per mhi device. >> >> Signed-off-by: Gokul Sriram Palanisamy >> --- >> drivers/bus/mhi/core/boot.c | 14 ++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c >> index c2546bf..5e5dad5 100644 >> --- a/drivers/bus/mhi/core/boot.c >> +++ b/drivers/bus/mhi/core/boot.c >> @@ -16,8 +16,12 @@ >> #include >> #include >> #include >> +#include >> #include "internal.h" >> >> +#define QRTR_INSTANCE_MASK 0x00FF >> +#define QRTR_INSTANCE_SHIFT 0 >> + >> /* Setup RDDM vector table for RDDM transfer and program RXVEC */ >> void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, >>struct image_info *img_info) >> @@ -391,6 +395,9 @@ void mhi_fw_load_handler(struct mhi_controller >> *mhi_cntrl) >> const struct firmware *firmware = NULL; >> struct image_info *image_info; >> struct device *dev = _cntrl->mhi_dev->dev; >> +struct pci_dev *pci_dev = to_pci_dev(mhi_cntrl->cntrl_dev); >> +struct pci_bus *bus = pci_dev->bus; >> +uint32_t instance; >> const char *fw_name; >> void *buf; >> dma_addr_t dma_addr; >> @@ -466,6 +473,13 @@ void mhi_fw_load_handler(struct mhi_controller >> *mhi_cntrl) >> return; >> } >> >> +instance = ((pci_domain_nr(bus) & 0xF) << 4) | (bus->number & 0xF); >> +instance &= QRTR_INSTANCE_MASK; >> + >> +mhi_write_reg_field(mhi_cntrl, mhi_cntrl->bhi, >> +BHI_ERRDBG2, QRTR_INSTANCE_MASK, >> +QRTR_INSTANCE_SHIFT, instance); > > You cannot not do this in MHI stack. Why can't you do this in the MHI > controller > specific to QCN9000? And btw, is QCN9000 supported in mainline? I'm not sure what QCN9000 means but I'm guessing it's QCN9074. We have initial QCN9074 support in ath11k but there are some issues still so it's not enabled by default (yet): https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next=4e80946197a83a6115e308334618449b77696d6a And I suspect we have this same qrtr issue with any ath11k PCI device, including QCA6390, so this is not a QCN9074 specific problem. BTW Gokul, please always CC the ath11k list when submitting patches which are related to ath11k. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v3 0/3] Add lockdep_assert_not_held()
Peter Zijlstra writes: > On Mon, Mar 01, 2021 at 10:45:32AM +0200, Kalle Valo wrote: >> Peter Zijlstra writes: >> >> > On Fri, Feb 26, 2021 at 05:06:57PM -0700, Shuah Khan wrote: >> >> Shuah Khan (3): >> >> lockdep: add lockdep_assert_not_held() >> >> lockdep: add lockdep lock state defines >> >> ath10k: detect conf_mutex held ath10k_drain_tx() calls >> > >> > Thanks! >> >> Via which tree should these go? > > I've just queued the lot for locking/core, which will show up in tip > when the robots don't hate on it. > > Does that work for you? That's perfect, thanks! Just making sure that the patches don't get lost. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v3 0/3] Add lockdep_assert_not_held()
Peter Zijlstra writes: > On Fri, Feb 26, 2021 at 05:06:57PM -0700, Shuah Khan wrote: >> Shuah Khan (3): >> lockdep: add lockdep_assert_not_held() >> lockdep: add lockdep lock state defines >> ath10k: detect conf_mutex held ath10k_drain_tx() calls > > Thanks! Via which tree should these go? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] [v2] mt76: mt7915: fix unused 'mode' variable
+ linux-wireless
Arnd Bergmann writes:
> From: Arnd Bergmann
>
> clang points out a possible corner case in the mt7915_tm_set_tx_cont()
> function if called with invalid arguments:
>
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:593:2: warning: variable
> 'mode' is used uninitialized whenever switch default is taken
> [-Wsometimes-uninitialized]
> default:
> ^~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:597:13: note:
> uninitialized use occurs here
> rateval = mode << 6 | rate_idx;
>^~~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:506:37: note: initialize
> the variable 'mode' to silence this warning
> u8 rate_idx = td->tx_rate_idx, mode;
>^
>
> Change it to return an error instead of continuing with invalid data
> here.
>
> Fixes: 3f0caa3cbf94 ("mt76: mt7915: add support for continuous tx in
> testmode")
> Signed-off-by: Arnd Bergmann
> ---
> v2: remove the extra 'break;' after return.
> ---
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
You forgot to Cc linux-wireless and hence patchwork didn't see this, so
I applied this manually instead:
c490492f15f6 mt76: mt7915: fix unused 'mode' variable
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 3/3] PCI: Convert rtw88 power cycle quirk to shutdown quirk
Heiner Kallweit writes: > On 26.02.2021 13:18, Kai-Heng Feng wrote: >> On Fri, Feb 26, 2021 at 8:10 PM Heiner Kallweit wrote: >>> >>> On 26.02.2021 08:12, Kalle Valo wrote: >>>> Kai-Heng Feng writes: >>>> >>>>> Now we have a generic D3 shutdown quirk, so convert the original >>>>> approach to a PCI quirk. >>>>> >>>>> Signed-off-by: Kai-Heng Feng >>>>> --- >>>>> drivers/net/wireless/realtek/rtw88/pci.c | 2 -- >>>>> drivers/pci/quirks.c | 6 ++ >>>>> 2 files changed, 6 insertions(+), 2 deletions(-) >>>> >>>> It would have been nice to CC linux-wireless also on patches 1-2. I only >>>> saw patch 3 and had to search the rest of patches from lkml. >>>> >>>> I assume this goes via the PCI tree so: >>>> >>>> Acked-by: Kalle Valo >>>> >>> >>> To me it looks odd to (mis-)use the quirk mechanism to set a device >>> to D3cold on shutdown. As I see it the quirk mechanism is used to work >>> around certain device misbehavior. And setting a device to a D3 >>> state on shutdown is a normal activity, and the shutdown() callback >>> seems to be a good place for it. >>> I miss an explanation what the actual benefit of the change is. >> >> To make putting device to D3 more generic, as there are more than one >> device need the quirk. >> >> Here's the discussion: >> https://lore.kernel.org/linux-usb/00de6927-3fa6-a9a3-2d65-2b4d4e8f0...@linux.intel.com/ >> > > Thanks for the link. For the AMD USB use case I don't have a strong opinion, > what's considered the better option may be a question of personal taste. > For rtw88 however I'd still consider it over-engineering to replace a simple > call to pci_set_power_state() with a PCI quirk. > I may be biased here because I find it sometimes bothering if I want to > look up how a device is handled and in addition to checking the respective > driver I also have to grep through quirks.c whether there's any special > handling. Good point about rtw88. And if there's a new PCI id for rtw88 we need to also update the quirk in the PCI subsystem, which will be most likely forgotten. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/2] mt76: mt7915: fix unused 'mode' variable
Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> clang points out a possible corner case in the mt7915_tm_set_tx_cont()
> function if called with invalid arguments:
>
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:593:2: warning: variable
> 'mode' is used uninitialized whenever switch default is taken
> [-Wsometimes-uninitialized]
> default:
> ^~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:597:13: note:
> uninitialized use occurs here
> rateval = mode << 6 | rate_idx;
>^~~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:506:37: note: initialize
> the variable 'mode' to silence this warning
> u8 rate_idx = td->tx_rate_idx, mode;
>^
>
> Change it to return an error instead of continuing with invalid data
> here.
>
> Fixes: 3f0caa3cbf94 ("mt76: mt7915: add support for continuous tx in
> testmode")
> Signed-off-by: Arnd Bergmann
Please remove the break and send v2.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210225145953.404859-1-a...@kernel.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 2/2] mt76: mt7921: remove incorrect error handling
Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Clang points out a mistake in the error handling in
> mt7921_mcu_tx_rate_report(), which tries to dereference a pointer that
> cannot be initialized because of the error that is being handled:
>
> drivers/net/wireless/mediatek/mt76/mt7921/mcu.c:409:3: warning: variable
> 'stats' is uninitialized when used here [-Wuninitialized]
> stats->tx_rate = rate;
> ^
> drivers/net/wireless/mediatek/mt76/mt7921/mcu.c:401:32: note: initialize the
> variable 'stats' to silence this warning
> struct mt7921_sta_stats *stats;
> ^
> Just remove the obviously incorrect line.
>
> Fixes: 1c099ab44727 ("mt76: mt7921: add MCU support")
> Signed-off-by: Arnd Bergmann
> Reviewed-by: Nick Desaulniers
Patch applied to wireless-drivers.git, thanks.
fb5fabb192b2 mt76: mt7921: remove incorrect error handling
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210225145953.404859-2-a...@kernel.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 3/3] PCI: Convert rtw88 power cycle quirk to shutdown quirk
Kai-Heng Feng writes: > Now we have a generic D3 shutdown quirk, so convert the original > approach to a PCI quirk. > > Signed-off-by: Kai-Heng Feng > --- > drivers/net/wireless/realtek/rtw88/pci.c | 2 -- > drivers/pci/quirks.c | 6 ++ > 2 files changed, 6 insertions(+), 2 deletions(-) It would have been nice to CC linux-wireless also on patches 1-2. I only saw patch 3 and had to search the rest of patches from lkml. I assume this goes via the PCI tree so: Acked-by: Kalle Valo -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] iwlwifi: fix link error without CONFIG_IWLMVM
Arnd Bergmann writes:
> From: Arnd Bergmann
>
> A runtime check that was introduced recently failed to
> check for the matching Kconfig symbol:
>
> ld.lld: error: undefined symbol: iwl_so_trans_cfg
referenced by drv.c
net/wireless/intel/iwlwifi/pcie/drv.o:(iwl_pci_probe)
>
> Fixes: 930be4e76f26 ("iwlwifi: add support for SnJ with Jf devices")
> Signed-off-by: Arnd Bergmann
A sent a similar patch this morning:
https://patchwork.kernel.org/project/linux-wireless/patch/1614236661-20274-1-git-send-email-kv...@codeaurora.org/
But I forgot to include an Fixes tag, I'll add that to my patch during commit.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/2] mt76: mt7915: fix unused 'mode' variable
Arnd Bergmann writes:
> From: Arnd Bergmann
>
> clang points out a possible corner case in the mt7915_tm_set_tx_cont()
> function if called with invalid arguments:
>
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:593:2: warning: variable
> 'mode' is used uninitialized whenever switch default is taken
> [-Wsometimes-uninitialized]
> default:
> ^~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:597:13: note:
> uninitialized use occurs here
> rateval = mode << 6 | rate_idx;
>^~~~
> drivers/net/wireless/mediatek/mt76/mt7915/testmode.c:506:37: note: initialize
> the variable 'mode' to silence this warning
> u8 rate_idx = td->tx_rate_idx, mode;
>^
>
> Change it to return an error instead of continuing with invalid data
> here.
>
> Fixes: 3f0caa3cbf94 ("mt76: mt7915: add support for continuous tx in
> testmode")
> Signed-off-by: Arnd Bergmann
Felix, can I take these two to wireless-drivers? An ack would be good.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] qtnfmac: remove meaningless goto statement and labels
kernel test robot writes: > Hi samirweng1979, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on wireless-drivers-next/master] > [also build test ERROR on wireless-drivers/master sparc-next/master v5.11 > next-20210225] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: > https://github.com/0day-ci/linux/commits/samirweng1979/qtnfmac-remove-meaningless-goto-statement-and-labels/20210225-145714 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git > master > config: x86_64-randconfig-a001-20210225 (attached as .config) > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project > a921aaf789912d981cbb2036bdc91ad7289e1523) > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # install x86_64 cross compiling tool for clang build > # apt-get install binutils-x86-64-linux-gnu > # > https://github.com/0day-ci/linux/commit/d18bea1fd25dee219ae56343ff9caf9cb6eb1519 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review > samirweng1979/qtnfmac-remove-meaningless-goto-statement-and-labels/20210225-145714 > git checkout d18bea1fd25dee219ae56343ff9caf9cb6eb1519 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > ARCH=x86_64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >>> drivers/net/wireless/quantenna/qtnfmac/commands.c:1901:8: error: use of >>> undeclared label 'out' >goto out; Do you compile test your patches? This error implies that not. Compilation test is a hard requirement for patches. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: linux-next: Tree for Feb 23 [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko]
Randy Dunlap writes: > On 2/22/21 10:35 PM, Stephen Rothwell wrote: >> Hi all, >> >> Please do not add any changes destined for v5.13 to your linux-next >> included branches until after v5.12-rc1 has been released. >> >> Changes since 20210222: >> > > Still seeing this build error on x86_64: > > ERROR: modpost: "iwl_so_trans_cfg" > [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined! > > > Full randconfig file is attached. Still? I don't recall seeing this report before, but maybe I have missed it. Anyway, I sent a fix but it would be great if you can test it: https://patchwork.kernel.org/project/linux-wireless/patch/1614236661-20274-1-git-send-email-kv...@codeaurora.org/ -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
writes:
> On 24/02/21 10:13 pm, Kalle Valo wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> Marcus Folkesson writes:
>>
>>> Write the value instead of reading it twice.
>>>
>>> Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for function
>>> in wilc_sdio.c file")
>>>
>>> Signed-off-by: Marcus Folkesson
>>> ---
>>> drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
>>> b/drivers/net/wireless/microchip/wilc1000/sdio.c
>>> index 351ff909ab1c..e14b9fc2c67a 100644
>>> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
>>> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
>>> @@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int
>>> nint)
>>> for (i = 0; (i < 3) && (nint > 0); i++, nint--)
>>> reg |= BIT(i);
>>>
>>> - ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE,
>>> );
>>> + ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE,
>>> reg);
>>
>> To me it looks like the bug existed before commit 5e63a598441a:
>
>
> Yes, you are correct. The bug existed from commit c5c77ba18ea6:
>
> https://git.kernel.org/linus/c5c77ba18ea6
So the fixes tag should be:
Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
I can change that during commit, ok?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
Marcus Folkesson writes:
> Write the value instead of reading it twice.
>
> Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for function in
> wilc_sdio.c file")
>
> Signed-off-by: Marcus Folkesson
> ---
> drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
> b/drivers/net/wireless/microchip/wilc1000/sdio.c
> index 351ff909ab1c..e14b9fc2c67a 100644
> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> @@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int nint)
> for (i = 0; (i < 3) && (nint > 0); i++, nint--)
> reg |= BIT(i);
>
> - ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE, );
> + ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE, reg);
To me it looks like the bug existed before commit 5e63a598441a:
- ret = sdio_read_reg(wilc, WILC_INTR2_ENABLE, );
+ ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE, );
https://git.kernel.org/linus/5e63a598441a
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] ath11k: qmi: use %pad to format dma_addr_t
Geert Uytterhoeven wrote:
> If CONFIG_ARCH_DMA_ADDR_T_64BIT=n:
>
> drivers/net/wireless/ath/ath11k/qmi.c: In function
> ‘ath11k_qmi_respond_fw_mem_request’:
> drivers/net/wireless/ath/ath11k/qmi.c:1690:8: warning: format ‘%llx’
> expects argument of type ‘long long unsigned int’, but argument 5 has type
> ‘dma_addr_t’ {aka ‘unsigned int’} [-Wformat=]
> 1690 |"qmi req mem_seg[%d] 0x%llx %u %u\n", i,
> |^~~~
> 1691 | ab->qmi.target_mem[i].paddr,
> | ~~~
> | |
> | dma_addr_t {aka unsigned int}
> drivers/net/wireless/ath/ath11k/debug.h:64:30: note: in definition of
> macro ‘ath11k_dbg’
>64 | __ath11k_dbg(ar, dbg_mask, fmt, ##__VA_ARGS__); \
> | ^~~
> drivers/net/wireless/ath/ath11k/qmi.c:1690:34: note: format string is
> defined here
> 1690 |"qmi req mem_seg[%d] 0x%llx %u %u\n", i,
> | ~~~^
> | |
> | long long unsigned int
> | %x
>
> Fixes: d5395a5486596308 ("ath11k: qmi: add debug message for allocated memory
> segment addresses and sizes")
> Signed-off-by: Geert Uytterhoeven
Patch applied to wireless-drivers.git, thanks.
ebb9d34e073d ath11k: qmi: use %pad to format dma_addr_t
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210221182754.2071863-1-ge...@linux-m68k.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] rtw88: 8822ce: fix wifi disconnect after S3/S4 on HONOR laptop
Pkshih writes:
> On Tue, 2021-02-23 at 09:08 +0200, Kalle Valo wrote:
>> Pkshih writes:
>>
>> >> > --- a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
>> >> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
>> >> > @@ -25,7 +25,6 @@ static struct pci_driver rtw_8822ce_driver = {
>> >> > .id_table = rtw_8822ce_id_table,
>> >> > .probe = rtw_pci_probe,
>> >> > .remove = rtw_pci_remove,
>> >> > - .driver.pm = _pm_ops,
>> >>
>> >> Why just 8822ce? Why not remove rtw_pm_ops entirely if it just creates
>> >> problems?
>> >
>> > I think we can't remove rtw_pm_ops, because wowlan will not work.
>>
>> Ah. A comment code in the code stating that would be nice.
>>
>
> I'll do it.
Thank you.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] rtw88: 8822ce: fix wifi disconnect after S3/S4 on HONOR laptop
Pkshih writes:
>> > --- a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
>> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
>> > @@ -25,7 +25,6 @@ static struct pci_driver rtw_8822ce_driver = {
>> > .id_table = rtw_8822ce_id_table,
>> > .probe = rtw_pci_probe,
>> > .remove = rtw_pci_remove,
>> > - .driver.pm = _pm_ops,
>>
>> Why just 8822ce? Why not remove rtw_pm_ops entirely if it just creates
>> problems?
>
> I think we can't remove rtw_pm_ops, because wowlan will not work.
Ah. A comment code in the code stating that would be nice.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2] rtw88: 8822ce: fix wifi disconnect after S3/S4 on HONOR laptop
Hao Chen writes:
> The laptop's wifi disconnect after the laptop HONOR MagicBook 14
> sleep to S3/S4 and wake up.
>
> The dmesg of kernel report:
> "[ 99.990168] pcieport :00:01.2: can't change power state from D3hot
> to D0 (config space inaccessible)
> [ 99.990176] ACPI: EC: interrupt unblocked
> [ 99.993334] rtw_pci :01:00.0: can't change power state from D3hot
> to D0 (config space inaccessible)
> ..
> [ 102.133500] rtw_pci :01:00.0: mac power on failed
> [ 102.133503] rtw_pci :01:00.0: failed to power on mac
> [ 102.133505] [ cut here ]
> [ 102.133506] Hardware became unavailable upon resume. This could be a
> software issue prior to suspend or a hardware issue.
> [ 102.133569] WARNING: CPU: 4 PID: 5612 at net/mac80211/util.c:2232
> ieee80211_reconfig+0x9b/0x1490 [mac80211]
> [ 102.133570] Modules linked in: ccm rfcomm uvcvideo videobuf2_vmalloc
> videobuf2_memops videobuf2_v4l2 videobuf2_common videodev mc cmac bnep
> btusb btrtl btbcm btintel edac_mce_amd bluetooth kvm_amd ecdh_generic
> ecc kvm nls_iso8859_1 rtwpci rtw88 crct10dif_pclmul crc32_pclmul mac80211
> ghash_clmulni_intel aesni_intel snd_hda_codec_realtek crypto_simd huawei_wmi
> snd_hda_codec_generic cryptd cfg80211 wmi_bmof serio_raw sparse_keymap
> ledtrig_audio sp5100_tco glue_helper joydev snd_hda_codec_hdmi snd_hda_intel
> snd_intel_dspcfg wdat_wdt snd_hda_codec snd_hda_core pcspkr snd_hwdep snd_pcm
> efi_pstore snd_timer libarc4 k10temp snd soundcore snd_pci_acp3x ccp mac_hid
> binfmt_misc ip_tables x_tables autofs4 amdgpu amd_iommu_v2 gpu_sched
> i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops
> usbmouse cec nvme hid_generic i2c_piix4 usbhid nvme_core drm wmi video
> [ 102.133617] CPU: 4 PID: 5612 Comm: kworker/u32:16 Not tainted
> 5.7.7-amd64-desktop-8822 #3
> [ 102.133618] Hardware name: HUAWEI NBLL-WXX9/NBLL-WXX9-PCB, BIOS 1.06
> 09/29/2020
> [ 102.133623] Workqueue: events_unbound async_run_entry_fn
> [ 102.133651] RIP: 0010:ieee80211_reconfig+0x9b/0x1490 [mac80211]
> [ 102.133654] Code: 31 db e8 e8 fb 27 c2 41 c6 85 34 05 00 00 00 4c 89 ef e8
> 38
> 56 fc ff 89 45 b8 85 c0 74 4c 48 c7 c7 d0 0c 09 c1 e8 01 e0 25 c2 <0f> 0b 4c
> 89 ef e8 2b d1 ff ff e9 02 03 00 00 80 7d 9f 00 0f 85 1d
> [ 102.133655] RSP: 0018:be52c059fd08 EFLAGS: 00010286
> [ 102.133657] RAX: RBX: RCX:
> 0007
> [ 102.133658] RDX: 0007 RSI: 0096 RDI:
> 9d573f519cc0
> [ 102.133659] RBP: be52c059fd80 R08: ffd96245 R09:
> 0002cb80
> [ 102.133660] R10: 00016989e54c R11: 0002a360 R12:
> 9d5731f50300
> [ 102.133661] R13: 9d5731f50800 R14: 9d5731f504c8 R15:
> 8463fbef
> [ 102.133664] FS: () GS:9d573f50()
> knlGS:
> [ 102.133665] CS: 0010 DS: ES: CR0: 80050033
> [ 102.133666] CR2: CR3: 00033320a000 CR4:
> 00340ee0
> [ 102.133667] Call Trace:
> [ 102.133673] ? enqueue_entity+0xe3/0x680
> [ 102.133705] ieee80211_resume+0x55/0x70 [mac80211]
> [ 102.133729] wiphy_resume+0x84/0x130 [cfg80211]
> [ 102.133752] ? addresses_show+0xa0/0xa0 [cfg80211]
> [ 102.133757] dpm_run_callback+0x5b/0x150
> [ 102.133760] device_resume+0xad/0x1f0
> [ 102.133762] async_resume+0x1d/0x30
> [ 102.133764] async_run_entry_fn+0x3e/0x170
> [ 102.133768] process_one_work+0x1ab/0x380
> [ 102.133771] worker_thread+0x37/0x3b0
> [ 102.133774] kthread+0x120/0x140
> [ 102.133776] ? create_worker+0x1b0/0x1b0
> [ 102.133778] ? kthread_park+0x90/0x90
> [ 102.133782] ret_from_fork+0x22/0x40
> [ 102.133785] ---[ end trace 46229bfd3a4273be ]---
> [ 102.134137] [ cut here ]
> [ 102.134141] wlp1s0: Failed check-sdata-in-driver check, flags: 0x0
> [ 102.134195] WARNING: CPU: 0 PID: 5612 at net/mac80211/driver-ops.h:19
> drv_remove_interface+0xfe/0x110 [mac80211]"
>
> When try to pointer the driver.pm to NULL, the problem is fixed.
> It makes the sleep and wake procedure expected when pm's ops not NULL.
>
> By `git blame` command, I know that the assignment of .driver.pm =
> RTW_PM_OPS was in commit 44bc17f7f5b3 ("rtw88: support wowlan feature for
> 8822c"), and another
> commit 7dc7c41607d1 ("rtw88: avoid unused function warnings")
> pointed out rtw_pci_resume() and rtw_pci_suspend() are not used at
> all.
>
> So I think it's safe to remove them.
>
> Fixes: 7dc7c41607d1 ("rtw88: avoid unused function warnings")
> Fixes: 44bc17f7f5b3 ("rtw88: support wowlan feature for 8822c")
>
> Signed-off-by: Hao Chen
> ---
> drivers/net/wireless/realtek/rtw88/rtw8822ce.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
> b/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
> index 3845b1333dc3..4c063192f801 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
> +++
Re: [PATCH] Revert "ath9k: fix ath_tx_process_buffer() potential null ptr dereference"
Shuah Khan wrote:
> This reverts commit a56c14bb21b296fb6d395164ab62ef2e419e5069.
>
> ath_tx_process_buffer() doesn't dereference or check sta and passes it
> to ath_tx_complete_aggr() and ath_tx_complete_buf().
>
> ath_tx_complete_aggr() checks the pointer before use. No problem here.
>
> ath_tx_complete_buf() doesn't check or dereference sta and passes it on
> to ath_tx_complete(). ath_tx_complete() doesn't check or dereference sta,
> but assigns it to tx_info->status.status_driver_data[0]
>
> ath_tx_complete_buf() is called from ath_tx_complete_aggr() passing
> null ieee80211_sta pointer.
>
> There is a potential for dereference later on, if and when the
> tx_info->status.status_driver_data[0]is referenced. In addition, the
> rcu read lock might be released before referencing the contents.
>
> ath_tx_complete_buf() should be fixed to check sta perhaps? Worth
> looking into.
>
> Reverting this patch because it doesn't solve the problem and introduces
> memory leak by skipping buffer completion if the pointer (sta) is NULL.
>
> Fixes: a56c14bb21b2 ("ath9k: fix ath_tx_process_buffer() potential null ptr
> dereference")
> Signed-off-by: Shuah Khan
> Signed-off-by: Kalle Valo
Patch applied to ath-next branch of ath.git, thanks.
14ebaeeff8d0 Revert "ath9k: fix ath_tx_process_buffer() potential null ptr
dereference"
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210217211801.22540-1-sk...@linuxfoundation.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtw88: 8822ce: fix wifi disconnect after S3/S4 on HONOR laptop
陈浩 writes:
> By git blame command, I know that the assignment of .driver.pm =
> RTW_PM_OPS
>
> was in commit 44bc17f7f5b3b("rtw88: support wowlan feature for
> 8822c"),
>
> and another commit 7dc7c41607d19("avoid unused function warnings")
>
> pointed out rtw_pci_resume() and rtw_pci_suspend() are not used at
> all.
>
> So I think it's safe to remove them.
>
> Currently, I find that the rtl8822ce wifi chip and the pci bridge of
> it are not linked by pci
>
> after wake up by `lspci` command.
>
> when I set `pcie_aspm.policy=performance ` in the GRUB. The machine
> sleep and
>
> wake up normal.So I think when this ops is assignmented,the sleep and
> wake up procedure
>
> may cause pci device not linked.
Please don't use HTML, our lists automatically drop all HTML email.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH] rtw88: 8822ce: fix wifi disconnect after S3/S4 on HONOR laptop
Hao Chen writes:
> When the laptop HONOR MagicBook 14 sleep to S3/S4, the laptop can't
> resume.
> The dmesg of kernel report:
> "[ 99.990168] pcieport :00:01.2: can't change power state
> from D3hot to D0 (config space inaccessible)
> [ 99.993334] rtw_pci :01:00.0: can't change power state
> from D3hot to D0 (config space inaccessible)
> [ 104.435004] rtw_pci :01:00.0: mac power on failed
> [ 104.435010] rtw_pci :01:00.0: failed to power on mac"
> When try to pointer the driver.pm to NULL, the problem is fixed.
> This driver hasn't implemented pm ops yet.It makes the sleep and
> wake procedure expected when pm's ops not NULL.
But why rtw_pci_suspend() and rtw_pci_resume() are empty? Should we just
remove them if they cause issues for the users? And if they are really
needed there should be a comment in the functions explaining the
situation.
> Fixed: commit e3037485c68e ("rtw88: new Realtek 802.11ac driver")
This should be:
Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: linux-next: build warnings after merge of the net-next tree
Stephen Rothwell writes:
> Hi all,
>
> On Fri, 19 Feb 2021 07:52:56 +1100 Stephen Rothwell
> wrote:
>>
>> After merging the net-next tree, today's linux-next build (htmldocs)
>> produced these warnings:
>>
>> Documentation/networking/filter.rst:1053: WARNING: Inline emphasis
>> start-string without end-string.
>> Documentation/networking/filter.rst:1053: WARNING: Inline emphasis
>> start-string without end-string.
>> Documentation/networking/filter.rst:1053: WARNING: Inline emphasis
>> start-string without end-string.
>> Documentation/networking/filter.rst:1053: WARNING: Inline emphasis
>> start-string without end-string.
>>
>> Introduced by commit
>>
>> 91c960b00566 ("bpf: Rename BPF_XADD and prepare to encode other atomics in
>> .imm")
>>
>> Sorry that I missed these earlier.
>
> These have been fixed in the net-next tree, actually. I was fooled
> because an earlier part of the net-next tree has been included in the
> wireless-drivers (not -next) tree today so these warnings popped up
> earlier, but are gone one the rest of the net-next tree is merged.
>
> Sorry for the noise.
Argh, sorry about that Stephen. I was preparing wireless-drivers for
followup fixes sent during the merge window, but didn't realise that it
will mess up your tree building. I need to avoid this in the future and
wireless-drivers should only follow the net tree.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: undefined reference to `led_classdev_unregister'
kernel test robot writes: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: f40ddce88593482919761f74910f42f4b84c004b > commit: 72cdab808714b1ec24b0c7bdebed163ce791f01f ath9k: Do not select > MAC80211_LEDS by default > date: 6 months ago > config: parisc-randconfig-r002-20210218 (attached as .config) > compiler: hppa64-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72cdab808714b1ec24b0c7bdebed163ce791f01f > git remote add linus > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > git fetch --no-tags linus master > git checkout 72cdab808714b1ec24b0c7bdebed163ce791f01f > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=parisc > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >hppa64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function > `.LC54': >>> (.data.rel.ro+0xe8): undefined reference to `led_classdev_unregister' >hppa64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function > `.LC64': >>> (.data.rel.ro+0x120): undefined reference to `led_classdev_register_ext' This commit should fix the issue: b64acb28da83 ath9k: fix build error with LEDS_CLASS=m -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
