Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v9 4/9] dt-bindings: display: imx: Add i.MX94 DCIF
From: Rob Herring (Arm) @ 2026-06-15 21:04 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Conor Dooley, Simona Vetter, Fabio Estevam, David Airlie,
	Luca Ceresoli, linux-arm-kernel, dri-devel, imx, linux-kernel,
	Krzysztof Kozlowski, Maxime Ripard, Maarten Lankhorst,
	Thomas Zimmermann, Pengutronix Kernel Team, devicetree, Frank Li,
	Philipp Zabel, linux-clk, Sascha Hauer, Ying Liu
In-Reply-To: <20260612-dcif-upstreaming-v9-4-8d0ff89aa3c5@oss.nxp.com>


On Fri, 12 Jun 2026 14:58:35 +0300, Laurentiu Palcu wrote:
> DCIF is the i.MX94 Display Controller Interface which is used to
> drive a TFT LCD panel or connects to a display interface depending
> on the chip configuration.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/display/imx/nxp,imx94-dcif.yaml       | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>



^ permalink raw reply

* Re: [PATCH] net: correcting section tags for .init and .exit data/functions
From: Rong Xu @ 2026-06-15 21:26 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Neal Cardwell, Kuniyuki Iwashima, Willem de Bruijn,
	David Ahern, Ido Schimmel, Andreas Färber,
	Manivannan Sadhasivam, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Maciej Żenczykowski, Yue Haibing, Jeff Layton,
	Kees Cook, Fernando Fernandez Mancera, Gustavo A. R. Silva,
	Sabrina Dubroca, Masahiro Yamada, Nicolas Schier, netdev,
	linux-kernel, linux-arm-kernel, linux-actions, llvm,
	kernel test robot
In-Reply-To: <20260613170143.GB3152432@ax162>

After studying the warnings further, I believe this is an incorrect fix.

Normally, *_net_ops structures are intended to live in .data (permanent memory),
even though they point to .init.text. Modpost is handling this by using
hardcoded whitelists to allow these specific exceptions. However, this
whitelist is
failing in ThinLTO because ThinLTO renames *_net_ops to *_net_ops.llvm.*.

I am withdrawing this patch and will send a fix to modpost instead.

Thanks,

-Rong


^ permalink raw reply

* Re: [PATCH v8 00/10] clk: realtek: Add RTD1625 clock support
From: Brian Masney @ 2026-06-15 21:28 UTC (permalink / raw)
  To: Yu-Chun Lin
  Cc: mturquette, sboyd, robh, krzk+dt, conor+dt, p.zabel, cylee12,
	afaerber, jyanchou, devicetree, linux-clk, linux-kernel,
	linux-arm-kernel, linux-realtek-soc, james.tai, cy.huang,
	stanley_chang
In-Reply-To: <20260610080824.255063-1-eleanor.lin@realtek.com>

Hi Yu-Chun,

On Wed, Jun 10, 2026 at 04:08:14PM +0800, Yu-Chun Lin wrote:
> Hello,
> 
> This patch series adds clock support for Realtek's RTD1625 platform.
> The series includes:
> 1. Device Tree: Add clock controller nodes.
> 2. Infrastructure: reset controller, basic clocks, PLLs, gate clocks, mux
> clocks, and MMC-tuned PLLs.
> 3. Platform drivers: two clock controller drivers for RTD1625-CRT and
> RTD1625-ISO.
> 
> Best regards,
> Yu-Chun Lin

Sashiko has some legitimate feedback about this patch set:

https://sashiko.dev/#/patchset/20260610080824.255063-1-eleanor.lin%40realtek.com

Can you go through that and post a new version? I'll review the next
version manually in more detail.

Brian



^ permalink raw reply

* Re: [PATCH v5 net-next 0/9] net: dsa: netc: add bridge mode support
From: patchwork-bot+netdevbpf @ 2026-06-15 21:50 UTC (permalink / raw)
  To: Wei Fang
  Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
	davem, edumazet, kuba, pabeni, chleroy, andrew, olteanv, linux,
	wei.fang, imx, netdev, linux-kernel, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20260611021458.2629145-1-wei.fang@oss.nxp.com>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 11 Jun 2026 10:14:49 +0800 you wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> This series adds bridge mode support to the NETC DSA switch driver,
> covering both VLAN-aware and VLAN-unaware operation.
> 
> The NETC switch manages forwarding through a set of hardware tables
> accessed via NTMP: the FDB table (FDBT), VLAN filter table (VFT), egress
> treatment table (ETT), and egress count table (ECT). The series extends
> the NTMP layer with the operations required for bridging, then builds the
> DSA bridge callbacks on top.
> 
> [...]

Here is the summary with links:
  - [v5,net-next,1/9] net: enetc: add interfaces to manage dynamic FDB entries
    https://git.kernel.org/netdev/net-next/c/ca394837dfdd
  - [v5,net-next,2/9] net: enetc: add "Update" and "Delete" operations to VLAN filter table
    https://git.kernel.org/netdev/net-next/c/c52b6702a948
  - [v5,net-next,3/9] net: enetc: add interfaces to manage egress treatment table
    https://git.kernel.org/netdev/net-next/c/3cc291a35939
  - [v5,net-next,4/9] net: enetc: add "Update" operation to the egress count table
    https://git.kernel.org/netdev/net-next/c/d51f238a154a
  - [v5,net-next,5/9] net: dsa: netc: initialize the group bitmap of ETT and ECT
    https://git.kernel.org/netdev/net-next/c/1a58ae73dd74
  - [v5,net-next,6/9] net: enetc: add helpers to set/clear table bitmap
    https://git.kernel.org/netdev/net-next/c/8469b17310d1
  - [v5,net-next,7/9] net: dsa: netc: add VLAN filter table and egress treatment management
    https://git.kernel.org/netdev/net-next/c/84b4a3b30abd
  - [v5,net-next,8/9] net: dsa: netc: add bridge mode support
    https://git.kernel.org/netdev/net-next/c/751aa5a5d593
  - [v5,net-next,9/9] net: dsa: netc: implement dynamic FDB entry ageing
    https://git.kernel.org/netdev/net-next/c/05b5ee610fbb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply

* Re: [PATCH v4 06/31] dt-bindings: firmware: arm,scmi: Add support for telemetry protocol
From: Rob Herring (Arm) @ 2026-06-15 22:14 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: linux-doc, Conor Dooley, puranjay, usama.arif, philip.radford,
	devicetree, souvik.chakravarty, linux-kernel, jic23, elif.topuz,
	lukasz.luba, sudeep.holla, leitao, vincent.guittot, james.quinlan,
	kernel-team, linux-arm-kernel, kas, arm-scmi, peng.fan,
	linux-fsdevel, michal.simek, brauner, etienne.carriere, d-gole,
	Krzysztof Kozlowski, f.fainelli
In-Reply-To: <20260612223802.1337232-7-cristian.marussi@arm.com>


On Fri, 12 Jun 2026 23:37:36 +0100, Cristian Marussi wrote:
> Add new SCMI v4.0 Telemetry protocol bindings definitions.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
>  - changed protocol number to lowercase 1b
>  - fixed misplaced block for protocol 0x1b
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>



^ permalink raw reply

* Re: [PATCH net v2] net: airoha: Fix skb->priority underflow in
From: Joe Damato @ 2026-06-15 22:34 UTC (permalink / raw)
  To: Wayen Yan; +Cc: netdev, lorenzo, horms, linux-arm-kernel, linux-mediatek
In-Reply-To: <6a2ff493.5934d26d.389ef4.d16d@mx.google.com>

On Mon, Jun 15, 2026 at 08:48:13PM +0800, Wayen Yan wrote:
> From b894fc031e307f1b6756ea9fcac98e82e23815e1 Mon Sep 17 00:00:00 2001
> From: "Wayen.Yan" <win847@gmail.com>
> Date: Sun, 14 Jun 2026 07:30:54 +0800
> Subject: [PATCH net v2] net: airoha: Fix skb->priority underflow in
>  airoha_dev_select_queue()
> 
> In airoha_dev_select_queue(), the expression:
> 
>   queue = (skb->priority - 1) % AIROHA_NUM_QOS_QUEUES;
> 
> implicitly converts to unsigned arithmetic: when skb->priority is 0
> (the default for unclassified traffic), (0u - 1u) wraps to UINT_MAX,
> and UINT_MAX % 8 = 7, routing default best-effort packets to the
> highest-priority QoS queue. This causes QoS inversion where the
> majority of traffic on a PON gateway starves actual high-priority
> flows (VoIP, gaming, etc.).
> 
> Fix by guarding the subtraction: when priority is 0, map to queue 0
> (lowest priority), otherwise apply the original (priority - 1) % 8
> mapping.
> 
> Fixes: 2b288b81560b ("net: airoha: Introduce ndo_select_queue callback")
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: Wayen <win847@gmail.com>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hm, I tried to apply this patch to my tree just to see what would happen with
the duplicated SMTP headers and it looks like they just get added to the
commit message.

I think you might need to re-send it (but you'll have to wait 24 hours between
re-sends).

