| Message ID | 20211006223603.18858-3-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Multiple improvement for qca8337 switch | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Series has a cover letter |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/checkpatch | warning | WARNING: Unknown commit id 'c6bcec0d6928', maybe rebased or not pulled? |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
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@gmail.com> 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
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@gmail.com> > > 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.
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 9090f384c29e..71237a4e85a3 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -87,6 +87,8 @@ #define AT803X_PSSR_MR_AN_COMPLETE 0x0200 #define AT803X_DEBUG_REG_0 0x00 +#define QCA8327_DEBUG_MANU_CTRL_EN BIT(2) +#define QCA8337_DEBUG_MANU_CTRL_EN GENMASK(3, 2) #define AT803X_DEBUG_RX_CLK_DLY_EN BIT(15) #define AT803X_DEBUG_REG_5 0x05 @@ -1314,9 +1316,37 @@ static int qca83xx_config_init(struct phy_device *phydev) break; } + /* 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); + } +} + static int qca83xx_resume(struct phy_device *phydev) { int ret, val; @@ -1471,6 +1501,7 @@ static struct phy_driver at803x_driver[] = { .phy_id_mask = QCA8K_PHY_ID_MASK, .name = "Qualcomm Atheros 8337 internal PHY", /* PHY_GBIT_FEATURES */ + .link_change_notify = qca83xx_link_change_notify, .probe = at803x_probe, .flags = PHY_IS_INTERNAL, .config_init = qca83xx_config_init, @@ -1486,6 +1517,7 @@ static struct phy_driver at803x_driver[] = { .phy_id_mask = QCA8K_PHY_ID_MASK, .name = "Qualcomm Atheros 8327-A internal PHY", /* PHY_GBIT_FEATURES */ + .link_change_notify = qca83xx_link_change_notify, .probe = at803x_probe, .flags = PHY_IS_INTERNAL, .config_init = qca83xx_config_init, @@ -1501,6 +1533,7 @@ static struct phy_driver at803x_driver[] = { .phy_id_mask = QCA8K_PHY_ID_MASK, .name = "Qualcomm Atheros 8327-B internal PHY", /* PHY_GBIT_FEATURES */ + .link_change_notify = qca83xx_link_change_notify, .probe = at803x_probe, .flags = PHY_IS_INTERNAL, .config_init = qca83xx_config_init,
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@gmail.com> --- drivers/net/phy/at803x.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)