[PATCH net-next v2 00/18] Support the Cadence MACB/GEM instances on Mobileye EyeQ5 SoCs

[PATCH net-next v2 00/18] Support the Cadence MACB/GEM instances on Mobileye EyeQ5 SoCs

[Théo Lebrun]

Mobileye EyeQ5 SoCs provides two GEM IP blocks. The end result of this
series is working 1G networking on the EyeQ5 eval board. It isn't just
a new macb_config & compatible, we also provide some cleanup & fixes
along the way.

dt-bindings:
  [PATCH net-next v2 01/18] dt-bindings: net: cdns,macb: sort compatibles
  [PATCH net-next v2 02/18] dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface
  [PATCH net-next v2 03/18] dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk
  [PATCH net-next v2 04/18] dt-bindings: net: cdns,macb: allow dma-coherent

Cleanup:
  [PATCH net-next v2 05/18] net: macb: use BIT() macro for capability definitions
  [PATCH net-next v2 06/18] net: macb: Remove local variables clk_init and init in macb_probe()
  [PATCH net-next v2 07/18] net: macb: drop macb_config NULL checking
  [PATCH net-next v2 08/18] net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
  [PATCH net-next v2 09/18] net: macb: sort #includes

Fixes:
  [PATCH net-next v2 10/18] net: macb: remove illusion about TBQPH/RBQPH being per-queue
  [PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors
  [PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
  [PATCH net-next v2 13/18] net: macb: avoid double endianness swap in macb_set_hwaddr()

EyeQ5 support:
  [PATCH net-next v2 14/18] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
  [PATCH net-next v2 15/18] net: macb: Add "mobileye,eyeq5-gem" compatible

DTS:
  [PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support
  [PATCH net-next v2 17/18] MIPS: mobileye: eyeq5: add two Cadence GEM Ethernet controllers
  [PATCH net-next v2 18/18] MIPS: mobileye: eyeq5-epm: add two Cadence GEM Ethernet PHYs

Note: I've categorised [PATCH 12/18] as a fix, but it is also mandatory
for working GEM on EyeQ5 SoCs. Hardware is RSC capable so we mustn't do
the skb_reserve(). It probably is the first platform where
NET_IP_ALIGN=2 and the devices are RSC capable.

Note: I did not extract the platform specific code (2 syscon registers
being written to) from macb_config->init into separate resources. I
cannot dissociate that register write sequence into something more
fluid (modeling clocks/PHYs).

Have a nice day,
Théo

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v2:
- Rebase upon latest net-next (1ea2e106130a).
- dt-bindings:
  - Add description to mobileye,olb phandle, following a
    `make dt_binding_check` warning reported by Rob's bot.
  - Add "sort compatibles" patch.
  - Take "Acked-by: Rob Herring" on dma-coherent patch.
  - Improve "allow tsu_clk without tx_clk" commit message following
    Rob's comment.
- Take "Reviewed-by: Andrew Lunn" trailer on four patches:
    net: macb: use BIT() macro for capability definitions
    net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
    net: macb: sort #includes
    MIPS: mobileye: eyeq5-epm: add two Cadence GEM Ethernet PHYs
- net: macb:
  - Split:
      simplify macb_probe() code touching match data
    into two commits, following comment by Andrew:
      Remove local variables clk_init and init in macb_probe()
      drop macb_config NULL checking
  - Replace match-data IP alignment value by a more robust solution: HW support
    IP alignment if it does not support RSC.
  - use regmap_update_bits() for OLB register writes.
  - fix insertion order of compatible.
  - fix LSO detection comment.
  - move local variables `struct macb_dma_desc_64` inside conditionals,
    in macb_{get,get}_addr().
  - add "remove illusion about TBQPH/RBQPH being per-queue" commit.
  - add "single dma_alloc_coherent() for DMA descriptors" commit.
  - add "avoid double endianness swap in macb_set_hwaddr()" commit.
- DTS: lowercase reg properties of PHYs.
- Link to v1: https://lore.kernel.org/r/20250321-macb-v1-0-537b7e37971d@bootlin.com

---
Théo Lebrun (18):
      dt-bindings: net: cdns,macb: sort compatibles
      dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface
      dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk
      dt-bindings: net: cdns,macb: allow dma-coherent
      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
      net: macb: remove illusion about TBQPH/RBQPH being per-queue
      net: macb: single dma_alloc_coherent() for DMA descriptors
      net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
      net: macb: avoid double endianness swap in macb_set_hwaddr()
      net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
      net: macb: Add "mobileye,eyeq5-gem" compatible
      MIPS: mobileye: add EyeQ5 DMA IOCU support
      MIPS: mobileye: eyeq5: add two Cadence GEM Ethernet controllers
      MIPS: mobileye: eyeq5-epm: add two Cadence GEM Ethernet PHYs

 .../devicetree/bindings/net/cdns,macb.yaml         |  38 ++-
 MAINTAINERS                                        |   2 +-
 arch/mips/boot/dts/mobileye/eyeq5-epm5.dts         |  26 ++
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |  34 ++
 arch/mips/mobileye/Kconfig                         |   1 +
 arch/mips/mobileye/Makefile                        |   2 +
 arch/mips/mobileye/eyeq5-iocu-dma.c                | 160 +++++++++
 drivers/net/ethernet/cadence/macb.h                |  56 ++-
 drivers/net/ethernet/cadence/macb_main.c           | 380 ++++++++++++---------
 9 files changed, 498 insertions(+), 201 deletions(-)
---
base-commit: bb26d1066f9db96d9b10d7724798aa14203f3ef8
change-id: 20250311-macb-65a7fa86af1d

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

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

[Théo Lebrun]

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

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 4423d038b2436ccf0d4c7f161510edbc5da3f131..df883354c7e635099885da42e4604e1c31b05c72 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -47,17 +47,17 @@ 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
           - sifive,fu540-c000-gem     # SiFive FU540-C000 SoC
-          - cdns,emac                 # Generic
-          - cdns,gem                  # Generic
-          - cdns,macb                 # Generic
 
       - items:
           - enum:

-- 
2.50.0

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

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 11:08:47AM +0200, Théo Lebrun wrote:
> Compatibles inside this enum are sorted-ish. Make it sorted.
> 
> 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(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH net-next v2 02/18] dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface

[Théo Lebrun]

Add cdns,eyeq5-gem as compatible for the integrated GEM block inside
Mobileye EyeQ5 SoCs. Add a phandle (and two offset arguments) for
accessing syscon registers.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../devicetree/bindings/net/cdns,macb.yaml         | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index df883354c7e635099885da42e4604e1c31b05c72..6cf43cc50377f23d60ef40bf1c8efa22ce1ae0bb 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -57,6 +57,7 @@ properties:
           - cdns,np4-macb             # NP4 SoC devices
           - microchip,sama7g5-emac    # Microchip SAMA7G5 ethernet interface
           - microchip,sama7g5-gem     # Microchip SAMA7G5 gigabit ethernet interface
+          - mobileye,eyeq5-gem        # Mobileye EyeQ5 SoCs
           - sifive,fu540-c000-gem     # SiFive FU540-C000 SoC
 
       - items:
@@ -137,6 +138,17 @@ properties:
       Node containing PHY children. If this node is not present, then PHYs will
       be direct children.
 
+  mobileye,olb:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Handle to the OLB system controller that owns registers configuring the
+      MACB integration.
+    items:
+      - items:
+          - description: phandle to OLB node
+          - description: MAC General-Purpose register offset
+          - description: MAC SGMII register offset
+
 patternProperties:
   "^ethernet-phy@[0-9a-f]$":
     type: object
@@ -175,6 +187,18 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq5-gem
+    then:
+      required:
+        - mobileye,olb
+    else:
+      properties:
+        mobileye,olb: false
+
 unevaluatedProperties: false
 
 examples:

-- 
2.50.0

Re: [PATCH net-next v2 02/18] dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 11:08:48AM +0200, Théo Lebrun wrote:
> Add cdns,eyeq5-gem as compatible for the integrated GEM block inside
> Mobileye EyeQ5 SoCs. Add a phandle (and two offset arguments) for
> accessing syscon registers.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../devicetree/bindings/net/cdns,macb.yaml         | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> index df883354c7e635099885da42e4604e1c31b05c72..6cf43cc50377f23d60ef40bf1c8efa22ce1ae0bb 100644
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -57,6 +57,7 @@ properties:
>            - cdns,np4-macb             # NP4 SoC devices
>            - microchip,sama7g5-emac    # Microchip SAMA7G5 ethernet interface
>            - microchip,sama7g5-gem     # Microchip SAMA7G5 gigabit ethernet interface
> +          - mobileye,eyeq5-gem        # Mobileye EyeQ5 SoCs
>            - sifive,fu540-c000-gem     # SiFive FU540-C000 SoC
>  
>        - items:
> @@ -137,6 +138,17 @@ properties:
>        Node containing PHY children. If this node is not present, then PHYs will
>        be direct children.
>  
> +  mobileye,olb:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Handle to the OLB system controller that owns registers configuring the
> +      MACB integration.

... to do what?

Maybe you just miss some sort of phy or power domain provider. Syscon is
not a replacement for that.

Best regards,
Krzysztof

[PATCH net-next v2 03/18] dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk

[Théo Lebrun]

Allow providing tsu_clk without a tx_clk as both are optional.

This is about relaxing unneeded constraints. It so happened that in the
past HW that needed a tsu_clk always needed a tx_clk.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/net/cdns,macb.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index 6cf43cc50377f23d60ef40bf1c8efa22ce1ae0bb..0112dc6aeed68a186f0ee0038f495b0234570427 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -86,7 +86,7 @@ properties:
     items:
       - enum: [ ether_clk, hclk, pclk ]
       - enum: [ hclk, pclk ]
-      - const: tx_clk
+      - enum: [ tx_clk, tsu_clk ]
       - enum: [ rx_clk, tsu_clk ]
       - const: tsu_clk
 

-- 
2.50.0

Re: [PATCH net-next v2 03/18] dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 11:08:49AM +0200, Théo Lebrun wrote:
> Allow providing tsu_clk without a tx_clk as both are optional.
> 
> This is about relaxing unneeded constraints. It so happened that in the
> past HW that needed a tsu_clk always needed a tx_clk.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/net/cdns,macb.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH net-next v2 04/18] dt-bindings: net: cdns,macb: allow dma-coherent

[Théo Lebrun]

On EyeQ5, the GEM DMA controller is coherent with the CPU;
allow specifying the information.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/net/cdns,macb.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index 0112dc6aeed68a186f0ee0038f495b0234570427..58f4933ec2ca58432d7eaa017317249b074c9fc7 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -113,6 +113,8 @@ properties:
   iommus:
     maxItems: 1
 
+  dma-coherent: true
+
   power-domains:
     maxItems: 1
 

-- 
2.50.0

[PATCH net-next v2 05/18] 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>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h | 40 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index c9a5c8beb2fa8166195d1d83f187d2d0c62668a8..3b43cb9468e3618754ff2bc6c5f360447bdeeed0 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -727,26 +727,26 @@
 #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_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_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.50.0

Re: [PATCH net-next v2 05/18] net: macb: use BIT() macro for capability definitions

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> Replace all capabilities values by calls to the BIT() macro.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb.h | 40 ++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index c9a5c8beb2fa8166195d1d83f187d2d0c62668a8..3b43cb9468e3618754ff2bc6c5f360447bdeeed0 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -727,26 +727,26 @@
>  #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_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_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
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

But since these capabilities don't correspond to hardware registers, can
you add a follow-up patch to remove the gap between QUEUE_DISABLE and
PCS?

[PATCH net-next v2 06/18] 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.

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 53aaf6b08e39a45bde5af61a05fb2b9fd653b2e7..6c8a34e40b77ea37ed25c2b0414440cd3f6962c3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5166,10 +5166,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;
@@ -5191,14 +5187,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;
 
@@ -5336,7 +5329,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.50.0

Re: [PATCH net-next v2 06/18] net: macb: Remove local variables clk_init and init in macb_probe()

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> Remove local variables clk_init and init. Those function pointers are
> always equivalent to macb_config->clk_init and macb_config->init.
> 
> 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 53aaf6b08e39a45bde5af61a05fb2b9fd653b2e7..6c8a34e40b77ea37ed25c2b0414440cd3f6962c3 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -5166,10 +5166,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;
> @@ -5191,14 +5187,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;
>  
> @@ -5336,7 +5329,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;
>  
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

[PATCH net-next v2 07/18] 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.

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 6c8a34e40b77ea37ed25c2b0414440cd3f6962c3..6926ea11d71adae7a25d5ee329c148f5882d4184 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5227,15 +5227,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.50.0

Re: [PATCH net-next v2 07/18] net: macb: drop macb_config NULL checking

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> 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.
> 
> 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 6c8a34e40b77ea37ed25c2b0414440cd3f6962c3..6926ea11d71adae7a25d5ee329c148f5882d4184 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -5227,15 +5227,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;
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

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

[Théo Lebrun]

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

This implies three changes:
 - Always compile related structure definitions inside <macb.h>.
 - Make the field hw_dma_cap in struct macb always present.
 - MACB_EXT_DESC can be dropped as it is useless now.

The common case is:

	#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

And replaced by:

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

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

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 3b43cb9468e3618754ff2bc6c5f360447bdeeed0..707b3286a6b8408a3bc4bbbcb1335ae8c3cd95ad 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
@@ -823,7 +819,6 @@ struct macb_dma_desc {
 	u32	ctrl;
 };
 
-#ifdef MACB_EXT_DESC
 #define HW_DMA_CAP_32B		0
 #define HW_DMA_CAP_64B		(1 << 0)
 #define HW_DMA_CAP_PTP		(1 << 1)
@@ -838,7 +833,6 @@ struct macb_dma_desc_ptp {
 	u32	ts_1;
 	u32	ts_2;
 };
-#endif
 
 /* DMA descriptor bitfields */
 #define MACB_RX_USED_OFFSET			0
@@ -1316,9 +1310,7 @@ 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 6926ea11d71adae7a25d5ee329c148f5882d4184..be667cb5acce85a9d11aaea1f5081a3380b60ef2 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -95,6 +95,18 @@ struct sifive_fu540_macb_mgmt {
 
 #define MACB_MDIO_TIMEOUT	1000000 /* in usecs */
 
+static bool macb_dma_is_64b(struct macb *bp)
+{
+	return IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
+	       bp->hw_dma_cap & HW_DMA_CAP_64B;
+}
+
+static bool macb_dma_is_ptp(struct macb *bp)
+{
+	return IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) &&
+	       bp->hw_dma_cap & HW_DMA_CAP_PTP;
+}
+
 /* DMA buffer descriptor might be different size
  * depends on hardware configuration:
  *
@@ -124,56 +136,31 @@ 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);
+
+	if (macb_dma_is_64b(bp))
+		desc_size += sizeof(struct macb_dma_desc_64);
+	if (macb_dma_is_ptp(bp))
+		desc_size += sizeof(struct macb_dma_desc_ptp);
 
-	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;
-#endif
-	return sizeof(struct macb_dma_desc);
 }
 
 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
-	return desc_idx;
+	if (macb_dma_is_64b(bp) && macb_dma_is_ptp(bp))
+		return desc_idx * 3;
+	else if (macb_dma_is_64b(bp) || macb_dma_is_ptp(bp))
+		return desc_idx << 1;
+	else
+		return desc_idx;
 }
 
-#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)
@@ -497,17 +484,13 @@ static void macb_init_buffers(struct macb *bp)
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+		if (macb_dma_is_64b(bp))
 			queue_writel(queue, RBQPH,
 				     upper_32_bits(queue->rx_ring_dma));
-#endif
 		queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+		if (macb_dma_is_64b(bp))
 			queue_writel(queue, TBQPH,
 				     upper_32_bits(queue->tx_ring_dma));
-#endif
 	}
 }
 
@@ -1028,10 +1011,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_dma_is_64b(bp)) {
+		struct macb_dma_desc_64 *desc_64;
 
-	if (bp->hw_dma_cap & HW_DMA_CAP_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
@@ -1040,26 +1022,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->hw_dma_cap & HW_DMA_CAP_64B) {
+	if (macb_dma_is_64b(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->hw_dma_cap & HW_DMA_CAP_PTP)
+	if (macb_dma_is_ptp(bp))
 		addr &= ~GEM_BIT(DMA_RXVALID);
-#endif
 	return addr;
 }
 
@@ -1166,10 +1145,8 @@ static void macb_tx_error_task(struct work_struct *work)
 
 	/* Reinitialize the TX desc queue */
 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+	if (macb_dma_is_64b(bp))
 		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
-#endif
 	/* Make TX ring reflect state of hardware */
 	queue->tx_head = 0;
 	queue->tx_tail = 0;
@@ -2321,11 +2298,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->hw_dma_cap & HW_DMA_CAP_PTP))
+	if (macb_dma_is_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);
 
