[PATCH net-next v3 0/5] net: macb: EyeQ5 support

[PATCH net-next v3 0/5] net: macb: EyeQ5 support

[Théo Lebrun]

This series' goal is adding support to the MACB driver for EyeQ5 GEM.
The specifics for this compatible are:

 - HW cannot add dummy bytes at the start of IP packets for alignment
   purposes. The behavior can be detected using DCFG6 so it isn't
   attached to compatible data.

 - The hardware LSO/TSO is known to be buggy: add a compatible
   capability flag to force disable it.

 - At init, we have to wiggle two syscon registers that configure the
   PHY integration.

   In past attempts [0] we did it in macb_config->init() using a syscon
   regmap. That was far from ideal so now a generic PHY driver
   abstracts that away. We reuse the bp->sgmii_phy field used by some
   compatibles.

   We have to add a phy_set_mode() call as the PHY power on sequence
   depends on whether we do RGMII or SGMII.

Thanks,
Have a nice day,
Théo

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

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v3:
- Drop Fixes: trailer on [2/5]. We don't fix any platform using the
  driver currently.
- Improve [5/5] commit message; add info about how an unconditional
  phy_set_mode_ext() won't break existing platforms.
- Hardbreak 82 characters line in [2/5]; warning by patchwork.
- Trailers:
  - 1x Acked-by: Conor Dooley on [1/5].
  - 2x Reviewed-by: Andrew Lunn on [1/5] and [4/5].
  - 2x Reviewed-by: Maxime Chevallier on [4/5] and [5/5].
- Link to v2: https://lore.kernel.org/r/20251022-macb-eyeq5-v2-0-7c140abb0581@bootlin.com

Changes in v2:
- Drop non net-next patches.
- Re-run get_maintainers.pl to shorten the To/Cc list.
- Rebase upon latest net-next; no changes. Tested on HW.
- Link to v1: https://lore.kernel.org/r/20251021-macb-eyeq5-v1-0-3b0b5a9d2f85@bootlin.com

Past versions of the MACB EyeQ5 patches:
 - March 2025: [PATCH net-next 00/13] Support the Cadence MACB/GEM
   instances on Mobileye EyeQ5 SoCs
   https://lore.kernel.org/lkml/20250321-macb-v1-0-537b7e37971d@bootlin.com/
 - June 2025: [PATCH net-next v2 00/18] Support the Cadence MACB/GEM
   instances on Mobileye EyeQ5 SoCs
   https://lore.kernel.org/lkml/20250627-macb-v2-0-ff8207d0bb77@bootlin.com/
 - August 2025: [PATCH net v3 00/16] net: macb: various fixes & cleanup
   https://lore.kernel.org/lkml/20250808-macb-fixes-v3-0-08f1fcb5179f@bootlin.com/

---
Théo Lebrun (5):
      dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface
      net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
      net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
      net: macb: rename bp->sgmii_phy field to bp->phy
      net: macb: Add "mobileye,eyeq5-gem" compatible

 .../devicetree/bindings/net/cdns,macb.yaml         | 10 +++
 drivers/net/ethernet/cadence/macb.h                |  6 +-
 drivers/net/ethernet/cadence/macb_main.c           | 94 +++++++++++++++++-----
 3 files changed, 91 insertions(+), 19 deletions(-)
---
base-commit: 61b7ade9ba8c3b16867e25411b5f7cf1abe35879
change-id: 20251020-macb-eyeq5-fe2c0d1edc75

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

[PATCH net-next v3 1/5] 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. It is different from other compatibles in two main
ways: (1) it requires a generic PHY and (2) it is better to keep TCP
Segmentation Offload (TSO) disabled.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/net/cdns,macb.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index 02f14a0b72f9..ea8337846ab2 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
           - raspberrypi,rp1-gem       # Raspberry Pi RP1 gigabit ethernet interface
           - sifive,fu540-c000-gem     # SiFive FU540-C000 SoC
 
@@ -183,6 +184,15 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq5-gem
+    then:
+      required:
+        - phys
+
 unevaluatedProperties: false
 
 examples:

-- 
2.51.1

[PATCH net-next v3 2/5] 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).

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

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5b7d4cdb204d..93e8dd092313 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -537,6 +537,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
@@ -775,6 +777,7 @@
 #define MACB_CAPS_MACB_IS_GEM			BIT(20)
 #define MACB_CAPS_DMA_64B			BIT(21)
 #define MACB_CAPS_DMA_PTP			BIT(22)
