[PATCH net-next 00/15] net: macb: various cleanups

[PATCH net-next 00/15] net: macb: various cleanups

[Théo Lebrun]

Fix many oddities inside the MACB driver. They accumulated in my
work-in-progress branch while working on MACB/GEM EyeQ5 support.

Part of this series has been seen on the lkml in March then June.
See below for a semblance of a changelog.

The initial goal was to post them alongside EyeQ5 support, but that
makes for too big of a series. It'll come afterwards, with new
features (interrupt coalescing, ethtool .set_channels() and XDP mostly).

Thanks,
Have a nice day,
Théo

[0]: https://lore.kernel.org/lkml/20250627-macb-v2-0-ff8207d0bb77@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes since June V2:
 - Six patches are straight copies:
   dt-bindings: net: cdns,macb: sort compatibles
   net: macb: use BIT() macro for capability definitions
   net: macb: Remove local variables clk_init and init in macb_probe()
   net: macb: drop macb_config NULL checking
   net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
   net: macb: sort #includes
 - The "introduce DMA descriptor helpers" patch was split in two:
   net: macb: simplify macb_dma_desc_get_size()
   net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
 - Three patches come from Sean's feedback:
   net: macb: remove gap in MACB_CAPS_* flags
   net: macb: simplify macb_adj_dma_desc_idx()
   net: macb: move bp->hw_dma_cap flags to bp->caps
 - Take 1x Reviewed-by: Krzysztof Kozlowski
 - Take 3x Reviewed-by: Sean Anderson
 - Link: https://lore.kernel.org/lkml/20250627-macb-v2-0-ff8207d0bb77@bootlin.com/

---
Théo Lebrun (15):
      dt-bindings: net: cdns,macb: sort compatibles
      net: macb: use BIT() macro for capability definitions
      net: macb: remove gap in MACB_CAPS_* flags
      net: macb: Remove local variables clk_init and init in macb_probe()
      net: macb: drop macb_config NULL checking
      net: macb: simplify macb_dma_desc_get_size()
      net: macb: simplify macb_adj_dma_desc_idx()
      net: macb: move bp->hw_dma_cap flags to bp->caps
      net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
      net: macb: remove bp->queue_mask
      net: macb: replace min() with umin() calls
      net: macb: drop `entry` local variable in macb_tx_map()
      net: macb: drop `count` local variable in macb_tx_map()
      net: macb: apply reverse christmas tree in macb_tx_map()
      net: macb: sort #includes

 .../devicetree/bindings/net/cdns,macb.yaml         |   8 +-
 drivers/net/ethernet/cadence/macb.h                |  71 +++---
 drivers/net/ethernet/cadence/macb_main.c           | 257 +++++++++------------
 drivers/net/ethernet/cadence/macb_ptp.c            |  16 +-
 4 files changed, 151 insertions(+), 201 deletions(-)
---
base-commit: 6a445aebc188bdb9a82519c5fe64eb92b1d025b9
change-id: 20251014-macb-cleanup-2ce7b8b1ec56

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH net-next 01/15] dt-bindings: net: cdns,macb: sort compatibles

[Théo Lebrun]

Compatibles inside this enum are sorted-ish. Make it sorted.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/net/cdns,macb.yaml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index 1029786a855c569f32171cd3484b0043622e9fc4..02f14a0b72f9c5c489c2e9a605d7f020c124fe31 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -47,18 +47,18 @@ properties:
           - const: cdns,macb          # Generic
 
       - enum:
-          - atmel,sama5d29-gem        # GEM XL IP (10/100) on Atmel sama5d29 SoCs
           - atmel,sama5d2-gem         # GEM IP (10/100) on Atmel sama5d2 SoCs
+          - atmel,sama5d29-gem        # GEM XL IP (10/100) on Atmel sama5d29 SoCs
           - atmel,sama5d3-gem         # Gigabit IP on Atmel sama5d3 SoCs
           - atmel,sama5d4-gem         # GEM IP (10/100) on Atmel sama5d4 SoCs
+          - cdns,emac                 # Generic
+          - cdns,gem                  # Generic
+          - cdns,macb                 # Generic
           - cdns,np4-macb             # NP4 SoC devices
           - microchip,sama7g5-emac    # Microchip SAMA7G5 ethernet interface
           - microchip,sama7g5-gem     # Microchip SAMA7G5 gigabit ethernet interface
           - raspberrypi,rp1-gem       # Raspberry Pi RP1 gigabit ethernet interface
           - sifive,fu540-c000-gem     # SiFive FU540-C000 SoC
-          - cdns,emac                 # Generic
-          - cdns,gem                  # Generic
-          - cdns,macb                 # Generic
 
       - items:
           - enum:

-- 
2.51.0

Re: [PATCH net-next 01/15] dt-bindings: net: cdns,macb: sort compatibles

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:02PM +0200, Théo Lebrun wrote:
> Compatibles inside this enum are sorted-ish. Make it sorted.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 02/15] net: macb: use BIT() macro for capability definitions

[Théo Lebrun]

Replace all capabilities values by calls to the BIT() macro.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Sean Anderson <sean.anderson@linux.dev>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h | 42 ++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 0830c48973aa0b9991f06c142d83b850572f2388..869d02284707cb771233276a073e1afdeeba43ce 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -756,27 +756,27 @@
 #define MACB_MAN_C45_CODE			2
 
 /* Capability mask bits */
-#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x00000001
-#define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
-#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	0x00000004
-#define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
-#define MACB_CAPS_USRIO_DISABLED		0x00000010
-#define MACB_CAPS_JUMBO				0x00000020
-#define MACB_CAPS_GEM_HAS_PTP			0x00000040
-#define MACB_CAPS_BD_RD_PREFETCH		0x00000080
-#define MACB_CAPS_NEEDS_RSTONUBR		0x00000100
-#define MACB_CAPS_MIIONRGMII			0x00000200
-#define MACB_CAPS_NEED_TSUCLK			0x00000400
-#define MACB_CAPS_QUEUE_DISABLE			0x00000800
-#define MACB_CAPS_QBV				0x00001000
-#define MACB_CAPS_PCS				0x01000000
-#define MACB_CAPS_HIGH_SPEED			0x02000000
-#define MACB_CAPS_CLK_HW_CHG			0x04000000
-#define MACB_CAPS_MACB_IS_EMAC			0x08000000
-#define MACB_CAPS_FIFO_MODE			0x10000000
-#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
-#define MACB_CAPS_SG_DISABLED			0x40000000
-#define MACB_CAPS_MACB_IS_GEM			0x80000000
+#define MACB_CAPS_ISR_CLEAR_ON_WRITE		BIT(0)
+#define MACB_CAPS_USRIO_HAS_CLKEN		BIT(1)
+#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	BIT(2)
+#define MACB_CAPS_NO_GIGABIT_HALF		BIT(3)
+#define MACB_CAPS_USRIO_DISABLED		BIT(4)
+#define MACB_CAPS_JUMBO				BIT(5)
+#define MACB_CAPS_GEM_HAS_PTP			BIT(6)
+#define MACB_CAPS_BD_RD_PREFETCH		BIT(7)
+#define MACB_CAPS_NEEDS_RSTONUBR		BIT(8)
+#define MACB_CAPS_MIIONRGMII			BIT(9)
+#define MACB_CAPS_NEED_TSUCLK			BIT(10)
+#define MACB_CAPS_QUEUE_DISABLE			BIT(11)
+#define MACB_CAPS_QBV				BIT(12)
+#define MACB_CAPS_PCS				BIT(24)
+#define MACB_CAPS_HIGH_SPEED			BIT(25)
+#define MACB_CAPS_CLK_HW_CHG			BIT(26)
+#define MACB_CAPS_MACB_IS_EMAC			BIT(27)
+#define MACB_CAPS_FIFO_MODE			BIT(28)
+#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(29)
+#define MACB_CAPS_SG_DISABLED			BIT(30)
+#define MACB_CAPS_MACB_IS_GEM			BIT(31)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01

-- 
2.51.0

[PATCH net-next 03/15] net: macb: remove gap in MACB_CAPS_* flags

[Théo Lebrun]

MACB_CAPS_* are bit constants that get used in bp->caps. They occupy
bits 0..12 + 24..31. Remove 11..23 gap by moving bits 24..31 to 13..20.

