Re: [PATCH v4] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support

["Lucien.Jheng" <lucienzx159@gmail.com>]

Hi all

I accidentally included the changelog in the commit message. I will 
correct this in the upcoming v5 patchset.

Please ignore the v4 patch.

Thanks.

Lucien.Jheng 於 2026/5/17 下午 02:42 寫道:
> 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 a registered mdio_device at PHY-addr+8.
>
> Add __air_pbus_reg_write() as a low-level helper taking a struct
> mdio_device *, create and register the PBUS mdio_device in
> an8811hb_probe() and store it in priv->pbusdev, then implement
> an8811hb_mcu_assert() / _deassert() on top of it. Add
> an8811hb_remove() to unregister the PBUS device on teardown. Wire
> both calls into an8811hb_load_firmware() and en8811h_restart_mcu()
> so every firmware load or MCU restart on AN8811HB correctly sequences
> the reset control registers.
>
> Fixes: 5afda1d734ed ("net: phy: air_en8811h: add Airoha AN8811HB support")
> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
>
> Changes in v4:
> - Correct Fixes tag to 5afda1d734ed.
> - Refine an8811hb_probe() error paths: split cleanup into err_dev_free and
>    err_dev_create so an unregistered PBUS mdio_device is only freed, while a
>    registered one is removed then freed. Simplify with fallthrough pattern.
>
> Changes in v3:
> - Register a dedicated mdio_device for the PBUS node (addr+8) in
>    an8811hb_probe() and store it in priv->pbusdev, replacing the
>    inline addr+8 computation in v2.
> - Change __air_pbus_reg_write() to take struct mdio_device * instead
>    of struct phy_device *.
> - Add an8811hb_remove() to unregister and free the PBUS device.
> - Fix error handling in an8811hb_probe() with goto error paths to
>    ensure the PBUS device is always cleaned up on failure.
>
> 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 | 143 +++++++++++++++++++++++++++++++++-
>   1 file changed, 139 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index 29ae73e65..07978979d 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c
> @@ -170,9 +170,21 @@
>   #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
>   
> +#define EN8811H_PBUS_ADDR_OFFS	8
> +
>   /* Default LED setup:
>    * GPIO5 <-> LED0  On: Link detected, blink Rx/Tx
>    * GPIO4 <-> LED1  On: Link detected at 2500 or 1000 Mbps
> @@ -201,6 +213,7 @@ struct en8811h_priv {
>   	struct clk_hw		hw;
>   	struct phy_device	*phydev;
>   	unsigned int		cko_is_enabled;
> +	struct mdio_device	*pbusdev;
>   };
>   
>   enum {
> @@ -254,6 +267,31 @@ 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 mdio_device *mdiodev,
> +				u32 pbus_reg, u32 pbus_data)
> +{
> +	int ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_EXT_PAGE_ACCESS,
> +			      upper_16_bits(pbus_reg));
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_ADDR_HIGH,
> +			      (pbus_reg & GENMASK(15, 6)) >> 6);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr,
> +			      (pbus_reg & GENMASK(5, 2)) >> 2,
> +			      lower_16_bits(pbus_data));
> +	if (ret < 0)
> +		return ret;
> +
> +	return __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_DATA_HIGH,
> +			       upper_16_bits(pbus_data));
> +}
> +
>   static int __air_buckpbus_reg_write(struct phy_device *phydev,
>   				    u32 pbus_address, u32 pbus_data)
>   {
> @@ -570,10 +608,67 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
>   	return ret;
>   }
>   
> +static int an8811hb_mcu_assert(struct phy_device *phydev)
> +{
> +	struct en8811h_priv *priv = phydev->priv;
> +	int ret;
> +
> +	phy_lock_mdio_bus(phydev);
> +
> +	ret = __air_pbus_reg_write(priv->pbusdev, AN8811HB_MCU_SW_RST,
> +				   AN8811HB_MCU_SW_RST_HOLD);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = __air_pbus_reg_write(priv->pbusdev, 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)
> +{
> +	struct en8811h_priv *priv = phydev->priv;
> +	int ret;
> +
> +	phy_lock_mdio_bus(phydev);
> +
> +	ret = __air_pbus_reg_write(priv->pbusdev, AN8811HB_MCU_SW_START,
> +				   AN8811HB_MCU_SW_START_EN);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = __air_pbus_reg_write(priv->pbusdev, 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)
> @@ -1166,6 +1271,7 @@ static int en8811h_leds_setup(struct phy_device *phydev)
>   
>   static int an8811hb_probe(struct phy_device *phydev)
>   {
> +	struct mdio_device *mdiodev;
>   	struct en8811h_priv *priv;
>   	int ret;
>   
> @@ -1175,10 +1281,21 @@ static int an8811hb_probe(struct phy_device *phydev)
>   		return -ENOMEM;
>   	phydev->priv = priv;
>   
> +	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)
> +		goto err_dev_free;
> +
> +	priv->pbusdev = mdiodev;
> +
>   	ret = an8811hb_load_firmware(phydev);
>   	if (ret < 0) {
>   		phydev_err(phydev, "Load firmware failed: %d\n", ret);
> -		return ret;
> +		goto err_dev_create;
>   	}
>   
>   	en8811h_print_fw_version(phydev);
> @@ -1191,22 +1308,29 @@ static int an8811hb_probe(struct phy_device *phydev)
>   
>   	ret = en8811h_leds_setup(phydev);
>   	if (ret < 0)
> -		return ret;
> +		goto err_dev_create;
>   
>   	priv->phydev = phydev;
>   	/* Co-Clock Output */
>   	ret = an8811hb_clk_provider_setup(&phydev->mdio.dev, &priv->hw);
>   	if (ret)
> -		return ret;
> +		goto err_dev_create;
>   
>   	/* Configure led gpio pins as output */
>   	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
>   				      AN8811HB_GPIO_OUTPUT_345,
>   				      AN8811HB_GPIO_OUTPUT_345);
>   	if (ret < 0)
> -		return ret;
> +		goto err_dev_create;
>   
>   	return 0;
> +
> +err_dev_create:
> +	mdio_device_remove(mdiodev);
> +
> +err_dev_free:
> +	mdio_device_free(mdiodev);
> +	return ret;
>   }
>   
>   static int en8811h_probe(struct phy_device *phydev)
> @@ -1561,6 +1685,16 @@ static int en8811h_suspend(struct phy_device *phydev)
>   	return genphy_suspend(phydev);
>   }
>   
> +static void an8811hb_remove(struct phy_device *phydev)
> +{
> +	struct en8811h_priv *priv = phydev->priv;
> +
> +	if (priv->pbusdev) {
> +		mdio_device_remove(priv->pbusdev);
> +		mdio_device_free(priv->pbusdev);
> +	}
> +}
> +
>   static struct phy_driver en8811h_driver[] = {
>   {
>   	PHY_ID_MATCH_MODEL(EN8811H_PHY_ID),
> @@ -1587,6 +1721,7 @@ static struct phy_driver en8811h_driver[] = {
>   	PHY_ID_MATCH_MODEL(AN8811HB_PHY_ID),
>   	.name			= "Airoha AN8811HB",
>   	.probe			= an8811hb_probe,
> +	.remove			= an8811hb_remove,
>   	.get_features		= en8811h_get_features,
>   	.config_init		= an8811hb_config_init,
>   	.get_rate_matching	= en8811h_get_rate_matching,