+#define MACB_CAPS_RSC				BIT(23)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 39673f5c3337..be3d0c2313a1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1300,8 +1300,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))
+				skb_reserve(skb, NET_IP_ALIGN);
 		} else {
 			desc->ctrl = 0;
 			dma_wmb();
@@ -2773,7 +2784,11 @@ 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))
+		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 */
@@ -4321,6 +4336,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;
 		if (gem_has_ptp(bp)) {
 			if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
 				dev_err(&bp->pdev->dev,

-- 
2.51.1

Re: [PATCH net-next v3 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Andrew Lunn]

On Thu, Oct 23, 2025 at 06:22:52PM +0200, Théo Lebrun wrote:
> 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).
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

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

    Andrew

Re: [PATCH net-next v3 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

[Nicolas Ferre]

On 23/10/2025 at 18:22, Théo Lebrun wrote:
> 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).
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

For the record:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Théo! Regards,
   Nicolas

> ---
>   drivers/net/ethernet/cadence/macb.h      |  3 +++
>   drivers/net/ethernet/cadence/macb_main.c | 23 ++++++++++++++++++++---
>   2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 5b7d4cdb204d..93e8dd092313 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -537,6 +537,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
> @@ -775,6 +777,7 @@
>   #define MACB_CAPS_MACB_IS_GEM                  BIT(20)
>   #define MACB_CAPS_DMA_64B                      BIT(21)
>   #define MACB_CAPS_DMA_PTP                      BIT(22)
> +#define MACB_CAPS_RSC                          BIT(23)
> 
>   /* LSO settings */
>   #define MACB_LSO_UFO_ENABLE                    0x01
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 39673f5c3337..be3d0c2313a1 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1300,8 +1300,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))
> +                               skb_reserve(skb, NET_IP_ALIGN);
>                  } else {
>                          desc->ctrl = 0;
>                          dma_wmb();
> @@ -2773,7 +2784,11 @@ 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))
> +               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 */
> @@ -4321,6 +4336,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;
>                  if (gem_has_ptp(bp)) {
>                          if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
>                                  dev_err(&bp->pdev->dev,
> 
> --
> 2.51.1
> 

[PATCH net-next v3 3/5] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)

[Théo Lebrun]

LSO is runtime-detected using the PBUF_LSO field inside register DCFG6.
Allow disabling that feature if it is broken by using bp->caps coming
from match data.

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 | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 93e8dd092313..05bfa9bd4782 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -778,6 +778,7 @@
 #define MACB_CAPS_DMA_64B			BIT(21)
 #define MACB_CAPS_DMA_PTP			BIT(22)
 #define MACB_CAPS_RSC				BIT(23)
+#define MACB_CAPS_NO_LSO			BIT(24)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index be3d0c2313a1..8b688a6cb2f9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4564,8 +4564,11 @@ 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.51.1

Re: [PATCH net-next v3 3/5] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)

[Nicolas Ferre]

On 23/10/2025 at 18:22, Théo Lebrun wrote:
> LSO is runtime-detected using the PBUF_LSO field inside register DCFG6.
> Allow disabling that feature if it is broken by using bp->caps coming
> from match data.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Even if already applied:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/macb.h      | 1 +
>   drivers/net/ethernet/cadence/macb_main.c | 7 +++++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 93e8dd092313..05bfa9bd4782 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -778,6 +778,7 @@
>   #define MACB_CAPS_DMA_64B                      BIT(21)
>   #define MACB_CAPS_DMA_PTP                      BIT(22)
>   #define MACB_CAPS_RSC                          BIT(23)
> +#define MACB_CAPS_NO_LSO                       BIT(24)
> 
>   /* LSO settings */
>   #define MACB_LSO_UFO_ENABLE                    0x01
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index be3d0c2313a1..8b688a6cb2f9 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4564,8 +4564,11 @@ 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.51.1
> 

[PATCH net-next v3 4/5] net: macb: rename bp->sgmii_phy field to bp->phy

[Théo Lebrun]

The bp->sgmii_phy field is initialised at probe by init_reset_optional()
if bp->phy_interface == PHY_INTERFACE_MODE_SGMII. It gets used by:
 - zynqmp_config: "cdns,zynqmp-gem" or "xlnx,zynqmp-gem" compatibles.
 - mpfs_config: "microchip,mpfs-macb" compatible.
 - versal_config: "xlnx,versal-gem" compatible.

Make name more generic as EyeQ5 requires the PHY in SGMII & RGMII cases.