@@ -2785,14 +2760,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->hw_dma_cap & HW_DMA_CAP_64B)
+		if (macb_dma_is_64b(bp))
 			dmacfg |= GEM_BIT(ADDR64);
-#endif
-#ifdef CONFIG_MACB_USE_HWSTAMP
-		if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
+		if (macb_dma_is_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);
@@ -4298,12 +4269,10 @@ static int macb_init(struct platform_device *pdev)
 			queue->TBQP = GEM_TBQP(hw_q - 1);
 			queue->RBQP = GEM_RBQP(hw_q - 1);
 			queue->RBQS = GEM_RBQS(hw_q - 1);
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-			if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
+			if (macb_dma_is_64b(bp)) {
 				queue->TBQPH = GEM_TBQPH(hw_q - 1);
 				queue->RBQPH = GEM_RBQPH(hw_q - 1);
 			}
-#endif
 		} else {
 			/* queue0 uses legacy registers */
 			queue->ISR  = MACB_ISR;
@@ -4312,12 +4281,10 @@ static int macb_init(struct platform_device *pdev)
 			queue->IMR  = MACB_IMR;
 			queue->TBQP = MACB_TBQP;
 			queue->RBQP = MACB_RBQP;
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-			if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
+			if (macb_dma_is_64b(bp)) {
 				queue->TBQPH = MACB_TBQPH;
 				queue->RBQPH = MACB_RBQPH;
 			}
-#endif
 		}
 
 		/* get irq: here we use the linux queue index, not the hardware

-- 
2.50.0

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

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> Introduce macb_dma_is_64b() and macb_dma_is_ptp() helper functions.
> Many codepaths are made simpler by dropping conditional compilation.
> 
> This implies three changes:
>  - Always compile related structure definitions inside <macb.h>.
>  - Make the field hw_dma_cap in struct macb always present.
>  - MACB_EXT_DESC can be dropped as it is useless now.
> 
> The common case is:
> 
> 	#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
> 
> And replaced by:
> 
> 	struct macb_dma_desc_64 *desc_64;
> 	if (macb_dma_is_64b(bp)) {
> 		desc_64 = macb_64b_desc(bp, desc);
> 		// ...
> 	}
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      |   8 ---
>  drivers/net/ethernet/cadence/macb_main.c | 113 +++++++++++--------------------
>  2 files changed, 40 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3b43cb9468e3618754ff2bc6c5f360447bdeeed0..707b3286a6b8408a3bc4bbbcb1335ae8c3cd95ad 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
> @@ -823,7 +819,6 @@ struct macb_dma_desc {
>  	u32	ctrl;
>  };
>  
> -#ifdef MACB_EXT_DESC
>  #define HW_DMA_CAP_32B		0
>  #define HW_DMA_CAP_64B		(1 << 0)
>  #define HW_DMA_CAP_PTP		(1 << 1)
> @@ -838,7 +833,6 @@ struct macb_dma_desc_ptp {
>  	u32	ts_1;
>  	u32	ts_2;
>  };
> -#endif
>  
>  /* DMA descriptor bitfields */
>  #define MACB_RX_USED_OFFSET			0
> @@ -1316,9 +1310,7 @@ struct macb {
>  
>  	struct phy		*sgmii_phy;	/* for ZynqMP SGMII mode */
>  
> -#ifdef MACB_EXT_DESC
>  	uint8_t hw_dma_cap;
> -#endif

Should this member really be here? Seems like it would eat at least
7 bytes of padding. It probably makes sense to move this somewhere else
in the struct. Or maybe even fold this into MACB_CAPS? Not sure what
that does to text size.

>  	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 6926ea11d71adae7a25d5ee329c148f5882d4184..be667cb5acce85a9d11aaea1f5081a3380b60ef2 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -95,6 +95,18 @@ struct sifive_fu540_macb_mgmt {
>  
>  #define MACB_MDIO_TIMEOUT	1000000 /* in usecs */
>  
> +static bool macb_dma_is_64b(struct macb *bp)

TBH I would prefer just `macb_dma64` and `macb_dma_ptp` since the "_is_"
isn't really adding much.

> +{
> +	return IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
> +	       bp->hw_dma_cap & HW_DMA_CAP_64B;
> +}
> +
> +static bool macb_dma_is_ptp(struct macb *bp)
> +{
> +	return IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) &&
> +	       bp->hw_dma_cap & HW_DMA_CAP_PTP;
> +}
> +
>  /* DMA buffer descriptor might be different size
>   * depends on hardware configuration:
>   *
> @@ -124,56 +136,31 @@ 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);
> +
> +	if (macb_dma_is_64b(bp))
> +		desc_size += sizeof(struct macb_dma_desc_64);
> +	if (macb_dma_is_ptp(bp))
> +		desc_size += sizeof(struct macb_dma_desc_ptp);
>  
> -	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;
> -#endif
> -	return sizeof(struct macb_dma_desc);
>  }
>  
>  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
> -	return desc_idx;
> +	if (macb_dma_is_64b(bp) && macb_dma_is_ptp(bp))
> +		return desc_idx * 3;
> +	else if (macb_dma_is_64b(bp) || macb_dma_is_ptp(bp))
> +		return desc_idx << 1;
> +	else
> +		return desc_idx;

Can this be

return desc_idx * (1 + 64() + ptp())

? Not sure what the assembly looks like.

>  }
>  
> -#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)
> @@ -497,17 +484,13 @@ static void macb_init_buffers(struct macb *bp)
>  
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>  		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +		if (macb_dma_is_64b(bp))
>  			queue_writel(queue, RBQPH,
>  				     upper_32_bits(queue->rx_ring_dma));
> -#endif
>  		queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +		if (macb_dma_is_64b(bp))
>  			queue_writel(queue, TBQPH,
>  				     upper_32_bits(queue->tx_ring_dma));
> -#endif
>  	}
>  }
>  
> @@ -1028,10 +1011,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_dma_is_64b(bp)) {
> +		struct macb_dma_desc_64 *desc_64;
>  
> -	if (bp->hw_dma_cap & HW_DMA_CAP_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
> @@ -1040,26 +1022,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->hw_dma_cap & HW_DMA_CAP_64B) {
> +	if (macb_dma_is_64b(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->hw_dma_cap & HW_DMA_CAP_PTP)
> +	if (macb_dma_is_ptp(bp))
>  		addr &= ~GEM_BIT(DMA_RXVALID);
> -#endif
>  	return addr;
>  }
>  
> @@ -1166,10 +1145,8 @@ static void macb_tx_error_task(struct work_struct *work)
>  
>  	/* Reinitialize the TX desc queue */
>  	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +	if (macb_dma_is_64b(bp))
>  		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
> -#endif
>  	/* Make TX ring reflect state of hardware */
>  	queue->tx_head = 0;
>  	queue->tx_tail = 0;
> @@ -2321,11 +2298,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->hw_dma_cap & HW_DMA_CAP_PTP))
> +	if (macb_dma_is_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);
>  
> @@ -2785,14 +2760,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->hw_dma_cap & HW_DMA_CAP_64B)
> +		if (macb_dma_is_64b(bp))
>  			dmacfg |= GEM_BIT(ADDR64);
> -#endif
> -#ifdef CONFIG_MACB_USE_HWSTAMP
> -		if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
> +		if (macb_dma_is_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);
> @@ -4298,12 +4269,10 @@ static int macb_init(struct platform_device *pdev)
>  			queue->TBQP = GEM_TBQP(hw_q - 1);
>  			queue->RBQP = GEM_RBQP(hw_q - 1);
>  			queue->RBQS = GEM_RBQS(hw_q - 1);
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -			if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
> +			if (macb_dma_is_64b(bp)) {
>  				queue->TBQPH = GEM_TBQPH(hw_q - 1);
>  				queue->RBQPH = GEM_RBQPH(hw_q - 1);
>  			}
> -#endif
>  		} else {
>  			/* queue0 uses legacy registers */
>  			queue->ISR  = MACB_ISR;
> @@ -4312,12 +4281,10 @@ static int macb_init(struct platform_device *pdev)
>  			queue->IMR  = MACB_IMR;
>  			queue->TBQP = MACB_TBQP;
>  			queue->RBQP = MACB_RBQP;
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -			if (bp->hw_dma_cap & HW_DMA_CAP_64B) {
> +			if (macb_dma_is_64b(bp)) {
>  				queue->TBQPH = MACB_TBQPH;
>  				queue->RBQPH = MACB_RBQPH;
>  			}
> -#endif
>  		}
>  
>  		/* get irq: here we use the linux queue index, not the hardware
> 

[PATCH net-next v2 09/18] net: macb: sort #includes

[Théo Lebrun]

Sort #include preprocessor directives.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index be667cb5acce85a9d11aaea1f5081a3380b60ef2..a6633e076644089c796453f856a766299bae2ec6 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -6,36 +6,36 @@
  */
 
 #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/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 "macb.h"
 
 /* This structure is only used for MACB on SiFive FU540 devices */

-- 
2.50.0

Re: [PATCH net-next v2 09/18] net: macb: sort #includes

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> Sort #include preprocessor directives.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 36 ++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index be667cb5acce85a9d11aaea1f5081a3380b60ef2..a6633e076644089c796453f856a766299bae2ec6 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -6,36 +6,36 @@
>   */
>  
>  #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/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 "macb.h"
>  
>  /* This structure is only used for MACB on SiFive FU540 devices */
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

[PATCH net-next v2 10/18] net: macb: remove illusion about TBQPH/RBQPH being per-queue

[Théo Lebrun]

The MACB driver acts as if TBQPH/RBQPH are configurable on a per queue
basis; this is a lie. A single register configures the upper 32 bits of
each DMA descriptor buffers for all queues.

Concrete actions:

 - Drop GEM_TBQPH/GEM_RBQPH macros which have a queue index argument.
   Only use MACB_TBQPH/MACB_RBQPH constants.

 - Drop struct macb_queue->TBQPH/RBQPH fields.

 - In macb_init_buffers(): do a single write to TBQPH and RBQPH for all
   queues instead of a write per queue.

 - In macb_tx_error_task(): drop the write to TBQPH.

 - In macb_alloc_consistent(): if allocations give different upper
   32-bits, fail. Previously, it would have lead to silent memory
   corruption as queues would have used the upper 32 bits of the alloc
   from queue 0 and their own low 32 bits.

 - In macb_suspend(): if we use the tie off descriptor for suspend, do
   the write once for all queues instead of once per queue.

Fixes: fff8019a08b6 ("net: macb: Add 64 bit addressing support for GEM")
Fixes: ae1f2a56d273 ("net: macb: Added support for many RX queues")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      |  4 ----
 drivers/net/ethernet/cadence/macb_main.c | 36 +++++++++++++-------------------
 2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 707b3286a6b8408a3bc4bbbcb1335ae8c3cd95ad..adc70b6efd52b0b11e436c2c95bb5108c40f3490 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -209,10 +209,8 @@
 
 #define GEM_ISR(hw_q)		(0x0400 + ((hw_q) << 2))
 #define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
-#define GEM_TBQPH(hw_q)		(0x04C8)
 #define GEM_RBQP(hw_q)		(0x0480 + ((hw_q) << 2))
 #define GEM_RBQS(hw_q)		(0x04A0 + ((hw_q) << 2))
-#define GEM_RBQPH(hw_q)		(0x04D4)
 #define GEM_IER(hw_q)		(0x0600 + ((hw_q) << 2))
 #define GEM_IDR(hw_q)		(0x0620 + ((hw_q) << 2))
 #define GEM_IMR(hw_q)		(0x0640 + ((hw_q) << 2))
@@ -1208,10 +1206,8 @@ struct macb_queue {
 	unsigned int		IDR;
 	unsigned int		IMR;
 	unsigned int		TBQP;
-	unsigned int		TBQPH;
 	unsigned int		RBQS;
 	unsigned int		RBQP;
-	unsigned int		RBQPH;
 
 	/* Lock to protect tx_head and tx_tail */
 	spinlock_t		tx_ptr_lock;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a6633e076644089c796453f856a766299bae2ec6..d3b3635998cad095246edf8a75faebbcf7115355 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -482,15 +482,15 @@ static void macb_init_buffers(struct macb *bp)
 	struct macb_queue *queue;
 	unsigned int q;
 
+	if (macb_dma_is_64b(bp)) {
+		/* Single register for all queues' high 32 bits. */
+		macb_writel(bp, RBQPH, upper_32_bits(bp->queues->rx_ring_dma));
+		macb_writel(bp, TBQPH, upper_32_bits(bp->queues->tx_ring_dma));
+	}
+
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
-		if (macb_dma_is_64b(bp))
-			queue_writel(queue, RBQPH,
-				     upper_32_bits(queue->rx_ring_dma));
 		queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
-		if (macb_dma_is_64b(bp))
-			queue_writel(queue, TBQPH,
-				     upper_32_bits(queue->tx_ring_dma));
 	}
 }
 
@@ -1145,8 +1145,6 @@ static void macb_tx_error_task(struct work_struct *work)
 
 	/* Reinitialize the TX desc queue */
 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
-	if (macb_dma_is_64b(bp))
-		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
 	/* Make TX ring reflect state of hardware */
 	queue->tx_head = 0;
 	queue->tx_tail = 0;
@@ -2524,7 +2522,8 @@ static int macb_alloc_consistent(struct macb *bp)
 		queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
 						    &queue->tx_ring_dma,
 						    GFP_KERNEL);
