Re: [PATCH net-next v4 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:
- [Medium] Register overwrite due to unconditional write instead of read-modify-write.
--

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

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

This commit adds a new driver for MediaTek MT7628 built-in Fast Ethernet PHYs.
It configures the PHY to establish 100mbps links by setting an initialization
bit in an undocumented register.

> 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..317944411fbe7
> --- /dev/null
> +++ b/drivers/net/phy/mediatek/mtk-fe-soc.c

[ ... ]

> +static int mt7628_phy_config_init(struct phy_device *phydev)
> +{
> +	/*
> +	 * This undocumented bit is required for the PHYs to be able to
> +	 * establish 100mbps links.
> +	 */
> +	return phy_write_paged(phydev, 0x8000, 30, BIT(13));

[Severity: Medium]
The comment and commit message mention setting an initialization bit. Does
this unconditional write safely preserve the other bits in this register?

If other fields in this undocumented register are initialized by hardware
reset or the bootloader, writing 0x2000 will destructively clear them to 0.
Should this use a read-modify-write operation like phy_modify_paged()
instead, or is overwriting the entire register intentional?

> +}

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