Drop "for ZynqMP SGMII mode" comment that is already a lie, as it gets
used on Microchip platforms as well. And soon it won't be SGMII-only.

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

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 05bfa9bd4782..87414a2ddf6e 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1341,7 +1341,7 @@ struct macb {
 
 	struct macb_ptp_info	*ptp_info;	/* macb-ptp interface */
 
-	struct phy		*sgmii_phy;	/* for ZynqMP SGMII mode */
+	struct phy		*phy;
 
 	spinlock_t tsu_clk_lock; /* gem tsu clock locking */
 	unsigned int tsu_rate;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 8b688a6cb2f9..44188e7eee56 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2965,7 +2965,7 @@ static int macb_open(struct net_device *dev)
 
 	macb_init_hw(bp);
 
-	err = phy_power_on(bp->sgmii_phy);
+	err = phy_power_on(bp->phy);
 	if (err)
 		goto reset_hw;
 
@@ -2981,7 +2981,7 @@ static int macb_open(struct net_device *dev)
 	return 0;
 
 phy_off:
-	phy_power_off(bp->sgmii_phy);
+	phy_power_off(bp->phy);
 
 reset_hw:
 	macb_reset_hw(bp);
@@ -3013,7 +3013,7 @@ static int macb_close(struct net_device *dev)
 	phylink_stop(bp->phylink);
 	phylink_disconnect_phy(bp->phylink);
 
-	phy_power_off(bp->sgmii_phy);
+	phy_power_off(bp->phy);
 
 	spin_lock_irqsave(&bp->lock, flags);
 	macb_reset_hw(bp);
@@ -5141,13 +5141,13 @@ static int init_reset_optional(struct platform_device *pdev)
 
 	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
 		/* Ensure PHY device used in SGMII mode is ready */
-		bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
+		bp->phy = devm_phy_optional_get(&pdev->dev, NULL);
 
-		if (IS_ERR(bp->sgmii_phy))
-			return dev_err_probe(&pdev->dev, PTR_ERR(bp->sgmii_phy),
+		if (IS_ERR(bp->phy))
+			return dev_err_probe(&pdev->dev, PTR_ERR(bp->phy),
 					     "failed to get SGMII PHY\n");
 
-		ret = phy_init(bp->sgmii_phy);
+		ret = phy_init(bp->phy);
 		if (ret)
 			return dev_err_probe(&pdev->dev, ret,
 					     "failed to init SGMII PHY\n");
@@ -5176,7 +5176,7 @@ static int init_reset_optional(struct platform_device *pdev)
 	/* Fully reset controller at hardware level if mapped in device tree */
 	ret = device_reset_optional(&pdev->dev);
 	if (ret) {
-		phy_exit(bp->sgmii_phy);
+		phy_exit(bp->phy);
 		return dev_err_probe(&pdev->dev, ret, "failed to reset controller");
 	}
 
@@ -5184,7 +5184,7 @@ static int init_reset_optional(struct platform_device *pdev)
 
 err_out_phy_exit:
 	if (ret)
-		phy_exit(bp->sgmii_phy);
+		phy_exit(bp->phy);
 
 	return ret;
 }
@@ -5594,7 +5594,7 @@ static int macb_probe(struct platform_device *pdev)
 	mdiobus_free(bp->mii_bus);
 
 err_out_phy_exit:
-	phy_exit(bp->sgmii_phy);
+	phy_exit(bp->phy);
 
 err_out_free_netdev:
 	free_netdev(dev);
@@ -5618,7 +5618,7 @@ static void macb_remove(struct platform_device *pdev)
 	if (dev) {
 		bp = netdev_priv(dev);
 		unregister_netdev(dev);
-		phy_exit(bp->sgmii_phy);
+		phy_exit(bp->phy);
 		mdiobus_unregister(bp->mii_bus);
 		mdiobus_free(bp->mii_bus);
 
@@ -5645,7 +5645,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
 	u32 tmp;
 
 	if (!device_may_wakeup(&bp->dev->dev))
-		phy_exit(bp->sgmii_phy);
+		phy_exit(bp->phy);
 
 	if (!netif_running(netdev))
 		return 0;
@@ -5774,7 +5774,7 @@ static int __maybe_unused macb_resume(struct device *dev)
 	int err;
 
 	if (!device_may_wakeup(&bp->dev->dev))
-		phy_init(bp->sgmii_phy);
+		phy_init(bp->phy);
 
 	if (!netif_running(netdev))
 		return 0;

-- 
2.51.1

Re: [PATCH net-next v3 4/5] net: macb: rename bp->sgmii_phy field to bp->phy

[Nicolas Ferre]

On 23/10/2025 at 18:22, Théo Lebrun wrote:
> The bp->sgmii_phy field is initialised at probe by init_reset_optional()
> if bp->phy_interface == PHY_INTERFACE_MODE_SGMII. It gets used by:
>   - zynqmp_config: "cdns,zynqmp-gem" or "xlnx,zynqmp-gem" compatibles.
>   - mpfs_config: "microchip,mpfs-macb" compatible.
>   - versal_config: "xlnx,versal-gem" compatible.
> 
> Make name more generic as EyeQ5 requires the PHY in SGMII & RGMII cases.
> 
> Drop "for ZynqMP SGMII mode" comment that is already a lie, as it gets
> used on Microchip platforms as well. And soon it won't be SGMII-only.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/macb.h      |  2 +-
>   drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++-------------
>   2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 05bfa9bd4782..87414a2ddf6e 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1341,7 +1341,7 @@ struct macb {
> 
>          struct macb_ptp_info    *ptp_info;      /* macb-ptp interface */
> 
> -       struct phy              *sgmii_phy;     /* for ZynqMP SGMII mode */
> +       struct phy              *phy;
> 
>          spinlock_t tsu_clk_lock; /* gem tsu clock locking */
>          unsigned int tsu_rate;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 8b688a6cb2f9..44188e7eee56 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2965,7 +2965,7 @@ static int macb_open(struct net_device *dev)
> 
>          macb_init_hw(bp);
> 
> -       err = phy_power_on(bp->sgmii_phy);
> +       err = phy_power_on(bp->phy);
>          if (err)
>                  goto reset_hw;
> 
> @@ -2981,7 +2981,7 @@ static int macb_open(struct net_device *dev)
>          return 0;
> 
>   phy_off:
> -       phy_power_off(bp->sgmii_phy);
> +       phy_power_off(bp->phy);
> 
>   reset_hw:
>          macb_reset_hw(bp);
> @@ -3013,7 +3013,7 @@ static int macb_close(struct net_device *dev)
>          phylink_stop(bp->phylink);
>          phylink_disconnect_phy(bp->phylink);
> 
> -       phy_power_off(bp->sgmii_phy);
> +       phy_power_off(bp->phy);
> 
>          spin_lock_irqsave(&bp->lock, flags);
>          macb_reset_hw(bp);
> @@ -5141,13 +5141,13 @@ static int init_reset_optional(struct platform_device *pdev)
> 
>          if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>                  /* Ensure PHY device used in SGMII mode is ready */
> -               bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
> +               bp->phy = devm_phy_optional_get(&pdev->dev, NULL);
> 
> -               if (IS_ERR(bp->sgmii_phy))
> -                       return dev_err_probe(&pdev->dev, PTR_ERR(bp->sgmii_phy),
> +               if (IS_ERR(bp->phy))
> +                       return dev_err_probe(&pdev->dev, PTR_ERR(bp->phy),
>                                               "failed to get SGMII PHY\n");
> 
> -               ret = phy_init(bp->sgmii_phy);
> +               ret = phy_init(bp->phy);
>                  if (ret)
>                          return dev_err_probe(&pdev->dev, ret,
>                                               "failed to init SGMII PHY\n");
> @@ -5176,7 +5176,7 @@ static int init_reset_optional(struct platform_device *pdev)
>          /* Fully reset controller at hardware level if mapped in device tree */
>          ret = device_reset_optional(&pdev->dev);
>          if (ret) {
> -               phy_exit(bp->sgmii_phy);
> +               phy_exit(bp->phy);
>                  return dev_err_probe(&pdev->dev, ret, "failed to reset controller");
>          }
> 
> @@ -5184,7 +5184,7 @@ static int init_reset_optional(struct platform_device *pdev)
> 
>   err_out_phy_exit:
>          if (ret)
> -               phy_exit(bp->sgmii_phy);
> +               phy_exit(bp->phy);
> 
>          return ret;
>   }
> @@ -5594,7 +5594,7 @@ static int macb_probe(struct platform_device *pdev)
>          mdiobus_free(bp->mii_bus);
> 
>   err_out_phy_exit:
> -       phy_exit(bp->sgmii_phy);
> +       phy_exit(bp->phy);
> 
>   err_out_free_netdev:
>          free_netdev(dev);
> @@ -5618,7 +5618,7 @@ static void macb_remove(struct platform_device *pdev)
>          if (dev) {
>                  bp = netdev_priv(dev);
>                  unregister_netdev(dev);
> -               phy_exit(bp->sgmii_phy);
> +               phy_exit(bp->phy);
>                  mdiobus_unregister(bp->mii_bus);
>                  mdiobus_free(bp->mii_bus);
> 
> @@ -5645,7 +5645,7 @@ static int __maybe_unused macb_suspend(struct device *dev)
>          u32 tmp;
> 
>          if (!device_may_wakeup(&bp->dev->dev))
> -               phy_exit(bp->sgmii_phy);
> +               phy_exit(bp->phy);
> 
>          if (!netif_running(netdev))
>                  return 0;
> @@ -5774,7 +5774,7 @@ static int __maybe_unused macb_resume(struct device *dev)
>          int err;
> 
>          if (!device_may_wakeup(&bp->dev->dev))
> -               phy_init(bp->sgmii_phy);
> +               phy_init(bp->phy);
> 
>          if (!netif_running(netdev))
>                  return 0;
> 
> --
> 2.51.1
> 

[PATCH net-next v3 5/5] 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 must grab a generic PHY and initialise it.

We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
the first users of bp->phy that use it in non-SGMII cases.

The phy_set_mode_ext() call is made unconditionally. It cannot cause
issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
those cases, the call is a no-op (returning zero). From reading
upstream DTS, we can figure out that no platform has a bp->phy and a
PHY driver that has a .set_mode() implementation:
 - cdns,zynqmp-gem: no DTS upstream.
 - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
 - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
 - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
   drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 44188e7eee56..b1ed98d9c438 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2965,6 +2965,10 @@ static int macb_open(struct net_device *dev)
 
 	macb_init_hw(bp);
 
+	err = phy_set_mode_ext(bp->phy, PHY_MODE_ETHERNET, bp->phy_interface);
+	if (err)
+		goto reset_hw;
+
 	err = phy_power_on(bp->phy);
 	if (err)
 		goto reset_hw;
@@ -5189,6 +5193,28 @@ static int init_reset_optional(struct platform_device *pdev)
 	return ret;
 }
 
+static int eyeq5_init(struct platform_device *pdev)
+{
+	struct net_device *netdev = platform_get_drvdata(pdev);
+	struct macb *bp = netdev_priv(netdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	bp->phy = devm_phy_get(dev, NULL);
+	if (IS_ERR(bp->phy))
+		return dev_err_probe(dev, PTR_ERR(bp->phy),
+				     "failed to get PHY\n");
+
+	ret = phy_init(bp->phy);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to init PHY\n");
+
+	ret = macb_init(pdev);
+	if (ret)
+		phy_exit(bp->phy);
+	return ret;
+}
+
 static const struct macb_usrio_config sama7g5_usrio = {
 	.mii = 0,
 	.rmii = 1,
@@ -5343,6 +5369,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 macb_config raspberrypi_rp1_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
 		MACB_CAPS_JUMBO |
@@ -5374,6 +5411,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 = "raspberrypi,rp1-gem", .data = &raspberrypi_rp1_config },
 	{ .compatible = "xlnx,zynqmp-gem", .data = &zynqmp_config},
 	{ .compatible = "xlnx,zynq-gem", .data = &zynq_config },

-- 
2.51.1

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Andrew Lunn]

On Thu, Oct 23, 2025 at 06:22:55PM +0200, 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 must grab a generic PHY and initialise it.
> 
> We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> the first users of bp->phy that use it in non-SGMII cases.
> 
> The phy_set_mode_ext() call is made unconditionally. It cannot cause
> issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> those cases, the call is a no-op (returning zero). From reading
> upstream DTS, we can figure out that no platform has a bp->phy and a
> PHY driver that has a .set_mode() implementation:
>  - cdns,zynqmp-gem: no DTS upstream.
>  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
>  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
>  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
>    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> 
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

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

    Andrew

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Nicolas Ferre]

On 23/10/2025 at 18:22, 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 must grab a generic PHY and initialise it.
> 
> We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> the first users of bp->phy that use it in non-SGMII cases.
> 
> The phy_set_mode_ext() call is made unconditionally. It cannot cause
> issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> those cases, the call is a no-op (returning zero). From reading
> upstream DTS, we can figure out that no platform has a bp->phy and a
> PHY driver that has a .set_mode() implementation:
>   - cdns,zynqmp-gem: no DTS upstream.
>   - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
>   - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
>   - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
>     drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> 
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Looks good to me, thanks:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
   Nicolas

> ---
>   drivers/net/ethernet/cadence/macb_main.c | 38 ++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 44188e7eee56..b1ed98d9c438 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2965,6 +2965,10 @@ static int macb_open(struct net_device *dev)
> 
>          macb_init_hw(bp);
> 
> +       err = phy_set_mode_ext(bp->phy, PHY_MODE_ETHERNET, bp->phy_interface);
> +       if (err)
> +               goto reset_hw;
> +
>          err = phy_power_on(bp->phy);
>          if (err)
>                  goto reset_hw;
> @@ -5189,6 +5193,28 @@ static int init_reset_optional(struct platform_device *pdev)
>          return ret;
>   }
> 
> +static int eyeq5_init(struct platform_device *pdev)
> +{
> +       struct net_device *netdev = platform_get_drvdata(pdev);
> +       struct macb *bp = netdev_priv(netdev);
> +       struct device *dev = &pdev->dev;
> +       int ret;
> +
> +       bp->phy = devm_phy_get(dev, NULL);
> +       if (IS_ERR(bp->phy))
> +               return dev_err_probe(dev, PTR_ERR(bp->phy),
> +                                    "failed to get PHY\n");
> +
> +       ret = phy_init(bp->phy);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to init PHY\n");
> +
> +       ret = macb_init(pdev);
> +       if (ret)
> +               phy_exit(bp->phy);
> +       return ret;
> +}
> +
>   static const struct macb_usrio_config sama7g5_usrio = {
>          .mii = 0,
>          .rmii = 1,
> @@ -5343,6 +5369,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 macb_config raspberrypi_rp1_config = {
>          .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
>                  MACB_CAPS_JUMBO |
> @@ -5374,6 +5411,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 = "raspberrypi,rp1-gem", .data = &raspberrypi_rp1_config },
>          { .compatible = "xlnx,zynqmp-gem", .data = &zynqmp_config},
>          { .compatible = "xlnx,zynq-gem", .data = &zynq_config },
> 
> --
> 2.51.1
> 

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

On Thu, Oct 23, 2025 at 06:22:55PM +0200, 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 must grab a generic PHY and initialise it.
> 
> We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> the first users of bp->phy that use it in non-SGMII cases.
> 
> The phy_set_mode_ext() call is made unconditionally. It cannot cause
> issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> those cases, the call is a no-op (returning zero). From reading
> upstream DTS, we can figure out that no platform has a bp->phy and a
> PHY driver that has a .set_mode() implementation:
>  - cdns,zynqmp-gem: no DTS upstream.
>  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
>  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
>  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
>    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().

Ran into this patch while looking at other stuff. Theo could you explain
this analysis to someone not really au fait with phys? Looking at
soc.dtsi files won't show you phys, since that's a board level decision,
but you have found one for the zynqmp-gem so I guess that's just the way
you presented the data?
mpfs definitely has phys attached, so is you not finding one for it but
finding for zynqmp, an indication that you were only looking for rgmii
phys? Also, is the analysis of the connected phy driver accurate for
zynmqmp?
zynqmp-zc1751-xm018-dc4.dts seems to have 4 ethernet phys:
		ethernet_phy0: ethernet-phy@0 { /* Marvell 88e1512 */
			reg = <0>;
		};
		ethernet_phy7: ethernet-phy@7 { /* Vitesse VSC8211 */
			reg = <7>;
		};
		ethernet_phy3: ethernet-phy@3 { /* Realtek RTL8211DN */
			reg = <3>;
		};
		ethernet_phy8: ethernet-phy@8 { /* Vitesse VSC8211 */
			reg = <8>;
		};

I'm not sure that this matters because they probably don't actually
implement mode changes as they're dedicated ethernet phys. The zynmp phy
seems to only be used for !ethernet devices. The phys I'm aware of being
used with mpfs are all also dedicated ethernet ones. Calling
phy_set_mode_ext() seems to be to be the right decision, but the rationale
confused me a little.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Russell King (Oracle)]

On Thu, Feb 26, 2026 at 10:46:24AM +0000, Conor Dooley wrote:
> On Thu, Oct 23, 2025 at 06:22:55PM +0200, 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 must grab a generic PHY and initialise it.
> > 
> > We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> > phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> > the first users of bp->phy that use it in non-SGMII cases.
> > 
> > The phy_set_mode_ext() call is made unconditionally. It cannot cause
> > issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> > those cases, the call is a no-op (returning zero). From reading
> > upstream DTS, we can figure out that no platform has a bp->phy and a
> > PHY driver that has a .set_mode() implementation:
> >  - cdns,zynqmp-gem: no DTS upstream.
> >  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
> >  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
> >  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
> >    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> 
> Ran into this patch while looking at other stuff. Theo could you explain
> this analysis to someone not really au fait with phys? Looking at
> soc.dtsi files won't show you phys, since that's a board level decision,
> but you have found one for the zynqmp-gem so I guess that's just the way
> you presented the data?
> mpfs definitely has phys attached, so is you not finding one for it but
> finding for zynqmp, an indication that you were only looking for rgmii
> phys? Also, is the analysis of the connected phy driver accurate for
> zynmqmp?
> zynqmp-zc1751-xm018-dc4.dts seems to have 4 ethernet phys:
> 		ethernet_phy0: ethernet-phy@0 { /* Marvell 88e1512 */
> 			reg = <0>;
> 		};
> 		ethernet_phy7: ethernet-phy@7 { /* Vitesse VSC8211 */
> 			reg = <7>;
> 		};
> 		ethernet_phy3: ethernet-phy@3 { /* Realtek RTL8211DN */
> 			reg = <3>;
> 		};
> 		ethernet_phy8: ethernet-phy@8 { /* Vitesse VSC8211 */
> 			reg = <8>;
> 		};

Ethernet PHYs (drivers/net/phy/) are different from generic PHYs
(drivers/phy/). Ethernet PHYs are completely different beast with a
completely separate subsystem, which doesn't have a "set_mode" method.

Théo is referring to generic PHYs not Ethernet PHYs.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]