-		if (!queue->tx_ring)
+		if (!queue->tx_ring ||
+		    upper_32_bits(queue->tx_ring_dma) != upper_32_bits(bp->queues->tx_ring_dma))
 			goto out_err;
 		netdev_dbg(bp->dev,
 			   "Allocated TX ring for queue %u of %d bytes at %08lx (mapped %p)\n",
@@ -2539,7 +2538,8 @@ static int macb_alloc_consistent(struct macb *bp)
 		size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
 		queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
 						 &queue->rx_ring_dma, GFP_KERNEL);
-		if (!queue->rx_ring)
+		if (!queue->rx_ring ||
+		    upper_32_bits(queue->rx_ring_dma) != upper_32_bits(bp->queues->rx_ring_dma))
 			goto out_err;
 		netdev_dbg(bp->dev,
 			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
@@ -4269,10 +4269,6 @@ static int macb_init(struct platform_device *pdev)
 			queue->TBQP = GEM_TBQP(hw_q - 1);
 			queue->RBQP = GEM_RBQP(hw_q - 1);
 			queue->RBQS = GEM_RBQS(hw_q - 1);
-			if (macb_dma_is_64b(bp)) {
-				queue->TBQPH = GEM_TBQPH(hw_q - 1);
-				queue->RBQPH = GEM_RBQPH(hw_q - 1);
-			}
 		} else {
 			/* queue0 uses legacy registers */
 			queue->ISR  = MACB_ISR;
@@ -4281,10 +4277,6 @@ static int macb_init(struct platform_device *pdev)
 			queue->IMR  = MACB_IMR;
 			queue->TBQP = MACB_TBQP;
 			queue->RBQP = MACB_RBQP;
-			if (macb_dma_is_64b(bp)) {
-				queue->TBQPH = MACB_TBQPH;
-				queue->RBQPH = MACB_RBQPH;
-			}
 		}
 
 		/* get irq: here we use the linux queue index, not the hardware
@@ -5401,6 +5393,10 @@ static int __maybe_unused macb_suspend(struct device *dev)
 		 */
 		tmp = macb_readl(bp, NCR);
 		macb_writel(bp, NCR, tmp & ~(MACB_BIT(TE) | MACB_BIT(RE)));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+		if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE))
+			macb_writel(bp, RBQPH, upper_32_bits(bp->rx_ring_tieoff_dma));
+#endif
 		for (q = 0, queue = bp->queues; q < bp->num_queues;
 		     ++q, ++queue) {
 			/* Disable RX queues */
@@ -5410,10 +5406,6 @@ static int __maybe_unused macb_suspend(struct device *dev)
 				/* Tie off RX queues */
 				queue_writel(queue, RBQP,
 					     lower_32_bits(bp->rx_ring_tieoff_dma));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-				queue_writel(queue, RBQPH,
-					     upper_32_bits(bp->rx_ring_tieoff_dma));
-#endif
 			}
 			/* Disable all interrupts */
 			queue_writel(queue, IDR, -1);

-- 
2.50.0

Re: [PATCH net-next v2 10/18] net: macb: remove illusion about TBQPH/RBQPH being per-queue

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> The MACB driver acts as if TBQPH/RBQPH are configurable on a per queue
> basis; this is a lie. A single register configures the upper 32 bits of
> each DMA descriptor buffers for all queues.
> 
> Concrete actions:
> 
>  - Drop GEM_TBQPH/GEM_RBQPH macros which have a queue index argument.
>    Only use MACB_TBQPH/MACB_RBQPH constants.
> 
>  - Drop struct macb_queue->TBQPH/RBQPH fields.
> 
>  - In macb_init_buffers(): do a single write to TBQPH and RBQPH for all
>    queues instead of a write per queue.
> 
>  - In macb_tx_error_task(): drop the write to TBQPH.
> 
>  - In macb_alloc_consistent(): if allocations give different upper
>    32-bits, fail. Previously, it would have lead to silent memory
>    corruption as queues would have used the upper 32 bits of the alloc
>    from queue 0 and their own low 32 bits.

While better than silent memory corruption, this is not a good solution
since bringing the netdev up will now randomly fail. Can we allocate the
rings in one contiguous chunk instead?

>  - In macb_suspend(): if we use the tie off descriptor for suspend, do
>    the write once for all queues instead of once per queue.
> 
> Fixes: fff8019a08b6 ("net: macb: Add 64 bit addressing support for GEM")
> Fixes: ae1f2a56d273 ("net: macb: Added support for many RX queues")
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

As this is a bugfix, can you move it before your cleanup patches? This
will make it easier to backport to stable kernels.

> ---
>  drivers/net/ethernet/cadence/macb.h      |  4 ----
>  drivers/net/ethernet/cadence/macb_main.c | 36 +++++++++++++-------------------
>  2 files changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 707b3286a6b8408a3bc4bbbcb1335ae8c3cd95ad..adc70b6efd52b0b11e436c2c95bb5108c40f3490 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -209,10 +209,8 @@
>  
>  #define GEM_ISR(hw_q)		(0x0400 + ((hw_q) << 2))
>  #define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
> -#define GEM_TBQPH(hw_q)		(0x04C8)
>  #define GEM_RBQP(hw_q)		(0x0480 + ((hw_q) << 2))
>  #define GEM_RBQS(hw_q)		(0x04A0 + ((hw_q) << 2))
> -#define GEM_RBQPH(hw_q)		(0x04D4)
>  #define GEM_IER(hw_q)		(0x0600 + ((hw_q) << 2))
>  #define GEM_IDR(hw_q)		(0x0620 + ((hw_q) << 2))
>  #define GEM_IMR(hw_q)		(0x0640 + ((hw_q) << 2))
> @@ -1208,10 +1206,8 @@ struct macb_queue {
>  	unsigned int		IDR;
>  	unsigned int		IMR;
>  	unsigned int		TBQP;
> -	unsigned int		TBQPH;
>  	unsigned int		RBQS;
>  	unsigned int		RBQP;
> -	unsigned int		RBQPH;
>  
>  	/* Lock to protect tx_head and tx_tail */
>  	spinlock_t		tx_ptr_lock;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index a6633e076644089c796453f856a766299bae2ec6..d3b3635998cad095246edf8a75faebbcf7115355 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -482,15 +482,15 @@ static void macb_init_buffers(struct macb *bp)
>  	struct macb_queue *queue;
>  	unsigned int q;
>  
> +	if (macb_dma_is_64b(bp)) {
> +		/* Single register for all queues' high 32 bits. */
> +		macb_writel(bp, RBQPH, upper_32_bits(bp->queues->rx_ring_dma));
> +		macb_writel(bp, TBQPH, upper_32_bits(bp->queues->tx_ring_dma));
> +	}
> +
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>  		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
> -		if (macb_dma_is_64b(bp))
> -			queue_writel(queue, RBQPH,
> -				     upper_32_bits(queue->rx_ring_dma));
>  		queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> -		if (macb_dma_is_64b(bp))
> -			queue_writel(queue, TBQPH,
> -				     upper_32_bits(queue->tx_ring_dma));
>  	}
>  }
>  
> @@ -1145,8 +1145,6 @@ static void macb_tx_error_task(struct work_struct *work)
>  
>  	/* Reinitialize the TX desc queue */
>  	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> -	if (macb_dma_is_64b(bp))
> -		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
>  	/* Make TX ring reflect state of hardware */
>  	queue->tx_head = 0;
>  	queue->tx_tail = 0;
> @@ -2524,7 +2522,8 @@ static int macb_alloc_consistent(struct macb *bp)
>  		queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>  						    &queue->tx_ring_dma,
>  						    GFP_KERNEL);
> -		if (!queue->tx_ring)
> +		if (!queue->tx_ring ||
> +		    upper_32_bits(queue->tx_ring_dma) != upper_32_bits(bp->queues->tx_ring_dma))
>  			goto out_err;
>  		netdev_dbg(bp->dev,
>  			   "Allocated TX ring for queue %u of %d bytes at %08lx (mapped %p)\n",
> @@ -2539,7 +2538,8 @@ static int macb_alloc_consistent(struct macb *bp)
>  		size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
>  		queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>  						 &queue->rx_ring_dma, GFP_KERNEL);
> -		if (!queue->rx_ring)
> +		if (!queue->rx_ring ||
> +		    upper_32_bits(queue->rx_ring_dma) != upper_32_bits(bp->queues->rx_ring_dma))

Can you write this as bp->queues[0].rx_ring_dma for clarity?

>  			goto out_err;
>  		netdev_dbg(bp->dev,
>  			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
> @@ -4269,10 +4269,6 @@ static int macb_init(struct platform_device *pdev)
>  			queue->TBQP = GEM_TBQP(hw_q - 1);
>  			queue->RBQP = GEM_RBQP(hw_q - 1);
>  			queue->RBQS = GEM_RBQS(hw_q - 1);
> -			if (macb_dma_is_64b(bp)) {
> -				queue->TBQPH = GEM_TBQPH(hw_q - 1);
> -				queue->RBQPH = GEM_RBQPH(hw_q - 1);
> -			}
>  		} else {
>  			/* queue0 uses legacy registers */
>  			queue->ISR  = MACB_ISR;
> @@ -4281,10 +4277,6 @@ static int macb_init(struct platform_device *pdev)
>  			queue->IMR  = MACB_IMR;
>  			queue->TBQP = MACB_TBQP;
>  			queue->RBQP = MACB_RBQP;
> -			if (macb_dma_is_64b(bp)) {
> -				queue->TBQPH = MACB_TBQPH;
> -				queue->RBQPH = MACB_RBQPH;
> -			}
>  		}
>  
>  		/* get irq: here we use the linux queue index, not the hardware
> @@ -5401,6 +5393,10 @@ static int __maybe_unused macb_suspend(struct device *dev)
>  		 */
>  		tmp = macb_readl(bp, NCR);
>  		macb_writel(bp, NCR, tmp & ~(MACB_BIT(TE) | MACB_BIT(RE)));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +		if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE))
> +			macb_writel(bp, RBQPH, upper_32_bits(bp->rx_ring_tieoff_dma));
> +#endif
>  		for (q = 0, queue = bp->queues; q < bp->num_queues;
>  		     ++q, ++queue) {
>  			/* Disable RX queues */
> @@ -5410,10 +5406,6 @@ static int __maybe_unused macb_suspend(struct device *dev)
>  				/* Tie off RX queues */
>  				queue_writel(queue, RBQP,
>  					     lower_32_bits(bp->rx_ring_tieoff_dma));
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -				queue_writel(queue, RBQPH,
> -					     upper_32_bits(bp->rx_ring_tieoff_dma));
> -#endif
>  			}
>  			/* Disable all interrupts */
>  			queue_writel(queue, IDR, -1);
> 

Re: [PATCH net-next v2 10/18] net: macb: remove illusion about TBQPH/RBQPH being per-queue

[Sean Anderson]

On 7/1/25 12:15, Sean Anderson wrote:
> On 6/27/25 05:08, Théo Lebrun wrote:
>> The MACB driver acts as if TBQPH/RBQPH are configurable on a per queue
>> basis; this is a lie. A single register configures the upper 32 bits of
>> each DMA descriptor buffers for all queues.
>> 
>> Concrete actions:
>> 
>>  - Drop GEM_TBQPH/GEM_RBQPH macros which have a queue index argument.
>>    Only use MACB_TBQPH/MACB_RBQPH constants.
>> 
>>  - Drop struct macb_queue->TBQPH/RBQPH fields.
>> 
>>  - In macb_init_buffers(): do a single write to TBQPH and RBQPH for all
>>    queues instead of a write per queue.
>> 
>>  - In macb_tx_error_task(): drop the write to TBQPH.
>> 
>>  - In macb_alloc_consistent(): if allocations give different upper
>>    32-bits, fail. Previously, it would have lead to silent memory
>>    corruption as queues would have used the upper 32 bits of the alloc
>>    from queue 0 and their own low 32 bits.
> 
> While better than silent memory corruption, this is not a good solution
> since bringing the netdev up will now randomly fail. Can we allocate the
> rings in one contiguous chunk instead?

Ah, looks like you do this in the next patch. In that case, (with the other
comments addressed)

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

