| Message ID | 20211008233426.1088-1-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] drivers: net: phy: at803x: fix resume for QCA8327 phy | expand |
| Context | Check | Description |
|---|---|---|
| netdev/apply | fail | Patch does not apply to net |
| netdev/tree_selection | success | Clearly marked for net |
On Sat, 9 Oct 2021 01:34:25 +0200 Ansuel Smith wrote: > From Documentation phy resume triggers phy reset and restart > auto-negotiation. Add a dedicated function to wait reset to finish as > it was notice a regression where port sometime are not reliable after a > suspend/resume session. The reset wait logic is copied from phy_poll_reset. > Add dedicated suspend function to use genphy_suspend only with QCA8337 > phy and set only additional debug settings for QCA8327. With more test > it was reported that QCA8327 doesn't proprely support this mode and > using this cause the unreliability of the switch ports, especially the > malfunction of the port0. > > Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy") Hm, there's some confusion here. This commit does not exist in net, and neither does the one from patch 2. We should be fine with these going into net-next, right Andrew?
On Fri, Oct 08, 2021 at 04:47:50PM -0700, Jakub Kicinski wrote: > On Sat, 9 Oct 2021 01:34:25 +0200 Ansuel Smith wrote: > > From Documentation phy resume triggers phy reset and restart > > auto-negotiation. Add a dedicated function to wait reset to finish as > > it was notice a regression where port sometime are not reliable after a > > suspend/resume session. The reset wait logic is copied from phy_poll_reset. > > Add dedicated suspend function to use genphy_suspend only with QCA8337 > > phy and set only additional debug settings for QCA8327. With more test > > it was reported that QCA8327 doesn't proprely support this mode and > > using this cause the unreliability of the switch ports, especially the > > malfunction of the port0. > > > > Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy") > > Hm, there's some confusion here. This commit does not exist in net, > and neither does the one from patch 2. > > We should be fine with these going into net-next, right Andrew? Took the hash from linux-next. Think this is the reason they are not in net? -- Ansuel
On Sat, 9 Oct 2021 01:50:21 +0200 Ansuel Smith wrote: > On Fri, Oct 08, 2021 at 04:47:50PM -0700, Jakub Kicinski wrote: > > On Sat, 9 Oct 2021 01:34:25 +0200 Ansuel Smith wrote: > > > From Documentation phy resume triggers phy reset and restart > > > auto-negotiation. Add a dedicated function to wait reset to finish as > > > it was notice a regression where port sometime are not reliable after a > > > suspend/resume session. The reset wait logic is copied from phy_poll_reset. > > > Add dedicated suspend function to use genphy_suspend only with QCA8337 > > > phy and set only additional debug settings for QCA8327. With more test > > > it was reported that QCA8327 doesn't proprely support this mode and > > > using this cause the unreliability of the switch ports, especially the > > > malfunction of the port0. > > > > > > Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy") > > > > Hm, there's some confusion here. This commit does not exist in net, > > and neither does the one from patch 2. > > > > We should be fine with these going into net-next, right Andrew? > > Took the hash from linux-next. Think this is the reason they are not in > net? Yup, just to be sure you understand the process please take a look at - How do the changes posted to netdev make their way into Linux? - How often do changes from these trees make it to the mainline Linus tree? here: https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-the-changes-posted-to-netdev-make-their-way-into-linux But yeah, I think we can go back to posting all 15 patches as one series. Let's see if Andrew has any feedback on the v2. Sorry for the confusion!
On Fri, Oct 08, 2021 at 05:13:55PM -0700, Jakub Kicinski wrote: > On Sat, 9 Oct 2021 01:50:21 +0200 Ansuel Smith wrote: > > On Fri, Oct 08, 2021 at 04:47:50PM -0700, Jakub Kicinski wrote: > > > On Sat, 9 Oct 2021 01:34:25 +0200 Ansuel Smith wrote: > > > > From Documentation phy resume triggers phy reset and restart > > > > auto-negotiation. Add a dedicated function to wait reset to finish as > > > > it was notice a regression where port sometime are not reliable after a > > > > suspend/resume session. The reset wait logic is copied from phy_poll_reset. > > > > Add dedicated suspend function to use genphy_suspend only with QCA8337 > > > > phy and set only additional debug settings for QCA8327. With more test > > > > it was reported that QCA8327 doesn't proprely support this mode and > > > > using this cause the unreliability of the switch ports, especially the > > > > malfunction of the port0. > > > > > > > > Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy") > > > > > > Hm, there's some confusion here. This commit does not exist in net, > > > and neither does the one from patch 2. > > > > > > We should be fine with these going into net-next, right Andrew? > > > > Took the hash from linux-next. Think this is the reason they are not in > > net? > > Yup, just to be sure you understand the process please take a look at > > - How do the changes posted to netdev make their way into Linux? > - How often do changes from these trees make it to the mainline Linus > tree? > > here: > > https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-the-changes-posted-to-netdev-make-their-way-into-linux > > But yeah, I think we can go back to posting all 15 patches as one > series. Let's see if Andrew has any feedback on the v2. > > Sorry for the confusion! It's ok. We got all confused with the Fixes tag. Pushing stuff too quickly... I should have notice they were not present in net and reporting that. Sorry for the mess.
> It's ok. We got all confused with the Fixes tag. Pushing stuff too > quickly... I should have notice they were not present in net and > reporting that. Sorry for the mess. It would still be good to post those fixes separate from the rest of the series. I think the DT binding will need more discussion. Andrew
On Sat, Oct 09, 2021 at 04:58:55PM +0200, Andrew Lunn wrote: > > It's ok. We got all confused with the Fixes tag. Pushing stuff too > > quickly... I should have notice they were not present in net and > > reporting that. Sorry for the mess. > > It would still be good to post those fixes separate from the rest of > the series. I think the DT binding will need more discussion. > > Andrew Considering the other phy changes are OK. Can I post the 4 phy patch to net-next as an unique series? (so split qca8k and phy patches)
On Sat, Oct 09, 2021 at 05:01:28PM +0200, Ansuel Smith wrote: > On Sat, Oct 09, 2021 at 04:58:55PM +0200, Andrew Lunn wrote: > > > It's ok. We got all confused with the Fixes tag. Pushing stuff too > > > quickly... I should have notice they were not present in net and > > > reporting that. Sorry for the mess. > > > > It would still be good to post those fixes separate from the rest of > > the series. I think the DT binding will need more discussion. > > > > Andrew > > Considering the other phy changes are OK. Can I post the 4 phy patch > to net-next as an unique series? (so split qca8k and phy patches) Yes, that is fine. Andrew
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 3feee4d59030..c6c87b82c95c 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -92,9 +92,14 @@ #define AT803X_DEBUG_REG_5 0x05 #define AT803X_DEBUG_TX_CLK_DLY_EN BIT(8) +#define AT803X_DEBUG_REG_HIB_CTRL 0x0b +#define AT803X_DEBUG_HIB_CTRL_SEL_RST_80U BIT(10) +#define AT803X_DEBUG_HIB_CTRL_EN_ANY_CHANGE BIT(13) + #define AT803X_DEBUG_REG_3C 0x3C #define AT803X_DEBUG_REG_3D 0x3D +#define AT803X_DEBUG_GATE_CLK_IN1000 BIT(6) #define AT803X_DEBUG_REG_1F 0x1F #define AT803X_DEBUG_PLL_ON BIT(2) @@ -1312,6 +1317,58 @@ static int qca83xx_config_init(struct phy_device *phydev) return 0; } +static int qca83xx_resume(struct phy_device *phydev) +{ + int ret, val; + + /* Skip reset if not suspended */ + if (!phydev->suspended) + return 0; + + /* Reinit the port, reset values set by suspend */ + qca83xx_config_init(phydev); + + /* Reset the port on port resume */ + phy_set_bits(phydev, MII_BMCR, BMCR_RESET | BMCR_ANENABLE); + + /* On resume from suspend the switch execute a reset and + * restart auto-negotiation. Wait for reset to complete. + */ + ret = phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET), + 50000, 600000, true); + if (ret) + return ret; + + msleep(1); + + return 0; +} + +static int qca83xx_suspend(struct phy_device *phydev) +{ + u16 mask = 0; + + /* Only QCA8337 support actual suspend. + * QCA8327 cause port unreliability when phy suspend + * is set. + */ + if (phydev->drv->phy_id == QCA8337_PHY_ID) { + genphy_suspend(phydev); + } else { + mask |= ~(BMCR_SPEED1000 | BMCR_FULLDPLX); + phy_modify(phydev, MII_BMCR, mask, 0); + } + + at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_3D, + AT803X_DEBUG_GATE_CLK_IN1000, 0); + + at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL, + AT803X_DEBUG_HIB_CTRL_EN_ANY_CHANGE | + AT803X_DEBUG_HIB_CTRL_SEL_RST_80U, 0); + + return 0; +} + static struct phy_driver at803x_driver[] = { { /* Qualcomm Atheros AR8035 */ @@ -1421,8 +1478,8 @@ static struct phy_driver at803x_driver[] = { .get_sset_count = at803x_get_sset_count, .get_strings = at803x_get_strings, .get_stats = at803x_get_stats, - .suspend = genphy_suspend, - .resume = genphy_resume, + .suspend = qca83xx_suspend, + .resume = qca83xx_resume, }, { /* QCA8327-A from switch QCA8327-AL1A */ .phy_id = QCA8327_A_PHY_ID, @@ -1436,8 +1493,8 @@ static struct phy_driver at803x_driver[] = { .get_sset_count = at803x_get_sset_count, .get_strings = at803x_get_strings, .get_stats = at803x_get_stats, - .suspend = genphy_suspend, - .resume = genphy_resume, + .suspend = qca83xx_suspend, + .resume = qca83xx_resume, }, { /* QCA8327-B from switch QCA8327-BL1A */ .phy_id = QCA8327_B_PHY_ID, @@ -1451,8 +1508,8 @@ static struct phy_driver at803x_driver[] = { .get_sset_count = at803x_get_sset_count, .get_strings = at803x_get_strings, .get_stats = at803x_get_stats, - .suspend = genphy_suspend, - .resume = genphy_resume, + .suspend = qca83xx_suspend, + .resume = qca83xx_resume, }, }; module_phy_driver(at803x_driver);
From Documentation phy resume triggers phy reset and restart auto-negotiation. Add a dedicated function to wait reset to finish as it was notice a regression where port sometime are not reliable after a suspend/resume session. The reset wait logic is copied from phy_poll_reset. Add dedicated suspend function to use genphy_suspend only with QCA8337 phy and set only additional debug settings for QCA8327. With more test it was reported that QCA8327 doesn't proprely support this mode and using this cause the unreliability of the switch ports, especially the malfunction of the port0. Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy") Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> --- drivers/net/phy/at803x.c | 69 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-)