Thread (14 messages) 14 messages, 6 authors, 2026-03-25

Re: [PATCH net-next 2/2] net: phy: Introduce Airoha AN8801/R Gigabit Ethernet PHY driver

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2026-03-04 16:32:30
Also in: linux-arm-kernel, linux-devicetree, linux-mediatek, lkml

On Wed, Mar 04, 2026 at 10:35:29AM +0100, Louis-Alexis Eyraud wrote:
+static void an8801r_get_wol(struct phy_device *phydev,
+			    struct ethtool_wolinfo *wol)
+{
+	u32 reg_val;
+
+	air_buckpbus_reg_read(phydev, AN8801_BPBUS_REG_WAKEUP_CTL1, &reg_val);
+
+	wol->supported = WAKE_MAGIC;
+
+	if (reg_val & AN8801_WOL_WAKE_MAGIC_EN)
+		wol->wolopts |= WAKE_MAGIC;
+	else
+		wol->wolopts &= ~WAKE_MAGIC;
Please only support WoL if you know that the PHY has been wired up in
such a way to allow it to actually wake the system. The PHY itself
merely supporting WoL is insufficient.

Please look at my recent change to realtek_main.c in commit
b826bf795564 ("net: phy: realtek: fix RTL8211F wake-on-lan support")
to see a possible way to achieve this.
+static int an8801r_config_init(struct phy_device *phydev)
+{
+	u8 led_default_function[AN8801R_NUM_LEDS] = { 0 };
+	int prev_page, ret;
+
+	ret = an8801r_of_init_leds(phydev, led_default_function);
+	if (ret)
+		return ret;
+
+	/* Disable Low Power Mode (LPM) */
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, AN8801_REG_PHY_INTERNAL0,
+			    FIELD_PREP(AN8801_PHY_INTFUNC_MASK, 0x1e));
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, AN8801_REG_PHY_INTERNAL1,
+			    FIELD_PREP(AN8801_PHY_INTFUNC_MASK, 0x2));
+	if (ret)
+		return ret;
+
+	/* Disable EEE by default */
+	ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+	if (ret)
+		return ret;
Are you sure this is safe, e.g. over a suspend/resume, and doesn't
cause the hardware vs software state to desync?
+
+	prev_page = phy_select_page(phydev, AIR_PHY_PAGE_EXTENDED_1);
+	if (prev_page < 0)
+		return prev_page;
No, this is buggy. Please read the phy_select_page() documentation to
find out why.
+
+	/* Set the PHY to perform auto-downshift after 3 auto-negotiation
+	 * attempts
+	 */
+	__phy_write(phydev, AN8801_EXT_REG_PHY,
+		    FIELD_PREP(AN8801_EXT_PHY_CTRL1, 0x1d) |
+		    FIELD_PREP(AN8801_EXT_PHY_DOWNSHIFT_CTL, 1) |
+		    AN8801_EXT_PHY_DOWNSHIFT_EN);
+
+	ret = phy_restore_page(phydev, prev_page, ret);
+	if (ret)
+		return ret;
However, the bug could've been avoided by using the appropriate
accessor:

	ret = phy_write_paged(phydev, AIR_PHY_PAGE_EXTENDED_1,
			      AN8801_EXT_REG_PHY,
			      FIELD_PREP(AN8801_EXT_PHY_CTRL1, 0x1d) |
			      FIELD_PREP(AN8801_EXT_PHY_DOWNSHIFT_CTL, 1) |
			      AN8801_EXT_PHY_DOWNSHIFT_EN);
	if (ret < 0)
		return ret;
+static int an8801r_read_status(struct phy_device *phydev)
+{
+	int prev_speed, ret;
+	u32 val;
+
+	prev_speed = phydev->speed;
+
+	ret = genphy_read_status(phydev);
+	if (ret)
+		return ret;
+
+	if (!phydev->link)
+		return 0;
+
+	if (prev_speed != phydev->speed) {
Maybe:

	if (phydev->link && prev_speed != phydev->speed) {

?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help