Occupation bitfields:

   31  29  27  25  23  21  19  17  15  13  11  09  07  05  03  01
     30  28  26  24  22  20  18  16  14  12  10  08  06  04  02  00
   -- Before ------------------------------------------------------
    1 1 1 1 1 1 1 1                       1 1 1 1 1 1 1 1 1 1 1 1 1
                    0 0 0 0 0 0 0 0 0 0 0
   -- After -------------------------------------------------------
                          1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
    0 0 0 0 0 0 0 0 0 0 0

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 869d02284707cb771233276a073e1afdeeba43ce..9d21ec482c8c62da28f9d5ff35d5ca46f293eb64 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -769,14 +769,14 @@
 #define MACB_CAPS_NEED_TSUCLK			BIT(10)
 #define MACB_CAPS_QUEUE_DISABLE			BIT(11)
 #define MACB_CAPS_QBV				BIT(12)
-#define MACB_CAPS_PCS				BIT(24)
-#define MACB_CAPS_HIGH_SPEED			BIT(25)
-#define MACB_CAPS_CLK_HW_CHG			BIT(26)
-#define MACB_CAPS_MACB_IS_EMAC			BIT(27)
-#define MACB_CAPS_FIFO_MODE			BIT(28)
-#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(29)
-#define MACB_CAPS_SG_DISABLED			BIT(30)
-#define MACB_CAPS_MACB_IS_GEM			BIT(31)
+#define MACB_CAPS_PCS				BIT(13)
+#define MACB_CAPS_HIGH_SPEED			BIT(14)
+#define MACB_CAPS_CLK_HW_CHG			BIT(15)
+#define MACB_CAPS_MACB_IS_EMAC			BIT(16)
+#define MACB_CAPS_FIFO_MODE			BIT(17)
+#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(18)
+#define MACB_CAPS_SG_DISABLED			BIT(19)
+#define MACB_CAPS_MACB_IS_GEM			BIT(20)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01

-- 
2.51.0

Re: [PATCH net-next 03/15] net: macb: remove gap in MACB_CAPS_* flags

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:04PM +0200, Théo Lebrun wrote:
> MACB_CAPS_* are bit constants that get used in bp->caps. They occupy
> bits 0..12 + 24..31. Remove 11..23 gap by moving bits 24..31 to 13..20.
> 
> Occupation bitfields:
> 
>    31  29  27  25  23  21  19  17  15  13  11  09  07  05  03  01
>      30  28  26  24  22  20  18  16  14  12  10  08  06  04  02  00
>    -- Before ------------------------------------------------------
>     1 1 1 1 1 1 1 1                       1 1 1 1 1 1 1 1 1 1 1 1 1
>                     0 0 0 0 0 0 0 0 0 0 0
>    -- After -------------------------------------------------------
>                           1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
>     0 0 0 0 0 0 0 0 0 0 0
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 04/15] net: macb: Remove local variables clk_init and init in macb_probe()

[Théo Lebrun]

Remove local variables clk_init and init. Those function pointers are
always equivalent to macb_config->clk_init and macb_config->init.

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ca2386b8347371810845290f6a7177a326fda4e7..dad1188ef9d87fc3846590032995223174177e89 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5424,10 +5424,6 @@ static const struct macb_config default_gem_config = {
 static int macb_probe(struct platform_device *pdev)
 {
 	const struct macb_config *macb_config = &default_gem_config;
-	int (*clk_init)(struct platform_device *, struct clk **,
-			struct clk **, struct clk **,  struct clk **,
-			struct clk **) = macb_config->clk_init;
-	int (*init)(struct platform_device *) = macb_config->init;
 	struct device_node *np = pdev->dev.of_node;
 	struct clk *pclk, *hclk = NULL, *tx_clk = NULL, *rx_clk = NULL;
 	struct clk *tsu_clk = NULL;
@@ -5449,14 +5445,11 @@ static int macb_probe(struct platform_device *pdev)
 		const struct of_device_id *match;
 
 		match = of_match_node(macb_dt_ids, np);
-		if (match && match->data) {
+		if (match && match->data)
 			macb_config = match->data;
-			clk_init = macb_config->clk_init;
-			init = macb_config->init;
-		}
 	}
 
-	err = clk_init(pdev, &pclk, &hclk, &tx_clk, &rx_clk, &tsu_clk);
+	err = macb_config->clk_init(pdev, &pclk, &hclk, &tx_clk, &rx_clk, &tsu_clk);
 	if (err)
 		return err;
 
@@ -5594,7 +5587,7 @@ static int macb_probe(struct platform_device *pdev)
 		bp->phy_interface = interface;
 
 	/* IP specific init */
-	err = init(pdev);
+	err = macb_config->init(pdev);
 	if (err)
 		goto err_out_free_netdev;
 

-- 
2.51.0

[PATCH net-next 05/15] net: macb: drop macb_config NULL checking

[Théo Lebrun]

Remove NULL checks on macb_config as it is always valid:
 - either it is its default value &default_gem_config,
 - or it got overridden using match data.

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dad1188ef9d87fc3846590032995223174177e89..33e99aab1dcb360a699d0e80762ef421001d19a1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5485,15 +5485,13 @@ static int macb_probe(struct platform_device *pdev)
 	}
 	bp->num_queues = num_queues;
 	bp->queue_mask = queue_mask;
-	if (macb_config)
-		bp->dma_burst_length = macb_config->dma_burst_length;
+	bp->dma_burst_length = macb_config->dma_burst_length;
 	bp->pclk = pclk;
 	bp->hclk = hclk;
 	bp->tx_clk = tx_clk;
 	bp->rx_clk = rx_clk;
 	bp->tsu_clk = tsu_clk;
-	if (macb_config)
-		bp->jumbo_max_len = macb_config->jumbo_max_len;
+	bp->jumbo_max_len = macb_config->jumbo_max_len;
 
 	if (!hw_is_gem(bp->regs, bp->native_io))
 		bp->max_tx_length = MACB_MAX_TX_LEN;

-- 
2.51.0

[PATCH net-next 06/15] net: macb: simplify macb_dma_desc_get_size()

[Théo Lebrun]

macb_dma_desc_get_size() does a switch on bp->hw_dma_cap and covers all
four cases: 0, 64B, PTP, 64B+PTP. It also covers the #ifndef
MACB_EXT_DESC separately, making it four codepaths.

Instead, notice the descriptor size grows with enabled features and use
plain if-statements on 64B and PTP flags.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 33e99aab1dcb360a699d0e80762ef421001d19a1..7f74e280a3351ee7f961ff5ecd9550470b2e68eb 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -121,29 +121,16 @@ struct sifive_fu540_macb_mgmt {
  */
 static unsigned int macb_dma_desc_get_size(struct macb *bp)
 {
-#ifdef MACB_EXT_DESC
-	unsigned int desc_size;
+	unsigned int desc_size = sizeof(struct macb_dma_desc);
 
-	switch (bp->hw_dma_cap) {
-	case HW_DMA_CAP_64B:
-		desc_size = sizeof(struct macb_dma_desc)
-			+ sizeof(struct macb_dma_desc_64);
-		break;
-	case HW_DMA_CAP_PTP:
-		desc_size = sizeof(struct macb_dma_desc)
-			+ sizeof(struct macb_dma_desc_ptp);
-		break;
-	case HW_DMA_CAP_64B_PTP:
-		desc_size = sizeof(struct macb_dma_desc)
-			+ sizeof(struct macb_dma_desc_64)
-			+ sizeof(struct macb_dma_desc_ptp);
-		break;
-	default:
-		desc_size = sizeof(struct macb_dma_desc);
-	}
-	return desc_size;
+#ifdef MACB_EXT_DESC
+	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+		desc_size += sizeof(struct macb_dma_desc_64);
+	if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
+		desc_size += sizeof(struct macb_dma_desc_ptp);
 #endif
-	return sizeof(struct macb_dma_desc);
+
+	return desc_size;
 }
 
 static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)

-- 
2.51.0

Re: [PATCH net-next 06/15] net: macb: simplify macb_dma_desc_get_size()

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:07PM +0200, Théo Lebrun wrote:
> macb_dma_desc_get_size() does a switch on bp->hw_dma_cap and covers all
> four cases: 0, 64B, PTP, 64B+PTP. It also covers the #ifndef
> MACB_EXT_DESC separately, making it four codepaths.
> 
> Instead, notice the descriptor size grows with enabled features and use
> plain if-statements on 64B and PTP flags.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 07/15] net: macb: simplify macb_adj_dma_desc_idx()

[Théo Lebrun]

