Re: [PATCH net-next v5 2/4] net: phy: mediatek: add phy driver for MT7628 built-in Fast Ethernet PHYs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `mtk_soc_fe_phy_driver` lacks `.suspend` and `.resume` callbacks (typically assigned to `genphy_suspend` and `genphy_resume`). This causes a severe power management regression and breaks hardware initialization after system resume. Without a `.suspend` callback, `phy_suspend()` returns 0 without powering down the PHY (`BMCR_PDOWN`). Furthermore, `mdio_bus_phy_may_suspend()` returns false, which prevents `phydev->suspended_by_mdio_bus` from being set to 1 during system suspend. During system resume, `mdio_bus_phy_resume()` checks this flag and, seeing it is false, completely skips calling `phy_init_hw()`. Because `phy_init_hw()` is bypassed, the driver's `.config_init` callback (`mt7628_phy_config_init`) is never executed. Since this callback sets a critical undocumented bit required for 100 Mbps links, the PHY will silently fail to establish correct links if it loses power or state during suspend.
--

commit 6c0828ab418aa7d550c1020af637c9ed3e4c1236
Author: Joris Vaisvila <joey@tinyisr.com>

net: phy: mediatek: add phy driver for MT7628 built-in Fast Ethernet PHYs

This commit adds support for the Fast Ethernet PHYs in the MT7628 SoCs.
It adds the MEDIATEK_FE_SOC_PHY Kconfig option and the corresponding
mtk-fe-soc.c driver to set an undocumented bit required for 100mbps links.

> diff --git a/drivers/net/phy/mediatek/mtk-fe-soc.c b/drivers/net/phy/mediatek/mtk-fe-soc.c
> new file mode 100644
> index 0000000000000..9eb4960bcaade
> --- /dev/null
> +++ b/drivers/net/phy/mediatek/mtk-fe-soc.c

[ ... ]

> +static struct phy_driver mtk_soc_fe_phy_driver[] = {
> +	{
> +		PHY_ID_MATCH_EXACT(MTK_FPHY_ID_MT7628),
> +		.name = "MediaTek MT7628 PHY",
> +		.config_init = mt7628_phy_config_init,
> +		.read_page = mt7628_phy_read_page,
> +		.write_page = mt7628_phy_write_page,
> +	},
> +};

[Severity: High]
Does this driver need .suspend and .resume callbacks (like genphy_suspend
and genphy_resume) to properly handle system power management?

Without a .suspend callback, mdio_bus_phy_may_suspend() returns false,
which prevents phydev->suspended_by_mdio_bus from being set during system
suspend.

During resume, mdio_bus_phy_resume() checks this flag and skips calling
phy_init_hw() entirely. Because phy_init_hw() is bypassed, the
mt7628_phy_config_init() callback is never executed after a resume.

Will this cause the PHY to silently fail to establish 100 Mbps links after
waking up from suspend because the undocumented bit is lost?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613181845.111877-1-joey@tinyisr.com?part=2