On Thu, Oct 07, 2021 at 01:59:55AM +0200, Andrew Lunn wrote: > On Thu, Oct 07, 2021 at 12:35:52AM +0200, Ansuel Smith wrote: > > QCA8327 internal phy require DAC amplitude adjustement set to +6% with > > 100m speed. Also add additional define to report a change of the same > > reg in QCA8337. (different scope it does set 1000m voltage) > > Add link_change_notify function to set the proper amplitude adjustement > > on PHY_RUNNING state and disable on any other state. > > > > Fixes: c6bcec0d6928 ("net: phy: at803x: add support for qca 8327 A variant internal phy") > > Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx> > > Since this is a fix, you might want to send it on its own, based on > net. > > > + /* QCA8327 require DAC amplitude adjustment for 100m set to +6%. > > + * Disable on init and enable only with 100m speed following > > + * qca original source code. > > + */ > > + if (phydev->drv->phy_id == QCA8327_A_PHY_ID || > > + phydev->drv->phy_id == QCA8327_B_PHY_ID) > > + at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, > > + QCA8327_DEBUG_MANU_CTRL_EN, 0); > > + > > return 0; > > } > > > > +static void qca83xx_link_change_notify(struct phy_device *phydev) > > +{ > > + /* QCA8337 doesn't require DAC Amplitude adjustement */ > > + if (phydev->drv->phy_id == QCA8337_PHY_ID) > > + return; > > + > > + /* Set DAC Amplitude adjustment to +6% for 100m on link running */ > > + if (phydev->state == PHY_RUNNING) { > > + if (phydev->speed == SPEED_100) > > + at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, > > + QCA8327_DEBUG_MANU_CTRL_EN, > > + QCA8327_DEBUG_MANU_CTRL_EN); > > + } else { > > + /* Reset DAC Amplitude adjustment */ > > + at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, > > + QCA8327_DEBUG_MANU_CTRL_EN, 0); > > Here you don't make it conditional on QCA8327_A_PHY_ID and > QCA8327_B_PHY_ID, where as above you do? > > Andrew We skip the DAC Amplitude for 8337. We support 8327 a/b and 8337. Anyway sending this and other patch to v2 with the asked changes. -- Ansuel