>>  - In macb_suspend(): if we use the tie off descriptor for suspend, do
>>    the write once for all queues instead of once per queue.
>> 
>> Fixes: fff8019a08b6 ("net: macb: Add 64 bit addressing support for GEM")
>> Fixes: ae1f2a56d273 ("net: macb: Added support for many RX queues")
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> 
> As this is a bugfix, can you move it before your cleanup patches? This
> will make it easier to backport to stable kernels.
> 
>> ---
>>  drivers/net/ethernet/cadence/macb.h      |  4 ----
>>  drivers/net/ethernet/cadence/macb_main.c | 36 +++++++++++++-------------------
>>  2 files changed, 14 insertions(+), 26 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
>> index 707b3286a6b8408a3bc4bbbcb1335ae8c3cd95ad..adc70b6efd52b0b11e436c2c95bb5108c40f3490 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -209,10 +209,8 @@
>>  
>>  #define GEM_ISR(hw_q)		(0x0400 + ((hw_q) << 2))
>>  #define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
>> -#define GEM_TBQPH(hw_q)		(0x04C8)
>>  #define GEM_RBQP(hw_q)		(0x0480 + ((hw_q) << 2))
>>  #define GEM_RBQS(hw_q)		(0x04A0 + ((hw_q) << 2))
>> -#define GEM_RBQPH(hw_q)		(0x04D4)
>>  #define GEM_IER(hw_q)		(0x0600 + ((hw_q) << 2))
>>  #define GEM_IDR(hw_q)		(0x0620 + ((hw_q) << 2))
>>  #define GEM_IMR(hw_q)		(0x0640 + ((hw_q) << 2))
>> @@ -1208,10 +1206,8 @@ struct macb_queue {
>>  	unsigned int		IDR;
>>  	unsigned int		IMR;
>>  	unsigned int		TBQP;
>> -	unsigned int		TBQPH;
>>  	unsigned int		RBQS;
>>  	unsigned int		RBQP;
>> -	unsigned int		RBQPH;
>>  
>>  	/* Lock to protect tx_head and tx_tail */
>>  	spinlock_t		tx_ptr_lock;
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index a6633e076644089c796453f856a766299bae2ec6..d3b3635998cad095246edf8a75faebbcf7115355 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -482,15 +482,15 @@ static void macb_init_buffers(struct macb *bp)
>>  	struct macb_queue *queue;
>>  	unsigned int q;
>>  
>> +	if (macb_dma_is_64b(bp)) {
>> +		/* Single register for all queues' high 32 bits. */
>> +		macb_writel(bp, RBQPH, upper_32_bits(bp->queues->rx_ring_dma));
>> +		macb_writel(bp, TBQPH, upper_32_bits(bp->queues->tx_ring_dma));
>> +	}
>> +
>>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>>  		queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
>> -		if (macb_dma_is_64b(bp))
>> -			queue_writel(queue, RBQPH,
>> -				     upper_32_bits(queue->rx_ring_dma));
>>  		queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>> -		if (macb_dma_is_64b(bp))
>> -			queue_writel(queue, TBQPH,
>> -				     upper_32_bits(queue->tx_ring_dma));
>>  	}
>>  }
>>  
>> @@ -1145,8 +1145,6 @@ static void macb_tx_error_task(struct work_struct *work)
>>  
>>  	/* Reinitialize the TX desc queue */
>>  	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>> -	if (macb_dma_is_64b(bp))
>> -		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
>>  	/* Make TX ring reflect state of hardware */
>>  	queue->tx_head = 0;
>>  	queue->tx_tail = 0;
>> @@ -2524,7 +2522,8 @@ static int macb_alloc_consistent(struct macb *bp)
>>  		queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>>  						    &queue->tx_ring_dma,
>>  						    GFP_KERNEL);
>> -		if (!queue->tx_ring)
>> +		if (!queue->tx_ring ||
>> +		    upper_32_bits(queue->tx_ring_dma) != upper_32_bits(bp->queues->tx_ring_dma))
>>  			goto out_err;
>>  		netdev_dbg(bp->dev,
>>  			   "Allocated TX ring for queue %u of %d bytes at %08lx (mapped %p)\n",
>> @@ -2539,7 +2538,8 @@ static int macb_alloc_consistent(struct macb *bp)
>>  		size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
>>  		queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>>  						 &queue->rx_ring_dma, GFP_KERNEL);
>> -		if (!queue->rx_ring)
>> +		if (!queue->rx_ring ||
>> +		    upper_32_bits(queue->rx_ring_dma) != upper_32_bits(bp->queues->rx_ring_dma))
> 
> Can you write this as bp->queues[0].rx_ring_dma for clarity?
> 
>>  			goto out_err;
>>  		netdev_dbg(bp->dev,
>>  			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
>> @@ -4269,10 +4269,6 @@ static int macb_init(struct platform_device *pdev)
>>  			queue->TBQP = GEM_TBQP(hw_q - 1);
>>  			queue->RBQP = GEM_RBQP(hw_q - 1);
>>  			queue->RBQS = GEM_RBQS(hw_q - 1);
>> -			if (macb_dma_is_64b(bp)) {
>> -				queue->TBQPH = GEM_TBQPH(hw_q - 1);
>> -				queue->RBQPH = GEM_RBQPH(hw_q - 1);
>> -			}
>>  		} else {
>>  			/* queue0 uses legacy registers */
>>  			queue->ISR  = MACB_ISR;
>> @@ -4281,10 +4277,6 @@ static int macb_init(struct platform_device *pdev)
>>  			queue->IMR  = MACB_IMR;
>>  			queue->TBQP = MACB_TBQP;
>>  			queue->RBQP = MACB_RBQP;
>> -			if (macb_dma_is_64b(bp)) {
>> -				queue->TBQPH = MACB_TBQPH;
>> -				queue->RBQPH = MACB_RBQPH;
>> -			}
>>  		}
>>  
>>  		/* get irq: here we use the linux queue index, not the hardware
>> @@ -5401,6 +5393,10 @@ static int __maybe_unused macb_suspend(struct device *dev)
>>  		 */
>>  		tmp = macb_readl(bp, NCR);
>>  		macb_writel(bp, NCR, tmp & ~(MACB_BIT(TE) | MACB_BIT(RE)));
>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +		if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE))
>> +			macb_writel(bp, RBQPH, upper_32_bits(bp->rx_ring_tieoff_dma));
>> +#endif
>>  		for (q = 0, queue = bp->queues; q < bp->num_queues;
>>  		     ++q, ++queue) {
>>  			/* Disable RX queues */
>> @@ -5410,10 +5406,6 @@ static int __maybe_unused macb_suspend(struct device *dev)
>>  				/* Tie off RX queues */
>>  				queue_writel(queue, RBQP,
>>  					     lower_32_bits(bp->rx_ring_tieoff_dma));
>> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> -				queue_writel(queue, RBQPH,
>> -					     upper_32_bits(bp->rx_ring_tieoff_dma));
>> -#endif
>>  			}
>>  			/* Disable all interrupts */
>>  			queue_writel(queue, IDR, -1);
>> 

[PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors

[Théo Lebrun]

Move from two (Tx/Rx) dma_alloc_coherent() for DMA descriptor rings *per
queue* to two dma_alloc_coherent() overall.

Issue is with how all queues share the same register for configuring the
upper 32-bits of Tx/Rx descriptor rings. For example, with Tx, notice
how TBQPH does *not* depend on the queue index:

	#define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
	#define GEM_TBQPH(hw_q)		(0x04C8)

	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
	#endif

To maxime our chances of getting valid DMA addresses, we do a single
dma_alloc_coherent() across queues. This improves the odds because
alloc_pages() guarantees natural alignment. It cannot ensure valid DMA
addresses because of IOMMU or codepaths that don't go through
alloc_pages().

We error out if all rings don't have the same upper 32 bits, which is
better than the current (theoretical, not reproduced) silent corruption
caused by hardware that accesses invalid addresses.

Two considerations:
 - dma_alloc_coherent() gives us page alignment. Here we remove this
   containst meaning each queue's ring won't be page-aligned anymore.
 - This can save some memory. Less allocations means less overhead
   (constant cost per alloc) and less wasted bytes due to alignment
   constraints.

Fixes: 02c958dd3446 ("net/macb: add TX multiqueue support for gem")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 83 ++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d3b3635998cad095246edf8a75faebbcf7115355..48b75d95861317b9925b366446c7572c7e186628 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2445,33 +2445,32 @@ static void macb_free_rx_buffers(struct macb *bp)
 
 static void macb_free_consistent(struct macb *bp)
 {
-	struct macb_queue *queue;
+	size_t size, tx_size_per_queue, rx_size_per_queue;
+	struct macb_queue *queue, *queue0 = bp->queues;
+	struct device *dev = &bp->pdev->dev;
 	unsigned int q;
-	int size;
 
 	if (bp->rx_ring_tieoff) {
-		dma_free_coherent(&bp->pdev->dev, macb_dma_desc_get_size(bp),
+		dma_free_coherent(dev, macb_dma_desc_get_size(bp),
 				  bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
 		bp->rx_ring_tieoff = NULL;
 	}
 
 	bp->macbgem_ops.mog_free_rx_buffers(bp);
 
+	tx_size_per_queue = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
+	size = bp->num_queues * tx_size_per_queue;
+	dma_free_coherent(dev, size, queue0->tx_ring, queue0->tx_ring_dma);
+
+	rx_size_per_queue = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
+	size = bp->num_queues * rx_size_per_queue;
+	dma_free_coherent(dev, size, queue0->rx_ring, queue0->rx_ring_dma);
+
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		kfree(queue->tx_skb);
 		queue->tx_skb = NULL;
-		if (queue->tx_ring) {
-			size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
-			dma_free_coherent(&bp->pdev->dev, size,
-					  queue->tx_ring, queue->tx_ring_dma);
-			queue->tx_ring = NULL;
-		}
-		if (queue->rx_ring) {
-			size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
-			dma_free_coherent(&bp->pdev->dev, size,
-					  queue->rx_ring, queue->rx_ring_dma);
-			queue->rx_ring = NULL;
-		}
+		queue->tx_ring = NULL; /* Single buffer owned by queue0 */
+		queue->rx_ring = NULL; /* Single buffer owned by queue0 */
 	}
 }
 
@@ -2513,37 +2512,47 @@ static int macb_alloc_rx_buffers(struct macb *bp)
 
 static int macb_alloc_consistent(struct macb *bp)
 {
+	size_t size, tx_size_per_queue, rx_size_per_queue;
+	dma_addr_t tx_dma, rx_dma;
+	struct device *dev = &bp->pdev->dev;
 	struct macb_queue *queue;
 	unsigned int q;
-	int size;
+	void *tx, *rx;
+
+	/*
+	 * Upper 32-bits of Tx/Rx DMA descriptor for each queues much match!
+	 * We cannot enforce this guarantee, the best we can do is do a single
+	 * allocation and hope it will land into alloc_pages() that guarantees
+	 * natural alignment of physical addresses.
+	 */
+
+	tx_size_per_queue = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
+	size = bp->num_queues * tx_size_per_queue;
+	tx = dma_alloc_coherent(dev, size, &tx_dma, GFP_KERNEL);
+	if (!tx || upper_32_bits(tx_dma) != upper_32_bits(tx_dma + size - 1))
+		goto out_err;
+	netdev_dbg(bp->dev, "Allocated %zu bytes for %u TX rings at %08lx (mapped %p)\n",
+		   size, bp->num_queues, (unsigned long)tx_dma, tx);
+
+	rx_size_per_queue = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
+	size = bp->num_queues * rx_size_per_queue;
+	rx = dma_alloc_coherent(dev, size, &rx_dma, GFP_KERNEL);
+	if (!rx || upper_32_bits(rx_dma) != upper_32_bits(rx_dma + size - 1))
+		goto out_err;
+	netdev_dbg(bp->dev, "Allocated %zu bytes for %u RX rings at %08lx (mapped %p)\n",
+		   size, bp->num_queues, (unsigned long)rx_dma, rx);
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
-		size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
-		queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
-						    &queue->tx_ring_dma,
-						    GFP_KERNEL);
-		if (!queue->tx_ring ||
-		    upper_32_bits(queue->tx_ring_dma) != upper_32_bits(bp->queues->tx_ring_dma))
-			goto out_err;
-		netdev_dbg(bp->dev,
-			   "Allocated TX ring for queue %u of %d bytes at %08lx (mapped %p)\n",
-			   q, size, (unsigned long)queue->tx_ring_dma,
-			   queue->tx_ring);
+		queue->tx_ring = tx + tx_size_per_queue * q;
+		queue->tx_ring_dma = tx_dma + tx_size_per_queue * q;
+
+		queue->rx_ring = rx + rx_size_per_queue * q;
+		queue->rx_ring_dma = rx_dma + rx_size_per_queue * q;
 
 		size = bp->tx_ring_size * sizeof(struct macb_tx_skb);
 		queue->tx_skb = kmalloc(size, GFP_KERNEL);
 		if (!queue->tx_skb)
 			goto out_err;
-
-		size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
-		queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
-						 &queue->rx_ring_dma, GFP_KERNEL);
-		if (!queue->rx_ring ||
-		    upper_32_bits(queue->rx_ring_dma) != upper_32_bits(bp->queues->rx_ring_dma))
-			goto out_err;
-		netdev_dbg(bp->dev,
-			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
-			   size, (unsigned long)queue->rx_ring_dma, queue->rx_ring);
 	}
 	if (bp->macbgem_ops.mog_alloc_rx_buffers(bp))
 		goto out_err;

-- 
2.50.0

Re: [PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> Move from two (Tx/Rx) dma_alloc_coherent() for DMA descriptor rings *per
> queue* to two dma_alloc_coherent() overall.
> 
> Issue is with how all queues share the same register for configuring the
> upper 32-bits of Tx/Rx descriptor rings. For example, with Tx, notice
> how TBQPH does *not* depend on the queue index:
> 
> 	#define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
> 	#define GEM_TBQPH(hw_q)		(0x04C8)
> 
> 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> 	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> 	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> 		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
> 	#endif
> 
> To maxime our chances of getting valid DMA addresses, we do a single

maximize

> dma_alloc_coherent() across queues.

Is there really any chance involved (other than avoiding ENOMEM)?

> This improves the odds because
> alloc_pages() guarantees natural alignment. It cannot ensure valid DMA
> addresses because of IOMMU or codepaths that don't go through
> alloc_pages().
> 
> We error out if all rings don't have the same upper 32 bits, which is
> better than the current (theoretical, not reproduced) silent corruption
> caused by hardware that accesses invalid addresses.

I think this is addressed by the previous patch.

> Two considerations:
>  - dma_alloc_coherent() gives us page alignment. Here we remove this
>    containst meaning each queue's ring won't be page-aligned anymore.

constraint

>  - This can save some memory. Less allocations means less overhead

fewer

>    (constant cost per alloc) and less wasted bytes due to alignment
>    constraints.

I think it's probably a bit of a wash with reasonably-sized rings.
Although the prefetch probably interacts poorly with the default "round"
power-of-two ring sizes.

> Fixes: 02c958dd3446 ("net/macb: add TX multiqueue support for gem")
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 83 ++++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index d3b3635998cad095246edf8a75faebbcf7115355..48b75d95861317b9925b366446c7572c7e186628 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2445,33 +2445,32 @@ static void macb_free_rx_buffers(struct macb *bp)
>  
>  static void macb_free_consistent(struct macb *bp)
>  {
> -	struct macb_queue *queue;
> +	size_t size, tx_size_per_queue, rx_size_per_queue;
> +	struct macb_queue *queue, *queue0 = bp->queues;
> +	struct device *dev = &bp->pdev->dev;
>  	unsigned int q;
> -	int size;
>  
>  	if (bp->rx_ring_tieoff) {
> -		dma_free_coherent(&bp->pdev->dev, macb_dma_desc_get_size(bp),
> +		dma_free_coherent(dev, macb_dma_desc_get_size(bp),
>  				  bp->rx_ring_tieoff, bp->rx_ring_tieoff_dma);
>  		bp->rx_ring_tieoff = NULL;
>  	}
>  
>  	bp->macbgem_ops.mog_free_rx_buffers(bp);
>  
> +	tx_size_per_queue = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> +	size = bp->num_queues * tx_size_per_queue;

Can you refactor the size calculation into a helper function?

> +	dma_free_coherent(dev, size, queue0->tx_ring, queue0->tx_ring_dma);
> +
> +	rx_size_per_queue = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> +	size = bp->num_queues * rx_size_per_queue;
> +	dma_free_coherent(dev, size, queue0->rx_ring, queue0->rx_ring_dma);
> +
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>  		kfree(queue->tx_skb);
>  		queue->tx_skb = NULL;
> -		if (queue->tx_ring) {
> -			size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> -			dma_free_coherent(&bp->pdev->dev, size,
> -					  queue->tx_ring, queue->tx_ring_dma);
> -			queue->tx_ring = NULL;
> -		}
> -		if (queue->rx_ring) {
> -			size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> -			dma_free_coherent(&bp->pdev->dev, size,
> -					  queue->rx_ring, queue->rx_ring_dma);
> -			queue->rx_ring = NULL;
> -		}
> +		queue->tx_ring = NULL; /* Single buffer owned by queue0 */
> +		queue->rx_ring = NULL; /* Single buffer owned by queue0 */

OK, but queue0 doesn't own the ring either at this point (it's free'd).

>  	}
>  }
>  
> @@ -2513,37 +2512,47 @@ static int macb_alloc_rx_buffers(struct macb *bp)
>  
>  static int macb_alloc_consistent(struct macb *bp)
>  {
> +	size_t size, tx_size_per_queue, rx_size_per_queue;
> +	dma_addr_t tx_dma, rx_dma;
> +	struct device *dev = &bp->pdev->dev;
>  	struct macb_queue *queue;
>  	unsigned int q;
> -	int size;
> +	void *tx, *rx;
> +
> +	/*
> +	 * Upper 32-bits of Tx/Rx DMA descriptor for each queues much match!
> +	 * We cannot enforce this guarantee, the best we can do is do a single
> +	 * allocation and hope it will land into alloc_pages() that guarantees
> +	 * natural alignment of physical addresses.
> +	 */
> +
> +	tx_size_per_queue = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> +	size = bp->num_queues * tx_size_per_queue;
> +	tx = dma_alloc_coherent(dev, size, &tx_dma, GFP_KERNEL);
> +	if (!tx || upper_32_bits(tx_dma) != upper_32_bits(tx_dma + size - 1))
> +		goto out_err;
> +	netdev_dbg(bp->dev, "Allocated %zu bytes for %u TX rings at %08lx (mapped %p)\n",
> +		   size, bp->num_queues, (unsigned long)tx_dma, tx);
> +
> +	rx_size_per_queue = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> +	size = bp->num_queues * rx_size_per_queue;
> +	rx = dma_alloc_coherent(dev, size, &rx_dma, GFP_KERNEL);
> +	if (!rx || upper_32_bits(rx_dma) != upper_32_bits(rx_dma + size - 1))
> +		goto out_err;
> +	netdev_dbg(bp->dev, "Allocated %zu bytes for %u RX rings at %08lx (mapped %p)\n",
> +		   size, bp->num_queues, (unsigned long)rx_dma, rx);
>  
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> -		size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> -		queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
> -						    &queue->tx_ring_dma,
> -						    GFP_KERNEL);
> -		if (!queue->tx_ring ||
> -		    upper_32_bits(queue->tx_ring_dma) != upper_32_bits(bp->queues->tx_ring_dma))
> -			goto out_err;
> -		netdev_dbg(bp->dev,
> -			   "Allocated TX ring for queue %u of %d bytes at %08lx (mapped %p)\n",
> -			   q, size, (unsigned long)queue->tx_ring_dma,
> -			   queue->tx_ring);
> +		queue->tx_ring = tx + tx_size_per_queue * q;
> +		queue->tx_ring_dma = tx_dma + tx_size_per_queue * q;
> +
> +		queue->rx_ring = rx + rx_size_per_queue * q;
> +		queue->rx_ring_dma = rx_dma + rx_size_per_queue * q;
>  
>  		size = bp->tx_ring_size * sizeof(struct macb_tx_skb);
>  		queue->tx_skb = kmalloc(size, GFP_KERNEL);
>  		if (!queue->tx_skb)
>  			goto out_err;
> -
> -		size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> -		queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
> -						 &queue->rx_ring_dma, GFP_KERNEL);
> -		if (!queue->rx_ring ||
> -		    upper_32_bits(queue->rx_ring_dma) != upper_32_bits(bp->queues->rx_ring_dma))
> -			goto out_err;
> -		netdev_dbg(bp->dev,
> -			   "Allocated RX ring of %d bytes at %08lx (mapped %p)\n",
> -			   size, (unsigned long)queue->rx_ring_dma, queue->rx_ring);
>  	}
>  	if (bp->macbgem_ops.mog_alloc_rx_buffers(bp))
>  		goto out_err;
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

