diff mbox series

[v2,1/2] net: phy: adin: disable diag clock & disable standby mode in config_aneg

Message ID 20201022074551.11520-1-alexandru.ardelean@analog.com
State Deferred
Delegated to: David Miller
Headers show
Series [v2,1/2] net: phy: adin: disable diag clock & disable standby mode in config_aneg | expand

Checks

Context Check Description
jkicinski/stable success Stable not CCed
jkicinski/header_inline success Link
jkicinski/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
jkicinski/checkpatch success total: 0 errors, 0 warnings, 0 checks, 32 lines checked
jkicinski/verify_fixes success Link
jkicinski/kdoc success Errors and warnings before: 0 this patch: 0
jkicinski/build_32bit success Errors and warnings before: 0 this patch: 0
jkicinski/module_param success Was 0 now: 0
jkicinski/verify_signedoff success Link
jkicinski/source_inline success Was 0 now: 0
jkicinski/subject_prefix warning Target tree name not specified in the subject
jkicinski/tree_selection success Guessed tree name to be net-next
jkicinski/patch_count success Link
jkicinski/fixes_present success Link
jkicinski/cover_letter success Link

Commit Message

Alexandru Ardelean Oct. 22, 2020, 7:45 a.m. UTC
When the PHY powers up, the diagnostics clock isn't enabled (bit 2 in
register PHY_CTRL_1 (0x0012)).
Also, the PHY is not in standby mode, so bit 13 in PHY_CTRL_3 (0x0017) is
always set at power up.

The standby mode and the diagnostics clock are both meant to be for the
cable diagnostics feature of the PHY (in phylib this would be equivalent to
the cable-test support), and for the frame-generator feature of the PHY.

In standby mode, the PHY doesn't negotiate links or manage links.

To use the cable diagnostics/test (or frame-generator), the PHY must be
first set in standby mode, so that the link operation doesn't interfere.
Then, the diagnostics clock must be enabled.

For the cable-test feature, when the operation finishes, the PHY goes into
PHY_UP state, and the config_aneg hook is called.

For the ADIN PHY, we need to make sure that during autonegotiation
configuration/setup the PHY is removed from standby mode and the
diagnostics clock is disabled, so that normal operation is resumed.

This change does that by moving the set of the ADIN1300_LINKING_EN bit (2)
in the config_aneg (to disable standby mode).
Previously, this was set in the downshift setup, because the downshift
retry value and the ADIN1300_LINKING_EN are in the same register.

And the ADIN1300_DIAG_CLK_EN bit (13) is cleared, to disable the
diagnostics clock.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* updated title; updated description to better describe the diagnostics
  clock & standby mode, and what they mean for the PHY


 drivers/net/phy/adin.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Oct. 23, 2020, 10:43 p.m. UTC | #1