The function body uses a switch statement on bp->hw_dma_cap and handles
its four possible values: 0, is_64b, is_ptp, is_64b && is_ptp.

Instead, refactor by noticing that the return value is:
   desc_size * MULT
with MULT = 3 if is_64b && is_ptp,
            2 if is_64b || is_ptp,
            1 otherwise.

MULT can be expressed as:
   1 + is_64b + is_ptp

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7f74e280a3351ee7f961ff5ecd9550470b2e68eb..44a411662786ca4f309d6f9389b0d36819fc40ad 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -136,19 +136,13 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
 static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)
 {
 #ifdef MACB_EXT_DESC
-	switch (bp->hw_dma_cap) {
-	case HW_DMA_CAP_64B:
-	case HW_DMA_CAP_PTP:
-		desc_idx <<= 1;
-		break;
-	case HW_DMA_CAP_64B_PTP:
-		desc_idx *= 3;
-		break;
-	default:
-		break;
-	}
-#endif
+	bool is_ptp = bp->hw_dma_cap & HW_DMA_CAP_PTP;
+	bool is_64b = bp->hw_dma_cap & HW_DMA_CAP_64B;
+
+	return desc_idx * (1 + is_64b + is_ptp);
+#else
 	return desc_idx;
+#endif
 }
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT

-- 
2.51.0

