* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
2026-03-26 15:35 [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support Lucien.Jheng
@ 2026-03-26 20:00 ` Eric Woudstra
2026-03-27 4:41 ` Eric Woudstra
1 sibling, 0 replies; 3+ messages in thread
From: Eric Woudstra @ 2026-03-26 20:00 UTC (permalink / raw)
To: Lucien.Jheng, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
On 3/26/26 4:35 PM, Lucien.Jheng wrote:
> AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
> Assert the MCU (hold it in reset) and immediately deassert (release)
> via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
> through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
> path. This clears the MCU state before the firmware loading sequence
> starts.
Perhapse you can register this extra device in the probe() function:
struct mdio_device *mdiodev;
mdiodev = mdio_device_create(phydev->mdio.bus,
phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
if (IS_ERR(mdiodev))
return PTR_ERR(mdiodev);
ret = mdio_device_register(mdiodev);
if (ret) {
mdio_device_free(mdiodev);
return ret;
}
priv->pbusdev = mdiodev;
In the define part add:
#define EN8811H_PBUS_ADDR_OFFS 8
Add to struct en8811h_priv:
struct mdio_device *pbusdev;
And in the remove function:
mdio_device_remove(priv->pbusdev);
mdio_device_free(priv->pbusdev);
Then you can use the device:
ret = en8811hb_some_neat_function(priv->pbusdev);
>
> Add __air_pbus_reg_write() as a low-level helper for this access, then
> implement an8811hb_mcu_assert() / _deassert() on top of it. Wire both
> into an8811hb_load_firmware() and en8811h_restart_mcu() so every
> firmware load or MCU restart on AN8811HB correctly sequences the reset
> control registers.
>
> Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
> ---
> Changes in v2:
> - Rewrite commit message: The previous wording was ambiguous,
> as it suggested the MCU remains asserted during the entire
> firmware loading process, rather than a quick reset cycle
> before it starts.
> Clarify that assert and deassert is an immediate soft-reset
> cycle to clear MCU state before firmware loading begins.
> - Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
> support").
> - Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
> an8811hb_mcu_deassert() to avoid noise during normal boot.
>
> drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index 29ae73e65caa..01fce1b93618 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c
> @@ -170,6 +170,16 @@
> #define AN8811HB_CLK_DRV_CKO_LDPWD BIT(13)
> #define AN8811HB_CLK_DRV_CKO_LPPWD BIT(14)
>
> +#define AN8811HB_MCU_SW_RST 0x5cf9f8
> +#define AN8811HB_MCU_SW_RST_HOLD BIT(16)
> +#define AN8811HB_MCU_SW_RST_RUN (BIT(16) | BIT(0))
> +#define AN8811HB_MCU_SW_START 0x5cf9fc
> +#define AN8811HB_MCU_SW_START_EN BIT(16)
> +
> +/* MII register constants for PBUS access (PHY addr + 8) */
> +#define AIR_PBUS_ADDR_HIGH 0x1c
> +#define AIR_PBUS_DATA_HIGH 0x10
> +
> /* Led definitions */
> #define EN8811H_LED_COUNT 3
>
> @@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
> return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
> }
>
> +static int __air_pbus_reg_write(struct phy_device *phydev,
> + u32 pbus_reg, u32 pbus_data)
> +{
> + struct mii_bus *bus = phydev->mdio.bus;
> + int pbus_addr = phydev->mdio.addr + 8;
> + int ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
> + upper_16_bits(pbus_reg));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
> + (pbus_reg & GENMASK(15, 6)) >> 6);
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
> + lower_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
> + upper_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> static int __air_buckpbus_reg_write(struct phy_device *phydev,
> u32 pbus_address, u32 pbus_data)
> {
> @@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
> return ret;
> }
>
> +static int an8811hb_mcu_assert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_HOLD);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU asserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> +static int an8811hb_mcu_deassert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
> + AN8811HB_MCU_SW_START_EN);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_RUN);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU deasserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> static int an8811hb_load_firmware(struct phy_device *phydev)
> {
> int ret;
>
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> @@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
> {
> int ret;
>
> + if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> + }
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
2026-03-26 15:35 [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support Lucien.Jheng
2026-03-26 20:00 ` Eric Woudstra
@ 2026-03-27 4:41 ` Eric Woudstra
1 sibling, 0 replies; 3+ messages in thread
From: Eric Woudstra @ 2026-03-27 4:41 UTC (permalink / raw)
To: Lucien.Jheng, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
On 3/26/26 4:35 PM, Lucien.Jheng wrote:
> AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
> Assert the MCU (hold it in reset) and immediately deassert (release)
> via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
> through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
> path. This clears the MCU state before the firmware loading sequence
> starts.
>
> Add __air_pbus_reg_write() as a low-level helper for this access, then
> implement an8811hb_mcu_assert() / _deassert() on top of it. Wire both
> into an8811hb_load_firmware() and en8811h_restart_mcu() so every
> firmware load or MCU restart on AN8811HB correctly sequences the reset
> control registers.
>
> Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
> ---
> Changes in v2:
> - Rewrite commit message: The previous wording was ambiguous,
> as it suggested the MCU remains asserted during the entire
> firmware loading process, rather than a quick reset cycle
> before it starts.
> Clarify that assert and deassert is an immediate soft-reset
> cycle to clear MCU state before firmware loading begins.
> - Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
> support").
> - Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
> an8811hb_mcu_deassert() to avoid noise during normal boot.
>
> drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index 29ae73e65caa..01fce1b93618 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c
> @@ -170,6 +170,16 @@
> #define AN8811HB_CLK_DRV_CKO_LDPWD BIT(13)
> #define AN8811HB_CLK_DRV_CKO_LPPWD BIT(14)
>
> +#define AN8811HB_MCU_SW_RST 0x5cf9f8
> +#define AN8811HB_MCU_SW_RST_HOLD BIT(16)
> +#define AN8811HB_MCU_SW_RST_RUN (BIT(16) | BIT(0))
> +#define AN8811HB_MCU_SW_START 0x5cf9fc
> +#define AN8811HB_MCU_SW_START_EN BIT(16)
> +
> +/* MII register constants for PBUS access (PHY addr + 8) */
> +#define AIR_PBUS_ADDR_HIGH 0x1c
> +#define AIR_PBUS_DATA_HIGH 0x10
> +
> /* Led definitions */
> #define EN8811H_LED_COUNT 3
>
> @@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
> return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
> }
>
> +static int __air_pbus_reg_write(struct phy_device *phydev,
> + u32 pbus_reg, u32 pbus_data)
> +{
> + struct mii_bus *bus = phydev->mdio.bus;
> + int pbus_addr = phydev->mdio.addr + 8;
> + int ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
> + upper_16_bits(pbus_reg));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
> + (pbus_reg & GENMASK(15, 6)) >> 6);
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
> + lower_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
> + upper_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
Maybe you can use mutex_lock() and mutex_unlock() here also, like so:
static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
u32 pbus_data)
{
int ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
(u16)(pbus_address >> 6));
if (ret < 0)
return ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
(u16)pbus_data);
if (ret < 0)
return ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
(u16)(pbus_data >> 16));
if (ret < 0)
return ret;
return 0;
}
static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
u32 pbus_data)
{
int ret;
mutex_lock(&mdio->bus->mdio_lock);
ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
if (ret < 0)
dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
pbus_address, ret);
mutex_unlock(&mdio->bus->mdio_lock);
return ret;
}
static int __air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
u32 *pbus_data)
{
int ret, pbus_data_low;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
(u16)(pbus_address >> 6));
if (ret < 0)
return ret;
ret = __mdiobus_read(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf);
if (ret < 0)
return ret;
pbus_data_low = ret;
ret = __mdiobus_read(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH);
if (ret < 0)
return ret;
*pbus_data = (ret << 16) + pbus_data_low;
return 0;
}
static int air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
u32 *pbus_data)
{
int ret;
mutex_lock(&mdio->bus->mdio_lock);
ret = __air_pbus_reg_read(mdio, pbus_address, pbus_data);
if (ret < 0)
dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
pbus_address, ret);
mutex_unlock(&mdio->bus->mdio_lock);
return ret;
}
With:
#define AIR_PBUS_DATA_HIGH 0x10
> static int __air_buckpbus_reg_write(struct phy_device *phydev,
> u32 pbus_address, u32 pbus_data)
> {
> @@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
> return ret;
> }
>
> +static int an8811hb_mcu_assert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_HOLD);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU asserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> +static int an8811hb_mcu_deassert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
> + AN8811HB_MCU_SW_START_EN);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_RUN);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU deasserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> static int an8811hb_load_firmware(struct phy_device *phydev)
> {
> int ret;
>
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> @@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
> {
> int ret;
>
> + if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> + }
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread