[PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[Marco Felsch]

From: Markus Niebel <Markus.Niebel@tqs.de>

handle errata #2 for KSZ9031: force 1000Base-T master

Attention: enabling the workaround will cause no link to
other GIGE master.

Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
[m.felsch@pengutronix.de: move dt binding to the KSZ9031 entry]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../devicetree/bindings/net/micrel-ksz90x1.txt |  3 +++
 drivers/net/phy/micrel.c                       | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
index 42a248301615..e2465fbbbcef 100644
--- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
@@ -57,6 +57,9 @@ KSZ9031:
       - txd2-skew-ps : Skew control of TX data 2 pad
       - txd3-skew-ps : Skew control of TX data 3 pad
 
+    - force-master: Boolean, force phy to master mode. This is a
+                    workaround at least for KSZ9031 errata #2.
+
 Examples:
 
 	mdio {
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f41b224a9cdb..3e4243e4f7a7 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -528,6 +528,8 @@ static int ksz9031_enable_edpd(struct phy_device *phydev)
 
 static int ksz9031_config_init(struct phy_device *phydev)
 {
+	int rc;
+	u16 val;
 	const struct device *dev = &phydev->mdio.dev;
 	const struct device_node *of_node = dev->of_node;
 	static const char *clk_skews[2] = {"rxc-skew-ps", "txc-skew-ps"};
@@ -573,6 +575,22 @@ static int ksz9031_config_init(struct phy_device *phydev)
 		ksz9031_of_load_skew_values(phydev, of_node,
 				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
 				tx_data_skews, 4);
+
+		/* force master mode -> errata #2
+		 * attention: Master <-> Master will not work
+		 */
+		if (of_property_read_bool(of_node, "force-master")) {
+			rc = phy_read(phydev, MII_CTRL1000);
+			if (rc >= 0) {
+				val = (u16)rc;
+				/* enable master mode, config &
+				 * prefer master
+				 */
+				val |= (CTL1000_ENABLE_MASTER |
+					CTL1000_AS_MASTER);
+				phy_write(phydev, MII_CTRL1000, val);
+			}
+		}
 	}
 
 	return ksz9031_center_flp_timing(phydev);
-- 
2.17.0

Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[Uwe Kleine-König]

Hello,

> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index f41b224a9cdb..3e4243e4f7a7 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -528,6 +528,8 @@ static int ksz9031_enable_edpd(struct phy_device *phydev)
>  
>  static int ksz9031_config_init(struct phy_device *phydev)
>  {
> +	int rc;
> +	u16 val;

These could be declared in more local scope.

>  	const struct device *dev = &phydev->mdio.dev;
>  	const struct device_node *of_node = dev->of_node;
>  	static const char *clk_skews[2] = {"rxc-skew-ps", "txc-skew-ps"};
> @@ -573,6 +575,22 @@ static int ksz9031_config_init(struct phy_device *phydev)
>  		ksz9031_of_load_skew_values(phydev, of_node,
>  				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>  				tx_data_skews, 4);
> +
> +		/* force master mode -> errata #2
> +		 * attention: Master <-> Master will not work
> +		 */
> +		if (of_property_read_bool(of_node, "force-master")) {
> +			rc = phy_read(phydev, MII_CTRL1000);
> +			if (rc >= 0) {
> +				val = (u16)rc;
> +				/* enable master mode, config &
> +				 * prefer master
> +				 */
> +				val |= (CTL1000_ENABLE_MASTER |
> +					CTL1000_AS_MASTER);
> +				phy_write(phydev, MII_CTRL1000, val);

There is no need for two variables, just use a single int variable that
on all Linux archs can hold the whole domain of u16 should be fine.

> +			}
> +		}

Maybe issue a warning if reading MII_CTRL1000 failed?

>  	}
>  
>  	return ksz9031_center_flp_timing(phydev);

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[Sergei Shtylyov]

Hello!

On 5/14/2018 11:22 AM, Marco Felsch wrote:

> From: Markus Niebel <Markus.Niebel@tqs.de>
> 
> handle errata #2 for KSZ9031: force 1000Base-T master
> 
> Attention: enabling the workaround will cause no link to
> other GIGE master.
> 
> Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
> [m.felsch@pengutronix.de: move dt binding to the KSZ9031 entry]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
[...]
> @@ -573,6 +575,22 @@ static int ksz9031_config_init(struct phy_device *phydev)
>   		ksz9031_of_load_skew_values(phydev, of_node,
>   				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>   				tx_data_skews, 4);
> +
> +		/* force master mode -> errata #2
> +		 * attention: Master <-> Master will not work
> +		 */
> +		if (of_property_read_bool(of_node, "force-master")) {
> +			rc = phy_read(phydev, MII_CTRL1000);
> +			if (rc >= 0) {
> +				val = (u16)rc;
> +				/* enable master mode, config &
> +				 * prefer master
> +				 */
> +				val |= (CTL1000_ENABLE_MASTER |
> +					CTL1000_AS_MASTER);

   Parens not needed.

> +				phy_write(phydev, MII_CTRL1000, val);
> +			}
> +		}
>   	}
>   
>   	return ksz9031_center_flp_timing(phydev);
> 

MBR, Sergei

Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[Sergei Shtylyov]

On 5/14/2018 11:22 AM, Marco Felsch wrote:

> From: Markus Niebel <Markus.Niebel@tqs.de>
> 
> handle errata #2 for KSZ9031: force 1000Base-T master
> 
> Attention: enabling the workaround will cause no link to
> other GIGE master.
> 
> Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
> [m.felsch@pengutronix.de: move dt binding to the KSZ9031 entry]
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>   .../devicetree/bindings/net/micrel-ksz90x1.txt |  3 +++
>   drivers/net/phy/micrel.c                       | 18 ++++++++++++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> index 42a248301615..e2465fbbbcef 100644
> --- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> +++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
> @@ -57,6 +57,9 @@ KSZ9031:
>         - txd2-skew-ps : Skew control of TX data 2 pad
>         - txd3-skew-ps : Skew control of TX data 3 pad
>   
> +    - force-master: Boolean, force phy to master mode. This is a

    Not "micrel,force-master"?

> +                    workaround at least for KSZ9031 errata #2.
> +
>   Examples:
>   
>   	mdio {
[...]

MBR, Sergei

Re: [PATCH] net: phy: micrel: workaround for errata #2 for KSZ9031

[Andrew Lunn]

On Mon, May 14, 2018 at 10:22:18AM +0200, Marco Felsch wrote:
> From: Markus Niebel <Markus.Niebel@tqs.de>
> 
> handle errata #2 for KSZ9031: force 1000Base-T master
> 
> Attention: enabling the workaround will cause no link to
> other GIGE master.

Hi Marco

It would be good to document why this is needed, when to use it, what
problems it fixes, etc. At the moment, there is not enough information
to decide if you should turn it on.

   Andrew