The code looks fine to me, though, so feel free to include my tag:

Reviewed-by: Joe Damato <joe@dama.to>


^ permalink raw reply

* Re: [PATCH] net: airoha: Fix always-true condition in PPE1 queue reservation loop
From: patchwork-bot+netdevbpf @ 2026-06-15 22:50 UTC (permalink / raw)
  To: Wayen Yan
  Cc: netdev, lorenzo, horms, pabeni, kuba, edumazet, andrew+netdev,
	angelogioacchino.delregno, matthias.bgg, linux-arm-kernel,
	linux-mediatek
In-Reply-To: <6a2ca3de.ad59c0a6.147df9.2ac1@mx.google.com>

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 13 Jun 2026 08:23:12 +0800 you wrote:
> In airoha_fe_pse_ports_init(), the inner condition for PPE1 queue
> reservation is identical to the for-loop bound, making it always true
> and the else branch dead code:
> 
>   for (q = 0; q < pse_port_num_queues[FE_PSE_PORT_PPE1]; q++) {
>       if (q < pse_port_num_queues[FE_PSE_PORT_PPE1])  /* always true */
>           set RSV_PAGES;
>       else
>           set 0;  /* unreachable */
>   }
> 
> [...]

Here is the summary with links:
  - net: airoha: Fix always-true condition in PPE1 queue reservation loop
    https://git.kernel.org/netdev/net/c/c66f8511a810

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply

* [PATCH] Revert "arm64: dts: rockchip: Further describe the WiFi for the Pinephone Pro"
From: Oren Klopfer @ 2026-06-15 22:50 UTC (permalink / raw)
  To: oklopfer37
  Cc: linux-arm-kernel, linux-rockchip, linux-kernel, Heiko Stuebner,
	Peter Robinson, Thorsten Leemhuis, stable

This reverts commit 096bd8c679185f898cae9933c6a68650fa26ea4f.

Just as with the Pinebook Pro, there are multiple chipset variants for the Pinephone Pro, and multiple firmware binaries for different distributions. The change causes issues with some of these combinations, and reverting it resolves the issues. See the Closes below for the full report.

Similarly with the Pinebook Pro adjustment, the original commit only indicates "further description" and not indicative of fixing any existing issues, so reverting should not kick any back up.

Fixes: 096bd8c67918 ("arm64: dts: rockchip: Further describe the WiFi for the Pinephone Pro")
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Peter Robinson <pbrobinson@gmail.com>
Cc: Thorsten Leemhuis <regressions@leemhuis.info>
Cc: stable@vger.kernel.org
Closes: https://lore.kernel.org/r/20260607225901.64019-1-oklopfer37@gmail.com/
Signed-off-by: Oren Klopfer <oklopfer37@gmail.com>
---
 .../boot/dts/rockchip/rk3399-pinephone-pro.dts | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
index 8d26bd9b7500..d46cdfe3f784 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -734,12 +734,6 @@ light_int_l: light-int-l {
         };
     };
 