Re: [PATCH net-next 07/15] net: macb: simplify macb_adj_dma_desc_idx()

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:08PM +0200, Théo Lebrun wrote:
> The function body uses a switch statement on bp->hw_dma_cap and handles
> its four possible values: 0, is_64b, is_ptp, is_64b && is_ptp.
> 
> Instead, refactor by noticing that the return value is:
>    desc_size * MULT
> with MULT = 3 if is_64b && is_ptp,
>             2 if is_64b || is_ptp,
>             1 otherwise.
> 
> MULT can be expressed as:
>    1 + is_64b + is_ptp
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 7f74e280a3351ee7f961ff5ecd9550470b2e68eb..44a411662786ca4f309d6f9389b0d36819fc40ad 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -136,19 +136,13 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
>  static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)
>  {
>  #ifdef MACB_EXT_DESC
> -	switch (bp->hw_dma_cap) {
> -	case HW_DMA_CAP_64B:
> -	case HW_DMA_CAP_PTP:
> -		desc_idx <<= 1;
> -		break;
> -	case HW_DMA_CAP_64B_PTP:

I _think_ this makes HW_DMA_CAP_64B_PTP unused and it can be removed?

  Andrew

Re: [PATCH net-next 07/15] net: macb: simplify macb_adj_dma_desc_idx()

[Théo Lebrun]

Hello Andrew,

On Fri Oct 17, 2025 at 8:00 PM CEST, Andrew Lunn wrote:
> On Tue, Oct 14, 2025 at 05:25:08PM +0200, Théo Lebrun wrote:
>> The function body uses a switch statement on bp->hw_dma_cap and handles
>> its four possible values: 0, is_64b, is_ptp, is_64b && is_ptp.
>> 
>> Instead, refactor by noticing that the return value is:
>>    desc_size * MULT
>> with MULT = 3 if is_64b && is_ptp,
>>             2 if is_64b || is_ptp,
>>             1 otherwise.
>> 
>> MULT can be expressed as:
>>    1 + is_64b + is_ptp
>> 
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>>  drivers/net/ethernet/cadence/macb_main.c | 18 ++++++------------
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 7f74e280a3351ee7f961ff5ecd9550470b2e68eb..44a411662786ca4f309d6f9389b0d36819fc40ad 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -136,19 +136,13 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
>>  static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)
>>  {
>>  #ifdef MACB_EXT_DESC
>> -	switch (bp->hw_dma_cap) {
>> -	case HW_DMA_CAP_64B:
>> -	case HW_DMA_CAP_PTP:
>> -		desc_idx <<= 1;
>> -		break;
>> -	case HW_DMA_CAP_64B_PTP:
>
> I _think_ this makes HW_DMA_CAP_64B_PTP unused and it can be removed?

It does indeed. That constant gets removed in the following patch
([08/15], "net: macb: move bp->hw_dma_cap flags to bp->caps").
It  appeared to make more sense to remove all HW_DMA_CAP_* at once.
You probably noticed as you continued your review.

Thanks for the review! I guess you have spotted that the series got
applied to netdev/net-next by Jakub [0].

[0]: https://lore.kernel.org/lkml/176066582948.1978978.752807229943547484.git-patchwork-notify@kernel.org/

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH net-next 08/15] net: macb: move bp->hw_dma_cap flags to bp->caps

[Théo Lebrun]

Drop bp->hw_dma_cap field and put its two flags into bp->caps.

On my specific config (eyeq5_defconfig), bloat-o-meter indicates:
 - macb_main.o: Before=56251, After=56359, chg +0.19%
 - macb_ptp.o:  Before= 3976, After= 3952, chg -0.60%

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      | 10 ++--------
 drivers/net/ethernet/cadence/macb_main.c | 28 ++++++++++++++--------------
 drivers/net/ethernet/cadence/macb_ptp.c  | 16 +++++++++-------
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 9d21ec482c8c62da28f9d5ff35d5ca46f293eb64..b3a4c9534240a3195adbd7acd325cdaafab60039 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -777,6 +777,8 @@
 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(18)
 #define MACB_CAPS_SG_DISABLED			BIT(19)
 #define MACB_CAPS_MACB_IS_GEM			BIT(20)
+#define MACB_CAPS_DMA_64B			BIT(21)
+#define MACB_CAPS_DMA_PTP			BIT(22)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
@@ -854,11 +856,6 @@ struct macb_dma_desc {
 };
 
 #ifdef MACB_EXT_DESC
-#define HW_DMA_CAP_32B		0
-#define HW_DMA_CAP_64B		(1 << 0)
-#define HW_DMA_CAP_PTP		(1 << 1)
-#define HW_DMA_CAP_64B_PTP	(HW_DMA_CAP_64B | HW_DMA_CAP_PTP)
-
 struct macb_dma_desc_64 {
 	u32 addrh;
 	u32 resvd;
@@ -1349,9 +1346,6 @@ struct macb {
 
 	struct phy		*sgmii_phy;	/* for ZynqMP SGMII mode */
 
-#ifdef MACB_EXT_DESC
-	uint8_t hw_dma_cap;
-#endif
 	spinlock_t tsu_clk_lock; /* gem tsu clock locking */
 	unsigned int tsu_rate;
 	struct ptp_clock *ptp_clock;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 44a411662786ca4f309d6f9389b0d36819fc40ad..44b96bf53ff6bcb351b27a165451cac852528501 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -124,9 +124,9 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
 	unsigned int desc_size = sizeof(struct macb_dma_desc);
 
 #ifdef MACB_EXT_DESC
-	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+	if (bp->caps & MACB_CAPS_DMA_64B)
 		desc_size += sizeof(struct macb_dma_desc_64);
-	if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
+	if (bp->caps & MACB_CAPS_DMA_PTP)
 		desc_size += sizeof(struct macb_dma_desc_ptp);
 #endif
 
@@ -136,8 +136,8 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
 static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)
 {
 #ifdef MACB_EXT_DESC
-	bool is_ptp = bp->hw_dma_cap & HW_DMA_CAP_PTP;
-	bool is_64b = bp->hw_dma_cap & HW_DMA_CAP_64B;
+	bool is_ptp = bp->caps & MACB_CAPS_DMA_PTP;
+	bool is_64b = bp->caps & MACB_CAPS_DMA_64B;
 
 	return desc_idx * (1 + is_64b + is_ptp);
 #else
@@ -475,7 +475,7 @@ static void macb_init_buffers(struct macb *bp)
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	/* Single register for all queues' high 32 bits. */
-	if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
+	if (bp->caps & MACB_CAPS_DMA_64B) {
 		macb_writel(bp, RBQPH,
 			    upper_32_bits(bp->queues[0].rx_ring_dma));
 		macb_writel(bp, TBQPH,
@@ -1009,7 +1009,7 @@ static void macb_set_addr(struct macb *bp, struct macb_dma_desc *desc, dma_addr_
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	struct macb_dma_desc_64 *desc_64;
 
-	if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
+	if (bp->caps & MACB_CAPS_DMA_64B) {
 		desc_64 = macb_64b_desc(bp, desc);
 		desc_64->addrh = upper_32_bits(addr);
 		/* The low bits of RX address contain the RX_USED bit, clearing
@@ -1028,14 +1028,14 @@ static dma_addr_t macb_get_addr(struct macb *bp, struct macb_dma_desc *desc)
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	struct macb_dma_desc_64 *desc_64;
 
-	if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
+	if (bp->caps & MACB_CAPS_DMA_64B) {
 		desc_64 = macb_64b_desc(bp, desc);
 		addr = ((u64)(desc_64->addrh) << 32);
 	}
 #endif
 	addr |= MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
 #ifdef CONFIG_MACB_USE_HWSTAMP
-	if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
+	if (bp->caps & MACB_CAPS_DMA_PTP)
 		addr &= ~GEM_BIT(DMA_RXVALID);
 #endif
 	return addr;
@@ -2301,7 +2301,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
 	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
-	    (bp->hw_dma_cap & HW_DMA_CAP_PTP))
+	    (bp->caps & MACB_CAPS_DMA_PTP))
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 #endif
 
@@ -2781,11 +2781,11 @@ static void macb_configure_dma(struct macb *bp)
 
 		dmacfg &= ~GEM_BIT(ADDR64);
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+		if (bp->caps & MACB_CAPS_DMA_64B)
 			dmacfg |= GEM_BIT(ADDR64);
 #endif
 #ifdef CONFIG_MACB_USE_HWSTAMP
-		if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
+		if (bp->caps & MACB_CAPS_DMA_PTP)
 			dmacfg |= GEM_BIT(RXEXT) | GEM_BIT(TXEXT);
 #endif
 		netdev_dbg(bp->dev, "Cadence configure DMA with 0x%08x\n",
@@ -3563,7 +3563,7 @@ static int gem_get_ts_info(struct net_device *dev,
 {
 	struct macb *bp = netdev_priv(dev);
 
-	if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0) {
+	if (!(bp->caps & MACB_CAPS_DMA_PTP)) {
 		ethtool_op_get_ts_info(dev, info);
 		return 0;
 	}
@@ -4351,7 +4351,7 @@ static void macb_configure_caps(struct macb *bp,
 					"GEM doesn't support hardware ptp.\n");
 			else {
 #ifdef CONFIG_MACB_USE_HWSTAMP
-				bp->hw_dma_cap |= HW_DMA_CAP_PTP;
+				bp->caps |= MACB_CAPS_DMA_PTP;
 				bp->ptp_info = &gem_ptp_info;
 #endif
 			}
@@ -5518,7 +5518,7 @@ static int macb_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "failed to set DMA mask\n");
 			goto err_out_free_netdev;
 		}
-		bp->hw_dma_cap |= HW_DMA_CAP_64B;
+		bp->caps |= MACB_CAPS_DMA_64B;
 	}
 #endif
 	platform_set_drvdata(pdev, dev);
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index a63bf29c4fa81b95f10aec8b8fe9918022e196d6..f4ab379f28493cffe30275fd335844ae2fefc89a 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -28,14 +28,16 @@
 static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp,
 					       struct macb_dma_desc *desc)
 {
-	if (bp->hw_dma_cap == HW_DMA_CAP_PTP)
-		return (struct macb_dma_desc_ptp *)
-				((u8 *)desc + sizeof(struct macb_dma_desc));
-	if (bp->hw_dma_cap == HW_DMA_CAP_64B_PTP)
+	if (!(bp->caps & MACB_CAPS_DMA_PTP))
+		return NULL;
+
+	if (bp->caps & MACB_CAPS_DMA_64B)
 		return (struct macb_dma_desc_ptp *)
 				((u8 *)desc + sizeof(struct macb_dma_desc)
 				+ sizeof(struct macb_dma_desc_64));
-	return NULL;
+	else
+		return (struct macb_dma_desc_ptp *)
+				((u8 *)desc + sizeof(struct macb_dma_desc));
 }
 
 static int gem_tsu_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts,
@@ -380,7 +382,7 @@ int gem_get_hwtst(struct net_device *dev,
 	struct macb *bp = netdev_priv(dev);
 
 	*tstamp_config = bp->tstamp_config;
-	if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
+	if (!(bp->caps & MACB_CAPS_DMA_PTP))
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -407,7 +409,7 @@ int gem_set_hwtst(struct net_device *dev,
 	struct macb *bp = netdev_priv(dev);
 	u32 regval;
 
-	if ((bp->hw_dma_cap & HW_DMA_CAP_PTP) == 0)
+	if (!(bp->caps & MACB_CAPS_DMA_PTP))
 		return -EOPNOTSUPP;
 
 	switch (tstamp_config->tx_type) {

-- 
2.51.0

Re: [PATCH net-next 08/15] net: macb: move bp->hw_dma_cap flags to bp->caps

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:09PM +0200, Théo Lebrun wrote:
> Drop bp->hw_dma_cap field and put its two flags into bp->caps.
> 
> On my specific config (eyeq5_defconfig), bloat-o-meter indicates:
>  - macb_main.o: Before=56251, After=56359, chg +0.19%
>  - macb_ptp.o:  Before= 3976, After= 3952, chg -0.60%
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 09/15] net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)

[Théo Lebrun]

Introduce macb_dma64() and macb_dma_ptp() helper functions.
Many codepaths are made simpler by dropping conditional compilation.

This implies two additional changes:
 - Always compile related structure definitions inside <macb.h>.
 - MACB_EXT_DESC can be dropped as it is useless now.

The common case:

	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
		struct macb_dma_desc_64 *desc_64;
		if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
			desc_64 = macb_64b_desc(bp, desc);
			// ...
		}
	#endif

Is replaced by:

	if (macb_dma64(bp)) {
		struct macb_dma_desc_64 *desc_64 = macb_64b_desc(bp, desc);
		// ...
	}

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      | 18 +++++++----
 drivers/net/ethernet/cadence/macb_main.c | 55 ++++++++++----------------------
 drivers/net/ethernet/cadence/macb_ptp.c  |  8 ++---
 3 files changed, 32 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index b3a4c9534240a3195adbd7acd325cdaafab60039..f696b2ddc412bffdbdee5dd32c59b338130907f3 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -15,10 +15,6 @@
 #include <linux/phy/phy.h>
 #include <linux/workqueue.h>
 
-#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
-#define MACB_EXT_DESC
-#endif
-
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
 #define MACB_MAX_QUEUES 8
@@ -855,7 +851,6 @@ struct macb_dma_desc {
 	u32	ctrl;
 };
 
-#ifdef MACB_EXT_DESC
 struct macb_dma_desc_64 {
 	u32 addrh;
 	u32 resvd;
@@ -865,7 +860,6 @@ struct macb_dma_desc_ptp {
 	u32	ts_1;
 	u32	ts_2;
 };
-#endif
 
 /* DMA descriptor bitfields */
 #define MACB_RX_USED_OFFSET			0
@@ -1437,6 +1431,18 @@ static inline u64 enst_max_hw_interval(u32 speed_mbps)
 			    ENST_TIME_GRANULARITY_NS * 1000, (speed_mbps));
 }
 
+static inline bool macb_dma64(struct macb *bp)
+{
+	return IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
+	       bp->caps & MACB_CAPS_DMA_64B;
+}
+
+static inline bool macb_dma_ptp(struct macb *bp)
+{
+	return IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) &&
+	       bp->caps & MACB_CAPS_DMA_PTP;
+}
+
 /**
  * struct macb_platform_data - platform data for MACB Ethernet used for PCI registration
  * @pclk:		platform clock
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 44b96bf53ff6bcb351b27a165451cac852528501..9db419b94d0b47c19cdafb707b1e500124b682c8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -123,35 +123,24 @@ static unsigned int macb_dma_desc_get_size(struct macb *bp)
 {
 	unsigned int desc_size = sizeof(struct macb_dma_desc);
 
-#ifdef MACB_EXT_DESC
-	if (bp->caps & MACB_CAPS_DMA_64B)
+	if (macb_dma64(bp))
 		desc_size += sizeof(struct macb_dma_desc_64);
-	if (bp->caps & MACB_CAPS_DMA_PTP)
+	if (macb_dma_ptp(bp))
 		desc_size += sizeof(struct macb_dma_desc_ptp);
-#endif
 
 	return desc_size;
 }
 
 static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int desc_idx)
 {
-#ifdef MACB_EXT_DESC
-	bool is_ptp = bp->caps & MACB_CAPS_DMA_PTP;
-	bool is_64b = bp->caps & MACB_CAPS_DMA_64B;
-
-	return desc_idx * (1 + is_64b + is_ptp);
-#else
-	return desc_idx;
-#endif
+	return desc_idx * (1 + macb_dma64(bp) + macb_dma_ptp(bp));
 }
 
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 static struct macb_dma_desc_64 *macb_64b_desc(struct macb *bp, struct macb_dma_desc *desc)
 {
 	return (struct macb_dma_desc_64 *)((void *)desc
 		+ sizeof(struct macb_dma_desc));
 }
-#endif
 
 /* Ring buffer accessors */
 static unsigned int macb_tx_ring_wrap(struct macb *bp, unsigned int index)
@@ -473,15 +462,13 @@ static void macb_init_buffers(struct macb *bp)
 	struct macb_queue *queue;
 	unsigned int q;
 
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	/* Single register for all queues' high 32 bits. */
-	if (bp->caps & MACB_CAPS_DMA_64B) {
+	if (macb_dma64(bp)) {
 		macb_writel(bp, RBQPH,
 			    upper_32_bits(bp->queues[0].rx_ring_dma));
 		macb_writel(bp, TBQPH,
 			    upper_32_bits(bp->queues[0].tx_ring_dma));
 	}
-#endif
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
@@ -1006,10 +993,9 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_skb *tx_skb, int budge
 
 static void macb_set_addr(struct macb *bp, struct macb_dma_desc *desc, dma_addr_t addr)
 {
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	struct macb_dma_desc_64 *desc_64;
+	if (macb_dma64(bp)) {
+		struct macb_dma_desc_64 *desc_64;
 
-	if (bp->caps & MACB_CAPS_DMA_64B) {
 		desc_64 = macb_64b_desc(bp, desc);
 		desc_64->addrh = upper_32_bits(addr);
 		/* The low bits of RX address contain the RX_USED bit, clearing
@@ -1018,26 +1004,23 @@ static void macb_set_addr(struct macb *bp, struct macb_dma_desc *desc, dma_addr_
 		 */
 		dma_wmb();
 	}
-#endif
+
 	desc->addr = lower_32_bits(addr);
 }
 
 static dma_addr_t macb_get_addr(struct macb *bp, struct macb_dma_desc *desc)
 {
 	dma_addr_t addr = 0;
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	struct macb_dma_desc_64 *desc_64;
 
-	if (bp->caps & MACB_CAPS_DMA_64B) {
+	if (macb_dma64(bp)) {
+		struct macb_dma_desc_64 *desc_64;
+
 		desc_64 = macb_64b_desc(bp, desc);
 		addr = ((u64)(desc_64->addrh) << 32);
 	}
-#endif
 	addr |= MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
-#ifdef CONFIG_MACB_USE_HWSTAMP
-	if (bp->caps & MACB_CAPS_DMA_PTP)
+	if (macb_dma_ptp(bp))
 		addr &= ~GEM_BIT(DMA_RXVALID);
-#endif
 	return addr;
 }
 
@@ -2299,11 +2282,9 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		return ret;
 	}
 
-#ifdef CONFIG_MACB_USE_HWSTAMP
-	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
-	    (bp->caps & MACB_CAPS_DMA_PTP))
+	if (macb_dma_ptp(bp) &&
+	    (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-#endif
 
 	is_lso = (skb_shinfo(skb)->gso_size != 0);
 
@@ -2780,14 +2761,10 @@ static void macb_configure_dma(struct macb *bp)
 			dmacfg &= ~GEM_BIT(TXCOEN);
 
 		dmacfg &= ~GEM_BIT(ADDR64);
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		if (bp->caps & MACB_CAPS_DMA_64B)
+		if (macb_dma64(bp))
 			dmacfg |= GEM_BIT(ADDR64);
-#endif
-#ifdef CONFIG_MACB_USE_HWSTAMP
-		if (bp->caps & MACB_CAPS_DMA_PTP)
+		if (macb_dma_ptp(bp))
 			dmacfg |= GEM_BIT(RXEXT) | GEM_BIT(TXEXT);
-#endif
 		netdev_dbg(bp->dev, "Cadence configure DMA with 0x%08x\n",
 			   dmacfg);
 		gem_writel(bp, DMACFG, dmacfg);
@@ -3563,7 +3540,7 @@ static int gem_get_ts_info(struct net_device *dev,
 {
 	struct macb *bp = netdev_priv(dev);
 
-	if (!(bp->caps & MACB_CAPS_DMA_PTP)) {
+	if (!macb_dma_ptp(bp)) {
 		ethtool_op_get_ts_info(dev, info);
 		return 0;
 	}
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index f4ab379f28493cffe30275fd335844ae2fefc89a..c9e77819196e17a5b88f6dab77dadabfe087a1bd 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -28,10 +28,10 @@
 static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp,
 					       struct macb_dma_desc *desc)
 {
-	if (!(bp->caps & MACB_CAPS_DMA_PTP))
+	if (!macb_dma_ptp(bp))
 		return NULL;
 
-	if (bp->caps & MACB_CAPS_DMA_64B)
+	if (macb_dma64(bp))
 		return (struct macb_dma_desc_ptp *)
 				((u8 *)desc + sizeof(struct macb_dma_desc)
 				+ sizeof(struct macb_dma_desc_64));
@@ -382,7 +382,7 @@ int gem_get_hwtst(struct net_device *dev,
 	struct macb *bp = netdev_priv(dev);
 
 	*tstamp_config = bp->tstamp_config;
-	if (!(bp->caps & MACB_CAPS_DMA_PTP))
+	if (!macb_dma_ptp(bp))
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -409,7 +409,7 @@ int gem_set_hwtst(struct net_device *dev,
 	struct macb *bp = netdev_priv(dev);
 	u32 regval;
 
-	if (!(bp->caps & MACB_CAPS_DMA_PTP))
+	if (!macb_dma_ptp(bp))
 		return -EOPNOTSUPP;
 
 	switch (tstamp_config->tx_type) {

-- 
2.51.0

Re: [PATCH net-next 09/15] net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:10PM +0200, Théo Lebrun wrote:
> Introduce macb_dma64() and macb_dma_ptp() helper functions.
> Many codepaths are made simpler by dropping conditional compilation.
> 
> This implies two additional changes:
>  - Always compile related structure definitions inside <macb.h>.
>  - MACB_EXT_DESC can be dropped as it is useless now.
> 
> The common case:
> 
> 	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> 		struct macb_dma_desc_64 *desc_64;
> 		if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
> 			desc_64 = macb_64b_desc(bp, desc);
> 			// ...
> 		}
> 	#endif
> 
> Is replaced by:
> 
> 	if (macb_dma64(bp)) {
> 		struct macb_dma_desc_64 *desc_64 = macb_64b_desc(bp, desc);
> 		// ...
> 	}
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 10/15] net: macb: remove bp->queue_mask

[Théo Lebrun]

The low 16 bits of GEM_DCFG6 tell us which queues are enabled in HW. In
theory, there could be holes in the bitfield. In practice, the macb
driver would fail if there were holes as most loops iterate upon
bp->num_queues. Only macb_init() iterated correctly.

 - Drop bp->queue_mask field.
 - Error out at probe if a hole is in the queue mask.
 - Rely upon bp->num_queues for iteration.
 - As we drop the queue_mask probe local variable, fix RCT.
 - Compute queue_mask on the fly for TAPRIO using bp->num_queues.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      |  1 -
 drivers/net/ethernet/cadence/macb_main.c | 69 +++++++++++++++++---------------
 2 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index f696b2ddc412bffdbdee5dd32c59b338130907f3..5b7d4cdb204d8362bfa81dcc58965edbbf4dd1f8 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1290,7 +1290,6 @@ struct macb {
 	unsigned int		tx_ring_size;
 
 	unsigned int		num_queues;
-	unsigned int		queue_mask;
 	struct macb_queue	queues[MACB_MAX_QUEUES];
 
 	spinlock_t		lock;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9db419b94d0b47c19cdafb707b1e500124b682c8..98e28d51a6e12c24ef27c939363eb43c0aec1951 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4066,6 +4066,8 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
 	struct macb *bp = netdev_priv(ndev);
 	struct ethtool_link_ksettings kset;
 	struct macb_queue *queue;
+	u32 queue_mask;
+	u8 queue_id;
 	size_t i;
 	int err;
 
@@ -4117,8 +4119,9 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
 			goto cleanup;
 		}
 
-		/* gate_mask must not select queues outside the valid queue_mask */
-		if (entry->gate_mask & ~bp->queue_mask) {
+		/* gate_mask must not select queues outside the valid queues */
+		queue_id = order_base_2(entry->gate_mask);
+		if (queue_id >= bp->num_queues) {
 			netdev_err(ndev, "Entry %zu: gate_mask 0x%x exceeds queue range (max_queues=%d)\n",
 				   i, entry->gate_mask, bp->num_queues);
 			err = -EINVAL;
@@ -4152,7 +4155,7 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
 			goto cleanup;
 		}
 
-		enst_queue[i].queue_id = order_base_2(entry->gate_mask);
+		enst_queue[i].queue_id = queue_id;
 		enst_queue[i].start_time_mask =
 			(start_time_sec << GEM_START_TIME_SEC_OFFSET) |
 			start_time_nsec;
@@ -4180,8 +4183,9 @@ static int macb_taprio_setup_replace(struct net_device *ndev,
 	/* All validations passed - proceed with hardware configuration */
 	scoped_guard(spinlock_irqsave, &bp->lock) {
 		/* Disable ENST queues if running before configuring */
+		queue_mask = BIT_U32(bp->num_queues) - 1;
 		gem_writel(bp, ENST_CONTROL,
-			   bp->queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
+			   queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
 
 		for (i = 0; i < conf->num_entries; i++) {
 			queue = &bp->queues[enst_queue[i].queue_id];
@@ -4210,15 +4214,16 @@ static void macb_taprio_destroy(struct net_device *ndev)
 {
 	struct macb *bp = netdev_priv(ndev);
 	struct macb_queue *queue;
-	u32 enst_disable_mask;
+	u32 queue_mask;
 	unsigned int q;
 
 	netdev_reset_tc(ndev);
-	enst_disable_mask = bp->queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET;
+	queue_mask = BIT_U32(bp->num_queues) - 1;
 
 	scoped_guard(spinlock_irqsave, &bp->lock) {
 		/* Single disable command for all queues */
-		gem_writel(bp, ENST_CONTROL, enst_disable_mask);
+		gem_writel(bp, ENST_CONTROL,
+			   queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
 
 		/* Clear all queue ENST registers in batch */
 		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
@@ -4341,26 +4346,25 @@ static void macb_configure_caps(struct macb *bp,
 	dev_dbg(&bp->pdev->dev, "Cadence caps 0x%08x\n", bp->caps);
 }
 
-static void macb_probe_queues(void __iomem *mem,
-			      bool native_io,
-			      unsigned int *queue_mask,
-			      unsigned int *num_queues)
+static int macb_probe_queues(struct device *dev, void __iomem *mem, bool native_io)
 {
-	*queue_mask = 0x1;
-	*num_queues = 1;
+	/* BIT(0) is never set but queue 0 always exists. */
+	unsigned int queue_mask = 0x1;
 
-	/* is it macb or gem ?
-	 *
-	 * We need to read directly from the hardware here because
-	 * we are early in the probe process and don't have the
-	 * MACB_CAPS_MACB_IS_GEM flag positioned
-	 */
-	if (!hw_is_gem(mem, native_io))
-		return;
+	/* Use hw_is_gem() as MACB_CAPS_MACB_IS_GEM is not yet positioned. */
+	if (hw_is_gem(mem, native_io)) {
+		if (native_io)
+			queue_mask |= __raw_readl(mem + GEM_DCFG6) & 0xFF;
+		else
+			queue_mask |= readl_relaxed(mem + GEM_DCFG6) & 0xFF;
 
-	/* bit 0 is never set but queue 0 always exists */
-	*queue_mask |= readl_relaxed(mem + GEM_DCFG6) & 0xff;
-	*num_queues = hweight32(*queue_mask);
+		if (fls(queue_mask) != ffz(queue_mask)) {
+			dev_err(dev, "queue mask %#x has a hole\n", queue_mask);
+			return -EINVAL;
+		}
+	}
+
+	return hweight32(queue_mask);
 }
 
 static void macb_clks_disable(struct clk *pclk, struct clk *hclk, struct clk *tx_clk,
@@ -4478,10 +4482,7 @@ static int macb_init(struct platform_device *pdev)
 	 * register mapping but we don't want to test the queue index then
 	 * compute the corresponding register offset at run time.
 	 */
-	for (hw_q = 0, q = 0; hw_q < MACB_MAX_QUEUES; ++hw_q) {
-		if (!(bp->queue_mask & (1 << hw_q)))
-			continue;
-
+	for (hw_q = 0, q = 0; hw_q < bp->num_queues; ++hw_q) {
 		queue = &bp->queues[q];
 		queue->bp = bp;
 		spin_lock_init(&queue->tx_ptr_lock);
@@ -5385,14 +5386,14 @@ static int macb_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct clk *pclk, *hclk = NULL, *tx_clk = NULL, *rx_clk = NULL;
 	struct clk *tsu_clk = NULL;
-	unsigned int queue_mask, num_queues;
-	bool native_io;
 	phy_interface_t interface;
 	struct net_device *dev;
 	struct resource *regs;
 	u32 wtrmrk_rst_val;
 	void __iomem *mem;
 	struct macb *bp;
+	int num_queues;
+	bool native_io;
 	int err, val;
 
 	mem = devm_platform_get_and_ioremap_resource(pdev, 0, &regs);
@@ -5418,7 +5419,12 @@ static int macb_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	native_io = hw_is_native_io(mem);
 
-	macb_probe_queues(mem, native_io, &queue_mask, &num_queues);
+	num_queues = macb_probe_queues(&pdev->dev, mem, native_io);
+	if (num_queues < 0) {
+		err = num_queues;
+		goto err_disable_clocks;
+	}
+
 	dev = alloc_etherdev_mq(sizeof(*bp), num_queues);
 	if (!dev) {
 		err = -ENOMEM;
@@ -5442,7 +5448,6 @@ static int macb_probe(struct platform_device *pdev)
 		bp->macb_reg_writel = hw_writel;
 	}
 	bp->num_queues = num_queues;
-	bp->queue_mask = queue_mask;
 	bp->dma_burst_length = macb_config->dma_burst_length;
 	bp->pclk = pclk;
 	bp->hclk = hclk;

-- 
2.51.0

Re: [PATCH net-next 10/15] net: macb: remove bp->queue_mask

[Andrew Lunn]

On Tue, Oct 14, 2025 at 05:25:11PM +0200, Théo Lebrun wrote:
> The low 16 bits of GEM_DCFG6 tell us which queues are enabled in HW. In
> theory, there could be holes in the bitfield. In practice, the macb
> driver would fail if there were holes as most loops iterate upon
> bp->num_queues. Only macb_init() iterated correctly.
> 
>  - Drop bp->queue_mask field.
>  - Error out at probe if a hole is in the queue mask.
>  - Rely upon bp->num_queues for iteration.
>  - As we drop the queue_mask probe local variable, fix RCT.
>  - Compute queue_mask on the fly for TAPRIO using bp->num_queues.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 11/15] net: macb: replace min() with umin() calls

[Théo Lebrun]

Whenever min(a, b) is used with a and b unsigned variables or literals,
`make W=2` complains. Change four min() calls into umin().

stderr extract (GCC 11.2.0, MIPS Codescape):

./include/linux/minmax.h:68:57: warning: comparison is always true due
                          to limited range of data type [-Wtype-limits]
   68 | #define __is_nonneg(ux) statically_true((long long)(ux) >= 0)
      |                                                         ^~
drivers/net/ethernet/cadence/macb_main.c:2299:26: note: in expansion of
                                                            macro ‘min’
 2299 |              hdrlen = min(skb_headlen(skb), bp->max_tx_length);
      |                       ^~~

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 98e28d51a6e12c24ef27c939363eb43c0aec1951..6c6bc6aa23c718772b95b398e807f193a38e141a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2035,7 +2035,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		count++;
 		tx_head++;
 
-		size = min(len, bp->max_tx_length);
+		size = umin(len, bp->max_tx_length);
 	}
 
 	/* Then, map paged data from fragments */
@@ -2045,7 +2045,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		len = skb_frag_size(frag);
 		offset = 0;
 		while (len) {
-			size = min(len, bp->max_tx_length);
+			size = umin(len, bp->max_tx_length);
 			entry = macb_tx_ring_wrap(bp, tx_head);
 			tx_skb = &queue->tx_skb[entry];
 
@@ -2301,7 +2301,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			return NETDEV_TX_BUSY;
 		}
 	} else
-		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
+		hdrlen = umin(skb_headlen(skb), bp->max_tx_length);
 
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
 	netdev_vdbg(bp->dev,
@@ -4573,8 +4573,8 @@ static int macb_init(struct platform_device *pdev)
 	 * each 4-tuple define requires 1 T2 screener reg + 3 compare regs
 	 */
 	reg = gem_readl(bp, DCFG8);
-	bp->max_tuples = min((GEM_BFEXT(SCR2CMP, reg) / 3),
-			GEM_BFEXT(T2SCR, reg));
+	bp->max_tuples = umin((GEM_BFEXT(SCR2CMP, reg) / 3),
+			      GEM_BFEXT(T2SCR, reg));
 	INIT_LIST_HEAD(&bp->rx_fs_list.list);
 	if (bp->max_tuples > 0) {
 		/* also needs one ethtype match to check IPv4 */

-- 
2.51.0

Re: [PATCH net-next 11/15] net: macb: replace min() with umin() calls

[David Laight]

On Tue, 14 Oct 2025 17:25:12 +0200
Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Whenever min(a, b) is used with a and b unsigned variables or literals,
> `make W=2` complains. Change four min() calls into umin().

It will, and you'll get the same 'error' all over the place.
Basically -Wtype-limits is broken.

Don't remove valid checks because it bleats.

	David


> 
> stderr extract (GCC 11.2.0, MIPS Codescape):
> 
> ./include/linux/minmax.h:68:57: warning: comparison is always true due
>                           to limited range of data type [-Wtype-limits]
>    68 | #define __is_nonneg(ux) statically_true((long long)(ux) >= 0)
>       |                                                         ^~
> drivers/net/ethernet/cadence/macb_main.c:2299:26: note: in expansion of
>                                                             macro ‘min’
>  2299 |              hdrlen = min(skb_headlen(skb), bp->max_tx_length);
>       |                       ^~~
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 98e28d51a6e12c24ef27c939363eb43c0aec1951..6c6bc6aa23c718772b95b398e807f193a38e141a 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2035,7 +2035,7 @@ static unsigned int macb_tx_map(struct macb *bp,
>  		count++;
>  		tx_head++;
>  
> -		size = min(len, bp->max_tx_length);
> +		size = umin(len, bp->max_tx_length);
>  	}
>  
>  	/* Then, map paged data from fragments */
> @@ -2045,7 +2045,7 @@ static unsigned int macb_tx_map(struct macb *bp,
>  		len = skb_frag_size(frag);
>  		offset = 0;
>  		while (len) {
> -			size = min(len, bp->max_tx_length);
> +			size = umin(len, bp->max_tx_length);
>  			entry = macb_tx_ring_wrap(bp, tx_head);
>  			tx_skb = &queue->tx_skb[entry];
>  
> @@ -2301,7 +2301,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  			return NETDEV_TX_BUSY;
>  		}
>  	} else
> -		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
> +		hdrlen = umin(skb_headlen(skb), bp->max_tx_length);
>  
>  #if defined(DEBUG) && defined(VERBOSE_DEBUG)
>  	netdev_vdbg(bp->dev,
> @@ -4573,8 +4573,8 @@ static int macb_init(struct platform_device *pdev)
>  	 * each 4-tuple define requires 1 T2 screener reg + 3 compare regs
>  	 */
>  	reg = gem_readl(bp, DCFG8);
> -	bp->max_tuples = min((GEM_BFEXT(SCR2CMP, reg) / 3),
> -			GEM_BFEXT(T2SCR, reg));
> +	bp->max_tuples = umin((GEM_BFEXT(SCR2CMP, reg) / 3),
> +			      GEM_BFEXT(T2SCR, reg));
>  	INIT_LIST_HEAD(&bp->rx_fs_list.list);
>  	if (bp->max_tuples > 0) {
>  		/* also needs one ethtype match to check IPv4 */
> 

Re: [PATCH net-next 11/15] net: macb: replace min() with umin() calls

[Théo Lebrun]

On Sun Oct 19, 2025 at 4:10 PM CEST, David Laight wrote:
> On Tue, 14 Oct 2025 17:25:12 +0200
> Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
>> Whenever min(a, b) is used with a and b unsigned variables or literals,
>> `make W=2` complains. Change four min() calls into umin().
>
> It will, and you'll get the same 'error' all over the place.
> Basically -Wtype-limits is broken.
>
> Don't remove valid checks because it bleats.

In theory I agree. In practice, this patch leads to a more readable
`make W=2 drivers/net/ethernet/cadence/` stderr output, by removing a
few false positives, and that's my only desire (not quite).

I am not sure what you mean by "Don't remove valid checks"; could you
clarify? My understanding is that the warning checks are about the
signedness of unsigned integers. Are you implying that we lose
something (safety?) when switching from min(a, b) to umin(a, b) with
a/b both unsigned ints?

Thanks David,

[0]: https://lore.kernel.org/lkml/176066582948.1978978.752807229943547484.git-patchwork-notify@kernel.org/
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f26c6438a285

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 11/15] net: macb: replace min() with umin() calls

[David Laight]

On Mon, 20 Oct 2025 13:44:43 +0200
Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> On Sun Oct 19, 2025 at 4:10 PM CEST, David Laight wrote:
> > On Tue, 14 Oct 2025 17:25:12 +0200
> > Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >  
> >> Whenever min(a, b) is used with a and b unsigned variables or literals,
> >> `make W=2` complains. Change four min() calls into umin().  
> >
> > It will, and you'll get the same 'error' all over the place.
> > Basically -Wtype-limits is broken.
> >
> > Don't remove valid checks because it bleats.  
> 
> In theory I agree. In practice, this patch leads to a more readable
> `make W=2 drivers/net/ethernet/cadence/` stderr output, by removing a
> few false positives, and that's my only desire (not quite).
> 
> I am not sure what you mean by "Don't remove valid checks"; could you
> clarify? My understanding is that the warning checks are about the
> signedness of unsigned integers. Are you implying that we lose
> something (safety?) when switching from min(a, b) to umin(a, b) with
> a/b both unsigned ints?

The issue is that -Wtype-limits warns for every case where min() is
used with two unsigned values.
It is pretty much impossible to code around it as well.
It also warns for other #defines that are trying to check for invalid
constant values.
The checks are there to pick up invalid calls, using umin() (and worse
min_t()) to avoid the warnings is making the checks pointless.
So you may know the code is ok, but the compile-time checks are there
to ensure it is ok.

	David

> [0]: https://lore.kernel.org/lkml/176066582948.1978978.752807229943547484.git-patchwork-notify@kernel.org/
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f26c6438a285
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

[PATCH net-next 12/15] net: macb: drop `entry` local variable in macb_tx_map()

[Théo Lebrun]

The pattern:
   entry = macb_tx_ring_wrap(bp, i);
   tx_skb = &queue->tx_skb[entry];
is the exact definition of:
   macb_tx_skb(queue, i);

The pattern:
   entry = macb_tx_ring_wrap(bp, i);
   desc = macb_tx_desc(queue, entry);
is redundant because macb_tx_desc() calls macb_tx_ring_wrap().

One explicit call to macb_tx_ring_wrap() is still required for checking
if it is the last buffer (TX_WRAP case).

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6c6bc6aa23c718772b95b398e807f193a38e141a..08e541d8f8e68e38442a538ce352508c5db63f52 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1989,7 +1989,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 				unsigned int hdrlen)
 {
 	dma_addr_t mapping;
-	unsigned int len, entry, i, tx_head = queue->tx_head;
+	unsigned int len, i, tx_head = queue->tx_head;
 	struct macb_tx_skb *tx_skb = NULL;
 	struct macb_dma_desc *desc;
 	unsigned int offset, size, count = 0;
@@ -2015,8 +2015,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 	offset = 0;
 	while (len) {
-		entry = macb_tx_ring_wrap(bp, tx_head);
-		tx_skb = &queue->tx_skb[entry];
+		tx_skb = macb_tx_skb(queue, tx_head);
 
 		mapping = dma_map_single(&bp->pdev->dev,
 					 skb->data + offset,
@@ -2046,8 +2045,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		offset = 0;
 		while (len) {
 			size = umin(len, bp->max_tx_length);
-			entry = macb_tx_ring_wrap(bp, tx_head);
-			tx_skb = &queue->tx_skb[entry];
+			tx_skb = macb_tx_skb(queue, tx_head);
 
 			mapping = skb_frag_dma_map(&bp->pdev->dev, frag,
 						   offset, size, DMA_TO_DEVICE);
@@ -2084,9 +2082,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 	 * to set the end of TX queue
 	 */
 	i = tx_head;
-	entry = macb_tx_ring_wrap(bp, i);
 	ctrl = MACB_BIT(TX_USED);
-	desc = macb_tx_desc(queue, entry);
+	desc = macb_tx_desc(queue, i);
 	desc->ctrl = ctrl;
 
 	if (lso_ctrl) {
@@ -2106,16 +2103,15 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 	do {
 		i--;
-		entry = macb_tx_ring_wrap(bp, i);
-		tx_skb = &queue->tx_skb[entry];
-		desc = macb_tx_desc(queue, entry);
+		tx_skb = macb_tx_skb(queue, i);
+		desc = macb_tx_desc(queue, i);
 
 		ctrl = (u32)tx_skb->size;
 		if (eof) {
 			ctrl |= MACB_BIT(TX_LAST);
 			eof = 0;
 		}
-		if (unlikely(entry == (bp->tx_ring_size - 1)))
+		if (unlikely(macb_tx_ring_wrap(bp, i) == bp->tx_ring_size - 1))
 			ctrl |= MACB_BIT(TX_WRAP);
 
 		/* First descriptor is header descriptor */

-- 
2.51.0

[PATCH net-next 13/15] net: macb: drop `count` local variable in macb_tx_map()

[Théo Lebrun]

Local variable `count` is useless: it counts number of DMA descriptors
used and returns it. But the return value is only checked for error.
Drop counting the number of DMA descriptors and return a usual
negative-if-error integer.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 08e541d8f8e68e38442a538ce352508c5db63f52..dd3b13fa304715b4629c20120a908262af106a2d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1992,7 +1992,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 	unsigned int len, i, tx_head = queue->tx_head;
 	struct macb_tx_skb *tx_skb = NULL;
 	struct macb_dma_desc *desc;
-	unsigned int offset, size, count = 0;
+	unsigned int offset, size;
 	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int eof = 1, mss_mfs = 0;
 	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
@@ -2031,7 +2031,6 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 		len -= size;
 		offset += size;
-		count++;
 		tx_head++;
 
 		size = umin(len, bp->max_tx_length);
@@ -2060,7 +2059,6 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 			len -= size;
 			offset += size;
-			count++;
 			tx_head++;
 		}
 	}
@@ -2139,7 +2137,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 
 	queue->tx_head = tx_head;
 
-	return count;
+	return 0;
 
 dma_error:
 	netdev_err(bp->dev, "TX DMA map failed\n");
@@ -2150,7 +2148,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		macb_tx_unmap(bp, tx_skb, 0);
 	}
 
-	return 0;
+	return -ENOMEM;
 }
 
 static netdev_features_t macb_features_check(struct sk_buff *skb,
@@ -2336,7 +2334,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	/* Map socket buffer for DMA transfer */
-	if (!macb_tx_map(bp, queue, skb, hdrlen)) {
+	if (macb_tx_map(bp, queue, skb, hdrlen)) {
 		dev_kfree_skb_any(skb);
 		goto unlock;
 	}

-- 
2.51.0

[PATCH net-next 14/15] net: macb: apply reverse christmas tree in macb_tx_map()

[Théo Lebrun]

The arguments grew over time; follow conventions and apply reverse
christmas tree (RCT).

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dd3b13fa304715b4629c20120a908262af106a2d..e15fcdd43d778c685dbb927386904362b8dba8b9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1988,14 +1988,14 @@ static unsigned int macb_tx_map(struct macb *bp,
 				struct sk_buff *skb,
 				unsigned int hdrlen)
 {
-	dma_addr_t mapping;
+	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int len, i, tx_head = queue->tx_head;
+	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
+	unsigned int eof = 1, mss_mfs = 0;
 	struct macb_tx_skb *tx_skb = NULL;
 	struct macb_dma_desc *desc;
 	unsigned int offset, size;
-	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
-	unsigned int eof = 1, mss_mfs = 0;
-	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
+	dma_addr_t mapping;
 
 	/* LSO */
 	if (skb_shinfo(skb)->gso_size != 0) {

-- 
2.51.0

[PATCH net-next 15/15] net: macb: sort #includes

[Théo Lebrun]

Sort #include preprocessor directives.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Sean Anderson <sean.anderson@linux.dev>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 37 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e15fcdd43d778c685dbb927386904362b8dba8b9..214f543af3b8fd4230d2be8095b26c3915dd9cfb 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -6,36 +6,37 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/clk.h>
+#include <linux/circ_buf.h>
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/crc32.h>
+#include <linux/dma-mapping.h>
+#include <linux/etherdevice.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/inetdevice.h>
+#include <linux/inetdevice.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ip.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/kernel.h>
-#include <linux/types.h>
-#include <linux/circ_buf.h>
-#include <linux/slab.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
 #include <linux/netdevice.h>
-#include <linux/etherdevice.h>
-#include <linux/dma-mapping.h>
-#include <linux/platform_device.h>
-#include <linux/phylink.h>
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
-#include <linux/ip.h>
-#include <linux/udp.h>
-#include <linux/tcp.h>
-#include <linux/iopoll.h>
 #include <linux/phy/phy.h>
+#include <linux/phylink.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/ptp_classify.h>
 #include <linux/reset.h>
-#include <linux/firmware/xlnx-zynqmp.h>
-#include <linux/inetdevice.h>
+#include <linux/slab.h>
+#include <linux/tcp.h>
+#include <linux/types.h>
+#include <linux/udp.h>
 #include <net/pkt_sched.h>
 #include "macb.h"
 

-- 
2.51.0

Re: [PATCH net-next 00/15] net: macb: various cleanups

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 14 Oct 2025 17:25:01 +0200 you wrote:
> Fix many oddities inside the MACB driver. They accumulated in my
> work-in-progress branch while working on MACB/GEM EyeQ5 support.
> 
> Part of this series has been seen on the lkml in March then June.
> See below for a semblance of a changelog.
> 
> The initial goal was to post them alongside EyeQ5 support, but that
> makes for too big of a series. It'll come afterwards, with new
> features (interrupt coalescing, ethtool .set_channels() and XDP mostly).
> 
> [...]

Here is the summary with links:
  - [net-next,01/15] dt-bindings: net: cdns,macb: sort compatibles
    https://git.kernel.org/netdev/net-next/c/f1150b779571
  - [net-next,02/15] net: macb: use BIT() macro for capability definitions
    https://git.kernel.org/netdev/net-next/c/a23b0b79e974
  - [net-next,03/15] net: macb: remove gap in MACB_CAPS_* flags
    https://git.kernel.org/netdev/net-next/c/bd0b35ec835a
  - [net-next,04/15] net: macb: Remove local variables clk_init and init in macb_probe()
    https://git.kernel.org/netdev/net-next/c/80cf78c59a1a
  - [net-next,05/15] net: macb: drop macb_config NULL checking
    https://git.kernel.org/netdev/net-next/c/d7a4a20abe25
  - [net-next,06/15] net: macb: simplify macb_dma_desc_get_size()
    https://git.kernel.org/netdev/net-next/c/94a164598d83
  - [net-next,07/15] net: macb: simplify macb_adj_dma_desc_idx()
    https://git.kernel.org/netdev/net-next/c/62e6c17463a7
  - [net-next,08/15] net: macb: move bp->hw_dma_cap flags to bp->caps
    https://git.kernel.org/netdev/net-next/c/731e991afb75
  - [net-next,09/15] net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
    https://git.kernel.org/netdev/net-next/c/02d11c610555
  - [net-next,10/15] net: macb: remove bp->queue_mask
    https://git.kernel.org/netdev/net-next/c/39a913db6a47
  - [net-next,11/15] net: macb: replace min() with umin() calls
    https://git.kernel.org/netdev/net-next/c/f26c6438a285
  - [net-next,12/15] net: macb: drop `entry` local variable in macb_tx_map()
    https://git.kernel.org/netdev/net-next/c/027202adf079
  - [net-next,13/15] net: macb: drop `count` local variable in macb_tx_map()
    https://git.kernel.org/netdev/net-next/c/b5fe4f3e5912
  - [net-next,14/15] net: macb: apply reverse christmas tree in macb_tx_map()
    https://git.kernel.org/netdev/net-next/c/1ce9662e31fd
  - [net-next,15/15] net: macb: sort #includes
    https://git.kernel.org/netdev/net-next/c/8ebeef3d01c8

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