Hi Alice,
On 10/07/2026 10:12, [email protected] wrote:
> From: Alice Guo <[email protected]>
>
> Add a driver for the MaxLinear GPY215C Ethernet PHY.
>
> Signed-off-by: Alice Guo <[email protected]>
> ---
> CI: https://github.com/u-boot/u-boot/pull/1009/checks
> ---
> drivers/net/phy/Kconfig | 6 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mxl_gpy.c | 342
> ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 349 insertions(+)
Is this derived from the Linux driver or re-written independently? AFAICT
the Linux driver is a bit more feature-rich and matches model IDs more
precisely.
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3f7953d693c..f76636a897a 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -242,6 +242,12 @@ config PHY_MSCC
> depends on !COMPILE_TEST
> bool "Microsemi Corp Ethernet PHYs support"
>
> +config PHY_MXL_GPY
> + bool "MaxLinear GPY215C PHY support"
> + help
> + This adds support for the MaxLinear GPY215C Ethernet PHY,
> + operating in SGMII with auto-negotiation.
> +
> config PHY_NATSEMI
> bool "National Semiconductor Ethernet PHYs support"
>
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 83520de7f1f..0e79c03df7e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_MICREL_KSZ8XXX) += micrel_ksz8xxx.o
> obj-$(CONFIG_PHY_MICREL_KSZ90X1) += micrel_ksz90x1.o
> obj-$(CONFIG_PHY_MESON_GXL) += meson-gxl.o
> obj-$(CONFIG_PHY_MOTORCOMM) += motorcomm.o
> +obj-$(CONFIG_PHY_MXL_GPY) += mxl_gpy.o
> obj-$(CONFIG_PHY_NATSEMI) += natsemi.o
> obj-$(CONFIG_PHY_NXP_C45_TJA11XX) += nxp-c45-tja11xx.o
> obj-$(CONFIG_PHY_NXP_TJA11XX) += nxp-tja11xx.o
> diff --git a/drivers/net/phy/mxl_gpy.c b/drivers/net/phy/mxl_gpy.c
> new file mode 100644
> index 00000000000..7445ec5f051
> --- /dev/null
> +++ b/drivers/net/phy/mxl_gpy.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * MaxLinear GPY215 2.5G Ethernet PHY driver for U-Boot
> + *
> + * Based on Ethernet Network Connection GPY215 (GPY215B1VI, GPY215C0VI) Data
> + * Sheet Revision 1.4
> + * Reference ID 617800
> + *
> + * Copyright 2026 NXP
> + */
> +
> +#include <phy.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +
> +/* -----------------------------------------------------------------------
> + * Standard MDIO registers (Clause 22, Device 0)
> + * -----------------------------------------------------------------------
> + */
> +#define GPY215_STD_CTRL 0x00
> +#define STD_CTRL_RST BIT(15) /* Self-clearing reset
> */
> +#define STD_CTRL_LB BIT(14) /* Loopback
> */
> +#define STD_CTRL_SSL BIT(13) /* Speed select LSB
> */
> +#define STD_CTRL_ANEN BIT(12) /* Auto-negotiation
> enable */
> +#define STD_CTRL_PD BIT(11) /* Power down (SLEEP)
> */
> +#define STD_CTRL_ANRS BIT(9) /* Restart
> auto-negotiation */
> +#define STD_CTRL_DPLX BIT(8) /* Duplex (1=full)
> */
> +#define STD_CTRL_SSM BIT(6) /* Speed select MSB
> */
> +
> +#define GPY215_STD_STAT 0x01
> +#define STD_STAT_LS BIT(2) /* Link status
> (latching low) */
> +#define STD_STAT_ANEG_DONE BIT(5) /* Auto-neg complete */
> +
> +#define GPY215_STD_PHYID1 0x02 /* OUI bits [3:18] = 0x67C9 */
> +#define GPY215_STD_PHYID2 0x03 /* OUI bits [19:24] + model +
> rev */
> +
> +/* PHY ID: OUI = 0x67C9 (bits 3:18) | 0xDC (bits 19:24)
> + * PHYID1 = 0x67C9, PHYID2 = 0xDCxx (model/rev in lower 10 bits)
> + */
> +#define GPY215_PHY_ID1 0x67C9
> +#define GPY215_PHY_ID2_MASK 0xFC00 /* OUI[19:24] only */
> +#define GPY215_PHY_ID2_VAL 0xDC00
> +
> +/* -----------------------------------------------------------------------
Coding style
/*
* -----------------------------------------------------------------------
* ...
*/
> + * GPY-specific registers (Clause 22, Device 0)
> + * -----------------------------------------------------------------------
> + */
> +#define GPY215_PHY_MIISTAT 0x18 /* Register 0.24 */
> +#define MIISTAT_LS BIT(10) /* TPI link status */
> +#define MIISTAT_DPX BIT(3) /* Duplex (1=full)
> */
> +#define MIISTAT_SPEED_MASK GENMASK(2, 0)
> +#define MIISTAT_SPEED_10 0x0
> +#define MIISTAT_SPEED_100 0x1
> +#define MIISTAT_SPEED_1000 0x2
> +#define MIISTAT_SPEED_ANEG 0x3
> +#define MIISTAT_SPEED_2500 0x4
> +
> +#define GPY215_PHY_IMASK 0x19 /* Register 0.25 */
> +#define IMASK_LSTC BIT(0) /* Link state change */
> +#define IMASK_LSPC BIT(1) /* Link speed change */
> +
> +#define GPY215_PHY_ISTAT 0x1A /* Register 0.26 (clr-on-rd) */
> +#define ISTAT_LSTC BIT(0)
> +#define ISTAT_LSPC BIT(1)
> +
> +/* -----------------------------------------------------------------------
> + * Vendor Specific 1 registers (MMD device 30)
> + * Accessed via Clause 45 or Clause 22 Extended indirect
> + * -----------------------------------------------------------------------
> + */
> +#define GPY215_MMD_VSPEC1 30 /* MMD device address */
> +
> +#define GPY215_VSPEC1_SGMII_CTRL 0x08 /* Register 30.8 */
> +#define SGMII_CTRL_RST BIT(15) /* SGMII reset (self-clear) */
> +#define SGMII_CTRL_LB BIT(14) /* SGMII loopback
> */
> +#define SGMII_CTRL_ANEN BIT(12) /* SGMII ANEG enable */
> +#define SGMII_CTRL_PD BIT(11) /* SGMII power down
> */
> +#define SGMII_CTRL_RXINV BIT(10) /* Invert RX0_M/RX0_P */
> +#define SGMII_CTRL_EEE_CAP BIT(7) /* Advertise EEE in ANEG */
> +#define SGMII_CTRL_FIXED2G5 BIT(5) /* Force SGMII to 2.5G
> */
> +#define SGMII_CTRL_ANMODE_MASK GENMASK(1, 0)
> +#define SGMII_CTRL_ANMODE_1000BX 0x1 /* IEEE 1000Bx Clause 37 */
> +#define SGMII_CTRL_ANMODE_CIS_PHY 0x2 /* Cisco SGMII, PHY side */
> +#define SGMII_CTRL_ANMODE_CIS_MAC 0x3 /* Cisco SGMII, MAC side */
> +/* Default reset value = 0x34DA:
> + * ANEN=1, ANMODE=AN_CIS_PHY(2), FIXED2G5=0
> + */
> +
> +#define GPY215_VSPEC1_SGMII_STAT 0x09 /* Register 30.9 (read-only) */
> +#define SGMII_STAT_ANOK BIT(5) /* ANEG completed */
> +#define SGMII_STAT_RF BIT(4) /* Remote fault (latch
> high) */
> +#define SGMII_STAT_ANAB BIT(3) /* ANEG ability (static) */
> +#define SGMII_STAT_LS BIT(2) /* SGMII link up (latch
> low) */
> +#define SGMII_STAT_DR_MASK GENMASK(1, 0)
> +#define SGMII_STAT_DR_10 0x0
> +#define SGMII_STAT_DR_100 0x1
> +#define SGMII_STAT_DR_1G 0x2
> +#define SGMII_STAT_DR_2G5 0x3
> +
> +/* -----------------------------------------------------------------------
> + * Timing / retry constants
> + * -----------------------------------------------------------------------
> + */
> +#define GPY215_RESET_TIMEOUT_MS 500
> +#define GPY215_ANEG_TIMEOUT_MS 5000
> +#define GPY215_POLL_INTERVAL_MS 10
> +
> +/**
Do not use a kernel-doc comment if you're not providing the parameter and
return code description
> + * gpy215_probe - Verify PHY identity
> + */
> +static int gpy215_probe(struct phy_device *phydev)
> +{
> + int id1, id2;
> +
> + id1 = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_PHYID1);
> + id2 = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_PHYID2);
> +
> + if (id1 < 0 || id2 < 0) {
> + printf("GPY215: MDIO read failed (id1=%d id2=%d)\n", id1, id2);
> + return -EIO;
This loses the actual error code (for instance -ETIMEDOUT or -ENODEV).
> + }
> +
> + if (id1 != GPY215_PHY_ID1 ||
> + (id2 & GPY215_PHY_ID2_MASK) != GPY215_PHY_ID2_VAL) {
> + printf("GPY215: unexpected PHY ID 0x%04X 0x%04X\n", id1, id2);
> + return -ENODEV;
> + }
I believe if you had proper model-specific .uid and .mask, this probe-time
identity check would be unnecessary altogether.
> +
> + debug("GPY215: detected (ID1=0x%04X ID2=0x%04X)\n", id1, id2);
> + return 0;
> +}
> +
> +/**
> + * gpy215_config - Configure PHY for SGMII operation
> + *
> + * The GPY215 SGMII interface comes up automatically after reset with:
> + * - SGMII ANEG enabled (Cisco SGMII, PHY side)
> + * - PHY side SGMII speed tracks TPI link speed
> + */
> +static int gpy215_config(struct phy_device *phydev)
> +{
> + int val, timeout;
> +
> + val = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL);
> + if (val < 0)
> + return val;
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL,
> + val | STD_CTRL_RST);
The return status should probably be checked, otherwise the
polling loop below might report success even if the reset request was
never issued.
> +
> + /* Wait for self-clearing RST bit */
> + timeout = GPY215_RESET_TIMEOUT_MS / GPY215_POLL_INTERVAL_MS;
> + do {
> + mdelay(GPY215_POLL_INTERVAL_MS);
> + val = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL);
> + if (val < 0)
> + return val;
> + } while ((val & STD_CTRL_RST) && --timeout);
> +
> + if (val & STD_CTRL_RST) {
> + printf("GPY215: reset timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + /* Clear stale interrupt status (read-to-clear) */
> + phy_read(phydev, MDIO_DEVAD_NONE, GPY215_PHY_ISTAT);
> +
> + val = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL);
> + if (val < 0)
> + return val;
> +
> + /* Enable ANEG, clear forced speed/duplex bits */
> + val |= STD_CTRL_ANEN;
> + val &= ~(STD_CTRL_SSL | STD_CTRL_SSM | STD_CTRL_PD);
> + phy_write(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL, val);
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL,
> + val | STD_CTRL_ANRS);
> +
> + /* Configure SGMII (Cisco ANEG, PHY side, ANEG enabled) */
> + val = phy_read_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_CTRL);
> + if (val < 0)
> + return val;
> +
> + val &= ~SGMII_CTRL_PD;
> + val |= SGMII_CTRL_ANEN;
> + val &= ~SGMII_CTRL_ANMODE_MASK;
> + val |= SGMII_CTRL_ANMODE_CIS_PHY;
> + val &= ~SGMII_CTRL_FIXED2G5;
> + phy_write_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_CTRL, val);
> +
> + debug("GPY215: configured for SGMII (Cisco ANEG, PHY side)\n");
> + return 0;
> +}
> +
> +/**
> + * gpy215_startup - Wait for link and report status
> + *
> + * Polls PHY_MIISTAT (TPI link) and VSPEC1_SGMII_STAT (SGMII link).
> + * Updates phydev->speed, phydev->duplex, phydev->link.
> + */
> +static int gpy215_startup(struct phy_device *phydev)
> +{
> + int miistat, sgmii_stat, val;
> + int timeout = GPY215_ANEG_TIMEOUT_MS / GPY215_POLL_INTERVAL_MS;
This timeout is initialized once and is therefore shared by all phases
below. It it the intent? Or should the timeout be reset between each
phase?
Also I tend to prefer the use of get_timer() but that's essentially
cosmetic.
start = get_timer(0);
do {
...
if (...)
break;
} while (get_timer(start) < SOME_TIMEOUT);
> + int speed_code;
> +
> + debug("GPY215: waiting for link");
> +
> + /* Poll TPI link status */
> + do {
> + mdelay(GPY215_POLL_INTERVAL_MS);
> + miistat = phy_read(phydev, MDIO_DEVAD_NONE,
> + GPY215_PHY_MIISTAT);
> + if (miistat < 0)
> + return miistat;
> + } while (!(miistat & MIISTAT_LS) && --timeout);
> +
> + if (!(miistat & MIISTAT_LS)) {
> + printf("GPY215: link down (TPI timeout)\n");
> + phydev->link = 0;
> + return 0;
Why return success?
> + }
> +
> + do {
> + mdelay(GPY215_POLL_INTERVAL_MS);
> + val = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_STAT);
> + if (val < 0)
> + return val;
> + } while (!(val & STD_STAT_ANEG_DONE) && --timeout);
> +
> + if (!(val & STD_STAT_ANEG_DONE)) {
> + printf("GPY215: auto-negotiation not complete\n");
> + phydev->link = 0;
> + return 0;
Same question here.
> + }
> +
> + do {
> + mdelay(GPY215_POLL_INTERVAL_MS);
> + phy_read_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_STAT);
> + sgmii_stat = phy_read_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_STAT);
> + if (sgmii_stat < 0)
> + return sgmii_stat;
> + } while (((sgmii_stat & (SGMII_STAT_LS | SGMII_STAT_ANOK)) !=
> + (SGMII_STAT_LS | SGMII_STAT_ANOK)) && --timeout);
> +
> + if ((sgmii_stat & (SGMII_STAT_LS | SGMII_STAT_ANOK)) !=
> + (SGMII_STAT_LS | SGMII_STAT_ANOK)) {
> + printf("GPY215: SGMII not ready (stat=0x%04X link=%d ANOK=%d
> RF=%d)\n",
> + sgmii_stat,
> + !!(sgmii_stat & SGMII_STAT_LS),
> + !!(sgmii_stat & SGMII_STAT_ANOK),
> + !!(sgmii_stat & SGMII_STAT_RF));
> + phydev->link = 0;
> + return 0;
Same here.
> + }
> +
> + /* Clear interrupt status */
> + phy_read(phydev, MDIO_DEVAD_NONE, GPY215_PHY_ISTAT);
> +
> + miistat = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_PHY_MIISTAT);
> + if (miistat < 0)
> + return miistat;
> +
> + speed_code = miistat & MIISTAT_SPEED_MASK;
> + switch (speed_code) {
> + case MIISTAT_SPEED_10:
> + phydev->speed = SPEED_10;
> + break;
> + case MIISTAT_SPEED_100:
> + phydev->speed = SPEED_100;
> + break;
> + case MIISTAT_SPEED_1000:
> + phydev->speed = SPEED_1000;
> + break;
> + case MIISTAT_SPEED_2500:
> + phydev->speed = SPEED_2500;
Here the driver reports 2.5 Gbps but see [1] below.
> + break;
> + default:
> + printf("GPY215: speed not resolved (miistat=0x%04X)\n",
> + miistat);
> + phydev->link = 0;
> + return 0;
Why return success?
> + }
> +
> + phydev->duplex = (miistat & MIISTAT_DPX) ? DUPLEX_FULL : DUPLEX_HALF;
> + phydev->link = 1;
Extra whitespace
> +
> + debug("GPY215: TPI link up %d Mbit/s %s-duplex\n",
> + phydev->speed,
> + phydev->duplex == DUPLEX_FULL ? "full" : "half");
> +
> + debug("GPY215: SGMII status 0x%04X (link=%d ANOK=%d DR=%ld)\n",
%04X is unusual, how about %04x?
%ld is incorrect. Use %d or (better?) %x
> + sgmii_stat,
> + !!(sgmii_stat & SGMII_STAT_LS),
> + !!(sgmii_stat & SGMII_STAT_ANOK),
> + sgmii_stat & SGMII_STAT_DR_MASK);
> +
> + return 0;
> +}
> +
> +/**
> + * gpy215_shutdown - Power down SGMII and TPI
> + */
> +static int gpy215_shutdown(struct phy_device *phydev)
> +{
> + int val;
> +
> + val = phy_read_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_CTRL);
> + if (val >= 0)
> + phy_write_mmd(phydev, GPY215_MMD_VSPEC1,
> + GPY215_VSPEC1_SGMII_CTRL,
> + val | SGMII_CTRL_PD);
> +
> + val = phy_read(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL);
> + if (val >= 0)
> + phy_write(phydev, MDIO_DEVAD_NONE, GPY215_STD_CTRL,
> + val | STD_CTRL_PD);
> +
> + return 0;
Not sure if always returning success is OK or not. In any case
proper error handling and reporting would not be hard.
> +}
> +
> +U_BOOT_PHY_DRIVER(mxl_gpy215c) = {
> + .name = "MaxLinear GPY215C",
> + /*
> + * Match on OUI only. The 22-bit OUI spans PHYID1[15:0] and
> PHYID2[15:10].
> + * Combined 22-bit PHY ID = (0x67C9 << 6) | (0xDC00 >> 10) = 0x19F277
> + * U-Boot phy_id = (PHYID1 << 16) | PHYID2 with mask 0xFFFFFC00
> + */
Why is it safe for this driver to bind to any MaxLinear PHY using this OUI?
> + .uid = (GPY215_PHY_ID1 << 16) | GPY215_PHY_ID2_VAL,
> + .mask = 0xFFFFFC00,
> + .features = PHY_GBIT_FEATURES,
[1] here it advertises only 10/100/1000 capability. Would it be
appropriate to use 'PHY_GBIT_FEATURES | SUPPORTED_2500baseT_Full'?
> + .probe = &gpy215_probe,
> + .config = &gpy215_config,
> + .startup = &gpy215_startup,
> + .shutdown = &gpy215_shutdown,
The & are not needed.
> +};
>
> ---
> base-commit: a18265f1ccb7a272721ed4286ed3b5a6182ff424
> change-id: 20260710-phy-a8d7427ab27d
>
> Best regards,
> --
> Alice Guo <[email protected]>
>
