* [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef
From: Lachlan Hodges @ 2026-03-11 6:43 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, arien.judge, Lachlan Hodges
When creating a chandef, the initial width is determined by the
chan_type argument leading to a default width of 20/20_NOHT/40 which
does not make sense for S1G channels. Fix this by ensuring we default
S1G chandefs to a width of 1MHz.
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:
- Remove the mac80211-based wording in the commit message and keep it
simple.
---
net/wireless/chan.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 68221b1ab45e..c0c0151fe55f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -32,6 +32,12 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
.freq1_offset = chan->freq_offset,
};
+ if (cfg80211_chandef_is_s1g(chandef)) {
+ chandef->width = NL80211_CHAN_WIDTH_1;
+ chandef->center_freq1 = chan->center_freq;
+ return;
+ }
+
switch (chan_type) {
case NL80211_CHAN_NO_HT:
chandef->width = NL80211_CHAN_WIDTH_20_NOHT;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH wireless-next] wifi: cfg80211: init S1G properly when creating chandef
From: Lachlan Hodges @ 2026-03-11 6:27 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, arien.judge
In-Reply-To: <62057e8c13c4b6358dcb42bd639aec2996cf2bca.camel@sipsolutions.net>
On Wed, Mar 11, 2026 at 07:24:00AM +0100, Johannes Berg wrote:
> On Wed, 2026-03-11 at 17:18 +1100, Lachlan Hodges wrote:
> > When emulating channel contexts, the first chandef passed to the
> > driver when starting an interface is the local->dflt_chandef
> > via ieee80211_calc_hw_conf_chan(). This means that currently an
> > S1G driver will be given a chandef with a width of 20MHz. This will
> > lead to either the driver or firmware rejecting this configuration
> > and a non-zero ret code - leading to a WARN within
> > ieee80211_hw_conf_init().
> >
> > In the case of emulation, ensure we initialise with a width of
> > 1MHz for S1G channels such that the default chandef is valid.
>
> I haven't had my morning tea ;-)
That was a fast reply ^.^
> But the commit message feels to me like it's conflating cfg80211 and
> mac80211 concepts? You change cfg80211 - but then start out saying "when
> emulating" which happens in mac80211?
Yea good point, it's probably makes more sense to simply say what
it is which is we aren't initialising the chandef properly for an S1G
channel. Will send a v2.
lachlan
^ permalink raw reply
* Re: [PATCH wireless-next] wifi: cfg80211: init S1G properly when creating chandef
From: Johannes Berg @ 2026-03-11 6:24 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: linux-wireless, arien.judge
In-Reply-To: <20260311061800.517849-1-lachlan.hodges@morsemicro.com>
On Wed, 2026-03-11 at 17:18 +1100, Lachlan Hodges wrote:
> When emulating channel contexts, the first chandef passed to the
> driver when starting an interface is the local->dflt_chandef
> via ieee80211_calc_hw_conf_chan(). This means that currently an
> S1G driver will be given a chandef with a width of 20MHz. This will
> lead to either the driver or firmware rejecting this configuration
> and a non-zero ret code - leading to a WARN within
> ieee80211_hw_conf_init().
>
> In the case of emulation, ensure we initialise with a width of
> 1MHz for S1G channels such that the default chandef is valid.
I haven't had my morning tea ;-)
But the commit message feels to me like it's conflating cfg80211 and
mac80211 concepts? You change cfg80211 - but then start out saying "when
emulating" which happens in mac80211?
johannes
^ permalink raw reply
* [PATCH wireless-next] wifi: cfg80211: init S1G properly when creating chandef
From: Lachlan Hodges @ 2026-03-11 6:18 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, arien.judge, Lachlan Hodges
When emulating channel contexts, the first chandef passed to the
driver when starting an interface is the local->dflt_chandef
via ieee80211_calc_hw_conf_chan(). This means that currently an
S1G driver will be given a chandef with a width of 20MHz. This will
lead to either the driver or firmware rejecting this configuration
and a non-zero ret code - leading to a WARN within
ieee80211_hw_conf_init().
In the case of emulation, ensure we initialise with a width of
1MHz for S1G channels such that the default chandef is valid.
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
net/wireless/chan.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 68221b1ab45e..c0c0151fe55f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -32,6 +32,12 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
.freq1_offset = chan->freq_offset,
};
+ if (cfg80211_chandef_is_s1g(chandef)) {
+ chandef->width = NL80211_CHAN_WIDTH_1;
+ chandef->center_freq1 = chan->center_freq;
+ return;
+ }
+
switch (chan_type) {
case NL80211_CHAN_NO_HT:
chandef->width = NL80211_CHAN_WIDTH_20_NOHT;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 11/14] media: qcom: Switch to generic PAS TZ APIs
From: Sumit Garg @ 2026-03-11 5:47 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Jorge Ramirez, linux-arm-msm, devicetree, dri-devel, freedreno,
linux-media, netdev, linux-wireless, ath12k, linux-remoteproc,
andersson, konradybcio, robh, krzk+dt, conor+dt, robin.clark,
sean, akhilpo, lumag, abhinav.kumar, jesszhan0024, marijn.suijten,
airlied, simona, vikash.garodia, dikshita.agarwal, bod, mchehab,
elder, andrew+netdev, davem, edumazet, kuba, pabeni, jjohnson,
mathieu.poirier, trilokkumar.soni, mukesh.ojha, pavan.kondeti,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, Sumit Garg
In-Reply-To: <a96cca02-54e4-4ed0-9fcb-816c72afe0e7@oss.qualcomm.com>
On Tue, Mar 10, 2026 at 12:18:29PM +0100, Konrad Dybcio wrote:
> On 3/9/26 10:12 AM, Jorge Ramirez wrote:
> > On 06/03/26 16:20:24, Sumit Garg wrote:
> >> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >>
> >> Switch qcom media client drivers over to generic PAS TZ APIs. Generic PAS
> >> TZ service allows to support multiple TZ implementation backends like QTEE
> >> based SCM PAS service, OP-TEE based PAS service and any further future TZ
> >> backend service.
> >>
> >> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >> ---
>
> [...]
>
> > are the calls to set_remote_state required?
> > 0 is not the IRIS/VENUS remote processor.
> >
> > If it is legacy, maybe they can be phased out?
>
> FWIW, in msm-3.10 (yes) QCOM_SCM_SVC_BOOT/0x0a used to be called
> 'TZBSP_VIDEO_SET_STATE', taking the arguments:
>
> args[0] = suspend->0, resume->1
> args[1] = spare=0
>
> seems like sdm845/msm-4.9 still had that and so did 8250/msm-4.19
I would assume passing actual PAS ID in that spare argument shouldn't
cause any issues for QTEE as it could just ignore that. Let me try to
see in my testing on RB3Gen2 if it causes any ABI issues. Since with
OP-TEE we would like to not deal with ABI inconsistencies.
-Sumit
^ permalink raw reply
* Re: [PATCH ath-next] wifi: ath12k: avoid dynamic alloc when parsing wmi tb
From: Baochen Qiang @ 2026-03-11 5:46 UTC (permalink / raw)
To: Nicolas Escande, ath12k; +Cc: linux-wireless
In-Reply-To: <DGZ17385SNYX.3149KUG92UUU1@gmail.com>
On 3/10/2026 6:31 PM, Nicolas Escande wrote:
> On Tue Mar 10, 2026 at 3:05 AM CET, Baochen Qiang wrote:
> [...]
>>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>>> index 59c193b24764..ebe7b94fd712 100644
>>> --- a/drivers/net/wireless/ath/ath12k/core.h
>>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>>> @@ -19,6 +19,7 @@
>>> #include <linux/average.h>
>>> #include <linux/of.h>
>>> #include <linux/rhashtable.h>
>>> +#include <linux/percpu.h>
>>> #include "qmi.h"
>>> #include "htc.h"
>>> #include "wmi.h"
>>> @@ -937,6 +938,7 @@ struct ath12k_base {
>>> struct device *dev;
>>> struct ath12k_qmi qmi;
>>> struct ath12k_wmi_base wmi_ab;
>>> + void __percpu *wmi_tb;
>>
>> any reason why my v1 suggestion is not considered?
>>
>
> I considered it but I for sure did not write enough about it in the changelog.
> Sorry about that, see my thoughts bellow.
>
>> instead of allocating it per device, how about making it global and define/allocate once
>> when loading driver. This way we may save some memory in case where more than one devices
>> get probed?
>
> So what I did try first is to use DEFINE_PER_CPU() directly in wmi.c to have
> this as a static array, directly in the compilation unit where it is used.
> But this failled at runtime as the allocated size it too big and it would have
hmm, I didn't expect that.
> needed modifying the max alloc size that the module loader would allow.
Let's not do this.
>
> My second option was to add something in the module_init so ath12k_wifi7_init().
> But as there is no ath12k 'global' struct, it meant adding a global variable to
> hold the per cpu array, that would be alloced in wifi7/core.c and used in wmi.c.
> And that felt weird too, the wifi version specific part of it at least.
Agree.
>
> So I stayed with the per ath12k_base, which as relatively low overhead for a
> clean consistant interface.
>
> But if you guys don't want it that way, I can rework it. Just tell me in more
> details what you think is the right way and I can modify it.
then how about adding module init path to ath12k module and do percpu allocation there:
in ath12k/core.c
+void __percpu *wmi_tb;
+
+static int ath12k_core_module_init(void)
+{
+ wmi_tb = __alloc_percpu(WMI_TAG_MAX * sizeof(void *),
+ __alignof__(void *));
+ if (!wmi_tb)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void ath12k_core_module_exit(void)
+{
+ free_percpu(wmi_tb);
+}
+
+module_init(ath12k_core_module_init);
+module_exit(ath12k_core_module_exit);
>
> Thanks
^ permalink raw reply
* Re: [PATCH 15/61] trace: Prefer IS_ERR_OR_NULL over manual NULL check
From: Masami Hiramatsu @ 2026-03-11 5:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel,
dri-devel, gfs2, intel-gfx, intel-wired-lan, iommu, kvm,
linux-arm-kernel, linux-block, linux-bluetooth, linux-btrfs,
linux-cifs, linux-clk, linux-erofs, linux-ext4, linux-fsdevel,
linux-gpio, linux-hyperv, linux-input, linux-kernel, linux-leds,
linux-media, linux-mips, linux-mm, linux-modules, linux-mtd,
linux-nfs, linux-omap, linux-phy, linux-pm, linux-rockchip,
linux-s390, linux-scsi, linux-sctp, linux-security-module,
linux-sh, linux-sound, linux-stm32, linux-trace-kernel, linux-usb,
linux-wireless, netdev, ntfs3, samba-technical, sched-ext,
target-devel, tipc-discussion, v9fs, Masami Hiramatsu,
Mathieu Desnoyers
In-Reply-To: <20260310100750.303af303@gandalf.local.home>
On Tue, 10 Mar 2026 10:07:50 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 10 Mar 2026 12:48:41 +0100
> Philipp Hahn <phahn-oss@avm.de> wrote:
>
> > Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> > check.
>
> Why?
>
> >
> > Change generated with coccinelle.
> >
> > To: Steven Rostedt <rostedt@goodmis.org>
> > To: Masami Hiramatsu <mhiramat@kernel.org>
> > To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-trace-kernel@vger.kernel.org
> > Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> > ---
> > kernel/trace/fprobe.c | 2 +-
> > kernel/trace/kprobe_event_gen_test.c | 2 +-
> > kernel/trace/trace_events_hist.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index dcadf1d23b8a31f571392d0c49cbd22df1716b4f..a94ce810d83b90f55d1178a9bd29c78fd068df4c 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -607,7 +607,7 @@ static int fprobe_module_callback(struct notifier_block *nb,
> > do {
> > rhashtable_walk_start(&iter);
> >
> > - while ((node = rhashtable_walk_next(&iter)) && !IS_ERR(node))
> > + while (!IS_ERR_OR_NULL((node = rhashtable_walk_next(&iter))))
>
> Ug, No!
>
> That looks so much worse than the original.
Hmm, now IS_ERR_OR_NULL() is an inline function, so it is safe.
But if you want to use IS_ERR_OR_NULL() here, it will be better something like
node = rhashtable_walk_next(&iter);
while (!IS_ERR_OR_NULL(node)) {
fprobe_remove_node_in_module(mod, node, &alist);
node = rhashtable_walk_next(&iter);
}
Thanks,
>
> -- Steve
>
> > fprobe_remove_node_in_module(mod, node, &alist);
> >
> > rhashtable_walk_stop(&iter);
> > diff --git a/kernel/trace/kprobe_event_gen_test.c b/kernel/trace/kprobe_event_gen_test.c
> > index 5a4b722b50451bfdee42769a6d3be39c055690d1..a1735ca273f0b756aa1fcfcdab30ddad9bc51c5f 100644
> > --- a/kernel/trace/kprobe_event_gen_test.c
> > +++ b/kernel/trace/kprobe_event_gen_test.c
> > @@ -75,7 +75,7 @@ static struct trace_event_file *gen_kretprobe_test;
> >
> > static bool trace_event_file_is_valid(struct trace_event_file *input)
> > {
> > - return input && !IS_ERR(input);
> > + return !IS_ERR_OR_NULL(input);
> > }
> >
> > /*
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 73ea180cad555898693e92ee397a1c9493c7c167..59df215e1dfd9349eca1c0823ed709ec7285f766 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -3973,7 +3973,7 @@ trace_action_create_field_var(struct hist_trigger_data *hist_data,
> > */
> > field_var = create_target_field_var(hist_data, system, event, var);
> >
> > - if (field_var && !IS_ERR(field_var)) {
> > + if (!IS_ERR_OR_NULL(field_var)) {
> > save_field_var(hist_data, field_var);
> > hist_field = field_var->var;
> > } else {
> >
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 57/61] reset: Prefer IS_ERR_OR_NULL over manual NULL check
From: Masami Hiramatsu @ 2026-03-11 4:59 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Philipp Zabel
In-Reply-To: <20260310-b4-is_err_or_null-v1-57-bd63b656022d@avm.de>
On Tue, 10 Mar 2026 12:49:23 +0100
Philipp Hahn <phahn-oss@avm.de> wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Semantich change: Previously the code only printed the warning on error,
> but not when the pointer was NULL. Now the warning is printed in both
> cases!
>
> Change found with coccinelle.
>
> To: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> drivers/reset/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index fceec45c8afc1e74fe46311bdc023ff257e8d770..649bb4ebabb20a09349ccbfc62f8280621df450e 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -715,7 +715,7 @@ EXPORT_SYMBOL_GPL(reset_control_bulk_acquire);
> */
> void reset_control_release(struct reset_control *rstc)
> {
> - if (!rstc || WARN_ON(IS_ERR(rstc)))
> + if (WARN_ON(IS_ERR_OR_NULL(rstc)))
This changes the behavior when rstc == NULL.
WARN_ON does not hit when rstc == NULL in the original code.
Thanks,
> return;
>
> if (reset_control_is_array(rstc))
>
> --
> 2.43.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] wifi: rtw89: retry efuse physical map dump on transient failure
From: Christian Hewitt @ 2026-03-11 4:20 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: Bitterblue Smith, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <2ab692371ff94a3f960d41b04288a084@realtek.com>
> On 11 Mar 2026, at 7:05 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
>
> Christian Hewitt <christianshewitt@gmail.com> wrote:
>>
>>> On 9 Mar 2026, at 6:35 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
>>>
>>> Christian Hewitt <christianshewitt@gmail.com> wrote:
>>>>
>>>>> On 2 Mar 2026, at 10:04 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
>>>>>
>>>>> Christian Hewitt <christianshewitt@gmail.com> wrote:
>>>>>>> On 2 Mar 2026, at 9:47 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
>>>>>>>
>>>>>>> Christian Hewitt <christianshewitt@gmail.com> wrote:
>>>>>>>> On Radxa Rock 5B with a RTL8852BE combo WiFi/BT card, the efuse
>>>>>>>> physical map dump intermittently fails with -EBUSY during probe.
>>>>>>>> The failure occurs in rtw89_dump_physical_efuse_map_ddv() where
>>>>>>>> read_poll_timeout_atomic() times out waiting for the B_AX_EF_RDY
>>>>>>>> bit after 1 second.
>>>>>>>
>>>>>>> I'm checking internally how we handle this case.
>>>
>>> Sorry for the late.
>>>
>>> We encountered WiFi/BT reading efuse at the same time causing similar
>>> problem as yours. The workaround is like yours, which adds timeout
>>> time.
>>>
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>
>>>>>>>> For context, firmware also fails (and recovers) sometimes:
>>>>>>>
>>>>>>> Did you mean this doesn't always happen? sometimes?
>>>>>>
>>>>>> It’s another intermittent behaviour observed on this board (and not
>>>>>> related to the issue this patch targets). It occurs less frequently
>>>>>> than the efuse issue and the existing retry mechanism in the driver
>>>>>> ensures firmware load always succeeds.
>>>
>>> This might be the same cause due to reading efuse in firmware.
>>>
>>> Though we can add more timeout and retry times as workaround, I wonder
>>> if you can control loading time of WiFi and BT kernel modules?
>>>
>>> More, can you do experiment that you load BT module first, and then load
>>> WiFi module after 10 seconds (choose a large number intentionally, or
>>> even larger)?
>>
>> https://paste.libreelec.tv/charmed-turkey.sh
>>
>> I’ve run the above script ^ which removes the wifi and bt modules in
>> sequence then reloads them in the reverse order with a delay between
>> bt and wifi modules loading, then checks for error messages. Over 200
>> test cycles with a 10s delay all were clean (no errors). I also ran
>> cycles with a 2 second delay and 0 second delay before starting wifi
>> module load and those were clear too. I guess that proves sequencing
>> avoids the efuse contention issue? - although it’s not possible in
>> the real-world so not sure there’s huge value in knowing that :)
>
> Thanks for the experiments.
>
> Still want to know is it possible to change sequence/time of loading
> kernel modules at boot time from system level? I mean can you adjust
> the sequence in the Rock 5B board?
I’m not a kernel expert, but I’ve always understood module probe and
load ordering to not be guaranteed; as many things run in parallel and
are highly subjective to the specific hardware capabilities and kernel
config being used.
> In addition, did below messages not appear in these experiments?
>
> [ 7.864148] rtw89_8852be 0002:21:00.0: fw security fail
> [ 7.864154] rtw89_8852be 0002:21:00.0: download firmware fail
No, because even if we have a 0s delay between each group of modules
being loaded, they are loaded in series, so we workaround the issue.
Tweaking the script to background the module load loops so both run
in parallel would be closer to normal conditions, and I would expect
to start seeing failures and the retry mechanisms within the modules
(as added in this patch) being triggered.
Christian
^ permalink raw reply
* Re: [PATCH v10 00/21] wifi: nxpwifi: create nxpwifi to support
From: Jeff Chen @ 2026-03-11 3:30 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, linux-kernel, francesco, wyatt.hsu, s.hauer
In-Reply-To: <9b0144261da2ce7f5ef0a533928732cc43459e40.camel@sipsolutions.net>
On Fri, Mar 06, 2026 at 10:19:43 AM +0100, Johannes Berg wrote:
> On Thu, 2026-03-05 at 22:39 +0800, Jeff Chen wrote:
> > This series adds a new full-MAC Wi-Fi driver `nxpwifi` to support NXP
> > IW611/IW612 chip family. These chips are tri-radio single-chip solutions
> > with Wi-Fi 6(1x1, 2.4/5 GHz), Bluetooth 5.4, and IEEE 802.15.4.
> > Communication with the external host is via SDIO interface. The driver is
> > tested on i.MX8M Mini EVK in both STA and AP mode.
>
> How exactly was it tested, it doesn't even build ;-)
Hi Johannes,
I’d like to double check whether it was caused by the missing IW61x SDIO IDs in sdio_ids.h:
#define SDIO_VENDOR_ID_NXP 0x0471
#define SDIO_DEVICE_ID_NXP_IW61X 0x0205
These definitions were not yet in wireless-next around 2026-03-06 when v10 was tested, and
only appeared in linux-next after 2026-03-10.
Could the build issue you saw be due to these two IDs not being defined at that time?
If not, could you share the specific error you hit so I can fix it right away?
Thanks,
Jeff
>
> There are a couple of things I'm not a huge fan of, in particular the
> whole "IOCTL" layer which is reminiscent of wireless extensions (but
> thankfully those aren't used here), and while I can't really review 35k
> lines here, I don't think I have any real problem with this now.
>
> As I also just said to Lachlan, I think you should probably send a pull
> request with just a single patch adding the driver once reviews settle.
>
> https://lore.kernel.org/linux-wireless/b71d0932b10b5c446681cef588cfcf6f869f3fca.camel@sipsolutions.net/
>
> johannes
>
^ permalink raw reply
* RE: [PATCH] wifi: rtw89: retry efuse physical map dump on transient failure
From: Ping-Ke Shih @ 2026-03-11 3:05 UTC (permalink / raw)
To: Christian Hewitt
Cc: Bitterblue Smith, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CC1F2CDA-830F-4351-A855-8C921B148F8D@gmail.com>
Christian Hewitt <christianshewitt@gmail.com> wrote:
>
> > On 9 Mar 2026, at 6:35 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
> >
> > Christian Hewitt <christianshewitt@gmail.com> wrote:
> >>
> >>> On 2 Mar 2026, at 10:04 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
> >>>
> >>> Christian Hewitt <christianshewitt@gmail.com> wrote:
> >>>>> On 2 Mar 2026, at 9:47 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
> >>>>>
> >>>>> Christian Hewitt <christianshewitt@gmail.com> wrote:
> >>>>>> On Radxa Rock 5B with a RTL8852BE combo WiFi/BT card, the efuse
> >>>>>> physical map dump intermittently fails with -EBUSY during probe.
> >>>>>> The failure occurs in rtw89_dump_physical_efuse_map_ddv() where
> >>>>>> read_poll_timeout_atomic() times out waiting for the B_AX_EF_RDY
> >>>>>> bit after 1 second.
> >>>>>
> >>>>> I'm checking internally how we handle this case.
> >
> > Sorry for the late.
> >
> > We encountered WiFi/BT reading efuse at the same time causing similar
> > problem as yours. The workaround is like yours, which adds timeout
> > time.
> >
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>>
> >>>>>> For context, firmware also fails (and recovers) sometimes:
> >>>>>
> >>>>> Did you mean this doesn't always happen? sometimes?
> >>>>
> >>>> It’s another intermittent behaviour observed on this board (and not
> >>>> related to the issue this patch targets). It occurs less frequently
> >>>> than the efuse issue and the existing retry mechanism in the driver
> >>>> ensures firmware load always succeeds.
> >
> > This might be the same cause due to reading efuse in firmware.
> >
> > Though we can add more timeout and retry times as workaround, I wonder
> > if you can control loading time of WiFi and BT kernel modules?
> >
> > More, can you do experiment that you load BT module first, and then load
> > WiFi module after 10 seconds (choose a large number intentionally, or
> > even larger)?
>
> https://paste.libreelec.tv/charmed-turkey.sh
>
> I’ve run the above script ^ which removes the wifi and bt modules in
> sequence then reloads them in the reverse order with a delay between
> bt and wifi modules loading, then checks for error messages. Over 200
> test cycles with a 10s delay all were clean (no errors). I also ran
> cycles with a 2 second delay and 0 second delay before starting wifi
> module load and those were clear too. I guess that proves sequencing
> avoids the efuse contention issue? - although it’s not possible in
> the real-world so not sure there’s huge value in knowing that :)
Thanks for the experiments.
Still want to know is it possible to change sequence/time of loading
kernel modules at boot time from system level? I mean can you adjust
the sequence in the Rock 5B board?
In addition, did below messages not appear in these experiments?
[ 7.864148] rtw89_8852be 0002:21:00.0: fw security fail
[ 7.864154] rtw89_8852be 0002:21:00.0: download firmware fail
Ping-Ke
^ permalink raw reply
* RE: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode
From: Ping-Ke Shih @ 2026-03-11 2:38 UTC (permalink / raw)
To: Bitterblue Smith, Martin Kaistra, linux-wireless@vger.kernel.org
Cc: Jes Sorensen, Sebastian Andrzej Siewior
In-Reply-To: <61d5c08b-d924-42ad-8a56-ba3ddb970297@gmail.com>
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 25/02/2026 04:28, Ping-Ke Shih wrote:
> > Martin Kaistra <martin.kaistra@linutronix.de> wrote:
> >> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
> >>> On 22/12/2023 12:14, Martin Kaistra wrote:
> >>>> Add a custom function for allocating entries in the sec cam. This allows
> >>>> us to store multiple keys with the same keyidx.
> >>>>
> >>>> The maximum number of sec cam entries for 8188f is 16 according to the
> >>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
> >>>> which might support more entries, can set a different number there.
> >>>>
> >>>> Set the bssid as mac address for group keys instead of just using the
> >>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> >>>> for differentiating them from pairwise keys like in the vendor driver.
> >>>>
> >>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> >>>> broadcast/multicast packets in AP mode.
> >>>>
> >>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> >>>>
> >>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> >>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> >>>> ---
> >>>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++
> >>>> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
> >>>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
> >>>> 3 files changed, 44 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> index c5e6d8f7d26bd..62e6318bc0924 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
> >>>> #define DESC_RATE_ID_SHIFT 16
> >>>> #define DESC_RATE_ID_MASK 0xf
> >>>> #define TXDESC_NAVUSEHDR BIT(20)
> >>>> +#define TXDESC_EN_DESC_ID BIT(21)
> >>>> #define TXDESC_SEC_RC4 0x00400000
> >>>> #define TXDESC_SEC_AES 0x00c00000
> >>>> #define TXDESC_PKT_OFFSET_SHIFT 26
> >>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
> >>>> #define RTL8XXXU_MAX_MAC_ID_NUM 128
> >>>> #define RTL8XXXU_BC_MC_MACID 0
> >>>> #define RTL8XXXU_BC_MC_MACID1 1
> >>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM 64
> >>>>
> >>>> struct rtl8xxxu_priv {
> >>>> struct ieee80211_hw *hw;
> >>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
> >>>> char led_name[32];
> >>>> struct led_classdev led_cdev;
> >>>> DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> >>>> + DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
> >>>> };
> >>>>
> >>>> struct rtl8xxxu_sta_info {
> >>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
> >>>>
> >>>> struct rtl8xxxu_vif {
> >>>> int port_num;
> >>>> + u8 hw_key_idx;
> >>>> };
> >>>>
> >>>> struct rtl8xxxu_rx_urb {
> >>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
> >>>> u16 max_aggr_num;
> >>>> u8 supports_ap:1;
> >>>> u16 max_macid_num;
> >>>> + u16 max_sec_cam_num;
> >>>> u32 adda_1t_init;
> >>>> u32 adda_1t_path_on;
> >>>> u32 adda_2t_path_on_a;
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> index 1e1c8fa194cb8..574a5fe951543 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> >>>> .max_aggr_num = 0x0c14,
> >>>> .supports_ap = 1,
> >>>> .max_macid_num = 16,
> >>>> + .max_sec_cam_num = 16,
> >>>> .adda_1t_init = 0x03c00014,
> >>>> .adda_1t_path_on = 0x03c00014,
> >>>> .trxff_boundary = 0x3f7f,
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> index ecf54eb8dba61..7aafae9fe76b8 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv
> *priv,
> >>>> * This is a bit of a hack - the lower bits of the cipher
> >>>> * suite selector happens to match the cipher index in the CAM
> >>>> */
> >>>> - addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> >>>> + addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
> >>>> ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> >>>> + if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >>>> + ctrl |= BIT(6);
> >>>>
> >>>> for (j = 5; j >= 0; j--) {
> >>>> switch (j) {
> >>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>> struct rtl8xxxu_tx_urb *tx_urb;
> >>>> struct ieee80211_sta *sta = NULL;
> >>>> struct ieee80211_vif *vif = tx_info->control.vif;
> >>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>>> struct device *dev = &priv->udev->dev;
> >>>> u32 queue, rts_rate;
> >>>> u16 pktlen = skb->len;
> >>>> int tx_desc_size = priv->fops->tx_desc_size;
> >>>> u8 macid;
> >>>> int ret;
> >>>> - bool ampdu_enable, sgi = false, short_preamble = false;
> >>>> + bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
> >>>>
> >>>> if (skb_headroom(skb) < tx_desc_size) {
> >>>> dev_warn(dev,
> >>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>> tx_desc->txdw0 =
> >>>> TXDESC_OWN | TXDESC_FIRST_SEGMENT |
> TXDESC_LAST_SEGMENT;
> >>>> if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> >>>> - is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> >>>> + is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
> >>>> tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> >>>> + bmc = true;
> >>>> + }
> >>>> +
> >>>>
> >>>> tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> >>>> + macid = rtl8xxxu_get_macid(priv, sta);
> >>>>
> >>>> if (tx_info->control.hw_key) {
> >>>> switch (tx_info->control.hw_key->cipher) {
> >>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>> default:
> >>>> break;
> >>>> }
> >>>> + if (bmc && rtlvif->hw_key_idx != 0xff) {
> >>>> + tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> >>>> + macid = rtlvif->hw_key_idx;
> >>>> + }
> >>>> }
> >>>>
> >>>> /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> >>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>> else
> >>>> rts_rate = 0;
> >>>>
> >>>> - macid = rtl8xxxu_get_macid(priv, sta);
> >>>> priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi,
> short_preamble,
> >>>> ampdu_enable, rts_rate, macid);
> >>>>
> >>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw
> *hw,
> >>>>
> >>>> priv->vifs[port_num] = vif;
> >>>> rtlvif->port_num = port_num;
> >>>> + rtlvif->hw_key_idx = 0xff;
> >>>>
> >>>> rtl8xxxu_set_linktype(priv, vif->type, port_num);
> >>>> ether_addr_copy(priv->mac_addr, vif->addr);
> >>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct
> ieee80211_hw *hw, u32 rts)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> >>>> +{
> >>>> + struct rtl8xxxu_priv *priv = hw->priv;
> >>>> +
> >>>> + return find_first_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num);
> >>>> +}
> >>>> +
> >>>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
> cmd,
> >>>> struct ieee80211_vif *vif,
> >>>> struct ieee80211_sta *sta,
> >>>> struct ieee80211_key_conf *key)
> >>>> {
> >>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>>> struct rtl8xxxu_priv *priv = hw->priv;
> >>>> struct device *dev = &priv->udev->dev;
> >>>> u8 mac_addr[ETH_ALEN];
> >>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>> dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
> >>>> __func__, cmd, key->cipher, key->keyidx);
> >>>>
> >>>> - if (vif->type != NL80211_IFTYPE_STATION)
> >>>> - return -EOPNOTSUPP;
> >>>> -
> >>>> if (key->keyidx > 3)
> >>>> return -EOPNOTSUPP;
> >>>>
> >>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>> ether_addr_copy(mac_addr, sta->addr);
> >>>> } else {
> >>>> dev_dbg(dev, "%s: group key\n", __func__);
> >>>> - eth_broadcast_addr(mac_addr);
> >>>> + ether_addr_copy(mac_addr, vif->bss_conf.bssid);
> >>>> }
> >>>>
> >>>> val16 = rtl8xxxu_read16(priv, REG_CR);
> >>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>>
> >>>> switch (cmd) {
> >>>> case SET_KEY:
> >>>> - key->hw_key_idx = key->keyidx;
> >>>> +
> >>>> + retval = rtl8xxxu_get_free_sec_cam(hw);
> >>>> + if (retval < 0)
> >>>> + return -EOPNOTSUPP;
> >>>> +
> >>>> + key->hw_key_idx = retval;
> >>>> +
> >>>> + if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> >>>> + rtlvif->hw_key_idx = key->hw_key_idx;
> >>>> +
> >>>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> >>>> rtl8xxxu_cam_write(priv, key, mac_addr);
> >>>> + set_bit(key->hw_key_idx, priv->cam_map);
> >>>
> >>> Hi Martin,
> >>
> >> Hi Bitterblue,
> >>
> >>>
> >>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
> >>> when we use iwd. After the WPA2 handshake no more data is transmitted.
> >>>
> >>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
> >>> this patch, when we use iwd, the group key is installed first. It has
> >>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
> >>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
> >>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
> >>>
> >>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
> >>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
> >>>
> >>> This patch imitating rtw88 makes the old chips work again with iwd.
> >>> What do you think?
> >>
> >> So you reserve the first 4 entries for group keys and use key->keyidx as
> >> key->hw_key_idx directly for those, right? Does that work if 2 virtual
> >> interfaces are used at the same time?
> >> I will do some tests in the next days, but I suspect this be an issue.
> >
> > Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
> > only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
> > special handling for this chip?
>
> I would like to enable supports_concurrent for more chips, if I can test
> this feature.
>
> >
> > I'd share some information about security CAM for reference. Since data is
> > quite old (10+ years), I can't guarantee all are correct.
> >
> > 1. default key switch in rtl8xxxu_set_key()
> > SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
> > broadcast/multicast packets use default key (CAM entries 0~3 for
> > keyidx 0~3).
> > For two interfaces case, that'd a problem, so maybe we should use
> > software encryption/decryption for non-pairwise key.
> >
> > val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
> > SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
> > val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
> > rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);
> >
> > 2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
> > The group key field of security CAM is BIT(6) which isn't supported
> > by earlier chips (sorry I have no data about the exact chips).
> >
>
> BIT 6 is used in the rtl8188eu driver, and not used in the rtl8192cu driver,
> so I assume it's just the older RTL8188CU/RTL8192CU and RTL8723AU that don't
> support it.
>
> > If a chip can support group key field, the CAM layout with default key
> > enabled for two interfaces (STA-1+AP-1) can be
> >
> > STA-1 <-> AP-10
> > AP-1 <-> STA-20
> >
> > CAM GK MAC KEY
> > 0
> > 1 1 00: AP-1 (GTK) // should be on entry 0~3 depends on key_idx
> > 2
> > 3
> > 4
> > 5 1 AP-10 AP-10 (GTK) // can be on any entry after 4
> > 6
> >
>
> What are the numbers 1, 10, and 20 in "AP-1", "AP-10", "STA-20" ?
Just represent different roles.
For rtl8xxxu, two interfaces are STA-1 and AP-1, and STA-1 connect to AP-10,
and AP-1 is connected from STA-20.
I don't add pairwise key to above example, because it is simple, just one
entry to a PTK. Instead, I only list group key in the example.
For AP role (AP-1), it can send broadcast packets to many stations, so
use default key.
(but it should not receive a broadcast packet from stations)
For station role (STA-1), it should receive broadcast packets from
specific AP-10, so add an entry with AP-10's MAC address.
>
> > (pairwise key can be on any entry after 4)
> >
> > if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> > ctrl |= BIT(6);
> >
> > My perspective are
> > 1. currently default key is enabled, so we should treat CAM entry 0~3 as
> > special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
> > as Bitterblue's version.
> >
> > 2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
> > to try the method of example 2 above.
> >
> > If we want earlier chips can support two interfaces, I think we should
> > disable default key.
>
> Indeed, the rtl8192cu driver (out of kernel) doesn't set SEC_CFG_TXBC_USE_DEFKEY
> and SEC_CFG_RXBC_USE_DEFKEY when concurrent mode is enabled.
>
> > Seemingly, it is not possible that using hardware
> > crypto when operating one interface and then switching to use software>
> crypto when operating two interfaces. That means broadcast/multicast
> > packets should be by software crypto entirely if we want to support two
> > interfaces.
> >
> > 3. I think rtw88 has similar problem for two interfaces.
> >
> >>
> >>>
> >>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> index ee278f0548e4..f7b35655bec5 100644
> >>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct
> ieee80211_hw *hw)
> >>> {
> >>> struct rtl8xxxu_priv *priv = hw->priv;
> >>>
> >>> - return find_first_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num);
> >>> + return find_next_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num,
> >>> + RTL8XXXU_SEC_DEFAULT_KEY_NUM);
> >>> }
> >>>
> >>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
> cmd,
> >>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>
> >>> switch (cmd) {
> >>> case SET_KEY:
> >>> + if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
> >>> + retval = rtl8xxxu_get_free_sec_cam(hw);
> >>> + if (retval < 0)
> >>> + return -EOPNOTSUPP;
> >>>
> >>> - retval = rtl8xxxu_get_free_sec_cam(hw);
> >>> - if (retval < 0)
> >>> - return -EOPNOTSUPP;
> >>> -
> >>> - key->hw_key_idx = retval;
> >>> + key->hw_key_idx = retval;
> >>> + } else {
> >>> + key->hw_key_idx = key->keyidx;
> >>> + }
> >>>
> >>> if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> >>> rtlvif->hw_key_idx = key->hw_key_idx;
> >>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> index 4b05dba22e67..188f4bbe99cd 100644
> >>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
> >>> #define RTL8XXXU_BC_MC_MACID 0
> >>> #define RTL8XXXU_BC_MC_MACID1 1
> >>> #define RTL8XXXU_MAX_SEC_CAM_NUM 64
> >>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
> >>>
> >>> struct rtl8xxxu_priv {
> >>> struct ieee80211_hw *hw;
> >
^ permalink raw reply
* RE: [BUG] wifi: rtw88: Hard system freeze on RTL8821CE when power_save is enabled (LPS/ASPM conflict)
From: Ping-Ke Shih @ 2026-03-11 2:22 UTC (permalink / raw)
To: Ping-Ke Shih, LB F
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <792645eed36041f0b3df951f1b28a08a@realtek.com>
Ping-Ke Shih <pkshih@realtek.com> wrote:
>
> LB F <goainwo@gmail.com> wrote:
> >
> > Hi Ping-Ke,
> >
> > Thank you for the incredibly fast response and assistance!
> >
> > > Can you dig kernel log (by netconsole or ramoops) if something useful?
> > > I'd like to know this is hardware level freeze or kernel can capture something
> > wrong.
> >
> > I managed to pull a call trace from a historic journald log just
> > before the system hung. The kernel gets trapped in an IRQ thread
> > inside `rtw_pci_interrupt_threadfn`, calling up into `mac80211`
> > `ieee80211_rx_list` before everything freezes. Here is the relevant
> > snippet:
> >
> > ```text
> > Call Trace:
> > <IRQ>
> > ? __alloc_skb+0x23a/0x2a0
> > ? __alloc_skb+0x10c/0x2a0
> > ? __pfx_irq_thread_fn+0x10/0x10
> > [ ... truncated module list ... ]
> > Tainted: G W I 6.19.6-2-cachyos #1 PREEMPT(full)
> > Hardware name: HP HP Notebook/81F0, BIOS F.50 11/20/2020
> > RIP: 0010:ieee80211_rx_list+0x1012/0x1020 [mac80211]
> > CPU: 2 UID: 0 PID: 765 Comm: irq/56-rtw88_pc
> > rtw_pci_interrupt_threadfn+0x239/0x310 [rtw88_pci]
> > ```
> >
> > It behaves exactly like a PCIe bus deadlock or a hardware fault that
> > eventually brings down the CPU handling the IRQ.
>
> I wonder if there is a malformed data, causing this trace and the leads
> kernel freezes. If we can do validation on RX data before calling
> ieee80211_rx_list(), maybe trace disappears and everything will be fine?
> Even no need workaround.
>
> >
> > > Are these totally needed to workaround the problem? Or disable_aspm is enough?
> > > I'd list them in order of power consumption impact:
> > > 1. disable_aspm=y
> > > 2. disable_lps_deep=y
> > > 3. disable WiFi power save
> >
> > To verify which parameters are strictly necessary, I performed
> > isolated testing today. I ensured no other modprobe configs were
> > active, rebuilt the initramfs, and manually enforced that
> > `wifi.powersave` was active via `iw dev wlan0 set power_save on`
> > during all tests (as the OS power management profiles were defaulting
> > it to off, which initially masked the issue).
> >
> > I tested each workaround individually across multiple sleep/wake
> > cycles and active usage:
> >
> > **Test 1 (ASPM Disabled, LPS Deep Enabled):**
> > - Kernel parameters: `rtw88_pci disable_aspm=y` (and `rtw88_core
> > disable_lps_deep=n`)
> > - Result: Stable. No freezes were observed during usage or transitions
> > into/out of S3 sleep while power saving was enforced.
> >
> > **Test 2 (ASPM Enabled, LPS Deep Disabled):**
> > - Kernel parameters: `rtw88_core disable_lps_deep=y` (and `rtw88_pci
> > disable_aspm=n`)
> > - Result: Stable. No freezes were observed under the same forced power
> > save conditions.
> >
> > **Conclusion:** It appears we do not need both workarounds
> > simultaneously for this specific hardware. Using only `disable_aspm=y`
> > seems to be sufficient to prevent the system freeze. Given your note
> > about the power consumption impact ranking, this looks like the
> > optimal path forward.
>
> Let's test my RFT patch to disable ASPM then.
>
> >
> > > But what does 'deadlock' mean? As I know NAPI poll is scheduled by ISR,
> > > and going to receive packets. The rx_no_aspm workaround is to forcely turn
> > > off ASPM during this period.
> >
> > By "deadlock" I meant a hardware-level bus lockup. It seems the
> > physical RTL8821CE chip itself crashes or hangs the system's PCIe bus
> > when trying to negotiate waking up from ASPM L1 while simultaneously
> > existing in `LPS_DEEP_MODE_LCLK`. The `rx_no_aspm` workaround in NAPI
> > helps during active Rx decoding, but the laptop often freezes while
> > completely idle, presumably when the AP sends a basic beacon, the chip
> > attempts to leave LPS Deep + L1, and the hardware simply gives up and
> > halts the system.
>
> I think this is your perspective and induction, right? Did you measure
> real hardware signals?
>
> My point is that if this is a hardware-level bus lockup, let's apply
> quirk. If some malformed data causing kernel hangs, I'd add sanity check
> on RX data, but I don't actually know what we should check for now.
>
> >
> > > We have not modified RTL8821CE for a long time, so I'd add workaround
> > > to specific platform as mentioned above.
> >
> > Adding a DMI/platform quirk specifically for this laptop to disable
> > ASPM would be wonderful and deeply appreciated. I agree it is safer
> > than touching the global flags for hardware that is functioning
> > correctly out in the wild.
> >
> > Here is the exact identifying information for my system:
> >
> > System Vendor: HP
> > Product Name: HP Notebook
> > SKU Number: P3S95EA#ACB
> > Family: 103C_5335KV
> > PCI ID: 10ec:c821
> > Subsystem ID: 103c:831a
> >
> > I am completely ready to test any patch or quirk you send my way.
> > Thank you so much for your time and helping track this down!
>
> I sent a RFT [1] for test. Please check if it works on your HP notebook.
> If you check rtw88 log, you can see I added similar patch 5 years ago,
> and replaced by preferred the change of "rtwpci->rx_no_aspm", which I
> think it can only resolve problem on partial notebooks though....
>
> [1]
> https://lore.kernel.org/linux-wireless/20260311020816.7065-1-pkshih@realtek.
> com/T/#u
Forgot to say. Could you share your full name for me as a reporter
in commit message?
^ permalink raw reply
* RE: [BUG] wifi: rtw88: Hard system freeze on RTL8821CE when power_save is enabled (LPS/ASPM conflict)
From: Ping-Ke Shih @ 2026-03-11 2:20 UTC (permalink / raw)
To: LB F; +Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CALdGYqSz3SNzoSjUQvK6FgTc2Xkac52=T5A7Lt=d+nxAXGgJVw@mail.gmail.com>
LB F <goainwo@gmail.com> wrote:
>
> Hi Ping-Ke,
>
> Thank you for your guidance. To provide you with the cleanest possible
> diagnostic data, we devised a strict testing environment:
>
> 1. **Live USB Environment:** We booted a completely fresh Live USB of
> CachyOS (Kernel 6.19.6) to eliminate any potential interference from
> installed software, TLP profiles, or custom NetworkManager
> configurations.
> 2. **Aggressive Local Logging:** Because the system freeze physically
> locks the PCIe bus and disables the Wi-Fi adapter instantly, using
> `netconsole` was impossible (the network drops microseconds before the
> freeze).
>
> To overcome this, we wrote an "aggressive logger" script that pipes
> `dmesg -w` directly to an independent FAT32 USB drive while issuing a
> `sync` command twice a second. This bypassed RAM caching and
> physically burned the logs to the drive right up to the moment of the
> hard freeze. The script we used was:
>
> ```bash
> #!/bin/bash
> LOG_FILE="/run/media/liveuser/LOGS/kernel_freeze.log"
> dmesg -w > "$LOG_FILE" &
> while true; do
> sync
> sleep 0.5
> done
> ```
>
> 3. No workarounds (`disable_aspm=n`, `disable_lps_deep=n`) were active
> in this test. We manually enabled power saving (`iw dev wlan0 set
> power_save on`) and triggered the freeze via typical web browsing.
>
> Here are the precise, unadulterated logs showing the adapter
> successfully connecting to the network, sitting idle for about 10
> seconds (presumably entering power-saving states), and then suffering
> a fatal firmware lockup right before the PCIe bus froze:
>
> ```
> [ 304.709201] audit: type=1111 ... op=connection-add-activate ...
> name="Andrey_5G" ...
> [ 305.617785] wlan0: authenticate with 6c:68:a4:1c:97:5b ...
> [ 305.660333] wlan0: authenticated
> [ 305.661661] wlan0: associate with 6c:68:a4:1c:97:5b (try 1/3)
> [ 305.663404] wlan0: associated
> [ 305.719997] wlan0: Limiting TX power to 30 (30 - 0) dBm as
> advertised by 6c:68:a4:1c:97:5b
> ... (~10 seconds of idle network time) ...
> [ 316.907114] rtw88_8821ce 0000:13:00.0: failed to send h2c command
> [ 316.911190] rtw88_8821ce 0000:13:00.0: failed to send h2c command
> [ 316.921504] rtw88_8821ce 0000:13:00.0: coex request time out
> ...
> [ 349.630952] rtw88_8821ce 0000:13:00.0: failed to send h2c command
> [ 349.635023] rtw88_8821ce 0000:13:00.0: failed to send h2c command
> [ 357.811235] rtw88_8821ce 0000:13:00.0: firmware failed to leave lps state
> [ 359.797238] rtw88_8821ce 0000:13:00.0: firmware failed to leave lps state
> ... (repeats indefinitely until hard reset) ...
> ```
Just want to clarify that these logs only appear in test 3, right?
No these logs in test 1/2.
>
> As the logs clearly demonstrate, the adapter authenticates perfectly
> but the firmware explicitly fails to leave the LPS state after a brief
> idle period, dropping all H2C commands immediately before the
> system-wide hard freeze begins.
>
> We will upload the full, unabridged `.log` file to our Bugzilla thread
> (Bug 221195) momentarily, but we wanted to provide you with this exact
> 'smoking gun' trace right away to help identify the root cause.
>
> Please let us know if this information is helpful or if there are any
> specific module patches or further tests you would like us to perform
> to assist with debugging.
Thanks for your detail tests and logs. With this kind of hardware problem,
to dig the cause, we need real hardware and hardware scope to measure
signals. I'd apply quirk or some validations on RX path. That'd be a
better way.
Ping-Ke
^ permalink raw reply
* RE: [BUG] wifi: rtw88: Hard system freeze on RTL8821CE when power_save is enabled (LPS/ASPM conflict)
From: Ping-Ke Shih @ 2026-03-11 2:15 UTC (permalink / raw)
To: LB F; +Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CALdGYqQb=Vt0jjqW7k8RGMV1gczL0cg-26cHgCm3MmzBjezGMQ@mail.gmail.com>
LB F <goainwo@gmail.com> wrote:
>
> Hi Ping-Ke,
>
> Thank you for the incredibly fast response and assistance!
>
> > Can you dig kernel log (by netconsole or ramoops) if something useful?
> > I'd like to know this is hardware level freeze or kernel can capture something
> wrong.
>
> I managed to pull a call trace from a historic journald log just
> before the system hung. The kernel gets trapped in an IRQ thread
> inside `rtw_pci_interrupt_threadfn`, calling up into `mac80211`
> `ieee80211_rx_list` before everything freezes. Here is the relevant
> snippet:
>
> ```text
> Call Trace:
> <IRQ>
> ? __alloc_skb+0x23a/0x2a0
> ? __alloc_skb+0x10c/0x2a0
> ? __pfx_irq_thread_fn+0x10/0x10
> [ ... truncated module list ... ]
> Tainted: G W I 6.19.6-2-cachyos #1 PREEMPT(full)
> Hardware name: HP HP Notebook/81F0, BIOS F.50 11/20/2020
> RIP: 0010:ieee80211_rx_list+0x1012/0x1020 [mac80211]
> CPU: 2 UID: 0 PID: 765 Comm: irq/56-rtw88_pc
> rtw_pci_interrupt_threadfn+0x239/0x310 [rtw88_pci]
> ```
>
> It behaves exactly like a PCIe bus deadlock or a hardware fault that
> eventually brings down the CPU handling the IRQ.
I wonder if there is a malformed data, causing this trace and the leads
kernel freezes. If we can do validation on RX data before calling
ieee80211_rx_list(), maybe trace disappears and everything will be fine?
Even no need workaround.
>
> > Are these totally needed to workaround the problem? Or disable_aspm is enough?
> > I'd list them in order of power consumption impact:
> > 1. disable_aspm=y
> > 2. disable_lps_deep=y
> > 3. disable WiFi power save
>
> To verify which parameters are strictly necessary, I performed
> isolated testing today. I ensured no other modprobe configs were
> active, rebuilt the initramfs, and manually enforced that
> `wifi.powersave` was active via `iw dev wlan0 set power_save on`
> during all tests (as the OS power management profiles were defaulting
> it to off, which initially masked the issue).
>
> I tested each workaround individually across multiple sleep/wake
> cycles and active usage:
>
> **Test 1 (ASPM Disabled, LPS Deep Enabled):**
> - Kernel parameters: `rtw88_pci disable_aspm=y` (and `rtw88_core
> disable_lps_deep=n`)
> - Result: Stable. No freezes were observed during usage or transitions
> into/out of S3 sleep while power saving was enforced.
>
> **Test 2 (ASPM Enabled, LPS Deep Disabled):**
> - Kernel parameters: `rtw88_core disable_lps_deep=y` (and `rtw88_pci
> disable_aspm=n`)
> - Result: Stable. No freezes were observed under the same forced power
> save conditions.
>
> **Conclusion:** It appears we do not need both workarounds
> simultaneously for this specific hardware. Using only `disable_aspm=y`
> seems to be sufficient to prevent the system freeze. Given your note
> about the power consumption impact ranking, this looks like the
> optimal path forward.
Let's test my RFT patch to disable ASPM then.
>
> > But what does 'deadlock' mean? As I know NAPI poll is scheduled by ISR,
> > and going to receive packets. The rx_no_aspm workaround is to forcely turn
> > off ASPM during this period.
>
> By "deadlock" I meant a hardware-level bus lockup. It seems the
> physical RTL8821CE chip itself crashes or hangs the system's PCIe bus
> when trying to negotiate waking up from ASPM L1 while simultaneously
> existing in `LPS_DEEP_MODE_LCLK`. The `rx_no_aspm` workaround in NAPI
> helps during active Rx decoding, but the laptop often freezes while
> completely idle, presumably when the AP sends a basic beacon, the chip
> attempts to leave LPS Deep + L1, and the hardware simply gives up and
> halts the system.
I think this is your perspective and induction, right? Did you measure
real hardware signals?
My point is that if this is a hardware-level bus lockup, let's apply
quirk. If some malformed data causing kernel hangs, I'd add sanity check
on RX data, but I don't actually know what we should check for now.
>
> > We have not modified RTL8821CE for a long time, so I'd add workaround
> > to specific platform as mentioned above.
>
> Adding a DMI/platform quirk specifically for this laptop to disable
> ASPM would be wonderful and deeply appreciated. I agree it is safer
> than touching the global flags for hardware that is functioning
> correctly out in the wild.
>
> Here is the exact identifying information for my system:
>
> System Vendor: HP
> Product Name: HP Notebook
> SKU Number: P3S95EA#ACB
> Family: 103C_5335KV
> PCI ID: 10ec:c821
> Subsystem ID: 103c:831a
>
> I am completely ready to test any patch or quirk you send my way.
> Thank you so much for your time and helping track this down!
I sent a RFT [1] for test. Please check if it works on your HP notebook.
If you check rtw88 log, you can see I added similar patch 5 years ago,
and replaced by preferred the change of "rtwpci->rx_no_aspm", which I
think it can only resolve problem on partial notebooks though....
[1] https://lore.kernel.org/linux-wireless/20260311020816.7065-1-pkshih@realtek.com/T/#u
Ping-Ke
^ permalink raw reply
* [RFT rtw-next] wifi: rtw88: add quirks to disable PCI ASPM for HP P3S95EA#ACB
From: Ping-Ke Shih @ 2026-03-11 2:08 UTC (permalink / raw)
To: linux-wireless; +Cc: goainwo
On an HP laptop (P3S95EA#ACB) equipped with a Realtek RTL8821CE 802.11ac
PCIe adapter (PCI ID: 10ec:c821), the system experiences a hard lockup
(complete freeze of the UI and kernel, sysrq doesn't work, requires
holding the power button) when the WiFi adapter enters the power
saving state.
Add a quirk to disable ASPM.
Reported-by: LB F <goainwo@gmail.com>
Closes: https://lore.kernel.org/linux-wireless/CALdGYqSQ1Ko2TTBhUizMu_FvLMUAuQfFrVwS10n_C-LSQJQQkQ@mail.gmail.com/
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
drivers/net/wireless/realtek/rtw88/pci.c | 30 ++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index ec0a45bfb670..0b60a0ce96a1 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -2,6 +2,7 @@
/* Copyright(c) 2018-2019 Realtek Corporation
*/
+#include <linux/dmi.h>
#include <linux/module.h>
#include <linux/pci.h>
#include "main.h"
@@ -1744,6 +1745,34 @@ const struct pci_error_handlers rtw_pci_err_handler = {
};
EXPORT_SYMBOL(rtw_pci_err_handler);
+enum rtw88_quirk_dis_pci_caps {
+ QUIRK_DIS_PCI_CAP_ASPM,
+};
+
+static int disable_pci_caps(const struct dmi_system_id *dmi)
+{
+ uintptr_t dis_caps = (uintptr_t)dmi->driver_data;
+
+ if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_ASPM))
+ rtw_pci_disable_aspm = true;
+
+ return 1;
+}
+
+static const struct dmi_system_id rtw88_pci_quirks[] = {
+ {
+ .callback = disable_pci_caps,
+ .ident = "HP Notebook - P3S95EA#ACB",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Notebook"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P3S95EA#ACB"),
+ },
+ .driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
+ },
+ {}
+};
+
int rtw_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
@@ -1808,6 +1837,7 @@ int rtw_pci_probe(struct pci_dev *pdev,
bridge && bridge->vendor == PCI_VENDOR_ID_INTEL)
rtwpci->rx_no_aspm = true;
+ dmi_check_system(rtw88_pci_quirks);
rtw_pci_phy_cfg(rtwdev);
ret = rtw_register_hw(rtwdev, hw);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 56/61] clk: Prefer IS_ERR_OR_NULL over manual NULL check
From: Chen-Yu Tsai @ 2026-03-11 2:07 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Michael Turquette, Stephen Boyd,
Daniel Lezcano, Thomas Gleixner
In-Reply-To: <20260310-b4-is_err_or_null-v1-56-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 9:57 PM Philipp Hahn <phahn-oss@avm.de> wrote:
>
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Semantich change: Previously the code only printed the warning on error,
> but not when the pointer was NULL. Now the warning is printed in both
> cases!
>
> Change found with coccinelle.
>
> To: Michael Turquette <mturquette@baylibre.com>
> To: Stephen Boyd <sboyd@kernel.org>
> To: Daniel Lezcano <daniel.lezcano@kernel.org>
> To: Thomas Gleixner <tglx@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
> ---
> drivers/clk/clk.c | 4 ++--
> drivers/clocksource/timer-pxa.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 47093cda9df32223c1120c3710261296027c4cd3..35146e3869a7dd93741d10b7223d4488a9216ed1 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4558,7 +4558,7 @@ void clk_unregister(struct clk *clk)
> unsigned long flags;
> const struct clk_ops *ops;
>
> - if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> + if (WARN_ON_ONCE(IS_ERR_OR_NULL(clk)))
> return;
>
> clk_debug_unregister(clk->core);
> @@ -4744,7 +4744,7 @@ void __clk_put(struct clk *clk)
> {
> struct module *owner;
>
> - if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> + if (WARN_ON_ONCE(IS_ERR_OR_NULL(clk)))
clk_get_optional() returns NULL if the clk isn't present.
Drivers would just pass this to clk_put(). Your change here would cause
this pattern to emit a very big warning.
I don't think this change should be landed.
ChenYu
> return;
>
> clk_prepare_lock();
> diff --git a/drivers/clocksource/timer-pxa.c b/drivers/clocksource/timer-pxa.c
> index 7ad0e5adb2ffac4125c34710fc67f4b45f30331d..f65fb0b7fc318b766227e5e7a4c0fb08ba11c8f9 100644
> --- a/drivers/clocksource/timer-pxa.c
> +++ b/drivers/clocksource/timer-pxa.c
> @@ -218,7 +218,7 @@ void __init pxa_timer_nodt_init(int irq, void __iomem *base)
>
> timer_base = base;
> clk = clk_get(NULL, "OSTIMER0");
> - if (clk && !IS_ERR(clk)) {
> + if (!IS_ERR_OR_NULL(clk)) {
> clk_prepare_enable(clk);
> pxa_timer_common_init(irq, clk_get_rate(clk));
> } else {
>
> --
> 2.43.0
>
>
^ permalink raw reply
* [PATCH] wifi: b43: kzalloc + kcalloc to kzalloc_flex
From: Rosen Penev @ 2026-03-11 0:47 UTC (permalink / raw)
To: linux-wireless
Cc: Kees Cook, Gustavo A. R. Silva, open list:B43 WIRELESS DRIVER,
open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
Simplifies allocation and allows using __counted_by for extra runtime
analysis.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/net/wireless/broadcom/b43/dma.c | 18 ++++++++----------
drivers/net/wireless/broadcom/b43/dma.h | 4 ++--
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 3a8df7a18042..05da6987a845 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -837,18 +837,19 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
struct b43_dmaring *ring;
int i, err;
dma_addr_t dma_test;
+ size_t nr_slots;
- ring = kzalloc_obj(*ring);
+ if (for_tx)
+ nr_slots = B43_TXRING_SLOTS;
+ else
+ nr_slots = B43_RXRING_SLOTS;
+
+ ring = kzalloc_flex(*ring, meta, nr_slots);
if (!ring)
goto out;
- ring->nr_slots = B43_RXRING_SLOTS;
- if (for_tx)
- ring->nr_slots = B43_TXRING_SLOTS;
+ ring->nr_slots = nr_slots;
- ring->meta = kzalloc_objs(struct b43_dmadesc_meta, ring->nr_slots);
- if (!ring->meta)
- goto err_kfree_ring;
for (i = 0; i < ring->nr_slots; i++)
ring->meta->skb = B43_DMA_PTR_POISON;
@@ -943,8 +944,6 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
err_kfree_txhdr_cache:
kfree(ring->txhdr_cache);
err_kfree_meta:
- kfree(ring->meta);
- err_kfree_ring:
kfree(ring);
ring = NULL;
goto out;
@@ -1004,7 +1003,6 @@ static void b43_destroy_dmaring(struct b43_dmaring *ring,
free_ringmemory(ring);
kfree(ring->txhdr_cache);
- kfree(ring->meta);
kfree(ring);
}
diff --git a/drivers/net/wireless/broadcom/b43/dma.h b/drivers/net/wireless/broadcom/b43/dma.h
index c2a357219d4b..f9f65bbe2d76 100644
--- a/drivers/net/wireless/broadcom/b43/dma.h
+++ b/drivers/net/wireless/broadcom/b43/dma.h
@@ -228,8 +228,6 @@ struct b43_dmaring {
const struct b43_dma_ops *ops;
/* Kernel virtual base address of the ring memory. */
void *descbase;
- /* Meta data about all descriptors. */
- struct b43_dmadesc_meta *meta;
/* Cache of TX headers for each TX frame.
* This is to avoid an allocation on each TX.
* This is NULL for an RX ring.
@@ -273,6 +271,8 @@ struct b43_dmaring {
/* Statistics: Total number of TX plus all retries. */
u64 nr_total_packet_tries;
#endif /* CONFIG_B43_DEBUG */
+ /* Meta data about all descriptors. */
+ struct b43_dmadesc_meta meta[] __counted_by(nr_slots);
};
static inline u32 b43_dma_read(struct b43_dmaring *ring, u16 offset)
--
2.53.0
^ permalink raw reply related
* Re: [BUG] wifi: mt76: mt7925u: probe with driver mt7925u failed with error -110
From: Sean Wang @ 2026-03-11 0:47 UTC (permalink / raw)
To: Nick
Cc: linux-wireless, Felix Fietkau, Lorenzo Bianconi, Ryder Lee,
Shayne Chen, Sean Wang, Deren Wu, Leon Yen
In-Reply-To: <CAFktD2dbdccWFodfFNya_XnbKro-O+BKrs4cZRNa0uPrK6FFfQ@mail.gmail.com>
On Mon, Mar 9, 2026 at 7:31 PM Nick <morrownr@gmail.com> wrote:
>
> On Fri, Mar 6, 2026 at 6:46 PM Sean Wang <sean.wang@kernel.org> wrote:
> >
> > Hi Nick,
> >
> > Thanks for the detailed report this is very helpful. I don't have an
> > MT7925 USB device to detail look into the issue, but based on the
> > trace I wrote a small change to try a different MT7925 reset sequence.
> > please help test.
> >
> > --- a/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
> > @@ -234,6 +234,36 @@ int mt792xu_dma_init(struct mt792x_dev *dev, bool resume)
> > }
> > EXPORT_SYMBOL_GPL(mt792xu_dma_init);
> >
> > +static int mt7925u_wfsys_reset(struct mt792x_dev *dev)
> > +{
> > + u32 val;
> > + int i;
> > +
> > +#define MT7925_CBTOP_RGU_WF_SUBSYS_RST 0x70028600
> > +#define MT7925_WFSYS_SW_INIT_DONE_ADDR 0x184c1604
> > +#define MT7925_WFSYS_SW_INIT_DONE 0x00001d1e
> > +
> > + val = mt792xu_uhw_rr(&dev->mt76, MT7925_CBTOP_RGU_WF_SUBSYS_RST);
> > + val |= MT_CBTOP_RGU_WF_SUBSYS_RST_WF_WHOLE_PATH;
> > + mt792xu_uhw_wr(&dev->mt76, MT7925_CBTOP_RGU_WF_SUBSYS_RST, val);
> > +
> > + msleep(20);
> > +
> > + val = mt792xu_uhw_rr(&dev->mt76, MT7925_CBTOP_RGU_WF_SUBSYS_RST);
> > + val &= ~MT_CBTOP_RGU_WF_SUBSYS_RST_WF_WHOLE_PATH;
> > + mt792xu_uhw_wr(&dev->mt76, MT7925_CBTOP_RGU_WF_SUBSYS_RST, val);
> > +
> > + for (i = 0; i < MT792x_WFSYS_INIT_RETRY_COUNT; i++) {
> > + val = mt792xu_uhw_rr(&dev->mt76, MT7925_WFSYS_SW_INIT_DONE_ADDR);
> > + if (val == MT7925_WFSYS_SW_INIT_DONE)
> > + return 0;
> > +
> > + msleep(100);
> > + }
> > +
> > + return -ETIMEDOUT;
> > +}
> > +
> > int mt792xu_wfsys_reset(struct mt792x_dev *dev)
> > {
> > u32 val;
> > @@ -241,6 +271,9 @@ int mt792xu_wfsys_reset(struct mt792x_dev *dev)
> >
> > mt792xu_epctl_rst_opt(dev, false);
> >
> > + if (is_mt7925(&dev->mt76))
> > + return mt7925u_wfsys_reset(dev);
> > +
> > val = mt792xu_uhw_rr(&dev->mt76, MT_CBTOP_RGU_WF_SUBSYS_RST);
> >
> >
>
> Sean,
>
> Testing complete. Results are good. You can show the following:
>
> Tested-by: Nick Morrow <morrownr@gmail.com>
> Tested-by: Satadru Pramanik <satadru@gmail.com>
>
> Thank you for working on this issue. If it is possible for your patch to go
> directly in to be in one of the next rc's, that would be great and setting
> it to backport would also be much appreciated.
>
> Nick Morrow
>
Hi Nick,
Thanks for testing!
I’ve already sent the patches upstream with a few small cosmetic
tweaks. If you have a chance to test the latest version again, that
would be great. If it still looks good, you can just reply with the
Tested-by tags below. I think we can let Johannes know if it is okay
to wireless.git directly I also CC’d stable so the fix can make its
way into the stable trees.
Sean
> > On Fri, Mar 6, 2026 at 1:24 PM Nick <morrownr@gmail.com> wrote:
> > >
> > > > > Reference: https://github.com/morrownr/USB-WiFi/issues/688#issuecomment-3999038526
> > > > >
> > > > > The above thread is rather lengthy as we have been working on this
> > > > > issue since Dec. 25.
> > > > >
> > > > > Testing with a Netgear A9000 USB WiFi adapter (mt7925u driver). Kernel
> > > > > 7.0 rc2 and a x86_64 system. Additional testing with older kernels was
> > > > > also performed with the same results.
> > > > >
> > > > > Problem description:
> > > > >
> > > > > Cold boot shows the adapter coming up and operating normally.
> > > > >
> > > > > Removing the adapter from the USB port and replacing it shows the
> > > > > adapter coming up and operating normally.
> > > > >
> > > > > A warm reboot does not provide a WiFi interface and shows the
> > > > > following in the system log:
> > > > >
> > > > > mt7925u 2-3.2:1.0: probe with driver mt7925u failed with error -110
> > > > >
> > > > > Using the commands rmmod and modeprobe do not provide a WiFi interface.
> > > > >
> > > > > Thoughts: The problem likely is not the module teardown. The problem
> > > > > seems to be that the firmware (or the mt7925u driver) leaves the
> > > > > adapter in a strange state such that a power cycle of the adapter
> > > > > hardware is needed before the mt7925u driver can properly initialize
> > > > > it a second time.
> > > > >
> > > >
> > > > Hi Nick,
> > > >
> > > > Could you enable debug logs and check whether the driver can still
> > > > read the correct chip ID after a warm reboot, and at which step the
> > > > initialization fails before the -110 error? Thanks for continuing to
> > > > test and gather this useful information.
> > > >
> > > > Sean
> > >
> > > Hi Sean,
> > >
> > > Github user @exct has performed the testing requested and provides the
> > > following report:
> > >
> > > mt7925u Probe Failure Debug Report
> > > Adapter: Netgear A9000 (USB ID 0846:9072, MediaTek MT7925)
> > > Kernel: 6.19.6-2-cachyos
> > >
> > > Summary
> > > The driver can read the correct chip ID after a warm reload. The
> > > failure occurs before firmware is loaded, inside
> > > mt792xu_wfsys_reset(), which times out waiting for the WiFi subsystem
> > > to reinitialize. -ETIMEDOUT (-110) is returned and the probe aborts.
> > >
> > > Findings
> > >
> > > Question: Chip ID readable after warm reload?
> > > Answer: Yes — MT_HW_CHIPID = 0x7925, MT_HW_REV = 0x8a00
> > >
> > > Question: Which step fails?
> > > Answer: mt792xu_wfsys_reset() — WFSYS_INIT_DONE never asserted
> > >
> > > Question: Does it reach mt792xu_mcu_power_on()?
> > > Answer: No
> > >
> > > Question: Does it reach mt7925_run_firmware()?
> > > Answer: No
> > >
> > > Probe Sequence Trace
> > >
> > > mt7925u_probe()
> > > ├─ mt76_alloc_device() OK
> > > ├─ __mt76u_init() OK
> > > ├─ read MT_HW_CHIPID (0x70010200) → 0x00007925 ✓
> > > ├─ read MT_HW_REV (0x70010204) → 0x00008a00 ✓
> > > ├─ read MT_CONN_ON_MISC (0x7c0600f0) → 0x00000003 ← FW_N9_RDY is SET
> > > ├─ enters mt792xu_wfsys_reset() ← triggered because FW_N9_RDY = 1
> > > │ ├─ write MT_CBTOP_RGU_WF_SUBSYS_RST (assert reset)
> > > │ ├─ write MT_CBTOP_RGU_WF_SUBSYS_RST (deassert reset)
> > > │ ├─ poll MT_UDMA_CONN_INFRA_STATUS (0x74000a20) → 0x00000000
> > > │ ├─ poll MT_UDMA_CONN_INFRA_STATUS (0x74000a20) → 0x00000000
> > > │ └─ ... timeout after ~212 ms → return -ETIMEDOUT
> > > └─ goto error: (mt76u_queues_deinit + mt76_free_device)
> > >
> > > Root Cause
> > >
> > > After rmmod, the firmware leaves FW_N9_RDY asserted in
> > > MT_CONN_ON_MISC. On re-probe, the driver correctly detects this and
> > > calls mt792xu_wfsys_reset() to recover. However, the WiFi subsystem
> > > never signals completion — WFSYS_INIT_DONE in
> > > MT_UDMA_CONN_INFRA_STATUS (reg 0x74000a20) stays 0x00000000 throughout
> > > the retry loop. The subsystem is stuck in a state that the software
> > > reset path cannot recover from. Only a full USB power cycle clears it.
> > >
> > > Hope this helps,
> > >
> > > Nick
^ permalink raw reply
* [PATCH 2/2] wifi: mt76: mt792x: fix mt7925u USB WFSYS reset handling
From: Sean Wang @ 2026-03-11 0:28 UTC (permalink / raw)
To: nbd, lorenzo.bianconi; +Cc: linux-wireless, linux-mediatek, Sean Wang, stable
In-Reply-To: <20260311002825.15502-1-sean.wang@kernel.org>
From: Sean Wang <sean.wang@mediatek.com>
mt7925u uses different reset/status registers from mt7921u. Reusing the
mt7921u register set causes the WFSYS reset to fail.
Add a chip-specific descriptor in mt792xu_wfsys_reset() to select the
correct registers and fix mt7925u failing to initialize after a warm
reboot.
Fixes: d28e1a48952e ("wifi: mt76: mt792x: introduce mt792x-usb module")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/wireless/mediatek/mt76/mt792x_regs.h | 4 ++++
drivers/net/wireless/mediatek/mt76/mt792x_usb.c | 13 ++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt792x_regs.h b/drivers/net/wireless/mediatek/mt76/mt792x_regs.h
index 7ddde9286861..d2a8b2b0df32 100644
--- a/drivers/net/wireless/mediatek/mt76/mt792x_regs.h
+++ b/drivers/net/wireless/mediatek/mt76/mt792x_regs.h
@@ -392,6 +392,10 @@
#define MT_CBTOP_RGU_WF_SUBSYS_RST MT_CBTOP_RGU(0x600)
#define MT_CBTOP_RGU_WF_SUBSYS_RST_WF_WHOLE_PATH BIT(0)
+#define MT7925_CBTOP_RGU_WF_SUBSYS_RST 0x70028600
+#define MT7925_WFSYS_INIT_DONE_ADDR 0x184c1604
+#define MT7925_WFSYS_INIT_DONE 0x00001d1e
+
#define MT_HW_BOUND 0x70010020
#define MT_HW_CHIPID 0x70010200
#define MT_HW_REV 0x70010204
diff --git a/drivers/net/wireless/mediatek/mt76/mt792x_usb.c b/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
index a92e872226cf..47827d1c5ccb 100644
--- a/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
@@ -224,6 +224,15 @@ static const struct mt792xu_wfsys_desc mt7921_wfsys_desc = {
.need_status_sel = true,
};
+static const struct mt792xu_wfsys_desc mt7925_wfsys_desc = {
+ .rst_reg = MT7925_CBTOP_RGU_WF_SUBSYS_RST,
+ .done_reg = MT7925_WFSYS_INIT_DONE_ADDR,
+ .done_mask = U32_MAX,
+ .done_val = MT7925_WFSYS_INIT_DONE,
+ .delay_ms = 20,
+ .need_status_sel = false,
+};
+
int mt792xu_dma_init(struct mt792x_dev *dev, bool resume)
{
int err;
@@ -254,7 +263,9 @@ EXPORT_SYMBOL_GPL(mt792xu_dma_init);
int mt792xu_wfsys_reset(struct mt792x_dev *dev)
{
- const struct mt792xu_wfsys_desc *desc = &mt7921_wfsys_desc;
+ const struct mt792xu_wfsys_desc *desc = is_mt7925(&dev->mt76) ?
+ &mt7925_wfsys_desc :
+ &mt7921_wfsys_desc;
u32 val;
int i;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] wifi: mt76: mt792x: describe USB WFSYS reset with a descriptor
From: Sean Wang @ 2026-03-11 0:28 UTC (permalink / raw)
To: nbd, lorenzo.bianconi; +Cc: linux-wireless, linux-mediatek, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
Prepare mt792xu_wfsys_reset() for chips that share the same USB WFSYS
reset flow but use different register definitions.
This is a pure refactor of the current mt7921u path and keeps the reset
sequence unchanged.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
.../net/wireless/mediatek/mt76/mt792x_usb.c | 40 +++++++++++++++----
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt792x_usb.c b/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
index 552808458138..a92e872226cf 100644
--- a/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt792x_usb.c
@@ -206,6 +206,24 @@ static void mt792xu_epctl_rst_opt(struct mt792x_dev *dev, bool reset)
mt792xu_uhw_wr(&dev->mt76, MT_SSUSB_EPCTL_CSR_EP_RST_OPT, val);
}
+struct mt792xu_wfsys_desc {
+ u32 rst_reg;
+ u32 done_reg;
+ u32 done_mask;
+ u32 done_val;
+ u32 delay_ms;
+ bool need_status_sel;
+};
+
+static const struct mt792xu_wfsys_desc mt7921_wfsys_desc = {
+ .rst_reg = MT_CBTOP_RGU_WF_SUBSYS_RST,
+ .done_reg = MT_UDMA_CONN_INFRA_STATUS,
+ .done_mask = MT_UDMA_CONN_WFSYS_INIT_DONE,
+ .done_val = MT_UDMA_CONN_WFSYS_INIT_DONE,
+ .delay_ms = 0,
+ .need_status_sel = true,
+};
+
int mt792xu_dma_init(struct mt792x_dev *dev, bool resume)
{
int err;
@@ -236,25 +254,31 @@ EXPORT_SYMBOL_GPL(mt792xu_dma_init);
int mt792xu_wfsys_reset(struct mt792x_dev *dev)
{
+ const struct mt792xu_wfsys_desc *desc = &mt7921_wfsys_desc;
u32 val;
int i;
mt792xu_epctl_rst_opt(dev, false);
- val = mt792xu_uhw_rr(&dev->mt76, MT_CBTOP_RGU_WF_SUBSYS_RST);
+ val = mt792xu_uhw_rr(&dev->mt76, desc->rst_reg);
val |= MT_CBTOP_RGU_WF_SUBSYS_RST_WF_WHOLE_PATH;
- mt792xu_uhw_wr(&dev->mt76, MT_CBTOP_RGU_WF_SUBSYS_RST, val);
+ mt792xu_uhw_wr(&dev->mt76, desc->rst_reg, val);
- usleep_range(10, 20);
+ if (desc->delay_ms)
+ msleep(desc->delay_ms);
+ else
+ usleep_range(10, 20);
- val = mt792xu_uhw_rr(&dev->mt76, MT_CBTOP_RGU_WF_SUBSYS_RST);
+ val = mt792xu_uhw_rr(&dev->mt76, desc->rst_reg);
val &= ~MT_CBTOP_RGU_WF_SUBSYS_RST_WF_WHOLE_PATH;
- mt792xu_uhw_wr(&dev->mt76, MT_CBTOP_RGU_WF_SUBSYS_RST, val);
+ mt792xu_uhw_wr(&dev->mt76, desc->rst_reg, val);
+
+ if (desc->need_status_sel)
+ mt792xu_uhw_wr(&dev->mt76, MT_UDMA_CONN_INFRA_STATUS_SEL, 0);
- mt792xu_uhw_wr(&dev->mt76, MT_UDMA_CONN_INFRA_STATUS_SEL, 0);
for (i = 0; i < MT792x_WFSYS_INIT_RETRY_COUNT; i++) {
- val = mt792xu_uhw_rr(&dev->mt76, MT_UDMA_CONN_INFRA_STATUS);
- if (val & MT_UDMA_CONN_WFSYS_INIT_DONE)
+ val = mt792xu_uhw_rr(&dev->mt76, desc->done_reg);
+ if ((val & desc->done_mask) == desc->done_val)
break;
msleep(100);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
From: Russell King (Oracle) @ 2026-03-11 0:16 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Igor Russkikh, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit
In-Reply-To: <20260310-b4-is_err_or_null-v1-38-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:49:04PM +0100, Philipp Hahn wrote:
> diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
> index a8f91a4b7fed0927ee14e408000cd3a2bfb9b09a..09b30b563295c6085dc1358ac361301e5cf6b2a8 100644
> --- a/drivers/net/mdio/mdio-xgene.c
> +++ b/drivers/net/mdio/mdio-xgene.c
> @@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
> struct phy_device *phy_dev;
>
> phy_dev = get_phy_device(bus, phy_addr, false);
> - if (!phy_dev || IS_ERR(phy_dev))
> + if (IS_ERR_OR_NULL(phy_dev))
As noted in reply to your cover message, the check for NULL here is
incorrect - get_phy_device() returns either a valid pointer or an
error pointer, but never NULL.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Russell King (Oracle) @ 2026-03-11 0:09 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Julia Lawall, Nicolas Palix, Chris Mason,
David Sterba, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Theodore Ts'o, Andreas Dilger, Steve French, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Miklos Szeredi,
Konstantin Komarov, Andreas Gruenbacher, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Jan Kara, Phillip Lougher, Alexander Viro,
Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Tejun Heo, David Vernet, Andrea Righi,
Changwoo Min, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Ben Segall, Mel Gorman,
Valentin Schneider, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Sylwester Nawrocki, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Max Filippov,
Paolo Bonzini, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Alasdair Kergon, Mike Snitzer,
Mikulas Patocka, Benjamin Marzinski, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Jamal Hadi Salim, Jiri Pirko,
Marcelo Ricardo Leitner, Xin Long, Trond Myklebust,
Anna Schumaker, Chuck Lever, Jeff Layton, NeilBrown,
Olga Kornievskaia, Dai Ngo, Jon Maloy, Johannes Berg,
Catalin Marinas, John Crispin, Thomas Bogendoerfer,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Zhenyu Wang,
Zhi Wang, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Alex Deucher, Christian König, Sandy Huang,
Heiko Stübner, Andy Yan, Igor Russkikh, Andrew Lunn,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Marc Zyngier, Thomas Gleixner, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Vinod Koul, Linus Walleij, Ulf Hansson,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Martin K. Petersen,
Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Alex Williamson, Mark Greer,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Shuah Khan, Kieran Bingham, Mauro Carvalho Chehab, Joerg Roedel,
Will Deacon, Robin Murphy, Lee Jones, Pavel Machek, Dave Penkler,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Justin Sanders, Jens Axboe, Georgi Djakov, Michael Turquette,
Stephen Boyd, Philipp Zabel, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Pali Rohár, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().
One thing you need to check for each of these cases is whether the tests
are actually correct.
There are certainly cases amongst those that you have identified where
the check for NULL is redundant.
For example, get_phy_device() never returns NULL, yet in your netdev
patch, you have at least one instance where the return value of
get_phy_device() is checked for NULL and IS_ERR() which you then
turn into IS_ERR_OR_NULL(). Instead, the NULL check should be dropped
(as a separate patch.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode
From: Bitterblue Smith @ 2026-03-10 23:33 UTC (permalink / raw)
To: Martin Kaistra, linux-wireless
Cc: Jes Sorensen, Ping-Ke Shih, Sebastian Andrzej Siewior
In-Reply-To: <b8e9ad0c-c148-40a2-b114-5558e74942b0@linutronix.de>
On 24/02/2026 17:43, Martin Kaistra wrote:
> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
>> On 22/12/2023 12:14, Martin Kaistra wrote:
>>> Add a custom function for allocating entries in the sec cam. This allows
>>> us to store multiple keys with the same keyidx.
>>>
>>> The maximum number of sec cam entries for 8188f is 16 according to the
>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>>> which might support more entries, can set a different number there.
>>>
>>> Set the bssid as mac address for group keys instead of just using the
>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>>> for differentiating them from pairwise keys like in the vendor driver.
>>>
>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>>> broadcast/multicast packets in AP mode.
>>>
>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>>
>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>>> ---
>>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++
>>> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
>>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>> 3 files changed, 44 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> index c5e6d8f7d26bd..62e6318bc0924 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>> #define DESC_RATE_ID_SHIFT 16
>>> #define DESC_RATE_ID_MASK 0xf
>>> #define TXDESC_NAVUSEHDR BIT(20)
>>> +#define TXDESC_EN_DESC_ID BIT(21)
>>> #define TXDESC_SEC_RC4 0x00400000
>>> #define TXDESC_SEC_AES 0x00c00000
>>> #define TXDESC_PKT_OFFSET_SHIFT 26
>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>> #define RTL8XXXU_MAX_MAC_ID_NUM 128
>>> #define RTL8XXXU_BC_MC_MACID 0
>>> #define RTL8XXXU_BC_MC_MACID1 1
>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM 64
>>> struct rtl8xxxu_priv {
>>> struct ieee80211_hw *hw;
>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>> char led_name[32];
>>> struct led_classdev led_cdev;
>>> DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>>> + DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>> };
>>> struct rtl8xxxu_sta_info {
>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>> struct rtl8xxxu_vif {
>>> int port_num;
>>> + u8 hw_key_idx;
>>> };
>>> struct rtl8xxxu_rx_urb {
>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>> u16 max_aggr_num;
>>> u8 supports_ap:1;
>>> u16 max_macid_num;
>>> + u16 max_sec_cam_num;
>>> u32 adda_1t_init;
>>> u32 adda_1t_path_on;
>>> u32 adda_2t_path_on_a;
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> index 1e1c8fa194cb8..574a5fe951543 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>> .max_aggr_num = 0x0c14,
>>> .supports_ap = 1,
>>> .max_macid_num = 16,
>>> + .max_sec_cam_num = 16,
>>> .adda_1t_init = 0x03c00014,
>>> .adda_1t_path_on = 0x03c00014,
>>> .trxff_boundary = 0x3f7f,
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index ecf54eb8dba61..7aafae9fe76b8 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>> * This is a bit of a hack - the lower bits of the cipher
>>> * suite selector happens to match the cipher index in the CAM
>>> */
>>> - addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>>> + addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>> ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>>> + if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>> + ctrl |= BIT(6);
>>> for (j = 5; j >= 0; j--) {
>>> switch (j) {
>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>> struct rtl8xxxu_tx_urb *tx_urb;
>>> struct ieee80211_sta *sta = NULL;
>>> struct ieee80211_vif *vif = tx_info->control.vif;
>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>> struct device *dev = &priv->udev->dev;
>>> u32 queue, rts_rate;
>>> u16 pktlen = skb->len;
>>> int tx_desc_size = priv->fops->tx_desc_size;
>>> u8 macid;
>>> int ret;
>>> - bool ampdu_enable, sgi = false, short_preamble = false;
>>> + bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>> if (skb_headroom(skb) < tx_desc_size) {
>>> dev_warn(dev,
>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>> tx_desc->txdw0 =
>>> TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>> if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>>> - is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>>> + is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>> tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>>> + bmc = true;
>>> + }
>>> +
>>> tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>>> + macid = rtl8xxxu_get_macid(priv, sta);
>>> if (tx_info->control.hw_key) {
>>> switch (tx_info->control.hw_key->cipher) {
>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>> default:
>>> break;
>>> }
>>> + if (bmc && rtlvif->hw_key_idx != 0xff) {
>>> + tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>>> + macid = rtlvif->hw_key_idx;
>>> + }
>>> }
>>> /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>> else
>>> rts_rate = 0;
>>> - macid = rtl8xxxu_get_macid(priv, sta);
>>> priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>> ampdu_enable, rts_rate, macid);
>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>> priv->vifs[port_num] = vif;
>>> rtlvif->port_num = port_num;
>>> + rtlvif->hw_key_idx = 0xff;
>>> rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>> ether_addr_copy(priv->mac_addr, vif->addr);
>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>> return 0;
>>> }
>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>> +{
>>> + struct rtl8xxxu_priv *priv = hw->priv;
>>> +
>>> + return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>> +}
>>> +
>>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> struct ieee80211_vif *vif,
>>> struct ieee80211_sta *sta,
>>> struct ieee80211_key_conf *key)
>>> {
>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>> struct rtl8xxxu_priv *priv = hw->priv;
>>> struct device *dev = &priv->udev->dev;
>>> u8 mac_addr[ETH_ALEN];
>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>> __func__, cmd, key->cipher, key->keyidx);
>>> - if (vif->type != NL80211_IFTYPE_STATION)
>>> - return -EOPNOTSUPP;
>>> -
>>> if (key->keyidx > 3)
>>> return -EOPNOTSUPP;
>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> ether_addr_copy(mac_addr, sta->addr);
>>> } else {
>>> dev_dbg(dev, "%s: group key\n", __func__);
>>> - eth_broadcast_addr(mac_addr);
>>> + ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>> }
>>> val16 = rtl8xxxu_read16(priv, REG_CR);
>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> switch (cmd) {
>>> case SET_KEY:
>>> - key->hw_key_idx = key->keyidx;
>>> +
>>> + retval = rtl8xxxu_get_free_sec_cam(hw);
>>> + if (retval < 0)
>>> + return -EOPNOTSUPP;
>>> +
>>> + key->hw_key_idx = retval;
>>> +
>>> + if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>> + rtlvif->hw_key_idx = key->hw_key_idx;
>>> +
>>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>> rtl8xxxu_cam_write(priv, key, mac_addr);
>>> + set_bit(key->hw_key_idx, priv->cam_map);
>>
>> Hi Martin,
>
> Hi Bitterblue,
>
>>
>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
>> when we use iwd. After the WPA2 handshake no more data is transmitted.
>>
>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
>> this patch, when we use iwd, the group key is installed first. It has
>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
>>
>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
>>
>> This patch imitating rtw88 makes the old chips work again with iwd.
>> What do you think?
>
> So you reserve the first 4 entries for group keys and use key->keyidx as key->hw_key_idx directly for those, right? Does that work if 2 virtual interfaces are used at the same time?
Yes. I don't know if that works with 2 interfaces. How can I test?
I never tried to use 2 interfaces at the same time.
> I will do some tests in the next days, but I suspect this be an issue.
>
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> index ee278f0548e4..f7b35655bec5 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>> {
>> struct rtl8xxxu_priv *priv = hw->priv;
>> - return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>> + return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
>> + RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>> }
>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> switch (cmd) {
>> case SET_KEY:
>> + if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
>> + retval = rtl8xxxu_get_free_sec_cam(hw);
>> + if (retval < 0)
>> + return -EOPNOTSUPP;
>> - retval = rtl8xxxu_get_free_sec_cam(hw);
>> - if (retval < 0)
>> - return -EOPNOTSUPP;
>> -
>> - key->hw_key_idx = retval;
>> + key->hw_key_idx = retval;
>> + } else {
>> + key->hw_key_idx = key->keyidx;
>> + }
>> if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>> rtlvif->hw_key_idx = key->hw_key_idx;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index 4b05dba22e67..188f4bbe99cd 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>> #define RTL8XXXU_BC_MC_MACID 0
>> #define RTL8XXXU_BC_MC_MACID1 1
>> #define RTL8XXXU_MAX_SEC_CAM_NUM 64
>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
>> struct rtl8xxxu_priv {
>> struct ieee80211_hw *hw;
>
^ permalink raw reply
* Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode
From: Bitterblue Smith @ 2026-03-10 23:32 UTC (permalink / raw)
To: Ping-Ke Shih, Martin Kaistra, linux-wireless@vger.kernel.org
Cc: Jes Sorensen, Sebastian Andrzej Siewior
In-Reply-To: <64617033bcdc445091186070350a0d20@realtek.com>
On 25/02/2026 04:28, Ping-Ke Shih wrote:
> Martin Kaistra <martin.kaistra@linutronix.de> wrote:
>> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
>>> On 22/12/2023 12:14, Martin Kaistra wrote:
>>>> Add a custom function for allocating entries in the sec cam. This allows
>>>> us to store multiple keys with the same keyidx.
>>>>
>>>> The maximum number of sec cam entries for 8188f is 16 according to the
>>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>>>> which might support more entries, can set a different number there.
>>>>
>>>> Set the bssid as mac address for group keys instead of just using the
>>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>>>> for differentiating them from pairwise keys like in the vendor driver.
>>>>
>>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>>>> broadcast/multicast packets in AP mode.
>>>>
>>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>>>
>>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>>>> ---
>>>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++
>>>> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
>>>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>>> 3 files changed, 44 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> index c5e6d8f7d26bd..62e6318bc0924 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>>> #define DESC_RATE_ID_SHIFT 16
>>>> #define DESC_RATE_ID_MASK 0xf
>>>> #define TXDESC_NAVUSEHDR BIT(20)
>>>> +#define TXDESC_EN_DESC_ID BIT(21)
>>>> #define TXDESC_SEC_RC4 0x00400000
>>>> #define TXDESC_SEC_AES 0x00c00000
>>>> #define TXDESC_PKT_OFFSET_SHIFT 26
>>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>>> #define RTL8XXXU_MAX_MAC_ID_NUM 128
>>>> #define RTL8XXXU_BC_MC_MACID 0
>>>> #define RTL8XXXU_BC_MC_MACID1 1
>>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM 64
>>>>
>>>> struct rtl8xxxu_priv {
>>>> struct ieee80211_hw *hw;
>>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>>> char led_name[32];
>>>> struct led_classdev led_cdev;
>>>> DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>>>> + DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>>> };
>>>>
>>>> struct rtl8xxxu_sta_info {
>>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>>>
>>>> struct rtl8xxxu_vif {
>>>> int port_num;
>>>> + u8 hw_key_idx;
>>>> };
>>>>
>>>> struct rtl8xxxu_rx_urb {
>>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>>> u16 max_aggr_num;
>>>> u8 supports_ap:1;
>>>> u16 max_macid_num;
>>>> + u16 max_sec_cam_num;
>>>> u32 adda_1t_init;
>>>> u32 adda_1t_path_on;
>>>> u32 adda_2t_path_on_a;
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> index 1e1c8fa194cb8..574a5fe951543 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>>> .max_aggr_num = 0x0c14,
>>>> .supports_ap = 1,
>>>> .max_macid_num = 16,
>>>> + .max_sec_cam_num = 16,
>>>> .adda_1t_init = 0x03c00014,
>>>> .adda_1t_path_on = 0x03c00014,
>>>> .trxff_boundary = 0x3f7f,
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> index ecf54eb8dba61..7aafae9fe76b8 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>>> * This is a bit of a hack - the lower bits of the cipher
>>>> * suite selector happens to match the cipher index in the CAM
>>>> */
>>>> - addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>>>> + addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>>> ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>>>> + if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>> + ctrl |= BIT(6);
>>>>
>>>> for (j = 5; j >= 0; j--) {
>>>> switch (j) {
>>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>> struct rtl8xxxu_tx_urb *tx_urb;
>>>> struct ieee80211_sta *sta = NULL;
>>>> struct ieee80211_vif *vif = tx_info->control.vif;
>>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>> struct device *dev = &priv->udev->dev;
>>>> u32 queue, rts_rate;
>>>> u16 pktlen = skb->len;
>>>> int tx_desc_size = priv->fops->tx_desc_size;
>>>> u8 macid;
>>>> int ret;
>>>> - bool ampdu_enable, sgi = false, short_preamble = false;
>>>> + bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>>>
>>>> if (skb_headroom(skb) < tx_desc_size) {
>>>> dev_warn(dev,
>>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>> tx_desc->txdw0 =
>>>> TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>>> if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>>>> - is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>>>> + is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>>> tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>>>> + bmc = true;
>>>> + }
>>>> +
>>>>
>>>> tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>>>> + macid = rtl8xxxu_get_macid(priv, sta);
>>>>
>>>> if (tx_info->control.hw_key) {
>>>> switch (tx_info->control.hw_key->cipher) {
>>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>> default:
>>>> break;
>>>> }
>>>> + if (bmc && rtlvif->hw_key_idx != 0xff) {
>>>> + tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>>>> + macid = rtlvif->hw_key_idx;
>>>> + }
>>>> }
>>>>
>>>> /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>> else
>>>> rts_rate = 0;
>>>>
>>>> - macid = rtl8xxxu_get_macid(priv, sta);
>>>> priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>>> ampdu_enable, rts_rate, macid);
>>>>
>>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>>>
>>>> priv->vifs[port_num] = vif;
>>>> rtlvif->port_num = port_num;
>>>> + rtlvif->hw_key_idx = 0xff;
>>>>
>>>> rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>>> ether_addr_copy(priv->mac_addr, vif->addr);
>>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>>> return 0;
>>>> }
>>>>
>>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>>> +{
>>>> + struct rtl8xxxu_priv *priv = hw->priv;
>>>> +
>>>> + return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>>> +}
>>>> +
>>>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>> struct ieee80211_vif *vif,
>>>> struct ieee80211_sta *sta,
>>>> struct ieee80211_key_conf *key)
>>>> {
>>>> + struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>> struct rtl8xxxu_priv *priv = hw->priv;
>>>> struct device *dev = &priv->udev->dev;
>>>> u8 mac_addr[ETH_ALEN];
>>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>> dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>>> __func__, cmd, key->cipher, key->keyidx);
>>>>
>>>> - if (vif->type != NL80211_IFTYPE_STATION)
>>>> - return -EOPNOTSUPP;
>>>> -
>>>> if (key->keyidx > 3)
>>>> return -EOPNOTSUPP;
>>>>
>>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>> ether_addr_copy(mac_addr, sta->addr);
>>>> } else {
>>>> dev_dbg(dev, "%s: group key\n", __func__);
>>>> - eth_broadcast_addr(mac_addr);
>>>> + ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>>> }
>>>>
>>>> val16 = rtl8xxxu_read16(priv, REG_CR);
>>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>
>>>> switch (cmd) {
>>>> case SET_KEY:
>>>> - key->hw_key_idx = key->keyidx;
>>>> +
>>>> + retval = rtl8xxxu_get_free_sec_cam(hw);
>>>> + if (retval < 0)
>>>> + return -EOPNOTSUPP;
>>>> +
>>>> + key->hw_key_idx = retval;
>>>> +
>>>> + if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>> + rtlvif->hw_key_idx = key->hw_key_idx;
>>>> +
>>>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>>> rtl8xxxu_cam_write(priv, key, mac_addr);
>>>> + set_bit(key->hw_key_idx, priv->cam_map);
>>>
>>> Hi Martin,
>>
>> Hi Bitterblue,
>>
>>>
>>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
>>> when we use iwd. After the WPA2 handshake no more data is transmitted.
>>>
>>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
>>> this patch, when we use iwd, the group key is installed first. It has
>>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
>>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
>>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
>>>
>>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
>>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
>>>
>>> This patch imitating rtw88 makes the old chips work again with iwd.
>>> What do you think?
>>
>> So you reserve the first 4 entries for group keys and use key->keyidx as
>> key->hw_key_idx directly for those, right? Does that work if 2 virtual
>> interfaces are used at the same time?
>> I will do some tests in the next days, but I suspect this be an issue.
>
> Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
> only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
> special handling for this chip?
I would like to enable supports_concurrent for more chips, if I can test
this feature.
>
> I'd share some information about security CAM for reference. Since data is
> quite old (10+ years), I can't guarantee all are correct.
>
> 1. default key switch in rtl8xxxu_set_key()
> SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
> broadcast/multicast packets use default key (CAM entries 0~3 for
> keyidx 0~3).
> For two interfaces case, that'd a problem, so maybe we should use
> software encryption/decryption for non-pairwise key.
>
> val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
> SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
> val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
> rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);
>
> 2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
> The group key field of security CAM is BIT(6) which isn't supported
> by earlier chips (sorry I have no data about the exact chips).
>
BIT 6 is used in the rtl8188eu driver, and not used in the rtl8192cu driver,
so I assume it's just the older RTL8188CU/RTL8192CU and RTL8723AU that don't
support it.
> If a chip can support group key field, the CAM layout with default key
> enabled for two interfaces (STA-1+AP-1) can be
>
> STA-1 <-> AP-10
> AP-1 <-> STA-20
>
> CAM GK MAC KEY
> 0
> 1 1 00: AP-1 (GTK) // should be on entry 0~3 depends on key_idx
> 2
> 3
> 4
> 5 1 AP-10 AP-10 (GTK) // can be on any entry after 4
> 6
>
What are the numbers 1, 10, and 20 in "AP-1", "AP-10", "STA-20" ?
> (pairwise key can be on any entry after 4)
>
> if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> ctrl |= BIT(6);
>
> My perspective are
> 1. currently default key is enabled, so we should treat CAM entry 0~3 as
> special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
> as Bitterblue's version.
>
> 2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
> to try the method of example 2 above.
>
> If we want earlier chips can support two interfaces, I think we should
> disable default key.
Indeed, the rtl8192cu driver (out of kernel) doesn't set SEC_CFG_TXBC_USE_DEFKEY
and SEC_CFG_RXBC_USE_DEFKEY when concurrent mode is enabled.
> Seemingly, it is not possible that using hardware
> crypto when operating one interface and then switching to use software> crypto when operating two interfaces. That means broadcast/multicast
> packets should be by software crypto entirely if we want to support two
> interfaces.
>
> 3. I think rtw88 has similar problem for two interfaces.
>
>>
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> index ee278f0548e4..f7b35655bec5 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>> {
>>> struct rtl8xxxu_priv *priv = hw->priv;
>>>
>>> - return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>> + return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
>>> + RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>>> }
>>>
>>> static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>
>>> switch (cmd) {
>>> case SET_KEY:
>>> + if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
>>> + retval = rtl8xxxu_get_free_sec_cam(hw);
>>> + if (retval < 0)
>>> + return -EOPNOTSUPP;
>>>
>>> - retval = rtl8xxxu_get_free_sec_cam(hw);
>>> - if (retval < 0)
>>> - return -EOPNOTSUPP;
>>> -
>>> - key->hw_key_idx = retval;
>>> + key->hw_key_idx = retval;
>>> + } else {
>>> + key->hw_key_idx = key->keyidx;
>>> + }
>>>
>>> if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>> rtlvif->hw_key_idx = key->hw_key_idx;
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> index 4b05dba22e67..188f4bbe99cd 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>>> #define RTL8XXXU_BC_MC_MACID 0
>>> #define RTL8XXXU_BC_MC_MACID1 1
>>> #define RTL8XXXU_MAX_SEC_CAM_NUM 64
>>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
>>>
>>> struct rtl8xxxu_priv {
>>> struct ieee80211_hw *hw;
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox