* Re: [PATCH] net/ftgmac100: fix ring allocation unwind leaks on open failure
From: Andrew Lunn @ 2026-03-28 14:37 UTC (permalink / raw)
To: Yufan Chen
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel, Yufan Chen
In-Reply-To: <20260328092428.75430-1-yufan.chen@linux.dev>
On Sat, Mar 28, 2026 at 05:24:28PM +0800, Yufan Chen wrote:
> From: Yufan Chen <ericterminal@gmail.com>
>
> ftgmac100_alloc_rings() allocated rx_skbs, tx_skbs, rxdes, txdes, and rx_scratch in stages but returned directly on any intermediate allocation failure. This left previously allocated objects unreleased and could accumulate leaks across repeated ifup retries under memory pressure.
Please take a read of
https://docs.kernel.org/process/submitting-patches.html
and
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
You have a number of process issues.
> static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
> {
> + int err = -ENOMEM;
> +
> /* Allocate skb arrays */
> priv->rx_skbs = kcalloc(MAX_RX_QUEUE_ENTRIES, sizeof(void *),
> GFP_KERNEL);
> if (!priv->rx_skbs)
> - return -ENOMEM;
> + goto out;
> priv->tx_skbs = kcalloc(MAX_TX_QUEUE_ENTRIES, sizeof(void *),
> GFP_KERNEL);
> if (!priv->tx_skbs)
> - return -ENOMEM;
> + goto out;
>
> /* Allocate descriptors */
> priv->rxdes = dma_alloc_coherent(priv->dev,
> MAX_RX_QUEUE_ENTRIES * sizeof(struct ftgmac100_rxdes),
> &priv->rxdes_dma, GFP_KERNEL);
> if (!priv->rxdes)
> - return -ENOMEM;
> + goto out;
> priv->txdes = dma_alloc_coherent(priv->dev,
> MAX_TX_QUEUE_ENTRIES * sizeof(struct ftgmac100_txdes),
> &priv->txdes_dma, GFP_KERNEL);
> if (!priv->txdes)
> - return -ENOMEM;
> + goto out;
>
> /* Allocate scratch packet buffer */
> priv->rx_scratch = dma_alloc_coherent(priv->dev,
> @@ -997,9 +1002,13 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
> &priv->rx_scratch_dma,
> GFP_KERNEL);
> if (!priv->rx_scratch)
> - return -ENOMEM;
> + goto out;
>
> return 0;
> +
> +out:
> + ftgmac100_free_rings(priv);
> + return err;
If you look at other drivers, you will notice that functions that do
allocations pretty much always do their own cleanup, rather than
calling a helper. You generally see multiple labels, one per
allocation, and gotos jumping to the needed code.
Andrew
---
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v2] net: stmmac: avoid repeated devm_gpiod_get_optional in mdio
From: Andrew Lunn @ 2026-03-28 14:31 UTC (permalink / raw)
To: Liu Xiang; +Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni
In-Reply-To: <20260328032520.7866-1-liu.xiang@zlingsmart.com>
On Sat, Mar 28, 2026 at 11:25:20AM +0800, Liu Xiang wrote:
> During system resume, stmmac_resume() calls stmmac_mdio_reset().
> This results in repeated calls to devm_gpiod_get_optional() for the same
> GPIO. The second invocation returns an error since the GPIO descriptor has
> already been obtained.
How is this different to v1?
> @@ -386,15 +386,15 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>
> #ifdef CONFIG_OF
> if (priv->device->of_node) {
> - struct gpio_desc *reset_gpio;
> u32 delays[3] = { 0, 0, 0 };
>
> - reset_gpio = devm_gpiod_get_optional(priv->device,
> - "snps,reset",
> - GPIOD_OUT_LOW);
> - if (IS_ERR(reset_gpio))
> - return PTR_ERR(reset_gpio);
> -
> + if (IS_ERR_OR_NULL(priv->reset_gpio)) {
> + priv->reset_gpio = devm_gpiod_get_optional(priv->device,
> + "snps,reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(priv->reset_gpio))
> + return PTR_ERR(priv->reset_gpio);
> + }
In general, a driver should get the resources it needs in probe, since
probe can only be successful once. So i suggest moving this into
stmmac_mdio_register() which is the equivalent of probe. To keep the
behaviour the same, pass GPIOD_ASIS, and then here in _reset set it
low.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Andrew Lunn @ 2026-03-28 14:15 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Eric Woudstra, Lucien.Jheng, hkallweit1, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn, frank-w, daniel,
lucien.jheng
In-Reply-To: <acesgklpv8GlNeVf@shell.armlinux.org.uk>
On Sat, Mar 28, 2026 at 10:25:06AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> > static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> > u32 pbus_data)
> > {
> > int ret;
> >
> > mutex_lock(&mdio->bus->mdio_lock);
>
> I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
> Should a helper be provided at mdio bus level to complement the PHY
> level helper if we're going to have MDIO drivers directly manipulating
> the lock?
DSA is pretty much the only class of mdio driver. And they mostly need
to use:
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
Not the plain mutex_lock().
Hence it has not been added up until now.
But yes, such a helper would make sense for a PHY driver having to do
odd things.
Andrew
^ permalink raw reply
* [PATCH] net: stmmac: dwmac-rk: Fix typo in comment
From: 谢致邦 (XIE Zhibang) @ 2026-03-28 13:43 UTC (permalink / raw)
To: linux-rockchip
Cc: 谢致邦 (XIE Zhibang), Heiko Stuebner,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
Russell King (Oracle), linux-arm-kernel, netdev, linux-stm32,
linux-kernel
Correct the typo "rk3520" to "rk3528" in comment.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index b0441a368cb1..8d7042e68926 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -214,7 +214,7 @@ static int rk_configure_io_clksel(struct rk_priv_data *bsp_priv)
cru = !io;
/* The io_clksel configuration can be either:
- * 0=CRU, 1=IO (rk3506, rk3520, rk3576) or
+ * 0=CRU, 1=IO (rk3506, rk3528, rk3576) or
* 0=IO, 1=CRU (rk3588)
* where CRU means the transmit clock comes from the CRU and IO
* means the transmit clock comes from IO.
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC 1/8] dt-bindings: clk: sun60i-a733-ccu: Add allwinner A733 support
From: Chen-Yu Tsai @ 2026-03-28 12:07 UTC (permalink / raw)
To: Junhui Liu
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jernej Skrabec, Samuel Holland, Philipp Zabel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Richard Cochran, linux-clk, devicetree, linux-arm-kernel,
linux-sunxi, linux-kernel, linux-riscv, netdev
In-Reply-To: <20260310-a733-clk-v1-1-36b4e9b24457@pigmoral.tech>
On Tue, Mar 10, 2026 at 4:42 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>
> The CCU and R-CCU (PRCM) modules provide clocks and reset functions for
> the Allwinner A733 SoC. The clock architecture of the A733 is evolved
> from the A523, though the root clocking strategy transitions from a
> static oscillator frequency in the Devicetree to the "hosc" clock, which
> is determined by choosing from three possible frequencies (19.2MHz,
> 24MHz, or 26MHz) by the RTC hardware, and finally feeds the CCU and
> R-CCU.
>
> Additionally, the MCU_CCU module found in previous designs is removed
> from the A733, and the clock tree is expanded with more clock outputs
> to support new functional modules.
>
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
> .../bindings/clock/allwinner,sun60i-a733-ccu.yaml | 107 ++++++++
> include/dt-bindings/clock/sun60i-a733-ccu.h | 289 +++++++++++++++++++++
> include/dt-bindings/clock/sun60i-a733-r-ccu.h | 39 +++
> include/dt-bindings/reset/sun60i-a733-ccu.h | 131 ++++++++++
> include/dt-bindings/reset/sun60i-a733-r-ccu.h | 23 ++
> 5 files changed, 589 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml
> new file mode 100644
> index 000000000000..aff3ff731285
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/allwinner,sun60i-a733-ccu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A733 Clock Control Unit
> +
> +maintainers:
> + - Junhui Liu <junhui.liu@pigmoral.tech>
> +
> +properties:
> + "#clock-cells":
> + const: 1
> +
> + "#reset-cells":
> + const: 1
> +
> + compatible:
> + enum:
> + - allwinner,sun60i-a733-ccu
> + - allwinner,sun60i-a733-r-ccu
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + minItems: 4
> + maxItems: 7
> +
> + clock-names:
> + minItems: 4
> + maxItems: 7
> +
> +required:
> + - "#clock-cells"
> + - "#reset-cells"
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun60i-a733-ccu
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: High Frequency Oscillator (19.2MHz, 24MHz, or 26MHz)
> + - description: Low Frequency Oscillator (usually at 32kHz)
> + - description: Internal Oscillator
> + - description: Low Frequency Oscillator fanout
> +
> + clock-names:
> + items:
> + - const: hosc
> + - const: losc
> + - const: iosc
> + - const: losc-fanout
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun60i-a733-r-ccu
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: High Frequency Oscillator (19.2MHz, 24MHz, or 26MHz)
> + - description: Low Frequency Oscillator (usually at 32kHz)
> + - description: Internal Oscillator
> + - description: System 24MHz Clock
> + - description: Peripherals PLL 0 (200 MHz output)
> + - description: Peripherals PLL 0 (300 MHz output)
> + - description: Peripherals PLL 1 (300 MHz output)
> +
> + clock-names:
> + items:
> + - const: hosc
> + - const: losc
> + - const: iosc
> + - const: sys-24m
> + - const: pll-periph0-200m
> + - const: pll-periph0-300m
> + - const: pll-periph1-300m
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + clock-controller@2002000 {
> + compatible = "allwinner,sun60i-a733-ccu";
> + reg = <0x02002000 0x2000>;
> + clocks = <&rtc 2>, <&rtc 1>, <&rtc 0>, <&rtc 4>;
> + clock-names = "hosc", "losc", "iosc", "losc-fanout";
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> +...
This part looks correct.
> diff --git a/include/dt-bindings/clock/sun60i-a733-ccu.h b/include/dt-bindings/clock/sun60i-a733-ccu.h
> new file mode 100644
> index 000000000000..1a98bea8ca9a
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun60i-a733-ccu.h
> @@ -0,0 +1,289 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_
> +
> +#define CLK_PLL_REF 0
> +#define CLK_SYS_24M 1
> +#define CLK_PLL_DDR 2
> +#define CLK_PLL_PERIPH0_4X 3
> +#define CLK_PLL_PERIPH0_2X 4
> +#define CLK_PLL_PERIPH0_800M 5
> +#define CLK_PLL_PERIPH0_480M 6
> +#define CLK_PLL_PERIPH0_600M 7
> +#define CLK_PLL_PERIPH0_400M 8
> +#define CLK_PLL_PERIPH0_300M 9
> +#define CLK_PLL_PERIPH0_200M 10
> +#define CLK_PLL_PERIPH0_160M 11
> +#define CLK_PLL_PERIPH0_150M 12
> +#define CLK_PLL_PERIPH1_4X 13
> +#define CLK_PLL_PERIPH1_2X 14
> +#define CLK_PLL_PERIPH1_800M 15
> +#define CLK_PLL_PERIPH1_480M 16
> +#define CLK_PLL_PERIPH1_600M 17
> +#define CLK_PLL_PERIPH1_400M 18
> +#define CLK_PLL_PERIPH1_300M 19
> +#define CLK_PLL_PERIPH1_200M 20
> +#define CLK_PLL_PERIPH1_160M 21
> +#define CLK_PLL_PERIPH1_150M 22
> +#define CLK_PLL_GPU 23
Please keep the unit number even if there is only one instance.
This is PLL_GPU0.
> +#define CLK_PLL_VIDEO0_8X 24
If this is the common parent for the 4X and 3X dividers, shouldn't it
be 12X?
> +#define CLK_PLL_VIDEO0_4X 25
> +#define CLK_PLL_VIDEO0_3X 26
> +#define CLK_PLL_VIDEO1_8X 27
Same here.
> +#define CLK_PLL_VIDEO1_4X 28
> +#define CLK_PLL_VIDEO1_3X 29
> +#define CLK_PLL_VIDEO2_8X 30
And here.
> +#define CLK_PLL_VIDEO2_4X 31
> +#define CLK_PLL_VIDEO2_3X 32
> +#define CLK_PLL_VE0 33
> +#define CLK_PLL_VE1 34
> +#define CLK_PLL_AUDIO0_4X 35
> +#define CLK_PLL_AUDIO1 36
> +#define CLK_PLL_AUDIO1_DIV2 37
> +#define CLK_PLL_AUDIO1_DIV5 38
> +#define CLK_PLL_NPU 39
> +#define CLK_PLL_DE 40
Should be DE_12X?
> +#define CLK_PLL_DE_4X 41
> +#define CLK_PLL_DE_3X 42
> +#define CLK_AHB 43
> +#define CLK_APB0 44
> +#define CLK_APB1 45
> +#define CLK_APB_UART 46
> +#define CLK_TRACE 47
> +#define CLK_GIC 48
> +#define CLK_CPU_PERI 49
> +#define CLK_BUS_ITS_PCIE 50
Please preserve the _ACLK suffix.
Please keep the unit number even if there is only one instance.
Basically, use the naming from the user manual. This is ITS0.
> +#define CLK_NSI 51
> +#define CLK_BUS_NSI 52
CLK_BUS_NSI_CFG
> +#define CLK_MBUS 53
> +#define CLK_MBUS_IOMMU0_SYS 54
> +#define CLK_APB_IOMMU0_SYS 55
> +#define CLK_AHB_IOMMU0_SYS 56
Please add a comment at the top describing the renaming to let the
reader know how to search for the in the user manual.
PCLK -> APB
HCLK -> AHB
MCLK -> MBUS
And that the bus name was moved from the end of the name to the front.
> +#define CLK_BUS_MSI_LITE0 57
> +#define CLK_BUS_MSI_LITE1 58
> +#define CLK_BUS_MSI_LITE2 59
> +#define CLK_MBUS_IOMMU1_SYS 60
> +#define CLK_APB_IOMMU1_SYS 61
> +#define CLK_AHB_IOMMU1_SYS 62
> +#define CLK_AHB_VE_DEC 63
> +#define CLK_AHB_VE_ENC 64
> +#define CLK_AHB_VID_IN 65
> +#define CLK_AHB_VID_COUT0 66
> +#define CLK_AHB_VID_COUT1 67
> +#define CLK_AHB_DE 68
> +#define CLK_AHB_NPU 69
> +#define CLK_AHB_GPU0 70
> +#define CLK_AHB_SERDES 71
> +#define CLK_AHB_USB_SYS 72
> +#define CLK_AHB_MSI_LITE0 73
> +#define CLK_AHB_STORE 74
> +#define CLK_AHB_CPUS 75
> +#define CLK_MBUS_IOMMU0 76
> +#define CLK_MBUS_IOMMU1 77
> +#define CLK_MBUS_DESYS 78
> +#define CLK_MBUS_VE_ENC_GATE 79
Please keep the unit number even if there is only one instance.
This is VE_ENC0.
> +#define CLK_MBUS_VE_DEC_GATE 80
> +#define CLK_MBUS_GPU0 81
> +#define CLK_MBUS_NPU 82
> +#define CLK_MBUS_VID_IN 83
> +#define CLK_MBUS_SERDES 84
> +#define CLK_MBUS_MSI_LITE0 85
> +#define CLK_MBUS_STORE 86
> +#define CLK_MBUS_MSI_LITE2 87
> +#define CLK_MBUS_DMA0 88
> +#define CLK_MBUS_VE_ENC 89
> +#define CLK_MBUS_CE 90
> +#define CLK_MBUS_DMA1 91
> +#define CLK_MBUS_NAND 92
> +#define CLK_MBUS_CSI 93
> +#define CLK_MBUS_ISP 94
> +#define CLK_MBUS_GMAC0 95
> +#define CLK_MBUS_GMAC1 96
There is no GMAC1 in the A733 user manual v0.92. Please add a note about
where this came from.
> +#define CLK_MBUS_VE_DEC 97
> +#define CLK_BUS_DMA0 98
> +#define CLK_BUS_DMA1 99
> +#define CLK_BUS_SPINLOCK 100
> +#define CLK_BUS_MSGBOX 101
Please keep the unit number even if there is only one instance.
This is MSGBOX0.
> +#define CLK_BUS_PWM0 102
> +#define CLK_BUS_PWM1 103
> +#define CLK_BUS_DBG 104
> +#define CLK_BUS_SYSDAP 105
> +#define CLK_TIMER0 106
> +#define CLK_TIMER1 107
> +#define CLK_TIMER2 108
> +#define CLK_TIMER3 109
> +#define CLK_TIMER4 110
> +#define CLK_TIMER5 111
> +#define CLK_TIMER6 112
> +#define CLK_TIMER7 113
> +#define CLK_TIMER8 114
> +#define CLK_TIMER9 115
> +#define CLK_BUS_TIMER 116
> +#define CLK_AVS 117
There is no AVS in the A733 user manual v0.92. Please add a note about
where this came from.
> +#define CLK_DE 118
Please keep the unit number even if there is only one instance.
This is DE0.
> +#define CLK_BUS_DE 119
Same here.
> +#define CLK_DI 120
> +#define CLK_BUS_DI 121
> +#define CLK_G2D 122
> +#define CLK_BUS_G2D 123
> +#define CLK_EINK 124
> +#define CLK_EINK_PANEL 125
> +#define CLK_BUS_EINK 126
> +#define CLK_VE_ENC 127
Please keep the unit number even if there is only one instance.
This is VE_ENC0.
> +#define CLK_VE_DEC 128
> +#define CLK_BUS_VE_ENC 129
Same here.
> +#define CLK_BUS_VE_DEC 130
> +#define CLK_CE 131
> +#define CLK_BUS_CE 132
> +#define CLK_BUS_CE_SYS 133
> +#define CLK_NPU 134
> +#define CLK_BUS_NPU 135
> +#define CLK_GPU 136
> +#define CLK_BUS_GPU 137
Please keep the unit number even if there is only one instance.
This is GPU0.
> +#define CLK_DRAM 138
> +#define CLK_BUS_DRAM 139
This is DRAM0.
> +#define CLK_NAND0 140
CLK_NAND0_CLK0
> +#define CLK_NAND1 141
CLK_NAND0_CLK1
> +#define CLK_BUS_NAND 142
CLK_BUS_NAND0
> +#define CLK_MMC0 143
> +#define CLK_MMC1 144
> +#define CLK_MMC2 145
> +#define CLK_MMC3 146
> +#define CLK_BUS_MMC0 147
> +#define CLK_BUS_MMC1 148
> +#define CLK_BUS_MMC2 149
> +#define CLK_BUS_MMC3 150
The ordering is wrong. The module clocks and bus clocks are interleaved.
MMC0, BUS_MMC0, MMC1, BUS_MMC1, ...
> +#define CLK_UFS_AXI 151
> +#define CLK_UFS_CFG 152
> +#define CLK_BUS_UFS 153
> +#define CLK_BUS_UART0 154
> +#define CLK_BUS_UART1 155
> +#define CLK_BUS_UART2 156
> +#define CLK_BUS_UART3 157
> +#define CLK_BUS_UART4 158
> +#define CLK_BUS_UART5 159
> +#define CLK_BUS_UART6 160
> +#define CLK_BUS_I2C0 161
> +#define CLK_BUS_I2C1 162
> +#define CLK_BUS_I2C2 163
> +#define CLK_BUS_I2C3 164
> +#define CLK_BUS_I2C4 165
> +#define CLK_BUS_I2C5 166
> +#define CLK_BUS_I2C6 167
> +#define CLK_BUS_I2C7 168
> +#define CLK_BUS_I2C8 169
> +#define CLK_BUS_I2C9 170
> +#define CLK_BUS_I2C10 171
> +#define CLK_BUS_I2C11 172
> +#define CLK_BUS_I2C12 173
> +#define CLK_SPI0 174
> +#define CLK_SPI1 175
> +#define CLK_SPI2 176
> +#define CLK_SPI3 177
> +#define CLK_SPI4 178
> +#define CLK_BUS_SPI0 179
> +#define CLK_BUS_SPI1 180
> +#define CLK_BUS_SPI2 181
> +#define CLK_BUS_SPI3 182
> +#define CLK_BUS_SPI4 183
Same as MMC ones, SPI module clocks and bus clocks are interleaved.
The user manual doesn't show SPI4. Please add a note saying where the
information is from.
> +#define CLK_SPIF 184
> +#define CLK_BUS_SPIF 185
SPIF comes before SPI3. Please keep the ordering based on the registers.
> +#define CLK_GPADC 186
CLK_GPADC_24M
> +#define CLK_BUS_GPADC 187
CLK_BUS_GPADC0
> +#define CLK_BUS_THS 188
CLK_BUS_THS0
> +#define CLK_IRRX 189
> +#define CLK_BUS_IRRX 190
> +#define CLK_IRTX 191
> +#define CLK_BUS_IRTX 192
> +#define CLK_BUS_LRADC 193
> +#define CLK_SGPIO 194
> +#define CLK_BUS_SGPIO 195
> +#define CLK_LPC 196
> +#define CLK_BUS_LPC 197
These aren't found in the user manual. Please document where they were found.
> +#define CLK_I2SPCM0 198
> +#define CLK_I2SPCM1 199
> +#define CLK_I2SPCM2 200
> +#define CLK_I2SPCM3 201
> +#define CLK_I2SPCM4 202
> +#define CLK_BUS_I2SPCM0 203
> +#define CLK_BUS_I2SPCM1 204
> +#define CLK_BUS_I2SPCM2 205
> +#define CLK_BUS_I2SPCM3 206
> +#define CLK_BUS_I2SPCM4 207
Module clocks and bus clocks are interleaved.
> +#define CLK_I2SPCM2_ASRC 208
This would come before BUS_I2SPCM2.
> +#define CLK_OWA_TX 209
> +#define CLK_OWA_RX 210
> +#define CLK_BUS_OWA 211
s/OWA/SPDIF/
> +#define CLK_DMIC 212
> +#define CLK_BUS_DMIC 213
> +#define CLK_USB_OHCI0 214
> +#define CLK_BUS_OTG 215
> +#define CLK_BUS_EHCI0 216
> +#define CLK_BUS_OHCI0 217
Order should be OHCI0 -> EHCI0 -> OTG
> +#define CLK_USB_OHCI1 218
> +#define CLK_BUS_EHCI1 219
> +#define CLK_BUS_OHCI1 220
OHCI1, then EHCI1.
> +#define CLK_USB_REF 221
USB01_REF to keep the numbering but keeping it short.
> +#define CLK_USB2_U2_REF 222
> +#define CLK_USB2_SUSPEND 223
> +#define CLK_USB2_MF 224
> +#define CLK_USB2_U3_UTMI 225
> +#define CLK_USB2_U2_PIPE 226
Side-note:
I think the manual got this reversed, since UTMI is used for USB 2.0,
while PIPE is used for USB 3.0. But we will stick to whatever the user
manual says.
> +#define CLK_PCIE_AUX 227
> +#define CLK_PCIE_AXI_SLV 228
> +#define CLK_SERDES_PHY 229
> +#define CLK_GMAC_PTP 230
> +#define CLK_GMAC0_PHY 231
> +#define CLK_GMAC1_PHY 232
> +#define CLK_BUS_GMAC0 233
Based on your driver, BUS_GMAC0 should come before GMAC1_PHY.
> +#define CLK_BUS_GMAC1 234
There is no GMAC1 mentioned in the A733 manual. Please document where
you found this.
> +#define CLK_TCON_LCD0 235
> +#define CLK_TCON_LCD1 236
> +#define CLK_TCON_LCD2 237
> +#define CLK_BUS_TCON_LCD0 238
> +#define CLK_BUS_TCON_LCD1 239
> +#define CLK_BUS_TCON_LCD2 240
Module clocks are interleaved with bus clock gates.
> +#define CLK_DSI0 241
> +#define CLK_DSI1 242
> +#define CLK_BUS_DSI0 243
> +#define CLK_BUS_DSI1 244
Same for DSI.
> +#define CLK_COMBPHY0 245
> +#define CLK_COMBPHY1 246
> +#define CLK_BUS_TCON_TV0 247
> +#define CLK_BUS_TCON_TV1 248
> +#define CLK_EDP_TV 249
> +#define CLK_BUS_EDP_TV 250
> +#define CLK_HDMI_CEC_32K 251
> +#define CLK_HDMI_CEC 252
> +#define CLK_HDMI_TV 253
> +#define CLK_BUS_HDMI_TV 254
> +#define CLK_HDMI_SFR 255
> +#define CLK_HDMI_ESM 256
HDCP_ESM.
> +#define CLK_BUS_DPSS_TOP0 257
> +#define CLK_BUS_DPSS_TOP1 258
> +#define CLK_LEDC 259
> +#define CLK_BUS_LEDC 260
> +#define CLK_BUS_DSC 261
> +#define CLK_CSI_MASTER0 262
> +#define CLK_CSI_MASTER1 263
> +#define CLK_CSI_MASTER2 264
> +#define CLK_CSI 265
> +#define CLK_BUS_CSI 266
> +#define CLK_ISP 267
I guess we don't need to add any of the PLL output gates since they are
auto-gated?
Also, there is a RES_DCAP_24M_GATE at 0x1a00. Any idea about this one?
> +#define CLK_APB2JTAG 268
> +#define CLK_FANOUT_24M 269
> +#define CLK_FANOUT_12M 270
> +#define CLK_FANOUT_16M 271
> +#define CLK_FANOUT_25M 272
> +#define CLK_FANOUT_27M 273
> +#define CLK_FANOUT_PCLK 274
> +#define CLK_FANOUT0 275
> +#define CLK_FANOUT1 276
> +#define CLK_FANOUT2 277
> +#define CLK_FANOUT3 278
The rest are fine.
> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_ */
> diff --git a/include/dt-bindings/clock/sun60i-a733-r-ccu.h b/include/dt-bindings/clock/sun60i-a733-r-ccu.h
> new file mode 100644
> index 000000000000..1c717cc588b8
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun60i-a733-r-ccu.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_
> +
> +#define CLK_R_AHB 0
> +#define CLK_R_APB0 1
> +#define CLK_R_APB1 2
> +#define CLK_R_TIMER0 3
> +#define CLK_R_TIMER1 4
> +#define CLK_R_TIMER2 5
> +#define CLK_R_TIMER3 6
> +#define CLK_BUS_R_TIMER 7
> +#define CLK_BUS_R_TWD 8
> +#define CLK_R_PWMCTRL 9
> +#define CLK_BUS_R_PWMCTRL 10
*_R_PWM.
> +#define CLK_R_SPI 11
> +#define CLK_BUS_R_SPI 12
> +#define CLK_BUS_R_MSGBOX 13
> +#define CLK_BUS_R_UART0 14
> +#define CLK_BUS_R_UART1 15
> +#define CLK_BUS_R_I2C0 16
> +#define CLK_BUS_R_I2C1 17
> +#define CLK_BUS_R_I2C2 18
> +#define CLK_BUS_R_PPU 19
> +#define CLK_BUS_R_TZMA 20
> +#define CLK_BUS_R_CPU_BIST 21
> +#define CLK_R_IR_RX 22
> +#define CLK_BUS_R_IR_RX 23
> +#define CLK_BUS_R_RTC 24
> +#define CLK_R_RISCV 25
> +#define CLK_BUS_R_RISCV 26
> +#define CLK_BUS_R_RISCV_CFG 27
> +#define CLK_BUS_R_CPUCFG 28
> +
> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_ */
> diff --git a/include/dt-bindings/reset/sun60i-a733-ccu.h b/include/dt-bindings/reset/sun60i-a733-ccu.h
> new file mode 100644
> index 000000000000..11ce78cf04dd
> --- /dev/null
> +++ b/include/dt-bindings/reset/sun60i-a733-ccu.h
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_RST_SUN60I_A733_CCU_H_
> +#define _DT_BINDINGS_RST_SUN60I_A733_CCU_H_
> +
> +#define RST_BUS_ITS_PCIE 0
> +#define RST_BUS_NSI 1
> +#define RST_BUS_NSI_CFG 2
> +#define RST_BUS_IOMMU0_SYS 3
> +#define RST_BUS_MSI_LITE0_AHB 4
> +#define RST_BUS_MSI_LITE0_MBUS 5
> +#define RST_BUS_MSI_LITE1_AHB 6
> +#define RST_BUS_MSI_LITE1_MBUS 7
> +#define RST_BUS_MSI_LITE2_AHB 8
> +#define RST_BUS_MSI_LITE2_MBUS 9
> +#define RST_BUS_IOMMU1_SYS 10
> +#define RST_BUS_DMA0 11
> +#define RST_BUS_DMA1 12
> +#define RST_BUS_SPINLOCK 13
> +#define RST_BUS_MSGBOX 14
> +#define RST_BUS_PWM0 15
> +#define RST_BUS_PWM1 16
> +#define RST_BUS_DBG 17
> +#define RST_BUS_SYSDAP 18
> +#define RST_BUS_TIMER0 19
> +#define RST_BUS_DE 20
Please keep the unit number even if there is only one instance.
This is DE0.
> +#define RST_BUS_DI 21
> +#define RST_BUS_G2D 22
> +#define RST_BUS_EINK 23
> +#define RST_BUS_DE_SYS 24
> +#define RST_BUS_VE_ENC 25
Please keep the unit number even if there is only one instance.
This is VE_ENC0.
> +#define RST_BUS_VE_DEC 26
> +#define RST_BUS_CE 27
> +#define RST_BUS_CE_SYS 28
> +#define RST_BUS_NPU_CORE 29
> +#define RST_BUS_NPU_AXI 30
> +#define RST_BUS_NPU_AHB 31
> +#define RST_BUS_NPU_SRAM 32
> +#define RST_BUS_GPU 33
> +#define RST_BUS_DRAM 34
> +#define RST_BUS_NAND 35
> +#define RST_BUS_MMC0 36
> +#define RST_BUS_MMC1 37
> +#define RST_BUS_MMC2 38
> +#define RST_BUS_MMC3 39
> +#define RST_BUS_UFS_AHB 40
> +#define RST_BUS_UFS_AXI 41
> +#define RST_BUS_UFS_PHY 42
> +#define RST_BUS_UFS_CORE 43
> +#define RST_BUS_UART0 44
> +#define RST_BUS_UART1 45
> +#define RST_BUS_UART2 46
> +#define RST_BUS_UART3 47
> +#define RST_BUS_UART4 48
> +#define RST_BUS_UART5 49
> +#define RST_BUS_UART6 50
> +#define RST_BUS_I2C0 51
> +#define RST_BUS_I2C1 52
> +#define RST_BUS_I2C2 53
> +#define RST_BUS_I2C3 54
> +#define RST_BUS_I2C4 55
> +#define RST_BUS_I2C5 56
> +#define RST_BUS_I2C6 57
> +#define RST_BUS_I2C7 58
> +#define RST_BUS_I2C8 59
> +#define RST_BUS_I2C9 60
> +#define RST_BUS_I2C10 61
> +#define RST_BUS_I2C11 62
> +#define RST_BUS_I2C12 63
> +#define RST_BUS_SPI0 64
> +#define RST_BUS_SPI1 65
> +#define RST_BUS_SPI2 66
> +#define RST_BUS_SPIF 67
Here you have SPIF in the correct position.
> +#define RST_BUS_SPI3 68
> +#define RST_BUS_SPI4 69
Please add a note saying where SPI4 was found.
> +#define RST_BUS_GPADC 70
RST_BUS_GPADC0
> +#define RST_BUS_THS 71
RST_BUS_THS0
> +#define RST_BUS_IRRX 72
> +#define RST_BUS_IRTX 73
> +#define RST_BUS_LRADC 74
> +#define RST_BUS_SGPIO 75
> +#define RST_BUS_LPC 76
> +#define RST_BUS_I2SPCM0 77
> +#define RST_BUS_I2SPCM1 78
> +#define RST_BUS_I2SPCM2 79
> +#define RST_BUS_I2SPCM3 80
> +#define RST_BUS_I2SPCM4 81
> +#define RST_BUS_OWA 82
> +#define RST_BUS_DMIC 83
> +#define RST_USB_PHY0 84
> +#define RST_BUS_OHCI0 85
> +#define RST_BUS_EHCI0 86
> +#define RST_BUS_OTG 87
> +#define RST_USB_PHY1 88
> +#define RST_BUS_OHCI1 89
> +#define RST_BUS_EHCI1 90
> +#define RST_BUS_USB2 91
> +#define RST_BUS_PCIE 92
> +#define RST_BUS_PCIE_PWRUP 93
PCIE_PWRUP comes before PCIE.
> +#define RST_BUS_SERDES 94
> +#define RST_BUS_GMAC0 95
> +#define RST_BUS_GMAC0_AXI 96
> +#define RST_BUS_GMAC1 97
> +#define RST_BUS_GMAC1_AXI 98
> +#define RST_BUS_TCON_LCD0 99
> +#define RST_BUS_TCON_LCD1 100
> +#define RST_BUS_TCON_LCD2 101
> +#define RST_BUS_LVDS0 102
> +#define RST_BUS_LVDS1 103
> +#define RST_BUS_DSI0 104
> +#define RST_BUS_DSI1 105
> +#define RST_BUS_TCON_TV0 106
> +#define RST_BUS_TCON_TV1 107
> +#define RST_BUS_EDP 108
> +#define RST_BUS_HDMI_MAIN 109
> +#define RST_BUS_HDMI_SUB 110
> +#define RST_BUS_HDMI_HDCP 111
> +#define RST_BUS_DPSS_TOP0 112
> +#define RST_BUS_DPSS_TOP1 113
> +#define RST_BUS_VIDEO_OUT0 114
> +#define RST_BUS_VIDEO_OUT1 115
> +#define RST_BUS_LEDC 116
> +#define RST_BUS_DSC 117
> +#define RST_BUS_CSI 118
> +#define RST_BUS_VIDEO_IN 119
> +#define RST_BUS_APB2JTAG 120
> +
The rest look correct.
> +#endif /* _DT_BINDINGS_RST_SUN60I_A733_CCU_H_ */
> diff --git a/include/dt-bindings/reset/sun60i-a733-r-ccu.h b/include/dt-bindings/reset/sun60i-a733-r-ccu.h
> new file mode 100644
> index 000000000000..629e546d1998
> --- /dev/null
> +++ b/include/dt-bindings/reset/sun60i-a733-r-ccu.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_
> +#define _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_
> +
> +#define RST_BUS_R_TIMER 0
> +#define RST_BUS_R_PWM 1
> +#define RST_BUS_R_SPI 2
> +#define RST_BUS_R_MSGBOX 3
> +#define RST_BUS_R_UART0 4
> +#define RST_BUS_R_UART1 5
> +#define RST_BUS_R_I2C0 6
> +#define RST_BUS_R_I2C1 7
> +#define RST_BUS_R_I2C2 8
> +#define RST_BUS_R_IR_RX 9
> +#define RST_BUS_R_RTC 10
> +#define RST_BUS_R_RISCV_CFG 11
> +#define RST_BUS_R_CPUCFG 12
> +
> +#endif /* _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_ */
This part looks correct.
ChenYu
^ permalink raw reply
* [PATCH] NFC: s3fwrn5: bound the UART receive buffer
From: Pengpeng Hou @ 2026-03-28 11:36 UTC (permalink / raw)
To: netdev, krzk
Cc: bongsu.jeon, davem, edumazet, kuba, pabeni, linux-kernel,
pengpeng
s3fwrn82_uart_read() appends bytes to phy->recv_skb with skb_put_u8().
It only resets the skb after a complete NCI frame is assembled.
The receive skb is allocated with NCI_SKB_BUFF_LEN bytes, but
malformed UART traffic can keep the frame incomplete and continue
growing recv_skb until skb_put_u8() overruns the tailroom.
Drop the accumulated partial frame once the fixed receive buffer is full
and no complete frame has been seen yet so malformed UART traffic cannot
grow the skb past NCI_SKB_BUFF_LEN.
Fixes: 3f52c2cb7e3a ("nfc: s3fwrn5: Support a UART interface")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/nfc/s3fwrn5/uart.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 9c09c10c2a46..c664fed78c1f 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -64,8 +64,17 @@ static size_t s3fwrn82_uart_read(struct serdev_device *serdev,
continue;
if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
- < phy->recv_skb->data[S3FWRN82_NCI_IDX])
+ < phy->recv_skb->data[S3FWRN82_NCI_IDX]) {
+ if (phy->recv_skb->len < NCI_SKB_BUFF_LEN)
+ continue;
+
+ dev_dbg(&serdev->dev, "dropping oversized UART frame\n");
+ kfree_skb(phy->recv_skb);
+ phy->recv_skb = alloc_skb(NCI_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!phy->recv_skb)
+ return 0;
continue;
+ }
s3fwrn5_recv_frame(phy->common.ndev, phy->recv_skb,
phy->common.mode);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH net v2] ipv6: fix data race in fib6_metric_set() using cmpxchg
From: Jiayuan Chen @ 2026-03-28 11:22 UTC (permalink / raw)
To: Hangbin Liu, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jiayuan Chen
Cc: David Ahern, netdev, linux-kernel, Fei Liu
In-Reply-To: <20260327-b4-fib6_metric_set-kmemleak-v2-1-366b2c78b5c2@gmail.com>
On 3/27/26 10:24 AM, Hangbin Liu wrote:
> fib6_metric_set() may be called concurrently from softirq context without
> holding the FIB table lock. A typical path is:
>
> ndisc_router_discovery()
> spin_unlock_bh(&table->tb6_lock) <- lock released
> fib6_metric_set(rt, RTAX_HOPLIMIT, ...) <- lockless call
>
> When two CPUs process Router Advertisement packets for the same router
> simultaneously, they can both arrive at fib6_metric_set() with the same
> fib6_info pointer whose fib6_metrics still points to dst_default_metrics.
>
> if (f6i->fib6_metrics == &dst_default_metrics) { /* both CPUs: true */
> struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
> refcount_set(&p->refcnt, 1);
> f6i->fib6_metrics = p; /* CPU1 overwrites CPU0's p -> p0 leaked */
> }
>
> The dst_metrics allocated by the losing CPU has refcnt=1 but no pointer
> to it anywhere in memory, producing a kmemleak report:
>
> unreferenced object 0xff1100025aca1400 (size 96):
> comm "softirq", pid 0, jiffies 4299271239
> backtrace:
> kmalloc_trace+0x28a/0x380
> fib6_metric_set+0xcd/0x180
> ndisc_router_discovery+0x12dc/0x24b0
> icmpv6_rcv+0xc16/0x1360
>
> Fix this by:
> - Set val for p->metrics before published via cmpxchg() so the metrics
> value is ready before the pointer becomes visible to other CPUs.
> - Replace the plain pointer store with cmpxchg() and free the allocation
> safely when competition failed.
> - Add READ_ONCE()/WRITE_ONCE() for metrics[] setting in the non-default
> metrics path to prevent compiler-based data races.
>
> Fixes: d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
> Reported-by: Fei Liu <feliu@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
https://sashiko.dev/#/patchset/20260327-b4-fib6_metric_set-kmemleak-v2-1-366b2c78b5c2%40gmail.com
The concern about reader paths (e.g., ip_dst_init_metrics, fib6_pmtu)
lacking READ_ONCE()
annotations is valid — if the compiler reloads from->fib6_metrics after
inlining, it could produce
an inconsistent pointer/flags combination in dst->_metrics, potentially
leading to a refcount_dec
on the read-only dst_default_metrics.
However, this is a pre-existing issue that exists before this patch.
The plain store f6i->fib6_metrics = p in the original code has the same
read-side race.
This patch focuses on fixing the writer-side data race that causes
kmemleak, and it
does so correctly.
BTW, please consider moving the declaration of m to the top of the
function if you have a next version
^ permalink raw reply
* Re: [PATCH bpf v3 2/2] selftests/bpf: Add protocol check test for bpf_sk_assign_tcp_reqsk()
From: Jiayuan Chen @ 2026-03-28 10:58 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Jiayuan Chen, Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Shuah Khan, Kuniyuki Iwashima, bpf, netdev, linux-kernel,
linux-kselftest
In-Reply-To: <8d619537-cb9a-4fdb-a440-20b59b7f5088@linux.dev>
On 3/28/26 3:53 AM, Martin KaFai Lau wrote:
>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>
>> Add test_tcp_custom_syncookie_protocol_check to verify that
>> bpf_sk_assign_tcp_reqsk() rejects non-TCP skbs. The test sends a UDP
>> packet through TC ingress where a BPF program calls
>> bpf_sk_assign_tcp_reqsk() on it and checks that the kfunc returns an
>> error. A UDP server recv() is used as synchronization to ensure the
>> BPF program has finished processing before checking the result.
>>
>> Without the fix in bpf_sk_assign_tcp_reqsk(), the kfunc succeeds and
>> attaches a TCP reqsk to the UDP skb, which causes a null pointer
>> dereference panic when the kernel processes it through the UDP receive
>> path.
>>
>> Test result:
>>
>> ./test_progs -a tcp_custom_syncookie_protocol_check -v
>> setup_netns:PASS:create netns 0 nsec
>> setup_netns:PASS:ip 0 nsec
>> write_sysctl:PASS:open sysctl 0 nsec
>> write_sysctl:PASS:write sysctl 0 nsec
>> setup_netns:PASS:write_sysctl 0 nsec
>> test_tcp_custom_syncookie_protocol_check:PASS:open_and_load 0 nsec
>> setup_tc:PASS:qdisc add dev lo clsact 0 nsec
>> setup_tc:PASS:filter add dev lo ingress 0 nsec
>> run_protocol_check:PASS:start tcp_server 0 nsec
>> run_protocol_check:PASS:start udp_server 0 nsec
>> run_protocol_check:PASS:connect udp_client 0 nsec
>> run_protocol_check:PASS:send udp 0 nsec
>> run_protocol_check:PASS:recv udp 0 nsec
>> run_protocol_check:PASS:udp_intercepted 0 nsec
>> run_protocol_check:PASS:assign_ret 0 nsec
>> #471/1 tcp_custom_syncookie_protocol_check/IPv4 TCP:OK
>> run_protocol_check:PASS:start tcp_server 0 nsec
>> run_protocol_check:PASS:start udp_server 0 nsec
>> run_protocol_check:PASS:connect udp_client 0 nsec
>> run_protocol_check:PASS:send udp 0 nsec
>> run_protocol_check:PASS:recv udp 0 nsec
>> run_protocol_check:PASS:udp_intercepted 0 nsec
>> run_protocol_check:PASS:assign_ret 0 nsec
>> #471/2 tcp_custom_syncookie_protocol_check/IPv6 TCP:OK
>> #471 tcp_custom_syncookie_protocol_check:OK
>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
>> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
>> ---
>> .../bpf/prog_tests/tcp_custom_syncookie.c | 94 ++++++++++++++-
>> .../bpf/progs/test_tcp_custom_syncookie.c | 109 ++++++++++++++++++
>> 2 files changed, 199 insertions(+), 4 deletions(-)
>>
>> diff --git
>> a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> index eaf441dc7e79..e46031d0786b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> @@ -5,6 +5,7 @@
>> #include <sched.h>
>> #include <stdlib.h>
>> #include <net/if.h>
>> +#include <netinet/in.h>
>> #include "test_progs.h"
>> #include "cgroup_helpers.h"
>> @@ -47,11 +48,10 @@ static int setup_netns(void)
>> return -1;
>> }
>> -static int setup_tc(struct test_tcp_custom_syncookie *skel)
>> +static int setup_tc(int prog_fd)
>> {
>> LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point =
>> BPF_TC_INGRESS);
>> - LIBBPF_OPTS(bpf_tc_opts, tc_attach,
>> - .prog_fd =
>> bpf_program__fd(skel->progs.tcp_custom_syncookie));
>> + LIBBPF_OPTS(bpf_tc_opts, tc_attach, .prog_fd = prog_fd);
>> qdisc_lo.ifindex = if_nametoindex("lo");
>> if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo
>> clsact"))
>> @@ -127,7 +127,7 @@ void test_tcp_custom_syncookie(void)
>> if (!ASSERT_OK_PTR(skel, "open_and_load"))
>> return;
>> - if (setup_tc(skel))
>> + if (setup_tc(bpf_program__fd(skel->progs.tcp_custom_syncookie)))
>> goto destroy_skel;
>> for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
>> @@ -145,6 +145,92 @@ void test_tcp_custom_syncookie(void)
>> destroy_skel:
>> system("tc qdisc del dev lo clsact");
>> + test_tcp_custom_syncookie__destroy(skel);
>> +}
>> +
>> +/* Test: bpf_sk_assign_tcp_reqsk() should reject non-TCP skb.
>> + *
>> + * Send a UDP packet through TC ingress where a BPF program calls
>> + * bpf_sk_assign_tcp_reqsk() on it. The kfunc should return an error
>> + * because the skb carries UDP, not TCP.
>> + *
>> + * TCP and UDP servers share the same port. The BPF program intercepts
>> + * the UDP packet, looks up the TCP listener via the dest port, and
>> + * attempts to assign a TCP reqsk to the UDP skb.
>> + */
>> +static void run_protocol_check(struct test_tcp_custom_syncookie *skel,
>> + int family, const char *addr)
>> +{
>> + int tcp_server = -1, udp_server = -1, udp_client = -1;
>> + char buf[32] = "test";
>> + int port, ret;
>> +
>> + tcp_server = start_server(family, SOCK_STREAM, addr, 0, 0);
>> + if (!ASSERT_NEQ(tcp_server, -1, "start tcp_server"))
>> + return;
>> +
>> + port = ntohs(get_socket_local_port(tcp_server));
>> +
>> + /* UDP server on same port for synchronization and port sharing */
>> + udp_server = start_server(family, SOCK_DGRAM, addr, port, 0);
>> + if (!ASSERT_NEQ(udp_server, -1, "start udp_server"))
>> + goto close_tcp;
>> +
>> + skel->bss->udp_intercepted = false;
>> + skel->data->assign_ret = -1;
>
> assign_ret is init to -1
>
>> +
>> + udp_client = connect_to_fd(udp_server, 0);
>> + if (!ASSERT_NEQ(udp_client, -1, "connect udp_client"))
>> + goto close_udp_server;
>> + ret = send(udp_client, buf, sizeof(buf), 0);
>> + if (!ASSERT_EQ(ret, sizeof(buf), "send udp"))
>> + goto close_udp_client;
>> +
>> + /* recv() ensures TC ingress BPF has processed the skb */
>> + ret = recv(udp_server, buf, sizeof(buf), 0);
>> + if (!ASSERT_EQ(ret, sizeof(buf), "recv udp"))
>> + goto close_udp_client;
>> +
>> + ASSERT_EQ(skel->bss->udp_intercepted, true, "udp_intercepted");
>> +
>> + /* assign_ret == 0 means kfunc accepted UDP skb (bug).
>> + * assign_ret < 0 means kfunc correctly rejected it (fixed).
>> + */
>> + ASSERT_NEQ(skel->data->assign_ret, 0, "assign_ret");
>
> and here it checks NEQ 0....
>
> Maybe init assign_ret to 0?
>
> Also, why not specifically check for -EINVAL?
You're right, checking for -EINVAL is more precise and avoids hiding a
silent failure in the lookup path.
Will init assign_ret to 0 and check for -EINVAL instead.
^ permalink raw reply
* Re: [PATCH 6.1.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work
From: Arend van Spriel @ 2026-03-28 10:54 UTC (permalink / raw)
To: Robert Garcia, stable, Duoming Zhou
Cc: Johannes Berg, Kalle Valo, Franky Lin, Hante Meuleman,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pieter-Paul Giesberts, Piotr Haber, John W . Linville,
linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
netdev, linux-kernel
In-Reply-To: <20260312031429.3432419-1-rob_garcia@163.com>
On 12/03/2026 04:14, Robert Garcia wrote:
> From: Duoming Zhou <duoming@zju.edu.cn>
>
> [ Upstream commit 9cb83d4be0b9b697eae93d321e0da999f9cdfcfc ]
>
> The brcmf_btcoex_detach() only shuts down the btcoex timer, if the
> flag timer_on is false. However, the brcmf_btcoex_timerfunc(), which
> runs as timer handler, sets timer_on to false. This creates critical
> race conditions:
[...]
> To resolve the race conditions, drop the conditional check and call
> timer_shutdown_sync() directly. It can deactivate the timer reliably,
> regardless of its current state. Once stopped, the timer_on state is
> then set to false.
>
> Fixes: 61730d4dfffc ("brcmfmac: support critical protocol API for DHCP")
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> Link: https://patch.msgid.link/20250822050839.4413-1-duoming@zju.edu.cn
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> [ Keep del_timer_sync() instead of timer_shutdown_sync() here. ]
> Signed-off-by: Robert Garcia <rob_garcia@163.com>
What tree should this go to. This looks like a stable patch so probably
it should have been CCed to stable@vger.kernel.org?
Regards,
Arend
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Russell King (Oracle) @ 2026-03-28 10:25 UTC (permalink / raw)
To: Eric Woudstra
Cc: Lucien.Jheng, andrew, hkallweit1, davem, edumazet, kuba, pabeni,
netdev, linux-kernel, bjorn, frank-w, daniel, lucien.jheng
In-Reply-To: <6f42d92b-a26d-49a4-bef7-2227c252eea7@gmail.com>
On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> u32 pbus_data)
> {
> int ret;
>
> mutex_lock(&mdio->bus->mdio_lock);
I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
Should a helper be provided at mdio bus level to complement the PHY
level helper if we're going to have MDIO drivers directly manipulating
the lock?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH net-next 4/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@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.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb_main.c | 116 ++++++++-----------------------
1 file changed, 29 insertions(+), 87 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c53b28b42a46489722461957625e8377be63e427..de166dd9e637f5e436274b99f2c5eef99dcdb351 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
@@ -2027,62 +2028,32 @@ 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);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, 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);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, 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);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- queue_writel(queue, ISR, 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);
+ if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ queue_writel(queue, ISR, GEM_BIT(WOL));
+ pm_wakeup_event(&bp->pdev->dev, 0);
}
static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
@@ -2147,6 +2118,14 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
queue_writel(queue, ISR, 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;
}
@@ -5943,7 +5922,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);
@@ -6007,39 +5985,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);
}
@@ -6081,7 +6035,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);
@@ -6108,17 +6061,6 @@ static int __maybe_unused macb_resume(struct device *dev)
queue_writel(bp->queues, ISR, -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 3/4] net: macb: Factor out the handling of non-hot IRQ events into a separate function
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@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 | 123 ++++++++++++++++++-------------
1 file changed, 72 insertions(+), 51 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 743abe11324c690c11993d7be9ed5b73422dd17c..c53b28b42a46489722461957625e8377be63e427 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)))
@@ -2082,12 +2085,77 @@ 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;
+ bool isr_clear;
+ u32 ctrl;
+
+ dev = bp->dev;
+ isr_clear = bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE;
+
+ if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
+ queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
+ schedule_work(&queue->tx_error_task);
+
+ if (isr_clear)
+ queue_writel(queue, ISR, 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));
+
+ if (isr_clear)
+ queue_writel(queue, ISR, 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);
+
+ if (isr_clear)
+ queue_writel(queue, ISR, 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 (isr_clear)
+ queue_writel(queue, ISR, 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;
bool isr_clear;
status = queue_readl(queue, ISR);
@@ -2141,57 +2209,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);
+ if (unlikely(status & MACB_INT_MISC_FLAGS))
+ if (macb_interrupt_misc(queue, status))
+ break;
- if (isr_clear)
- queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
-
- 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));
-
- if (isr_clear)
- queue_writel(queue, ISR, 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);
-
- if (isr_clear)
- queue_writel(queue, ISR, 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 (isr_clear)
- queue_writel(queue, ISR, MACB_BIT(HRESP));
- }
status = queue_readl(queue, ISR);
}
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 2/4] net: macb: Consolidate MACB_CAPS_ISR_CLEAR_ON_WRITE checks in IRQ handler
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com>
Currently, the MACB_CAPS_ISR_CLEAR_ON_WRITE flag is checked in every
branch of the IRQ handler. This repeated evaluation is unnecessary.
By consolidating the flag check, we eliminate redundant loads of
bp->caps when TX and RX events occur simultaneously, a common scenario
under high network throughput. Additionally, this optimization reduces
the function size from 0x2e8 to 0x2c4.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
drivers/net/ethernet/cadence/macb_main.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 886246a6f6bdd0b6a8cb4b86d7788ac181ee602a..743abe11324c690c11993d7be9ed5b73422dd17c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2088,19 +2088,22 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
struct macb *bp = queue->bp;
struct net_device *dev = bp->dev;
u32 status, ctrl;
+ bool isr_clear;
status = queue_readl(queue, ISR);
if (unlikely(!status))
return IRQ_NONE;
+ isr_clear = bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE;
+
spin_lock(&bp->lock);
while (status) {
/* 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)
+ if (isr_clear)
queue_writel(queue, ISR, -1);
break;
}
@@ -2117,7 +2120,7 @@ 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)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
napi_schedule(&queue->napi_rx);
@@ -2126,7 +2129,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
if (status & (MACB_BIT(TCOMP) |
MACB_BIT(TXUBR))) {
queue_writel(queue, IDR, MACB_BIT(TCOMP));
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_BIT(TCOMP) |
MACB_BIT(TXUBR));
@@ -2142,7 +2145,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
schedule_work(&queue->tx_error_task);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
break;
@@ -2165,7 +2168,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
wmb();
macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_BIT(RXUBR));
}
@@ -2178,7 +2181,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
bp->hw_stats.macb.rx_overruns++;
spin_unlock(&bp->stats_lock);
- if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_BIT(ISR_ROVR));
}
@@ -2186,7 +2189,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
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)
+ if (isr_clear)
queue_writel(queue, ISR, MACB_BIT(HRESP));
}
status = queue_readl(queue, ISR);
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 1/4] net: macb: Replace open-coded implementation with napi_schedule()
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@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 c8182559edf602e7f5d94644cffb22e8f58423cc..886246a6f6bdd0b6a8cb4b86d7788ac181ee602a 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 0/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kevin Hao, netdev
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.
I am skeptical that the minor optimizations to the IRQ handler proposed in this
patch series would yield any measurable performance improvement. However, it
does appear that the execution time of the macb_interrupt() function is
slightly reduced.
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 857 Mbits/sec 0 sender
[SUM] 0.00-60.00 sec 5.98 GBytes 856 Mbits/sec receiver
Function Hit Time Avg s^2
-------- --- ---- --- ---
macb_interrupt 218538 723327.5 us 3.309 us 1.022 us
After:
------
[SUM] 0.00-60.00 sec 5.99 GBytes 857 Mbits/sec 0 sender
[SUM] 0.00-60.00 sec 5.98 GBytes 857 Mbits/sec receiver
Function Hit Time Avg s^2
-------- --- ---- --- ---
macb_interrupt 218558 646355.1 us 2.957 us 1.290 us
---
Kevin Hao (4):
net: macb: Replace open-coded implementation with napi_schedule()
net: macb: Consolidate MACB_CAPS_ISR_CLEAR_ON_WRITE checks in IRQ handler
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_main.c | 244 +++++++++++++------------------
1 file changed, 102 insertions(+), 142 deletions(-)
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260321-macb-irq-453ee09b3394
Best regards,
--
Kevin Hao <haokexin@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-28 9:31 UTC (permalink / raw)
To: Andrew Lunn, Ong Boon Leong
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Paolo Abeni
In-Reply-To: <aceQRSc5o4D-HHmq@shell.armlinux.org.uk>
On Sat, Mar 28, 2026 at 08:24:37AM +0000, Russell King (Oracle) wrote:
> On Fri, Mar 27, 2026 at 09:40:09AM +0000, Russell King (Oracle) wrote:
> > The test in stmmac_xmit() to see whether we should pass the skbuff to
> > stmmac_tso_xmit() is more complex than it needs to be. This test can
> > be simplified by storing the mask of GSO types that we will pass, and
> > setting it according to the enabled features.
> >
> > Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
> > stmmac: Implement UDP Segmentation Offload"). Also note that this
> > commit controls both via the TSO feature. We preserve this behaviour
> > in this commit.
> >
> > Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
> > for all frames, even when skb_is_gso() was false. This access is
> > eliminated.
> >
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>
> AI review of this patch regurgitates Jakub's point that was discussed.
>
> > @@ -3700,7 +3700,7 @@ static int stmmac_hw_setup(struct net_device *dev)
> > stmmac_set_rings_length(priv);
> >
> > /* Enable TSO */
> > - if (priv->tso) {
> > + if (priv->gso_enabled_types) {
> > for (chan = 0; chan < tx_cnt; chan++) {
> > struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
> >
>
> ...
>
> > @@ -7828,7 +7834,7 @@ static int __stmmac_dvr_probe(struct device *device,
> > ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
> > if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> > ndev->hw_features |= NETIF_F_GSO_UDP_L4;
> > - priv->tso = true;
> > + stmmac_set_gso_types(priv, true);
>
> Clearly, the issue it is regurgitating has been there for a long time
> and isn't a new issue introduced by this patch.
>
> AI needs to stop doing this, because it is encouraging multiple changes
> in a single patch, which is against the normal kernel process.
>
> As already pointed out, there are multiple issues with stmmac TSO
> support, particularly with glue drivers that enable TSO on some
> queues/channels and not others, since netdev core TSO support is
> global across all channels.
>
> So, won't the AI response in this patch - it's just another pre-
> existing issue that needs fixing in a separate patch.
Looking at the TSO vs TBS issue (which precludes the use of TSO on a
channel in stmmac) I can't find an obvious reason for this in the
available documentation. However, unfortunately, iMX8MP doesn't support
TSO, so the TSO bits are elided there, but does support TBS (needing
enhanced descriptors to be enabled). STM32MP151 on the other hand
supports TSO but not TBS, and thus fails to mention anything about
enhanced descriptors or TBS.
When stmmac_enable_tbs() enables TBS, it isn't actually enabling a
feature specific bit, but switching the channel to use enhanced
descriptor format. This format extends the basic descriptors by
placing four extra 32-bit words before the basic descriptor.
Looking at the enhanced normal descriptor format for TDES3, it
indicates that the format includes bit 18 in the control field, which
is the TSE bit (TCP segmentation enable for this packet.) So, it seems
it's not a limitation of the descriptor format.
So, either "TSO and TBS cannot co-exist" is incorrect, or there is a
hardware limitation that isn't documented between these two manuals.
One other interesting point is that stmmac_tso_xmit() seems to
handle the case where TSO and TBS are enabled on the channel:
if (tx_q->tbs & STMMAC_TBS_AVAIL)
mss_desc = &tx_q->dma_entx[tx_q->cur_tx].basic;
else
mss_desc = &tx_q->dma_tx[tx_q->cur_tx];
stmmac_set_mss(priv, mss_desc, mss);
...
if (tx_q->tbs & STMMAC_TBS_AVAIL)
desc = &tx_q->dma_entx[first_entry].basic;
else
desc = &tx_q->dma_tx[first_entry];
first = desc;
etc.
Avoiding enabling TSO for a TBS channel was added by this commit:
commit 5e6038b88a5718910dd74b949946d9d9cee9a041
Author: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Wed Apr 21 17:11:49 2021 +0800
net: stmmac: fix TSO and TBS feature enabling during driver open
TSO and TBS cannot co-exist and current implementation requires two
fixes:
1) stmmac_open() does not need to call stmmac_enable_tbs() because
the MAC is reset in stmmac_init_dma_engine() anyway.
2) Inside stmmac_hw_setup(), we should call stmmac_enable_tso() for
TX Q that is _not_ configured for TBS.
Fixes: 579a25a854d4 ("net: stmmac: Initial support for TBS")
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
which doesn't really explain the background, and leaves all the TBS
cruft in the TSO transmit path (nothing like properly updating the
driver, eh? No wonder stmmac is such a mess!)
Maybe Ong Boon Leong can indicate where this restriction comes from?
Note: as mentioned previously, disabling TSO only on some channels is
actually wrong - the netdev core doesn't know which channels support
TSO and which don't, so the driver is likely to still get TSO skbuffs
for channels that the above commit has disabled TSO support. So, if
TBS is enabled and it is incompatible with TSO, then either we need
to use software TSO support _or_ disable TSO for the entire interface.
Incidentally, while looking at this, I found a few more pre-conditions
for TSO:
- TxPBL must be >= 4
- MSS[13:0] must be more than the configured data width in bytes, up to
a maximum of 1023 bytes.
I'm fairly certain that the driver does nothing to ensure that this
in the case for either of these two points.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH] net/ftgmac100: fix ring allocation unwind leaks on open failure
From: Yufan Chen @ 2026-03-28 9:24 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel
Cc: yufan.chen, Yufan Chen
From: Yufan Chen <ericterminal@gmail.com>
ftgmac100_alloc_rings() allocated rx_skbs, tx_skbs, rxdes, txdes, and rx_scratch in stages but returned directly on any intermediate allocation failure. This left previously allocated objects unreleased and could accumulate leaks across repeated ifup retries under memory pressure.
Switch ftgmac100_alloc_rings() to a centralized goto-based unwind path that calls ftgmac100_free_rings() on failure so partially allocated ring resources are always released before returning -ENOMEM.
Also clear rx_skbs, tx_skbs, and rx_scratch pointers in ftgmac100_free_rings() after freeing. This prevents stale pointers from being reused during later retry failures and keeps ring teardown idempotent.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
drivers/net/ethernet/faraday/ftgmac100.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 1e91e79c8..147300e60 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -946,7 +946,9 @@ static void ftgmac100_free_rings(struct ftgmac100 *priv)
{
/* Free skb arrays */
kfree(priv->rx_skbs);
+ priv->rx_skbs = NULL;
kfree(priv->tx_skbs);
+ priv->tx_skbs = NULL;
/* Free descriptors */
if (priv->rxdes)
@@ -965,31 +967,34 @@ static void ftgmac100_free_rings(struct ftgmac100 *priv)
if (priv->rx_scratch)
dma_free_coherent(priv->dev, RX_BUF_SIZE,
priv->rx_scratch, priv->rx_scratch_dma);
+ priv->rx_scratch = NULL;
}
static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
{
+ int err = -ENOMEM;
+
/* Allocate skb arrays */
priv->rx_skbs = kcalloc(MAX_RX_QUEUE_ENTRIES, sizeof(void *),
GFP_KERNEL);
if (!priv->rx_skbs)
- return -ENOMEM;
+ goto out;
priv->tx_skbs = kcalloc(MAX_TX_QUEUE_ENTRIES, sizeof(void *),
GFP_KERNEL);
if (!priv->tx_skbs)
- return -ENOMEM;
+ goto out;
/* Allocate descriptors */
priv->rxdes = dma_alloc_coherent(priv->dev,
MAX_RX_QUEUE_ENTRIES * sizeof(struct ftgmac100_rxdes),
&priv->rxdes_dma, GFP_KERNEL);
if (!priv->rxdes)
- return -ENOMEM;
+ goto out;
priv->txdes = dma_alloc_coherent(priv->dev,
MAX_TX_QUEUE_ENTRIES * sizeof(struct ftgmac100_txdes),
&priv->txdes_dma, GFP_KERNEL);
if (!priv->txdes)
- return -ENOMEM;
+ goto out;
/* Allocate scratch packet buffer */
priv->rx_scratch = dma_alloc_coherent(priv->dev,
@@ -997,9 +1002,13 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
&priv->rx_scratch_dma,
GFP_KERNEL);
if (!priv->rx_scratch)
- return -ENOMEM;
+ goto out;
return 0;
+
+out:
+ ftgmac100_free_rings(priv);
+ return err;
}
static void ftgmac100_init_rings(struct ftgmac100 *priv)
--
2.47.3
^ permalink raw reply related
* [PATCH v4] net: stmmac: skip VLAN restore when VLAN hash ops are missing
From: Michal Piekos @ 2026-03-28 8:55 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Ovidiu Panait
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
Michal Piekos
stmmac_vlan_restore() unconditionally calls stmmac_vlan_update() when
NETIF_F_VLAN_FEATURES is set. On platforms where priv->hw->vlan (or
->update_vlan_hash) is not provided, stmmac_update_vlan_hash() returns
-EINVAL via stmmac_do_void_callback(), resulting in a spurious
"Failed to restore VLANs" error even when no VLAN filtering is in use.
Remove not needed comment.
Remove not used return value from stmmac_vlan_restore().
Tested on Orange Pi Zero 3.
Fixes: bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
This patch fixes a noisy "Failed to restore VLANs" message on platforms
where stmmac VLAN hash ops are not implemented.
stmmac_vlan_restore() calls stmmac_vlan_update() without checking for
VLAN hash ops presence which results in -EINVAL.
---
Changes in v4:
- Remove not used return value of stmmac_vlan_restore()
- Link to v3: https://lore.kernel.org/r/20260328-vlan-restore-error-v3-1-df47a039c6f6@mmpsystems.pl
Changes in v3:
- Remove the offending comment
- Restore the original check for NETIF_F_VLAN_FEATURES
- Link to v2: https://lore.kernel.org/r/20260321-vlan-restore-error-v2-1-45cf56a5223d@mmpsystems.pl
Changes in v2:
- Replace check for hash ops with check for HW FILTER flags
- Link to v1: https://lore.kernel.org/r/20260314-vlan-restore-error-v1-1-4fc6c3e2115f@mmpsystems.pl
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..13d3cac056be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -156,7 +156,7 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);
static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
u32 rxmode, u32 chan);
-static int stmmac_vlan_restore(struct stmmac_priv *priv);
+static void stmmac_vlan_restore(struct stmmac_priv *priv);
#ifdef CONFIG_DEBUG_FS
static const struct net_device_ops stmmac_netdev_ops;
@@ -6859,21 +6859,15 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
return ret;
}
-static int stmmac_vlan_restore(struct stmmac_priv *priv)
+static void stmmac_vlan_restore(struct stmmac_priv *priv)
{
- int ret;
-
if (!(priv->dev->features & NETIF_F_VLAN_FEATURES))
- return 0;
+ return;
if (priv->hw->num_vlan)
stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
- ret = stmmac_vlan_update(priv, priv->num_double_vlans);
- if (ret)
- netdev_err(priv->dev, "Failed to restore VLANs\n");
-
- return ret;
+ stmmac_vlan_update(priv, priv->num_double_vlans);
}
static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)
---
base-commit: be762d8b6dd7efacb61937d20f8475db8f207655
change-id: 20260314-vlan-restore-error-f8b3a1c7f50a
Best regards,
--
Michal Piekos <michal.piekos@mmpsystems.pl>
^ permalink raw reply related
* [PATCH iproute2-next] dpll: Send object per event in JSON monitor mode
From: Vitaly Grinberg @ 2026-03-28 8:45 UTC (permalink / raw)
To: netdev; +Cc: stephen
Previously, monitor mode wrapped all events in a single JSON array
inside a top-level object, which made piping the output to external
tools (such as `jq`) impossible.
Send a separate JSON object for each event in monitor mode,
making the output suitable for line-by-line consumers. Skip the
global JSON wrapper for monitor mode.
Signed-off-by: Vitaly Grinberg <vgrinber@redhat.com>
---
dpll/dpll.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/dpll/dpll.c b/dpll/dpll.c
index 7b05bf86..6736c85e 100644
--- a/dpll/dpll.c
+++ b/dpll/dpll.c
@@ -571,8 +571,15 @@ int main(int argc, char **argv)
argc -= optind;
argv += optind;
- new_json_obj_plain(json);
- open_json_object(NULL);
+ /* Monitor emits one JSON object per event for streaming;
+ * other commands use a single JSON wrapper object.
+ */
+ bool is_monitor = argc > 0 && strcmp(argv[0], "monitor") == 0;
+
+ if (!is_monitor) {
+ new_json_obj_plain(json);
+ open_json_object(NULL);
+ }
/* Skip netlink init for help commands */
bool need_nl = true;
@@ -602,8 +609,10 @@ dpll_fini:
if (need_nl)
dpll_fini(dpll);
json_cleanup:
- close_json_object();
- delete_json_obj_plain();
+ if (!is_monitor) {
+ close_json_object();
+ delete_json_obj_plain();
+ }
dpll_free:
dpll_free(dpll);
return ret;
@@ -1840,6 +1849,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)
mnl_attr_parse(nlh, sizeof(struct genlmsghdr), attr_cb, tb);
+ new_json_obj_plain(json);
open_json_object(NULL);
print_string(PRINT_JSON, "name", NULL, json_name);
open_json_object("msg");
@@ -1849,6 +1859,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)
close_json_object();
close_json_object();
+ delete_json_obj_plain();
break;
}
case DPLL_CMD_PIN_CREATE_NTF:
@@ -1867,6 +1878,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)
json_name = "pin-delete-ntf";
}
+ new_json_obj_plain(json);
open_json_object(NULL);
print_string(PRINT_JSON, "name", NULL, json_name);
open_json_object("msg");
@@ -1876,6 +1888,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)
close_json_object();
close_json_object();
+ delete_json_obj_plain();
break;
}
default:
@@ -1941,8 +1954,6 @@ static int cmd_monitor(struct dpll *dpll)
goto err_signalfd;
}
- open_json_array(PRINT_JSON, "monitor");
-
pfds[0].fd = signal_fd;
pfds[0].events = POLLIN;
pfds[1].fd = netlink_fd;
@@ -1975,8 +1986,6 @@ static int cmd_monitor(struct dpll *dpll)
}
}
- close_json_array(PRINT_JSON, NULL);
-
err_signalfd:
if (signal_fd >= 0)
close(signal_fd);
--
2.52.0
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-28 8:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Paolo Abeni
In-Reply-To: <E1w63fl-0000000E3Vb-3JkY@rmk-PC.armlinux.org.uk>
On Fri, Mar 27, 2026 at 09:40:09AM +0000, Russell King (Oracle) wrote:
> The test in stmmac_xmit() to see whether we should pass the skbuff to
> stmmac_tso_xmit() is more complex than it needs to be. This test can
> be simplified by storing the mask of GSO types that we will pass, and
> setting it according to the enabled features.
>
> Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
> stmmac: Implement UDP Segmentation Offload"). Also note that this
> commit controls both via the TSO feature. We preserve this behaviour
> in this commit.
>
> Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
> for all frames, even when skb_is_gso() was false. This access is
> eliminated.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
AI review of this patch regurgitates Jakub's point that was discussed.
> @@ -3700,7 +3700,7 @@ static int stmmac_hw_setup(struct net_device *dev)
> stmmac_set_rings_length(priv);
>
> /* Enable TSO */
> - if (priv->tso) {
> + if (priv->gso_enabled_types) {
> for (chan = 0; chan < tx_cnt; chan++) {
> struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
>
...
> @@ -7828,7 +7834,7 @@ static int __stmmac_dvr_probe(struct device *device,
> ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
> if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> ndev->hw_features |= NETIF_F_GSO_UDP_L4;
> - priv->tso = true;
> + stmmac_set_gso_types(priv, true);
Clearly, the issue it is regurgitating has been there for a long time
and isn't a new issue introduced by this patch.
AI needs to stop doing this, because it is encouraging multiple changes
in a single patch, which is against the normal kernel process.
As already pointed out, there are multiple issues with stmmac TSO
support, particularly with glue drivers that enable TSO on some
queues/channels and not others, since netdev core TSO support is
global across all channels.
So, won't the AI response in this patch - it's just another pre-
existing issue that needs fixing in a separate patch.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v3] net: stmmac: skip VLAN restore when VLAN hash ops are missing
From: Michal Piekos @ 2026-03-28 8:17 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Ovidiu Panait,
netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <aceIeFWGAZwr1njk@shell.armlinux.org.uk>
On Sat, Mar 28, 2026 at 07:51:20AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 07:51:53AM +0100, Michal Piekos wrote:
> > stmmac_vlan_restore() unconditionally calls stmmac_vlan_update() when
> > NETIF_F_VLAN_FEATURES is set. On platforms where priv->hw->vlan (or
> > ->update_vlan_hash) is not provided, stmmac_update_vlan_hash() returns
> > -EINVAL via stmmac_do_void_callback(), resulting in a spurious
> > "Failed to restore VLANs" error even when no VLAN filtering is in use.
> >
> > Remove the unneeded comment.
> >
> > Tested on Orange Pi Zero 3.
> >
> > Fixes: bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
> > Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> > ---
> > This patch fixes a noisy "Failed to restore VLANs" message on platforms
> > where stmmac VLAN hash ops are not implemented.
> > stmmac_vlan_restore() calls stmmac_vlan_update() without checking for
> > VLAN hash ops presence which results in -EINVAL.
> > ---
> > Changes in v3:
> > - Remove the offending comment
> > - Restore the original check for NETIF_F_VLAN_FEATURES
> > - Link to v2: https://lore.kernel.org/r/20260321-vlan-restore-error-v2-1-45cf56a5223d@mmpsystems.pl
> >
> > Changes in v2:
> > - Replace check for hash ops with check for HW FILTER flags
> > - Link to v1: https://lore.kernel.org/r/20260314-vlan-restore-error-v1-1-4fc6c3e2115f@mmpsystems.pl
> > ---
> > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6827c99bde8c..0f3e5ac05faa 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -6861,19 +6861,13 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
> >
> > static int stmmac_vlan_restore(struct stmmac_priv *priv)
> > {
> > - int ret;
> > -
> > if (!(priv->dev->features & NETIF_F_VLAN_FEATURES))
> > return 0;
> >
> > if (priv->hw->num_vlan)
> > stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
> >
> > - ret = stmmac_vlan_update(priv, priv->num_double_vlans);
> > - if (ret)
> > - netdev_err(priv->dev, "Failed to restore VLANs\n");
> > -
> > - return ret;
> > + return stmmac_vlan_update(priv, priv->num_double_vlans);
> > }
>
> Yes, but as both Andrew and myself have pointed out, no one checks
> the return value of stmmac_vlan_restore(), so why does it return
> int?
Was too quick with the patch. Will spin another version.
Michal
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH net-next v2 5/5] dpll: zl3073x: add ref-sync pair support
From: Ivan Vecera @ 2026-03-28 8:06 UTC (permalink / raw)
To: netdev
Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>
Add support for ref-sync pair registration using the 'ref-sync-sources'
phandle property from device tree. A ref-sync pair consists of a clock
reference and a low-frequency sync signal where the DPLL locks to the
clock reference but phase-aligns to the sync reference.
The implementation:
- Stores fwnode handle in zl3073x_dpll_pin during pin registration
- Adds ref_sync_get/set callbacks to read and write the sync control
mode and pair registers
- Validates ref-sync frequency constraints: sync signal must be 8 kHz
or less, clock reference must be 1 kHz or more and higher than sync
- Excludes sync source from automatic reference selection by setting
its priority to NONE on connect; on disconnect the priority is left
as NONE and the user must explicitly make the pin selectable again
- Iterates ref-sync-sources phandles to register declared pairings
via dpll_pin_ref_sync_pair_add()
Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/zl3073x/dpll.c | 207 +++++++++++++++++++++++++++++++++++-
1 file changed, 206 insertions(+), 1 deletion(-)
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index b5b411425f6b3..79589400f0cc2 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/netlink.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/sprintf.h>
@@ -30,6 +31,7 @@
* @dpll: DPLL the pin is registered to
* @dpll_pin: pointer to registered dpll_pin
* @tracker: tracking object for the acquired reference
+ * @fwnode: firmware node handle
* @label: package label
* @dir: pin direction
* @id: pin id
@@ -45,6 +47,7 @@ struct zl3073x_dpll_pin {
struct zl3073x_dpll *dpll;
struct dpll_pin *dpll_pin;
dpll_tracker tracker;
+ struct fwnode_handle *fwnode;
char label[8];
enum dpll_pin_direction dir;
u8 id;
@@ -184,6 +187,109 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
return zl3073x_ref_state_set(zldev, ref_id, &ref);
}
+static int
+zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
+ void *pin_priv,
+ const struct dpll_pin *ref_sync_pin,
+ void *ref_sync_pin_priv,
+ enum dpll_pin_state *state,
+ struct netlink_ext_ack *extack)
+{
+ struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+ struct zl3073x_dpll_pin *pin = pin_priv;
+ struct zl3073x_dpll *zldpll = pin->dpll;
+ struct zl3073x_dev *zldev = zldpll->dev;
+ const struct zl3073x_ref *ref;
+ u8 ref_id, mode, pair;
+
+ ref_id = zl3073x_input_pin_ref_get(pin->id);
+ ref = zl3073x_ref_state_get(zldev, ref_id);
+ mode = zl3073x_ref_sync_mode_get(ref);
+ pair = zl3073x_ref_sync_pair_get(ref);
+
+ if (mode == ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR &&
+ pair == zl3073x_input_pin_ref_get(sync_pin->id))
+ *state = DPLL_PIN_STATE_CONNECTED;
+ else
+ *state = DPLL_PIN_STATE_DISCONNECTED;
+
+ return 0;
+}
+
+static int
+zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
+ void *pin_priv,
+ const struct dpll_pin *ref_sync_pin,
+ void *ref_sync_pin_priv,
+ const enum dpll_pin_state state,
+ struct netlink_ext_ack *extack)
+{
+ struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+ struct zl3073x_dpll_pin *pin = pin_priv;
+ struct zl3073x_dpll *zldpll = pin->dpll;
+ struct zl3073x_dev *zldev = zldpll->dev;
+ u8 mode, ref_id, sync_ref_id;
+ struct zl3073x_chan chan;
+ struct zl3073x_ref ref;
+ int rc;
+
+ ref_id = zl3073x_input_pin_ref_get(pin->id);
+ sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
+ ref = *zl3073x_ref_state_get(zldev, ref_id);
+
+ if (state == DPLL_PIN_STATE_CONNECTED) {
+ const struct zl3073x_ref *sync_ref;
+ u32 ref_freq, sync_freq;
+
+ sync_ref = zl3073x_ref_state_get(zldev, sync_ref_id);
+ ref_freq = zl3073x_ref_freq_get(&ref);
+ sync_freq = zl3073x_ref_freq_get(sync_ref);
+
+ /* Sync signal must be 8 kHz or less and clock reference
+ * must be 1 kHz or more and higher than the sync signal.
+ */
+ if (sync_freq > 8000) {
+ NL_SET_ERR_MSG(extack,
+ "sync frequency must be 8 kHz or less");
+ return -EINVAL;
+ }
+ if (ref_freq < 1000) {
+ NL_SET_ERR_MSG(extack,
+ "clock frequency must be 1 kHz or more");
+ return -EINVAL;
+ }
+ if (ref_freq <= sync_freq) {
+ NL_SET_ERR_MSG(extack,
+ "clock frequency must be higher than sync frequency");
+ return -EINVAL;
+ }
+
+ zl3073x_ref_sync_pair_set(&ref, sync_ref_id);
+ mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR;
+ } else {
+ mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF;
+ }
+
+ zl3073x_ref_sync_mode_set(&ref, mode);
+
+ rc = zl3073x_ref_state_set(zldev, ref_id, &ref);
+ if (rc)
+ return rc;
+
+ /* Exclude sync source from automatic reference selection by setting
+ * its priority to NONE. On disconnect the priority is left as NONE
+ * and the user must explicitly make the pin selectable again.
+ */
+ if (state == DPLL_PIN_STATE_CONNECTED) {
+ chan = *zl3073x_chan_state_get(zldev, zldpll->id);
+ zl3073x_chan_ref_prio_set(&chan, sync_ref_id,
+ ZL_DPLL_REF_PRIO_NONE);
+ return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
+ }
+
+ return 0;
+}
+
static int
zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
const struct dpll_device *dpll, void *dpll_priv,
@@ -1100,6 +1206,8 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
.phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
.prio_get = zl3073x_dpll_input_pin_prio_get,
.prio_set = zl3073x_dpll_input_pin_prio_set,
+ .ref_sync_get = zl3073x_dpll_input_pin_ref_sync_get,
+ .ref_sync_set = zl3073x_dpll_input_pin_ref_sync_set,
.state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
.state_on_dpll_set = zl3073x_dpll_input_pin_state_on_dpll_set,
};
@@ -1190,8 +1298,11 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
if (IS_ERR(props))
return PTR_ERR(props);
- /* Save package label, esync capability and phase adjust granularity */
+ /* Save package label, fwnode, esync capability and phase adjust
+ * granularity.
+ */
strscpy(pin->label, props->package_label);
+ pin->fwnode = fwnode_handle_get(props->fwnode);
pin->esync_control = props->esync_control;
pin->phase_gran = props->dpll_props.phase_gran;
@@ -1236,6 +1347,8 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
dpll_pin_put(pin->dpll_pin, &pin->tracker);
pin->dpll_pin = NULL;
err_pin_get:
+ fwnode_handle_put(pin->fwnode);
+ pin->fwnode = NULL;
zl3073x_pin_props_put(props);
return rc;
@@ -1265,6 +1378,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
dpll_pin_put(pin->dpll_pin, &pin->tracker);
pin->dpll_pin = NULL;
+
+ fwnode_handle_put(pin->fwnode);
+ pin->fwnode = NULL;
}
/**
@@ -1735,6 +1851,88 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
kfree(zldpll);
}
+/**
+ * zl3073x_dpll_ref_sync_pair_register - register ref_sync pairs for a pin
+ * @pin: pointer to zl3073x_dpll_pin structure
+ *
+ * Iterates 'ref-sync-sources' phandles in the pin's firmware node and
+ * registers each declared pairing.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pair_register(struct zl3073x_dpll_pin *pin)
+{
+ struct zl3073x_dev *zldev = pin->dpll->dev;
+ struct fwnode_handle *fwnode;
+ struct dpll_pin *sync_pin;
+ dpll_tracker tracker;
+ int n, rc;
+
+ for (n = 0; ; n++) {
+ /* Get n'th ref-sync source */
+ fwnode = fwnode_find_reference(pin->fwnode, "ref-sync-sources",
+ n);
+ if (IS_ERR(fwnode)) {
+ rc = PTR_ERR(fwnode);
+ break;
+ }
+
+ /* Find associated dpll pin */
+ sync_pin = fwnode_dpll_pin_find(fwnode, &tracker);
+ fwnode_handle_put(fwnode);
+ if (!sync_pin) {
+ dev_warn(zldev->dev, "%s: ref-sync source %d not found",
+ pin->label, n);
+ continue;
+ }
+
+ /* Register new ref-sync pair */
+ rc = dpll_pin_ref_sync_pair_add(pin->dpll_pin, sync_pin);
+ dpll_pin_put(sync_pin, &tracker);
+
+ /* -EBUSY means pairing already exists from another DPLL's
+ * registration.
+ */
+ if (rc && rc != -EBUSY) {
+ dev_err(zldev->dev,
+ "%s: failed to add ref-sync source %d: %pe",
+ pin->label, n, ERR_PTR(rc));
+ break;
+ }
+ }
+
+ return rc != -ENOENT ? rc : 0;
+}
+
+/**
+ * zl3073x_dpll_ref_sync_pairs_register - register ref_sync pairs for a DPLL
+ * @zldpll: pointer to zl3073x_dpll structure
+ *
+ * Iterates all registered input pins of the given DPLL and establishes
+ * ref_sync pairings declared by 'ref-sync-sources' phandles in the
+ * device tree.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
+{
+ struct zl3073x_dpll_pin *pin;
+ int rc;
+
+ list_for_each_entry(pin, &zldpll->pins, list) {
+ if (!zl3073x_dpll_is_input_pin(pin) || !pin->fwnode)
+ continue;
+
+ rc = zl3073x_dpll_ref_sync_pair_register(pin);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+
/**
* zl3073x_dpll_register - register DPLL device and all its pins
* @zldpll: pointer to zl3073x_dpll structure
@@ -1758,6 +1956,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
return rc;
}
+ rc = zl3073x_dpll_ref_sync_pairs_register(zldpll);
+ if (rc) {
+ zl3073x_dpll_pins_unregister(zldpll);
+ zl3073x_dpll_device_unregister(zldpll);
+ return rc;
+ }
+
return 0;
}
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v2 4/5] dt-bindings: dpll: add ref-sync-sources property
From: Ivan Vecera @ 2026-03-28 8:06 UTC (permalink / raw)
To: netdev
Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>
Add ref-sync-sources phandle-array property to the dpll-pin schema
allowing board designers to declare which input pins can serve as
sync sources in a Reference-Sync pair. A Ref-Sync pair consists of
a clock reference and a low-frequency sync signal where the DPLL locks
to the clock but phase-aligns to the sync reference.
Update both examples in the Microchip ZL3073x binding to demonstrate
the new property with a 1 PPS sync source paired to a clock source.
Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
.../devicetree/bindings/dpll/dpll-pin.yaml | 11 +++++++
.../bindings/dpll/microchip,zl30731.yaml | 30 ++++++++++++++-----
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
index 51db93b77306f..7084f102e274c 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
@@ -36,6 +36,17 @@ properties:
description: String exposed as the pin board label
$ref: /schemas/types.yaml#/definitions/string
+ ref-sync-sources:
+ description: |
+ List of phandles to input pins that can serve as the sync source
+ in a Reference-Sync pair with this pin acting as the clock source.
+ A Ref-Sync pair consists of a clock reference and a low-frequency
+ sync signal. The DPLL locks to the clock reference but
+ phase-aligns to the sync reference.
+ Only valid for input pins. Each referenced pin must be a
+ different input pin on the same device.
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+
supported-frequencies-hz:
description: List of supported frequencies for this pin, expressed in Hz.
diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
index 17747f754b845..fa5a8f8e390cd 100644
--- a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
@@ -52,11 +52,19 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- pin@0 { /* REF0P */
+ sync0: pin@0 { /* REF0P - 1 PPS sync source */
reg = <0>;
connection-type = "ext";
- label = "Input 0";
- supported-frequencies-hz = /bits/ 64 <1 1000>;
+ label = "SMA1";
+ supported-frequencies-hz = /bits/ 64 <1>;
+ };
+
+ pin@1 { /* REF0N - clock source, can pair with sync0 */
+ reg = <1>;
+ connection-type = "ext";
+ label = "SMA2";
+ supported-frequencies-hz = /bits/ 64 <10000 10000000>;
+ ref-sync-sources = <&sync0>;
};
};
@@ -90,11 +98,19 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- pin@0 { /* REF0P */
+ sync1: pin@0 { /* REF0P - 1 PPS sync source */
reg = <0>;
- connection-type = "ext";
- label = "Input 0";
- supported-frequencies-hz = /bits/ 64 <1 1000>;
+ connection-type = "gnss";
+ label = "GNSS_1PPS_IN";
+ supported-frequencies-hz = /bits/ 64 <1>;
+ };
+
+ pin@1 { /* REF0N - clock source */
+ reg = <1>;
+ connection-type = "gnss";
+ label = "GNSS_10M_IN";
+ supported-frequencies-hz = /bits/ 64 <10000000>;
+ ref-sync-sources = <&sync1>;
};
};
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v2 3/5] dpll: zl3073x: add ref sync and output clock type helpers
From: Ivan Vecera @ 2026-03-28 8:06 UTC (permalink / raw)
To: netdev
Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>
Add ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR and ZL_REF_SYNC_CTRL_PAIR
register definitions.
Add inline helpers to get and set the sync control mode and sync pair
fields of the reference sync control register:
zl3073x_ref_sync_mode_get/set() - ZL_REF_SYNC_CTRL_MODE field
zl3073x_ref_sync_pair_get/set() - ZL_REF_SYNC_CTRL_PAIR field
Add inline helpers to get and set the clock type field of the output
mode register:
zl3073x_out_clock_type_get/set() - ZL_OUTPUT_MODE_CLOCK_TYPE field
Convert existing esync callbacks to use the new helpers.
Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/zl3073x/dpll.c | 24 ++++++++-----------
drivers/dpll/zl3073x/out.h | 22 ++++++++++++++++++
drivers/dpll/zl3073x/ref.h | 46 +++++++++++++++++++++++++++++++++++++
drivers/dpll/zl3073x/regs.h | 2 ++
4 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 2476c14028210..b5b411425f6b3 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -137,7 +137,7 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
esync->range = esync_freq_ranges;
esync->range_num = ARRAY_SIZE(esync_freq_ranges);
- switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
+ switch (zl3073x_ref_sync_mode_get(ref)) {
case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
esync->pulse = 25;
@@ -173,8 +173,7 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
else
sync_mode = ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75;
- ref.sync_ctrl &= ~ZL_REF_SYNC_CTRL_MODE;
- ref.sync_ctrl |= FIELD_PREP(ZL_REF_SYNC_CTRL_MODE, sync_mode);
+ zl3073x_ref_sync_mode_set(&ref, sync_mode);
if (freq) {
/* 1 Hz is only supported frequency now */
@@ -578,7 +577,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
const struct zl3073x_synth *synth;
const struct zl3073x_out *out;
u32 synth_freq, out_freq;
- u8 clock_type, out_id;
+ u8 out_id;
out_id = zl3073x_output_pin_out_get(pin->id);
out = zl3073x_out_state_get(zldev, out_id);
@@ -601,8 +600,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
esync->range = esync_freq_ranges;
esync->range_num = ARRAY_SIZE(esync_freq_ranges);
- clock_type = FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
- if (clock_type != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
+ if (zl3073x_out_clock_type_get(out) != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
/* No need to read esync data if it is not enabled */
esync->freq = 0;
esync->pulse = 0;
@@ -635,8 +633,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
struct zl3073x_dpll_pin *pin = pin_priv;
const struct zl3073x_synth *synth;
struct zl3073x_out out;
- u8 clock_type, out_id;
u32 synth_freq;
+ u8 out_id;
out_id = zl3073x_output_pin_out_get(pin->id);
out = *zl3073x_out_state_get(zldev, out_id);
@@ -648,15 +646,13 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
if (zl3073x_out_is_ndiv(&out))
return -EOPNOTSUPP;
- /* Select clock type */
+ /* Update clock type in output mode */
if (freq)
- clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC;
+ zl3073x_out_clock_type_set(&out,
+ ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC);
else
- clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL;
-
- /* Update clock type in output mode */
- out.mode &= ~ZL_OUTPUT_MODE_CLOCK_TYPE;
- out.mode |= FIELD_PREP(ZL_OUTPUT_MODE_CLOCK_TYPE, clock_type);
+ zl3073x_out_clock_type_set(&out,
+ ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL);
/* If esync is being disabled just write mailbox and finish */
if (!freq)
diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h
index edf40432bba5f..660889c57bffa 100644
--- a/drivers/dpll/zl3073x/out.h
+++ b/drivers/dpll/zl3073x/out.h
@@ -42,6 +42,28 @@ const struct zl3073x_out *zl3073x_out_state_get(struct zl3073x_dev *zldev,
int zl3073x_out_state_set(struct zl3073x_dev *zldev, u8 index,
const struct zl3073x_out *out);
+/**
+ * zl3073x_out_clock_type_get - get output clock type
+ * @out: pointer to out state
+ *
+ * Return: clock type of given output (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
+ */
+static inline u8 zl3073x_out_clock_type_get(const struct zl3073x_out *out)
+{
+ return FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
+}
+
+/**
+ * zl3073x_out_clock_type_set - set output clock type
+ * @out: pointer to out state
+ * @type: clock type (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
+ */
+static inline void
+zl3073x_out_clock_type_set(struct zl3073x_out *out, u8 type)
+{
+ FIELD_MODIFY(ZL_OUTPUT_MODE_CLOCK_TYPE, &out->mode, type);
+}
+
/**
* zl3073x_out_signal_format_get - get output signal format
* @out: pointer to out state
diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
index 06d8d4d97ea26..09fab97a71d7e 100644
--- a/drivers/dpll/zl3073x/ref.h
+++ b/drivers/dpll/zl3073x/ref.h
@@ -106,6 +106,52 @@ zl3073x_ref_freq_set(struct zl3073x_ref *ref, u32 freq)
return 0;
}
+/**
+ * zl3073x_ref_sync_mode_get - get sync control mode
+ * @ref: pointer to ref state
+ *
+ * Return: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
+ */
+static inline u8
+zl3073x_ref_sync_mode_get(const struct zl3073x_ref *ref)
+{
+ return FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl);
+}
+
+/**
+ * zl3073x_ref_sync_mode_set - set sync control mode
+ * @ref: pointer to ref state
+ * @mode: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
+ */
+static inline void
+zl3073x_ref_sync_mode_set(struct zl3073x_ref *ref, u8 mode)
+{
+ FIELD_MODIFY(ZL_REF_SYNC_CTRL_MODE, &ref->sync_ctrl, mode);
+}
+
+/**
+ * zl3073x_ref_sync_pair_get - get sync pair reference index
+ * @ref: pointer to ref state
+ *
+ * Return: paired reference index
+ */
+static inline u8
+zl3073x_ref_sync_pair_get(const struct zl3073x_ref *ref)
+{
+ return FIELD_GET(ZL_REF_SYNC_CTRL_PAIR, ref->sync_ctrl);
+}
+
+/**
+ * zl3073x_ref_sync_pair_set - set sync pair reference index
+ * @ref: pointer to ref state
+ * @pair: paired reference index
+ */
+static inline void
+zl3073x_ref_sync_pair_set(struct zl3073x_ref *ref, u8 pair)
+{
+ FIELD_MODIFY(ZL_REF_SYNC_CTRL_PAIR, &ref->sync_ctrl, pair);
+}
+
/**
* zl3073x_ref_is_diff - check if the given input reference is differential
* @ref: pointer to ref state
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index 5ae50cb761a97..d425dc67250fe 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -213,7 +213,9 @@
#define ZL_REG_REF_SYNC_CTRL ZL_REG(10, 0x2e, 1)
#define ZL_REF_SYNC_CTRL_MODE GENMASK(2, 0)
#define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF 0
+#define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR 1
#define ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75 2
+#define ZL_REF_SYNC_CTRL_PAIR GENMASK(7, 4)
#define ZL_REG_REF_ESYNC_DIV ZL_REG(10, 0x30, 4)
#define ZL_REF_ESYNC_DIV_1HZ 0
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v2 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns
From: Ivan Vecera @ 2026-03-28 8:06 UTC (permalink / raw)
To: netdev
Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>
Replace open-coded clear-and-set bitfield operations with
FIELD_MODIFY().
Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/zl3073x/chan.h | 17 ++++++-----------
drivers/dpll/zl3073x/core.c | 3 +--
drivers/dpll/zl3073x/flash.c | 3 +--
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index e0f02d3432086..481da2133202b 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -66,8 +66,7 @@ static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
*/
static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
{
- chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
- chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
+ FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_MODE, &chan->mode_refsel, mode);
}
/**
@@ -77,8 +76,7 @@ static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
*/
static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
{
- chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
- chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+ FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_REF, &chan->mode_refsel, ref);
}
/**
@@ -110,13 +108,10 @@ zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)
{
u8 *val = &chan->ref_prio[ref / 2];
- if (!(ref & 1)) {
- *val &= ~ZL_DPLL_REF_PRIO_REF_P;
- *val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
- } else {
- *val &= ~ZL_DPLL_REF_PRIO_REF_N;
- *val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
- }
+ if (!(ref & 1))
+ FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_P, val, prio);
+ else
+ FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_N, val, prio);
}
/**
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 6363002d48d46..7eebfc1ad1019 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -743,8 +743,7 @@ int zl3073x_dev_phase_avg_factor_set(struct zl3073x_dev *zldev, u8 factor)
value = (factor + 1) & 0x0f;
/* Update phase measurement control register */
- dpll_meas_ctrl &= ~ZL_DPLL_MEAS_CTRL_AVG_FACTOR;
- dpll_meas_ctrl |= FIELD_PREP(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, value);
+ FIELD_MODIFY(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, &dpll_meas_ctrl, value);
rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, dpll_meas_ctrl);
if (rc)
return rc;
diff --git a/drivers/dpll/zl3073x/flash.c b/drivers/dpll/zl3073x/flash.c
index 83452a77e3e98..f85535c8ad246 100644
--- a/drivers/dpll/zl3073x/flash.c
+++ b/drivers/dpll/zl3073x/flash.c
@@ -194,8 +194,7 @@ zl3073x_flash_cmd_wait(struct zl3073x_dev *zldev, u32 operation,
if (rc)
return rc;
- value &= ~ZL_WRITE_FLASH_OP;
- value |= FIELD_PREP(ZL_WRITE_FLASH_OP, operation);
+ FIELD_MODIFY(ZL_WRITE_FLASH_OP, &value, operation);
rc = zl3073x_write_u8(zldev, ZL_REG_WRITE_FLASH, value);
if (rc)
--
2.52.0
^ permalink raw reply related
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