On Thu, Feb 26, 2026 at 07:24:47PM +0000, Russell King (Oracle) wrote:
> On Thu, Feb 26, 2026 at 10:46:24AM +0000, Conor Dooley wrote:
> > On Thu, Oct 23, 2025 at 06:22:55PM +0200, 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 must grab a generic PHY and initialise it.
> > > 
> > > We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> > > phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> > > the first users of bp->phy that use it in non-SGMII cases.
> > > 
> > > The phy_set_mode_ext() call is made unconditionally. It cannot cause
> > > issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> > > those cases, the call is a no-op (returning zero). From reading
> > > upstream DTS, we can figure out that no platform has a bp->phy and a
> > > PHY driver that has a .set_mode() implementation:
> > >  - cdns,zynqmp-gem: no DTS upstream.
> > >  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
> > >  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
> > >  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
> > >    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> > 
> > Ran into this patch while looking at other stuff. Theo could you explain
> > this analysis to someone not really au fait with phys? Looking at
> > soc.dtsi files won't show you phys, since that's a board level decision,
> > but you have found one for the zynqmp-gem so I guess that's just the way
> > you presented the data?
> > mpfs definitely has phys attached, so is you not finding one for it but
> > finding for zynqmp, an indication that you were only looking for rgmii
> > phys? Also, is the analysis of the connected phy driver accurate for
> > zynmqmp?
> > zynqmp-zc1751-xm018-dc4.dts seems to have 4 ethernet phys:
> > 		ethernet_phy0: ethernet-phy@0 { /* Marvell 88e1512 */
> > 			reg = <0>;
> > 		};
> > 		ethernet_phy7: ethernet-phy@7 { /* Vitesse VSC8211 */
> > 			reg = <7>;
> > 		};
> > 		ethernet_phy3: ethernet-phy@3 { /* Realtek RTL8211DN */
> > 			reg = <3>;
> > 		};
> > 		ethernet_phy8: ethernet-phy@8 { /* Vitesse VSC8211 */
> > 			reg = <8>;
> > 		};
> 
> Ethernet PHYs (drivers/net/phy/) are different from generic PHYs
> (drivers/phy/). Ethernet PHYs are completely different beast with a
> completely separate subsystem, which doesn't have a "set_mode" method.
> 
> Théo is referring to generic PHYs not Ethernet PHYs.

