* [PATCH] net: fec: make FIXED_PHY dependency unconditional
From: Arnd Bergmann @ 2026-04-02 14:10 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Heiner Kallweit
Cc: Arnd Bergmann, netdev, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_FIXED_PHY is in a loadable module, the fec driver cannot be
built-in any more:
x86_64-linux-ld: vmlinux.o: in function `fec_enet_mii_probe':
fec_main.c:(.text+0xc4f367): undefined reference to `fixed_phy_unregister'
x86_64-linux-ld: vmlinux.o: in function `fec_enet_close':
fec_main.c:(.text+0xc59591): undefined reference to `fixed_phy_unregister'
x86_64-linux-ld: vmlinux.o: in function `fec_enet_mii_probe.cold':
Select the fixed phy support on all targets to make this build
correctly, not just on coldfire.
Notat that Essentially the stub helpers in include/linux/phy_fixed.h
cannot be used correctly because of this build time dependency,
and we could just remove them to hit the build failure more often
when a driver uses them without the 'select FIXED_PHY'.
Fixes: dc86b621e1b4 ("net: fec: register a fixed phy using fixed_phy_register_100fd if needed")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I sent the same fix for B44 earlier, see commit 3f0f591b44b0 ("net: b44:
always select CONFIG_FIXED_PHY"). I checked that there are no other
conditional users of FIXED_PHY this time.
---
drivers/net/ethernet/freescale/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index e2a591cf9601..11edbb46a118 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -28,7 +28,7 @@ config FEC
depends on PTP_1588_CLOCK_OPTIONAL
select CRC32
select PHYLIB
- select FIXED_PHY if M5272
+ select FIXED_PHY
select PAGE_POOL
imply PAGE_POOL_STATS
imply NET_SELFTESTS
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 1/2] net: fix skb_ext_total_length() BUILD_BUG_ON with CONFIG_GCOV_PROFILE_ALL
From: Vasileios Almpanis @ 2026-04-02 14:09 UTC (permalink / raw)
To: Konstantin Khorenko, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Thomas Weißschuh, Arnd Bergmann,
Peter Oberparleiter, Mikhail Zaslonko, netdev, linux-kernel,
Pavel Tikhomirov
In-Reply-To: <20260402140558.1437002-2-khorenko@virtuozzo.com>
Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
On 4/2/26 4:05 PM, Konstantin Khorenko wrote:
> When CONFIG_GCOV_PROFILE_ALL=y is enabled, the kernel fails to build:
>
> In file included from <command-line>:
> In function 'skb_extensions_init',
> inlined from 'skb_init' at net/core/skbuff.c:5214:2:
> ././include/linux/compiler_types.h:706:45: error: call to
> '__compiletime_assert_1490' declared with attribute error:
> BUILD_BUG_ON failed: skb_ext_total_length() > 255
>
> CONFIG_GCOV_PROFILE_ALL adds -fprofile-arcs -ftest-coverage
> -fno-tree-loop-im to CFLAGS globally. GCC inserts branch profiling
> counters into the skb_ext_total_length() loop and, combined with
> -fno-tree-loop-im (which disables loop invariant motion), cannot
> constant-fold the result.
> BUILD_BUG_ON requires a compile-time constant and fails.
>
> The issue manifests in kernels with 5+ SKB extension types enabled
> (e.g., after addition of SKB_EXT_CAN, SKB_EXT_PSP). With 4 extensions
> GCC can still unroll and fold the loop despite GCOV instrumentation;
> with 5+ it gives up.
>
> Mark skb_ext_total_length() with __no_profile to prevent GCOV from
> inserting counters into this function. Without counters the loop is
> "clean" and GCC can constant-fold it even with -fno-tree-loop-im active.
> This allows BUILD_BUG_ON to work correctly while keeping GCOV profiling
> for the rest of the kernel.
>
> This also removes the CONFIG_KCOV_INSTRUMENT_ALL preprocessor guard
> introduced by d6e5794b06c0, as __no_profile handles both GCOV and KCOV
> instrumentation at the root cause level rather than just disabling the
> check.
>
> Fixes: 5d21d0a65b57 ("net: generalize calculation of skb extensions length")
> Fixes: d6e5794b06c0 ("net: avoid build bug in skb extension length calculation")
>
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> net/core/skbuff.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0e217041958a..47c7f0ab6e84 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5145,7 +5145,7 @@ static const u8 skb_ext_type_len[] = {
> #endif
> };
>
> -static __always_inline unsigned int skb_ext_total_length(void)
> +static __always_inline __no_profile unsigned int skb_ext_total_length(void)
> {
> unsigned int l = SKB_EXT_CHUNKSIZEOF(struct skb_ext);
> int i;
> @@ -5159,9 +5159,7 @@ static __always_inline unsigned int skb_ext_total_length(void)
> static void skb_extensions_init(void)
> {
> BUILD_BUG_ON(SKB_EXT_NUM > 8);
> -#if !IS_ENABLED(CONFIG_KCOV_INSTRUMENT_ALL)
> BUILD_BUG_ON(skb_ext_total_length() > 255);
> -#endif
>
> skbuff_ext_cache = kmem_cache_create("skbuff_ext_cache",
> SKB_EXT_ALIGN_VALUE * skb_ext_total_length(),
^ permalink raw reply
* [PATCH 0/2] net: fix skb_ext BUILD_BUG_ON failures with GCOV
From: Konstantin Khorenko @ 2026-04-02 14:05 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Thomas Weißschuh, Arnd Bergmann,
Peter Oberparleiter, Mikhail Zaslonko, netdev, linux-kernel,
Konstantin Khorenko, Pavel Tikhomirov, Vasileios Almpanis
This mini-series fixes build failures in net/core/skbuff.c when the
kernel is built with CONFIG_GCOV_PROFILE_ALL=y.
This is part of a larger effort to add -fprofile-update=atomic to
global CFLAGS_GCOV (posted earlier as a combined series):
https://lore.kernel.org/lkml/20260401142020.1434243-1-khorenko@virtuozzo.com/T/#t
That combined series was split per subsystem as requested by Jakub.
The companion patches are:
- iommu: disable GCOV for iommu_amdv1.o (sent to iommu maintainers)
- gcov: add -fprofile-update=atomic globally (sent to gcov/kbuild
maintainers, depends on this series and the iommu patch)
Patch 1/2 fixes a pre-existing build failure with CONFIG_GCOV_PROFILE_ALL:
GCOV counters prevent GCC from constant-folding the skb_ext_total_length()
loop. This is v3 of a previously posted standalone fix:
https://lore.kernel.org/lkml/20260331165125.959833-1-khorenko@virtuozzo.com/T/#t
Patch 2/2 is an additional fix needed when -fprofile-update=atomic is
added to CFLAGS_GCOV: __no_profile on the __always_inline function alone
is insufficient because after inlining, the code resides in the caller's
profiled body. The caller needs __no_profile as well.
Konstantin Khorenko (2):
net: fix skb_ext_total_length() BUILD_BUG_ON with CONFIG_GCOV_PROFILE_ALL
net: add __no_profile to skb_extensions_init() for GCOV compatibility
net/core/skbuff.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
2.43.5
^ permalink raw reply
* [PATCH 1/2] net: fix skb_ext_total_length() BUILD_BUG_ON with CONFIG_GCOV_PROFILE_ALL
From: Konstantin Khorenko @ 2026-04-02 14:05 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Thomas Weißschuh, Arnd Bergmann,
Peter Oberparleiter, Mikhail Zaslonko, netdev, linux-kernel,
Konstantin Khorenko, Pavel Tikhomirov, Vasileios Almpanis
In-Reply-To: <20260402140558.1437002-1-khorenko@virtuozzo.com>
When CONFIG_GCOV_PROFILE_ALL=y is enabled, the kernel fails to build:
In file included from <command-line>:
In function 'skb_extensions_init',
inlined from 'skb_init' at net/core/skbuff.c:5214:2:
././include/linux/compiler_types.h:706:45: error: call to
'__compiletime_assert_1490' declared with attribute error:
BUILD_BUG_ON failed: skb_ext_total_length() > 255
CONFIG_GCOV_PROFILE_ALL adds -fprofile-arcs -ftest-coverage
-fno-tree-loop-im to CFLAGS globally. GCC inserts branch profiling
counters into the skb_ext_total_length() loop and, combined with
-fno-tree-loop-im (which disables loop invariant motion), cannot
constant-fold the result.
BUILD_BUG_ON requires a compile-time constant and fails.
The issue manifests in kernels with 5+ SKB extension types enabled
(e.g., after addition of SKB_EXT_CAN, SKB_EXT_PSP). With 4 extensions
GCC can still unroll and fold the loop despite GCOV instrumentation;
with 5+ it gives up.
Mark skb_ext_total_length() with __no_profile to prevent GCOV from
inserting counters into this function. Without counters the loop is
"clean" and GCC can constant-fold it even with -fno-tree-loop-im active.
This allows BUILD_BUG_ON to work correctly while keeping GCOV profiling
for the rest of the kernel.
This also removes the CONFIG_KCOV_INSTRUMENT_ALL preprocessor guard
introduced by d6e5794b06c0, as __no_profile handles both GCOV and KCOV
instrumentation at the root cause level rather than just disabling the
check.
Fixes: 5d21d0a65b57 ("net: generalize calculation of skb extensions length")
Fixes: d6e5794b06c0 ("net: avoid build bug in skb extension length calculation")
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
---
net/core/skbuff.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0e217041958a..47c7f0ab6e84 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5145,7 +5145,7 @@ static const u8 skb_ext_type_len[] = {
#endif
};
-static __always_inline unsigned int skb_ext_total_length(void)
+static __always_inline __no_profile unsigned int skb_ext_total_length(void)
{
unsigned int l = SKB_EXT_CHUNKSIZEOF(struct skb_ext);
int i;
@@ -5159,9 +5159,7 @@ static __always_inline unsigned int skb_ext_total_length(void)
static void skb_extensions_init(void)
{
BUILD_BUG_ON(SKB_EXT_NUM > 8);
-#if !IS_ENABLED(CONFIG_KCOV_INSTRUMENT_ALL)
BUILD_BUG_ON(skb_ext_total_length() > 255);
-#endif
skbuff_ext_cache = kmem_cache_create("skbuff_ext_cache",
SKB_EXT_ALIGN_VALUE * skb_ext_total_length(),
--
2.43.5
^ permalink raw reply related
* [PATCH 2/2] net: add __no_profile to skb_extensions_init() for GCOV compatibility
From: Konstantin Khorenko @ 2026-04-02 14:05 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Thomas Weißschuh, Arnd Bergmann,
Peter Oberparleiter, Mikhail Zaslonko, netdev, linux-kernel,
Konstantin Khorenko, Pavel Tikhomirov, Vasileios Almpanis
In-Reply-To: <20260402140558.1437002-1-khorenko@virtuozzo.com>
With -fprofile-update=atomic in global CFLAGS_GCOV, GCC still cannot
constant-fold the skb_ext_total_length() loop when it is inlined into a
profiled caller. The existing __no_profile on skb_ext_total_length()
itself is insufficient because after __always_inline expansion the code
resides in the caller's body, which still carries GCOV instrumentation.
Mark skb_extensions_init() with __no_profile so the BUILD_BUG_ON checks
can be evaluated at compile time.
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
net/core/skbuff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 47c7f0ab6e84..99704d6832e2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5156,7 +5156,7 @@ static __always_inline __no_profile unsigned int skb_ext_total_length(void)
return l;
}
-static void skb_extensions_init(void)
+static void __no_profile skb_extensions_init(void)
{
BUILD_BUG_ON(SKB_EXT_NUM > 8);
BUILD_BUG_ON(skb_ext_total_length() > 255);
--
2.43.5
^ permalink raw reply related
* [PATCH net] bridge: guard local VLAN-0 FDB helpers against NULL vlan group
From: Zijing Yin @ 2026-04-02 14:01 UTC (permalink / raw)
To: netdev
Cc: bridge, razor, idosch, davem, edumazet, kuba, pabeni, horms,
nathan, nick.desaulniers+lkml, morbo, justinstitt, petrm,
linux-kernel, llvm, Zijing Yin
When CONFIG_BRIDGE_VLAN_FILTERING is not set, br_vlan_group() and
nbp_vlan_group() return NULL (br_private.h stub definitions). The
BR_BOOLOPT_FDB_LOCAL_VLAN_0 toggle code is compiled unconditionally and
reaches br_fdb_delete_locals_per_vlan_port() and
br_fdb_insert_locals_per_vlan_port(), where the NULL vlan group pointer
is dereferenced via list_for_each_entry(v, &vg->vlan_list, vlist).
The observed crash is in the delete path, triggered when creating a
bridge with IFLA_BR_MULTI_BOOLOPT containing BR_BOOLOPT_FDB_LOCAL_VLAN_0
via RTM_NEWLINK. The insert helper has the same bug pattern.
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000056: 0000 [#1] KASAN NOPTI
KASAN: null-ptr-deref in range [0x00000000000002b0-0x00000000000002b7]
RIP: 0010:br_fdb_delete_locals_per_vlan+0x2b9/0x310
Call Trace:
br_fdb_toggle_local_vlan_0+0x452/0x4c0
br_toggle_fdb_local_vlan_0+0x31/0x80 net/bridge/br.c:276
br_boolopt_toggle net/bridge/br.c:313
br_boolopt_multi_toggle net/bridge/br.c:364
br_changelink net/bridge/br_netlink.c:1542
br_dev_newlink net/bridge/br_netlink.c:1575
Add NULL checks for the vlan group pointer in both helpers, returning
early when there are no VLANs to iterate. This matches the existing
pattern used by other bridge FDB functions such as br_fdb_add() and
br_fdb_delete().
Fixes: 21446c06b441 ("net: bridge: Introduce UAPI for BR_BOOLOPT_FDB_LOCAL_VLAN_0")
Signed-off-by: Zijing Yin <yzjaurora@gmail.com>
---
Tested on Linux v7.0-rc5 (upstream tag) with clang 20.1.0, KASAN
enabled, CONFIG_BRIDGE_VLAN_FILTERING=n.
Bug independently reproduced with the attached C reproducer
(repro_br_fdb.c). The crash triggers deterministically on the first
run with CONFIG_BRIDGE_VLAN_FILTERING=n on a clang-built kernel.
Exact crash signature from reproduction:
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000056: 0000 [#1] KASAN NOPTI
KASAN: null-ptr-deref in range [0x00000000000002b0-0x00000000000002b7]
RIP: 0010:br_fdb_delete_locals_per_vlan+0x72/0x3f0
Call Trace:
br_fdb_toggle_local_vlan_0+0x3d/0x1d0
br_boolopt_toggle+0xba/0x1a0
br_boolopt_multi_toggle+0x129/0x250
br_changelink+0x1100/0x1490
br_dev_newlink+0x115/0x190
rtnl_newlink+0xe15/0x25c0
Note: gcc 13.3 with the same config optimizes away the NULL dereference
path (UB elimination), so the crash does not trigger on gcc-built
kernels. The code is still incorrect regardless of compiler behavior.
Reproducer (C source): [PASTE_URL_HERE]
Kernel .config: [PASTE_URL_HERE]
To reproduce: compile the C reproducer with `gcc -static -o repro repro.c`,
run as root on a clang-built kernel. The crash triggers during
br_dev_newlink() -> br_changelink() when the boolopt toggle reaches
br_fdb_delete_locals_per_vlan_port() with a NULL vlan group. Note:
RTM_SETLINK on an existing bridge may not trigger it due to different
code ordering.
net/bridge/br_fdb.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 0501ffcb8..e2c17f620 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -597,6 +597,9 @@ static void br_fdb_delete_locals_per_vlan_port(struct net_bridge *br,
dev = br->dev;
}
+ if (!vg)
+ return;
+
list_for_each_entry(v, &vg->vlan_list, vlist)
br_fdb_find_delete_local(br, p, dev->dev_addr, v->vid);
}
@@ -630,6 +633,9 @@ static int br_fdb_insert_locals_per_vlan_port(struct net_bridge *br,
dev = br->dev;
}
+ if (!vg)
+ return 0;
+
list_for_each_entry(v, &vg->vlan_list, vlist) {
if (!br_vlan_should_use(v))
continue;
--
2.43.0
^ permalink raw reply related
* Re: "Dead loop on virtual device" error without softirq-BKL on PREEMPT_RT
From: Daniel Vacek @ 2026-04-02 13:58 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: edumazet, kuba, linux-kernel, linux-rt-devel, netdev, spasswolf,
tglx, Aaron Tomlin
In-Reply-To: <20260402134633.bRxz1V8a@linutronix.de>
On Thu, 2 Apr 2026 at 15:46, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 2026-04-02 11:21:20 [+0200], Daniel Vacek wrote:
> >
> > Well, that's the patch I originally sent then.
> >
> > https://lore.kernel.org/linux-rt-devel/20260318103009.2120920-1-neelx@suse.com/
>
> That was an alternative after everything was already done.
That is true. I don't mean to complain. But also because I haven't
seen the applied version on the mailing list. That's why I originally
sent my patch as a reply to this thread.
I'm simply asking whether we should proceed with the discussed
approach? Would that be better in the end?
If you don't object, I can send a rebased version along with a
follow-up cleanup.
--nX
> > --nX
>
> Sebastian
^ permalink raw reply
* Re: [PATCH net-next 05/11] net: macb: allocate tieoff descriptor once across device lifetime
From: Théo Lebrun @ 2026-04-02 13:57 UTC (permalink / raw)
To: Nicolai Buchwitz, Théo Lebrun
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran,
Russell King, Paolo Valerio, Conor Dooley, Vladimir Kondratiev,
Gregory CLEMENT, Benoît Monin, Tawfik Bayouk,
Thomas Petazzoni, Maxime Chevallier, netdev, linux-kernel
In-Reply-To: <8c500ccc919ac2d7b350eacca0ab6ccf@tipi-net.de>
On Thu Apr 2, 2026 at 1:14 PM CEST, Nicolai Buchwitz wrote:
> On 1.4.2026 18:39, Théo Lebrun wrote:
>> The tieoff descriptor is a RX DMA descriptor ring of size one. It gets
>> configured onto queues for Wake-on-LAN during system-wide suspend when
>> hardware does not support disabling individual queues
>> (MACB_CAPS_QUEUE_DISABLE).
>>
>> MACB/GEM driver allocates it alongside the main RX ring
>> inside macb_alloc_consistent() at open. Free is done by
>> macb_free_consistent() at close.
>>
>> Change to allocate once at probe and free on probe failure or device
>> removal. This makes the tieoff descriptor lifetime much longer,
>> avoiding repeating coherent buffer allocation on each open/close cycle.
>>
>> Main benefit: we dissociate its lifetime from the main ring's lifetime.
>> That way there is less work to be doing on resources (re)alloc. This
>> currently happens on close/open, but will soon also happen on context
>> swap operations (set_ringparam, change_mtu, set_channels, etc).
>>
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> drivers/net/ethernet/cadence/macb_main.c | 70
>> ++++++++++++++++----------------
>
>> [...]
>
>>
>> +static int macb_alloc_tieoff(struct macb *bp)
>> +{
>> + /* Tieoff is a workaround in case HW cannot disable queues, for PM.
>> */
>> + if (bp->caps & MACB_CAPS_QUEUE_DISABLE)
>> + return 0;
>> +
>> + bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
>> + macb_dma_desc_get_size(bp),
>> + &bp->rx_ring_tieoff_dma,
>> + GFP_KERNEL);
>> + if (!bp->rx_ring_tieoff)
>> + return -ENOMEM;
>> +
>> + return 0;
>> +}
>
> The old macb_init_tieoff() that wrote WRAP+USED into the
> descriptor is deleted but its work is not replicated here.
> dma_alloc_coherent zeroes the memory, so RX_USED=0 and the
> hardware will treat it as a valid receive buffer pointing to
> DMA address 0 during suspend.
>
> Shouldn't this have a macb_set_addr() + ctrl=0 after the
> allocation?
Clearly! This V1 uses tieoff uninitialised. The two instructions from
old macb_init_tieoff() have been appended to macb_alloc_tieoff().
static int macb_alloc_tieoff(struct macb *bp)
{
/* Tieoff is a workaround in case HW cannot disable queues, for PM. */
if (bp->caps & MACB_CAPS_QUEUE_DISABLE)
return 0;
bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
macb_dma_desc_get_size(bp),
&bp->rx_ring_tieoff_dma,
GFP_KERNEL);
if (!bp->rx_ring_tieoff)
return -ENOMEM;
macb_set_addr(bp, bp->rx_ring_tieoff,
MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED));
bp->rx_ring_tieoff->ctrl = 0;
return 0;
}
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [net-next PATCH 04/10] bitfield.h: add FIELD_WIDTH()
From: Yury Norov @ 2026-04-02 13:52 UTC (permalink / raw)
To: David Laight
Cc: Luiz Angelo Daros de Luca, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Linus Walleij, Alvin Šipraga, Yury Norov,
Rasmus Villemoes, Russell King, netdev, linux-kernel
In-Reply-To: <20260402102717.5eb48393@pumpkin>
On Thu, Apr 02, 2026 at 10:27:17AM +0100, David Laight wrote:
> On Thu, 2 Apr 2026 01:00:20 -0300
> Luiz Angelo Daros de Luca <luizluca@gmail.com> wrote:
>
> > > > +/**
> > > > + * FIELD_WIDTH() - return the width of a bitfield
> > > > + * @_mask: shifted mask defining the field's length and position
> > > > + *
> > > > + * Returns the number of contiguous bits covered by @_mask.
> > > > + * This corresponds to the bit width of FIELD_MAX(@_mask).
> > > > + */
> > > > +#define FIELD_WIDTH(_mask) \
> > >
> > > Please no underscored names unless necessary.
> >
> > I used _mask to maintain consistency with the existing public macros
> > in this file, such as FIELD_GET, FIELD_PREP, FIELD_MAX, and FIELD_FIT.
> > All of them use the underscore prefix for parameters. Should I diverge
> > from them?
> >
> > > > + ({ \
> > > > + __BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_WIDTH: "); \
> > > > + __bf_shf(~FIELD_MAX(_mask)); \
> > > > + })
> > >
> > > I believe, this should be:
> > >
> > > #define FIELD_WIDTH(mask) ({ \
> > > __BF_FIELD_CHECK_MASK(mask, 0ULL, "FIELD_WIDTH: "); \
> > > HWEIGHT(mask); \
> > > })
> >
> > HWEIGHT() is indeed much cleaner. However, to keep bitfield.h
> > self-contained and avoid adding more includes, I'll try
> > __builtin_popcountll() in a similar way __builtin_ffsll is already
> > used.
> >
> > I also noticed the suggestion to use __BF_FIELD_CHECK_MASK instead of
> > __BF_FIELD_CHECK. Both FIELD_MAX and FIELD_FIT currently use the full
> > __BF_FIELD_CHECK with 0ULL as a dummy register to ensure the mask fits
> > within the header's supported limits. If __BF_FIELD_CHECK_MASK is
> > preferred for FIELD_WIDTH, I can certainly use it, but should we also
> > update FIELD_MAX and FIELD_FIT for consistency?
>
> All of the calls with the 0ULL placeholder (especially for the register)
> should really be removed.
> Last time I looked there where some calls that only had placeholders.
> They just bloat the pre-processor output and slow down compilation.
Have you any numbers? Can you send a patch?
> > I intend to send a v2 with the following implementation:
> >
> > #define __bf_shf(x) (__builtin_ffsll(x) - 1)
> > +#define __bf_hweight(x) __builtin_popcountll(x)
> >
> > #define __scalar_type_to_unsigned_cases(type) \
> > unsigned type: (unsigned type)0, \
> > @@ -111,6 +112,19 @@
> > (typeof(_mask))((_mask) >> __bf_shf(_mask)); \
> > })
> >
> > +/**
> > + * FIELD_WIDTH() - return the width of a bitfield
> > + * @_mask: shifted mask defining the field's length and position
> > + *
> > + * Returns the number of contiguous bits covered by @_mask.
> > + * This corresponds to the bit width of FIELD_MAX(@_mask).
> > + */
> > +#define FIELD_WIDTH(_mask) \
> > + ({ \
>
> You ought to have:
> auto _fw_mask = mask;
> here. While _mask has to be a constant, if it comes from GENMASK()
> it is very long.
Yes, but what this _fw means? Here it could be just auto __mask = mask.
That is what the underscores are used.
> > + __BF_FIELD_CHECK_MASK(_mask, 0ULL, "FIELD_WIDTH: "); \
> > + (typeof(_mask))__bf_hweight(_mask); \
>
> Why the cast of the result?
> They are everywhere in that file, and many are pointless.
> But there is no point adding another one.
>
> I'm not even sure you need the extra define.
> Just use __builtin_popcountll().
For __BF_FIELD_CHECK_MASK() check, I guess.
^ permalink raw reply
* Re: "Dead loop on virtual device" error without softirq-BKL on PREEMPT_RT
From: Sebastian Andrzej Siewior @ 2026-04-02 13:46 UTC (permalink / raw)
To: Daniel Vacek
Cc: edumazet, kuba, linux-kernel, linux-rt-devel, netdev, spasswolf,
tglx, Aaron Tomlin
In-Reply-To: <CAPjX3Ff8Zv2EArKaCSKFAAFfBCCVu0YgqHk3SV85BBPm6-D7HQ@mail.gmail.com>
On 2026-04-02 11:21:20 [+0200], Daniel Vacek wrote:
>
> Well, that's the patch I originally sent then.
>
> https://lore.kernel.org/linux-rt-devel/20260318103009.2120920-1-neelx@suse.com/
That was an alternative after everything was already done.
> --nX
Sebastian
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: macb: implement context swapping
From: Théo Lebrun @ 2026-04-02 13:46 UTC (permalink / raw)
To: Nicolai Buchwitz, Théo Lebrun
Cc: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Richard Cochran,
Russell King, Paolo Valerio, Conor Dooley, Vladimir Kondratiev,
Gregory CLEMENT, Benoît Monin, Tawfik Bayouk,
Thomas Petazzoni, Maxime Chevallier, netdev, linux-kernel
In-Reply-To: <f399076d1791d9df325f967e50184c2e@tipi-net.de>
Hello Nicolai,
On Thu Apr 2, 2026 at 1:35 PM CEST, Nicolai Buchwitz wrote:
> On 1.4.2026 18:39, Théo Lebrun wrote:
>> MACB has a pretty primitive approach to buffer management. They are all
>> stored in `struct macb *bp`. On operations that require buffer realloc
>> (set_ringparam & change_mtu ATM), the only option is to close the
>> interface, change our global state and re-open the interface.
>>
>> Two issues:
>> - It doesn't fly on memory pressured systems; we free our precious
>> buffers and don't manage to reallocate fully, meaning our machine
>> just lost its network access.
>> - Anecdotally, it is pretty slow because it implies a full PHY reinit.
>>
>> Instead, we shall:
>> - allocate a new context (including buffers) first
>> - if it fails, early return without any impact to the interface
>> - stop interface
>> - update global state (bp, netdev, etc)
>> - pass newly allocated buffer pointers to the hardware
>> - start interface
>> - free old context
>>
>> This is what we implement here. Both .set_ringparam() and
>> .ndo_change_mtu() are covered by this series. In the future,
>> at least .set_channels() [0], XDP [1] and XSK [2] would benefit.
>
> Thanks for your work, the context swapping approach probably
> makes a lot of sense and will finally bring proper MTU change
> support that I tried to patch earlier.
Thanks for the review!
>> The change is super intrusive so conflicts will be major. Sorry!
>>
>> Thanks,
>> Have a nice day,
>> Théo
>>
>> [0]:
>> https://lore.kernel.org/netdev/20260317-macb-set-channels-v4-0-1bd4f4ffcfca@bootlin.com/
>> [1]:
>> https://lore.kernel.org/netdev/20260323221047.2749577-1-pvalerio@redhat.com/
>> [2]:
>> https://lore.kernel.org/netdev/20260304-macb-xsk-v1-0-ba2ebe2bdaa3@bootlin.com/
>>
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> Théo Lebrun (11):
>> net: macb: unify device pointer naming convention
>> net: macb: unify `struct macb *` naming convention
>> net: macb: unify queue index variable naming convention and types
>> net: macb: enforce reverse christmas tree (RCT) convention
>> net: macb: allocate tieoff descriptor once across device lifetime
>> net: macb: introduce macb_context struct for buffer management
>> net: macb: avoid macb_init_rx_buffer_size() modifying state
>> net: macb: make `struct macb` subset reachable from macb_context
>> struct
>> net: macb: introduce macb_context_alloc() helper
>> net: macb: use context swapping in .set_ringparam()
>> net: macb: use context swapping in .ndo_change_mtu()
>>
>> drivers/net/ethernet/cadence/macb.h | 119 +-
>> drivers/net/ethernet/cadence/macb_main.c | 1731
>> +++++++++++++++++-------------
>> drivers/net/ethernet/cadence/macb_pci.c | 46 +-
>> drivers/net/ethernet/cadence/macb_ptp.c | 26 +-
>> 4 files changed, 1090 insertions(+), 832 deletions(-)
>> ---
>> base-commit: 321d1ee521de1362c22adadbc0ce066050a17783
>
> The series didn't apply cleanly on current net-next. The
> base commit 321d1ee521de doesn't seem to be upstream yet, is
> this based on your set_channels v4 series?
Surprising. I fetched net-next/main yesterday morning. My branch is:
- net-next/main @ f1359c240191
- my three series needed for working networking on MACB [0][1][2]
- some dev defconfigs
- finally the series sent upstream (b4 cover letter then series)
I confirm it applies on f1359c240191 but not on today's
net-next/main @ 269389ba5398.
I should experiment with b4 series dependency management. IIUC it would
have exposed through public metadata that my parent commit was
f1359c240191 even if it isn't strictly true locally.
We happen to add macb_{alloc,free}_tieoff() just above
at91_default_usrio which got edited by Conor in cee10a01e286 ("net:
macb: fix use of at91_default_usrio without CONFIG_OF"). Will fix in V2.
Thanks,
[0]: https://lore.kernel.org/all/20260225-macb-phy-v7-0-665bd8619d51@bootlin.com/
[1]: https://lore.kernel.org/all/20260225-macb-phy-v7-0-d3c9842ec931@bootlin.com/
[2]: https://lore.kernel.org/linux-phy/20260309-macb-phy-v9-0-5afd87d9db43@bootlin.com/
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [net-next PATCH 04/10] bitfield.h: add FIELD_WIDTH()
From: Yury Norov @ 2026-04-02 13:45 UTC (permalink / raw)
To: Luiz Angelo Daros de Luca
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Linus Walleij,
Alvin Šipraga, Yury Norov, Rasmus Villemoes, Russell King,
netdev, linux-kernel
In-Reply-To: <CAJq09z7+cA9_1fWzkrJebutJD0b1BGhrC1sUyoqqb=ddKCAXcw@mail.gmail.com>
On Thu, Apr 02, 2026 at 01:00:20AM -0300, Luiz Angelo Daros de Luca wrote:
> > > +/**
> > > + * FIELD_WIDTH() - return the width of a bitfield
> > > + * @_mask: shifted mask defining the field's length and position
> > > + *
> > > + * Returns the number of contiguous bits covered by @_mask.
> > > + * This corresponds to the bit width of FIELD_MAX(@_mask).
> > > + */
> > > +#define FIELD_WIDTH(_mask) \
> >
> > Please no underscored names unless necessary.
>
> I used _mask to maintain consistency with the existing public macros
> in this file, such as FIELD_GET, FIELD_PREP, FIELD_MAX, and FIELD_FIT.
> All of them use the underscore prefix for parameters. Should I diverge
> from them?
There's no consistency, but if you look at the recently added code,
you'll find it doesn't add those underscored prefixes. If you want
them, it's OK. Just explain what the hell do they mean?
> > > + ({ \
> > > + __BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_WIDTH: "); \
> > > + __bf_shf(~FIELD_MAX(_mask)); \
> > > + })
> >
> > I believe, this should be:
> >
> > #define FIELD_WIDTH(mask) ({ \
> > __BF_FIELD_CHECK_MASK(mask, 0ULL, "FIELD_WIDTH: "); \
> > HWEIGHT(mask); \
> > })
>
> HWEIGHT() is indeed much cleaner. However, to keep bitfield.h
> self-contained and avoid adding more includes, I'll try
> __builtin_popcountll() in a similar way __builtin_ffsll is already
> used.
>
> I also noticed the suggestion to use __BF_FIELD_CHECK_MASK instead of
> __BF_FIELD_CHECK. Both FIELD_MAX and FIELD_FIT currently use the full
> __BF_FIELD_CHECK with 0ULL as a dummy register to ensure the mask fits
> within the header's supported limits. If __BF_FIELD_CHECK_MASK is
> preferred for FIELD_WIDTH, I can certainly use it, but should we also
> update FIELD_MAX and FIELD_FIT for consistency?
>
> I intend to send a v2 with the following implementation:
>
> #define __bf_shf(x) (__builtin_ffsll(x) - 1)
> +#define __bf_hweight(x) __builtin_popcountll(x)
So, we've got only 2 __bf_xxx() functions here:
#define __bf_shf(x) (__builtin_ffsll(x) - 1)
#define __bf_cast_unsigned(type, x) ((__unsigned_scalar_typeof(type))(x))
They both do something with the builtins. Your __bf_hweight() is a
pure redefinition. What for do you need it? Why not just use that
compiler-provided popcount() that everybody knows?
> #define __scalar_type_to_unsigned_cases(type) \
> unsigned type: (unsigned type)0, \
> @@ -111,6 +112,19 @@
> (typeof(_mask))((_mask) >> __bf_shf(_mask)); \
> })
>
> +/**
> + * FIELD_WIDTH() - return the width of a bitfield
> + * @_mask: shifted mask defining the field's length and position
> + *
> + * Returns the number of contiguous bits covered by @_mask.
> + * This corresponds to the bit width of FIELD_MAX(@_mask).
> + */
> +#define FIELD_WIDTH(_mask) \
> + ({ \
> + __BF_FIELD_CHECK_MASK(_mask, 0ULL, "FIELD_WIDTH: "); \
> + (typeof(_mask))__bf_hweight(_mask); \
No need to typecast the result.
> + })
> +
>
> Regards,
>
> Luiz
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: macb: Consolidate MACB_CAPS_ISR_CLEAR_ON_WRITE checks in IRQ handler
From: Kevin Hao @ 2026-04-02 13:44 UTC (permalink / raw)
To: Nicolai Buchwitz
Cc: Jakub Kicinski, Nicolas Ferre, Claudiu Beznea, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, netdev
In-Reply-To: <e113d9e0ad3d3b63bfe124509c4af851@tipi-net.de>
[-- Attachment #1: Type: text/plain, Size: 665 bytes --]
On Wed, Apr 01, 2026 at 01:49:28PM +0200, Nicolai Buchwitz wrote:
>
> I'm always a fan of optimizations, but I guess in this case the
> saved ldrb is negligible next to the MMIO in the same path. We're
> talking a single L1 cache hit (~1ns) vs an uncacheable register
> write (~100ns+). Patch 3 also re-reads bp->caps in
> macb_interrupt_misc() anyway, undoing the local bool for the misc
> path.
>
> I'd ack Jakub's helper approach. A macb_queue_isr_clear() would
> be consistent across all callsites, including the 7 other
> instances you counted outside macb_interrupt().
Fair enough. I have used macb_queue_isr_clear() in v2.
Thanks,
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH net-next v2 4/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-04-02 13:41 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev, Nicolai Buchwitz
In-Reply-To: <20260402-macb-irq-v2-0-942d98ab1154@gmail.com>
In the current implementation, the suspend/resume path frees the
existing IRQ handler and sets up a dedicated WoL IRQ handler, then
restores the original handler upon resume. This approach is not used
by any other Ethernet driver and unnecessarily complicates the
suspend/resume process. After adjusting the IRQ handler in the previous
patches, we can now handle WoL interrupts without introducing any
overhead in the TX/RX hot path. Therefore, the dedicated WoL IRQ
handler is removed.
I have verified WoL functionality on my AMD ZynqMP board using the
following steps:
root@amd-zynqmp:~# ifconfig end0 192.168.3.3
root@amd-zynqmp:~# ethtool -s end0 wol a
root@amd-zynqmp:~# echo mem >/sys/power/state
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb_main.c | 112 ++++++++-----------------------
1 file changed, 27 insertions(+), 85 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 98da191a2428b471ff9ac54e1f24beb3a882c546..a4961abb95ae7fa2e40e61b5a8f0da5a83572131 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -71,7 +71,8 @@ struct sifive_fu540_macb_mgmt {
| MACB_BIT(TXUBR))
#define MACB_INT_MISC_FLAGS (MACB_TX_ERR_FLAGS | MACB_BIT(RXUBR) | \
- MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP))
+ MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP) | \
+ GEM_BIT(WOL) | MACB_BIT(WOL))
/* Max length of transmit frame must be a multiple of 8 bytes */
#define MACB_TX_LEN_ALIGN 8
@@ -2025,60 +2026,30 @@ static void macb_hresp_error_task(struct work_struct *work)
netif_tx_start_all_queues(dev);
}
-static irqreturn_t macb_wol_interrupt(int irq, void *dev_id)
+static void macb_wol_interrupt(struct macb_queue *queue, u32 status)
{
- struct macb_queue *queue = dev_id;
struct macb *bp = queue->bp;
- u32 status;
- status = queue_readl(queue, ISR);
-
- if (unlikely(!status))
- return IRQ_NONE;
-
- spin_lock(&bp->lock);
-
- if (status & MACB_BIT(WOL)) {
- queue_writel(queue, IDR, MACB_BIT(WOL));
- macb_writel(bp, WOL, 0);
- netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
- (unsigned int)(queue - bp->queues),
- (unsigned long)status);
- macb_queue_isr_clear(bp, queue, MACB_BIT(WOL));
- pm_wakeup_event(&bp->pdev->dev, 0);
- }
-
- spin_unlock(&bp->lock);
-
- return IRQ_HANDLED;
+ queue_writel(queue, IDR, MACB_BIT(WOL));
+ macb_writel(bp, WOL, 0);
+ netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
+ (unsigned int)(queue - bp->queues),
+ (unsigned long)status);
+ macb_queue_isr_clear(bp, queue, MACB_BIT(WOL));
+ pm_wakeup_event(&bp->pdev->dev, 0);
}
-static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
+static void gem_wol_interrupt(struct macb_queue *queue, u32 status)
{
- struct macb_queue *queue = dev_id;
struct macb *bp = queue->bp;
- u32 status;
- status = queue_readl(queue, ISR);
-
- if (unlikely(!status))
- return IRQ_NONE;
-
- spin_lock(&bp->lock);
-
- if (status & GEM_BIT(WOL)) {
- queue_writel(queue, IDR, GEM_BIT(WOL));
- gem_writel(bp, WOL, 0);
- netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
- (unsigned int)(queue - bp->queues),
- (unsigned long)status);
- macb_queue_isr_clear(bp, queue, GEM_BIT(WOL));
- pm_wakeup_event(&bp->pdev->dev, 0);
- }
-
- spin_unlock(&bp->lock);
-
- return IRQ_HANDLED;
+ queue_writel(queue, IDR, GEM_BIT(WOL));
+ gem_writel(bp, WOL, 0);
+ netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
+ (unsigned int)(queue - bp->queues),
+ (unsigned long)status);
+ macb_queue_isr_clear(bp, queue, GEM_BIT(WOL));
+ pm_wakeup_event(&bp->pdev->dev, 0);
}
static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
@@ -2132,6 +2103,14 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
}
+ if (macb_is_gem(bp)) {
+ if (status & GEM_BIT(WOL))
+ gem_wol_interrupt(queue, status);
+ } else {
+ if (status & MACB_BIT(WOL))
+ macb_wol_interrupt(queue, status);
+ }
+
return 0;
}
@@ -6004,7 +5983,6 @@ static int __maybe_unused macb_suspend(struct device *dev)
unsigned long flags;
u32 tmp, ifa_local;
unsigned int q;
- int err;
if (!device_may_wakeup(&bp->dev->dev))
phy_exit(bp->phy);
@@ -6067,39 +6045,15 @@ static int __maybe_unused macb_suspend(struct device *dev)
/* write IP address into register */
tmp |= MACB_BFEXT(IP, ifa_local);
}
- spin_unlock_irqrestore(&bp->lock, flags);
- /* Change interrupt handler and
- * Enable WoL IRQ on queue 0
- */
- devm_free_irq(dev, bp->queues[0].irq, bp->queues);
if (macb_is_gem(bp)) {
- err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
- IRQF_SHARED, netdev->name, bp->queues);
- if (err) {
- dev_err(dev,
- "Unable to request IRQ %d (error %d)\n",
- bp->queues[0].irq, err);
- return err;
- }
- spin_lock_irqsave(&bp->lock, flags);
queue_writel(bp->queues, IER, GEM_BIT(WOL));
gem_writel(bp, WOL, tmp);
- spin_unlock_irqrestore(&bp->lock, flags);
} else {
- err = devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt,
- IRQF_SHARED, netdev->name, bp->queues);
- if (err) {
- dev_err(dev,
- "Unable to request IRQ %d (error %d)\n",
- bp->queues[0].irq, err);
- return err;
- }
- spin_lock_irqsave(&bp->lock, flags);
queue_writel(bp->queues, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, tmp);
- spin_unlock_irqrestore(&bp->lock, flags);
}
+ spin_unlock_irqrestore(&bp->lock, flags);
enable_irq_wake(bp->queues[0].irq);
}
@@ -6141,7 +6095,6 @@ static int __maybe_unused macb_resume(struct device *dev)
struct macb_queue *queue;
unsigned long flags;
unsigned int q;
- int err;
if (!device_may_wakeup(&bp->dev->dev))
phy_init(bp->phy);
@@ -6167,17 +6120,6 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_queue_isr_clear(bp, bp->queues, -1);
spin_unlock_irqrestore(&bp->lock, flags);
- /* Replace interrupt handler on queue 0 */
- devm_free_irq(dev, bp->queues[0].irq, bp->queues);
- err = devm_request_irq(dev, bp->queues[0].irq, macb_interrupt,
- IRQF_SHARED, netdev->name, bp->queues);
- if (err) {
- dev_err(dev,
- "Unable to request IRQ %d (error %d)\n",
- bp->queues[0].irq, err);
- return err;
- }
-
disable_irq_wake(bp->queues[0].irq);
/* Now make sure we disable phy before moving
--
2.53.0
^ permalink raw reply related
* [PATCH net-next v2 3/4] net: macb: Factor out the handling of non-hot IRQ events into a separate function
From: Kevin Hao @ 2026-04-02 13:41 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev, Nicolai Buchwitz
In-Reply-To: <20260402-macb-irq-v2-0-942d98ab1154@gmail.com>
In the current code, the IRQ handler checks each IRQ event sequentially.
Since most IRQ events are related to TX/RX operations, while other
events occur infrequently, this approach introduces unnecessary overhead
in the hot path for TX/RX processing. This patch reduces such overhead
by extracting the handling of all non-TX/RX events into a new function
and consolidating these events under a new flag. As a result, only a
single check is required to determine whether any non-TX/RX events have
occurred. If such events exist, the handler jumps to the new function.
This optimization reduces four conditional checks to one and prevents
the instruction cache from being polluted with rarely used code in the
hot path.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb_main.c | 103 ++++++++++++++++++-------------
1 file changed, 61 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index aa25bfed67b67b1dc059e7aefcfb351bc298b3b1..98da191a2428b471ff9ac54e1f24beb3a882c546 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -70,6 +70,9 @@ struct sifive_fu540_macb_mgmt {
#define MACB_TX_INT_FLAGS (MACB_TX_ERR_FLAGS | MACB_BIT(TCOMP) \
| MACB_BIT(TXUBR))
+#define MACB_INT_MISC_FLAGS (MACB_TX_ERR_FLAGS | MACB_BIT(RXUBR) | \
+ MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP))
+
/* Max length of transmit frame must be a multiple of 8 bytes */
#define MACB_TX_LEN_ALIGN 8
#define MACB_MAX_TX_LEN ((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
@@ -2078,12 +2081,66 @@ static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
+{
+ struct macb *bp = queue->bp;
+ struct net_device *dev;
+ u32 ctrl;
+
+ dev = bp->dev;
+
+ if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
+ queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
+ schedule_work(&queue->tx_error_task);
+ macb_queue_isr_clear(bp, queue, MACB_TX_ERR_FLAGS);
+ return -1;
+ }
+
+ /* Link change detection isn't possible with RMII, so we'll
+ * add that if/when we get our hands on a full-blown MII PHY.
+ */
+
+ /* There is a hardware issue under heavy load where DMA can
+ * stop, this causes endless "used buffer descriptor read"
+ * interrupts but it can be cleared by re-enabling RX. See
+ * the at91rm9200 manual, section 41.3.1 or the Zynq manual
+ * section 16.7.4 for details. RXUBR is only enabled for
+ * these two versions.
+ */
+ if (status & MACB_BIT(RXUBR)) {
+ ctrl = macb_readl(bp, NCR);
+ macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+ wmb();
+ macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(RXUBR));
+ }
+
+ if (status & MACB_BIT(ISR_ROVR)) {
+ /* We missed at least one packet */
+ spin_lock(&bp->stats_lock);
+ if (macb_is_gem(bp))
+ bp->hw_stats.gem.rx_overruns++;
+ else
+ bp->hw_stats.macb.rx_overruns++;
+ spin_unlock(&bp->stats_lock);
+ macb_queue_isr_clear(bp, queue, MACB_BIT(ISR_ROVR));
+ }
+
+ if (status & MACB_BIT(HRESP)) {
+ queue_work(system_bh_wq, &bp->hresp_err_bh_work);
+ netdev_err(dev, "DMA bus error: HRESP not OK\n");
+ macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
+ }
+
+ return 0;
+}
+
static irqreturn_t macb_interrupt(int irq, void *dev_id)
{
struct macb_queue *queue = dev_id;
struct macb *bp = queue->bp;
struct net_device *dev = bp->dev;
- u32 status, ctrl;
+ u32 status;
status = queue_readl(queue, ISR);
@@ -2129,48 +2186,10 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
napi_schedule(&queue->napi_tx);
}
- if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
- queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
- schedule_work(&queue->tx_error_task);
- macb_queue_isr_clear(bp, queue, MACB_TX_ERR_FLAGS);
- break;
- }
+ if (unlikely(status & MACB_INT_MISC_FLAGS))
+ if (macb_interrupt_misc(queue, status))
+ break;
- /* Link change detection isn't possible with RMII, so we'll
- * add that if/when we get our hands on a full-blown MII PHY.
- */
-
- /* There is a hardware issue under heavy load where DMA can
- * stop, this causes endless "used buffer descriptor read"
- * interrupts but it can be cleared by re-enabling RX. See
- * the at91rm9200 manual, section 41.3.1 or the Zynq manual
- * section 16.7.4 for details. RXUBR is only enabled for
- * these two versions.
- */
- if (status & MACB_BIT(RXUBR)) {
- ctrl = macb_readl(bp, NCR);
- macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
- wmb();
- macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
- macb_queue_isr_clear(bp, queue, MACB_BIT(RXUBR));
- }
-
- if (status & MACB_BIT(ISR_ROVR)) {
- /* We missed at least one packet */
- spin_lock(&bp->stats_lock);
- if (macb_is_gem(bp))
- bp->hw_stats.gem.rx_overruns++;
- else
- bp->hw_stats.macb.rx_overruns++;
- spin_unlock(&bp->stats_lock);
- macb_queue_isr_clear(bp, queue, MACB_BIT(ISR_ROVR));
- }
-
- if (status & MACB_BIT(HRESP)) {
- queue_work(system_bh_wq, &bp->hresp_err_bh_work);
- netdev_err(dev, "DMA bus error: HRESP not OK\n");
- macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
- }
status = queue_readl(queue, ISR);
}
--
2.53.0
^ permalink raw reply related
* [PATCH net-next v2 2/4] net: macb: Introduce macb_queue_isr_clear() helper function
From: Kevin Hao @ 2026-04-02 13:41 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev, Nicolai Buchwitz
In-Reply-To: <20260402-macb-irq-v2-0-942d98ab1154@gmail.com>
The current implementation includes several occurrences of the
following pattern:
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, value);
Introduces a helper function to consolidate these repeated code
segments. No functional changes are made.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb.h | 7 +++++
drivers/net/ethernet/cadence/macb_main.c | 51 ++++++++++----------------------
2 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 16527dbab875dc2f65c74987c54b5b323a0c252c..2de56017ee0d6152dbe2c4ad0d8d7a8dfadbb3d5 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1474,6 +1474,13 @@ static inline bool macb_dma_ptp(struct macb *bp)
bp->caps & MACB_CAPS_DMA_PTP;
}
+static inline void macb_queue_isr_clear(struct macb *bp,
+ struct macb_queue *queue, u32 value)
+{
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, value);
+}
+
/**
* struct macb_platform_data - platform data for MACB Ethernet used for PCI registration
* @pclk: platform clock
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6bfba329bc9f1c918295d44ebfa38eefe209b648..aa25bfed67b67b1dc059e7aefcfb351bc298b3b1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1887,8 +1887,7 @@ static int macb_rx_poll(struct napi_struct *napi, int budget)
*/
if (macb_rx_pending(queue)) {
queue_writel(queue, IDR, bp->rx_intr_mask);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(RCOMP));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(RCOMP));
netdev_vdbg(bp->dev, "poll: packets pending, reschedule\n");
napi_schedule(napi);
}
@@ -1975,8 +1974,7 @@ static int macb_tx_poll(struct napi_struct *napi, int budget)
*/
if (macb_tx_complete_pending(queue)) {
queue_writel(queue, IDR, MACB_BIT(TCOMP));
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(TCOMP));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(TCOMP));
netdev_vdbg(bp->dev, "TX poll: packets pending, reschedule\n");
napi_schedule(napi);
}
@@ -2043,8 +2041,7 @@ static irqreturn_t macb_wol_interrupt(int irq, void *dev_id)
netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
(unsigned int)(queue - bp->queues),
(unsigned long)status);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(WOL));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(WOL));
pm_wakeup_event(&bp->pdev->dev, 0);
}
@@ -2072,8 +2069,7 @@ static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
(unsigned int)(queue - bp->queues),
(unsigned long)status);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, GEM_BIT(WOL));
+ macb_queue_isr_clear(bp, queue, GEM_BIT(WOL));
pm_wakeup_event(&bp->pdev->dev, 0);
}
@@ -2100,8 +2096,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
/* close possible race with dev_close */
if (unlikely(!netif_running(dev))) {
queue_writel(queue, IDR, -1);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, -1);
+ macb_queue_isr_clear(bp, queue, -1);
break;
}
@@ -2117,19 +2112,15 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
* now.
*/
queue_writel(queue, IDR, bp->rx_intr_mask);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(RCOMP));
-
+ macb_queue_isr_clear(bp, queue, MACB_BIT(RCOMP));
napi_schedule(&queue->napi_rx);
}
if (status & (MACB_BIT(TCOMP) |
MACB_BIT(TXUBR))) {
queue_writel(queue, IDR, MACB_BIT(TCOMP));
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(TCOMP) |
- MACB_BIT(TXUBR));
-
+ macb_queue_isr_clear(bp, queue, MACB_BIT(TCOMP) |
+ MACB_BIT(TXUBR));
if (status & MACB_BIT(TXUBR)) {
queue->txubr_pending = true;
wmb(); // ensure softirq can see update
@@ -2141,10 +2132,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
schedule_work(&queue->tx_error_task);
-
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
-
+ macb_queue_isr_clear(bp, queue, MACB_TX_ERR_FLAGS);
break;
}
@@ -2164,9 +2152,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
wmb();
macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
-
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(RXUBR));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(RXUBR));
}
if (status & MACB_BIT(ISR_ROVR)) {
@@ -2177,17 +2163,13 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
else
bp->hw_stats.macb.rx_overruns++;
spin_unlock(&bp->stats_lock);
-
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(ISR_ROVR));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(ISR_ROVR));
}
if (status & MACB_BIT(HRESP)) {
queue_work(system_bh_wq, &bp->hresp_err_bh_work);
netdev_err(dev, "DMA bus error: HRESP not OK\n");
-
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, MACB_BIT(HRESP));
+ macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
}
status = queue_readl(queue, ISR);
}
@@ -2883,8 +2865,7 @@ static void macb_reset_hw(struct macb *bp)
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
queue_writel(queue, IDR, -1);
queue_readl(queue, ISR);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, -1);
+ macb_queue_isr_clear(bp, queue, -1);
}
}
@@ -6053,8 +6034,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
/* Disable all interrupts */
queue_writel(queue, IDR, -1);
queue_readl(queue, ISR);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, -1);
+ macb_queue_isr_clear(bp, queue, -1);
}
/* Enable Receive engine */
macb_writel(bp, NCR, tmp | MACB_BIT(RE));
@@ -6165,8 +6145,7 @@ static int __maybe_unused macb_resume(struct device *dev)
}
/* Clear ISR on queue 0 */
queue_readl(bp->queues, ISR);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(bp->queues, ISR, -1);
+ macb_queue_isr_clear(bp, bp->queues, -1);
spin_unlock_irqrestore(&bp->lock, flags);
/* Replace interrupt handler on queue 0 */
--
2.53.0
^ permalink raw reply related
* [PATCH net-next v2 1/4] net: macb: Replace open-coded implementation with napi_schedule()
From: Kevin Hao @ 2026-04-02 13:41 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev, Nicolai Buchwitz
In-Reply-To: <20260402-macb-irq-v2-0-942d98ab1154@gmail.com>
The driver currently duplicates the logic of napi_schedule() primarily
to include additional debug information. However, these debug details
are not essential for a specific driver and can be effectively obtained
through existing tracepoints in the networking core, such as
/sys/kernel/tracing/events/napi/napi_poll. Therefore, this patch
replaces the open-coded implementation with napi_schedule() to
simplify the driver's code.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb_main.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7a48ebe0741f3b031d4c3c266cc0e565bab61211..6bfba329bc9f1c918295d44ebfa38eefe209b648 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2120,10 +2120,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
- if (napi_schedule_prep(&queue->napi_rx)) {
- netdev_vdbg(bp->dev, "scheduling RX softirq\n");
- __napi_schedule(&queue->napi_rx);
- }
+ napi_schedule(&queue->napi_rx);
}
if (status & (MACB_BIT(TCOMP) |
@@ -2138,10 +2135,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
wmb(); // ensure softirq can see update
}
- if (napi_schedule_prep(&queue->napi_tx)) {
- netdev_vdbg(bp->dev, "scheduling TX softirq\n");
- __napi_schedule(&queue->napi_tx);
- }
+ napi_schedule(&queue->napi_tx);
}
if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
--
2.53.0
^ permalink raw reply related
* [PATCH net-next v2 0/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-04-02 13:41 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev, Nicolai Buchwitz
During debugging of a suspend/resume issue, I observed that the macb driver
employs a dedicated IRQ handler for Wake-on-LAN (WoL) support. To my knowledge,
no other Ethernet driver adopts this approach. This implementation unnecessarily
complicates the suspend/resume process without providing any clear benefit.
Instead, we can easily modify the existing IRQ handler to manage WoL events,
avoiding any overhead in the TX/RX hot path.
The net throughput shows no significant difference following these changes.
The following data(net throughput and execution time of macb_interrupt) were
collected from my AMD Zynqmp board using the commands:
taskset -c 1,2,3 iperf3 -c 192.168.3.4 -t 60 -Z -P 3 -R
cat /sys/kernel/debug/tracing/trace_stat/function0
Before:
-------
[SUM] 0.00-60.00 sec 5.99 GBytes 858 Mbits/sec 0 sender
[SUM] 0.00-60.00 sec 5.99 GBytes 857 Mbits/sec receiver
Function Hit Time Avg s^2
-------- --- ---- --- ---
macb_interrupt 217996 678425.2 us 3.112 us 1.446 us
After:
------
[SUM] 0.00-60.00 sec 6.00 GBytes 858 Mbits/sec 0 sender
[SUM] 0.00-60.00 sec 5.99 GBytes 857 Mbits/sec receiver
Function Hit Time Avg s^2
-------- --- ---- --- ---
macb_interrupt 218212 668107.3 us 3.061 us 1.413 us
---
Changes in v2:
- The patch 2 has been reimplemented as suggested by Jakub Kicinski.
- Adjust patches 3 and 4 to align with the reimplementation of patch 2.
- Update the commit log for patch 4 to include verification steps.
- Link to v1: https://lore.kernel.org/r/20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com
---
Kevin Hao (4):
net: macb: Replace open-coded implementation with napi_schedule()
net: macb: Introduce macb_queue_isr_clear() helper function
net: macb: Factor out the handling of non-hot IRQ events into a separate function
net: macb: Remove dedicated IRQ handler for WoL
drivers/net/ethernet/cadence/macb.h | 7 +
drivers/net/ethernet/cadence/macb_main.c | 250 ++++++++++++-------------------
2 files changed, 99 insertions(+), 158 deletions(-)
---
base-commit: bd0f139e5fc11182777b81cefc3893ea508544ec
change-id: 20260321-macb-irq-453ee09b3394
Best regards,
--
Kevin Hao <haokexin@gmail.com>
^ permalink raw reply
* Re: RFC: phylink: disable PHY autonomous EEE when MAC manages LPI
From: Andrew Lunn @ 2026-04-02 13:32 UTC (permalink / raw)
To: Russell King (Oracle); +Cc: Nicolai Buchwitz, netdev
In-Reply-To: <ac5h4C99Lh2ez7GN@shell.armlinux.org.uk>
> > Thanks both. So if I understand correctly:
> >
> > 1. Add a .disable_autonomous_eee callback to struct phy_driver
> >
> > 2. Call it from phy_support_eee() so MAC drivers don't need to know
> > or care whether the PHY does (autonomous|smart)eee
> >
> > 3. BCM54xx (AutogrEEEn) and RTL8211F as first users
> >
> > 4. No enable counterpart for now. PHY drivers maintain their current
> > default behavior unless phy_support_eee() is called.
> >
> > Sounds good. Sorting out the warts of (autonomous|smart)eee will be
> > iterative anyway, so starting with just the disable path makes sense
> > (and hopefully will make setups a bit less broken than they were with
> > both fighting for LPI).
> >
> > I'll put the RFC patches together.
>
> Sounds good to me, but I'd wait for Andrew's input.
Yes, this sounds good, but lets see the patches.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts
From: Andrew Lunn @ 2026-04-02 13:31 UTC (permalink / raw)
To: Charles Perry
Cc: netdev, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
In-Reply-To: <20260402131229.319599-4-charles.perry@microchip.com>
> +static int phy_write_barrier(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_read(phydev, MII_PHYSID1);
> + if (err < 0)
> + return err;
There are a small number of MDIO busses which don't implement C22,
only C45. You are likely to get -EIO or maybe ENODEV, -EOPNOTSUPP,
-EINVAL for such a read. Returning the error than makes
phy_disable_interrupts() fail, etc.
This is why i suggested throwing away the return value.
Maybe call phydev_info() if there is an error, but i suggest making
this a void function.
Andrew
---
pw-bot: cr
^ permalink raw reply
* [PATCH v1 07/22] dt-bindings: clock: Add StarFive JHB100 System-1 clock and reset generator
From: Changhuang Liang @ 2026-04-02 10:55 UTC (permalink / raw)
To: Michael Turquette, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Stephen Boyd, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Philipp Zabel, Emil Renner Berthing, Kees Cook,
Gustavo A . R . Silva, Richard Cochran
Cc: linux-clk, linux-kernel, devicetree, linux-riscv, linux-hardening,
netdev, Sia Jee Heng, Hal Feng, Ley Foon Tan, Changhuang Liang
In-Reply-To: <20260402105523.447523-1-changhuang.liang@starfivetech.com>
Add bindings for the System-1 clocks and reset generator (SYS1CRG) on
JHB100 SoC.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
.../clock/starfive,jhb100-sys1crg.yaml | 71 +++++++++++++++++++
.../dt-bindings/clock/starfive,jhb100-crg.h | 20 ++++++
.../dt-bindings/reset/starfive,jhb100-crg.h | 13 ++++
3 files changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/starfive,jhb100-sys1crg.yaml
diff --git a/Documentation/devicetree/bindings/clock/starfive,jhb100-sys1crg.yaml b/Documentation/devicetree/bindings/clock/starfive,jhb100-sys1crg.yaml
new file mode 100644
index 000000000000..0cfeb8400b58
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/starfive,jhb100-sys1crg.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/starfive,jhb100-sys1crg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JHB100 System-1 Clock and Reset Generator
+
+maintainers:
+ - Changhuang Liang <changhuang.liang@starfivetech.com>
+
+properties:
+ compatible:
+ const: starfive,jhb100-sys1crg
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Main Oscillator (25 MHz)
+ - description: PLL0
+ - description: PLL1
+ - description: PLL2
+ - description: PLL4
+ - description: PLL5
+ - description: System-1 NPU 600MHz
+
+ clock-names:
+ items:
+ - const: osc
+ - const: pll0
+ - const: pll1
+ - const: pll2
+ - const: pll4
+ - const: pll5
+ - const: sys1_npu_600
+
+ '#clock-cells':
+ const: 1
+ description:
+ See <dt-bindings/clock/starfive,jhb100-crg.h> for valid indices.
+
+ '#reset-cells':
+ const: 1
+ description:
+ See <dt-bindings/reset/starfive-jhb100-crg.h> for valid indices.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - '#clock-cells'
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ clock-controller@13004000 {
+ compatible = "starfive,jhb100-sys1crg";
+ reg = <0x13004000 0x4000>;
+ clocks = <&osc>, <&pll0>, <&pll1>,
+ <&syspll 0>, <&syspll 2>,
+ <&syspll 3>, <&sys0crg 61>;
+ clock-names = "osc", "pll0", "pll1", "pll2",
+ "pll4", "pll5", "sys1_npu_600";
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
diff --git a/include/dt-bindings/clock/starfive,jhb100-crg.h b/include/dt-bindings/clock/starfive,jhb100-crg.h
index b257cd104a10..510a5c6fa89a 100644
--- a/include/dt-bindings/clock/starfive,jhb100-crg.h
+++ b/include/dt-bindings/clock/starfive,jhb100-crg.h
@@ -53,4 +53,24 @@
#define JHB100_SYS0CLK_GPU0_600 73
#define JHB100_SYS0CLK_GPU1_600 74
+/* SYS1CRG clocks */
+#define JHB100_SYS1CLK_APB_MAIN_SYS1 0
+#define JHB100_SYS1CLK_APB_SENSOR_ICG_BUF 1
+
+#define JHB100_SYS1CLK_GPIO_ESPI1_66 5
+
+#define JHB100_SYS1CLK_HOSTSS1_100 7
+#define JHB100_SYS1CLK_HOSTSS1_PHY_SCAN_1000_ICG_BUF 8
+#define JHB100_SYS1CLK_NPU_200 9
+#define JHB100_SYS1CLK_NPU_CORE_DIV 10
+#define JHB100_SYS1CLK_DOM_NPU_CORE_CLK 11
+#define JHB100_SYS1CLK_DOM_NPU_BUS_CLK 12
+#define JHB100_SYS1CLK_DOM_NPU_INIT_CLK 13
+#define JHB100_SYS1CLK_DOM_NPU_OSC_CLK 14
+#define JHB100_SYS1CLK_VOUT_100 15
+#define JHB100_SYS1CLK_VOUT_PIX0 16
+#define JHB100_SYS1CLK_VOUT_PIX1 17
+#define JHB100_SYS1CLK_BMCPER3_100 18
+#define JHB100_SYS1CLK_BMCPER3_125 19
+
#endif /* __DT_BINDINGS_CLOCK_STARFIVE_JHB100_H__ */
diff --git a/include/dt-bindings/reset/starfive,jhb100-crg.h b/include/dt-bindings/reset/starfive,jhb100-crg.h
index 71affdcdf733..9a0ab64abafa 100644
--- a/include/dt-bindings/reset/starfive,jhb100-crg.h
+++ b/include/dt-bindings/reset/starfive,jhb100-crg.h
@@ -27,4 +27,17 @@
#define JHB100_SYS0RST_BMCUSB_RSTN_BUS 23
#define JHB100_SYS0RST_BMCUSB_RSTN_CRG 24
+/* SYS1CRG resets */
+#define JHB100_SYS1RST_SYS1_IOMUX_PRESETN 1
+
+#define JHB100_SYS1RST_MAIN_RSTN_CHIPTOP_SENSOR 5
+
+#define JHB100_SYS1RST_VOUT_RSTN_HOST0 8
+#define JHB100_SYS1RST_VOUT_RSTN_HOST1 9
+#define JHB100_SYS1RST_HOSTSS1_RSTN_BUS_ESPI 10
+#define JHB100_SYS1RST_HOSTSS1_RSTN_BUS_PCIE 11
+#define JHB100_SYS1RST_HOSTSS1_RSTN_CRG 12
+#define JHB100_SYS1RST_BMCPERIPH3_RSTN_CRG 13
+#define JHB100_SYS1RST_BMCPERIPH3_RSTN_BUS 14
+
#endif /* __DT_BINDINGS_RESET_STARFIVE_JHB100_CRG_H__ */
--
2.25.1
^ permalink raw reply related
* RE: [PATCH net-next] r8152: Add helper functions for SRAM2
From: Hayes Wang @ 2026-04-02 13:30 UTC (permalink / raw)
To: Chih Kai Hsu, kuba@kernel.org, davem@davemloft.net
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, edumazet@google.com, bjorn@mork.no,
pabeni@redhat.com, Chih Kai Hsu
In-Reply-To: <20260401115542.34601-1-nic_swsd@realtek.com>
> From: Chih Kai Hsu <hsu.chih.kai@realtek.com>
> Sent: Wednesday, April 1, 2026 7:56 PM
> To: kuba@kernel.org; davem@davemloft.net
> Cc: netdev@vger.kernel.org; nic_swsd <nic_swsd@realtek.com>;
> linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org;
> edumazet@google.com; bjorn@mork.no; pabeni@redhat.com; Chih Kai Hsu
> <hsu.chih.kai@realtek.com>
> Subject: [PATCH net-next] r8152: Add helper functions for SRAM2
>
> Add the following helper functions for SRAM2 access to simplify the code and
> improve readability:
>
> - sram2_write() - write data to SRAM2 address
> - sram2_read() - read data from SRAM2 address
> - sram2_write_w0w1() - read-modify-write operation
>
> Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
Reviewed-by: Hayes Wang <hayeswang@realtek.com>
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
From: Stefano Garzarella @ 2026-04-02 13:28 UTC (permalink / raw)
To: Luigi Leonardi
Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Arseniy Krasnov, kvm, virtualization,
netdev, linux-kernel
In-Reply-To: <20260402-fix_peek-v1-2-ad274fcef77b@redhat.com>
On Thu, Apr 02, 2026 at 10:18:02AM +0200, Luigi Leonardi wrote:
>Add a test that verifies MSG_PEEK works correctly after a partial
>recv().
>
>This is to test a bug that was present in the `virtio_transport_stream_do_peek()`
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#11: This is to test a bug that was present in the
`virtio_transport_stream_do_peek()`
>when computing the number of bytes to copy: After a partial read, the
>peek function didn't take into consideration the number of bytes that
>were already read. So peeking the whole buffer would cause a out-of-bounds read,
>that resulted in a -EFAULT.
>
>This test does exactly this: do a partial recv on a buffer, then try to
>peek the whole buffer content.
>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
> return test_msg_peek_server(opts, false);
> }
>
>+#define PEEK_AFTER_RECV_LEN 100
Why 100 ?
Better to use a power of 2 IMO like we do in all other cases IIRC.
>+
>+static void test_stream_peek_after_recv_client(const struct test_opts *opts)
>+{
>+ unsigned char buf[PEEK_AFTER_RECV_LEN];
>+ int fd;
>+ int i;
nit: int fd, i;
>+
>+ fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>+ if (fd < 0) {
>+ perror("connect");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ for (i = 0; i < sizeof(buf); i++)
>+ buf[i] = (unsigned char)i;
Why setting the payload in this way ? Can we just do a memset() ?
>+
>+ control_expectln("SRVREADY");
Why we need this barrier ?
>+
>+ send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>+
>+ close(fd);
>+}
>+
>+static void test_stream_peek_after_recv_server(const struct test_opts *opts)
>+{
>+ unsigned char buf[PEEK_AFTER_RECV_LEN];
>+ int half = PEEK_AFTER_RECV_LEN / 2;
>+ ssize_t ret;
>+ int fd;
>+
>+ fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+ if (fd < 0) {
>+ perror("accept");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ control_writeln("SRVREADY");
>+
>+ /* Partial recv to advance offset within the skb */
>+ recv_buf(fd, buf, half, 0, half);
Why reading half of the size ?
IMO is better to read just 1 byte, since it is almost certain that an
skb does not have a 1-byte payload.
>+
>+ /* Try to peek more than what remains: should return only 'half'
How we are sure that the sender sent all the bytes ?
>+ * bytes. Note: we can't use recv_buf() because it loops until
>+ * all requested bytes are returned.
Why this is a problem ? (an useful comment should explain the reason)
>+ */
>+ ret = recv(fd, buf, sizeof(buf), MSG_PEEK);
>+ if (ret < 0) {
Should we handle EINTR like we do in recv_buf() ?
But I still don't understand why we can't use it directly.
Thanks,
Stefano
>+ perror("recv");
>+ exit(EXIT_FAILURE);
>+ } else if (ret != half) {
>+ fprintf(stderr, "MSG_PEEK after partial recv returned %d (expected %d)\n",
>+ ret, half);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ close(fd);
>+}
>+
> #define SOCK_BUF_SIZE (2 * 1024 * 1024)
> #define SOCK_BUF_SIZE_SMALL (64 * 1024)
> #define MAX_MSG_PAGES 4
>@@ -2520,6 +2579,11 @@ static struct test_case test_cases[] = {
> .run_client = test_stream_tx_credit_bounds_client,
> .run_server = test_stream_tx_credit_bounds_server,
> },
>+ {
>+ .name = "SOCK_STREAM MSG_PEEK after partial recv",
>+ .run_client = test_stream_peek_after_recv_client,
>+ .run_server = test_stream_peek_after_recv_server,
>+ },
> {},
> };
>
>
>--
>2.53.0
>
^ permalink raw reply
* Re: [PATCH net-next] net: airoha: Set REG_RX_CPU_IDX() once in airoha_qdma_fill_rx_queue()
From: patchwork-bot+netdevbpf @ 2026-04-02 13:20 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260331-airoha-cpu-idx-out-off-loop-v1-1-75c66b428f50@kernel.org>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 31 Mar 2026 12:33:24 +0200 you wrote:
> It is not necessary to update REG_RX_CPU_IDX register for each iteration
> of the descriptor loop in airoha_qdma_fill_rx_queue routine.
> Move REG_RX_CPU_IDX configuration out of the descriptor loop and rely on
> the last queue head value updated in the descriptor loop.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>
> [...]
Here is the summary with links:
- [net-next] net: airoha: Set REG_RX_CPU_IDX() once in airoha_qdma_fill_rx_queue()
https://git.kernel.org/netdev/net-next/c/269389ba5398
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 v3 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: patchwork-bot+netdevbpf @ 2026-04-02 13:20 UTC (permalink / raw)
To: Xiang Mei
Cc: netdev, jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs
In-Reply-To: <20260331050217.504278-1-xmei5@asu.edu>
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 30 Mar 2026 22:02:15 -0700 you wrote:
> The old-method path in fw_classify() calls tcf_block_q() and
> dereferences q->handle. Shared blocks leave block->q NULL, causing a
> NULL deref when an empty cls_fw filter is attached to a shared block
> and a packet with a nonzero major skb mark is classified.
>
> Reject the configuration in fw_change() when the old method (no
> TCA_OPTIONS) is used on a shared block, since fw_classify()'s
> old-method path needs block->q which is NULL for shared blocks.
>
> [...]
Here is the summary with links:
- [net,v3,1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
https://git.kernel.org/netdev/net/c/faeea8bbf6e9
- [net,v3,2/3] net/sched: cls_flow: fix NULL pointer dereference on shared blocks
https://git.kernel.org/netdev/net/c/1a280dd4bd1d
- [net,v3,3/3] selftests/tc-testing: add tests for cls_fw and cls_flow on shared blocks
https://git.kernel.org/netdev/net/c/70f73562d278
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ 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