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, ®_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!