-    wifi {
-        wifi_host_wake_l: wifi-host-wake-l {
-            rockchip,pins = <4 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>;
-        };
-    };
-
     wireless-bluetooth {
         bt_wake_pin: bt-wake-pin {
             rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
@@ -766,19 +760,7 @@ &sdio0 {
     pinctrl-names = "default";
     pinctrl-0 = <&sdio0_bus4 &sdio0_cmd &sdio0_clk>;
     sd-uhs-sdr104;
-    #address-cells = <1>;
-    #size-cells = <0>;
     status = "okay";
-
-    brcmf: wifi@1 {
-        compatible = "brcm,bcm4329-fmac";
-        reg = <1>;
-        interrupt-parent = <&gpio4>;
-        interrupts = <RK_PD0 IRQ_TYPE_LEVEL_HIGH>;
-        interrupt-names = "host-wake";
-        pinctrl-names = "default";
-        pinctrl-0 = <&wifi_host_wake_l>;
-    };
 };
 
 &pwm0 {
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH] net: airoha: Fix typos in comments and Kconfig
From: patchwork-bot+netdevbpf @ 2026-06-15 22:50 UTC (permalink / raw)
  To: Wayen Yan
  Cc: netdev, lorenzo, horms, pabeni, kuba, edumazet, andrew+netdev,
	angelogioacchino.delregno, matthias.bgg, linux-arm-kernel,
	linux-mediatek
In-Reply-To: <6a2ca74a.c5b1db4e.21a698.01e7@mx.google.com>

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 13 Jun 2026 08:41:16 +0800 you wrote:
> Fix several typos found during code review:
> - Kconfig: "Aiorha" -> "Airoha" in NET_AIROHA_FLOW_STATS help text
> - Comment: "CMD1" -> "CDM1" (Central DMA, not Command)
> - Comments: "GMD1/2/3/4" -> "GDM1/2/3/4" (Gigabit DMA, not GMD)
> 
> These are pure comment and documentation fixes with no functional impact.
> 
> [...]

Here is the summary with links:
  - net: airoha: Fix typos in comments and Kconfig
    https://git.kernel.org/netdev/net-next/c/a061dfb063fa

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply

* Re: [PATCH] net: airoha: Fix non-standard return value in airoha_ppe_get_wdma_info()
From: patchwork-bot+netdevbpf @ 2026-06-15 22:50 UTC (permalink / raw)
  To: Wayen Yan
  Cc: netdev, lorenzo, horms, pabeni, kuba, edumazet, andrew+netdev,
	angelogioacchino.delregno, matthias.bgg, linux-arm-kernel,
	linux-mediatek
In-Reply-To: <6a2ca3d9.ad59c0a6.147df9.2a62@mx.google.com>

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 13 Jun 2026 08:22:31 +0800 you wrote:
> airoha_ppe_get_wdma_info() returns -1 when the last path in the
> forwarding path stack is not of type DEV_PATH_MTK_WDMA. This is not
> a standard kernel error code. Replace it with -EINVAL since the
> input path type is invalid from the caller's perspective.
> 
> Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
> Signed-off-by: Wayen.Yan <win847@gmail.com>
> 
> [...]

Here is the summary with links:
  - net: airoha: Fix non-standard return value in airoha_ppe_get_wdma_info()
    https://git.kernel.org/netdev/net-next/c/05173fa30add

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply

* [PATCH v2] media: mt2063: correct CONFIG_MEDIA_TUNER_MT2063 macro name in comment
From: Ethan Nelson-Moore @ 2026-06-15 22:53 UTC (permalink / raw)
  To: GitAuthor: Ethan Nelson-Moore, linux-media, linux-arm-kernel,
	linux-mediatek
  Cc: Mauro Carvalho Chehab, Matthias Brugger,
	AngeloGioacchino Del Regno

A comment in drivers/media/tuners/mt2063.h incorrectly refers to
CONFIG_DVB_MT2063 instead of CONFIG_MEDIA_TUNER_MT2063. Correct it.

Discovered while searching for CONFIG_* symbols referenced in code but
not defined in any Kconfig file.

Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
Changes in v2: Use correct media: commit message prefix

 drivers/media/tuners/mt2063.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/tuners/mt2063.h b/drivers/media/tuners/mt2063.h
index 30d03cd76061..6c4b6c68ec25 100644
--- a/drivers/media/tuners/mt2063.h
+++ b/drivers/media/tuners/mt2063.h
@@ -24,6 +24,6 @@ static inline struct dvb_frontend *mt2063_attach(struct dvb_frontend *fe,
 	return NULL;
 }
 
-#endif /* CONFIG_DVB_MT2063 */
+#endif /* IS_REACHABLE(CONFIG_MEDIA_TUNER_MT2063) */
 
 #endif /* __MT2063_H__ */
-- 
2.43.0



^ permalink raw reply related

* Re: [PATCH v2 2/4] iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver
From: Roman Vivchar @ 2026-06-15 22:52 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Roman Vivchar via B4 Relay, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Lee Jones,
	linux-iio, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Ben Grisdale
In-Reply-To: <20260614182214.65d052e4@jic23-huawei>

Hi Jonathan,

On Sunday, June 14th, 2026 at 8:22 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Tue, 09 Jun 2026 16:31:59 +0300
> Roman Vivchar via B4 Relay <devnull+rva333.protonmail.com@kernel.org> wrote:

...
 
> > +
> > +#define MTK_PMIC_IIO_CHAN(_name, _chan, _addr)                  \
> > +{                                                               \
> > +	.type = IIO_VOLTAGE,                                    \
> > +	.indexed = 1,                                           \
> > +	.channel = _chan,                                       \
> > +	.address = _addr,                                       \
> > +	.datasheet_name = __stringify(_name),                   \
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
> > +			      BIT(IIO_CHAN_INFO_SCALE),         \
> > +}
> > +
> > +static const struct iio_chan_spec mt6323_auxadc_channels[] = {
> > +	MTK_PMIC_IIO_CHAN(baton2,    MT6323_AUXADC_BATON2,    MT6323_AUXADC_ADC6),
> > +	MTK_PMIC_IIO_CHAN(ch6,       MT6323_AUXADC_CH6,       MT6323_AUXADC_ADC11),
> > +	MTK_PMIC_IIO_CHAN(bat_temp,  MT6323_AUXADC_BAT_TEMP,  MT6323_AUXADC_ADC5),
> 
> Reasonable query from Sashiko on why temperature channels are presented as voltages.
> If for some reason that is the right choice, then maybe a comment here.

mt6323 ADC always returns voltage. The thermal driver (which was in the
previous series and will be sent later) is required to map these to the
actual temperature. Ack.

...

> > +/*
> > + * The MediaTek MT6323 (as well as a lot of other PMICs) has the following hierarchy:
> > + * PMIC AUXADC <- PMIC MFD <- SoC PWRAP (wrapper for PWRAP FSM)
> > + *
> > + * Therefore, PWRAP regmap should be obtained using dev->parent->parent.
> > + */
> > +struct mt6323_auxadc {
> > +	struct regmap *regmap;
> > +	struct mutex lock;
> Locks should always have a comment on what data they are protecting.
> I think this one is about protecting the state of a device during a channel read
> by serializing those reads.

Nuno said kerneldoc looks unnecessary on v1 [1]. How the comment should
look?

...

> > +static int mt6323_auxadc_request(struct mt6323_auxadc *auxadc,
> > +				 unsigned long channel)
> > +{
> > +	struct regmap *map = auxadc->regmap;
> > +	int ret;
> > +
> > +	ret = regmap_set_bits(map, MT6323_AUXADC_CON11, AUXADC_CON11_VBUF_EN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return regmap_set_bits(map, MT6323_AUXADC_CON22, BIT(channel));
> 
> I'm not sure whether the sashiko question on this is valid or not. Make sure to take
> a look.
> 
> https://sashiko.dev/#/patchset/20260609-mt6323-adc-v2-0-aa93a22309f9%40protonmail.com
> You may have carefully selected the numbering so the channel numbering matches
> the bits in this register.  If so, it is probably worth a comment in the header
> to provide a cross reference.  No idea if Sashiko will notice that, but at least
> humans should!

The hardware is pretty weird, but dt-bindings have correct numbers.
I have double checked with the vendor driver and the logic is the same.

'If regmap_set_bits() fails to set MT6323_AUXADC_CON22, does this leave the
AUXADC voltage buffer (VBUF) permanently enabled?' - if this happens,
then there's something really wrong with PWRAP and disabling VBUF may
not be possible. Same about the 'mt6323_auxadc_release' comment.

...

> > +	case IIO_CHAN_INFO_RAW:
> 
> What Andy suggested here is the preferred path in IIO at least.
> Mainly because it reduced indent without hurting readability.
> Just be careful to define the scope with { }

Ack.

> 
> 
> > +		scoped_guard(mutex, &auxadc->lock) {
> > +			ret = mt6323_auxadc_prepare_channel(auxadc);
> > +			if (ret)
> > +				return ret;
> > +
> > +			ret = mt6323_auxadc_request(auxadc, chan->channel);
> > +			if (ret)
> > +				return ret;
> > +
> > +			/* Hardware limitation: the AUXADC needs a delay to become ready. */
> > +			fsleep(300);
> > +
> > +			ret = mt6323_auxadc_read(auxadc, chan, val);
> > +
> > +			if (mt6323_auxadc_release(auxadc, chan->channel))
> > +				dev_err(&indio_dev->dev,
> > +					"failed to release channel %d\n", chan->channel);
> > +
> > +			if (ret)
> > +				return ret;
> > +		}
> > +		return IIO_VAL_INT;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> 
> 

After these changes, should I keep or drop Andy's Reviewed-by?

[1]: https://lore.kernel.org/linux-iio/2df4cad5e29fbcb4c5c5f59ea0bf322c7a301bdc.camel@gmail.com/

Best regards,
Roman


^ permalink raw reply

* Re: [PATCH] net: airoha: Fix MODULE_LICENSE to match SPDX GPL-2.0-only identifier
From: patchwork-bot+netdevbpf @ 2026-06-15 23:00 UTC (permalink / raw)
  To: Wayen Yan
  Cc: netdev, lorenzo, horms, pabeni, kuba, edumazet, andrew+netdev,
	angelogioacchino.delregno, matthias.bgg, linux-arm-kernel,
	linux-mediatek
In-Reply-To: <6a2ded59.63d39acb.391892.7632@mx.google.com>

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 14 Jun 2026 07:52:39 +0800 you wrote:
> Both airoha_eth.c and airoha_npu.c declare SPDX-License-Identifier:
> GPL-2.0-only but use MODULE_LICENSE("GPL"), which the kernel module
> loader interprets as GPL-2.0+ (any GPL version). This mismatch causes
> license compliance tools (FOSSology, ScanCode, etc.) to misidentify
> the effective license as more permissive than intended.
> 
> Replace MODULE_LICENSE("GPL") with MODULE_LICENSE("GPL v2") to
> align with the GPL-2.0-only SPDX identifier. Per include/linux/module.h,
> "GPL v2" maps to GPL-2.0-only, matching the source files' declared
> license.
> 
> [...]

Here is the summary with links:
  - net: airoha: Fix MODULE_LICENSE to match SPDX GPL-2.0-only identifier
    https://git.kernel.org/netdev/net-next/c/b0d62ed16424

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




^ permalink raw reply

* Re: [PATCH 3/4] mfd: mt6397-core: add mt6323 AUXADC support
From: Roman Vivchar @ 2026-06-15 23:10 UTC (permalink / raw)
  To: Lee Jones
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, linux-iio, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Ben Grisdale
In-Reply-To: <20260611163748.GB1212816@google.com>

Hi Lee,

On Thursday, June 11th, 2026 at 7:37 PM, Lee Jones <lee@kernel.org> wrote:
> Interesting.  Has this been considered?
> 
> /* Sashiko Automation: Issues Found (1 Findings) */
> 
> On Tue, 02 Jun 2026, Roman Vivchar via B4 Relay wrote:
> 
> > From: Roman Vivchar <rva333@protonmail.com>
> >
> > The mt6323 PMIC includes an AUXADC. Register the AUXADC in the mt6323
> > devices array to allow the corresponding driver to probe using compatible
> > string.
> >
> > Tested-by: Ben Grisdale <bengris32@protonmail.ch> # Amazon Echo Dot (2nd Generation)
> > Signed-off-by: Roman Vivchar <rva333@protonmail.com>
> > ---
> >  drivers/mfd/mt6397-core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> > index 3e58d0764c7e..013b0857fb54 100644
> > --- a/drivers/mfd/mt6397-core.c
> > +++ b/drivers/mfd/mt6397-core.c
> > @@ -125,6 +125,9 @@ static const struct resource mt6323_pwrc_resources[] = {
> >
> >  static const struct mfd_cell mt6323_devs[] = {
> >  	{
> > +		.name = "mt6323-auxadc",
> > +		.of_compatible = "mediatek,mt6323-auxadc",
> 
> [Severity: High]
> Does adding this compatible string expose an unhandled edge case in the
> auxadc driver?
> 
> If this device is instantiated directly from a Device Tree without an MFD
> parent (e.g., at the DT root), dev->parent could be the platform bus
> (which has a NULL parent) or NULL itself.
> 
> Looking at mt6323_auxadc_probe() in drivers/iio/adc/mt6323-auxadc.c:
> 
>     regmap = dev_get_regmap(dev->parent->parent, NULL);
> 
> Could blindly dereferencing dev->parent->parent here, or passing a NULL
> device to dev_get_regmap() (which calls devres_find()), result in a
> kernel oops if probed as a root node?

This is intended behavior, ADC always should be probed as a child of
the mt6323 MFD because it doesn't have own MMIO space. Otherwise the
devicetree is malformed. Recent mt6359 ADC driver has same thing with
dev->parent->parent regmap.

Best regards,
Roman


^ permalink raw reply

* Re: [PATCH net-next v7 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
From: Jakub Kicinski @ 2026-06-15 23:10 UTC (permalink / raw)
  To: m-malladi
  Cc: Jakub Kicinski, elfring, haokexin, vadim.fedorenko, devnexen,
	horms, jacob.e.keller, arnd, basharath, afd, parvathi,
	vladimir.oltean, rogerq, danishanwar, pabeni, edumazet, davem,
	andrew+netdev, linux-arm-kernel, netdev, linux-kernel, srk,
	vigneshr
In-Reply-To: <20260610052511.781752-2-m-malladi@ti.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: ti: icssg-prueth: Add Frame Preemption MAC Merge support

This patch adds QoS infrastructure for Frame Preemption (FPE) in the
ICSSG Ethernet driver, including a verify state machine driver helper,
mqprio-based preemptible-tc mapping, and a link-state callback. A
follow-up patch in this series wires this up to ethtool's MAC Merge ops.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -392,6 +392,8 @@ static void emac_adjust_link(struct net_device *ndev)
>  		} else {
>  			icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
>  		}
> +
> +		icssg_qos_link_state_update(ndev);
>  	}

[Medium]
emac_adjust_link() sets emac->link = 0 before reaching this call. Inside
icssg_qos_link_state_update() -> icssg_iet_change_preemptible_tcs() ->
icssg_config_ietfpe(), the very first check is:

    if (!emac->link) {
            netdev_dbg(ndev, "cannot change IET/FPE state ... \n");
            return 0;
    }

so on a link-down the IET path returns early without clearing
iet->fpe_active or iet->verify_status, even though firmware has already
been told ICSSG_EMAC_PORT_DISABLE just above. After the follow-up patch
adds get_mm(), would ethtool then report stale tx_active=true and a
stale verify_status while the link is down?

The commit message states:

    Verify state machine re-triggers on link up/down events based on
    fpe_enabled and fpe_active flags

Is the link-down half of that contract implemented anywhere?

[Medium]
A related question on the locking design: emac->link is written here by
emac_adjust_link() without taking iet->fpe_lock, but icssg_config_ietfpe()
reads emac->link inside the fpe_lock-held critical section. The lockset
for writers and readers is disjoint, so fpe_lock does not actually
serialize emac->link.

Can the following interleaving occur?

  CPU0 (ndo_setup_tc)               CPU1 (emac_adjust_link)
  mutex_lock(&iet->fpe_lock);
  reads emac->link == 1
                                     emac->link = 0;
                                     icssg_set_port_state(... DISABLE);
  proceeds to write FPE regs
  and issue PREMPT_TX_ENABLE
  mutex_unlock(&iet->fpe_lock);
                                     icssg_qos_link_state_update();
                                       sees emac->link == 0, returns

leaving stale FPE writes on a port the firmware has just disabled? The
commit message says "fpe_lock serializes all FPE state mutations" but
emac->link is one of those mutations and is not covered.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> @@ -0,0 +1,282 @@

[ ... ]

> +static int icssg_iet_verify_wait(struct prueth_emac *emac)
> +{
> +	void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +	unsigned long delay_us, timeout_us;
> +	u32 status;
> +	int ret;
> +
> +	delay_us = iet->verify_time_ms * 1000;
> +	timeout_us = delay_us * ICSSG_IET_VERIFY_ATTEMPTS;
> +
> +	ret = readb_poll_timeout(config + PRE_EMPTION_VERIFY_STATUS,
> +				 status,
> +				 status == ICSSG_IETFPE_STATE_SUCCEEDED,
> +				 delay_us,
> +				 timeout_us);
> +
> +	iet->verify_status = status;
> +	return ret;
> +}

[High]
With ICSSG_IET_MAX_VERIFY_TIME = 128 and ICSSG_IET_VERIFY_ATTEMPTS = 3,
this can sleep for up to 384ms while iet->fpe_lock is held. fpe_lock is
acquired by icssg_iet_change_preemptible_tcs() from two paths:

  - emac_tc_setup_mqprio() via .ndo_setup_tc, called with RTNL held.
  - icssg_qos_link_state_update() from emac_adjust_link(), invoked by
    phylib with phydev->lock held (prueth_emac_common_start()
    explicitly does mutex_lock(phydev->lock) before calling
    emac_adjust_link).

Once the follow-up patch wires set_mm() to write iet->mac_verify_configure,
this sleep becomes reachable from any user with CAP_NET_ADMIN. Should
the verify wait be deferred to a workqueue or a completion driven by a
firmware notification rather than block under RTNL / phydev->lock for
hundreds of milliseconds? Other drivers with MAC Merge support
(e.g. fec, stmmac) take the deferred-work approach for the same reason.

[Low]
The condition only breaks on ICSSG_IETFPE_STATE_SUCCEEDED, but the enum
also defines a distinct ICSSG_IETFPE_STATE_FAILED value reported by
firmware on a definitive verify failure. With the current condition,
would a FAILED state cause the loop to keep polling for the entire
timeout and then surface as:

    netdev_err(ndev, "MAC Verification failed with timeout\n");

even though it was not actually a timeout? Could the cond also break on
FAILED so the caller can distinguish the two?

> +int icssg_config_ietfpe(struct net_device *ndev, bool enable)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +	int ret;
> +	u8 val;
> +
> +	lockdep_assert_held(&iet->fpe_lock);
> +
> +	if (!emac->link) {
> +		netdev_dbg(ndev, "cannot change IET/FPE state when interface is down\n");
> +		return 0;
> +	}

[ ... ]

> +	if (enable && iet->mac_verify_configure) {
> +		ret = icssg_iet_verify_wait(emac);
> +		if (ret) {
> +			netdev_err(ndev, "MAC Verification failed with timeout\n");
> +			goto disable_tx;
> +		}
> +	} else if (enable) {
> +		/* Give firmware some time to update
> +		 * PRE_EMPTION_ACTIVE_TX state
> +		 */
> +		usleep_range(100, 200);
> +	}
> +
> +	if (enable) {
> +		val = readb(config + PRE_EMPTION_ACTIVE_TX);
> +		if (val != 1) {
> +			netdev_err(ndev,
> +				   "Firmware fails to activate IET/FPE\n");
> +			ret = -EIO;
> +			goto disable_tx;
> +		}
> +		iet->fpe_active = true;
> +	} else {
> +		iet->fpe_active = false;
> +	}
> +
> +	icssg_iet_set_preempt_mask(emac);
> +	netdev_dbg(ndev, "IET FPE %s successfully\n",
> +		   str_enable_disable(enable));
> +	return 0;
> +
> +disable_tx:
> +	icssg_set_port_state(emac, ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
> +fallback:
> +	writeb(0, config + PRE_EMPTION_ENABLE_TX);
> +	writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
> +	iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
> +	iet->fpe_active =  false;
> +	return ret;
> +}

[Medium]
icssg_set_port_state() only synchronizes the R30 command-done handshake;
it does not guarantee firmware has finished updating the
PRE_EMPTION_ACTIVE_TX status byte. Is a 100-200us sleep a documented
worst-case latency for firmware to publish that byte under load
(concurrent Rx, TAS, FDB ageing, link events)?

If the byte has not yet been written, this code returns -EIO,
mischaracterizing it as "Firmware fails to activate IET/FPE", and the
fallback path then writes PRE_EMPTION_ENABLE_TX = 0, defeating the
user's enable. icssg_iet_verify_wait() already uses
readb_poll_timeout() for an analogous status byte; could the same
pattern be applied here with a small millisecond-range timeout?

[Medium]
The success path always calls icssg_iet_set_preempt_mask(), and on
fpe_active=false that helper takes the reset_hw branch and clears all
per-queue BIT(4) preemptive markers. The disable_tx and fallback paths
do not call icssg_iet_set_preempt_mask().

If FPE was previously active (so EXPRESS_PRE_EMPTIVE_Q_MAP[i] = BIT(4)
on some queues) and a subsequent reconfigure fails (verify timeout, or
PRE_EMPTION_ACTIVE_TX != 1), the failure paths only clear
PRE_EMPTION_ENABLE_TX / PRE_EMPTION_ENABLE_VERIFY but leave the
per-queue map and the EXPRESS_PRE_EMPTIVE_Q_MASK with stale preemptive
bits. Should the failure paths also call icssg_iet_set_preempt_mask()
(after iet->fpe_active is set to false) to keep the queue map and the
TX-enable bit consistent?

[ ... ]

> +EXPORT_SYMBOL_GPL(icssg_qos_init);

[ ... ]

> +EXPORT_SYMBOL_GPL(icssg_qos_ndo_setup_tc);

[ ... ]

> +EXPORT_SYMBOL_GPL(icssg_qos_link_state_update);

[Low]
Per the Makefile change in this patch:

  icssg-y := icssg/icssg_common.o \
             ...
             icssg/icssg_ethtool.o \
             icssg/icssg_qos.o

icssg_qos.o is linked into the same icssg.ko module as the only callers
(icssg_prueth.c and icssg_common.c). Are there cross-module consumers
that need these symbols exported, or can the EXPORT_SYMBOL_GPL lines be
dropped since EXPORT_SYMBOL_GPL is only required for cross-module
linkage?
-- 
pw-bot: cr


^ permalink raw reply

* Re: [PATCH net-next v7 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
From: Jakub Kicinski @ 2026-06-15 23:10 UTC (permalink / raw)
  To: m-malladi
  Cc: Jakub Kicinski, elfring, haokexin, vadim.fedorenko, devnexen,
	horms, jacob.e.keller, arnd, basharath, afd, parvathi,
	vladimir.oltean, rogerq, danishanwar, pabeni, edumazet, davem,
	andrew+netdev, linux-arm-kernel, netdev, linux-kernel, srk,
	vigneshr
In-Reply-To: <20260610052511.781752-3-m-malladi@ti.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge

This patch wires up the ethtool MAC Merge ops (.get_mm, .set_mm,
.get_mm_stats) for the ICSSG PRU Ethernet driver and adds five firmware
preemption counters that feed both the new MM stats and the existing
ethtool -S table.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> index b715af21d23a..0620782318ab 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> @@ -294,6 +294,103 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,

[ ... ]

> +static int emac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
> +		       struct netlink_ext_ack *extack)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +	int err;
> +
> +	if (emac->is_sr1)
> +		return -EOPNOTSUPP;
> +
> +	if (!cfg->pmac_enabled) {
> +		NL_SET_ERR_MSG_MOD(extack, "preemptible MAC is always enabled");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	mutex_lock(&iet->fpe_lock);
> +	iet->verify_time_ms = cfg->verify_time;
> +	iet->tx_min_frag_size = cfg->tx_min_frag_size;
> +	iet->fpe_enabled = cfg->tx_enabled;
> +	iet->mac_verify_configure = cfg->verify_enabled;
> +	err = icssg_config_ietfpe(ndev, cfg->tx_enabled);
> +	mutex_unlock(&iet->fpe_lock);
> +
> +	return err;
> +}

[Medium]
Can the cached software state in iet diverge from the hardware here when
icssg_config_ietfpe() fails?

The new fields (verify_time_ms, tx_min_frag_size, fpe_enabled,
mac_verify_configure) are written to iet before icssg_config_ietfpe() is
called, but the error paths in icssg_config_ietfpe() only roll back a
subset of state:

icssg_qos.c:icssg_config_ietfpe() {
    ...
disable_tx:
        icssg_set_port_state(emac, ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
fallback:
        writeb(0, config + PRE_EMPTION_ENABLE_TX);
        writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
        iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
        iet->fpe_active =  false;
        return ret;
}

iet->fpe_enabled is left set to cfg->tx_enabled even though the hardware
TX preempt path has been disabled.

Two follow-on effects appear to come from this:

emac_get_mm() reports state->tx_enabled = iet->fpe_enabled, so userspace
sees tx_enabled=true after a set_mm that returned an error and left FPE
disabled in hardware.

icssg_qos_link_state_update() drives icssg_iet_change_preemptible_tcs(),
which calls icssg_config_ietfpe(emac->ndev, iet->fpe_enabled) on every
link transition, so the failed configuration is retried each link-up
without further user action.

Would it make sense to stage cfg into local variables, only commit them
to iet after icssg_config_ietfpe() returns success, or otherwise
restore the previous iet fields on the error return?

> +static void emac_get_mm_stats(struct net_device *ndev,
> +			      struct ethtool_mm_stats *s)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +
> +	if (emac->is_sr1)
> +		return;
> +
> +	if (!emac->prueth->pa_stats)
> +		return;
> +
> +	emac_update_hardware_stats(emac);
> +
> +	/* MACMergeHoldCount stats is not tracked by the firmware */
> +	s->MACMergeFrameAssOkCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_OK");
> +	s->MACMergeFrameAssErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_ERR");
> +	s->MACMergeFragCountRx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_RX");
> +	s->MACMergeFragCountTx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_TX");
> +	s->MACMergeFrameSmdErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_BAD_FRAG");
> +}

[ ... ]

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> index 5ec0b38e0c67..8073deac35c3 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> @@ -189,6 +187,11 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
>  	ICSSG_PA_STATS(FW_INF_DROP_PRIOTAGGED),
>  	ICSSG_PA_STATS(FW_INF_DROP_NOTAG),
>  	ICSSG_PA_STATS(FW_INF_DROP_NOTMEMBER),
> +	ICSSG_PA_STATS(FW_PREEMPT_BAD_FRAG),
> +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_ERR),
> +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_TX),
> +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_OK),
> +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_RX),
>  	ICSSG_PA_STATS(FW_RX_EOF_SHORT_FRMERR),
>  	ICSSG_PA_STATS(FW_RX_B0_DROP_EARLY_EOF),
>  	ICSSG_PA_STATS(FW_TX_JUMBO_FRM_CUTOFF),

[Medium]
Are these five new entries duplicating values that already have a
standard uAPI?

The same five firmware counters are exposed through the new
.get_mm_stats callback as the standardized MAC Merge stats
(MACMergeFrameAssOkCount, MACMergeFrameAssErrorCount, MACMergeFragCountRx,
MACMergeFragCountTx, MACMergeFrameSmdErrorCount in struct
ethtool_mm_stats), and adding them to icssg_all_pa_stats[] also
publishes them via emac_get_strings() / emac_get_ethtool_stats() as
ethtool -S strings.

Documentation/networking/statistics.rst describes ethtool -S as the
private-driver-stats interface; counters that have a standard uAPI are
expected to flow only through that uAPI.

Could the firmware-register lookup table used by emac_get_stat_by_name()
be separated from the ethtool -S string table, so the new preemption
counters feed get_mm_stats without also showing up under ethtool -S?


^ permalink raw reply

* Re: [PATCH] iommu/arm-smmu-v3: Declare eats_s1chk and eats_trans as host-endian u64
From: Pranjal Shrivastava @ 2026-06-15 23:18 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Will Deacon, Robin Murphy, joro, Jason Gunthorpe, Shuai Xue,
	linux-arm-kernel, iommu, linux-kernel
In-Reply-To: <20260615194533.3290010-1-nicolinc@nvidia.com>

On Mon, Jun 15, 2026 at 12:45:33PM -0700, Nicolin Chen wrote:
> arm_smmu_get_ste_update_safe() declares the eats_s1chk and eats_trans
> locals as __le64, but initializes them from FIELD_PREP(), which returns a
> host-endian value, and passes them through cpu_to_le64() at the use sites.
> 
> Sparse reports the following warnings:
> 
> >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1122:38: sparse: sparse: cast from restricted __le64
>    drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1124:33: sparse: sparse: cast from restricted __le64
> 
> Declare both locals as u64 so the type matches FIELD_PREP() and the
> existing cpu_to_le64() at the use sites performs the host-to-little-endian
> conversion. No functional change.
> 
> Fixes: 7cad80048595 ("iommu/arm-smmu-v3: Mark EATS_TRANS safe when computing the update sequence")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/all/202606151017.QU0evpH9-lkp@intel.com/
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 8ce3e801eda3b..4c0f7b17b1f37 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1240,9 +1240,9 @@ VISIBLE_IF_KUNIT
>  void arm_smmu_get_ste_update_safe(const __le64 *cur, const __le64 *target,
>  				  __le64 *safe_bits)
>  {
> -	const __le64 eats_s1chk =
> +	const u64 eats_s1chk =
>  		FIELD_PREP(STRTAB_STE_1_EATS, STRTAB_STE_1_EATS_S1CHK);
> -	const __le64 eats_trans =
> +	const u64 eats_trans =
>  		FIELD_PREP(STRTAB_STE_1_EATS, STRTAB_STE_1_EATS_TRANS);

Reviewed-by: Pranjal Shrivastava <praan@google.com>

Thanks,
Praan


^ permalink raw reply

* Re: [PATCH net 4/4] net: ti: icssg: Fix XSK zero copy TX during application wakeup
From: Jakub Kicinski @ 2026-06-15 23:21 UTC (permalink / raw)
  To: Meghana Malladi
  Cc: diogo.ivo, haokexin, vadim.fedorenko, devnexen, horms,
	jacob.e.keller, sdf, john.fastabend, hawk, daniel, ast, pabeni,
	edumazet, davem, andrew+netdev, bpf, linux-kernel, netdev,
	linux-arm-kernel, srk, Vignesh Raghavendra, Roger Quadros,
	danishanwar
In-Reply-To: <20260611185744.2498070-5-m-malladi@ti.com>

On Fri, 12 Jun 2026 00:27:44 +0530 Meghana Malladi wrote:
> @@ -169,9 +169,6 @@ static int emac_xsk_xmit_zc(struct prueth_emac *emac,
>  
>  		num_tx++;
>  	}
> -
> -	xsk_tx_release(tx_chn->xsk_pool);
> -	return num_tx;

Why are you deleting this?

>  }
>  
>  void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
> @@ -279,9 +276,6 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
>  		num_tx++;
>  	}
>  
> -	if (!num_tx)
> -		return 0;

Does something prevent us from running all this code if budget is 0?
If budget is 0 we can complete normal Tx with skbs but we must
not touch any AF-XDP related state.

>  	netif_txq = netdev_get_tx_queue(ndev, chn);
>  	netdev_tx_completed_queue(netif_txq, num_tx, total_bytes);
>  
> @@ -306,7 +300,9 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
>  
>  		netif_txq = netdev_get_tx_queue(ndev, chn);
>  		txq_trans_cond_update(netif_txq);

This looks misplaced, now we will hit it even if we didn't complete 
or submit any Tx.

> +		__netif_tx_lock(netif_txq, smp_processor_id());
>  		emac_xsk_xmit_zc(emac, chn);
> +		__netif_tx_unlock(netif_txq);
>  	}


^ permalink raw reply

* Re: [PATCH net-next 1/9] net: microchip: vcap: fix rule move for rules of coprime size
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-1-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: microchip: vcap: fix rule move for rules of coprime size

This patch addresses a discrepancy in how software and hardware move rule
addresses on insert and delete for the LPM VCAP. It introduces a uniform
offset computation aligned to the least common multiple of all rule sizes
in a block. This ensures that hardware and software representations remain
synchronized and all rules land on correctly aligned addresses.

> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> index 0fdb5e363badc..6946fd7384581 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c

[ ... ]

> @@ -2275,8 +2336,7 @@ int vcap_add_rule(struct vcap_rule *rule)
>  		       __func__, __LINE__, ret);
>  		goto out;
>  	}
> -	if (move.count > 0)
> -		vcap_move_rules(ri, &move);
> +	vcap_move_rules(ri, &move);
>  
>  	/* Set the counter to zero */
>  	ret = vcap_write_counter(ri, &ctr);

