[PATCH (net.git)] net: phy: at803x: disable by default the hibernation feature

[PATCH (net.git)] net: phy: at803x: disable by default the hibernation feature

[Giuseppe Cavallaro]

These PHY chips, by default, enable the hibernation feature
so, if the cable is unplugged the device enters in hibernation
mode after some time. This can generate problems on some cases.
It has been noticed, on some platforms that, if the phy enters
in hibernation, the missing of the rx clock signal can force
a mac to fail when setup some parts that need to be properly
clocked.
For example, while booting a Kernel the SYNP MAC (stmmac) fails
to initialize own DMA engine if the phy entered in hibernation
before.

So, the patch just disables this feature by default when init
the PHY driver.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Matus Ujhelyi <ujhelyi.m@gmail.com>
---
 drivers/net/phy/at803x.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index f279a89..d60953e 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -52,6 +52,9 @@
 #define AT803X_DEBUG_REG_5			0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
+#define	AT803X_DEBUG_REG_B			0x0B
+#define	AT803X_DEBUG_PS_HIB_EN			BIT(15)
+
 #define AT803X_REG_CHIP_CONFIG			0x1f
 #define AT803X_BT_BX_REG_SEL			0x8000
 
@@ -117,6 +120,12 @@ static inline int at803x_enable_tx_delay(struct phy_device *phydev)
 					AT803X_DEBUG_TX_CLK_DLY_EN);
 }
 
+static inline int at803x_disable_hibernation(struct phy_device *phydev)
+{
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_B,
+				     AT803X_DEBUG_PS_HIB_EN, 0);
+}
+
 /* save relevant PHY registers to private copy */
 static void at803x_context_save(struct phy_device *phydev,
 				struct at803x_context *context)
@@ -314,6 +323,9 @@ static int at803x_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	/* By default disable the Power Hibernation feature */
+	at803x_disable_hibernation(phydev);
+
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH (net.git)] net: phy: at803x: disable by default the hibernation feature

[Andrew Lunn]

> For example, while booting a Kernel the SYNP MAC (stmmac) fails
> to initialize own DMA engine if the phy entered in hibernation
> before.

Have you tried fixing stmmac instead?

     Andrew

Re: [PATCH (net.git)] net: phy: at803x: disable by default the hibernation feature

[Giuseppe CAVALLARO]

Hello Andrew.

On 10/25/2016 11:00 AM, Andrew Lunn wrote:
>> For example, while booting a Kernel the SYNP MAC (stmmac) fails
>> to initialize own DMA engine if the phy entered in hibernation
>> before.
>
> Have you tried fixing stmmac instead?

Let me describe better what happens, to be honest, this is a
marginal user-case, but, maybe it makes sense to share this patch in
case of somebody meets the same issue.

When performing "ifconfig eth0 up", if this phy is not in hibernation,
the iface comes up w/o any issues.
If the PHY is in hibernation, because the cable is unplugged (and
this is a default for these transceivers), the phy clock does down and
the MAC cannot init own DMA. The stmmac is designed to fail the open in
this case.
If I plug the cable the next ifconfig up is ok.

The meaning of the patch, I proposed, is to remove by default this
hibernation feature at PHY level that, for me, should be an option
not a default. For example, I have used other HW where some
power state features could be enabled but, by default, were turned
off. Also these transceivers support EEE so, I guess, there is all the
technology to manage the power consumption on new setup.

Concerning the stmmac, how the driver could fix this situation?
The PHY does not provide the clock required for GMAC and the stmmac
cannot reset own DMA. I had thought to delay this as soon as the link
is UP but I don't like this approach where the open should return
a sane state but this is not true and we should wait the ACK from the
PHY to reset the MAC DMAC.

Anyway, as said, the patch covers a marginal user-case so feel free to
consider it or not. For sure, I am open to change something at MAC
level if you have better idea.

Regards
Peppe

>
>      Andrew
>

Re: [PATCH (net.git)] net: phy: at803x: disable by default the hibernation feature

[David Miller]

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Tue, 25 Oct 2016 10:31:22 +0200

> These PHY chips, by default, enable the hibernation feature
> so, if the cable is unplugged the device enters in hibernation
> mode after some time. This can generate problems on some cases.
> It has been noticed, on some platforms that, if the phy enters
> in hibernation, the missing of the rx clock signal can force
> a mac to fail when setup some parts that need to be properly
> clocked.
> For example, while booting a Kernel the SYNP MAC (stmmac) fails
> to initialize own DMA engine if the phy entered in hibernation
> before.
> 
> So, the patch just disables this feature by default when init
> the PHY driver.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Matus Ujhelyi <ujhelyi.m@gmail.com>

Like Andrew, I think this is attacking the problem from the wrong
side.

The stmmac driver, if it needs the proper PHY RX clocks to initialize
properly, should do the work to make sure it is there.

If this means forcing the PHY out of hibernation mode, so that it
can initialize properly, this is what the stammc can do.

After initialization, it can allow the chip to fall back into
hibernation mode again if the cable is still unplugged.

Disabling a whole feature of the PHY instead doesn't make a lot of
sense when it certainly seems like stmmac can do what it needs to do
in the presence of this feature being enabled.