Right, that's pretty much what I figured and cos of that the patch
itself seemed like it was fine to me. It is the analysis of users in
devicetrees that I don't understand - the "no PHY attached" bits
seemed to me like they should be saying "ethernet-only PHY attached, so
no .set_mode()". Ultimately, I think it makes no difference to the patch
itself, I just wanted to understand the commit message.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Russell King (Oracle)]

On Thu, Feb 26, 2026 at 08:20:41PM +0000, Conor Dooley wrote:
> On Thu, Feb 26, 2026 at 07:24:47PM +0000, Russell King (Oracle) wrote:
> > On Thu, Feb 26, 2026 at 10:46:24AM +0000, Conor Dooley wrote:
> > > On Thu, Oct 23, 2025 at 06:22:55PM +0200, 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 must grab a generic PHY and initialise it.
> > > > 
> > > > We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> > > > phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> > > > the first users of bp->phy that use it in non-SGMII cases.
> > > > 
> > > > The phy_set_mode_ext() call is made unconditionally. It cannot cause
> > > > issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> > > > those cases, the call is a no-op (returning zero). From reading
> > > > upstream DTS, we can figure out that no platform has a bp->phy and a
> > > > PHY driver that has a .set_mode() implementation:
> > > >  - cdns,zynqmp-gem: no DTS upstream.
> > > >  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
> > > >  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
> > > >  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
> > > >    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> > > 
> > > Ran into this patch while looking at other stuff. Theo could you explain
> > > this analysis to someone not really au fait with phys? Looking at
> > > soc.dtsi files won't show you phys, since that's a board level decision,
> > > but you have found one for the zynqmp-gem so I guess that's just the way
> > > you presented the data?
> > > mpfs definitely has phys attached, so is you not finding one for it but
> > > finding for zynqmp, an indication that you were only looking for rgmii
> > > phys? Also, is the analysis of the connected phy driver accurate for
> > > zynmqmp?
> > > zynqmp-zc1751-xm018-dc4.dts seems to have 4 ethernet phys:
> > > 		ethernet_phy0: ethernet-phy@0 { /* Marvell 88e1512 */
> > > 			reg = <0>;
> > > 		};
> > > 		ethernet_phy7: ethernet-phy@7 { /* Vitesse VSC8211 */
> > > 			reg = <7>;
> > > 		};
> > > 		ethernet_phy3: ethernet-phy@3 { /* Realtek RTL8211DN */
> > > 			reg = <3>;
> > > 		};
> > > 		ethernet_phy8: ethernet-phy@8 { /* Vitesse VSC8211 */
> > > 			reg = <8>;
> > > 		};
> > 
> > Ethernet PHYs (drivers/net/phy/) are different from generic PHYs
> > (drivers/phy/). Ethernet PHYs are completely different beast with a
> > completely separate subsystem, which doesn't have a "set_mode" method.
> > 
> > Théo is referring to generic PHYs not Ethernet PHYs.
> 
> Right, that's pretty much what I figured and cos of that the patch
> itself seemed like it was fine to me. It is the analysis of users in
> devicetrees that I don't understand - the "no PHY attached" bits
> seemed to me like they should be saying "ethernet-only PHY attached, so
> no .set_mode()". Ultimately, I think it makes no difference to the patch
> itself, I just wanted to understand the commit message.