[Severity: High]
This is a pre-existing issue, but does vcap_add_rule() leak the newly
duplicated rule if operations like vcap_write_counter() or
vcap_write_rule() fail later on?

If a failure occurs, the code takes the goto out path:

vcap_add_rule() {
    ...
    ret = vcap_write_rule(ri);
    if (ret) {
        pr_err("%s:%d: rule write error: %d\n", __func__, __LINE__, ret);
        goto out;
    }
out:
    mutex_unlock(&ri->admin->lock);
    return ret;
}

This seems to exit without reverting the earlier list insertions and
hardware moves.

Could this leave the software and hardware states permanently inconsistent
and leak the newly duplicated rule?

[ ... ]

> @@ -2488,59 +2548,52 @@ int vcap_mod_rule(struct vcap_rule *rule)
>  }
>  EXPORT_SYMBOL_GPL(vcap_mod_rule);

[Severity: High]
This isn't a bug introduced by this patch, but does vcap_mod_rule() discard
the active software rule pointer and overwrite hardware with stale addresses?

Looking at vcap_mod_rule(), the function validates the rule using
vcap_get_locked_rule() but ignores the returned pointer:

vcap_mod_rule(struct vcap_rule *rule) {
    struct vcap_rule_internal *ri = to_intrule(rule);
    ...
    if (!vcap_get_locked_rule(ri->vctrl, ri->data.id))
        return -ENOENT;
    ...
    err = vcap_encode_rule(ri);
    if (err)
        goto out;

    err = vcap_write_rule(ri);
}

