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.
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 914677f30f2c..c45c6a7f42e6 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2963,6 +2963,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;
@@ -5187,6 +5191,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,
@@ -5341,6 +5367,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 |
@@ -5372,6 +5409,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
Hi, On 22/10/2025 09:38, 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. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> This seems good to me. I was worried that introducing the unconditionnal call to phy_set_mode_ext() could trigger spurious errors should the generic PHY driver not support the requested interface, but AFAICT there's only the zynqmp in-tree that use the 'phys' property with macb, and the associated generic PHY driver (drivers/phy/phy-zynqmp.c) doesn't implement a .set_mode, so that looks safe. Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Thanks ! Maxime
On Wed, Oct 22, 2025 at 10:09:49AM +0200, Maxime Chevallier wrote: > Hi, > > On 22/10/2025 09:38, 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. > > > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > This seems good to me. I was worried that introducing the unconditionnal > call to phy_set_mode_ext() could trigger spurious errors should the > generic PHY driver not support the requested interface, but AFAICT > there's only the zynqmp in-tree that use the 'phys' property with macb, > and the associated generic PHY driver (drivers/phy/phy-zynqmp.c) doesn't > implement a .set_mode, so that looks safe. I was thinking along the same lines, is this actually safe? It would be good to add something like this to the commit message to indicate this change is safe, the needed code analysis has been performed. Andrew
On Wed Oct 22, 2025 at 9:33 PM CEST, Andrew Lunn wrote: > On Wed, Oct 22, 2025 at 10:09:49AM +0200, Maxime Chevallier wrote: >> Hi, >> >> On 22/10/2025 09:38, 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. >> > >> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> >> >> This seems good to me. I was worried that introducing the unconditionnal >> call to phy_set_mode_ext() could trigger spurious errors should the >> generic PHY driver not support the requested interface, but AFAICT >> there's only the zynqmp in-tree that use the 'phys' property with macb, >> and the associated generic PHY driver (drivers/phy/phy-zynqmp.c) doesn't >> implement a .set_mode, so that looks safe. > > I was thinking along the same lines, is this actually safe? It would > be good to add something like this to the commit message to indicate > this change is safe, the needed code analysis has been performed. Sure, will integrate a summary similar to my reply to Maxime's message. https://lore.kernel.org/lkml/DDOQYH87ZV1H.1QZH1R36WMIC6@bootlin.com/ Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed Oct 22, 2025 at 10:09 AM CEST, Maxime Chevallier wrote: > On 22/10/2025 09:38, 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. >> >> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > This seems good to me. I was worried that introducing the unconditionnal > call to phy_set_mode_ext() could trigger spurious errors should the > generic PHY driver not support the requested interface, but AFAICT > there's only the zynqmp in-tree that use the 'phys' property with macb, > and the associated generic PHY driver (drivers/phy/phy-zynqmp.c) doesn't > implement a .set_mode, so that looks safe. > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Ah, good catch. I checked that both !phy || !phy->ops->set_mode lead to return 0, but I hadn't checked if other PHY drivers could have a .set_mode() implementation that failed on this new call. Compatibles that might read a "phys" DT property: - cdns,zynqmp-gem => no DT upstream - microchip,mpfs-macb => no DT upstream - xlnx,versal-gem => xilinx/versal-net.dtsi, &gem0..1, no PHY attached. - xlnx,zynqmp-gem => xilinx/zynqmp.dtsi, &gem0..3, PHY attached in xilinx/zynqmp-sck-kr-g-rev*.dtso. PHY provider is &psgtr, "xlnx,zynqmp-psgtr-v1.1", drivers/phy/xilinx/phy-zynqmp.c So as you pointed out, only xilinx/phy-zynqmp.c is used according to upstream DTs. I also checked lkml, no patches adding a .set_mode(). We shouldn't be breaking upstream DTs with the current patch. Thanks for the review Maxime, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
© 2016 - 2026 Red Hat, Inc.