Re: [PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors

[Théo Lebrun]

Hello Sean,

Thanks for the review! I'll reply only to questions (or comments about
which I have questions).

On Tue Jul 1, 2025 at 6:32 PM CEST, Sean Anderson wrote:
> On 6/27/25 05:08, Théo Lebrun wrote:
>> Move from two (Tx/Rx) dma_alloc_coherent() for DMA descriptor rings *per
>> queue* to two dma_alloc_coherent() overall.
>> 
>> Issue is with how all queues share the same register for configuring the
>> upper 32-bits of Tx/Rx descriptor rings. For example, with Tx, notice
>> how TBQPH does *not* depend on the queue index:
>> 
>> 	#define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
>> 	#define GEM_TBQPH(hw_q)		(0x04C8)
>> 
>> 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>> 	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> 	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>> 		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
>> 	#endif
>> 
>> To maxime our chances of getting valid DMA addresses, we do a single
>
> maximize
>
>> dma_alloc_coherent() across queues.
>
> Is there really any chance involved (other than avoiding ENOMEM)?

If we land in the the page allocator codepath of dma_alloc_coherent(),
then we get natural alignment guarantees, see alloc_pages() comment [0].

[0]: https://elixir.bootlin.com/linux/v6.16/source/mm/mempolicy.c#L2499-L2502

However, we cannot be certain we land in that path. If we have an
IOMMU, then I don't think the API provides strong enough guarantees.

Same for custom `struct dma_map_ops`, be it per-device or arch-specific.
I am not aware (is anything documented on that?) of any alignment
guarantees.

Even if those give us page-aligned allocations, that isn't enough. For
example let's say we want 256KiB. We get 0xFFFF0000 from an allocator.
That is page aligned, but:

   upper_32_bits(START)      != upper_32_bits(START + SIZE - 1)
   upper_32_bits(0xFFFF0000) != upper_32_bits(0xFFFF0000 + 0x40000 - 1)
   0x0                       != 0x1

Thanks!

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

[PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Théo Lebrun]

If HW is RSC capable, it cannot add dummy bytes at the start of IP
packets. Alignment (ie number of dummy bytes) is configured using the
RBOF field inside the NCFGR register.

On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
be done if those dummy bytes are added by the hardware; notice the
skb_reserve() is done AFTER writing the address to the device.

We cannot do the skb_reserve() call BEFORE writing the address because
the address field ignores the low 2/3 bits. Conclusion: in some cases,
we risk not being able to respect the NET_IP_ALIGN value (which is
picked based on unaligned CPU access performance).

Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      |  3 +++
 drivers/net/ethernet/cadence/macb_main.c | 21 ++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index adc70b6efd52b0b11e436c2c95bb5108c40f3490..d42c81cf441ce435cad38e2dfd779b0e6a141bf3 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -523,6 +523,8 @@
 /* Bitfields in DCFG6. */
 #define GEM_PBUF_LSO_OFFSET			27
 #define GEM_PBUF_LSO_SIZE			1
+#define GEM_PBUF_RSC_OFFSET			26
+#define GEM_PBUF_RSC_SIZE			1
 #define GEM_PBUF_CUTTHRU_OFFSET			25
 #define GEM_PBUF_CUTTHRU_SIZE			1
 #define GEM_DAW64_OFFSET			23
@@ -733,6 +735,7 @@
 #define MACB_CAPS_MIIONRGMII			BIT(9)
 #define MACB_CAPS_NEED_TSUCLK			BIT(10)
 #define MACB_CAPS_QUEUE_DISABLE			BIT(11)
+#define MACB_CAPS_RSC_CAPABLE			BIT(12)
 #define MACB_CAPS_PCS				BIT(24)
 #define MACB_CAPS_HIGH_SPEED			BIT(25)
 #define MACB_CAPS_CLK_HW_CHG			BIT(26)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 48b75d95861317b9925b366446c7572c7e186628..578e72c7727d4f578478ff2b3d0a6316327271b1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1317,8 +1317,19 @@ static void gem_rx_refill(struct macb_queue *queue)
 			dma_wmb();
 			macb_set_addr(bp, desc, paddr);
 