Because the function continues to use the caller provided copy ri, it
doesn't see updates to the real rule in the admin->rules list.

If the real rule's hardware address was previously shifted due to insertions
or deletions, could writing to the stale address in ri->addr corrupt other
rules in the hardware VCAP?

[ ... ]

One other pre-existing issue noticed while reviewing this subsystem:

[Severity: High]
This is a pre-existing issue, but is it safe to iterate over the admin->rules
list across different VCAP instances without acquiring admin->lock for
each instance?

In vcap_rule_exists():

static bool vcap_rule_exists(struct vcap_control *vctrl, u32 id) {
    ...
    list_for_each_entry(admin, &vctrl->list, list)
        list_for_each_entry(ri, &admin->rules, list)
            if (ri->data.id == id)
                return true;
    return false;
}

If another thread is concurrently inserting or deleting a rule on another
VCAP instance while holding that instance's lock, could this unprotected
concurrent access encounter a data race and dereference a poisoned pointer?
-- 
pw-bot: cr


^ permalink raw reply

* Re: [PATCH net-next 4/9] net: microchip: vcap: expose helpers in vcap api and update debugfs
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-4-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: microchip: vcap: expose helpers in vcap api and update debugfs

Add new helpers to the vcap client api in preparation for L3 routing
functionality, rename PREROUTING to PREROUTING_L0, and fix a latent
undefined-behavior bug in the debugfs action-field printer.

> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
> index 30700648672fc..0905e4f192a02 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c

[ ... ]

> @@ -3547,6 +3560,18 @@ int vcap_rule_mod_action_u32(struct vcap_rule *rule,
>  }
>  EXPORT_SYMBOL_GPL(vcap_rule_mod_action_u32);
>  
> +/* Modify a bit action with value in the rule */
> +int vcap_rule_mod_action_bit(struct vcap_rule *rule,
> +			     enum vcap_action_field action,
> +			     enum vcap_bit val)
> +{
> +	struct vcap_client_actionfield_data data;
> +
> +	vcap_rule_set_action_bitsize(&data.u1, val);
> +	return vcap_rule_mod_action(rule, action, VCAP_FIELD_BIT, &data);

[Severity: Medium]
Could this copy uninitialized stack memory to the heap?

The data union is allocated on the stack but not initialized. The call to
vcap_rule_set_action_bitsize() only initializes the first byte (data.u1).

When data is passed to vcap_rule_mod_action(), it performs a memcpy() of the
entire 16-byte union:

vcap_rule_mod_action() {
	...
	memcpy(&field->data, data, sizeof(field->data));
	...
}

This appears to copy 15 bytes of uninitialized kernel stack memory into the
heap-allocated field->data, which might trigger KMSAN uninit-value warnings.

While a similar pattern exists in the pre-existing vcap_rule_mod_action_u32()
function, should the new data union be zero-initialized here?

> +}
> +EXPORT_SYMBOL_GPL(vcap_rule_mod_action_bit);
> +
>  /* Drop keys in a keylist and any keys that are not supported by the keyset */
>  int vcap_filter_rule_keys(struct vcap_rule *rule,
>  			  enum vcap_key_field keylist[], int length,


^ permalink raw reply

* Re: [PATCH net-next 7/9] net: sparx5: add L3 router infrastructure and leg management
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-7-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: sparx5: add L3 router infrastructure and leg management

This commit adds L3 router infrastructure and leg management to the
Microchip Sparx5 switch driver. It handles netdevice and inetaddr events
to intercept VLAN uppers of a bridge device, associating them with router
legs (VMIDs) for hardware offloading.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_router.c b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> new file mode 100644
> index 0000000000000..03923d91fdfb8
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c

[ ... ]

> +static void sparx5_rr_inet6_make_mask_le(int logmask, u8 *mask)
> +{
> +	/* Caller must ensure 0 <= logmask <= 128 */
> +	int rem, byte_prefix = logmask;
> +
> +	rem = do_div(byte_prefix, BITS_PER_BYTE);

[Severity: High]
Will this do_div() call cause a build failure or stack corruption?

The do_div() macro expects a 64-bit unsigned integer for the dividend. Passing
byte_prefix, which is a 32-bit int, causes a compiler error on architectures
like x86. On architectures like ARM, it can overwrite stack memory since the
macro assumes the pointer points to a 64-bit value.

Could byte_prefix be defined as a u64, or could this use a standard division
operation instead?

[ ... ]

> +static bool
> +sparx5_rr_router_leg_addr_list_empty_rcu(struct sparx5_rr_router_leg *leg)
> +{
> +	struct inet6_dev *inet6_dev;
> +	struct in_device *in_dev;
> +
> +	in_dev = __in_dev_get_rcu(leg->dev);
> +	if (in_dev && in_dev->ifa_list)
> +		return false;

[Severity: Medium]
Does this code miss an RCU pointer annotation?

The ifa_list pointer in struct in_device is annotated with __rcu, so accessing
it directly violates RCU rules and can cause sparse warnings.

Could this be updated to use rcu_access_pointer() since it's just doing a
boolean check?

[ ... ]

> +/* Called in atomic context. */
> +static int sparx5_rr_inet6addr_event(struct notifier_block *nb,
> +				     unsigned long event, void *ptr)
> +{
> +	struct inet6_ifaddr *if6 = (struct inet6_ifaddr *)ptr;
> +	struct sparx5_rr_inet6addr_event_work *work;
> +	struct net_device *dev = if6->idev->dev;
> +	struct sparx5_router *router;
> +
> +	if (event != NETDEV_DOWN)
> +		return NOTIFY_DONE;
> +
> +	work = kzalloc_obj(*work, GFP_ATOMIC);
> +	if (!work)
> +		return NOTIFY_BAD;

[Severity: High]
Does returning NOTIFY_BAD during a NETDEV_DOWN event illegally abort the atomic
notification chain?

The inet6addr_chain is an atomic notifier chain, so returning NOTIFY_BAD stops
chain traversal. Because NETDEV_DOWN is a post-action event indicating the
address has already been removed, halting the chain prevents subsequent
notifiers (e.g., routing tables, IPsec) from observing the removal. This
leaves stale state that can cause use-after-free or routing blackholes.

Should this driver log an error and return NOTIFY_DONE or NOTIFY_OK instead
if memory allocation fails?

[ ... ]

> +/* Called with RTNL. */
> +static int sparx5_rr_inetaddr_valid_event(struct notifier_block *nb,
> +					  unsigned long event, void *ptr)
> +{
> +	struct in_validator_info *ivi = (struct in_validator_info *)ptr;
> +	struct net_device *dev = ivi->ivi_dev->dev;
> +	struct sparx5_router *router;
> +
> +	ASSERT_RTNL();
> +
> +	if (event != NETDEV_UP)
> +		return NOTIFY_DONE;
> +
> +	router = container_of(nb, struct sparx5_router, inetaddr_valid_nb);
> +
> +	return sparx5_rr_inetaddr_event_handle(router->sparx5, dev, event);
> +}

[Severity: High]
Does allocating hardware resources in the validator chain cause a leak?

The driver allocates a hardware router leg (VMID) via
sparx5_rr_router_leg_create() and takes a netdev_hold reference during the
NETDEV_UP validation phase.

If a downstream validator rejects the address addition, or if the addition
natively fails, NETDEV_DOWN is never fired. Because there is no rollback hook
handling this failure, the VMID and net_device tracker reference are leaked. A
leaked reference permanently prevents the net_device from being unregistered,
hanging unregister_netdevice() in netdev_wait_allrefs().

[ ... ]

> +/* Called with RTNL. */
> +static int sparx5_rr_netdevice_event(struct notifier_block *nb,
> +				     unsigned long event, void *ptr)
> +{
> +	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> +	unsigned char mac[ETH_ALEN] __aligned(2);
> +	struct sparx5_router *router;
> +	struct sparx5 *sparx5;
> +
> +	ASSERT_RTNL();
> +
> +	router = container_of(nb, struct sparx5_router, netdevice_nb);
> +	sparx5 = router->sparx5;
> +
> +	/* Allow single bridge. Global router leg MAC tracks bridge mac. */
> +	if (!netif_is_bridge_master(dev))
> +		return NOTIFY_OK;
> +
> +	switch (event) {
> +	case NETDEV_CHANGEADDR:
> +		ether_addr_copy(mac, dev->dev_addr);
> +		sparx5_rr_leg_base_mac_set(sparx5, mac);
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}

[Severity: High]
Does this notifier miss network namespace and offload ownership checks?

The netdevice and inetaddr notifiers process events globally. In
sparx5_rr_netdevice_event(), it only checks netif_is_bridge_master(dev). The
driver doesn't verify if the bridge actually contains Sparx5 physical ports or
if it is inside the target network namespace (init_net).

An unprivileged user inside a user or network namespace could create dummy
VLAN-aware bridges and change their MAC addresses or assign IPs. This would
trigger these global notifiers, causing the driver to program physical
hardware registers, overwrite the global router leg MAC, and exhaust the
finite 511 VMID slots, potentially leading to a denial of service.


^ permalink raw reply

* Re: [PATCH net-next 6/9] net: sparx5: vcap: add lpm vcap implementation
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-6-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: sparx5: vcap: add lpm vcap implementation

Add the implementation for the LPM VCAP for lan969x and sparx5. The LPM
VCAP has a static keyset configuration, and a single lookup dedicated to
LPM, so this is straight forward, and many functions can use existing
functionality.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> index 95b93e46a41dc..e25e759c24f60 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> @@ -1083,6 +1146,46 @@ static void sparx5_vcap_es2_cache_write(struct sparx5 *sparx5,
>  	}
>  }
>  
> +static void sparx5_vcap_lpm_cache_write(struct sparx5 *sparx5,
> +					struct vcap_admin *admin,
> +					enum vcap_selection sel,
> +					u32 start,
> +					u32 count)
> +{
> +	u32 *keystr, *mskstr, *actstr;
> +	int idx;
> +
> +	keystr = &admin->cache.keystream[start];
> +	mskstr = &admin->cache.maskstream[start];
> +	actstr = &admin->cache.actionstream[start];
> +
> +	switch (sel) {
> +	case VCAP_SEL_ENTRY:
> +		for (idx = 0; idx < count; ++idx) {
> +			/* Avoid 'match-off' by setting value & mask */
> +			spx5_wr(keystr[idx] & mskstr[idx], sparx5,
> +				VCAP_SUPER_VCAP_ENTRY_DAT(idx));

[Severity: High]
Are concurrent writes to the shared VCAP_SUPER_VCAP_ENTRY_DAT registers
safe here without a global hardware lock across the different VCAP types?

It looks like IS0 and IS2 rules are updated via vcap_add_rule() under the
rtnl_lock(). However, the LPM rules are updated under the
sparx5->router->lock inside a workqueue.

Since both execution paths share the Super VCAP hardware block, could
concurrent updates from TC (IS0/IS2) and routing (LPM) race on these shared
cache registers, and also on VCAP_SUPER_CTRL in sparx5_vcap_super_update(),
garbling the hardware command and corrupting the cache memory?

> +			spx5_wr(~mskstr[idx], sparx5,
> +				VCAP_SUPER_VCAP_MASK_DAT(idx));
> +		}
> +		break;

[ ... ]

> @@ -1379,6 +1523,7 @@ static void sparx5_vcap_update(struct net_device *ndev,
>  	struct sparx5 *sparx5 = port->sparx5;
>  
>  	switch (admin->vtype) {
> +	case VCAP_TYPE_LPM:
>  	case VCAP_TYPE_IS0:
>  	case VCAP_TYPE_IS2:
>  		sparx5_vcap_super_update(sparx5, cmd, sel, addr);


^ permalink raw reply

* Re: [PATCH net-next 8/9] net: sparx5: add L3 FIB, nexthop and neighbour entry management
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-8-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: sparx5: add L3 FIB, nexthop and neighbour entry management

Add the data path for L3 unicast route offloading for IPv4 and IPv6 FIB
and nexthop groups.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_router.c b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> @@ -396,6 +400,14 @@ static bool sparx5_rr_fib_info_should_ignore(struct sparx5_rr_fib_info *fi)
>  		       (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL);
>  }
>  
> +#if IS_ENABLED(CONFIG_IPV6)
> +static void sparx5_rr_rt6_release(struct fib6_info *rt)
> +{
> +	if (!rt->nh)
> +		rt->fib6_nh->fib_nh_flags &= ~RTNH_F_OFFLOAD;
> +
> +	fib6_info_release(rt);
> +}

[Severity: High]
Does this non-atomic flag modification race with the core IPv6 stack?

The driver clears RTNH_F_OFFLOAD directly from an async worker thread
without holding the rtnl_lock. Since the core networking stack updates
these flags under the rtnl_lock, could this lockless non-atomic operation
cause torn writes and overwrite state updates like RTNH_F_DEAD?

[ ... ]

> @@ -826,6 +1143,14 @@ static void sparx5_rr_neigh_entry_destroy(struct sparx5 *sparx5,
>  	kfree(entry);
>  }
>  
> +static void sparx5_rr_neigh_entry_put(struct sparx5 *sparx5,
> +				      struct sparx5_rr_neigh_entry *neigh_entry)
> +{
> +	if (neigh_entry && list_empty(&neigh_entry->nexthop_list) &&
> +	    !neigh_entry->hw_route.vrule_id_valid)
> +		sparx5_rr_neigh_entry_destroy(sparx5, neigh_entry);
> +}

[Severity: High]
Can a neighbour entry leak permanently if its subnet route is deleted while
a gateway nexthop still references it?

When the subnet route is deleted, sparx5_rr_fib_entry_destroy() sets
neigh_entry->fib_entry = NULL and disables the VCAP rule. However, if an
ARP update arrives while the gateway route still references the neighbour,
sparx5_rr_neigh_event_work() re-enables the VCAP rule by setting
vrule_id_valid to true.

Later, when the gateway route is deleted, sparx5_rr_neigh_entry_put() will
skip destroying the neighbour because vrule_id_valid is true. The neighbour
would be left with an empty nexthop_list and a NULL fib_entry, meaning it
cannot be found during teardown by sparx5_rr_fib_flush(). Does this leak
the dev_tracker and hardware VCAP rule?

[ ... ]

> @@ -2126,6 +2753,8 @@ static int sparx5_rr_fib_replace(struct sparx5 *sparx5,
>  	/* Release and allow any previous fib_info to be deleted */
>  	sparx5_rr_fib_info_put(&old_fi);
>  
> +	fib_entry->type = sparx5_rr_rtm_type2fib_type(fi_type);
> +
>  	err = sparx5_rr_entry_nexthop_group_update(sparx5, fib_entry);

[Severity: Critical]
Could this in-place type update corrupt the LPM list?

If sparx5_rr_fib_replace() changes the route type from UNICAST to LOCAL,
the entry remains in the fib_lpm4_list or fib_lpm6_list. Later, when the
route is deleted, sparx5_rr_fib_entry_destroy() calls
sparx5_rr_fib_lpm_remove(), which has this check:

static void sparx5_rr_fib_lpm_remove(struct sparx5_rr_fib_entry *fib_entry)
{
        if (fib_entry->type != SPARX5_RR_FIB_TYPE_UNICAST)
                return;

        list_del(&fib_entry->fib_lpm_node);
}

Since the type is now LOCAL, the function returns early and skips the
list_del() operation. The entry is then freed, leaving a dangling pointer
in the linked list. Conversely, replacing a LOCAL route with a UNICAST
route appears to cause a NULL pointer dereference in list_del() because
it was never added to the list.

[ ... ]

> @@ -2247,6 +2873,28 @@ static int sparx5_rr_fib6_nexthop_prune(struct sparx5 *sparx5,
> +	old_nrt6 = fib_entry->fi.fe6_info.nrt6;
> +	new_nrt6 = old_nrt6 >= f6i->nrt6 ? old_nrt6 - f6i->nrt6 : 0;
> +
> +	rt_arr = kzalloc_objs(struct fib6_info *, new_nrt6);
> +	if (!rt_arr)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < old_nrt6; i++) {
> +		struct fib6_info *fi = old_rt_arr[i];
> +
> +		if (sparx5_rr_fib6_rt_exists(f6i, fi)) {
> +			sparx5_rr_rt6_release(fi);
> +			continue;
> +		}
> +
> +		rt_arr[j++] = fi;
> +	}

[Severity: Critical]
Is there a risk of a heap buffer overflow in this loop?

The new array size is calculated as the difference between the old count
and the count of nexthops to remove. However, if the deletion request
contains nexthops that do not perfectly match the tracked nexthops,
sparx5_rr_fib6_rt_exists() will return false.

The loop then copies these non-matching elements into rt_arr. Could the
index j exceed new_nrt6 in this scenario, writing past the end of the
dynamically allocated buffer?

[ ... ]

> @@ -2939,6 +3032,12 @@ int sparx5_rr_router_init(struct sparx5 *sparx5)
>  err_unreg_inet_addr_val_notifier:
>  	unregister_inetaddr_validator_notifier(&r->inetaddr_valid_nb);
>  err_unreg_inet_notifier:
>  	unregister_inetaddr_notifier(&r->inetaddr_nb);
> +err_unreg_fib_notifier:
> +	unregister_fib_notifier(&init_net, &r->fib_nb);
>  err_workqueue_destroy:
>  	destroy_workqueue(r->sparx5_router_owq);
> +err_fib_ht_destroy:
> +	rhashtable_destroy(&r->fib_ht);
> +err_neigh_ht_destroy:
> +	rhashtable_destroy(&r->neigh_ht);

[Severity: High]
Does this error path leak all offloaded route hardware and software
structures?

If an error occurs after register_fib_notifier() has successfully run, the
error path calls destroy_workqueue(). This allows the initial FIB dump to
finish populating routes into the hardware and the r->fib_ht structure.

However, the error path subsequently calls rhashtable_destroy() without
first calling sparx5_rr_fib_flush(). Since rhashtable_destroy() only frees
the hash buckets and not the elements, does this permanently leak all the
inserted fib_entry objects, their dev_trackers, and their hardware VCAP
rules upon a module load failure?


^ permalink raw reply

* Re: [PATCH net-next 9/9] net: sparx5: add neighbour event handling for L3 routing
From: Jakub Kicinski @ 2026-06-15 23:38 UTC (permalink / raw)
  To: jensemil.schulzostergaard
  Cc: Jakub Kicinski, horatiu.vultur, UNGLinuxDriver, andrew+netdev,
	davem, edumazet, pabeni, daniel.machon, Steen.Hegelund, kees,
	gustavoars, netdev, linux-kernel, linux-arm-kernel,
	linux-hardening
In-Reply-To: <20260612-sparx5_l3_routing-v1-9-fc3c10160f49@microchip.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: sparx5: add neighbour event handling for L3 routing

Register a netevent notifier to handle NETEVENT_NEIGH_UPDATE events.
This completes the L3 unicast forwarding data path by updating hardware
ARP table entries and linked nexthops when neighbours are resolved.

> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_router.c b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_router.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_router.c

[ ... ]

> @@ -2525,6 +2525,104 @@ static int sparx5_rr_fib_event(struct notifier_block *nb, unsigned long event,
> +static void sparx5_rr_neigh_event_work(struct work_struct *work)
> +{

[ ... ]

> +	if (entry->connected && entry_connected &&
> +	    ether_addr_equal(entry->hwaddr, hwaddr))
> +		goto out_mutex;
> +
> +	ether_addr_copy(entry->hwaddr, hwaddr);
> +	sparx5_rr_neigh_entry_update(sparx5, entry, entry_connected);

[Severity: High]
Does this code risk re-offloading an orphaned neighbour entry with a stale
VMID?

When a subnet route is deleted, the fib_entry is destroyed. This unlinks the
neigh_entries, sets neigh_entry->fib_entry to NULL, and un-offloads them.
However, if another nexthop still references this neighbour,
sparx5_rr_neigh_entry_put() skips destruction.

The interface's router leg might then be destroyed, freeing its VMID for
reuse.

If an ARP update arrives later, this code finds the orphaned neigh_entry and
calls sparx5_rr_neigh_entry_update() to offload it again. This could apply a
hardware VCAP rule using the potentially reassigned VMID.

Also, because this sets vrule_id_valid to true, when the referencing nexthop
is eventually deleted, sparx5_rr_neigh_entry_put() will refuse to destroy it.
Since fib_entry is NULL, it won't be found during module unload, permanently
leaking the memory and the netdevice tracker reference.

> +	sparx5_rr_nexthops_update_notify(sparx5, entry, entry_connected);
> +	if (!entry_connected)
> +		sparx5_rr_neigh_entry_put(sparx5, entry);
> +
> +out_mutex:
> +	mutex_unlock(&sparx5->router->lock);
> +out:
> +	neigh_release(n);
> +	kfree(net_work);
> +}

[ ... ]

> +static int sparx5_rr_netevent_event(struct notifier_block *nb,
> +				    unsigned long event, void *ptr)
> +{

[ ... ]

> +	switch (event) {
> +	case NETEVENT_NEIGH_UPDATE:
> +		n = ptr;
> +
> +		if (n->tbl->family != AF_INET && n->tbl->family != AF_INET6)
> +			return NOTIFY_DONE;
> +
> +		port = sparx5_port_dev_lower_find(n->dev);
> +		if (!port)
> +			return NOTIFY_DONE;
> +
> +		net_work = kzalloc_obj(*net_work, GFP_ATOMIC);
> +		if (!net_work)
> +			return NOTIFY_BAD;

[Severity: High]
Could returning NOTIFY_BAD on allocation failure interrupt the global
notification chain?

The NETEVENT_NEIGH_UPDATE event is broadcast to all registered listeners via
the netevent_notif_chain atomic notifier chain. Returning NOTIFY_BAD from the
notifier callback prematurely aborts the chain traversal.

If this allocation fails, it appears this driver will silently prevent all
subsequent subsystems and drivers from receiving the neighbour update, which
might cause system-wide stale ARP/NDP caches.

Should this return NOTIFY_DONE instead?

> +
> +		INIT_WORK(&net_work->work, sparx5_rr_neigh_event_work);
> +		net_work->sparx5 = router->sparx5;
> +		net_work->neigh = neigh_clone(n);


^ permalink raw reply

* Re: [PATCH net-next v7 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
From: Jakub Kicinski @ 2026-06-15 23:39 UTC (permalink / raw)
  To: m-malladi
  Cc: elfring, haokexin, vadim.fedorenko, devnexen, horms,
	jacob.e.keller, arnd, basharath, afd, parvathi, vladimir.oltean,
	rogerq, danishanwar, pabeni, edumazet, davem, andrew+netdev,
	linux-arm-kernel, netdev, linux-kernel, srk, vigneshr
In-Reply-To: <20260615231041.1007484-1-kuba@kernel.org>

On Mon, 15 Jun 2026 16:10:41 -0700 Jakub Kicinski wrote:
> > diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > index 5ec0b38e0c67..8073deac35c3 100644
> > --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > @@ -189,6 +187,11 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
> >  	ICSSG_PA_STATS(FW_INF_DROP_PRIOTAGGED),
> >  	ICSSG_PA_STATS(FW_INF_DROP_NOTAG),
> >  	ICSSG_PA_STATS(FW_INF_DROP_NOTMEMBER),
> > +	ICSSG_PA_STATS(FW_PREEMPT_BAD_FRAG),
> > +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_ERR),
> > +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_TX),
> > +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_OK),
> > +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_RX),
> >  	ICSSG_PA_STATS(FW_RX_EOF_SHORT_FRMERR),
> >  	ICSSG_PA_STATS(FW_RX_B0_DROP_EARLY_EOF),
> >  	ICSSG_PA_STATS(FW_TX_JUMBO_FRM_CUTOFF),  
> 
> [Medium]
> Are these five new entries duplicating values that already have a
> standard uAPI?
> 
> The same five firmware counters are exposed through the new
> .get_mm_stats callback as the standardized MAC Merge stats
> (MACMergeFrameAssOkCount, MACMergeFrameAssErrorCount, MACMergeFragCountRx,
> MACMergeFragCountTx, MACMergeFrameSmdErrorCount in struct
> ethtool_mm_stats), and adding them to icssg_all_pa_stats[] also
> publishes them via emac_get_strings() / emac_get_ethtool_stats() as
> ethtool -S strings.
> 
> Documentation/networking/statistics.rst describes ethtool -S as the
> private-driver-stats interface; counters that have a standard uAPI are
> expected to flow only through that uAPI.
> 
> Could the firmware-register lookup table used by emac_get_stat_by_name()
> be separated from the ethtool -S string table, so the new preemption
> counters feed get_mm_stats without also showing up under ethtool -S?

This -- not sure about the other complaints but this one looks legit.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox