Re: [PATCH net-next v4 5/6] net: phy: mediatek: add calibration logic for AN7581

[Christian Marangi <ansuelsmth@gmail.com>]

On Thu, Nov 05, 2026 at 11:47:26PM +0100, Andrew Lunn wrote:
> > +static int airoha_cal_cycle(struct phy_device *phydev, int devad,
> > +			    u32 regnum, u16 mask, u16 cal_val)
> > +{
> > +	struct airoha_socphy_shared *shared = phy_package_get_priv(phydev);
> > +	struct phy_device *phydev_p0;
> > +	int ret;
> > +
> > +	phydev_p0 = shared->phydev_p0;
> > +
> > +	phy_modify_mmd(phydev, devad, regnum, mask, cal_val);
> > +
> > +	ret = mtk_cal_cycle_wait(phydev_p0);
> 
> 
> > +static int airoha_rext_cal_sw(struct phy_device *phydev)
> > +{
> 
> ...
> 
> > +	phydev_dbg(phydev, "Start REXT SW cal.\n");
> > +	first_calib = airoha_cal_cycle(phydev, MDIO_MMD_VEND1, MTK_PHY_RG_ANA_CAL_RG5,
> > +				       MTK_PHY_RG_REXT_ZCAL_CTRL_MASK, zcal_ctrl);
> > +
> 
> > +static int airoha_cal_sw(struct phy_device *phydev, enum CAL_ITEM cal_item,
> > +			 u8 start_pair, u8 end_pair)
> > +{
> > +	u8 pair_n;
> > +	int ret;
> > +
> > +	for (pair_n = start_pair; pair_n <= end_pair; pair_n++) {
> > +		switch (cal_item) {
> > +		case REXT:
> > +			ret = airoha_rext_cal_sw(phydev);
> > +			break;
> 
> > +static int airoha_start_cal(struct phy_device *phydev, enum CAL_ITEM cal_item,
> > +			    enum CAL_MODE cal_mode, u8 start_pair,
> > +			    u8 end_pair, u32 *buf)
> > +{
> > +	int ret;
> > +
> > +	switch (cal_mode) {
> > +	case SW_M:
> > +		ret = airoha_cal_sw(phydev, cal_item, start_pair, end_pair);
> 
> 
> > +static int airoha_phy_calib(struct phy_device *phydev)
> > +{
> > +	phy_package_lock(phydev);
> > +	if (!shared->rext_sw_calib_done) {
> > +		ret = airoha_start_cal(phydev, REXT, SW_M, NO_PAIR, NO_PAIR, NULL);
> > +		if (ret) {
> > +			phy_package_unlock(phydev);
> > +			return ret;
> > +		}
> > +
> > +		shared->rext_sw_calib_done = true;
> > +	}
> > +	phy_package_unlock(phydev);
> > +
> > +	ret = airoha_start_cal(phydev, TX_R50, SW_M, PAIR_A, PAIR_D, NULL);
> 
> 
> > +static int an7581_phy_config_init(struct phy_device *phydev)
> > +{
> > +	struct airoha_socphy_shared *shared = phy_package_get_priv(phydev);
> > +	struct phy_device *phydev_p0;
> > +	u8 phy_offset;
> > +	int oldpage;
> > +	int ret;
> > +
> > +	phydev_p0 = shared->phydev_p0;
> > +	phy_offset = phydev->mdio.addr - phydev_p0->mdio.addr;
> > +
> > +	if (phy_package_init_once(phydev)) {
> > +		ret = an7581_phy_package_config_init_once(phydev);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	ret = airoha_phy_calib(phydev);
> 
> > @@ -88,6 +1610,7 @@ static struct phy_driver mtk_socphy_driver[] = {
> >  	{
> >  		PHY_ID_MATCH_EXACT(AIROHA_GPHY_ID_AN7581),
> >  		.name		= "Airoha AN7581 PHY",
> > +		.config_init	= an7581_phy_config_init,
> 
> > > There are 4 phys in total, only the phy with mdio address 9 has the
> > > calibration capability. Thus only one phy at the time can perform
> > > the calibration sequence.
> 
> config_init -> airoha_phy_calib -> airoha_start_cal -> airoha_cal_sw
> -> airoha_cal_cycle
> 
> I cannot say i looked at all the details closely, maybe i missed an ==
> 9 somewhere, but to me, it looks like two config_init can run in
> parallel.
> 

Hi Andrew, in v5 I converted the driver to full phy_package_lock/unlock and
I set the base address correctly to the hardcoded value of the SoC.

This permitted me to simplify the validation code instead of saving the
phydev for port0 in the phy_package struct (a much solid implementation in
short)

-- 
	Ansuel