On Thu, Oct 22, 2020 at 10:45:50AM +0300, Alexandru Ardelean wrote:
> When the PHY powers up, the diagnostics clock isn't enabled (bit 2 in
> register PHY_CTRL_1 (0x0012)).
> Also, the PHY is not in standby mode, so bit 13 in PHY_CTRL_3 (0x0017) is
> always set at power up.
> 
> The standby mode and the diagnostics clock are both meant to be for the
> cable diagnostics feature of the PHY (in phylib this would be equivalent to
> the cable-test support), and for the frame-generator feature of the PHY.
> 
> In standby mode, the PHY doesn't negotiate links or manage links.
> 
> To use the cable diagnostics/test (or frame-generator), the PHY must be
> first set in standby mode, so that the link operation doesn't interfere.
> Then, the diagnostics clock must be enabled.
> 
> For the cable-test feature, when the operation finishes, the PHY goes into
> PHY_UP state, and the config_aneg hook is called.
> 
> For the ADIN PHY, we need to make sure that during autonegotiation
> configuration/setup the PHY is removed from standby mode and the
> diagnostics clock is disabled, so that normal operation is resumed.
> 
> This change does that by moving the set of the ADIN1300_LINKING_EN bit (2)
> in the config_aneg (to disable standby mode).
> Previously, this was set in the downshift setup, because the downshift
> retry value and the ADIN1300_LINKING_EN are in the same register.
> 
> And the ADIN1300_DIAG_CLK_EN bit (13) is cleared, to disable the
> diagnostics clock.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Alexandru Ardelean Oct. 31, 2020, 12:16 p.m. UTC | #2
On Sat, Oct 24, 2020 at 1:43 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Oct 22, 2020 at 10:45:50AM +0300, Alexandru Ardelean wrote:
> > When the PHY powers up, the diagnostics clock isn't enabled (bit 2 in
> > register PHY_CTRL_1 (0x0012)).
> > Also, the PHY is not in standby mode, so bit 13 in PHY_CTRL_3 (0x0017) is
> > always set at power up.
> >
> > The standby mode and the diagnostics clock are both meant to be for the
> > cable diagnostics feature of the PHY (in phylib this would be equivalent to
> > the cable-test support), and for the frame-generator feature of the PHY.
> >
> > In standby mode, the PHY doesn't negotiate links or manage links.
> >
> > To use the cable diagnostics/test (or frame-generator), the PHY must be
> > first set in standby mode, so that the link operation doesn't interfere.
> > Then, the diagnostics clock must be enabled.
> >
> > For the cable-test feature, when the operation finishes, the PHY goes into
> > PHY_UP state, and the config_aneg hook is called.
> >
> > For the ADIN PHY, we need to make sure that during autonegotiation
> > configuration/setup the PHY is removed from standby mode and the
> > diagnostics clock is disabled, so that normal operation is resumed.
> >
> > This change does that by moving the set of the ADIN1300_LINKING_EN bit (2)
> > in the config_aneg (to disable standby mode).
> > Previously, this was set in the downshift setup, because the downshift
> > retry value and the ADIN1300_LINKING_EN are in the same register.
> >
> > And the ADIN1300_DIAG_CLK_EN bit (13) is cleared, to disable the
> > diagnostics clock.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

So, then re-send for this or just this patch ping?
Naturally, this is for net-next.
I don't mind doing either way.

Thanks
Alex

>
>     Andrew
Andrew Lunn Oct. 31, 2020, 2:36 p.m. UTC | #3
> So, then re-send for this or just this patch ping?
> Naturally, this is for net-next.
> I don't mind doing either way.

Please resend, with all the acked-by, reviewed-by added, now that
net-next is open.

	 Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 5bc3926c52f0..619d36685b5d 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -23,6 +23,7 @@ 
 #define ADIN1300_PHY_CTRL1			0x0012
 #define   ADIN1300_AUTO_MDI_EN			BIT(10)
 #define   ADIN1300_MAN_MDIX_EN			BIT(9)
+#define   ADIN1300_DIAG_CLK_EN			BIT(2)
 
 #define ADIN1300_RX_ERR_CNT			0x0014
 
@@ -326,10 +327,9 @@  static int adin_set_downshift(struct phy_device *phydev, u8 cnt)
 		return -E2BIG;
 
 	val = FIELD_PREP(ADIN1300_DOWNSPEED_RETRIES_MSK, cnt);
-	val |= ADIN1300_LINKING_EN;
 
 	rc = phy_modify(phydev, ADIN1300_PHY_CTRL3,
-			ADIN1300_LINKING_EN | ADIN1300_DOWNSPEED_RETRIES_MSK,
+			ADIN1300_DOWNSPEED_RETRIES_MSK,
 			val);
 	if (rc < 0)
 		return rc;
@@ -560,6 +560,14 @@  static int adin_config_aneg(struct phy_device *phydev)
 {
 	int ret;
 
+	ret = phy_clear_bits(phydev, ADIN1300_PHY_CTRL1, ADIN1300_DIAG_CLK_EN);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_set_bits(phydev, ADIN1300_PHY_CTRL3, ADIN1300_LINKING_EN);
+	if (ret < 0)
+		return ret;
+
 	ret = adin_config_mdix(phydev);
 	if (ret)
 		return ret;