[PATCH RFC net-next 0/3] net: phy: Read if PHY can stop its clock

[PATCH RFC net-next 0/3] net: phy: Read if PHY can stop its clock

[Florian Fainelli]

This patch series update PHYLIB to read whether the PHY is actually capable of
stopping its clocks during EEE low power modes.

One problem (not the only one) with phy_init_eee() is that it takes a
clk_stop_enable argument that the caller has no idea how to determine.

This patch series makes the PHY library read whether the PHY is capable of
stopping its clock (after config_init has been called) which will allow
future patches to rename clk_stop_enable into something that conveys an
intention (and therefore could fail if the PHY does not support it).

Florian Fainelli (3):
  net: mdio: add definition for MDIO_STAT1_CLOCK_STOP_CAPABLE
  net: phy: read whether PHY supports stopping clock during LPI
  net: phy: stop the PHY clock during LPI only if supported

 drivers/net/phy/phy.c        |  2 +-
 drivers/net/phy/phy_device.c | 23 ++++++++++++++++++++++-
 include/linux/phy.h          |  2 ++
 include/uapi/linux/mdio.h    |  1 +
 4 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.9.3

[PATCH RFC net-next 1/3] net: mdio: add definition for MDIO_STAT1_CLOCK_STOP_CAPABLE

[Florian Fainelli]

Add the definition for the Clause 45 IEEE PCS Status 1 Register (3.1)
reporting whether a PHY supports stopping its clock or not during LPI
(EEE).

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/uapi/linux/mdio.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index c94a510a577e..4f17427db4cd 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -96,6 +96,7 @@
 #define MDIO_STAT1_LPOWERABLE		0x0002	/* Low-power ability */
 #define MDIO_STAT1_LSTATUS		BMSR_LSTATUS
 #define MDIO_STAT1_FAULT		0x0080	/* Fault */
+#define MDIO_STAT1_CLOCK_STOP_CAPABLE	0x0040	/* Clock stop capable */
 #define MDIO_AN_STAT1_LPABLE		0x0001	/* Link partner AN ability */
 #define MDIO_AN_STAT1_ABLE		BMSR_ANEGCAPABLE
 #define MDIO_AN_STAT1_RFAULT		BMSR_RFAULT
-- 
2.9.3

[PATCH RFC net-next 2/3] net: phy: read whether PHY supports stopping clock during LPI

[Florian Fainelli]

In order to use phy_init_eee() correctly, in particular the clk_stop
argument, we need to know whether the Ethernet PHY supports stopping its
clock.

Right now, we would have to call phy_init_eee(phydev, 1), see if that
tails, and call again with phy_init_eee(phydev, 0) to enable EEE this is
not an acceptable API use.

Update phy_init_hw() to read whether the PHY supports this, and retain
that information in the phydev structure so we can re-use it later.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy_device.c | 23 ++++++++++++++++++++++-
 include/linux/phy.h          |  2 ++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1219eeab69d1..2755d77626f7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -837,6 +837,21 @@ static int phy_poll_reset(struct phy_device *phydev)
 	return 0;
 }
 
+static void phy_read_clock_stop_capable(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Read if the PHY supports stopping its clocks (reg 3.1) */
+	ret = phy_read_mmd_indirect(phydev, MDIO_MMD_PCS, MDIO_STAT1,
+			            phydev->addr);
+	if (ret < 0)
+		return;
+
+	/* Do not make this fatal */
+	if (ret & MDIO_STAT1_CLOCK_STOP_CAPABLE)
+		phydev->clk_stop_cap = true;
+}
+
 int phy_init_hw(struct phy_device *phydev)
 {
 	int ret = 0;
@@ -856,7 +871,13 @@ int phy_init_hw(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	return phydev->drv->config_init(phydev);
+	ret = phydev->drv->config_init(phydev);
+	if (ret < 0)
+		return ret;
+
+	phy_read_clock_stop_capable(phydev);
+
+	return 0;
 }
 EXPORT_SYMBOL(phy_init_hw);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 624cecf69c28..c61fd519f341 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -357,6 +357,7 @@ struct phy_c45_device_ids {
  * is_pseudo_fixed_link: Set to true if this phy is an Ethernet switch, etc.
  * has_fixups: Set to true if this phy has fixups/quirks.
  * suspended: Set to true if this phy has been suspended successfully.
+ * clk_stop_cap: Set to true if this phy supports TX clock stopping during EEE.
  * state: state of the PHY for management purposes
  * dev_flags: Device-specific flags used by the PHY driver.
  * link_timeout: The number of timer firings to wait before the
@@ -393,6 +394,7 @@ struct phy_device {
 	bool is_pseudo_fixed_link;
 	bool has_fixups;
 	bool suspended;
+	bool clk_stop_cap;
 
 	enum phy_state state;
 
-- 
2.9.3

[PATCH RFC net-next 3/3] net: phy: stop the PHY clock during LPI only if supported

[Florian Fainelli]

Now that we detect whether a PHY supports stopping its clock during LPI,
deny a call to phy_init_eee() with clk_stop_enable being set and the PHY
not supporting that.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index ba4676ee9018..1c3800e01d82 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1251,7 +1251,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 		if (!phy_check_valid(phydev->speed, phydev->duplex, lp & adv))
 			goto eee_exit_err;
 
-		if (clk_stop_enable) {
+		if (clk_stop_enable && phydev->clk_stop_cap) {
 			/* Configure the PHY to stop receiving xMII
 			 * clock while it is signaling LPI.
 			 */
-- 
2.9.3

Re: [PATCH RFC net-next 2/3] net: phy: read whether PHY supports stopping clock during LPI

[Andrew Lunn]

On Mon, Mar 27, 2017 at 11:47:20AM -0700, Florian Fainelli wrote:
> In order to use phy_init_eee() correctly, in particular the clk_stop
> argument, we need to know whether the Ethernet PHY supports stopping its
> clock.
> 
> Right now, we would have to call phy_init_eee(phydev, 1), see if that
> tails, and call again with phy_init_eee(phydev, 0) to enable EEE this is
> not an acceptable API use.

Hi Florain

I'm having trouble parsing this paragraph. Should tails be fails?  I
think "This is not an acceptable API use." should be a sentence?

> 
> Update phy_init_hw() to read whether the PHY supports this, and retain
> that information in the phydev structure so we can re-use it later.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/phy/phy_device.c | 23 ++++++++++++++++++++++-
>  include/linux/phy.h          |  2 ++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1219eeab69d1..2755d77626f7 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -837,6 +837,21 @@ static int phy_poll_reset(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static void phy_read_clock_stop_capable(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	/* Read if the PHY supports stopping its clocks (reg 3.1) */
> +	ret = phy_read_mmd_indirect(phydev, MDIO_MMD_PCS, MDIO_STAT1,
> +			            phydev->addr);
> +	if (ret < 0)
> +		return;

It is pretty unusual to ignore a real error. It might be better to
make this an int function, and return the error.

     Andrew

Re: [PATCH RFC net-next 3/3] net: phy: stop the PHY clock during LPI only if supported

[Andrew Lunn]

On Mon, Mar 27, 2017 at 11:47:21AM -0700, Florian Fainelli wrote:
> Now that we detect whether a PHY supports stopping its clock during LPI,
> deny a call to phy_init_eee() with clk_stop_enable being set and the PHY
> not supporting that.

Hi Florian

We are not denying the call. This just ignores the clk_stop_enable
parameter if the clock cannot be stopped. So i think this message
could be better worded.

Maybe also update the function comment?

* and it programs the MMD register 3.0 setting the "Clock stop enable"
* bit if supported by the device.

  Andrew