I think you're still thinking that ethernet PHYs and generic PHYs are
the same, but only differ in e.g. whether .set_mode() is populated.
This is completely wrong.

Ethernet PHYs are _completely_ separate from Generic PHYs. Forget the
"PHY" bit, that's totally irrelevant that they happen to be called
the same. The two classes share _no_ common infrastructure in the
kernel.

Ethernet PHYs can't have phy_set_mode_ext() called on them, because
ethernet PHYs have no "struct phy" associated with them. They also
can't have phy_power_on() either for the same reason, nor any of the
other generic PHY API calls.

Ethernet PHYs aren't a sub-class of generic PHYs. As I said,
Ethernet PHYs and generic PHYs are two entirely separate and different
subsystems with no commonality.

So... the commit message states "generic PHY". It states the generic
PHY API calls of phy_set_mode_ext() and phy_power_up(). From that we
deduce that bp->phy is a generic PHY. Thus, the commit message is
talking about generic PHYs not ethernet PHYs.

Ethernet PHYs are described in Ethernet controller DT using the
"phy-handle" property (and previously "phy" or "phy-device"

Generic PHYs are described using the "phys" property.

The presence of an ethernet-phy node in DT does not automatically
associate that device with anything - it has to be referenced by
a "phy-handle" property to connect it to an Ethernet controller.

However, as I've been trying to point out, as this commit is about
generic PHYs, Ethernet PHYs are just not relevant to this commit.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Thu, Feb 26, 2026 at 08:44:53PM +0000, Russell King (Oracle) wrote:

> Ethernet PHYs are described in Ethernet controller DT using the
> "phy-handle" property (and previously "phy" or "phy-device"
> 
> Generic PHYs are described using the "phys" property.

Ahh, this was key and I think I understand the analysis now.
Thanks for explaining Russell.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

[Andrew Lunn]

On Thu, Feb 26, 2026 at 09:05:28PM +0000, Conor Dooley wrote:
> On Thu, Feb 26, 2026 at 08:44:53PM +0000, Russell King (Oracle) wrote:
> 
> > Ethernet PHYs are described in Ethernet controller DT using the
> > "phy-handle" property (and previously "phy" or "phy-device"
> > 
> > Generic PHYs are described using the "phys" property.
> 
> Ahh, this was key and I think I understand the analysis now.
> Thanks for explaining Russell.

There is history here.

In Linux, PHY has meant Ethernet PHY for a few decades. But then
SERDES came along which has a completely different sort of PHY as
well. So things like PCIe, USB, SATA, and MAC chips using speeds >
1Gbps all can make use of Generic PHYs for their SERDES.

So you need to be careful with wording, are we talking about PHYs or
Generic PHYs.

	Andrew

Re: [PATCH net-next v3 0/5] net: macb: EyeQ5 support

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 23 Oct 2025 18:22:50 +0200 you wrote:
> This series' goal is adding support to the MACB driver for EyeQ5 GEM.
> The specifics for this compatible are:
> 
>  - HW cannot add dummy bytes at the start of IP packets for alignment
>    purposes. The behavior can be detected using DCFG6 so it isn't
>    attached to compatible data.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/5] dt-bindings: net: cdns,macb: add Mobileye EyeQ5 ethernet interface
    https://git.kernel.org/netdev/net-next/c/c51aa14be9c4
  - [net-next,v3,2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
    https://git.kernel.org/netdev/net-next/c/ae7a9585ea69
  - [net-next,v3,3/5] net: macb: add no LSO capability (MACB_CAPS_NO_LSO)
    https://git.kernel.org/netdev/net-next/c/7a3d209145d1
  - [net-next,v3,4/5] net: macb: rename bp->sgmii_phy field to bp->phy
    https://git.kernel.org/netdev/net-next/c/3f7e51cd5fbf
  - [net-next,v3,5/5] net: macb: Add "mobileye,eyeq5-gem" compatible
    https://git.kernel.org/netdev/net-next/c/48cf0be9b9a6

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