* [PATCH wireless-next] wifi: mac80211: fix AEAD functions not handling errors
From: Miri Korenblit @ 2026-03-23 21:15 UTC (permalink / raw)
To: linux-wireless; +Cc: Emmanuel Grumbach, Johannes Berg
From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
This was done for CMAC but not for AEAD.
This was spotted by Coverity.
Fixes: 26717828b75d ("mac80211: aes-cmac: switch to shash CMAC driver")
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
---
net/mac80211/fils_aead.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index 912c46f74d24..a24c0d0a2b2a 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -28,25 +28,35 @@ static int aes_s2v(struct crypto_shash *tfm,
u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
SHASH_DESC_ON_STACK(desc, tfm);
size_t i;
+ int err;
desc->tfm = tfm;
/* D = AES-CMAC(K, <zero>) */
- crypto_shash_digest(desc, tmp, AES_BLOCK_SIZE, d);
+ err = crypto_shash_digest(desc, tmp, AES_BLOCK_SIZE, d);
+ if (err)
+ return err;
for (i = 0; i < num_elem - 1; i++) {
/* D = dbl(D) xor AES_CMAC(K, Si) */
gf_mulx(d); /* dbl */
- crypto_shash_digest(desc, addr[i], len[i], tmp);
+ err = crypto_shash_digest(desc, addr[i], len[i], tmp);
+ if (err)
+ return err;
crypto_xor(d, tmp, AES_BLOCK_SIZE);
}
- crypto_shash_init(desc);
+ err = crypto_shash_init(desc);
+ if (err)
+ return err;
if (len[i] >= AES_BLOCK_SIZE) {
/* len(Sn) >= 128 */
/* T = Sn xorend D */
- crypto_shash_update(desc, addr[i], len[i] - AES_BLOCK_SIZE);
+ err = crypto_shash_update(desc, addr[i],
+ len[i] - AES_BLOCK_SIZE);
+ if (err)
+ return err;
crypto_xor(d, addr[i] + len[i] - AES_BLOCK_SIZE,
AES_BLOCK_SIZE);
} else {
@@ -57,9 +67,7 @@ static int aes_s2v(struct crypto_shash *tfm,
d[len[i]] ^= 0x80;
}
/* V = AES-CMAC(K, T) */
- crypto_shash_finup(desc, d, AES_BLOCK_SIZE, v);
-
- return 0;
+ return crypto_shash_finup(desc, d, AES_BLOCK_SIZE, v);
}
/* Note: addr[] and len[] needs to have one extra slot at the end. */
--
2.34.1
^ permalink raw reply related
* [PATCH wireless-next] wifi: radiotap: add a field for PHY SERVICE field related data
From: Miri Korenblit @ 2026-03-23 21:12 UTC (permalink / raw)
To: linux-wireless; +Cc: Benjamin Berg, Johannes Berg
From: Benjamin Berg <benjamin.berg@intel.com>
For certain frames bits are encoded into the DATA field of the PHY
header in a way that cannot be properly reported in other ways. Add a
new field to be able to report the SERVICE field and scrambler
initialization value as well as the bandwidth signalling information
that may be encoded here.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
---
include/net/ieee80211_radiotap.h | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/include/net/ieee80211_radiotap.h b/include/net/ieee80211_radiotap.h
index 11c1544bc449..34419d4fd898 100644
--- a/include/net/ieee80211_radiotap.h
+++ b/include/net/ieee80211_radiotap.h
@@ -96,6 +96,7 @@ enum ieee80211_radiotap_presence {
IEEE80211_RADIOTAP_EHT_USIG = 33,
IEEE80211_RADIOTAP_EHT = 34,
IEEE80211_RADIOTAP_OBSERVED_ENERGY = 35,
+ IEEE80211_RADIOTAP_SERVICE_FIELD = 36,
IEEE80211_RADIOTAP_UHR_ELR = 37,
IEEE80211_RADIOTAP_UHR = 38,
};
@@ -624,6 +625,47 @@ struct ieee80211_radiotap_observed_energy {
__le16 bw;
} __packed;
+enum ieee80211_radiotap_service_field_known {
+ IEEE80211_RADIOTAP_SERVICE_FIELD_KNOWN_SERVICE_FIELD = 0x01,
+ IEEE80211_RADIOTAP_SERVICE_FIELD_KNOWN_SCRAMBLER_INIT = 0x02,
+ IEEE80211_RADIOTAP_SERVICE_FIELD_KNOWN_DYN_BW_IN_NON_HT = 0x04,
+ IEEE80211_RADIOTAP_SERVICE_FIELD_KNOWN_CH_BW_IN_NON_HT = 0x08,
+};
+
+enum ieee80211_radiotap_service_field_flags {
+ IEEE80211_RADIOTAP_SERVICE_FIELD_FLAG_DYN_BW_IN_NON_HT = 0x01,
+ IEEE80211_RADIOTAP_SERVICE_FIELD_FLAG_SCRAMBLER_INIT_11 = 0x02,
+};
+
+/**
+ * struct ieee80211_radiotap_service_field - SERVICE field information (type 36)
+ * see www.radiotap.org/fields/SERVICE%20field.html for details.
+ *
+ * For certain frames, information may be encoded early in the SERVICE field of
+ * the PHY header. This information may be reported here.
+ *
+ * The DYN_BANDWIDTH_IN_NON_HT and CH_BANDWIDTH_IN_NON_HT may be reported
+ * separately if the other bits cannot be reported. These fields are only valid
+ * for certain frames when bandwidth signalling is in use.
+ *
+ * @known: Bitmap indicating which information is present
+ * @flags: Bitmap of flags
+ * @service_field: SERVICE field, the lower bits are used to store the
+ * scrambler initialization as the SERVICE field must be all zero there.
+ * The scrambler initialization may be 7 or 11 bit long depending on the
+ * PHY type of the frame. If it is 11 bit, then
+ * %IEEE80211_RADIOTAP_SERVICE_FIELD_FLAG_SCRAMBLER_INIT_11 is set.
+ * @ch_bw_in_non_ht: Value of CH_BANDWIDTH_IN_NON_HT as defined in
+ * IEEE802.11REVmf-D1.0, Table 17-8 - TXVECTOR parameter
+ * CH_BANDWIDTH_IN_NON_HT values.
+ */
+struct ieee80211_radiotap_service_field {
+ u8 known;
+ u8 flags;
+ __le16 service_field;
+ u8 ch_bw_in_non_ht;
+} __packed;
+
/*
* ieee80211_radiotap_uhr_elr - content of UHR-ELR TLV (type 35)
* see https://www.radiotap.org/fields/UHR-ELR for details
--
2.34.1
^ permalink raw reply related
* [PATCH wireless-next] wifi: radiotap: add field for information about observed energy
From: Miri Korenblit @ 2026-03-23 21:09 UTC (permalink / raw)
To: linux-wireless; +Cc: Benjamin Berg
From: Benjamin Berg <benjamin.berg@intel.com>
This can be used to report some information about frames that are
duplicated across multiple channels. The reported information may just
be an estimate based on the energy observed on each subchannel. An
example of such duplication are RTS frames that are sent using a legacy
rate but are duplicated into multiple subchannels for compatibility.
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
---
include/net/ieee80211_radiotap.h | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/net/ieee80211_radiotap.h b/include/net/ieee80211_radiotap.h
index 6c2210a253cd..11c1544bc449 100644
--- a/include/net/ieee80211_radiotap.h
+++ b/include/net/ieee80211_radiotap.h
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2017 Intel Deutschland GmbH
- * Copyright (c) 2018-2019, 2021-2022, 2025 Intel Corporation
+ * Copyright (c) 2018-2019, 2021-2022, 2025-2026 Intel Corporation
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -95,6 +95,7 @@ enum ieee80211_radiotap_presence {
IEEE80211_RADIOTAP_EXT = 31,
IEEE80211_RADIOTAP_EHT_USIG = 33,
IEEE80211_RADIOTAP_EHT = 34,
+ IEEE80211_RADIOTAP_OBSERVED_ENERGY = 35,
IEEE80211_RADIOTAP_UHR_ELR = 37,
IEEE80211_RADIOTAP_UHR = 38,
};
@@ -604,6 +605,25 @@ enum ieee80211_radiotap_eht_usig_tb {
IEEE80211_RADIOTAP_EHT_USIG2_TB_B20_B25_TAIL = 0xfc000000,
};
+/* for IEEE80211_RADIOTAP_OBSERVED_ENERGY */
+enum ieee80211_radiotap_observed_energy_known {
+ IEEE80211_RADIOTAP_OBSERVED_ENERGY_KNOWN_BW = 0x0001,
+};
+
+/**
+ * struct ieee80211_radiotap_observed_energy - Observed Energy (type 35)
+ * see www.radiotap.org/fields/observed%20energy.html for details.
+ *
+ * @known: Bitmap indicating which information is present
+ * @bw: The bandwidth in MHz sufficient to contain all subchannels where energy
+ * likely belonging to the same transmission was observed. This may
+ * indicate that the transmission was duplicated on multiple channels.
+ */
+struct ieee80211_radiotap_observed_energy {
+ __le16 known;
+ __le16 bw;
+} __packed;
+
/*
* ieee80211_radiotap_uhr_elr - content of UHR-ELR TLV (type 35)
* see https://www.radiotap.org/fields/UHR-ELR for details
--
2.34.1
^ permalink raw reply related
* [PATCH wireless-next] wifi: cfg80211: Add support for additional 7 GHz channels
From: Miri Korenblit @ 2026-03-23 21:02 UTC (permalink / raw)
To: linux-wireless; +Cc: Ilan Peer
From: Ilan Peer <ilan.peer@intel.com>
Add support for channels 237, 241, 245, 249, 253 and support
for additional 320 MHz segment.
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
---
net/wireless/chan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index fa0764ede9c5..2dcf18f5655e 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -317,7 +317,7 @@ static bool cfg80211_valid_center_freq(u32 center,
int step;
/* We only do strict verification on 6 GHz */
- if (center < 5955 || center > 7115)
+ if (center < 5955 || center > 7215)
return true;
bw = nl80211_chan_width_to_mhz(width);
@@ -325,7 +325,7 @@ static bool cfg80211_valid_center_freq(u32 center,
return false;
/* Validate that the channels bw is entirely within the 6 GHz band */
- if (center - bw / 2 < 5945 || center + bw / 2 > 7125)
+ if (center - bw / 2 < 5945 || center + bw / 2 > 7225)
return false;
/* With 320 MHz the permitted channels overlap */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 2/9] dt-bindings: mmc: amlogic: Add compatible for T7 mmc
From: Conor Dooley @ 2026-03-23 19:38 UTC (permalink / raw)
To: Ronald Claveau
Cc: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Johannes Berg, van Spriel, linux-arm-kernel, linux-amlogic,
devicetree, linux-kernel, linux-mmc, linux-wireless
In-Reply-To: <20260323-add-emmc-t7-vim4-v3-2-5159d90a984c@aliel.fr>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 8/9] dt-bindings: net: wireless: brcm: Add compatible for bcm43752
From: Conor Dooley @ 2026-03-23 19:37 UTC (permalink / raw)
To: Ronald Claveau
Cc: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Johannes Berg, van Spriel, linux-arm-kernel, linux-amlogic,
devicetree, linux-kernel, linux-mmc, linux-wireless
In-Reply-To: <20260323-add-emmc-t7-vim4-v3-8-5159d90a984c@aliel.fr>
[-- Attachment #1: Type: text/plain, Size: 250 bytes --]
On Mon, Mar 23, 2026 at 10:55:33AM +0100, Ronald Claveau wrote:
> Add bcm43752 compatible with its bcm4329 compatible fallback.
>
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] wifi: mac80211: Replace strncpy() with strscpy_pad() in drv_switch_vif_chanctx tracepoint
From: Johannes Berg @ 2026-03-23 17:34 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-kernel, linux-wireless, linux-hardening
In-Reply-To: <2cc7113dfe94622c1e8e482948f26d9268ef2538.camel@sipsolutions.net>
On Mon, 2026-03-23 at 18:32 +0100, Johannes Berg wrote:
> On Mon, 2026-03-23 at 10:20 -0700, Kees Cook wrote:
> > ring buffer. Since ring buffer entries are not zeroed on allocation,
> >
> > No behavioral change: since interface names are always at most 15
> > characters plus a NUL terminator, strscpy_pad() with size IFNAMSIZ (16)
> > produces identical output to the original strncpy().
>
> This doesn't add up - if it's not zero-filled on allocation, then it
> *is* a (desired) behaviour change?
Err, no, never mind, it's str*n*cpy() now...
johannes
^ permalink raw reply
* Re: [PATCH] wifi: mac80211: Replace strncpy() with strscpy_pad() in drv_switch_vif_chanctx tracepoint
From: Johannes Berg @ 2026-03-23 17:32 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-kernel, linux-wireless, linux-hardening
In-Reply-To: <20260323172015.work.146-kees@kernel.org>
On Mon, 2026-03-23 at 10:20 -0700, Kees Cook wrote:
> ring buffer. Since ring buffer entries are not zeroed on allocation,
>
> No behavioral change: since interface names are always at most 15
> characters plus a NUL terminator, strscpy_pad() with size IFNAMSIZ (16)
> produces identical output to the original strncpy().
This doesn't add up - if it's not zero-filled on allocation, then it
*is* a (desired) behaviour change?
johannes
^ permalink raw reply
* [PATCH] wifi: mac80211: Replace strncpy() with strscpy_pad() in drv_switch_vif_chanctx tracepoint
From: Kees Cook @ 2026-03-23 17:20 UTC (permalink / raw)
To: Johannes Berg; +Cc: Kees Cook, linux-kernel, linux-wireless, linux-hardening
Replace the deprecated[1] strncpy() with strscpy_pad() for copying the
interface name into a tracepoint entry.
The source "sdata->name" is a NUL-terminated char[IFNAMSIZ] buffer
populated via NUL-guaranteeing paths: strscpy() in ieee80211_if_add(),
snprintf() in ieee80211_add_virtual_monitor(), or memcpy() from
ndev->name in ieee80211_if_add() and netdev_notify()
(net/mac80211/iface.c). In the memcpy() cases, the source ndev->name
is itself always NUL-terminated (populated via snprintf() or strscpy()
in __dev_alloc_name() and dev_prep_valid_name() in net/core/dev.c).
The destination "local_vifs[i].vif.vif_name" is a char[IFNAMSIZ] field
in struct trace_vif_entry, stored in a __dynamic_array within the trace
ring buffer. Since ring buffer entries are not zeroed on allocation,
strscpy_pad() is used to zero-fill trailing bytes and prevent exposing
stale ring buffer contents to userspace readers of tracefs.
No behavioral change: since interface names are always at most 15
characters plus a NUL terminator, strscpy_pad() with size IFNAMSIZ (16)
produces identical output to the original strncpy().
Link: https://github.com/KSPP/linux/issues/90 [1]
Signed-off-by: Kees Cook <kees@kernel.org>
---
net/mac80211/trace.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index c04d4547e8f4..3b139fb67d1b 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1796,9 +1796,8 @@ TRACE_EVENT(drv_switch_vif_chanctx,
SWITCH_ENTRY_ASSIGN(vif.vif_type, vif->type);
SWITCH_ENTRY_ASSIGN(vif.p2p, vif->p2p);
SWITCH_ENTRY_ASSIGN(link_id, link_conf->link_id);
- strncpy(local_vifs[i].vif.vif_name,
- sdata->name,
- sizeof(local_vifs[i].vif.vif_name));
+ strscpy_pad(local_vifs[i].vif.vif_name,
+ sdata->name);
SWITCH_ENTRY_ASSIGN(old_chandef.control_freq,
old_ctx->def.chan->center_freq);
SWITCH_ENTRY_ASSIGN(old_chandef.freq_offset,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH ath-next v3] wifi: ath11k: Pass the correct value of each TID during a stop AMPDU session
From: Vasanthakumar Thiagarajan @ 2026-03-23 16:01 UTC (permalink / raw)
To: Reshma Immaculate Rajkumar, ath11k; +Cc: linux-wireless
In-Reply-To: <20260319065608.2408179-1-reshma.rajkumar@oss.qualcomm.com>
On 3/19/2026 12:26 PM, Reshma Immaculate Rajkumar wrote:
> During ongoing traffic, a request to stop an AMPDU session
> for one TID could incorrectly affect other active sessions.
> This can happen because an incorrect TID reference would be
> passed when updating the BA session state, causing the wrong
> session to be stopped. As a result, the affected session would
> be reduced to a minimal BA size, leading to a noticeable
> throughput degradation.
>
> Fix this issue by passing the correct argument from
> ath11k_dp_rx_ampdu_stop() to ath11k_peer_rx_tid_reo_update()
> during a stop AMPDU session. Instead of passing peer->tx_tid, which
> is the base address of the array, corresponding to TID 0; pass
> the value of &peer->rx_tid[params->tid], where the different TID numbers
> are accounted for.
>
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.9.0.1-02146-QCAHKSWPL_SILICONZ-1
>
> Fixes: d5c65159f2895 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Reshma Immaculate Rajkumar <reshma.rajkumar@oss.qualcomm.com>
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
From: Pablo MARTIN-GOMEZ @ 2026-03-23 16:00 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless; +Cc: johannes
In-Reply-To: <20260323101954.874299-1-nbd@nbd.name>
Hello,
On 23/03/2026 11:19, Felix Fietkau wrote:
> Create ieee80211_rate_expected_tx_airtime helper function, which returns
> the expected tx airtime for a given rate and packet length in units of
> 1024 usec, for more accuracy.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
> net/mac80211/airtime.c | 87 ++++++++++++++++++++++----------------
> net/mac80211/ieee80211_i.h | 5 +++
> 2 files changed, 56 insertions(+), 36 deletions(-)
>
> diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
> index c61df637232a..0c54cdbd753c 100644
> --- a/net/mac80211/airtime.c
> +++ b/net/mac80211/airtime.c
> @@ -685,7 +685,7 @@ static int ieee80211_fill_rx_status(struct ieee80211_rx_status *stat,
> if (ieee80211_fill_rate_info(hw, stat, band, ri))
> return 0;
>
> - if (!ieee80211_rate_valid(rate))
> + if (!rate || !ieee80211_rate_valid(rate))
> return -1;
>
> if (rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
> @@ -753,6 +753,53 @@ u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
> }
> EXPORT_SYMBOL_GPL(ieee80211_calc_tx_airtime);
>
> +u32 ieee80211_rate_expected_tx_airtime(struct ieee80211_hw *hw,
> + struct ieee80211_tx_rate *tx_rate,
> + struct rate_info *ri,
> + enum nl80211_band band,
> + bool ampdu, int len)
> +{
> + struct ieee80211_rx_status stat;
> + u32 duration, overhead;
> + u8 agg_shift;
> +
> + if (ieee80211_fill_rx_status(&stat, hw, tx_rate, ri, band, len))
> + return 0;
> +
> + if (stat.encoding == RX_ENC_LEGACY || !ampdu)
> + return ieee80211_calc_rx_airtime(hw, &stat, len) * 1024;
> +
> + duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
> +
> + /*
> + * Assume that HT/VHT transmission on any AC except VO will
> + * use aggregation. Since we don't have reliable reporting
> + * of aggregation length, assume an average size based on the
> + * tx rate.
> + * This will not be very accurate, but much better than simply
> + * assuming un-aggregated tx in all cases.
> + */
> + if (duration > 400 * 1024) /* <= VHT20 MCS2 1S */
> + agg_shift = 1;
> + else if (duration > 250 * 1024) /* <= VHT20 MCS3 1S or MCS1 2S */
> + agg_shift = 2;
> + else if (duration > 150 * 1024) /* <= VHT20 MCS5 1S or MCS2 2S */
> + agg_shift = 3;
> + else if (duration > 70 * 1024) /* <= VHT20 MCS5 2S */
> + agg_shift = 4;
> + else if (stat.encoding != RX_ENC_HE ||
> + duration > 20 * 1024) /* <= HE40 MCS6 2S */
> + agg_shift = 5;
> + else
> + agg_shift = 6;
> +
> + duration *= len;
> + duration /= AVG_PKT_SIZE;
> + duration += (overhead * 1024 >> agg_shift);
I know this patch is just a refactoring, but I think this moved code is
bugged. If (and it's a big if) I understood correctly the chain of
macros and the comments, `ieee80211_get_rate_duration` return the
`duration` in 1024 µs of an average packet (which would imply
1f38b8c564b8 is wrong) and the (PHY) `overhead` in µs for a (average)
packet. So I believe the code should be:
```c
duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
duration *= 1024; /* now duration is in µs */
/* the agg_shift calculation has to be fixed */
duration += (overhead >> agg_shift); /* for one packet, we "assign" a
fraction of the overhead */
duration *= len/AVG_PKT_SIZE; /* we multiply by the number of packets */
duration /= 1024; /* we go back to a duration in 1024 µs*/
return duration;
```
If this is correct, then `ieee80211_calc_rx_airtime` is also bugged
likewise.
> +
> + return duration;
> +}
> +
> u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif,
> struct ieee80211_sta *pubsta,
Pablo MG
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Konrad Dybcio @ 2026-03-23 14:26 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sumit Garg
Cc: 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,
jorge.ramirez, tonyh, vignesh.viswanathan, srinivas.kandagatla,
amirreza.zarrabi, jens.wiklander, op-tee, apurupa, skare,
linux-kernel, Sumit Garg
In-Reply-To: <5c5b49aa-7819-44c6-b5f7-19ec780d73fa@kernel.org>
On 3/23/26 3:19 PM, Krzysztof Kozlowski wrote:
> On 23/03/2026 14:22, Sumit Garg wrote:
>> On Mon, Mar 16, 2026 at 08:51:16AM +0100, Krzysztof Kozlowski wrote:
>>> On 12/03/2026 07:27, Sumit Garg wrote:
>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>>>
>>>> Qcom platforms has the legacy of using non-standard SCM calls
>>>> splintered over the various kernel drivers. These SCM calls aren't
>>>> compliant with the standard SMC calling conventions which is a
>>>> prerequisite to enable migration to the FF-A specifications from Arm.
>>>>
>>>> OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
>>>> support these non-standard SCM calls. And even for newer architectures
>>>> with S-EL2 and Hafnium support, QTEE won't be able to support SCM
>>>> calls either with FF-A requirements coming in. And with both OP-TEE
>>>> and QTEE drivers well integrated in the TEE subsystem, it makes further
>>>> sense to reuse the TEE bus client drivers infrastructure.
>>>>
>>>> The added benefit of TEE bus infrastructure is that there is support
>>>> for discoverable/enumerable services. With that client drivers don't
>>>> have to manually invoke a special SCM call to know the service status.
>>>>
>>>> So enable the generic Peripheral Authentication Service (PAS) provided
>>>> by the firmware. It acts as the common layer with different TZ
>>>> backends plugged in whether it's an SCM implementation or a proper
>>>> TEE bus based PAS service implementation.
>>>>
>>>> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>>> ---
>>>> drivers/firmware/qcom/Kconfig | 8 +
>>>> drivers/firmware/qcom/Makefile | 1 +
>>>> drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
>>>> drivers/firmware/qcom/qcom_pas.h | 53 +++++
>>>> include/linux/firmware/qcom/qcom_pas.h | 41 ++++
>>>> 5 files changed, 401 insertions(+)
>>>> create mode 100644 drivers/firmware/qcom/qcom_pas.c
>>>> create mode 100644 drivers/firmware/qcom/qcom_pas.h
>>>> create mode 100644 include/linux/firmware/qcom/qcom_pas.h
>>>>
>>>> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
>>>> index b477d54b495a..8653639d06db 100644
>>>> --- a/drivers/firmware/qcom/Kconfig
>>>> +++ b/drivers/firmware/qcom/Kconfig
>>>> @@ -6,6 +6,14 @@
>>>>
>>>> menu "Qualcomm firmware drivers"
>>>>
>>>> +config QCOM_PAS
>>>> + tristate
>>>> + help
>>>> + Enable the generic Peripheral Authentication Service (PAS) provided
>>>> + by the firmware. It acts as the common layer with different TZ
>>>> + backends plugged in whether it's an SCM implementation or a proper
>>>> + TEE bus based PAS service implementation.
>>>> +
>>>> config QCOM_SCM
>>>> select QCOM_TZMEM
>>>> tristate
>>>> diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
>>>> index 0be40a1abc13..dc5ab45f906a 100644
>>>> --- a/drivers/firmware/qcom/Makefile
>>>> +++ b/drivers/firmware/qcom/Makefile
>>>> @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
>>>> obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
>>>> obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
>>>> obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
>>>> +obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
>>>> diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
>>>> new file mode 100644
>>>> index 000000000000..beb1bae55546
>>>> --- /dev/null
>>>> +++ b/drivers/firmware/qcom/qcom_pas.c
>>>> @@ -0,0 +1,298 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>>> + */
>>>> +
>>>> +#include <linux/device/devres.h>
>>>> +#include <linux/firmware/qcom/qcom_pas.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +
>>>> +#include "qcom_pas.h"
>>>> +
>>>> +struct qcom_pas_ops *ops_ptr;
>>>
>>> Same comment as before. Don't create singletons. And for sure not global
>>> ones.
>>
>> This pattern has been carried from the PAS API contract among kernel
>> clients and the SCM PAS service earlier. The clients don't hold a
>> reference to the PAS data like underlying platform or TEE device etc.
>> Hence the need to have a global data pointer to hold reference to the
>> ops data structure registered by drivers having different lifetime of
>> devices. Also, the PAS APIs can be called from very different client
>> driver contexts.
>>
>> Surely, avoiding global data is always better given a better alternative
>> is there. Do you have any better alternative proposal here?
>
> Why it cannot be part of the context?
>
> Look at your API, e.g.:
> qcom_pas_init_image(). It takes struct qcom_pas_context which should
> contain the ops.
This would make the client have to select the ops. The whole point is to
avoid that, since the client has no clue (and is supposed not to have any).
What Sumit does is to bind the ops based on the runtime-discovered
mechanism (which needs to only happen once, given we're not replacing the
TZ at runtime)
Konrad
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Krzysztof Kozlowski @ 2026-03-23 14:19 UTC (permalink / raw)
To: Sumit Garg
Cc: 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,
jorge.ramirez, tonyh, vignesh.viswanathan, srinivas.kandagatla,
amirreza.zarrabi, jens.wiklander, op-tee, apurupa, skare,
linux-kernel, Sumit Garg
In-Reply-To: <acE-kAi2tkPh2qie@sumit-xelite>
On 23/03/2026 14:22, Sumit Garg wrote:
> On Mon, Mar 16, 2026 at 08:51:16AM +0100, Krzysztof Kozlowski wrote:
>> On 12/03/2026 07:27, Sumit Garg wrote:
>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>>
>>> Qcom platforms has the legacy of using non-standard SCM calls
>>> splintered over the various kernel drivers. These SCM calls aren't
>>> compliant with the standard SMC calling conventions which is a
>>> prerequisite to enable migration to the FF-A specifications from Arm.
>>>
>>> OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
>>> support these non-standard SCM calls. And even for newer architectures
>>> with S-EL2 and Hafnium support, QTEE won't be able to support SCM
>>> calls either with FF-A requirements coming in. And with both OP-TEE
>>> and QTEE drivers well integrated in the TEE subsystem, it makes further
>>> sense to reuse the TEE bus client drivers infrastructure.
>>>
>>> The added benefit of TEE bus infrastructure is that there is support
>>> for discoverable/enumerable services. With that client drivers don't
>>> have to manually invoke a special SCM call to know the service status.
>>>
>>> So enable the generic Peripheral Authentication Service (PAS) provided
>>> by the firmware. It acts as the common layer with different TZ
>>> backends plugged in whether it's an SCM implementation or a proper
>>> TEE bus based PAS service implementation.
>>>
>>> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>> ---
>>> drivers/firmware/qcom/Kconfig | 8 +
>>> drivers/firmware/qcom/Makefile | 1 +
>>> drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
>>> drivers/firmware/qcom/qcom_pas.h | 53 +++++
>>> include/linux/firmware/qcom/qcom_pas.h | 41 ++++
>>> 5 files changed, 401 insertions(+)
>>> create mode 100644 drivers/firmware/qcom/qcom_pas.c
>>> create mode 100644 drivers/firmware/qcom/qcom_pas.h
>>> create mode 100644 include/linux/firmware/qcom/qcom_pas.h
>>>
>>> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
>>> index b477d54b495a..8653639d06db 100644
>>> --- a/drivers/firmware/qcom/Kconfig
>>> +++ b/drivers/firmware/qcom/Kconfig
>>> @@ -6,6 +6,14 @@
>>>
>>> menu "Qualcomm firmware drivers"
>>>
>>> +config QCOM_PAS
>>> + tristate
>>> + help
>>> + Enable the generic Peripheral Authentication Service (PAS) provided
>>> + by the firmware. It acts as the common layer with different TZ
>>> + backends plugged in whether it's an SCM implementation or a proper
>>> + TEE bus based PAS service implementation.
>>> +
>>> config QCOM_SCM
>>> select QCOM_TZMEM
>>> tristate
>>> diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
>>> index 0be40a1abc13..dc5ab45f906a 100644
>>> --- a/drivers/firmware/qcom/Makefile
>>> +++ b/drivers/firmware/qcom/Makefile
>>> @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
>>> obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
>>> obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
>>> obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
>>> +obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
>>> diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
>>> new file mode 100644
>>> index 000000000000..beb1bae55546
>>> --- /dev/null
>>> +++ b/drivers/firmware/qcom/qcom_pas.c
>>> @@ -0,0 +1,298 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +
>>> +#include <linux/device/devres.h>
>>> +#include <linux/firmware/qcom/qcom_pas.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +
>>> +#include "qcom_pas.h"
>>> +
>>> +struct qcom_pas_ops *ops_ptr;
>>
>> Same comment as before. Don't create singletons. And for sure not global
>> ones.
>
> This pattern has been carried from the PAS API contract among kernel
> clients and the SCM PAS service earlier. The clients don't hold a
> reference to the PAS data like underlying platform or TEE device etc.
> Hence the need to have a global data pointer to hold reference to the
> ops data structure registered by drivers having different lifetime of
> devices. Also, the PAS APIs can be called from very different client
> driver contexts.
>
> Surely, avoiding global data is always better given a better alternative
> is there. Do you have any better alternative proposal here?
Why it cannot be part of the context?
Look at your API, e.g.:
qcom_pas_init_image(). It takes struct qcom_pas_context which should
contain the ops.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH rtw-next] wifi: rtw89: phy: fix uninitialized variable access in rtw89_phy_cfo_set_crystal_cap()
From: Alexey Velichayshiy @ 2026-03-23 14:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: Alexey Velichayshiy, Yuan-Han Zhang, Kalle Valo, linux-wireless,
linux-kernel, lvc-project
In the rtw89_phy_cfo_set_crystal_cap() function, for chips other than
RTL8852A/RTL8851B, the values read by rtw89_mac_read_xtal_si() are
stored into the local variables sc_xi_val and sc_xo_val. If either
read fails, these variables remain uninitialized, they are later
used to update cfo->crystal_cap and in debug print statements. This
can lead to undefined behavior.
Fix the issue by initializing sc_xi_val and sc_xo_val to zero,
like is implemented in vendor driver.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 8379fa611536 ("rtw89: 8852c: add write/read crystal function in CFO tracking")
Signed-off-by: Alexey Velichayshiy <a.velichayshiy@ispras.ru>
---
drivers/net/wireless/realtek/rtw89/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index ee6ab2136b9a..ee8a36003e5d 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -4860,7 +4860,7 @@ static void rtw89_phy_cfo_set_crystal_cap(struct rtw89_dev *rtwdev,
{
struct rtw89_cfo_tracking_info *cfo = &rtwdev->cfo_tracking;
const struct rtw89_chip_info *chip = rtwdev->chip;
- u8 sc_xi_val, sc_xo_val;
+ u8 sc_xi_val = 0, sc_xo_val = 0;
if (!force && cfo->crystal_cap == crystal_cap)
return;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 04/15] firmware: qcom: Add a PAS TEE service
From: Sumit Garg @ 2026-03-23 13:38 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313110747.v5bx2snpbtyja3ur@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 04:37:47PM +0530, Mukesh Ojha wrote:
> On Thu, Mar 12, 2026 at 11:57:45AM +0530, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > Add support for Peripheral Authentication Service (PAS) driver based
> > on TEE bus with OP-TEE providing the backend PAS service implementation.
> >
> > The TEE PAS service ABI is designed to be extensible with additional API
> > as PTA_QCOM_PAS_CAPABILITIES. This allows to accommodate any future
> > extensions of the PAS service needed while still maintaining backwards
> > compatibility.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 9 +
> > drivers/firmware/qcom/Makefile | 1 +
> > drivers/firmware/qcom/qcom_pas_tee.c | 477 +++++++++++++++++++++++++++
> > 3 files changed, 487 insertions(+)
> > create mode 100644 drivers/firmware/qcom/qcom_pas_tee.c
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index 9a12ae2b639d..fff47abdaafd 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -14,6 +14,15 @@ config QCOM_PAS
> > backends plugged in whether it's an SCM implementation or a proper
> > TEE bus based PAS service implementation.
> >
> > +config QCOM_PAS_TEE
> > + tristate
> > + select QCOM_PAS
> > + depends on TEE
> > + depends on !CPU_BIG_ENDIAN
> > + help
> > + Enable the generic Peripheral Authentication Service (PAS) provided
> > + by the firmware TEE implementation as the backend.
> > +
> > config QCOM_SCM
> > select QCOM_PAS
> > select QCOM_TZMEM
> > diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
> > index dc5ab45f906a..48801d18f37b 100644
> > --- a/drivers/firmware/qcom/Makefile
> > +++ b/drivers/firmware/qcom/Makefile
> > @@ -9,3 +9,4 @@ obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
> > obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
> > obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
> > obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
> > +obj-$(CONFIG_QCOM_PAS_TEE) += qcom_pas_tee.o
> > diff --git a/drivers/firmware/qcom/qcom_pas_tee.c b/drivers/firmware/qcom/qcom_pas_tee.c
> > new file mode 100644
> > index 000000000000..7db9fd736369
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas_tee.c
> > @@ -0,0 +1,477 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/of.h>
> > +#include <linux/firmware/qcom/qcom_pas.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/tee_drv.h>
> > +#include <linux/uuid.h>
> > +
> > +#include "qcom_pas.h"
> > +
> > +/*
> > + * Peripheral Authentication Service (PAS) supported.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + */
> > +#define PTA_QCOM_PAS_IS_SUPPORTED 1
> > +
> > +/*
> > + * PAS capabilities.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [out] params[1].value.a: PAS capability flags
> > + */
> > +#define PTA_QCOM_PAS_CAPABILITIES 2
> > +
> > +/*
> > + * PAS image initialization.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [in] params[1].memref: Loadable firmware metadata
> > + */
> > +#define PTA_QCOM_PAS_INIT_IMAGE 3
> > +
> > +/*
> > + * PAS memory setup.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [in] params[0].value.b: Relocatable firmware size
> > + * [in] params[1].value.a: 32bit LSB relocatable firmware memory address
> > + * [in] params[1].value.b: 32bit MSB relocatable firmware memory address
> > + */
> > +#define PTA_QCOM_PAS_MEM_SETUP 4
> > +
> > +/*
> > + * PAS get resource table.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [inout] params[1].memref: Resource table config
> > + */
> > +#define PTA_QCOM_PAS_GET_RESOURCE_TABLE 5
> > +
> > +/*
> > + * PAS image authentication and co-processor reset.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [in] params[0].value.b: Firmware size
> > + * [in] params[1].value.a: 32bit LSB firmware memory address
> > + * [in] params[1].value.b: 32bit MSB firmware memory address
> > + * [in] params[2].memref: Optional fw memory space shared/lent
> > + */
> > +#define PTA_QCOM_PAS_AUTH_AND_RESET 6
> > +
> > +/*
> > + * PAS co-processor set suspend/resume state.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + * [in] params[0].value.b: Co-processor state identifier
> > + */
> > +#define PTA_QCOM_PAS_SET_REMOTE_STATE 7
> > +
> > +/*
> > + * PAS co-processor shutdown.
> > + *
> > + * [in] params[0].value.a: Unique 32bit remote processor identifier
> > + */
> > +#define PTA_QCOM_PAS_SHUTDOWN 8
> > +
> > +#define TEE_NUM_PARAMS 4
> > +
> > +/**
> > + * struct qcom_pas_tee_private - PAS service private data
> > + * @dev: PAS service device.
> > + * @ctx: TEE context handler.
> > + * @session_id: PAS TA session identifier.
> > + */
> > +struct qcom_pas_tee_private {
> > + struct device *dev;
> > + struct tee_context *ctx;
> > + u32 session_id;
> > +};
> > +
> > +static bool qcom_pas_tee_supported(struct device *dev, u32 pas_id)
> > +{
> > + struct qcom_pas_tee_private *data = dev_get_drvdata(dev);
> > + struct tee_ioctl_invoke_arg inv_arg = {
> > + .func = PTA_QCOM_PAS_IS_SUPPORTED,
> > + .session = data->session_id,
> > + .num_params = TEE_NUM_PARAMS
> > + };
> > + struct tee_param param[4] = {
> > + [0] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + .u.value.a = pas_id
> > + }
> > + };
> > + int ret;
> > +
> > + ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
> > + if (ret < 0 || inv_arg.ret != 0) {
> > + dev_err(dev, "PAS not supported, pas_id: %d, err: %x\n",
> > + pas_id, inv_arg.ret);
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static int qcom_pas_tee_init_image(struct device *dev, u32 pas_id,
> > + const void *metadata, size_t size,
> > + struct qcom_pas_context *ctx)
> > +{
> > + struct qcom_pas_tee_private *data = dev_get_drvdata(dev);
> > + struct tee_ioctl_invoke_arg inv_arg = {
> > + .func = PTA_QCOM_PAS_INIT_IMAGE,
> > + .session = data->session_id,
> > + .num_params = TEE_NUM_PARAMS
> > + };
> > + struct tee_param param[4] = {
> > + [0] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + .u.value.a = pas_id
> > + },
> > + [1] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
> > + }
> > + };
> > + struct tee_shm *mdata_shm;
> > + u8 *mdata_buf = NULL;
> > + int ret;
> > +
> > + mdata_shm = tee_shm_alloc_kernel_buf(data->ctx, size);
> > + if (IS_ERR(mdata_shm)) {
> > + dev_err(dev, "mdata_shm allocation failed\n");
> > + return PTR_ERR(mdata_shm);
> > + }
> > +
> > + mdata_buf = tee_shm_get_va(mdata_shm, 0);
> > + if (IS_ERR(mdata_buf)) {
> > + dev_err(dev, "mdata_buf get VA failed\n");
> > + tee_shm_free(mdata_shm);
> > + return PTR_ERR(mdata_buf);
> > + }
> > + memcpy(mdata_buf, metadata, size);
> > +
> > + param[1].u.memref.shm = mdata_shm;
> > + param[1].u.memref.size = size;
> > +
> > + ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
> > + if (ret < 0 || inv_arg.ret != 0) {
> > + dev_err(dev, "PAS init image failed, pas_id: %d, err: %x\n",
> > + pas_id, inv_arg.ret);
> > + tee_shm_free(mdata_shm);
> > + return -EINVAL;
> > + }
> > + ctx->ptr = (void *)mdata_shm;
> > +
> > + return 0;
> > +}
> > +
> > +static int qcom_pas_tee_mem_setup(struct device *dev, u32 pas_id,
> > + phys_addr_t addr, phys_addr_t size)
> > +{
> > + struct qcom_pas_tee_private *data = dev_get_drvdata(dev);
> > + struct tee_ioctl_invoke_arg inv_arg = {
> > + .func = PTA_QCOM_PAS_MEM_SETUP,
> > + .session = data->session_id,
> > + .num_params = TEE_NUM_PARAMS
> > + };
> > + struct tee_param param[4] = {
> > + [0] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + .u.value.a = pas_id,
> > + .u.value.b = size,
> > + },
> > + [1] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + .u.value.a = lower_32_bits(addr),
> > + .u.value.b = upper_32_bits(addr),
> > + }
> > + };
> > + int ret;
> > +
> > + ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
> > + if (ret < 0 || inv_arg.ret != 0) {
> > + dev_err(dev, "PAS mem setup failed, pas_id: %d, err: %x\n",
> > + pas_id, inv_arg.ret);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_FREE(shm_free, struct tee_shm *, tee_shm_free(_T))
> > +
> > +static void *qcom_pas_tee_get_rsc_table(struct device *dev,
> > + struct qcom_pas_context *ctx,
> > + void *input_rt, size_t input_rt_size,
> > + size_t *output_rt_size)
> > +{
> > + struct qcom_pas_tee_private *data = dev_get_drvdata(dev);
> > + struct tee_ioctl_invoke_arg inv_arg = {
> > + .func = PTA_QCOM_PAS_GET_RESOURCE_TABLE,
> > + .session = data->session_id,
> > + .num_params = TEE_NUM_PARAMS
> > + };
> > + struct tee_param param[4] = {
> > + [0] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + .u.value.a = ctx->pas_id,
> > + },
> > + [1] = {
> > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> > + .u.memref.size = input_rt_size,
> > + }
> > + };
> > + void *rt_buf = NULL;
> > + int ret;
> > +
> > + ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
>
> What is the purpose of this function ? looks like, this is for, how
> much Linux need to allocate for output buffer ?
That's right.
>
> > + if (ret < 0 || inv_arg.ret != 0) {
> > + dev_err(dev, "PAS get RT failed, pas_id: %d, err: %x\n",
> > + ctx->pas_id, inv_arg.ret);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + if (param[1].u.memref.size) {
> > + struct tee_shm *rt_shm __free(shm_free) =
> > + tee_shm_alloc_kernel_buf(data->ctx,
> > + param[1].u.memref.size);
> > + void *rt_shm_va;
> > +
> > + if (IS_ERR(rt_shm)) {
> > + dev_err(dev, "rt_shm allocation failed\n");
> > + return rt_shm;
> > + }
> > +
> > + rt_shm_va = tee_shm_get_va(rt_shm, 0);
> > + if (IS_ERR_OR_NULL(rt_shm_va)) {
> > + dev_err(dev, "rt_shm get VA failed\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > + memcpy(rt_shm_va, input_rt, input_rt_size);
> > +
> > + param[1].u.memref.shm = rt_shm;
>
> Here, you are passing only one buffer for both input and output ?
>
> Like, you are allocating of buffer of size returned from qtee which I
s/qtee/optee/
> assume includes both input + output rt size and copying the input_rt
> and calling invoke and in return you will get combine table in return ?
That's right.
>
> > + ret = tee_client_invoke_func(data->ctx, &inv_arg, param);
> > + if (ret < 0 || inv_arg.ret != 0) {
> > + dev_err(dev, "PAS get RT failed, pas_id: %d, err: %x\n",
> > + ctx->pas_id, inv_arg.ret);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + if (param[1].u.memref.size) {
> > + *output_rt_size = param[1].u.memref.size;
> > + rt_buf = kmalloc(param[1].u.memref.size, GFP_KERNEL);
> > + if (!rt_buf)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + memcpy(rt_buf, rt_shm_va, *output_rt_size);
>
> rt_buf = kmemdup(rt_shm_va, *output_rt_size, GFP_KERNEL);
>
> https://lore.kernel.org/lkml/20260310140255.2520230-1-mukesh.ojha@oss.qualcomm.com/
>
Sure, I will use that instead.
-Sumit
^ permalink raw reply
* Re: [PATCH v2 03/15] firmware: qcom_scm: Migrate to generic PAS service
From: Sumit Garg @ 2026-03-23 13:33 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313075607.2mw3dzaf274xxe2j@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 01:26:07PM +0530, Mukesh Ojha wrote:
> On Thu, Mar 12, 2026 at 11:57:44AM +0530, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > With the availability of generic PAS service, let's add SCM calls as
> > a backend to keep supporting legacy QTEE interfaces. The exported
> > qcom_scm* wrappers will get dropped once all the client drivers get
> > migrated as part of future patches.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 1 +
> > drivers/firmware/qcom/qcom_scm.c | 336 ++++++++++++++-----------------
> > 2 files changed, 156 insertions(+), 181 deletions(-)
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index 8653639d06db..9a12ae2b639d 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -15,6 +15,7 @@ config QCOM_PAS
> > TEE bus based PAS service implementation.
> >
> > config QCOM_SCM
> > + select QCOM_PAS
> > select QCOM_TZMEM
> > tristate
> >
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index 8fbc96693a55..2d7937ae7c8f 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -13,6 +13,7 @@
> > #include <linux/dma-mapping.h>
> > #include <linux/err.h>
> > #include <linux/export.h>
> > +#include <linux/firmware/qcom/qcom_pas.h>
> > #include <linux/firmware/qcom/qcom_scm.h>
> > #include <linux/firmware/qcom/qcom_tzmem.h>
> > #include <linux/init.h>
> > @@ -33,6 +34,7 @@
> >
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> >
> > +#include "qcom_pas.h"
> > #include "qcom_scm.h"
> > #include "qcom_tzmem.h"
> >
> > @@ -480,25 +482,6 @@ void qcom_scm_cpu_power_down(u32 flags)
> > }
> > EXPORT_SYMBOL_GPL(qcom_scm_cpu_power_down);
> >
> > -int qcom_scm_set_remote_state(u32 state, u32 id)
> > -{
> > - struct qcom_scm_desc desc = {
> > - .svc = QCOM_SCM_SVC_BOOT,
> > - .cmd = QCOM_SCM_BOOT_SET_REMOTE_STATE,
> > - .arginfo = QCOM_SCM_ARGS(2),
> > - .args[0] = state,
> > - .args[1] = id,
> > - .owner = ARM_SMCCC_OWNER_SIP,
> > - };
> > - struct qcom_scm_res res;
> > - int ret;
> > -
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > -
> > - return ret ? : res.result[0];
> > -}
> > -EXPORT_SYMBOL_GPL(qcom_scm_set_remote_state);
> > -
> > static int qcom_scm_disable_sdi(void)
> > {
> > int ret;
> > @@ -571,26 +554,12 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
> > dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
> > }
> >
> > -/**
> > - * devm_qcom_scm_pas_context_alloc() - Allocate peripheral authentication service
> > - * context for a given peripheral
> > - *
> > - * PAS context is device-resource managed, so the caller does not need
> > - * to worry about freeing the context memory.
> > - *
> > - * @dev: PAS firmware device
> > - * @pas_id: peripheral authentication service id
> > - * @mem_phys: Subsystem reserve memory start address
> > - * @mem_size: Subsystem reserve memory size
> > - *
> > - * Returns: The new PAS context, or ERR_PTR() on failure.
> > - */
> > struct qcom_scm_pas_context *devm_qcom_scm_pas_context_alloc(struct device *dev,
> > u32 pas_id,
> > phys_addr_t mem_phys,
> > size_t mem_size)
> > {
> > - struct qcom_scm_pas_context *ctx;
> > + struct qcom_pas_context *ctx;
>
> Why this change..
This is needed for migration to the generic PAS APIs as the internal
wrappers switched over. However, this is just a temporary global wrapper
until all the clients switch over..
>
> >
> > ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > if (!ctx)
> > @@ -601,11 +570,12 @@ struct qcom_scm_pas_context *devm_qcom_scm_pas_context_alloc(struct device *dev,
> > ctx->mem_phys = mem_phys;
> > ctx->mem_size = mem_size;
> >
> > - return ctx;
> > + return (struct qcom_scm_pas_context *)ctx;
>
> and this change as well ?
..ditto as above to maintain API compatibility. This whole wrapper gets
dropped in the last patch.
>
> > }
> > EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_alloc);
> >
> > -static int __qcom_scm_pas_init_image(u32 pas_id, dma_addr_t mdata_phys,
> > +static int __qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
> > + dma_addr_t mdata_phys,
> > struct qcom_scm_res *res)
> > {
> > struct qcom_scm_desc desc = {
> > @@ -627,7 +597,7 @@ static int __qcom_scm_pas_init_image(u32 pas_id, dma_addr_t mdata_phys,
> >
> > desc.args[1] = mdata_phys;
> >
> > - ret = qcom_scm_call(__scm->dev, &desc, res);
> > + ret = qcom_scm_call(dev, &desc, res);
> > qcom_scm_bw_disable();
> >
> > disable_clk:
> > @@ -636,7 +606,8 @@ static int __qcom_scm_pas_init_image(u32 pas_id, dma_addr_t mdata_phys,
> > return ret;
> > }
> >
> > -static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
> > +static int qcom_scm_pas_prep_and_init_image(struct device *dev,
> > + struct qcom_pas_context *ctx,
> > const void *metadata, size_t size)
> > {
> > struct qcom_scm_res res;
> > @@ -651,7 +622,7 @@ static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
> > memcpy(mdata_buf, metadata, size);
> > mdata_phys = qcom_tzmem_to_phys(mdata_buf);
> >
> > - ret = __qcom_scm_pas_init_image(ctx->pas_id, mdata_phys, &res);
> > + ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res);
> > if (ret < 0)
> > qcom_tzmem_free(mdata_buf);
> > else
> > @@ -660,25 +631,9 @@ static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
> > return ret ? : res.result[0];
> > }
> >
> > -/**
> > - * qcom_scm_pas_init_image() - Initialize peripheral authentication service
> > - * state machine for a given peripheral, using the
> > - * metadata
> > - * @pas_id: peripheral authentication service id
> > - * @metadata: pointer to memory containing ELF header, program header table
> > - * and optional blob of data used for authenticating the metadata
> > - * and the rest of the firmware
> > - * @size: size of the metadata
> > - * @ctx: optional pas context
> > - *
> > - * Return: 0 on success.
> > - *
> > - * Upon successful return, the PAS metadata context (@ctx) will be used to
> > - * track the metadata allocation, this needs to be released by invoking
> > - * qcom_scm_pas_metadata_release() by the caller.
> > - */
> > -int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > - struct qcom_scm_pas_context *ctx)
> > +static int __qcom_scm_pas_init_image2(struct device *dev, u32 pas_id,
> > + const void *metadata, size_t size,
> > + struct qcom_pas_context *ctx)
>
> Looks like alignment got wrong..
It isn't, try applying this patch-set and see if you still see any
alignment issues.
>
> > {
> > struct qcom_scm_res res;
> > dma_addr_t mdata_phys;
> > @@ -686,7 +641,8 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > int ret;
> >
> > if (ctx && ctx->use_tzmem)
> > - return qcom_scm_pas_prep_and_init_image(ctx, metadata, size);
> > + return qcom_scm_pas_prep_and_init_image(dev, ctx, metadata,
> > + size);
>
> unwrap this..
Ack.
>
> >
> > /*
> > * During the scm call memory protection will be enabled for the meta
> > @@ -700,16 +656,15 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > * If we pass a buffer that is already part of an SHM Bridge to this
> > * call, it will fail.
> > */
> > - mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
> > - GFP_KERNEL);
> > + mdata_buf = dma_alloc_coherent(dev, size, &mdata_phys, GFP_KERNEL);
> > if (!mdata_buf)
> > return -ENOMEM;
> >
> > memcpy(mdata_buf, metadata, size);
> >
> > - ret = __qcom_scm_pas_init_image(pas_id, mdata_phys, &res);
> > + ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res);
> > if (ret < 0 || !ctx) {
> > - dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys);
> > + dma_free_coherent(dev, size, mdata_buf, mdata_phys);
> > } else if (ctx) {
> > ctx->ptr = mdata_buf;
> > ctx->phys = mdata_phys;
> > @@ -718,36 +673,35 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> >
> > return ret ? : res.result[0];
> > }
> > -EXPORT_SYMBOL_GPL(qcom_scm_pas_init_image);
> >
> > -/**
> > - * qcom_scm_pas_metadata_release() - release metadata context
> > - * @ctx: pas context
> > - */
> > -void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx)
> > +int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > + struct qcom_scm_pas_context *ctx)
> > {
> > - if (!ctx->ptr)
> > - return;
> > + return __qcom_scm_pas_init_image2(__scm->dev, pas_id, metadata, size,
> > + (struct qcom_pas_context *)ctx);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_scm_pas_init_image);
> >
> > +static void __qcom_scm_pas_metadata_release(struct device *dev,
> > + struct qcom_pas_context *ctx)
> > +{
> > if (ctx->use_tzmem)
> > qcom_tzmem_free(ctx->ptr);
> > else
> > - dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys);
> > + dma_free_coherent(dev, ctx->size, ctx->ptr, ctx->phys);
> >
> > ctx->ptr = NULL;
> > }
> > +
> > +void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx)
> > +{
> > + __qcom_scm_pas_metadata_release(__scm->dev,
> > + (struct qcom_pas_context *)ctx);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_metadata_release);
> >
> > -/**
> > - * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
> > - * for firmware loading
> > - * @pas_id: peripheral authentication service id
> > - * @addr: start address of memory area to prepare
> > - * @size: size of the memory area to prepare
> > - *
> > - * Returns 0 on success.
> > - */
> > -int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > +static int __qcom_scm_pas_mem_setup(struct device *dev, u32 pas_id,
> > + phys_addr_t addr, phys_addr_t size)
> > {
> > int ret;
> > struct qcom_scm_desc desc = {
> > @@ -769,7 +723,7 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > if (ret)
> > goto disable_clk;
> >
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > + ret = qcom_scm_call(dev, &desc, &res);
> > qcom_scm_bw_disable();
> >
> > disable_clk:
> > @@ -777,9 +731,15 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> >
> > return ret ? : res.result[0];
> > }
> > +
> > +int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > +{
> > + return __qcom_scm_pas_mem_setup(__scm->dev, pas_id, addr, size);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
> >
> > -static void *__qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt_tzm,
> > +static void *__qcom_scm_pas_get_rsc_table(struct device *dev, u32 pas_id,
> > + void *input_rt_tzm,
> > size_t input_rt_size,
> > size_t *output_rt_size)
> > {
> > @@ -814,7 +774,7 @@ static void *__qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt_tzm,
> > * with output_rt_tzm buffer with res.result[2] size however, It should not
> > * be of unresonable size.
> > */
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > + ret = qcom_scm_call(dev, &desc, &res);
> > if (!ret && res.result[2] > SZ_1G) {
> > ret = -E2BIG;
> > goto free_output_rt;
> > @@ -831,51 +791,11 @@ static void *__qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt_tzm,
> > return ret ? ERR_PTR(ret) : output_rt_tzm;
> > }
> >
> > -/**
> > - * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> > - * for a given peripheral.
> > - *
> > - * Qualcomm remote processor may rely on both static and dynamic resources for
> > - * its functionality. Static resources typically refer to memory-mapped addresses
> > - * required by the subsystem and are often embedded within the firmware binary
> > - * and dynamic resources, such as shared memory in DDR etc., are determined at
> > - * runtime during the boot process.
> > - *
> > - * On Qualcomm Technologies devices, it's possible that static resources are not
> > - * embedded in the firmware binary and instead are provided by TrustZone However,
> > - * dynamic resources are always expected to come from TrustZone. This indicates
> > - * that for Qualcomm devices, all resources (static and dynamic) will be provided
> > - * by TrustZone via the SMC call.
> > - *
> > - * If the remote processor firmware binary does contain static resources, they
> > - * should be passed in input_rt. These will be forwarded to TrustZone for
> > - * authentication. TrustZone will then append the dynamic resources and return
> > - * the complete resource table in output_rt_tzm.
> > - *
> > - * If the remote processor firmware binary does not include a resource table,
> > - * the caller of this function should set input_rt as NULL and input_rt_size
> > - * as zero respectively.
> > - *
> > - * More about documentation on resource table data structures can be found in
> > - * include/linux/remoteproc.h
> > - *
> > - * @ctx: PAS context
> > - * @pas_id: peripheral authentication service id
> > - * @input_rt: resource table buffer which is present in firmware binary
> > - * @input_rt_size: size of the resource table present in firmware binary
> > - * @output_rt_size: TrustZone expects caller should pass worst case size for
> > - * the output_rt_tzm.
> > - *
> > - * Return:
> > - * On success, returns a pointer to the allocated buffer containing the final
> > - * resource table and output_rt_size will have actual resource table size from
> > - * TrustZone. The caller is responsible for freeing the buffer. On failure,
> > - * returns ERR_PTR(-errno).
> > - */
> > -struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx,
> > - void *input_rt,
> > - size_t input_rt_size,
> > - size_t *output_rt_size)
> > +static void *__qcom_scm_pas_get_rsc_table2(struct device *dev,
> > + struct qcom_pas_context *ctx,
> > + void *input_rt,
> > + size_t input_rt_size,
> > + size_t *output_rt_size)
> > {
> > struct resource_table empty_rsc = {};
> > size_t size = SZ_16K;
> > @@ -910,11 +830,12 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
> >
> > memcpy(input_rt_tzm, input_rt, input_rt_size);
> >
> > - output_rt_tzm = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt_tzm,
> > + output_rt_tzm = __qcom_scm_pas_get_rsc_table(dev, ctx->pas_id,
> > + input_rt_tzm,
> > input_rt_size, &size);
> > if (PTR_ERR(output_rt_tzm) == -EOVERFLOW)
> > /* Try again with the size requested by the TZ */
> > - output_rt_tzm = __qcom_scm_pas_get_rsc_table(ctx->pas_id,
> > + output_rt_tzm = __qcom_scm_pas_get_rsc_table(dev, ctx->pas_id,
> > input_rt_tzm,
> > input_rt_size,
> > &size);
> > @@ -945,16 +866,20 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
> >
> > return ret ? ERR_PTR(ret) : tbl_ptr;
> > }
> > +
> > +struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx,
> > + void *input_rt,
> > + size_t input_rt_size,
> > + size_t *output_rt_size)
> > +{
> > + return __qcom_scm_pas_get_rsc_table2(__scm->dev,
>
> Instead of using integar, we could use addition of more '_' to reflect
> inner level functions..
Let me rather drop the integer as part of last patch when I am already
dropping wrappers.
>
> > + (struct qcom_pas_context *)ctx,
> > + input_rt, input_rt_size,
> > + output_rt_size);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_get_rsc_table);
> >
> > -/**
> > - * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
> > - * and reset the remote processor
> > - * @pas_id: peripheral authentication service id
> > - *
> > - * Return 0 on success.
> > - */
> > -int qcom_scm_pas_auth_and_reset(u32 pas_id)
> > +static int __qcom_scm_pas_auth_and_reset(struct device *dev, u32 pas_id)
> > {
> > int ret;
> > struct qcom_scm_desc desc = {
> > @@ -974,7 +899,7 @@ int qcom_scm_pas_auth_and_reset(u32 pas_id)
> > if (ret)
> > goto disable_clk;
> >
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > + ret = qcom_scm_call(dev, &desc, &res);
> > qcom_scm_bw_disable();
> >
> > disable_clk:
> > @@ -982,28 +907,15 @@ int qcom_scm_pas_auth_and_reset(u32 pas_id)
> >
> > return ret ? : res.result[0];
> > }
> > +
> > +int qcom_scm_pas_auth_and_reset(u32 pas_id)
> > +{
> > + return __qcom_scm_pas_auth_and_reset(__scm->dev, pas_id);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_auth_and_reset);
> >
> > -/**
> > - * qcom_scm_pas_prepare_and_auth_reset() - Prepare, authenticate, and reset the
> > - * remote processor
> > - *
> > - * @ctx: Context saved during call to qcom_scm_pas_context_init()
> > - *
> > - * This function performs the necessary steps to prepare a PAS subsystem,
> > - * authenticate it using the provided metadata, and initiate a reset sequence.
> > - *
> > - * It should be used when Linux is in control setting up the IOMMU hardware
> > - * for remote subsystem during secure firmware loading processes. The preparation
> > - * step sets up a shmbridge over the firmware memory before TrustZone accesses the
> > - * firmware memory region for authentication. The authentication step verifies
> > - * the integrity and authenticity of the firmware or configuration using secure
> > - * metadata. Finally, the reset step ensures the subsystem starts in a clean and
> > - * sane state.
> > - *
> > - * Return: 0 on success, negative errno on failure.
> > - */
> > -int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
> > +static int __qcom_scm_pas_prepare_and_auth_reset(struct device *dev,
> > + struct qcom_pas_context *ctx)
> > {
> > u64 handle;
> > int ret;
> > @@ -1014,7 +926,7 @@ int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
> > * memory region and then invokes a call to TrustZone to authenticate.
> > */
> > if (!ctx->use_tzmem)
> > - return qcom_scm_pas_auth_and_reset(ctx->pas_id);
> > + return __qcom_scm_pas_auth_and_reset(dev, ctx->pas_id);
> >
> > /*
> > * When Linux runs @ EL2 Linux must create the shmbridge itself and then
> > @@ -1024,20 +936,45 @@ int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
> > if (ret)
> > return ret;
> >
> > - ret = qcom_scm_pas_auth_and_reset(ctx->pas_id);
> > + ret = __qcom_scm_pas_auth_and_reset(dev, ctx->pas_id);
> > qcom_tzmem_shm_bridge_delete(handle);
> >
> > return ret;
> > }
> > +
> > +int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
> > +{
> > + return __qcom_scm_pas_prepare_and_auth_reset(__scm->dev,
> > + (struct qcom_pas_context *)ctx);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_prepare_and_auth_reset);
> >
> > -/**
> > - * qcom_scm_pas_shutdown() - Shut down the remote processor
> > - * @pas_id: peripheral authentication service id
> > - *
> > - * Returns 0 on success.
> > - */
> > -int qcom_scm_pas_shutdown(u32 pas_id)
> > +static int __qcom_scm_pas_set_remote_state(struct device *dev, u32 state,
> > + u32 pas_id)
> > +{
> > + struct qcom_scm_desc desc = {
> > + .svc = QCOM_SCM_SVC_BOOT,
> > + .cmd = QCOM_SCM_BOOT_SET_REMOTE_STATE,
> > + .arginfo = QCOM_SCM_ARGS(2),
> > + .args[0] = state,
> > + .args[1] = pas_id,
> > + .owner = ARM_SMCCC_OWNER_SIP,
> > + };
> > + struct qcom_scm_res res;
> > + int ret;
> > +
> > + ret = qcom_scm_call(dev, &desc, &res);
> > +
> > + return ret ? : res.result[0];
> > +}
> > +
> > +int qcom_scm_set_remote_state(u32 state, u32 id)
> > +{
> > + return __qcom_scm_pas_set_remote_state(__scm->dev, state, id);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_scm_set_remote_state);
> > +
> > +static int __qcom_scm_pas_shutdown(struct device *dev, u32 pas_id)
> > {
> > int ret;
> > struct qcom_scm_desc desc = {
> > @@ -1057,7 +994,7 @@ int qcom_scm_pas_shutdown(u32 pas_id)
> > if (ret)
> > goto disable_clk;
> >
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > + ret = qcom_scm_call(dev, &desc, &res);
> > qcom_scm_bw_disable();
> >
> > disable_clk:
> > @@ -1065,16 +1002,14 @@ int qcom_scm_pas_shutdown(u32 pas_id)
> >
> > return ret ? : res.result[0];
> > }
> > +
> > +int qcom_scm_pas_shutdown(u32 pas_id)
> > +{
> > + return __qcom_scm_pas_shutdown(__scm->dev, pas_id);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_shutdown);
> >
> > -/**
> > - * qcom_scm_pas_supported() - Check if the peripheral authentication service is
> > - * available for the given peripherial
> > - * @pas_id: peripheral authentication service id
> > - *
> > - * Returns true if PAS is supported for this peripheral, otherwise false.
> > - */
> > -bool qcom_scm_pas_supported(u32 pas_id)
> > +static bool __qcom_scm_pas_supported(struct device *dev, u32 pas_id)
> > {
> > int ret;
> > struct qcom_scm_desc desc = {
> > @@ -1086,16 +1021,49 @@ bool qcom_scm_pas_supported(u32 pas_id)
> > };
> > struct qcom_scm_res res;
> >
> > - if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL,
> > + if (!__qcom_scm_is_call_available(dev, QCOM_SCM_SVC_PIL,
> > QCOM_SCM_PIL_PAS_IS_SUPPORTED))
> > return false;
> >
> > - ret = qcom_scm_call(__scm->dev, &desc, &res);
> > + ret = qcom_scm_call(dev, &desc, &res);
> >
> > return ret ? false : !!res.result[0];
> > }
> > +
> > +bool qcom_scm_pas_supported(u32 pas_id)
> > +{
> > + return __qcom_scm_pas_supported(__scm->dev, pas_id);
> > +}
> > EXPORT_SYMBOL_GPL(qcom_scm_pas_supported);
> >
> > +static struct qcom_pas_ops qcom_pas_ops_scm = {
> > + .drv_name = "qcom_scm",
> > + .supported = __qcom_scm_pas_supported,
> > + .init_image = __qcom_scm_pas_init_image2,
> > + .mem_setup = __qcom_scm_pas_mem_setup,
> > + .get_rsc_table = __qcom_scm_pas_get_rsc_table2,
> > + .auth_and_reset = __qcom_scm_pas_auth_and_reset,
> > + .prepare_and_auth_reset = __qcom_scm_pas_prepare_and_auth_reset,
> > + .set_remote_state = __qcom_scm_pas_set_remote_state,
> > + .shutdown = __qcom_scm_pas_shutdown,
> > + .metadata_release = __qcom_scm_pas_metadata_release,
> > +};
> > +
> > +/**
> > + * qcom_scm_is_pas_available() - Check if the peripheral authentication service
> > + * is available via SCM or not
> > + *
> > + * Returns true if PAS is available, otherwise false.
> > + */
> > +static bool qcom_scm_is_pas_available(void)
> > +{
> > + if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL,
> > + QCOM_SCM_PIL_PAS_AUTH_AND_RESET))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > static int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)
> > {
> > struct qcom_scm_desc desc = {
> > @@ -2782,6 +2750,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
> >
> > __get_convention();
> >
> > + if (qcom_scm_is_pas_available()) {
> > + qcom_pas_ops_scm.dev = scm->dev;
> > + qcom_pas_ops_register(&qcom_pas_ops_scm);
> > + }
> > +
> > /*
> > * If "download mode" is requested, from this point on warmboot
> > * will cause the boot stages to enter download mode, unless
> > @@ -2818,6 +2791,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev)
> > {
> > /* Clean shutdown, disable download mode to allow normal restart */
> > qcom_scm_set_download_mode(QCOM_DLOAD_NODUMP);
> > + qcom_pas_ops_unregister();
> > }
> >
> > static const struct of_device_id qcom_scm_dt_match[] = {
> > --
> > 2.51.0
> >
>
> nit: please double check the alignment due to name and 'static' addition
> to the function..
Alignment is fine, it doesn't show appropriately on the plain text
emails.
-Sumit
^ permalink raw reply
* Re: [PATCH wireless-next v2 00/34] wifi: inffmac: introducing a driver for Infineon's new generation chipsets
From: Gokul Sivakumar @ 2026-03-23 13:24 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-wireless, Johannes Berg, Arend van Spriel,
wlan-kernel-dev-list
In-Reply-To: <dc7ba2e6-ff7e-4581-b127-0c0a00e39051@nabladev.com>
On 03/21, Marek Vasut wrote:
> On 2/27/26 3:34 PM, Gokul Sivakumar wrote:
> > On 02/27, Marek Vasut wrote:
> > > On 1/16/26 5:33 PM, Gokul Sivakumar wrote:
> > > > On 01/15, Marek Vasut wrote:
> > > > > On 1/14/26 9:12 AM, Gokul Sivakumar wrote:
> > > > > > On 01/14, Marek Vasut wrote:
> > > > > > > On 1/13/26 9:33 PM, Gokul Sivakumar wrote:
> > > > > > > > Infineon(Cypress) is introducing a new INFFMAC (WLAN FULLMAC) Linux driver
> > > > > > > > specifically for its new-generation AIROC family of Wi-Fi Connectivity
> > > > > > > > Processor (CP) chipsets (CYW5591x), Wi-Fi + Bluetooth combo chipsets
> > > > > > > > (CYW5557x, CYW5551x, CYW5591x, CYW43022), and also for all future chipsets.
> > > > > > > Support for the CYW55572 can be easily added into the existing brcmfmac
> > > > > > > driver, I already posted a patch over a year ago [1], but it was blocked
> > > > > > > by an off-list email.
> > > > > >
> > > > > > > Frankly, I do not see any good reason why the brcmfmac driver shouldn't
> > > > > > > be extended when it is clearly easily doable. Adding new fork of the
> > > > > > > brcmfmac would only increase maintenance burden and prevent bugfixes
> > > > > > > from reaching the brcmfmac.
> > > > > > >
> > > > > > > [1] https://lore.kernel.org/all/20240909203133.74777-2-marex@denx.de/
> > > > > >
> > > > > > There are multiple reasons behind Infineon's proposal for this new INFFMAC
> > > > > > driver for its new-generation chips. Sharing a few here, and more info
> > > > > > is available in the v1 cover-letter [1]. For Example, the CYW5591x family
> > > > > > chipsets that is currently supported in this INFFMAC driver has a unique
> > > > > > Connected-MCU / Connectivtiy Processor (CP) Architecture [2], primarly
> > > > > > intended for IoT applications and it is completely different from any of
> > > > > > the legacy Broadcom architecture chipsets of Infineon supported currently
> > > > > > in upstream BRCMFMAC.
> > > > >
> > > > > This does not prevent them from being integrated in brcmfmac like many
> > > > > of the other already supported chips, there seems to be no technical
> > > > > reason here.
> > > >
> > > > I hope you would have got a chance to look into the elaborate info provided
> > > > in v1 cover-letter about the challenges that we faced with that approach.
> > > > In Infineon's new architecture CYW5591x family of chipsets coming with an
> > > > onboard FLASH Memory, it does not follow the traditional Device firmware
> > > > download sequence used by any of the legacy Broadcom chipsets and it would
> > > > be appropriate to use a dedicated driver for Infineon's chipsets which were
> > > > developed and intended mainly for IoT use-cases.
> > > >
> > > > Here it requires a Boot firmware download, and a host handshake with the
> > > > downloaded RAM boot firmware, followed by a CP Firmware image Download and
> > > > validation. Sharing here the sequence for coldboot image download to FLASH.
> > > >
> > > > Host Driver Device
> > > > ___________ ____________
> > > > | |
> > > > | Power UP and execute CP Firmware
> > > > | from FLASH if available
> > > > Attempt to enable SDIO F2 |
> > > > if fails, FLASH is empty, |
> > > > Need to Download Firmware |
> > > > | |
> > > > Wait for |
> > > > DFU_CP_D2H_MSG_BL_READY |
> > > > |<<======== DFU_CP_D2H_MSG_BL_READY <<============|
> > > > | |
> > > > Fetch Boot Firmware |
> > > > from Filesystem |
> > > > |======>> DFU_CP_H2D_MSG_BOOT_FWLOAD_START =====>>|
> > > > | |
> > > > |======>> Transfer Boot Firmware over SDIO =====>>|
> > > > | in multiple chunks |
> > > > | |
> > > > |======>> DFU_CP_H2D_MSG_BOOT_FWLOAD_DONE ======>>|
> > > > | Execute the downloaded
> > > > | RAM Bootloader
> > > > Wait for |
> > > > DFU_CP_D2H_MSG_BOOTFW_READY |
> > > > |<<========= DFU_CP_D2H_MSG_BOOTFW_READY <<=======|
> > > > | |
> > > > Fetch CP Firmware |
> > > > from Filesystem |
> > > > |=======>> Transfer CP Firmware over SDIO ======>>|
> > > > | in multiple chunks |
> > > > | Download CP Firmware
> > > > | to FLASH Memory
> > > > | |
> > > > |=======>> DFU_CP_H2D_MSG_BOOTFW_DATA_LOADED ====>|
> > > > | |
> > > > |=======>> DFU_CP_D2H_MSG_FW_VALIDAT_START ======>|
> > > > | |
> > > > Wait for Validate the CP Firmware
> > > > DFU_CP_D2H_MSG_FW_VALIDAT_DONE |
> > > > |<<======= DFU_CP_D2H_MSG_FW_VALIDAT_DONE <<======|
> > > > | |
> > > > | Execute the CP Firmware
> > > > Proceed with |
> > > > ctrl cmds to Device |
> > > > | |
> > > >
> > > > Here there is no need to redownload the Device Firmware from the Host
> > > > filesystem everyime the Host or Device goes through a power cycle. Also the
> > > > device is capable of fully operating in an offloaded mode even when the
> > > > host is in suspended state or even if fully powered off. This is critical
> > > > to save power for extended durations in IoT use cases which are often
> > > > battery operated.
> > >
> > > This seems like a technical detail of the firmware loading, which can be
> > > handled by the driver ?
> >
> > If this is the base of your argument, then theoritically any vendor driver can
> > be added with the functionality to manage any other WLAN chip from another vendor.
> > A single monolithic wireless driver could potentially manage every vendor's chips,
> > without the need for any hardware specific abstraction. But, clearly this is not
> > how it is currently in wireless subsystem, because of multiple compelling reasons.
>
> No, my argument is, that whatever firmware loading mechanism can be
> easily separated from the operation of the driver.
Well, the separation is not that straight forward. The firmware loading mechanism is
different because the internal architecture of the chipset is different from any of the
existing chipsets. And this changes the driver operation too, since the driver needs to
communicate with different components of CYW5591x chipset (CP Core, FW Core, etc) for
different operations it needs to perform.
> > And given that Infineon's new architecture CYW5591x family of chipsets are completely
> > new, tailor made inhouse for IoT use-cases, with a different chipset architecture,
> > different bootflow, etc. it is not practical to add it into another vendor's driver
> > source which has a different flow for managing its devices.
>
> It seems all the development even for the new chips was done on top of
> brcmfmac [1] , and this driver seems like a copy-and-rename of brcmfmac.
>
> [1] https://github.com/Infineon/ifx-wireless-drivers/
No, "downstream Infineon Github release" was a very special case, given only on older
stable kernel versions for selected subset of customers who had specific use cases.
There Infineon customers don't expect downstream drivers to support any non-Infineon
WLAN chipset, and so Infineon specific Driver-Device communication flow and other IoT
specific features were integrated properly as intended, because of no functionality
conflict with other vendors in the same downstream driver. And this support will not
be continued for ever.
> > > > > > The CYW5591x family chipsets has dedicated MCU Core
> > > > > > in addition to the WLAN core and an onboard FLASH memory for the Firmware.
> > > > >
> > > > > It seems all brcmfmac devices have a cortex-M core in them since a long
> > > > > time.
> > > >
> > > > We were intending to mention that CYW5591x Cortex-M33 core is dedicated to
> > > > function as a Co-Processor to the Host CPU/MCU and offload multiple wifi
> > > > and network operations if configured by the user before host goes to sleep.
> > > > You can refer the chipset block diagram in CYW55913/2/1 Product Brief [1]
> > > > The CYW5591x SoC Backplane interconnects are also different from any of the
> > > > legacy Broadcom chipsets.
> > >
> > > Maybe the driver can be forked when it turns out this functionality
> > > cannot be easily added into the existing driver ?
> >
> > Our response remains the same, as given for your exact same question few weeks
> > back in this thread. We shared our explantion with the rationale behind our proposal.
>
> Writing pages of text won't get you too far, preparing a patchset on top
> of brcmfmac will be a good start, and it would show willingness to
> cooperate with upstream.
We strongly believe that sharing our detailed reasoning in our responses, will help the
wider community to understand why we are proposing this and why we could not be tied to
brcmfmac, for Infineon's in-house newer generation chipsets. We are definitely
committed to co-operating with the community and welcoming all constructive feedback.
> > > > > > And with respect to the support for the new-generation CYW5557x family of
> > > > > > Secure chipsets, that requires a Secure boot handshake with the Bootloader.
> > > > >
> > > > > It seems like the TRX firmware loading is trivial to add, and parts of
> > > > > it are already in the Linux kernel for other brcm components. It seems
> > > > > this TRX was used since old broadcom MIPS SoCs.
> > > >
> > > > No, your understanding needs a correction. Infineon's TRXSE and TRXS Device
> > > > Bootloader handshake and Firmware formats are quite different from the old
> > > > Broadcom TRX formats that you are mentioning here. Being two individual
> > > > WLAN vendors, the chipset internals, firmware formats, Device Bootloader
> > > > handshake cannot be expected to be the same. Yet another reason why it is
> > > > not practically feasible to use upstream BRCMFMAC for the new-generation
> > > > chipsets and new firmware releases from Infineon.
> > >
> > > Surely it is possible to do something like
> > >
> > > if (wlan_id_needs_trxse)
> > > load_trxse();
> > > else
> > > load_old_firmware_style();
> > >
> > > ?
> > >
> > > It seems this was already done in the two patches I submitted about a
> > > year ago, so this is clearly doable.
> >
> > Along with TRXSE and TRXS firmwares, Infineon also uses a TRX format firmware,
> > which is also different with the TRX format of Broadcom's firmware. So still
> > need to differentiate between TRX firmwares because the WLAN vendor is different.
> > This is not a question about whether it is about doable, more than that, whether
> > it is practical.
>
> It is a firmware loader, a piece of code that is well isolated.
What we are saying is, Infineon has a different variants of TRX, TRXS, TRXSE firmware
binaries and Broadcom/other WLAN vendors have different variants of TRX firmwares.
So it is not practical to extensively add multiple nested levels vendor specific checks
(Vendor type check -> Vendor bin type check -> Vendor FW bin TRX* type check ->
Vendor FW bin TRX* Version check, etc) in an upstream WLAN driver that is already
convoluted with too many vendor specific checks (like FWVID) for sending cmds to Device.
And those existing FWVID checks only help solve some minor problems. But cannot handle
the scenario like, Firmware/Chipsets from two different WLAN vendors expect the driver
to send the control cmds to Device in a different sequence. Given that the existing model
is not scalable in the longer run, we believe this is an appropriate time to move away
from further convoluting the same upstream driver shared by multiple vendors.
> > > > > > Even if the enumeration and firmware download support for the CYW55572 is
> > > > > > somehow retro-fitted into the existing upstream BRCMFMAC driver, there are
> > > > > > multiple other features and aspects of the Driver-Device communication that
> > > > > > are unique to each WLAN vendor, which are not always practically feasible
> > > > > > to support in the same upstream driver.
> > > > >
> > > > > Why ?
> > > >
> > > > As mentioned before, various aspects of the Driver-Device communication are
> > > > unique to each WLAN vendor. And it is not practically feasible to confine
> > > > them into a common upstream driver, also not sustainable to maintain it that
> > > > way througout the lifetime of the hardware.
> > >
> > > The patches I submitted showed this was feasible, so I am not buying
> > > that argument. Can you give that a try instead ?
> >
> > Our argument is on practical feasibility of doing so and its implications throughout
> > the lifetime of this new-generation device, when later tring to add other features and
> > when Infineon specific Driver-Device communiction flow evolve even more. This is quite
> > different from feasbility of making brcmfmac just detect the chip and load the firmware.
> > It is not sustainable to confine the support for a chips from multiple WLAN vendors
> > into the same upstream Linux driver.
>
> Apparently it is, and it was already done by infineon themselves, see
> [1] above.
No, you are conflating two different things. My argument on "lack of practical feasibility"
and "unsustainability" in adding/maintaining new-gen Infineon chipsets in a driver shared
among WLAN vendors, is in the context of "upstream Linux driver" on latest development kernel.
This may not apply for the "downstream Infineon Github release" created for special cases,
which is expected to be always used by only one vendor who created it and its customers,
where there no conflict among vendor implementations.
> In fact, extending and maintaining existing driver is a much better way
> to start with Linux kernel upstreaming than dumping a 50 kLoC duplicate
> of existing driver onto the list.
As we mentioned in our earlier emails, for legacy Infineon chipsets (from broadcom) that are
already supported on upstream brcmfmac, we will surely extend it with new Infineon features.
And also willing to help maintain upstream brcmfmac. But for new-generation Infineon chipsets
developed in house, the same driver cannot be extended to avoid convoluting the current
upstream brcmfmac driver codebase even more with any additional vendor specific checks and
continue the risk of frequent regressions among different WLAN vendors.
The critical aspects of the driver operation, functionality of managing the device, chipset
architecture, Driver-Device Secure chip handshake. Runtime firmware update, control-path,
data-path and other features are quite different. Only some commands/events IDs and few
functions might look similar between the two drivers, that is primarily because some of the
fundamental aspects of these Full MAC chipsets need change very rarely. This do not make the
drivers an exact duplicate of each other.
> Even if the initial support is not complete, it can be extended over
> time, that is normal. And it is much easier to upstream support by
> gradually improving/extending the already upstream code than dump a
> large chunk of code all at once and expect it to get applied.
We took the similar feedback received on V1 very seriously, and already removed many of the
functionality in v2 to reduce the driver size and make it easier for review. Will shrink it
even further in upcoming revision, so that removed funcationality can be sent later as
incremental patches.
> > Yes, we are aware of the couple patches that you submitted for upstream review. Before
> > that, initially those two patches were first released by Infineon downstream, only for
> > specific end use-cases and not actually intended for upstream.
> >
> > > > > > Because currently BRCMFMAC driver
> > > > > > has a shared ownership with more than 3 WLAN vendor organizations and this
> > > > > > approach has its limitations.
> > > > >
> > > > > Yes, this means it is necessary to cooperate and coordinate with other
> > > > > people, on the mailing list.
> > > >
> > > > Infineon is committed to coordinate with other vendors in managing upstream
> > > > BRCMFMAC driver and continuing the support for its legacy chipsets that
> > > > still follows the legacy Broadcom architecture. Like for example, we have
> > > > added support for CYW54591-PCIe in upstream BRCMFMAC few months back [2],
> > > > because it has many things in common with other legacy Broadcom chips.
> > > > But the same does not applies for all the new-generation Infineon chipsets.
> > > >
> > > > > > For Example, the version of the PCIe and SDIO
> > > > > > BUS specific shared info in Device Memory is expected same from chipsets
> > > > > > from all vendors. There would be a complex need to maintain vendor specifc
> > > > > > as well as BUS specific Shared info version, vendor specific BUS Protocol
> > > > > > layers, vendor specific firmware event header OUIs (currently always expects
> > > > > > BRCM OUI even from other vendor chips) and even more.
> > > > >
> > > > > This sounds like code refactoring is necessary.
> > > > >
> > > > > > Confining different architecture chips from different WLAN vendors into the
> > > > > > same legacy BRCMFMAC driver codebase, may not be sustainable and could
> > > > > > potentially increase the maintainence effort and codebase complexity.
> > > > > > And not practically feasible to continue splitting this driver with more
> > > > > > vendor specific checks in the longer run. Since being different vendors,
> > > > > > each will only naturally diverge even more in the future with their chipset
> > > > > > Architecture, Driver-Device communication flow, etc. Infineon will continue
> > > > > > to support its legacy chipsets, already supported in the upstream BRCMFMAC.
> > > > > Maybe all the extra functionality can be added later, and the driver can
> > > > > be forked later, when it becomes clear that refactoring is not an option
> > > > > and it is becoming too difficult to maintain ?
> > > >
> > > > Refactoring the existing upstream BRCMFMAC driver for the new-architecture
> > > > chipsets developed by Infineon is not a practical option now because these
> > > > devices are fairly new. In the lifetime of these devices, the functionality
> > > > of the Devices and Driver interaction are only going to evolve even more.
> > > > Eventually, it would become more cumbersome to maintain as well as to avoid
> > > > regressions for different architecture chips from different WLAN vendors
> > > > if same upstream driver is used.
> > > >
> > > > We strongly believe that this is an appropriate time for thinking about a
> > > > proposal to proceed with a dedicated driver for the new-generation chips
> > > > developed individually by Infineon.
> > > >
> > > > > So far, it seems the current generation chips can be easily added to
> > > > > brcmfmac, even if the feature set would be limited. Adding them would
> > > >
> > > > We are not in the same page here. As mentioned earlier and as you might
> > > > know, there is infact much more to it than adding enumeration support for a
> > > > chip in a driver. Certainly not easy when eventually Infineon is required
> > > > to enable the end users with the actual capabilities of the chipset, also
> > > > Infineon has the responsibility to ensure driver compatibility with all the
> > > > newly shipped Device firmwares.
> > > >
> > > > > allow the maintainers to review such a smaller patchset and get at least
> > > > > some hardware support in, step by step, instead of this mega-patchset.
> > > >
> > > > We understood that review bandwidth is limited and have taken the feedback
> > > > received on v1 very seriously, so multiple features were already stripped
> > > > off in v2. Willing to skip even more functionality in the upcoming versions
> > > > and also ready to provide more information as needed to make the INFFMAC
> > > > driver review as much easy as possible.
> > >
> > > So, can you try and implement minimal TRXSE firmware loading and support
> > > for the CYW555xx for brcmfmac as an example, so it would be clear
> > > whether or not it can be added into the brcmfmac driver ?
> >
> > No, as we mentioned earlier, just adding the enumeration & FW loading support
> > for the chipset in the brcmfmac driver and claiming that the device support is
> > available in the upstream brcmfmac driver would never be enough.
>
> Clearly it is, and the rest can be extended, cf. [1] above.
No, it is incorrect to compare the limitations of an upstream driver (shared among
multiple vendors) available on latest kernel, to a downstream driver (single vendor)
released on some older stable kernel for that vendor's specific customers.
> > Further, Infineon
> > has the responsibility to eventually add the actual features and capabilities of
> > this new-generation chip in the same upstream driver, so that the end users can
> > utilize the chipset to its fullest capability. It would not be practical to do this
> > later for the new generation Infineon chips, once the DEV ID is added into brcmfmac.
> > Because chipsets from multiple WLAN vendors are currently being managed by the same
> > driver, continuing to add more vendor specific checks for every driver functionality
> > is not sustainable.
>
> Again, gradually improving upstream code is the way to go.
Definitely, this way is applicable for new functionality needed for an existing chipset
supported by an upstream driver. Given that the nature of Infineon's new generation
IoT WiFi Connectivity Processor chipsets like CYW5591x is completely different from any
of the legacy Broadcom WLAN chipsets, it is not appropriate to introduce them into the
legacy brcmfmac driver shared with different WLAN vendors, also whose target end product
use cases are completely different.
> > The dedicated vendor WLAN drivers existing below the common cfg80211 & mac80211 layers
> > in the linux WLAN stack, helps to maintain the Device Hardware specific abstraction,
> > by taking care of the unique Device specific operation, Driver-Device Communication flow.
> > Confining multiple WLAN Vendor chipsets in same upstream driver, would only weaken the
> > abstraction even more.
> >
> > We would like to emphasize that Infineon objective is not to be forever tied to the
> > existing common BRCMFMAC driver for its new generation chipsets, only because the
> > then Cypress (now Infineon) acquired few legacy chipsets (supported in brcmfmac) from
> > Broadcom nearly a decade ago in 2016.
> >
> > > Oh ... and I also noticed, that infineon recently fully rewrote the
> > > history of the firmware repository [1], both the master branch history
> > > and the tags now point to different commit SHAs.
> > >
> > > [1] https://github.com/Infineon/ifx-linux-firmware
> >
> > Ok, we will notify the respective folks internally.
> It seems the problem is still not solved, the repository history was
> rewritten and nobody seems to care.
I noticed that you have filed an issue on the Infineon Github Repository, the appropriate
folks managing it, will respond in Github as soon as possible.
> I also tested this inffmac patchset with CYW55513 and not even scanning
> seems to work.
You must have used an incompatible firmware version. Infineon has different flavours
of firmware binaries with different changes/features, for specific end use cases.
For using specific Infineon chipsets with this new inffmac driver, you would need to
contact us for the compatible firmware binary.
Gokul
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Sumit Garg @ 2026-03-23 13:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: 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,
jorge.ramirez, tonyh, vignesh.viswanathan, srinivas.kandagatla,
amirreza.zarrabi, jens.wiklander, op-tee, apurupa, skare,
linux-kernel, Sumit Garg
In-Reply-To: <28d63822-f191-400a-8005-5185dd480dbb@kernel.org>
On Mon, Mar 16, 2026 at 08:51:16AM +0100, Krzysztof Kozlowski wrote:
> On 12/03/2026 07:27, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > Qcom platforms has the legacy of using non-standard SCM calls
> > splintered over the various kernel drivers. These SCM calls aren't
> > compliant with the standard SMC calling conventions which is a
> > prerequisite to enable migration to the FF-A specifications from Arm.
> >
> > OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> > support these non-standard SCM calls. And even for newer architectures
> > with S-EL2 and Hafnium support, QTEE won't be able to support SCM
> > calls either with FF-A requirements coming in. And with both OP-TEE
> > and QTEE drivers well integrated in the TEE subsystem, it makes further
> > sense to reuse the TEE bus client drivers infrastructure.
> >
> > The added benefit of TEE bus infrastructure is that there is support
> > for discoverable/enumerable services. With that client drivers don't
> > have to manually invoke a special SCM call to know the service status.
> >
> > So enable the generic Peripheral Authentication Service (PAS) provided
> > by the firmware. It acts as the common layer with different TZ
> > backends plugged in whether it's an SCM implementation or a proper
> > TEE bus based PAS service implementation.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 8 +
> > drivers/firmware/qcom/Makefile | 1 +
> > drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
> > drivers/firmware/qcom/qcom_pas.h | 53 +++++
> > include/linux/firmware/qcom/qcom_pas.h | 41 ++++
> > 5 files changed, 401 insertions(+)
> > create mode 100644 drivers/firmware/qcom/qcom_pas.c
> > create mode 100644 drivers/firmware/qcom/qcom_pas.h
> > create mode 100644 include/linux/firmware/qcom/qcom_pas.h
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index b477d54b495a..8653639d06db 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -6,6 +6,14 @@
> >
> > menu "Qualcomm firmware drivers"
> >
> > +config QCOM_PAS
> > + tristate
> > + help
> > + Enable the generic Peripheral Authentication Service (PAS) provided
> > + by the firmware. It acts as the common layer with different TZ
> > + backends plugged in whether it's an SCM implementation or a proper
> > + TEE bus based PAS service implementation.
> > +
> > config QCOM_SCM
> > select QCOM_TZMEM
> > tristate
> > diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
> > index 0be40a1abc13..dc5ab45f906a 100644
> > --- a/drivers/firmware/qcom/Makefile
> > +++ b/drivers/firmware/qcom/Makefile
> > @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> > obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
> > obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
> > obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
> > +obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
> > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
> > new file mode 100644
> > index 000000000000..beb1bae55546
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/device/devres.h>
> > +#include <linux/firmware/qcom/qcom_pas.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +
> > +#include "qcom_pas.h"
> > +
> > +struct qcom_pas_ops *ops_ptr;
>
> Same comment as before. Don't create singletons. And for sure not global
> ones.
This pattern has been carried from the PAS API contract among kernel
clients and the SCM PAS service earlier. The clients don't hold a
reference to the PAS data like underlying platform or TEE device etc.
Hence the need to have a global data pointer to hold reference to the
ops data structure registered by drivers having different lifetime of
devices. Also, the PAS APIs can be called from very different client
driver contexts.
Surely, avoiding global data is always better given a better alternative
is there. Do you have any better alternative proposal here?
-Sumit
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Sumit Garg @ 2026-03-23 13:12 UTC (permalink / raw)
To: Harshal Dev
Cc: 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,
jorge.ramirez, tonyh, vignesh.viswanathan, srinivas.kandagatla,
amirreza.zarrabi, jens.wiklander, op-tee, apurupa, skare,
linux-kernel, Sumit Garg
In-Reply-To: <124f5a78-a3be-4906-bea0-a82bb74b2f96@oss.qualcomm.com>
On Sun, Mar 15, 2026 at 10:03:16PM +0530, Harshal Dev wrote:
>
>
> On 3/12/2026 11:57 AM, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > Qcom platforms has the legacy of using non-standard SCM calls
> > splintered over the various kernel drivers. These SCM calls aren't
> > compliant with the standard SMC calling conventions which is a
> > prerequisite to enable migration to the FF-A specifications from Arm.
> >
> > OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> > support these non-standard SCM calls. And even for newer architectures
> > with S-EL2 and Hafnium support, QTEE won't be able to support SCM
> > calls either with FF-A requirements coming in. And with both OP-TEE
> > and QTEE drivers well integrated in the TEE subsystem, it makes further
> > sense to reuse the TEE bus client drivers infrastructure.
> >
> > The added benefit of TEE bus infrastructure is that there is support
> > for discoverable/enumerable services. With that client drivers don't
> > have to manually invoke a special SCM call to know the service status.
> >
> > So enable the generic Peripheral Authentication Service (PAS) provided
> > by the firmware. It acts as the common layer with different TZ
> > backends plugged in whether it's an SCM implementation or a proper
> > TEE bus based PAS service implementation.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 8 +
> > drivers/firmware/qcom/Makefile | 1 +
> > drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
> > drivers/firmware/qcom/qcom_pas.h | 53 +++++
> > include/linux/firmware/qcom/qcom_pas.h | 41 ++++
> > 5 files changed, 401 insertions(+)
> > create mode 100644 drivers/firmware/qcom/qcom_pas.c
> > create mode 100644 drivers/firmware/qcom/qcom_pas.h
> > create mode 100644 include/linux/firmware/qcom/qcom_pas.h
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index b477d54b495a..8653639d06db 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -6,6 +6,14 @@
> >
> > menu "Qualcomm firmware drivers"
> >
> > +config QCOM_PAS
> > + tristate
> > + help
> > + Enable the generic Peripheral Authentication Service (PAS) provided
> > + by the firmware. It acts as the common layer with different TZ
> > + backends plugged in whether it's an SCM implementation or a proper
> > + TEE bus based PAS service implementation.
> > +
> > config QCOM_SCM
> > select QCOM_TZMEM
> > tristate
> > diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
> > index 0be40a1abc13..dc5ab45f906a 100644
> > --- a/drivers/firmware/qcom/Makefile
> > +++ b/drivers/firmware/qcom/Makefile
> > @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> > obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
> > obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
> > obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
> > +obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
> > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
> > new file mode 100644
> > index 000000000000..beb1bae55546
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/device/devres.h>
> > +#include <linux/firmware/qcom/qcom_pas.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +
> > +#include "qcom_pas.h"
> > +
> > +struct qcom_pas_ops *ops_ptr;
> > +
> > +/**
> > + * devm_qcom_pas_context_alloc() - Allocate peripheral authentication service
> > + * context for a given peripheral
> > + *
> > + * PAS context is device-resource managed, so the caller does not need
> > + * to worry about freeing the context memory.
> > + *
> > + * @dev: PAS firmware device
> > + * @pas_id: peripheral authentication service id
> > + * @mem_phys: Subsystem reserve memory start address
> > + * @mem_size: Subsystem reserve memory size
> > + *
> > + * Return: The new PAS context, or ERR_PTR() on failure.
> > + */
> > +struct qcom_pas_context *devm_qcom_pas_context_alloc(struct device *dev,
> > + u32 pas_id,
> > + phys_addr_t mem_phys,
> > + size_t mem_size)
> > +{
> > + struct qcom_pas_context *ctx;
> > +
> > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ctx->dev = dev;
> > + ctx->pas_id = pas_id;
> > + ctx->mem_phys = mem_phys;
> > + ctx->mem_size = mem_size;
> > +
> > + return ctx;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_qcom_pas_context_alloc);
> > +
> > +/**
> > + * qcom_pas_init_image() - Initialize peripheral authentication service state
> > + * machine for a given peripheral, using the metadata
> > + * @pas_id: peripheral authentication service id
> > + * @metadata: pointer to memory containing ELF header, program header table
> > + * and optional blob of data used for authenticating the metadata
> > + * and the rest of the firmware
> > + * @size: size of the metadata
> > + * @ctx: optional pas context
> > + *
> > + * Return: 0 on success.
> > + *
> > + * Upon successful return, the PAS metadata context (@ctx) will be used to
> > + * track the metadata allocation, this needs to be released by invoking
> > + * qcom_pas_metadata_release() by the caller.
> > + */
> > +int qcom_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > + struct qcom_pas_context *ctx)
> > +{
>
> I think we should check for !ctx everywhere the way we are doing in qcom_pas_metadata_release().
> !ctx->ptr may be checked conditionally based on whether the underlying implementation
> uses it.
The ctx for this API is optional, that means the backend should check
apporpriately for it. I will add that check for TEE backend though.
>
> > + if (ops_ptr)
> > + return ops_ptr->init_image(ops_ptr->dev, pas_id,
> > + metadata, size, ctx);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_init_image);
> > +
> > +/**
> > + * qcom_pas_metadata_release() - release metadata context
> > + * @ctx: pas context
> > + */
> > +void qcom_pas_metadata_release(struct qcom_pas_context *ctx)
> > +{
> > + if (!ctx || !ctx->ptr)
> > + return;
> > +
> > + if (ops_ptr)
> > + ops_ptr->metadata_release(ops_ptr->dev, ctx);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_metadata_release);
> > +
> > +/**
> > + * qcom_pas_mem_setup() - Prepare the memory related to a given peripheral
> > + * for firmware loading
> > + * @pas_id: peripheral authentication service id
> > + * @addr: start address of memory area to prepare
> > + * @size: size of the memory area to prepare
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->mem_setup(ops_ptr->dev, pas_id, addr, size);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_mem_setup);
> > +
> > +/**
> > + * qcom_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> > + * for a given peripheral.
> > + *
> > + * Qualcomm remote processor may rely on both static and dynamic resources for
> > + * its functionality. Static resources typically refer to memory-mapped
> > + * addresses required by the subsystem and are often embedded within the
> > + * firmware binary and dynamic resources, such as shared memory in DDR etc.,
> > + * are determined at runtime during the boot process.
> > + *
> > + * On Qualcomm Technologies devices, it's possible that static resources are
> > + * not embedded in the firmware binary and instead are provided by TrustZone.
> > + * However, dynamic resources are always expected to come from TrustZone. This
> > + * indicates that for Qualcomm devices, all resources (static and dynamic) will
> > + * be provided by TrustZone PAS service.
> > + *
> > + * If the remote processor firmware binary does contain static resources, they
> > + * should be passed in input_rt. These will be forwarded to TrustZone for
> > + * authentication. TrustZone will then append the dynamic resources and return
> > + * the complete resource table in output_rt_tzm.
> > + *
> > + * If the remote processor firmware binary does not include a resource table,
> > + * the caller of this function should set input_rt as NULL and input_rt_size
> > + * as zero respectively.
> > + *
> > + * More about documentation on resource table data structures can be found in
> > + * include/linux/remoteproc.h
> > + *
> > + * @ctx: PAS context
> > + * @pas_id: peripheral authentication service id
> > + * @input_rt: resource table buffer which is present in firmware binary
> > + * @input_rt_size: size of the resource table present in firmware binary
> > + * @output_rt_size: TrustZone expects caller should pass worst case size for
> > + * the output_rt_tzm.
> > + *
> > + * Return:
> > + * On success, returns a pointer to the allocated buffer containing the final
> > + * resource table and output_rt_size will have actual resource table size from
> > + * TrustZone. The caller is responsible for freeing the buffer. On failure,
> > + * returns ERR_PTR(-errno).
> > + */
> > +struct resource_table *qcom_pas_get_rsc_table(struct qcom_pas_context *ctx,
> > + void *input_rt,
> > + size_t input_rt_size,
> > + size_t *output_rt_size)
> > +{
>
> Check for !ctx here as well.
Ack.
>
> > + if (ops_ptr)
> > + return ops_ptr->get_rsc_table(ops_ptr->dev, ctx, input_rt,
> > + input_rt_size, output_rt_size);
> > +
> > + return ERR_PTR(-ENODEV);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_get_rsc_table);
> > +
> > +/**
> > + * qcom_pas_auth_and_reset() - Authenticate the given peripheral firmware
> > + * and reset the remote processor
> > + * @pas_id: peripheral authentication service id
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_auth_and_reset(u32 pas_id)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_auth_and_reset);
> > +
> > +/**
> > + * qcom_pas_prepare_and_auth_reset() - Prepare, authenticate, and reset the
> > + * remote processor
> > + *
> > + * @ctx: Context saved during call to qcom_scm_pas_context_init()
> > + *
> > + * This function performs the necessary steps to prepare a PAS subsystem,
> > + * authenticate it using the provided metadata, and initiate a reset sequence.
> > + *
> > + * It should be used when Linux is in control setting up the IOMMU hardware
> > + * for remote subsystem during secure firmware loading processes. The
> > + * preparation step sets up a shmbridge over the firmware memory before
> > + * TrustZone accesses the firmware memory region for authentication. The
> > + * authentication step verifies the integrity and authenticity of the firmware
> > + * or configuration using secure metadata. Finally, the reset step ensures the
> > + * subsystem starts in a clean and sane state.
> > + *
> > + * Return: 0 on success, negative errno on failure.
> > + */
> > +int qcom_pas_prepare_and_auth_reset(struct qcom_pas_context *ctx)
> > +{
>
> Same here.
Ack.
-Sumit
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Sumit Garg @ 2026-03-23 13:01 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313075948.nbopdkctdcvzlj3f@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 01:29:48PM +0530, Mukesh Ojha wrote:
> On Thu, Mar 12, 2026 at 11:57:43AM +0530, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > Qcom platforms has the legacy of using non-standard SCM calls
> > splintered over the various kernel drivers. These SCM calls aren't
> > compliant with the standard SMC calling conventions which is a
> > prerequisite to enable migration to the FF-A specifications from Arm.
> >
> > OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> > support these non-standard SCM calls. And even for newer architectures
> > with S-EL2 and Hafnium support, QTEE won't be able to support SCM
> > calls either with FF-A requirements coming in. And with both OP-TEE
> > and QTEE drivers well integrated in the TEE subsystem, it makes further
> > sense to reuse the TEE bus client drivers infrastructure.
> >
> > The added benefit of TEE bus infrastructure is that there is support
> > for discoverable/enumerable services. With that client drivers don't
> > have to manually invoke a special SCM call to know the service status.
> >
> > So enable the generic Peripheral Authentication Service (PAS) provided
> > by the firmware. It acts as the common layer with different TZ
> > backends plugged in whether it's an SCM implementation or a proper
> > TEE bus based PAS service implementation.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 8 +
> > drivers/firmware/qcom/Makefile | 1 +
> > drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
> > drivers/firmware/qcom/qcom_pas.h | 53 +++++
> > include/linux/firmware/qcom/qcom_pas.h | 41 ++++
> > 5 files changed, 401 insertions(+)
> > create mode 100644 drivers/firmware/qcom/qcom_pas.c
> > create mode 100644 drivers/firmware/qcom/qcom_pas.h
> > create mode 100644 include/linux/firmware/qcom/qcom_pas.h
> >
<snip>
> > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
> > new file mode 100644
> > index 000000000000..beb1bae55546
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
>
> I know, this is new file but most of the documentation and some of the
> function are rename to reflect pas service.
>
> Should this carry original file copyright ? Not sure..
>
This file only contains the wrapper generic PAS APIs which aren't
re-used from any other file. Carrying copyrights solely based on moving
API documentation in not very clear to me. Any other thoughts?
-Sumit
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Sumit Garg @ 2026-03-23 12:55 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313073121.qb7yq7tcga3sshcr@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 01:01:21PM +0530, Mukesh Ojha wrote:
> On Fri, Mar 13, 2026 at 12:54:50PM +0530, Mukesh Ojha wrote:
> > On Thu, Mar 12, 2026 at 11:57:43AM +0530, Sumit Garg wrote:
> > > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > >
> > > Qcom platforms has the legacy of using non-standard SCM calls
> > > splintered over the various kernel drivers. These SCM calls aren't
> > > compliant with the standard SMC calling conventions which is a
> > > prerequisite to enable migration to the FF-A specifications from Arm.
> > >
> > > OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> > > support these non-standard SCM calls. And even for newer architectures
> > > with S-EL2 and Hafnium support, QTEE won't be able to support SCM
> >
> > using S‑EL2 with Hafnium
> >
> > > calls either with FF-A requirements coming in. And with both OP-TEE
> > > and QTEE drivers well integrated in the TEE subsystem, it makes further
> > > sense to reuse the TEE bus client drivers infrastructure.
> > >
> > > The added benefit of TEE bus infrastructure is that there is support
> > > for discoverable/enumerable services. With that client drivers don't
> > > have to manually invoke a special SCM call to know the service status.
> > >
> > > So enable the generic Peripheral Authentication Service (PAS) provided
> > > by the firmware. It acts as the common layer with different TZ
> > > backends plugged in whether it's an SCM implementation or a proper
> > > TEE bus based PAS service implementation.
> > >
> > > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > > ---
> > > drivers/firmware/qcom/Kconfig | 8 +
> > > drivers/firmware/qcom/Makefile | 1 +
> > > drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
> > > drivers/firmware/qcom/qcom_pas.h | 53 +++++
> > > include/linux/firmware/qcom/qcom_pas.h | 41 ++++
> > > 5 files changed, 401 insertions(+)
> > > create mode 100644 drivers/firmware/qcom/qcom_pas.c
> > > create mode 100644 drivers/firmware/qcom/qcom_pas.h
> > > create mode 100644 include/linux/firmware/qcom/qcom_pas.h
> > >
<snip>
> > > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
> > > new file mode 100644
> > > index 000000000000..beb1bae55546
> > > --- /dev/null
> > > +++ b/drivers/firmware/qcom/qcom_pas.c
> > > @@ -0,0 +1,298 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > > + */
> > > +
> > > +#include <linux/device/devres.h>
> > > +#include <linux/firmware/qcom/qcom_pas.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +
> > > +#include "qcom_pas.h"
> > > +
> > > +struct qcom_pas_ops *ops_ptr;
> >
> > Should this be static ?
> >
> > > +
> > > +/**
> > > + * devm_qcom_pas_context_alloc() - Allocate peripheral authentication service
> > > + * context for a given peripheral
> > > + *
> > > + * PAS context is device-resource managed, so the caller does not need
> > > + * to worry about freeing the context memory.
> > > + *
> > > + * @dev: PAS firmware device
> > > + * @pas_id: peripheral authentication service id
> > > + * @mem_phys: Subsystem reserve memory start address
> > > + * @mem_size: Subsystem reserve memory size
> > > + *
> > > + * Return: The new PAS context, or ERR_PTR() on failure.
> > > + */
> > > +struct qcom_pas_context *devm_qcom_pas_context_alloc(struct device *dev,
> > > + u32 pas_id,
> > > + phys_addr_t mem_phys,
> > > + size_t mem_size)
> > > +{
> > > + struct qcom_pas_context *ctx;
> > > +
> > > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > + if (!ctx)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + ctx->dev = dev;
> > > + ctx->pas_id = pas_id;
> > > + ctx->mem_phys = mem_phys;
> > > + ctx->mem_size = mem_size;
> > > +
> > > + return ctx;
> > > +}
> > > +EXPORT_SYMBOL_GPL(devm_qcom_pas_context_alloc);
> > > +
> > > +/**
> > > + * qcom_pas_init_image() - Initialize peripheral authentication service state
> > > + * machine for a given peripheral, using the metadata
> > > + * @pas_id: peripheral authentication service id
> > > + * @metadata: pointer to memory containing ELF header, program header table
> > > + * and optional blob of data used for authenticating the metadata
> > > + * and the rest of the firmware
> > > + * @size: size of the metadata
> > > + * @ctx: optional pas context
> > > + *
> > > + * Return: 0 on success.
> > > + *
> > > + * Upon successful return, the PAS metadata context (@ctx) will be used to
> > > + * track the metadata allocation, this needs to be released by invoking
> > > + * qcom_pas_metadata_release() by the caller.
> > > + */
> > > +int qcom_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > > + struct qcom_pas_context *ctx)
>
> please, align this with previous line '(' for all the functions.
>
The alignment is fine here, not sure why the plain text replies show
them as not aligned.
-Sumit
^ permalink raw reply
* Re: [PATCH v2 02/15] firmware: qcom: Add a generic PAS service
From: Sumit Garg @ 2026-03-23 12:50 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313072450.sx7vqtvh62nflhff@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 12:54:50PM +0530, Mukesh Ojha wrote:
> On Thu, Mar 12, 2026 at 11:57:43AM +0530, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >
> > Qcom platforms has the legacy of using non-standard SCM calls
> > splintered over the various kernel drivers. These SCM calls aren't
> > compliant with the standard SMC calling conventions which is a
> > prerequisite to enable migration to the FF-A specifications from Arm.
> >
> > OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> > support these non-standard SCM calls. And even for newer architectures
> > with S-EL2 and Hafnium support, QTEE won't be able to support SCM
>
> using S‑EL2 with Hafnium
Ack.
>
> > calls either with FF-A requirements coming in. And with both OP-TEE
> > and QTEE drivers well integrated in the TEE subsystem, it makes further
> > sense to reuse the TEE bus client drivers infrastructure.
> >
> > The added benefit of TEE bus infrastructure is that there is support
> > for discoverable/enumerable services. With that client drivers don't
> > have to manually invoke a special SCM call to know the service status.
> >
> > So enable the generic Peripheral Authentication Service (PAS) provided
> > by the firmware. It acts as the common layer with different TZ
> > backends plugged in whether it's an SCM implementation or a proper
> > TEE bus based PAS service implementation.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > drivers/firmware/qcom/Kconfig | 8 +
> > drivers/firmware/qcom/Makefile | 1 +
> > drivers/firmware/qcom/qcom_pas.c | 298 +++++++++++++++++++++++++
> > drivers/firmware/qcom/qcom_pas.h | 53 +++++
> > include/linux/firmware/qcom/qcom_pas.h | 41 ++++
> > 5 files changed, 401 insertions(+)
> > create mode 100644 drivers/firmware/qcom/qcom_pas.c
> > create mode 100644 drivers/firmware/qcom/qcom_pas.h
> > create mode 100644 include/linux/firmware/qcom/qcom_pas.h
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index b477d54b495a..8653639d06db 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -6,6 +6,14 @@
> >
> > menu "Qualcomm firmware drivers"
> >
> > +config QCOM_PAS
> > + tristate
> > + help
> > + Enable the generic Peripheral Authentication Service (PAS) provided
> > + by the firmware. It acts as the common layer with different TZ
> > + backends plugged in whether it's an SCM implementation or a proper
> > + TEE bus based PAS service implementation.
> > +
> > config QCOM_SCM
> > select QCOM_TZMEM
> > tristate
> > diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
> > index 0be40a1abc13..dc5ab45f906a 100644
> > --- a/drivers/firmware/qcom/Makefile
> > +++ b/drivers/firmware/qcom/Makefile
> > @@ -8,3 +8,4 @@ qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> > obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
> > obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
> > obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
> > +obj-$(CONFIG_QCOM_PAS) += qcom_pas.o
> > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c
> > new file mode 100644
> > index 000000000000..beb1bae55546
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#include <linux/device/devres.h>
> > +#include <linux/firmware/qcom/qcom_pas.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +
> > +#include "qcom_pas.h"
> > +
> > +struct qcom_pas_ops *ops_ptr;
>
> Should this be static ?
It was static earlier in v1. I dropped it based on earlier v1 discussion
with Krzysztof. Let me conclude that discussion on the other thread
again.
>
> > +
> > +/**
> > + * devm_qcom_pas_context_alloc() - Allocate peripheral authentication service
> > + * context for a given peripheral
> > + *
> > + * PAS context is device-resource managed, so the caller does not need
> > + * to worry about freeing the context memory.
> > + *
> > + * @dev: PAS firmware device
> > + * @pas_id: peripheral authentication service id
> > + * @mem_phys: Subsystem reserve memory start address
> > + * @mem_size: Subsystem reserve memory size
> > + *
> > + * Return: The new PAS context, or ERR_PTR() on failure.
> > + */
> > +struct qcom_pas_context *devm_qcom_pas_context_alloc(struct device *dev,
> > + u32 pas_id,
> > + phys_addr_t mem_phys,
> > + size_t mem_size)
> > +{
> > + struct qcom_pas_context *ctx;
> > +
> > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ctx->dev = dev;
> > + ctx->pas_id = pas_id;
> > + ctx->mem_phys = mem_phys;
> > + ctx->mem_size = mem_size;
> > +
> > + return ctx;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_qcom_pas_context_alloc);
> > +
> > +/**
> > + * qcom_pas_init_image() - Initialize peripheral authentication service state
> > + * machine for a given peripheral, using the metadata
> > + * @pas_id: peripheral authentication service id
> > + * @metadata: pointer to memory containing ELF header, program header table
> > + * and optional blob of data used for authenticating the metadata
> > + * and the rest of the firmware
> > + * @size: size of the metadata
> > + * @ctx: optional pas context
> > + *
> > + * Return: 0 on success.
> > + *
> > + * Upon successful return, the PAS metadata context (@ctx) will be used to
> > + * track the metadata allocation, this needs to be released by invoking
> > + * qcom_pas_metadata_release() by the caller.
> > + */
> > +int qcom_pas_init_image(u32 pas_id, const void *metadata, size_t size,
> > + struct qcom_pas_context *ctx)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->init_image(ops_ptr->dev, pas_id,
> > + metadata, size, ctx);
> > +
> > + return -ENODEV;
>
>
> if (!ops_ptr)
> return -ENODEV;
>
> return ops_ptr->init_image(ops_ptr->dev, pas_id, metadata, size, ctx);
>
I think it's a matter of taste here, I usually prefer to check for the
positive scenarios until there is benefit to make an early return.
>
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_init_image);
> > +
> > +/**
> > + * qcom_pas_metadata_release() - release metadata context
> > + * @ctx: pas context
> > + */
> > +void qcom_pas_metadata_release(struct qcom_pas_context *ctx)
> > +{
> > + if (!ctx || !ctx->ptr)
> > + return;
> > +
> > + if (ops_ptr)
> > + ops_ptr->metadata_release(ops_ptr->dev, ctx);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_metadata_release);
> > +
> > +/**
> > + * qcom_pas_mem_setup() - Prepare the memory related to a given peripheral
> > + * for firmware loading
> > + * @pas_id: peripheral authentication service id
> > + * @addr: start address of memory area to prepare
> > + * @size: size of the memory area to prepare
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->mem_setup(ops_ptr->dev, pas_id, addr, size);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_mem_setup);
> > +
> > +/**
> > + * qcom_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> > + * for a given peripheral.
> > + *
> > + * Qualcomm remote processor may rely on both static and dynamic resources for
> > + * its functionality. Static resources typically refer to memory-mapped
> > + * addresses required by the subsystem and are often embedded within the
> > + * firmware binary and dynamic resources, such as shared memory in DDR etc.,
> > + * are determined at runtime during the boot process.
> > + *
> > + * On Qualcomm Technologies devices, it's possible that static resources are
> > + * not embedded in the firmware binary and instead are provided by TrustZone.
> > + * However, dynamic resources are always expected to come from TrustZone. This
> > + * indicates that for Qualcomm devices, all resources (static and dynamic) will
> > + * be provided by TrustZone PAS service.
> > + *
> > + * If the remote processor firmware binary does contain static resources, they
> > + * should be passed in input_rt. These will be forwarded to TrustZone for
> > + * authentication. TrustZone will then append the dynamic resources and return
> > + * the complete resource table in output_rt_tzm.
> > + *
> > + * If the remote processor firmware binary does not include a resource table,
> > + * the caller of this function should set input_rt as NULL and input_rt_size
> > + * as zero respectively.
> > + *
> > + * More about documentation on resource table data structures can be found in
> > + * include/linux/remoteproc.h
> > + *
> > + * @ctx: PAS context
> > + * @pas_id: peripheral authentication service id
> > + * @input_rt: resource table buffer which is present in firmware binary
> > + * @input_rt_size: size of the resource table present in firmware binary
> > + * @output_rt_size: TrustZone expects caller should pass worst case size for
> > + * the output_rt_tzm.
> > + *
> > + * Return:
> > + * On success, returns a pointer to the allocated buffer containing the final
> > + * resource table and output_rt_size will have actual resource table size from
> > + * TrustZone. The caller is responsible for freeing the buffer. On failure,
> > + * returns ERR_PTR(-errno).
> > + */
> > +struct resource_table *qcom_pas_get_rsc_table(struct qcom_pas_context *ctx,
> > + void *input_rt,
> > + size_t input_rt_size,
> > + size_t *output_rt_size)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->get_rsc_table(ops_ptr->dev, ctx, input_rt,
> > + input_rt_size, output_rt_size);
> > +
> > + return ERR_PTR(-ENODEV);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_get_rsc_table);
> > +
> > +/**
> > + * qcom_pas_auth_and_reset() - Authenticate the given peripheral firmware
> > + * and reset the remote processor
> > + * @pas_id: peripheral authentication service id
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_auth_and_reset(u32 pas_id)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_auth_and_reset);
> > +
> > +/**
> > + * qcom_pas_prepare_and_auth_reset() - Prepare, authenticate, and reset the
> > + * remote processor
> > + *
> > + * @ctx: Context saved during call to qcom_scm_pas_context_init()
> > + *
> > + * This function performs the necessary steps to prepare a PAS subsystem,
> > + * authenticate it using the provided metadata, and initiate a reset sequence.
> > + *
> > + * It should be used when Linux is in control setting up the IOMMU hardware
> > + * for remote subsystem during secure firmware loading processes. The
> > + * preparation step sets up a shmbridge over the firmware memory before
> > + * TrustZone accesses the firmware memory region for authentication. The
> > + * authentication step verifies the integrity and authenticity of the firmware
> > + * or configuration using secure metadata. Finally, the reset step ensures the
> > + * subsystem starts in a clean and sane state.
> > + *
> > + * Return: 0 on success, negative errno on failure.
> > + */
> > +int qcom_pas_prepare_and_auth_reset(struct qcom_pas_context *ctx)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->prepare_and_auth_reset(ops_ptr->dev, ctx);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_prepare_and_auth_reset);
> > +
> > +/**
> > + * qcom_pas_set_remote_state() - Set the remote processor state
> > + * @state: peripheral state
> > + * @pas_id: peripheral authentication service id
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_set_remote_state(u32 state, u32 pas_id)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->set_remote_state(ops_ptr->dev, state, pas_id);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_set_remote_state);
> > +
> > +/**
> > + * qcom_pas_shutdown() - Shut down the remote processor
> > + * @pas_id: peripheral authentication service id
> > + *
> > + * Return: 0 on success.
> > + */
> > +int qcom_pas_shutdown(u32 pas_id)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->shutdown(ops_ptr->dev, pas_id);
> > +
> > + return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_shutdown);
> > +
> > +/**
> > + * qcom_pas_supported() - Check if the peripheral authentication service is
> > + * available for the given peripheral
> > + * @pas_id: peripheral authentication service id
> > + *
> > + * Return: true if PAS is supported for this peripheral, otherwise false.
> > + */
> > +bool qcom_pas_supported(u32 pas_id)
> > +{
> > + if (ops_ptr)
> > + return ops_ptr->supported(ops_ptr->dev, pas_id);
> > +
> > + return false;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_supported);
> > +
> > +/**
> > + * qcom_pas_is_available() - Check for PAS service
> > + *
>
> Name of the function is self sufficient, we can avoid for one liner
> documentation.
The documentation here is for the sake of completeness for all the APIs.
Sure, I can drop it if it doesn't add any value.
>
> > + * Return: true on success.
> > + */
> > +bool qcom_pas_is_available(void)
> > +{
> > + /*
> > + * The barrier for ops_ptr is intended to synchronize the data stores
> > + * for the ops data structure when client drivers are in parallel
> > + * checking for PAS service availability.
> > + *
> > + * Once the PAS backend becomes available, it is allowed for multiple
> > + * threads to enter TZ for parallel bringup of co-processors during
> > + * boot.
> > + */
> > + return !!smp_load_acquire(&ops_ptr);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_is_available);
> > +
> > +/**
> > + * qcom_pas_ops_register() - Register PAS service ops
> > + * @ops: PAS service ops pointer
> > + */
>
> same here..
>
> > +void qcom_pas_ops_register(struct qcom_pas_ops *ops)
> > +{
> > + if (!qcom_pas_is_available())
> > + /* Paired with smp_load_acquire() in qcom_pas_is_available() */
> > + smp_store_release(&ops_ptr, ops);
> > + else
> > + pr_err("qcom_pas: ops already registered\n");
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_ops_register);
> > +
> > +/**
> > + * qcom_pas_ops_unregister() - Unregister PAS service ops
> > + */
>
> same here to avoid verbose..
>
> > +void qcom_pas_ops_unregister(void)
> > +{
> > + /* Paired with smp_load_acquire() in qcom_pas_is_available() */
> > + smp_store_release(&ops_ptr, NULL);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_pas_ops_unregister);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("Qualcomm common TZ PAS driver");
> > diff --git a/drivers/firmware/qcom/qcom_pas.h b/drivers/firmware/qcom/qcom_pas.h
> > new file mode 100644
> > index 000000000000..4ebed22178f8
> > --- /dev/null
> > +++ b/drivers/firmware/qcom/qcom_pas.h
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +#ifndef __QCOM_PAS_INT_H
> > +#define __QCOM_PAS_INT_H
> > +
> > +struct device;
> > +
> > +/**
> > + * struct qcom_pas_ops - Qcom Peripheral Authentication Service (PAS) ops
> > + * @drv_name: PAS driver name.
> > + * @dev: PAS device pointer.
> > + * @supported: Peripheral supported callback.
> > + * @init_image: Peripheral image initialization callback.
> > + * @mem_setup: Peripheral memory setup callback.
> > + * @get_rsc_table: Peripheral get resource table callback.
> > + * @prepare_and_auth_reset: Peripheral prepare firmware authentication and
> > + * reset callback.
> > + * @auth_and_reset: Peripheral firmware authentication and reset
> > + * callback.
> > + * @set_remote_state: Peripheral set remote state callback.
> > + * @shutdown: Peripheral shutdown callback.
> > + * @metadata_release: Image metadata release callback.
> > + */
> > +struct qcom_pas_ops {
> > + const char *drv_name;
> > + struct device *dev;
> > + bool (*supported)(struct device *dev, u32 pas_id);
> > + int (*init_image)(struct device *dev, u32 pas_id,
> > + const void *metadata, size_t size,
> > + struct qcom_pas_context *ctx);
> > + int (*mem_setup)(struct device *dev, u32 pas_id,
> > + phys_addr_t addr, phys_addr_t size);
> > + void *(*get_rsc_table)(struct device *dev,
> > + struct qcom_pas_context *ctx,
> > + void *input_rt,
> > + size_t input_rt_size,
> > + size_t *output_rt_size);
> > + int (*prepare_and_auth_reset)(struct device *dev,
> > + struct qcom_pas_context *ctx);
> > + int (*auth_and_reset)(struct device *dev, u32 pas_id);
> > + int (*set_remote_state)(struct device *dev, u32 state, u32 pas_id);
> > + int (*shutdown)(struct device *dev, u32 pas_id);
> > + void (*metadata_release)(struct device *dev,
> > + struct qcom_pas_context *ctx);
>
> I think, some of them can be unwrapped to look cleaner..
Not sure if I understand what you meant by unwrap here, can you
ellaborate a bit?
>
> > +};
> > +
> > +void qcom_pas_ops_register(struct qcom_pas_ops *ops);
> > +void qcom_pas_ops_unregister(void);
> > +
> > +#endif /* __QCOM_PAS_INT_H */
> > diff --git a/include/linux/firmware/qcom/qcom_pas.h b/include/linux/firmware/qcom/qcom_pas.h
> > new file mode 100644
> > index 000000000000..ef7328ecfa47
> > --- /dev/null
> > +++ b/include/linux/firmware/qcom/qcom_pas.h
> > @@ -0,0 +1,41 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>
>
> Should this not carry all copyright coming from qcom_scm.h
These are all new wrapper APIs for the generic PAS service. However, the
API design is influenced by the SCM APIs only since we don't want to
break client drivers API contract in this patch-set. I will carry those
copyrights here.
-Sumit
^ permalink raw reply
* Re: [PATCH ath-next v4] wifi: ath12k: avoid dynamic alloc when parsing wmi tb
From: Rameshkumar Sundaram @ 2026-03-23 12:40 UTC (permalink / raw)
To: Jeff Johnson, Nicolas Escande, ath12k; +Cc: linux-wireless
In-Reply-To: <40756be1-6a9a-4821-8c90-34f37db01e8b@oss.qualcomm.com>
On 3/19/2026 9:29 PM, Jeff Johnson wrote:
> On 3/19/2026 7:35 AM, Nicolas Escande wrote:
>> On Thu Mar 19, 2026 at 12:08 PM CET, Rameshkumar Sundaram wrote:
>>>
>>> Since CONFIG_ATH12K is tristate, a built-in boot can continue past a
>>> failed ath12k_init() and still run ath12k_wifi7_init().
>>>
>> I genuinely thought the kernel prevented this. I was wrong.
>>
>>> Please ensure that later initialization path is guarded against
>>> allocation failure.
>>>
>> I can add a flag like so to be able to check from ath12k_wifi7_init() if the
>> init finished ok. Something in the lines of
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
>> index 6c034071cc6d..742fb33f41ff 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>> @@ -34,6 +34,9 @@ module_param_named(ftm_mode, ath12k_ftm_mode, bool, 0444);
>> MODULE_PARM_DESC(ftm_mode, "Boots up in factory test mode");
>> EXPORT_SYMBOL(ath12k_ftm_mode);
>>
>> +bool ath12k_init_ok = false;
>> +EXPORT_SYMBOL(ath12k_init_ok);
>> +
>> /* protected with ath12k_hw_group_mutex */
>> static struct list_head ath12k_hw_group_list = LIST_HEAD_INIT(ath12k_hw_group_list);
>>
>> @@ -2323,7 +2326,14 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>>
>> static int ath12k_init(void)
>> {
>> - return ath12k_wmi_alloc();
>> + int ret;
>> +
>> + ret = ath12k_wmi_alloc();
>> + if (ret)
>> + return -ENOMEM;
>> +
>> + ath12k_init_ok = true;
>> + return 0;
>> }
>>
>> static void ath12k_exit(void)
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 59c193b24764..f35571b1a541 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -101,6 +101,8 @@ enum ath12k_crypt_mode {
>> ATH12K_CRYPT_MODE_SW,
>> };
>>
>> +extern bool ath12k_init_ok;
>> +
>> static inline enum wme_ac ath12k_tid_to_ac(u32 tid)
>> {
>> return (((tid == 0) || (tid == 3)) ? WME_AC_BE :
>> diff --git a/drivers/net/wireless/ath/ath12k/wifi7/core.c b/drivers/net/wireless/ath/ath12k/wifi7/core.c
>> index a02c57acf137..542ec10fabf1 100644
>> --- a/drivers/net/wireless/ath/ath12k/wifi7/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/wifi7/core.c
>> @@ -38,6 +38,9 @@ void ath12k_wifi7_arch_deinit(struct ath12k_base *ab)
>>
>> static int ath12k_wifi7_init(void)
>> {
>> + if (!ath12k_init_ok)
>> + return -ENOTSUPP;
>> +
>> ahb_err = ath12k_wifi7_ahb_init();
>> if (ahb_err)
>> pr_warn("Failed to initialize ath12k Wi-Fi 7 AHB device: %d\n",
>>
>>
>> I don't like it much but it is easy enough.
>> But I don't know if there is a more idiomatic way of doing things
>
> I'd prefer to expose a function rather than a global variable.
> In other words keep the flag static, and expose a function that returns the
> value of the flag, i.e.:
>
> bool ath12k_core_initialized(void)
> {
> return ath12k_init_ok;
> }
> EXPORT_SYMBOL(ath12k_core_initialized);
>
>>
>>> Or may be have this allocated on first device probe and free it on last
>>> device deinit ?
>>
>> That seems even more involved. It would be easier to go back to the previous
>> version and simply, alloc it once per ath12k_base
>>
>> What do you guys think ?
>>
>
> Going back to that may be the better solution. It isn't nice that this current
> solution may allocate memory when the driver isn't actually used. But I'll let
> others on the team weigh in as well.
>
Yeah, allocating once per ath12k_base is definitely the simpler
ownership model.
I was only wondering whether sharing it across devices might be worth a
look, since this is per-CPU scratch space and the table itself is fairly
large.
--
Ramesh
^ permalink raw reply
* Re: [PATCH v2 01/15] arm64: dts: qcom: kodiak: Add EL2 overlay
From: Sumit Garg @ 2026-03-23 12:35 UTC (permalink / raw)
To: Mukesh Ojha
Cc: 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, pavan.kondeti, jorge.ramirez,
tonyh, vignesh.viswanathan, srinivas.kandagatla, amirreza.zarrabi,
jens.wiklander, op-tee, apurupa, skare, linux-kernel, Sumit Garg
In-Reply-To: <20260313060451.hswg6snnnexchmzs@hu-mojha-hyd.qualcomm.com>
On Fri, Mar 13, 2026 at 11:34:51AM +0530, Mukesh Ojha wrote:
> On Thu, Mar 12, 2026 at 11:57:42AM +0530, Sumit Garg wrote:
> > From: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> >
> > All the existing variants Kodiak boards are using Gunyah hypervisor
> > which means that, so far, Linux-based OS could only boot in EL1 on those
> > devices. However, it is possible for us to boot Linux at EL2 on these
> > devices [1].
> >
> > When running under Gunyah, the remote processor firmware IOMMU
> > streams are controlled by Gunyah. However, without Gunyah, the IOMMU is
> > managed by the consumer of this DeviceTree. Therefore, describe the
> > firmware streams for each remote processor.
> >
> > Add a EL2-specific DT overlay and apply it to Kodiak IOT variant
> > devices to create -el2.dtb for each of them alongside "normal" dtb.
> >
> > [1]
> > https://docs.qualcomm.com/bundle/publicresource/topics/80-70020-4/boot-developer-touchpoints.html#uefi
> >
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > [SG: watchdog fixup]
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/Makefile | 2 ++
> > arch/arm64/boot/dts/qcom/kodiak-el2.dtso | 35 ++++++++++++++++++++++++
> > 2 files changed, 37 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/qcom/kodiak-el2.dtso
> >
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index f80b5d9cf1e8..09a7f943190e 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -139,6 +139,8 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
> > dtb-$(CONFIG_ARCH_QCOM) += qcs615-ride.dtb
> > dtb-$(CONFIG_ARCH_QCOM) += qcs6490-radxa-dragon-q6a.dtb
> > dtb-$(CONFIG_ARCH_QCOM) += qcs6490-rb3gen2.dtb
> > +qcs6490-rb3gen2-el2-dtbs := qcs6490-rb3gen2.dtb kodiak-el2.dtbo
> > +dtb-$(CONFIG_ARCH_QCOM) += qcs6490-rb3gen2-el2.dtb
>
> We may need to add for couple of more variants..
Sure, those can be follow up patches if Bjorn is happy to pick this one
independently.
>
> >
> > qcs6490-rb3gen2-vision-mezzanine-dtbs := qcs6490-rb3gen2.dtb qcs6490-rb3gen2-vision-mezzanine.dtbo
> > qcs6490-rb3gen2-industrial-mezzanine-dtbs := qcs6490-rb3gen2.dtb qcs6490-rb3gen2-industrial-mezzanine.dtbo
> > diff --git a/arch/arm64/boot/dts/qcom/kodiak-el2.dtso b/arch/arm64/boot/dts/qcom/kodiak-el2.dtso
> > new file mode 100644
> > index 000000000000..0b3a69a0d765
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/kodiak-el2.dtso
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + *
> > + * Kodiak specific modifications required to boot in EL2.
> > + */
> > +
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&gpu_zap_shader {
> > + status = "disabled";
> > +};
> > +
> > +&remoteproc_adsp {
> > + iommus = <&apps_smmu 0x1800 0x0>;
> > +};
> > +
> > +&remoteproc_cdsp {
> > + iommus = <&apps_smmu 0x11a0 0x0400>;
> > +};
> > +
> > +&remoteproc_wpss {
> > + iommus = <&apps_smmu 0x1c03 0x1>,
> > + <&apps_smmu 0x1c83 0x1>;
> > +};
> > +
> > +&venus {
> > + status = "disabled";
> > +};
> > +
> > +&watchdog {
> > + status = "okay";
> > +};
>
>
> rb3gen2 has modem as well, did we test that as well ?
OP-TEE don't have access to modem, it's locked down in XBL-SEC.
-Sumit
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH net-next v2 03/13] net: introduce ndo_set_rx_mode_async and dev_rx_mode_work
From: Loktionov, Aleksandr @ 2026-03-23 12:09 UTC (permalink / raw)
To: Stanislav Fomichev, netdev@vger.kernel.org
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, horms@kernel.org, corbet@lwn.net,
skhan@linuxfoundation.org, andrew+netdev@lunn.ch,
michael.chan@broadcom.com, pavan.chebbi@broadcom.com,
Nguyen, Anthony L, Kitszel, Przemyslaw, saeedm@nvidia.com,
tariqt@nvidia.com, mbloch@nvidia.com, alexanderduyck@fb.com,
kernel-team@meta.com, johannes@sipsolutions.net,
sd@queasysnail.net, jianbol@nvidia.com, dtatulea@nvidia.com,
mohsin.bashr@gmail.com, Keller, Jacob E, willemb@google.com,
skhawaja@google.com, bestswngs@gmail.com,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
linux-wireless@vger.kernel.org, linux-kselftest@vger.kernel.org,
leon@kernel.org
In-Reply-To: <20260318150305.123900-4-sdf@fomichev.me>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Stanislav Fomichev
> Sent: Wednesday, March 18, 2026 4:03 PM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; horms@kernel.org; corbet@lwn.net;
> skhan@linuxfoundation.org; andrew+netdev@lunn.ch;
> michael.chan@broadcom.com; pavan.chebbi@broadcom.com; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; saeedm@nvidia.com; tariqt@nvidia.com;
> mbloch@nvidia.com; alexanderduyck@fb.com; kernel-team@meta.com;
> johannes@sipsolutions.net; sd@queasysnail.net; jianbol@nvidia.com;
> dtatulea@nvidia.com; sdf@fomichev.me; mohsin.bashr@gmail.com; Keller,
> Jacob E <jacob.e.keller@intel.com>; willemb@google.com;
> skhawaja@google.com; bestswngs@gmail.com; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> rdma@vger.kernel.org; linux-wireless@vger.kernel.org; linux-
> kselftest@vger.kernel.org; leon@kernel.org
> Subject: [Intel-wired-lan] [PATCH net-next v2 03/13] net: introduce
> ndo_set_rx_mode_async and dev_rx_mode_work
>
> Add ndo_set_rx_mode_async callback that drivers can implement instead
> of the legacy ndo_set_rx_mode. The legacy callback runs under the
> netif_addr_lock spinlock with BHs disabled, preventing drivers from
> sleeping. The async variant runs from a work queue with rtnl_lock and
> netdev_lock_ops held, in fully sleepable context.
>
> When __dev_set_rx_mode() sees ndo_set_rx_mode_async, it schedules
> dev_rx_mode_work instead of calling the driver inline. The work
> function takes two snapshots of each address list (uc/mc) under the
> addr_lock, then drops the lock and calls the driver with the work
> copies. After the driver returns, it reconciles the snapshots back to
> the real lists under the lock.
>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> Documentation/networking/netdevices.rst | 8 +++
> include/linux/netdevice.h | 20 ++++++
> net/core/dev.c | 94 +++++++++++++++++++++++-
> -
> 3 files changed, 115 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/networking/netdevices.rst
> b/Documentation/networking/netdevices.rst
> index 35704d115312..dc83d78d3b27 100644
> --- a/Documentation/networking/netdevices.rst
> +++ b/Documentation/networking/netdevices.rst
> @@ -289,6 +289,14 @@ struct net_device synchronization rules
> ndo_set_rx_mode:
> Synchronization: netif_addr_lock spinlock.
> Context: BHs disabled
...
>
> -/*
> - * Upload unicast and multicast address lists to device and
> - * configure RX filtering. When the device doesn't support unicast
> - * filtering it is put in promiscuous mode while unicast addresses
> - * are present.
> +static void dev_rx_mode_work(struct work_struct *work) {
> + struct net_device *dev = container_of(work, struct net_device,
> + rx_mode_work);
> + struct netdev_hw_addr_list uc_snap, mc_snap, uc_ref, mc_ref;
> + const struct net_device_ops *ops = dev->netdev_ops;
> + int err;
> +
> + __hw_addr_init(&uc_snap);
> + __hw_addr_init(&mc_snap);
> + __hw_addr_init(&uc_ref);
> + __hw_addr_init(&mc_ref);
> +
> + rtnl_lock();
> + netdev_lock_ops(dev);
> +
> + if (!netif_up_and_present(dev))
> + goto out;
> +
> + if (ops->ndo_set_rx_mode_async) {
> + netif_addr_lock_bh(dev);
> +
> + err = __hw_addr_list_snapshot(&uc_snap, &dev->uc,
> + dev->addr_len);
> + if (!err)
> + err = __hw_addr_list_snapshot(&uc_ref, &dev->uc,
> + dev->addr_len);
> + if (!err)
> + err = __hw_addr_list_snapshot(&mc_snap, &dev->mc,
> + dev->addr_len);
> + if (!err)
> + err = __hw_addr_list_snapshot(&mc_ref, &dev->mc,
> + dev->addr_len);
> + netif_addr_unlock_bh(dev);
> +
> + if (err) {
> + __hw_addr_flush(&uc_snap);
> + __hw_addr_flush(&uc_ref);
> + __hw_addr_flush(&mc_snap);
Shouldn't here go cleanup for symmetry?
__hw_addr_flush(&mc_ref);
> + goto out;
> + }
> +
> + ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
> +
> + netif_addr_lock_bh(dev);
> + __hw_addr_list_reconcile(&dev->uc, &uc_snap,
> + &uc_ref, dev->addr_len);
> + __hw_addr_list_reconcile(&dev->mc, &mc_snap,
> + &mc_ref, dev->addr_len);
> + netif_addr_unlock_bh(dev);
> + }
> +
> +out:
> + netdev_unlock_ops(dev);
> + rtnl_unlock();
> +}
...
> --
> 2.53.0
^ 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