-			/* properly align Ethernet header */
-			skb_reserve(skb, NET_IP_ALIGN);
+			/* Properly align Ethernet header.
+			 *
+			 * Hardware can add dummy bytes if asked using the RBOF
+			 * field inside the NCFGR register. That feature isn't
+			 * available if hardware is RSC capable.
+			 *
+			 * We cannot fallback to doing the 2-byte shift before
+			 * DMA mapping because the address field does not allow
+			 * setting the low 2/3 bits.
+			 * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
+			 */
+			if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
+				skb_reserve(skb, NET_IP_ALIGN);
 		} else {
 			desc->ctrl = 0;
 			dma_wmb();
@@ -2787,7 +2798,9 @@ static void macb_init_hw(struct macb *bp)
 	macb_set_hwaddr(bp);
 
 	config = macb_mdc_clk_div(bp);
-	config |= MACB_BF(RBOF, NET_IP_ALIGN);	/* Make eth data aligned */
+	/* Make eth data aligned. If RSC capable, that offset is ignored by HW. */
+	if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
+		config |= MACB_BF(RBOF, NET_IP_ALIGN);
 	config |= MACB_BIT(DRFCS);		/* Discard Rx FCS */
 	if (bp->caps & MACB_CAPS_JUMBO)
 		config |= MACB_BIT(JFRAME);	/* Enable jumbo frames */
@@ -4108,6 +4121,8 @@ static void macb_configure_caps(struct macb *bp,
 		dcfg = gem_readl(bp, DCFG2);
 		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
 			bp->caps |= MACB_CAPS_FIFO_MODE;
+		if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
+			bp->caps |= MACB_CAPS_RSC_CAPABLE;
 		if (gem_has_ptp(bp)) {
 			if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
 				dev_err(&bp->pdev->dev,

-- 
2.50.0

Re: [PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> If HW is RSC capable, it cannot add dummy bytes at the start of IP

Receive-side coalescing? Can you add a brief description of this
feature to your commit message?

> packets. Alignment (ie number of dummy bytes) is configured using the
> RBOF field inside the NCFGR register.
> 
> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
> be done if those dummy bytes are added by the hardware; notice the
> skb_reserve() is done AFTER writing the address to the device.
> 
> We cannot do the skb_reserve() call BEFORE writing the address because
> the address field ignores the low 2/3 bits. Conclusion: in some cases,
> we risk not being able to respect the NET_IP_ALIGN value (which is
> picked based on unaligned CPU access performance).
> 
> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")

Do any existing MACBs support RSC? Is this a fix? 

> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      |  3 +++
>  drivers/net/ethernet/cadence/macb_main.c | 21 ++++++++++++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index adc70b6efd52b0b11e436c2c95bb5108c40f3490..d42c81cf441ce435cad38e2dfd779b0e6a141bf3 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -523,6 +523,8 @@
>  /* Bitfields in DCFG6. */
>  #define GEM_PBUF_LSO_OFFSET			27
>  #define GEM_PBUF_LSO_SIZE			1
> +#define GEM_PBUF_RSC_OFFSET			26
> +#define GEM_PBUF_RSC_SIZE			1
>  #define GEM_PBUF_CUTTHRU_OFFSET			25
>  #define GEM_PBUF_CUTTHRU_SIZE			1
>  #define GEM_DAW64_OFFSET			23
> @@ -733,6 +735,7 @@
>  #define MACB_CAPS_MIIONRGMII			BIT(9)
>  #define MACB_CAPS_NEED_TSUCLK			BIT(10)
>  #define MACB_CAPS_QUEUE_DISABLE			BIT(11)
> +#define MACB_CAPS_RSC_CAPABLE			BIT(12)

No need to be _CAPABLE, we're already _CAPS_

>  #define MACB_CAPS_PCS				BIT(24)
>  #define MACB_CAPS_HIGH_SPEED			BIT(25)
>  #define MACB_CAPS_CLK_HW_CHG			BIT(26)
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 48b75d95861317b9925b366446c7572c7e186628..578e72c7727d4f578478ff2b3d0a6316327271b1 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1317,8 +1317,19 @@ static void gem_rx_refill(struct macb_queue *queue)
>  			dma_wmb();
>  			macb_set_addr(bp, desc, paddr);
>  
> -			/* properly align Ethernet header */
> -			skb_reserve(skb, NET_IP_ALIGN);
> +			/* Properly align Ethernet header.
> +			 *
> +			 * Hardware can add dummy bytes if asked using the RBOF
> +			 * field inside the NCFGR register. That feature isn't
> +			 * available if hardware is RSC capable.
> +			 *
> +			 * We cannot fallback to doing the 2-byte shift before
> +			 * DMA mapping because the address field does not allow
> +			 * setting the low 2/3 bits.
> +			 * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
> +			 */
> +			if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
> +				skb_reserve(skb, NET_IP_ALIGN);
>  		} else {
>  			desc->ctrl = 0;
>  			dma_wmb();
> @@ -2787,7 +2798,9 @@ static void macb_init_hw(struct macb *bp)
>  	macb_set_hwaddr(bp);
>  
>  	config = macb_mdc_clk_div(bp);
> -	config |= MACB_BF(RBOF, NET_IP_ALIGN);	/* Make eth data aligned */
> +	/* Make eth data aligned. If RSC capable, that offset is ignored by HW. */
> +	if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
> +		config |= MACB_BF(RBOF, NET_IP_ALIGN);
>  	config |= MACB_BIT(DRFCS);		/* Discard Rx FCS */
>  	if (bp->caps & MACB_CAPS_JUMBO)
>  		config |= MACB_BIT(JFRAME);	/* Enable jumbo frames */
> @@ -4108,6 +4121,8 @@ static void macb_configure_caps(struct macb *bp,
>  		dcfg = gem_readl(bp, DCFG2);
>  		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
>  			bp->caps |= MACB_CAPS_FIFO_MODE;
> +		if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
> +			bp->caps |= MACB_CAPS_RSC_CAPABLE;
>  		if (gem_has_ptp(bp)) {
>  			if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
>  				dev_err(&bp->pdev->dev,
> 

Re: [PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Théo Lebrun]

On Tue Jul 1, 2025 at 6:40 PM CEST, Sean Anderson wrote:
> On 6/27/25 05:08, Théo Lebrun wrote:
>> If HW is RSC capable, it cannot add dummy bytes at the start of IP
>
> Receive-side coalescing? Can you add a brief description of this
> feature to your commit message?

Yes that is Receive Side Coalescing. Clearly it needs to be mentioned
out loud, and briefly described.

>> packets. Alignment (ie number of dummy bytes) is configured using the
>> RBOF field inside the NCFGR register.
>> 
>> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
>> be done if those dummy bytes are added by the hardware; notice the
>> skb_reserve() is done AFTER writing the address to the device.
>> 
>> We cannot do the skb_reserve() call BEFORE writing the address because
>> the address field ignores the low 2/3 bits. Conclusion: in some cases,
>> we risk not being able to respect the NET_IP_ALIGN value (which is
>> picked based on unaligned CPU access performance).
>> 
>> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
>
> Do any existing MACBs support RSC? Is this a fix? 

I have no idea. If any MACB supports RSC, it must be those running with
NET_IP_ALIGN=0, so arm64/powerpc/x86.

Is it a fix? We can guess that all boards fall in either category:
 - Don't support RSC (=> RBOF works fine).
 - Support RSC (=> RBOF not working) AND NET_IP_ALIGN=0.

Both of those are not impacted, so we technically don't fix anything for
current users.

Regards,

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

Re: [PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Sean Anderson]

On 8/7/25 11:24, Théo Lebrun wrote:
> On Tue Jul 1, 2025 at 6:40 PM CEST, Sean Anderson wrote:
>> On 6/27/25 05:08, Théo Lebrun wrote:
>>> If HW is RSC capable, it cannot add dummy bytes at the start of IP
>>
>> Receive-side coalescing? Can you add a brief description of this
>> feature to your commit message?
> 
> Yes that is Receive Side Coalescing. Clearly it needs to be mentioned
> out loud, and briefly described.
> 
>>> packets. Alignment (ie number of dummy bytes) is configured using the
>>> RBOF field inside the NCFGR register.
>>> 
>>> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
>>> be done if those dummy bytes are added by the hardware; notice the
>>> skb_reserve() is done AFTER writing the address to the device.
>>> 
>>> We cannot do the skb_reserve() call BEFORE writing the address because
>>> the address field ignores the low 2/3 bits. Conclusion: in some cases,
>>> we risk not being able to respect the NET_IP_ALIGN value (which is
>>> picked based on unaligned CPU access performance).
>>> 
>>> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
>>
>> Do any existing MACBs support RSC? Is this a fix? 
> 
> I have no idea. If any MACB supports RSC, it must be those running with
> NET_IP_ALIGN=0, so arm64/powerpc/x86.
> 
> Is it a fix? We can guess that all boards fall in either category:
>  - Don't support RSC (=> RBOF works fine).
>  - Support RSC (=> RBOF not working) AND NET_IP_ALIGN=0.
> 
> Both of those are not impacted, so we technically don't fix anything for
> current users.

OK, then please drop the fixes tag then.

--Sean

[PATCH net-next v2 13/18] net: macb: avoid double endianness swap in macb_set_hwaddr()

[Théo Lebrun]

writel() does a CPU->LE conversion. Drop manual cpu_to_le*() calls.

On little-endian system:
 - cpu_to_le32() is a no-op (LE->LE),
 - writel() is a no-op (LE->LE),
 - dev_addr will therefore not be swapped and written as-is.

On big-endian system:
 - cpu_to_le32() is a swap (BE->LE),
 - writel() is a swap (BE->LE),
 - dev_addr will therefore be swapped twice and written as a BE value.

This was found using sparse:
   ⟩ make C=2 drivers/net/ethernet/cadence/macb_main.o
   warning: incorrect type in assignment (different base types)
      expected unsigned int [usertype] bottom
      got restricted __le32 [usertype]
   warning: incorrect type in assignment (different base types)
      expected unsigned short [usertype] top
      got restricted __le16 [usertype]
   ...

Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 578e72c7727d4f578478ff2b3d0a6316327271b1..34223dad2d01ae4bcefc0823c868a67f59435638 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -265,9 +265,9 @@ static void macb_set_hwaddr(struct macb *bp)
 	u32 bottom;
 	u16 top;
 
-	bottom = cpu_to_le32(*((u32 *)bp->dev->dev_addr));
+	bottom = *((u32 *)bp->dev->dev_addr);
 	macb_or_gem_writel(bp, SA1B, bottom);
-	top = cpu_to_le16(*((u16 *)(bp->dev->dev_addr + 4)));
+	top = *((u16 *)(bp->dev->dev_addr + 4));
 	macb_or_gem_writel(bp, SA1T, top);
 
 	if (gem_has_ptp(bp)) {

-- 
2.50.0

Re: [PATCH net-next v2 13/18] net: macb: avoid double endianness swap in macb_set_hwaddr()

[Sean Anderson]

On 6/27/25 05:08, Théo Lebrun wrote:
> writel() does a CPU->LE conversion. Drop manual cpu_to_le*() calls.
> 
> On little-endian system:
>  - cpu_to_le32() is a no-op (LE->LE),
>  - writel() is a no-op (LE->LE),
>  - dev_addr will therefore not be swapped and written as-is.
> 
> On big-endian system:
>  - cpu_to_le32() is a swap (BE->LE),
>  - writel() is a swap (BE->LE),
>  - dev_addr will therefore be swapped twice and written as a BE value.
> 
> This was found using sparse:
>    ⟩ make C=2 drivers/net/ethernet/cadence/macb_main.o
>    warning: incorrect type in assignment (different base types)
>       expected unsigned int [usertype] bottom
>       got restricted __le32 [usertype]
>    warning: incorrect type in assignment (different base types)
>       expected unsigned short [usertype] top
>       got restricted __le16 [usertype]
>    ...
> 
> Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 578e72c7727d4f578478ff2b3d0a6316327271b1..34223dad2d01ae4bcefc0823c868a67f59435638 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -265,9 +265,9 @@ static void macb_set_hwaddr(struct macb *bp)
>  	u32 bottom;
>  	u16 top;
>  
> -	bottom = cpu_to_le32(*((u32 *)bp->dev->dev_addr));
> +	bottom = *((u32 *)bp->dev->dev_addr);
>  	macb_or_gem_writel(bp, SA1B, bottom);
> -	top = cpu_to_le16(*((u16 *)(bp->dev->dev_addr + 4)));
> +	top = *((u16 *)(bp->dev->dev_addr + 4));>  	macb_or_gem_writel(bp, SA1T, top);
>  
>  	if (gem_has_ptp(bp)) {
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

[PATCH net-next v2 14/18] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)

[Théo Lebrun]

LSO is runtime-detected using the PBUF_LSO field inside register
designcfg_debug6/GEM_DCFG6. Allow disabling that feature if it is
broken by using struct macb_config->caps.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      | 1 +
 drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d42c81cf441ce435cad38e2dfd779b0e6a141bf3..e5de6549861965e2823044d81b6abc20d2b27ceb 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -736,6 +736,7 @@
 #define MACB_CAPS_NEED_TSUCLK			BIT(10)
 #define MACB_CAPS_QUEUE_DISABLE			BIT(11)
 #define MACB_CAPS_RSC_CAPABLE			BIT(12)
+#define MACB_CAPS_NO_LSO			BIT(13)
 #define MACB_CAPS_PCS				BIT(24)
 #define MACB_CAPS_HIGH_SPEED			BIT(25)
 #define MACB_CAPS_CLK_HW_CHG			BIT(26)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 34223dad2d01ae4bcefc0823c868a67f59435638..f9a3a5caebcafe3d9197a3bc7681b64734d7ac93 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4346,8 +4346,10 @@ static int macb_init(struct platform_device *pdev)
 	/* Set features */
 	dev->hw_features = NETIF_F_SG;
 
-	/* Check LSO capability */
-	if (GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
+	/* Check LSO capability; runtime detection can be overridden by a cap
+	 * flag if the hardware is known to be buggy */
+	if (!(bp->caps & MACB_CAPS_NO_LSO) &&
+	    GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
 		dev->hw_features |= MACB_NETIF_LSO;
 
 	/* Checksum offload is only available on gem with packet buffer */

-- 
2.50.0

Re: [PATCH net-next v2 14/18] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)

[Sean Anderson]

On 6/27/25 05:09, Théo Lebrun wrote:
> LSO is runtime-detected using the PBUF_LSO field inside register
> designcfg_debug6/GEM_DCFG6. Allow disabling that feature if it is
> broken by using struct macb_config->caps.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      | 1 +
>  drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index d42c81cf441ce435cad38e2dfd779b0e6a141bf3..e5de6549861965e2823044d81b6abc20d2b27ceb 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -736,6 +736,7 @@
>  #define MACB_CAPS_NEED_TSUCLK			BIT(10)
>  #define MACB_CAPS_QUEUE_DISABLE			BIT(11)
>  #define MACB_CAPS_RSC_CAPABLE			BIT(12)
> +#define MACB_CAPS_NO_LSO			BIT(13)
>  #define MACB_CAPS_PCS				BIT(24)
>  #define MACB_CAPS_HIGH_SPEED			BIT(25)
>  #define MACB_CAPS_CLK_HW_CHG			BIT(26)
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 34223dad2d01ae4bcefc0823c868a67f59435638..f9a3a5caebcafe3d9197a3bc7681b64734d7ac93 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4346,8 +4346,10 @@ static int macb_init(struct platform_device *pdev)
>  	/* Set features */
>  	dev->hw_features = NETIF_F_SG;
>  
> -	/* Check LSO capability */
> -	if (GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
> +	/* Check LSO capability; runtime detection can be overridden by a cap
> +	 * flag if the hardware is known to be buggy */
> +	if (!(bp->caps & MACB_CAPS_NO_LSO) &&
> +	    GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
>  		dev->hw_features |= MACB_NETIF_LSO;
>  
>  	/* Checksum offload is only available on gem with packet buffer */
> 

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

[PATCH net-next v2 15/18] net: macb: Add "mobileye,eyeq5-gem" compatible

[Théo Lebrun]

Add support for the two GEM instances inside Mobileye EyeQ5 SoCs, using
compatible "mobileye,eyeq5-gem". With it, add a custom init sequence
that accesses two system-controller registers.

Noteworthy: NET_IP_ALIGN=2 on MIPS but the hardware does not align and
low bits aren't configurable, so we cannot respect the requested IP
header alignment.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index f9a3a5caebcafe3d9197a3bc7681b64734d7ac93..ed394e5d1ec9b1748282f1448628d5006f3b0971 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -20,6 +20,7 @@
 #include <linux/iopoll.h>
 #include <linux/ip.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/netdevice.h>
@@ -31,6 +32,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/ptp_classify.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/tcp.h>
@@ -4957,6 +4959,72 @@ static int init_reset_optional(struct platform_device *pdev)
 	return ret;
 }
 
+#define EYEQ5_OLB_GP_TX_SWRST_DIS	BIT(0)		// Tx SW reset
+#define EYEQ5_OLB_GP_TX_M_CLKE		BIT(1)		// Tx M clock enable
+#define EYEQ5_OLB_GP_SYS_SWRST_DIS	BIT(2)		// Sys SW reset
+#define EYEQ5_OLB_GP_SYS_M_CLKE		BIT(3)		// Sys clock enable
+#define EYEQ5_OLB_GP_SGMII_MODE		BIT(4)		// SGMII mode
+#define EYEQ5_OLB_GP_RGMII_DRV		GENMASK(8, 5)	// RGMII mode
+
+#define EYEQ5_OLB_SGMII_PWR_EN		BIT(0)
+#define EYEQ5_OLB_SGMII_RST_DIS		BIT(1)
+#define EYEQ5_OLB_SGMII_PLL_EN		BIT(2)
+#define EYEQ5_OLB_SGMII_SIG_DET_SW	BIT(3)
+#define EYEQ5_OLB_SGMII_PWR_STATE	BIT(4)
+#define EYEQ5_OLB_SGMII_PLL_ACK		BIT(18)
+
+static int eyeq5_init(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct net_device *netdev = platform_get_drvdata(pdev);
+	struct macb *bp = netdev_priv(netdev);
+	struct device_node *np = dev->of_node;
+	unsigned int gp, sgmii;
+	struct regmap *regmap;
+	unsigned int args[2];
+	unsigned int reg;
+	int ret;
+
+	regmap = syscon_regmap_lookup_by_phandle_args(np, "mobileye,olb", 2, args);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	gp = args[0];
+	sgmii = args[1];
+
+	/* Forced reset */
+	regmap_write(regmap, gp, 0);
+	regmap_write(regmap, sgmii, 0);
+	usleep_range(5, 20);
+
+	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+		regmap_write(regmap, gp, EYEQ5_OLB_GP_SGMII_MODE);
+
+		reg = EYEQ5_OLB_SGMII_PWR_EN | EYEQ5_OLB_SGMII_RST_DIS |
+		      EYEQ5_OLB_SGMII_PLL_EN;
+		regmap_write(regmap, sgmii, reg);
+
+		ret = regmap_read_poll_timeout(regmap, sgmii, reg,
+					       reg & EYEQ5_OLB_SGMII_PLL_ACK,
+					       1, 100);
+		if (ret)
+			return dev_err_probe(dev, ret, "PLL timeout");
+
+		reg = EYEQ5_OLB_SGMII_PWR_STATE | EYEQ5_OLB_SGMII_SIG_DET_SW;
+		regmap_update_bits(regmap, sgmii, reg, reg);
+	}
+
+	reg = phy_interface_mode_is_rgmii(bp->phy_interface) ? 0x9 : 0x0;
+	regmap_update_bits(regmap, gp, EYEQ5_OLB_GP_RGMII_DRV,
+			   FIELD_PREP(EYEQ5_OLB_GP_RGMII_DRV, reg));
+
+	reg = EYEQ5_OLB_GP_TX_SWRST_DIS | EYEQ5_OLB_GP_TX_M_CLKE |
+	      EYEQ5_OLB_GP_SYS_SWRST_DIS | EYEQ5_OLB_GP_SYS_M_CLKE;
+	regmap_update_bits(regmap, gp, reg, reg);
+
+	return macb_init(pdev);
+}
+
 static const struct macb_usrio_config sama7g5_usrio = {
 	.mii = 0,
 	.rmii = 1,
@@ -5109,6 +5177,17 @@ static const struct macb_config versal_config = {
 	.usrio = &macb_default_usrio,
 };
 
+static const struct macb_config eyeq5_config = {
+	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
+		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_QUEUE_DISABLE |
+		MACB_CAPS_NO_LSO,
+	.dma_burst_length = 16,
+	.clk_init = macb_clk_init,
+	.init = eyeq5_init,
+	.jumbo_max_len = 10240,
+	.usrio = &macb_default_usrio,
+};
+
 static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
 	{ .compatible = "cdns,macb" },
@@ -5129,6 +5208,7 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
 	{ .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
 	{ .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
+	{ .compatible = "mobileye,eyeq5-gem", .data = &eyeq5_config },
 	{ .compatible = "xlnx,zynqmp-gem", .data = &zynqmp_config},
 	{ .compatible = "xlnx,zynq-gem", .data = &zynq_config },
 	{ .compatible = "xlnx,versal-gem", .data = &versal_config},

-- 
2.50.0

Re: [PATCH net-next v2 15/18] net: macb: Add "mobileye,eyeq5-gem" compatible

[Sean Anderson]

On 6/27/25 05:09, Théo Lebrun wrote:
> Add support for the two GEM instances inside Mobileye EyeQ5 SoCs, using
> compatible "mobileye,eyeq5-gem". With it, add a custom init sequence
> that accesses two system-controller registers.
> 
> Noteworthy: NET_IP_ALIGN=2 on MIPS but the hardware does not align and
> low bits aren't configurable, so we cannot respect the requested IP
> header alignment.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 80 ++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index f9a3a5caebcafe3d9197a3bc7681b64734d7ac93..ed394e5d1ec9b1748282f1448628d5006f3b0971 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -20,6 +20,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/ip.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/netdevice.h>
> @@ -31,6 +32,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/ptp_classify.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/tcp.h>
> @@ -4957,6 +4959,72 @@ static int init_reset_optional(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +#define EYEQ5_OLB_GP_TX_SWRST_DIS	BIT(0)		// Tx SW reset
> +#define EYEQ5_OLB_GP_TX_M_CLKE		BIT(1)		// Tx M clock enable
> +#define EYEQ5_OLB_GP_SYS_SWRST_DIS	BIT(2)		// Sys SW reset
> +#define EYEQ5_OLB_GP_SYS_M_CLKE		BIT(3)		// Sys clock enable
> +#define EYEQ5_OLB_GP_SGMII_MODE		BIT(4)		// SGMII mode
> +#define EYEQ5_OLB_GP_RGMII_DRV		GENMASK(8, 5)	// RGMII mode
> +
> +#define EYEQ5_OLB_SGMII_PWR_EN		BIT(0)
> +#define EYEQ5_OLB_SGMII_RST_DIS		BIT(1)
> +#define EYEQ5_OLB_SGMII_PLL_EN		BIT(2)
> +#define EYEQ5_OLB_SGMII_SIG_DET_SW	BIT(3)
> +#define EYEQ5_OLB_SGMII_PWR_STATE	BIT(4)
> +#define EYEQ5_OLB_SGMII_PLL_ACK		BIT(18)
> +
> +static int eyeq5_init(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct net_device *netdev = platform_get_drvdata(pdev);
> +	struct macb *bp = netdev_priv(netdev);
> +	struct device_node *np = dev->of_node;
> +	unsigned int gp, sgmii;
> +	struct regmap *regmap;
> +	unsigned int args[2];
> +	unsigned int reg;
> +	int ret;
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(np, "mobileye,olb", 2, args);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	gp = args[0];
> +	sgmii = args[1];
> +
> +	/* Forced reset */
> +	regmap_write(regmap, gp, 0);
> +	regmap_write(regmap, sgmii, 0);
> +	usleep_range(5, 20);
> +
> +	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) 

Isn't this too early? What if you have an SFP and the interface mode is initially
1000Base-X and the user plugs in an SGMII module?

{
> +		regmap_write(regmap, gp, EYEQ5_OLB_GP_SGMII_MODE);
> +
> +		reg = EYEQ5_OLB_SGMII_PWR_EN | EYEQ5_OLB_SGMII_RST_DIS |
> +		      EYEQ5_OLB_SGMII_PLL_EN;
> +		regmap_write(regmap, sgmii, reg);
> +
> +		ret = regmap_read_poll_timeout(regmap, sgmii, reg,
> +					       reg & EYEQ5_OLB_SGMII_PLL_ACK,
> +					       1, 100);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "PLL timeout");
> +
> +		reg = EYEQ5_OLB_SGMII_PWR_STATE | EYEQ5_OLB_SGMII_SIG_DET_SW;
> +		regmap_update_bits(regmap, sgmii, reg, reg);
> +	}
> +
> +	reg = phy_interface_mode_is_rgmii(bp->phy_interface) ? 0x9 : 0x0;
> +	regmap_update_bits(regmap, gp, EYEQ5_OLB_GP_RGMII_DRV,
> +			   FIELD_PREP(EYEQ5_OLB_GP_RGMII_DRV, reg));
> +
> +	reg = EYEQ5_OLB_GP_TX_SWRST_DIS | EYEQ5_OLB_GP_TX_M_CLKE |
> +	      EYEQ5_OLB_GP_SYS_SWRST_DIS | EYEQ5_OLB_GP_SYS_M_CLKE;
> +	regmap_update_bits(regmap, gp, reg, reg);
> +
> +	return macb_init(pdev);
> +}
> +
>  static const struct macb_usrio_config sama7g5_usrio = {
>  	.mii = 0,
>  	.rmii = 1,
> @@ -5109,6 +5177,17 @@ static const struct macb_config versal_config = {
>  	.usrio = &macb_default_usrio,
>  };
>  
> +static const struct macb_config eyeq5_config = {
> +	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
> +		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_QUEUE_DISABLE |
> +		MACB_CAPS_NO_LSO,
> +	.dma_burst_length = 16,
> +	.clk_init = macb_clk_init,
> +	.init = eyeq5_init,
> +	.jumbo_max_len = 10240,
> +	.usrio = &macb_default_usrio,
> +};
> +
>  static const struct of_device_id macb_dt_ids[] = {
>  	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
>  	{ .compatible = "cdns,macb" },
> @@ -5129,6 +5208,7 @@ static const struct of_device_id macb_dt_ids[] = {
>  	{ .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
>  	{ .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
>  	{ .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
> +	{ .compatible = "mobileye,eyeq5-gem", .data = &eyeq5_config },
>  	{ .compatible = "xlnx,zynqmp-gem", .data = &zynqmp_config},
>  	{ .compatible = "xlnx,zynq-gem", .data = &zynq_config },
>  	{ .compatible = "xlnx,versal-gem", .data = &versal_config},
> 

[PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support

[Théo Lebrun]

Both Cadence GEM Ethernet controllers on EyeQ5 are hardwired through CM3
IO Coherency Units (IOCU). For DMA coherent accesses, BIT(36) must be
set in DMA addresses.

Implement that in platform-specific dma_map_ops which get attached to
both instances of `cdns,eyeq5-gem` through a notifier block.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS                         |   2 +-
 arch/mips/mobileye/Kconfig          |   1 +
 arch/mips/mobileye/Makefile         |   2 +
 arch/mips/mobileye/eyeq5-iocu-dma.c | 160 ++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index bb9df569a3fff41ab40d7da5843f1e8564b47bf2..7ee68d7f8e8d0632846f59579412458e301bd8fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16789,7 +16789,7 @@ F:	Documentation/devicetree/bindings/mips/mobileye.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
-F:	arch/mips/mobileye/board-epm5.its.S
+F:	arch/mips/mobileye/
 F:	drivers/clk/clk-eyeq.c
 F:	drivers/pinctrl/pinctrl-eyeq5.c
 F:	drivers/reset/reset-eyeq.c
diff --git a/arch/mips/mobileye/Kconfig b/arch/mips/mobileye/Kconfig
index f9abb2d6e1787dbc5a173db48606ed5a02088e41..b9040f3a9b3ddc7f5addcd8e5f110cb9c775b6b1 100644
--- a/arch/mips/mobileye/Kconfig
+++ b/arch/mips/mobileye/Kconfig
@@ -9,6 +9,7 @@ choice
 
 	config MACH_EYEQ5
 		bool "Mobileye EyeQ5 SoC"
+		select ARCH_HAS_DMA_OPS
 
 	config MACH_EYEQ6H
 		bool "Mobileye EyeQ6H SoC"
diff --git a/arch/mips/mobileye/Makefile b/arch/mips/mobileye/Makefile
index 315c06b689cfbb83f9f205d1140ecf5058e2aa02..50fc7d0ae167c3fb3dc8585bcd45583c6cc3f2d2 100644
--- a/arch/mips/mobileye/Makefile
+++ b/arch/mips/mobileye/Makefile
@@ -1 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
+
+obj-$(CONFIG_MACH_EYEQ5)               += eyeq5-iocu-dma.o
diff --git a/arch/mips/mobileye/eyeq5-iocu-dma.c b/arch/mips/mobileye/eyeq5-iocu-dma.c
new file mode 100644
index 0000000000000000000000000000000000000000..71d1c35f911636db141c4467dccc405af69835ec
--- /dev/null
+++ b/arch/mips/mobileye/eyeq5-iocu-dma.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/device/bus.h>
+#include <linux/dma-direct.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-map-ops.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/gfp_types.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/mm_types.h>
+#include <linux/notifier.h>
+#include <linux/pfn.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/scatterlist.h>
+#include <linux/types.h>
+
+static void *eyeq5_iocu_alloc(struct device *dev, size_t size,
+			      dma_addr_t *dma_handle, gfp_t gfp,
+			      unsigned long attrs)
+{
+	void *p = dma_direct_alloc(dev, size, dma_handle, gfp, attrs);
+
+	*dma_handle |= BIT_ULL(36);
+	return p;
+}
+
+static void eyeq5_iocu_free(struct device *dev, size_t size,
+			    void *vaddr, dma_addr_t dma_handle,
+			    unsigned long attrs)
+{
+	dma_handle &= ~BIT_ULL(36);
+	dma_direct_free(dev, size, vaddr, dma_handle, attrs);
+}
+
+static int eyeq5_iocu_mmap(struct device *dev, struct vm_area_struct *vma,
+			   void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			   unsigned long attrs)
+{
+	unsigned long pfn = PHYS_PFN(dma_to_phys(dev, dma_addr));
+	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+	unsigned long user_count = vma_pages(vma);
+	int ret;
+
+	vma->vm_page_prot = dma_pgprot(dev, vma->vm_page_prot, attrs);
+
+	if (dma_mmap_from_dev_coherent(dev, vma, cpu_addr, size, &ret))
+		return ret;
+
+	if (vma->vm_pgoff >= count || user_count > count - vma->vm_pgoff)
+		return -ENXIO;
+
+	return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
+			       user_count << PAGE_SHIFT, vma->vm_page_prot);
+}
+
+static int eyeq5_iocu_get_sgtable(struct device *dev, struct sg_table *sgt,
+				  void *cpu_addr, dma_addr_t dma_addr, size_t size,
+				  unsigned long attrs)
+{
+	struct page *page = virt_to_page(cpu_addr);
+	int ret;
+
+	ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
+	if (!ret)
+		sg_set_page(sgt->sgl, page, PAGE_ALIGN(size), 0);
+	return ret;
+}
+
+static dma_addr_t eyeq5_iocu_map_page(struct device *dev, struct page *page,
+				      unsigned long offset, size_t size,
+				      enum dma_data_direction dir,
+				      unsigned long attrs)
+{
+	phys_addr_t phys = page_to_phys(page) + offset;
+
+	/* BIT(36) toggles routing through IOCU for DMA operations. */
+	return phys_to_dma(dev, phys) | BIT_ULL(36);
+}
+
+static void eyeq5_iocu_unmap_page(struct device *dev, dma_addr_t dma_handle,
+				  size_t size, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+}
+
+static int eyeq5_iocu_map_sg(struct device *dev, struct scatterlist *sgl,
+			     int nents, enum dma_data_direction dir,
+			     unsigned long attrs)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nents, i) {
+		sg->dma_address = eyeq5_iocu_map_page(dev, sg_page(sg),
+						      sg->offset, sg->length,
+						      dir, attrs);
+		if (sg->dma_address == DMA_MAPPING_ERROR)
+			return 0; /* No cleanup because ->unmap_page() is a no-op. */
+		sg_dma_len(sg) = sg->length;
+	}
+
+	return nents;
+}
+
+static void eyeq5_iocu_unmap_sg(struct device *dev, struct scatterlist *sgl,
+				int nents, enum dma_data_direction dir,
+				unsigned long attrs)
+{
+	/* We know page ->unmap_page() is a no-op. */
+}
+
+const struct dma_map_ops eyeq5_iocu_ops = {
+	.alloc			= eyeq5_iocu_alloc,
+	.free			= eyeq5_iocu_free,
+	.alloc_pages_op		= dma_direct_alloc_pages,
+	.free_pages		= dma_direct_free_pages,
+	.mmap			= eyeq5_iocu_mmap,
+	.get_sgtable		= eyeq5_iocu_get_sgtable,
+	.map_page		= eyeq5_iocu_map_page,
+	.unmap_page		= eyeq5_iocu_unmap_page,
+	.map_sg			= eyeq5_iocu_map_sg,
+	.unmap_sg		= eyeq5_iocu_unmap_sg,
+	.get_required_mask	= dma_direct_get_required_mask,
+};
+EXPORT_SYMBOL(eyeq5_iocu_ops);
+
+static int eyeq5_iocu_notifier(struct notifier_block *nb,
+			       unsigned long event,
+			       void *data)
+{
+	struct device *dev = data;
+
+	/*
+	 * IOCU routing is hardwired; we must use our above custom
+	 * routines for cache-coherent DMA on ethernet interfaces.
+	 */
+	if (event == BUS_NOTIFY_ADD_DEVICE &&
+	    device_is_compatible(dev, "mobileye,eyeq5-gem")) {
+		set_dma_ops(dev, &eyeq5_iocu_ops);
+		return NOTIFY_OK;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block eyeq5_iocu_nb = {
+	.notifier_call = eyeq5_iocu_notifier,
+};
+
+static int __init eyeq5_iocu_init(void)
+{
+	return bus_register_notifier(&platform_bus_type, &eyeq5_iocu_nb);
+}
+postcore_initcall(eyeq5_iocu_init);

-- 
2.50.0

Re: [PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support

[Simon Horman]

On Fri, Jun 27, 2025 at 11:09:02AM +0200, Théo Lebrun wrote:
> Both Cadence GEM Ethernet controllers on EyeQ5 are hardwired through CM3
> IO Coherency Units (IOCU). For DMA coherent accesses, BIT(36) must be
> set in DMA addresses.
> 
> Implement that in platform-specific dma_map_ops which get attached to
> both instances of `cdns,eyeq5-gem` through a notifier block.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

...

> diff --git a/arch/mips/mobileye/eyeq5-iocu-dma.c b/arch/mips/mobileye/eyeq5-iocu-dma.c

...

> +const struct dma_map_ops eyeq5_iocu_ops = {
> +	.alloc			= eyeq5_iocu_alloc,
> +	.free			= eyeq5_iocu_free,
> +	.alloc_pages_op		= dma_direct_alloc_pages,
> +	.free_pages		= dma_direct_free_pages,
> +	.mmap			= eyeq5_iocu_mmap,
> +	.get_sgtable		= eyeq5_iocu_get_sgtable,
> +	.map_page		= eyeq5_iocu_map_page,
> +	.unmap_page		= eyeq5_iocu_unmap_page,
> +	.map_sg			= eyeq5_iocu_map_sg,
> +	.unmap_sg		= eyeq5_iocu_unmap_sg,
> +	.get_required_mask	= dma_direct_get_required_mask,
> +};
> +EXPORT_SYMBOL(eyeq5_iocu_ops);

Hi Théo,

Does eyeq5_iocu_ops need to be exported?
If so it should probably be declared in a header file somewhere.
But I if not probably the EXPORT_SYMBOL line should be
dropped, and the structure made static.

Flagged by Sparse.

> +
> +static int eyeq5_iocu_notifier(struct notifier_block *nb,
> +			       unsigned long event,
> +			       void *data)
> +{
> +	struct device *dev = data;
> +
> +	/*
> +	 * IOCU routing is hardwired; we must use our above custom
> +	 * routines for cache-coherent DMA on ethernet interfaces.
> +	 */
> +	if (event == BUS_NOTIFY_ADD_DEVICE &&
> +	    device_is_compatible(dev, "mobileye,eyeq5-gem")) {
> +		set_dma_ops(dev, &eyeq5_iocu_ops);
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}

...

Re: [PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support

[Jiaxun Yang]

在2025年6月27日周五 上午10:09，Théo Lebrun写道：
> Both Cadence GEM Ethernet controllers on EyeQ5 are hardwired through CM3
> IO Coherency Units (IOCU). For DMA coherent accesses, BIT(36) must be
> set in DMA addresses.

Hi Théo,

Just quick question, it seems like this special driver is only applying a
fixed offset (1 << 36) to the DMA physical address, can we achieve that with dma-ranges
property in DeviceTree?

I belive:
```
dma-coherent;
             # Bus addr       # Phys  # Size
dma-ranges = <0x10 0x00000000 0x0 0x0 0x10 0>;
```

Will do the job.

Thanks
Jiaxun
>
> Implement that in platform-specific dma_map_ops which get attached to
> both instances of `cdns,eyeq5-gem` through a notifier block.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  MAINTAINERS                         |   2 +-
>  arch/mips/mobileye/Kconfig          |   1 +
>  arch/mips/mobileye/Makefile         |   2 +
>  arch/mips/mobileye/eyeq5-iocu-dma.c | 160 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 
> bb9df569a3fff41ab40d7da5843f1e8564b47bf2..7ee68d7f8e8d0632846f59579412458e301bd8fb 
> 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16789,7 +16789,7 @@ 
> F:	Documentation/devicetree/bindings/mips/mobileye.yaml
>  F:	Documentation/devicetree/bindings/soc/mobileye/
>  F:	arch/mips/boot/dts/mobileye/
>  F:	arch/mips/configs/eyeq5_defconfig
> -F:	arch/mips/mobileye/board-epm5.its.S
> +F:	arch/mips/mobileye/
>  F:	drivers/clk/clk-eyeq.c
>  F:	drivers/pinctrl/pinctrl-eyeq5.c
>  F:	drivers/reset/reset-eyeq.c
> diff --git a/arch/mips/mobileye/Kconfig b/arch/mips/mobileye/Kconfig
> index 
> f9abb2d6e1787dbc5a173db48606ed5a02088e41..b9040f3a9b3ddc7f5addcd8e5f110cb9c775b6b1 
> 100644
> --- a/arch/mips/mobileye/Kconfig
> +++ b/arch/mips/mobileye/Kconfig
> @@ -9,6 +9,7 @@ choice
> 
>  	config MACH_EYEQ5
>  		bool "Mobileye EyeQ5 SoC"
> +		select ARCH_HAS_DMA_OPS
> 
>  	config MACH_EYEQ6H
>  		bool "Mobileye EyeQ6H SoC"
> diff --git a/arch/mips/mobileye/Makefile b/arch/mips/mobileye/Makefile
> index 
> 315c06b689cfbb83f9f205d1140ecf5058e2aa02..50fc7d0ae167c3fb3dc8585bcd45583c6cc3f2d2 
> 100644
> --- a/arch/mips/mobileye/Makefile
> +++ b/arch/mips/mobileye/Makefile
> @@ -1 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
> +
> +obj-$(CONFIG_MACH_EYEQ5)               += eyeq5-iocu-dma.o
> diff --git a/arch/mips/mobileye/eyeq5-iocu-dma.c 
> b/arch/mips/mobileye/eyeq5-iocu-dma.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..71d1c35f911636db141c4467dccc405af69835ec
> --- /dev/null
> +++ b/arch/mips/mobileye/eyeq5-iocu-dma.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/device/bus.h>
> +#include <linux/dma-direct.h>
> +#include <linux/dma-direction.h>
> +#include <linux/dma-map-ops.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/gfp_types.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/mm_types.h>
> +#include <linux/notifier.h>
> +#include <linux/pfn.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/scatterlist.h>
> +#include <linux/types.h>
> +
> +static void *eyeq5_iocu_alloc(struct device *dev, size_t size,
> +			      dma_addr_t *dma_handle, gfp_t gfp,
> +			      unsigned long attrs)
> +{
> +	void *p = dma_direct_alloc(dev, size, dma_handle, gfp, attrs);
> +
> +	*dma_handle |= BIT_ULL(36);
> +	return p;
> +}
> +
> +static void eyeq5_iocu_free(struct device *dev, size_t size,
> +			    void *vaddr, dma_addr_t dma_handle,
> +			    unsigned long attrs)
> +{
> +	dma_handle &= ~BIT_ULL(36);
> +	dma_direct_free(dev, size, vaddr, dma_handle, attrs);
> +}
> +
> +static int eyeq5_iocu_mmap(struct device *dev, struct vm_area_struct 
> *vma,
> +			   void *cpu_addr, dma_addr_t dma_addr, size_t size,
> +			   unsigned long attrs)
> +{
> +	unsigned long pfn = PHYS_PFN(dma_to_phys(dev, dma_addr));
> +	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +	unsigned long user_count = vma_pages(vma);
> +	int ret;
> +
> +	vma->vm_page_prot = dma_pgprot(dev, vma->vm_page_prot, attrs);
> +
> +	if (dma_mmap_from_dev_coherent(dev, vma, cpu_addr, size, &ret))
> +		return ret;
> +
> +	if (vma->vm_pgoff >= count || user_count > count - vma->vm_pgoff)
> +		return -ENXIO;
> +
> +	return remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> +			       user_count << PAGE_SHIFT, vma->vm_page_prot);
> +}
> +
> +static int eyeq5_iocu_get_sgtable(struct device *dev, struct sg_table 
> *sgt,
> +				  void *cpu_addr, dma_addr_t dma_addr, size_t size,
> +				  unsigned long attrs)
> +{
> +	struct page *page = virt_to_page(cpu_addr);
> +	int ret;
> +
> +	ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> +	if (!ret)
> +		sg_set_page(sgt->sgl, page, PAGE_ALIGN(size), 0);
> +	return ret;
> +}
> +
> +static dma_addr_t eyeq5_iocu_map_page(struct device *dev, struct page 
> *page,
> +				      unsigned long offset, size_t size,
> +				      enum dma_data_direction dir,
> +				      unsigned long attrs)
> +{
> +	phys_addr_t phys = page_to_phys(page) + offset;
> +
> +	/* BIT(36) toggles routing through IOCU for DMA operations. */
> +	return phys_to_dma(dev, phys) | BIT_ULL(36);
> +}
> +
> +static void eyeq5_iocu_unmap_page(struct device *dev, dma_addr_t 
> dma_handle,
> +				  size_t size, enum dma_data_direction dir,
> +		unsigned long attrs)
> +{
> +}
> +
> +static int eyeq5_iocu_map_sg(struct device *dev, struct scatterlist 
> *sgl,
> +			     int nents, enum dma_data_direction dir,
> +			     unsigned long attrs)
> +{
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sg(sgl, sg, nents, i) {
> +		sg->dma_address = eyeq5_iocu_map_page(dev, sg_page(sg),
> +						      sg->offset, sg->length,
> +						      dir, attrs);
> +		if (sg->dma_address == DMA_MAPPING_ERROR)
> +			return 0; /* No cleanup because ->unmap_page() is a no-op. */
> +		sg_dma_len(sg) = sg->length;
> +	}
> +
> +	return nents;
> +}
> +
> +static void eyeq5_iocu_unmap_sg(struct device *dev, struct scatterlist 
> *sgl,
> +				int nents, enum dma_data_direction dir,
> +				unsigned long attrs)
> +{
> +	/* We know page ->unmap_page() is a no-op. */
> +}
> +
> +const struct dma_map_ops eyeq5_iocu_ops = {
> +	.alloc			= eyeq5_iocu_alloc,
> +	.free			= eyeq5_iocu_free,
> +	.alloc_pages_op		= dma_direct_alloc_pages,
> +	.free_pages		= dma_direct_free_pages,
> +	.mmap			= eyeq5_iocu_mmap,
> +	.get_sgtable		= eyeq5_iocu_get_sgtable,
> +	.map_page		= eyeq5_iocu_map_page,
> +	.unmap_page		= eyeq5_iocu_unmap_page,
> +	.map_sg			= eyeq5_iocu_map_sg,
> +	.unmap_sg		= eyeq5_iocu_unmap_sg,
> +	.get_required_mask	= dma_direct_get_required_mask,
> +};
> +EXPORT_SYMBOL(eyeq5_iocu_ops);
> +
> +static int eyeq5_iocu_notifier(struct notifier_block *nb,
> +			       unsigned long event,
> +			       void *data)
> +{
> +	struct device *dev = data;
> +
> +	/*
> +	 * IOCU routing is hardwired; we must use our above custom
> +	 * routines for cache-coherent DMA on ethernet interfaces.
> +	 */
> +	if (event == BUS_NOTIFY_ADD_DEVICE &&
> +	    device_is_compatible(dev, "mobileye,eyeq5-gem")) {
> +		set_dma_ops(dev, &eyeq5_iocu_ops);
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block eyeq5_iocu_nb = {
> +	.notifier_call = eyeq5_iocu_notifier,
> +};
> +
> +static int __init eyeq5_iocu_init(void)
> +{
> +	return bus_register_notifier(&platform_bus_type, &eyeq5_iocu_nb);
> +}
> +postcore_initcall(eyeq5_iocu_init);
>
> -- 
> 2.50.0

-- 
- Jiaxun

Re: [PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support

[Théo Lebrun]

Hello Jiaxun,

On Mon Jun 30, 2025 at 3:35 PM CEST, Jiaxun Yang wrote:
> 在2025年6月27日周五 上午10:09，Théo Lebrun写道：
>> Both Cadence GEM Ethernet controllers on EyeQ5 are hardwired through CM3
>> IO Coherency Units (IOCU). For DMA coherent accesses, BIT(36) must be
>> set in DMA addresses.
>
> Just quick question, it seems like this special driver is only applying a
> fixed offset (1 << 36) to the DMA physical address, can we achieve that with dma-ranges
> property in DeviceTree?
>
> I belive:
> ```
> dma-coherent;
>              # Bus addr       # Phys  # Size
> dma-ranges = <0x10 0x00000000 0x0 0x0 0x10 0>;
> ```
>
> Will do the job.

This is perfect! Can confirm it works just fine. When you are stuck in
an issue for too long you don't think about broadening your viewpoint.

Thanks,

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

[PATCH net-next v2 17/18] MIPS: mobileye: eyeq5: add two Cadence GEM Ethernet controllers

[Théo Lebrun]

Add both MACB/GEM instances found in the Mobileye EyeQ5 SoC.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index a84e6e720619ef99e1405ae6296d8bad1aa3fa23..420cb27607bfdd8d5ea510fb668b0a1c85dd7d83 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -77,6 +77,8 @@ aliases {
 		serial0 = &uart0;
 		serial1 = &uart1;
 		serial2 = &uart2;
+		ethernet0 = &macb0;
+		ethernet1 = &macb1;
 	};
 
 	cpu_intc: interrupt-controller {
@@ -178,6 +180,38 @@ timer {
 				clocks = <&olb EQ5C_CPU_CORE0>;
 			};
 		};
+
+		macb0: ethernet@2a00000 {
+			compatible = "mobileye,eyeq5-gem";
+			reg = <0x0 0x02a00000 0x0 0x4000>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>, /* queue0 */
+				     <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>, /* queue1 */
+				     <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>, /* queue2 */
+				     <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>; /* queue3 */
+			clock-names = "pclk", "hclk", "tsu_clk";
+			clocks = <&pclk>, <&pclk>, <&tsu_clk>;
+			dma-coherent;
+			nvmem-cells = <&eth0_mac>;
+			nvmem-cell-names = "mac-address";
+			mobileye,olb = <&olb 0x128 0x134>;
+		};
+
+		macb1: ethernet@2b00000 {
+			compatible = "mobileye,eyeq5-gem";
+			reg = <0x0 0x02b00000 0x0 0x4000>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>, /* queue0 */
+				     <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>, /* queue1 */
+				     <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>, /* queue2 */
+				     <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>; /* queue3 */
+			clock-names = "pclk", "hclk", "tsu_clk";
+			clocks = <&pclk>, <&pclk>, <&tsu_clk>;
+			dma-coherent;
+			nvmem-cells = <&eth1_mac>;
+			nvmem-cell-names = "mac-address";
+			mobileye,olb = <&olb 0x12c 0x138>;
+		};
 	};
 };
 

-- 
2.50.0

[PATCH net-next v2 18/18] MIPS: mobileye: eyeq5-epm: add two Cadence GEM Ethernet PHYs

[Théo Lebrun]

The Mobileye EyeQ5 eval board (EPM) embeds two MDIO PHYs.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5-epm5.dts | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5-epm5.dts b/arch/mips/boot/dts/mobileye/eyeq5-epm5.dts
index 6898b2d8267dfadeea511a84d1df3f70744f17bb..3d8af5b4675b24c2fa284a52e537a4366226acc2 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5-epm5.dts
+++ b/arch/mips/boot/dts/mobileye/eyeq5-epm5.dts
@@ -21,3 +21,29 @@ memory@0 {
 		      <0x8 0x02000000 0x0 0x7E000000>;
 	};
 };
+
+&macb0 {
+	phy-mode = "sgmii";
+	phy-handle = <&macb0_phy>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		macb0_phy: ethernet-phy@e {
+			reg = <0xe>;
+		};
+	};
+};
+
+&macb1 {
+	phy-mode = "rgmii-id";
+	phy-handle = <&macb1_phy>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		macb1_phy: ethernet-phy@e {
+			reg = <0xe>;
+		};
+	};
+};

-- 
2.50.0

Re: [PATCH net-next v2 00/18] Support the Cadence MACB/GEM instances on Mobileye EyeQ5 SoCs

[Maxime Chevallier]

Hi Théo,

On Fri, 27 Jun 2025 11:08:46 +0200
Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Mobileye EyeQ5 SoCs provides two GEM IP blocks. The end result of this
> series is working 1G networking on the EyeQ5 eval board. It isn't just
> a new macb_config & compatible, we also provide some cleanup & fixes
> along the way.
> 
> dt-bindings:
>   [PATCH net-next v2 01/18] dt-bindings: net: cdns,macb: sort compatibles
>   [PATCH net-next v2 02/18] dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface
>   [PATCH net-next v2 03/18] dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk
>   [PATCH net-next v2 04/18] dt-bindings: net: cdns,macb: allow dma-coherent
> 
> Cleanup:
>   [PATCH net-next v2 05/18] net: macb: use BIT() macro for capability definitions
>   [PATCH net-next v2 06/18] net: macb: Remove local variables clk_init and init in macb_probe()
>   [PATCH net-next v2 07/18] net: macb: drop macb_config NULL checking
>   [PATCH net-next v2 08/18] net: macb: introduce DMA descriptor helpers (is 64bit? is PTP?)
>   [PATCH net-next v2 09/18] net: macb: sort #includes
> 
> Fixes:
>   [PATCH net-next v2 10/18] net: macb: remove illusion about TBQPH/RBQPH being per-queue
>   [PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors
>   [PATCH net-next v2 12/18] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
>   [PATCH net-next v2 13/18] net: macb: avoid double endianness swap in macb_set_hwaddr()

Fixes go through the net tree, and new features through net-next :

https://docs.kernel.org/process/maintainer-netdev.html#netdev-faq

As this series is quite big, it could be nice to send the cleanups and
fixes in dedicated series, and then the eyeQ5 support in itself as a
standalone series. This would make the review work easier.


> EyeQ5 support:
>   [PATCH net-next v2 14/18] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
>   [PATCH net-next v2 15/18] net: macb: Add "mobileye,eyeq5-gem" compatible
> 
> DTS:
>   [PATCH net-next v2 16/18] MIPS: mobileye: add EyeQ5 DMA IOCU support
>   [PATCH net-next v2 17/18] MIPS: mobileye: eyeq5: add two Cadence GEM Ethernet controllers
>   [PATCH net-next v2 18/18] MIPS: mobileye: eyeq5-epm: add two Cadence GEM Ethernet PHYs

Maxime

Re: [PATCH net-next v2 00/18] Support the Cadence MACB/GEM instances on Mobileye EyeQ5 SoCs

[Sean Anderson]

Hi Theo,

Please remember to take the following recipients off CC for next patch:

<cyrille.pitchen@atmel.com>: host mx1.microchip.iphmx.com[68.232.147.91] said:
    550 #5.1.0 Address rejected. (in reply to RCPT TO command)

<rafalo@cadence.com>: host mxa-0014ca01.gslb.pphosted.com[208.86.201.193] said:
    550 5.1.1 User Unknown (in reply to DATA command)

<hskinnemoen@atmel.com>: host mx1.microchip.iphmx.com[68.232.147.91] said: 550
    #5.1.0 Address rejected. (in reply to RCPT TO command)

Thanks.

--Sean