[PATCH net-next 0/2] net: phy: Add Downshift, FLF2 drivers for Microsemi

[PATCH net-next 0/2] net: phy: Add Downshift, FLF2 drivers for Microsemi

[Raju Lakkaraju]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

This series adds support to the Speed downshift, Fast Link Failure 2,
set drivers for Microsemi PHYs.

Patch 1/4: Link Speed downshift:
        For operation in cabling environments that are incompatible with
        1000BAST-T, VSC8531 device provides an automatic link speed
        downshift operation. When enabled, the device automatically changes
        its 1000BAST-T auto-negotiation to the next slower speed after
        a set number of failed attempts at 1000BAST-T.
        This feature is useful in setting up in networks using older cable
        installations that include only pairs A and B, and not pairs C and D.

Patch 2/4: Fast Link Failure 2:
        VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
        onset of a potential link failure in < 100 usec for 100BASE-TX
        operation. FLF2 is supported through the MDINT (active low) pin.

All these features tested on Beaglebone Black with VSC 8531 PHY.

Raju Lakkaraju (2):
  net: phy: Add Speed downshift set driver for Microsemi PHYs.
  net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  12 +++
 drivers/net/phy/mscc.c                             | 120 ++++++++++++++++++++-
 2 files changed, 131 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Raju Lakkaraju]

From: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>

For operation in cabling environments that are incompatible with
1000BAST-T, VSC8531 device provides an automatic link speed
downshift operation. When enabled, the device automatically changes
its 1000BAST-T auto-negotiation to the next slower speed after
a configured number of failed attempts at 1000BAST-T.
This feature is useful in setting up in networks using older cable
installations that include only pairs A and B, and not pairs C and D.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
Signed-off-by: Allan W. Nielsen <allan.nielsen-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  6 ++
 drivers/net/phy/mscc.c                             | 75 +++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index bdefefc6..062d115 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -27,6 +27,11 @@ Optional properties:
 			  'vddmac'.
 			  Default value is 0%.
 			  Ref: Table:1 - Edge rate change (below).
+- downshift-cnt		: When enabled, the device automatically changes its
+			  1000BAST-T auto-negotiation to the next slower speed
+			  after a 'downshift-cnt' of failed attempts at
+			  1000BAST-T. Allowed values: 0, 2, 3, 4, 5.
+			  0 is default and will disable downshifting.
 
 Table: 1 - Edge rate change
 ----------------------------------------------------------------|
@@ -60,4 +65,5 @@ Example:
                 compatible = "ethernet-phy-id0007.0570";
                 vsc8531,vddmac		= <3300>;
                 vsc8531,edge-slowdown	= <7>;
+                vsc8531,downshift-cnt   = <3>;
         };
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 43a7545..e87d9f0 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -46,8 +46,15 @@ enum rgmii_rx_clock_delay {
 
 #define MSCC_EXT_PAGE_ACCESS		  31
 #define MSCC_PHY_PAGE_STANDARD		  0x0000 /* Standard registers */
+#define MSCC_PHY_PAGE_EXTENDED		  0x0001 /* Extended registers */
 #define MSCC_PHY_PAGE_EXTENDED_2	  0x0002 /* Extended reg - page 2 */
 
+/* Extended Page 1 Registers */
+#define MSCC_PHY_ACTIPHY_CNTL		  20
+#define DOWNSHIFT_CNTL_MASK		  0x000C
+#define DOWNSHIFT_EN			  0x0010
+#define DOWNSHIFT_CNTL_POS		  2
+
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL		  20
 #define RGMII_RX_CLK_DELAY_MASK		  0x0070
@@ -75,6 +82,7 @@ enum rgmii_rx_clock_delay {
 
 struct vsc8531_private {
 	int rate_magic;
+	u8  downshift_magic;
 };
 
 #ifdef CONFIG_OF_MDIO
@@ -99,6 +107,31 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, u8 page)
 	return rc;
 }
 
+static int vsc85xx_downshift_set(struct phy_device *phydev, u8 magic)
+{
+	int rc;
+	u16 reg_val;
+
+	mutex_lock(&phydev->lock);
+	rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+	if (rc != 0)
+		goto out_unlock;
+
+	reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+	reg_val &= ~(DOWNSHIFT_CNTL_MASK);
+	reg_val |= magic;
+	rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
+	if (rc != 0)
+		goto out_unlock;
+
+	rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+	mutex_unlock(&phydev->lock);
+
+	return rc;
+}
+
 static int vsc85xx_wol_set(struct phy_device *phydev,
 			   struct ethtool_wolinfo *wol)
 {
@@ -239,11 +272,42 @@ static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
 
 	return -EINVAL;
 }
+
+static int vsc85xx_downshift_magic_get(struct phy_device *phydev)
+{
+	int rc;
+	u8 ds;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+
+	if (!of_node)
+		return -ENODEV;
+
+	rc = of_property_read_u8(of_node, "vsc8531,downshift-cnt", &ds);
+	if ((rc == -EINVAL) || (ds == 0))
+		return 0;
+	if (ds == 1 || ds > 5) {
+		phydev_err(phydev, "Invalid downshift count\n");
+		return -EINVAL;
+	}
+
+	/* ds is either 2,3,4 or 5 */
+	ds -= 2;
+	ds <<= DOWNSHIFT_CNTL_POS;
+	ds |= DOWNSHIFT_EN;
+
+	return ds;
+}
 #else
 static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
 {
 	return 0;
 }
+
+static int vsc85xx_downshift_magic_get(struct phy_device *phydev)
+{
+	return 0;
+}
 #endif /* CONFIG_OF_MDIO */
 
 static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8 edge_rate)
@@ -344,6 +408,10 @@ static int vsc85xx_config_init(struct phy_device *phydev)
 	if (rc)
 		return rc;
 
+	rc = vsc85xx_downshift_set(phydev, vsc8531->downshift_magic);
+	if (rc)
+		return rc;
+
 	rc = genphy_config_init(phydev);
 
 	return rc;
@@ -379,19 +447,24 @@ static int vsc85xx_config_intr(struct phy_device *phydev)
 static int vsc85xx_probe(struct phy_device *phydev)
 {
 	int rate_magic;
+	int downshift_magic;
 	struct vsc8531_private *vsc8531;
 
 	rate_magic = vsc85xx_edge_rate_magic_get(phydev);
 	if (rate_magic < 0)
 		return rate_magic;
 
+	downshift_magic = vsc85xx_downshift_magic_get(phydev);
+	if (downshift_magic < 0)
+		return downshift_magic;
+
 	vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
 	if (!vsc8531)
 		return -ENOMEM;
 
 	phydev->priv = vsc8531;
-
 	vsc8531->rate_magic = rate_magic;
+	vsc8531->downshift_magic = downshift_magic;
 
 	return 0;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Andrew Lunn]

On Fri, Oct 14, 2016 at 05:10:32PM +0530, Raju Lakkaraju wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> 
> For operation in cabling environments that are incompatible with
> 1000BAST-T, VSC8531 device provides an automatic link speed
> downshift operation. When enabled, the device automatically changes
> its 1000BAST-T auto-negotiation to the next slower speed after
> a configured number of failed attempts at 1000BAST-T.
> This feature is useful in setting up in networks using older cable
> installations that include only pairs A and B, and not pairs C and D.

Any reason not to just turn this on by default when auto-neg is
enabled?

	Andrew

Re: [PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Raju Lakkaraju]

Hi Andrew,

Thank you for code review and comments.

On Fri, Oct 14, 2016 at 02:12:32PM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> On Fri, Oct 14, 2016 at 05:10:32PM +0530, Raju Lakkaraju wrote:
> > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> >
> > For operation in cabling environments that are incompatible with
> > 1000BAST-T, VSC8531 device provides an automatic link speed
> > downshift operation. When enabled, the device automatically changes
> > its 1000BAST-T auto-negotiation to the next slower speed after
> > a configured number of failed attempts at 1000BAST-T.
> > This feature is useful in setting up in networks using older cable
> > installations that include only pairs A and B, and not pairs C and D.
> 
> Any reason not to just turn this on by default when auto-neg is
> enabled?
> 
Downshift can enable by default when auto-neg enabled. This is good idea.
But we would like to provide option to customer can choose whether this
feature need to enable or disable and also configure failure attempts.

Do you have any other suggestion how to configure failure attempts?

>         Andrew

---

Thanks,
Raju.

Re: [PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Florian Fainelli]

On October 17, 2016 12:31:54 AM PDT, Raju Lakkaraju <Raju.Lakkaraju@microsemi.com> wrote:
>Hi Andrew,
>
>Thank you for code review and comments.
>
>On Fri, Oct 14, 2016 at 02:12:32PM +0200, Andrew Lunn wrote:
>> EXTERNAL EMAIL
>> 
>> 
>> On Fri, Oct 14, 2016 at 05:10:32PM +0530, Raju Lakkaraju wrote:
>> > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>> >
>> > For operation in cabling environments that are incompatible with
>> > 1000BAST-T, VSC8531 device provides an automatic link speed
>> > downshift operation. When enabled, the device automatically changes
>> > its 1000BAST-T auto-negotiation to the next slower speed after
>> > a configured number of failed attempts at 1000BAST-T.
>> > This feature is useful in setting up in networks using older cable
>> > installations that include only pairs A and B, and not pairs C and
>D.
>> 
>> Any reason not to just turn this on by default when auto-neg is
>> enabled?
>> 
>Downshift can enable by default when auto-neg enabled. This is good
>idea.
>But we would like to provide option to customer can choose whether this
>feature need to enable or disable and also configure failure attempts.
>
>Do you have any other suggestion how to configure failure attempts?

Is the speed downshift feature similar to what Intel and Broadcom refer to as wirespeed? I have seen cases with Broadcom PHYs where we had to turn such a feature on to allow auto-negotiation to complete with 4-wire cables, but this had the downside of impacting normal autoneg, so it is left disabled.

I would expect the number of customers using this feature to be fairly limited, so having a tunable to turn this downshift on/off may be acceptable. Ethtool supports a number of tunable parameters now (such as rx_copybreak), there may be room for using something similar for boolean flags like these.

-- 
Florian

Re: [PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Raju Lakkaraju]

Hi Florian,

Thank you for review comments.

On Mon, Oct 17, 2016 at 05:38:46AM -0700, Florian Fainelli wrote:
> EXTERNAL EMAIL
> 
> 
> On October 17, 2016 12:31:54 AM PDT, Raju Lakkaraju <Raju.Lakkaraju@microsemi.com> wrote:
> >Hi Andrew,
> >
> >Thank you for code review and comments.
> >
> >On Fri, Oct 14, 2016 at 02:12:32PM +0200, Andrew Lunn wrote:
> >> EXTERNAL EMAIL
> >>
> >>
> >> On Fri, Oct 14, 2016 at 05:10:32PM +0530, Raju Lakkaraju wrote:
> >> > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> >> >
> >> > For operation in cabling environments that are incompatible with
> >> > 1000BAST-T, VSC8531 device provides an automatic link speed
> >> > downshift operation. When enabled, the device automatically changes
> >> > its 1000BAST-T auto-negotiation to the next slower speed after
> >> > a configured number of failed attempts at 1000BAST-T.
> >> > This feature is useful in setting up in networks using older cable
> >> > installations that include only pairs A and B, and not pairs C and
> >D.
> >>
> >> Any reason not to just turn this on by default when auto-neg is
> >> enabled?
> >>
> >Downshift can enable by default when auto-neg enabled. This is good
> >idea.
> >But we would like to provide option to customer can choose whether this
> >feature need to enable or disable and also configure failure attempts.
> >
> >Do you have any other suggestion how to configure failure attempts?
> 
> Is the speed downshift feature similar to what Intel and Broadcom refer to as wirespeed? I have seen cases with Broadcom PHYs where we had to turn such a feature on to allow auto-negotiation to complete with 4-wire cables, but this had the downside of impacting normal autoneg, so it is left disabled.
> 

Yes. I check the Broadcom wirespeed code. Downshift is similar to wirespeed.
But Broadcom wirespeed configuration in Ethernet controller. 

> I would expect the number of customers using this feature to be fairly limited, so having a tunable to turn this downshift on/off may be acceptable. Ethtool supports a number of tunable parameters now (such as rx_copybreak), there may be room for using something similar for boolean flags like these.
> 

This implementation shows little bit specific to Ethernet controller.
Do you have any PHY specific examples?

In another mail thread, you proposed similar to net device features.
Shall i implement that suggestion here?

> --
> Florian

---
Thanks,
Raju.

Re: [PATCH net-next 1/2] net: phy: Add Speed downshift set driver for Microsemi PHYs.

[Rob Herring]

On Fri, Oct 14, 2016 at 05:10:32PM +0530, Raju Lakkaraju wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
> 
> For operation in cabling environments that are incompatible with
> 1000BAST-T, VSC8531 device provides an automatic link speed
> downshift operation. When enabled, the device automatically changes
> its 1000BAST-T auto-negotiation to the next slower speed after
> a configured number of failed attempts at 1000BAST-T.
> This feature is useful in setting up in networks using older cable
> installations that include only pairs A and B, and not pairs C and D.
> 
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
> Signed-off-by: Allan W. Nielsen <allan.nielsen-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
> ---
>  .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  6 ++
>  drivers/net/phy/mscc.c                             | 75 +++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> index bdefefc6..062d115 100644
> --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> @@ -27,6 +27,11 @@ Optional properties:
>  			  'vddmac'.
>  			  Default value is 0%.
>  			  Ref: Table:1 - Edge rate change (below).
> +- downshift-cnt		: When enabled, the device automatically changes its
> +			  1000BAST-T auto-negotiation to the next slower speed
> +			  after a 'downshift-cnt' of failed attempts at
> +			  1000BAST-T. Allowed values: 0, 2, 3, 4, 5.
> +			  0 is default and will disable downshifting.

Not sure if you are dropping this based on the discussion. Is this 
end-user controlled or a board property? Only the latter should be in 
DT.

>  
>  Table: 1 - Edge rate change
>  ----------------------------------------------------------------|
> @@ -60,4 +65,5 @@ Example:
>                  compatible = "ethernet-phy-id0007.0570";
>                  vsc8531,vddmac		= <3300>;
>                  vsc8531,edge-slowdown	= <7>;
> +                vsc8531,downshift-cnt   = <3>;

This doesn't match the above. Also, 'vsc8531' should be the vendor 
prefix, not a part number. The existing properties are fine, but fix new 
ones.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Raju Lakkaraju]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
onset of a potential link failure in < 100 usec for 100BASE-TX
operation. FLF2 is supported through the MDINT (active low) pin.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  6 +++
 drivers/net/phy/mscc.c                             | 45 ++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 062d115..472fc68 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -32,6 +32,11 @@ Optional properties:
 			  after a 'downshift-cnt' of failed attempts at
 			  1000BAST-T. Allowed values: 0, 2, 3, 4, 5.
 			  0 is default and will disable downshifting.
+- flf2			: Fast Link Failure 2 (FLF2) feature enables the PHY
+			  to indicate the onset of a potential link failure in
+			  < 100 usec for 100BASE-TX operation. FLF2 is
+			  supported through the MDINT (active low) pin.
+			  Default will be disable flf2.
 
 Table: 1 - Edge rate change
 ----------------------------------------------------------------|
@@ -66,4 +71,5 @@ Example:
                 vsc8531,vddmac		= <3300>;
                 vsc8531,edge-slowdown	= <7>;
                 vsc8531,downshift-cnt   = <3>;
+		vsc8531,flf2;
         };
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index e87d9f0..57bd628 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -57,6 +57,7 @@ enum rgmii_rx_clock_delay {
 
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL		  20
+#define FLF2_ENABLE			  0x8000
 #define RGMII_RX_CLK_DELAY_MASK		  0x0070
 #define RGMII_RX_CLK_DELAY_POS		  4
 
@@ -83,6 +84,7 @@ enum rgmii_rx_clock_delay {
 struct vsc8531_private {
 	int rate_magic;
 	u8  downshift_magic;
+	bool flf2;		/* Fast Link Failure-2 Enable/Disable */
 };
 
 #ifdef CONFIG_OF_MDIO
@@ -107,6 +109,33 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, u8 page)
 	return rc;
 }
 
+static int vsc85xx_flf2_set(struct phy_device *phydev, bool op)
+{
+	int rc;
+	u16 reg_val;
+
+	mutex_lock(&phydev->lock);
+	rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
+	if (rc != 0)
+		goto out_unlock;
+
+	reg_val = phy_read(phydev, MSCC_PHY_RGMII_CNTL);
+	if (op)
+		reg_val |= FLF2_ENABLE;
+	else
+		reg_val &= ~FLF2_ENABLE;
+	rc = phy_write(phydev, MSCC_PHY_RGMII_CNTL, reg_val);
+	if (rc != 0)
+		goto out_unlock;
+
+	rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+	mutex_unlock(&phydev->lock);
+
+	return rc;
+}
+
 static int vsc85xx_downshift_set(struct phy_device *phydev, u8 magic)
 {
 	int rc;
@@ -412,6 +441,10 @@ static int vsc85xx_config_init(struct phy_device *phydev)
 	if (rc)
 		return rc;
 
+	rc = vsc85xx_flf2_set(phydev, vsc8531->flf2);
+	if (rc)
+		return rc;
+
 	rc = genphy_config_init(phydev);
 
 	return rc;
@@ -449,6 +482,11 @@ static int vsc85xx_probe(struct phy_device *phydev)
 	int rate_magic;
 	int downshift_magic;
 	struct vsc8531_private *vsc8531;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+
+	if (!of_node)
+		return -ENODEV;
 
 	rate_magic = vsc85xx_edge_rate_magic_get(phydev);
 	if (rate_magic < 0)
@@ -466,6 +504,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
 	vsc8531->rate_magic = rate_magic;
 	vsc8531->downshift_magic = downshift_magic;
 
+#ifdef CONFIG_OF_MDIO
+	/* Fast Link Failure 2 */
+	vsc8531->flf2 = of_property_read_bool(of_node, "vsc8531,flf2");
+#else
+	vsc8531->flf2 = 0;
+#endif
+
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Andrew Lunn]

> On Fri, Oct 14, 2016 at 05:10:33PM +0530, Raju Lakkaraju wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> 
> VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
> onset of a potential link failure in < 100 usec for 100BASE-TX
> operation. FLF2 is supported through the MDINT (active low) pin.

Is the MDINT pin specific to this feature, or a general interrupt pin?

Device tree is used to describe the hardware. It should not really
describe software or configuration. But the borders are a bit
fluffly. Signal edge rates is near to hardware. This is a lot more
towards configuration. So i'm not sure a device tree property is the
correct way to describe this.

This is also a feature i know other PHYs support. The Marvell PHY has
a "Metro Ethernet" extension which allows it to report link failures
for 1000BASE-T in 10, 20 or 40ms, instead of the usual 750ms. So we
need a generic solution other PHYs can implement.

As with cable testing, i think it should be an ethtool option.

   Andrew

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Raju Lakkaraju]

Hi Andrew,

Thank you for code review and comments.

On Fri, Oct 14, 2016 at 02:02:28PM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> > On Fri, Oct 14, 2016 at 05:10:33PM +0530, Raju Lakkaraju wrote:
> > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> >
> > VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
> > onset of a potential link failure in < 100 usec for 100BASE-TX
> > operation. FLF2 is supported through the MDINT (active low) pin.
> 
> Is the MDINT pin specific to this feature, or a general interrupt pin?
> 

MDINT pin is general interrupt. MDINT pin share the interrupt with
FLF2 along with another 13 interrupts.

> Device tree is used to describe the hardware. It should not really
> describe software or configuration. But the borders are a bit
> fluffly. Signal edge rates is near to hardware. This is a lot more
> towards configuration. So i'm not sure a device tree property is the
> correct way to describe this.
> 
> This is also a feature i know other PHYs support. The Marvell PHY has
> a "Metro Ethernet" extension which allows it to report link failures
> for 1000BASE-T in 10, 20 or 40ms, instead of the usual 750ms. So we
> need a generic solution other PHYs can implement.
> 
> As with cable testing, i think it should be an ethtool option.

I agree with you.
I thought this is one time initialization either enable or disable.
if customer need this feature, they can enable in DT.
Do you want me to implement through IOCTL instead of Device tree?
Do you have any other suggestions?

> 
>    Andrew

---
Thanks,
Raju.

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Florian Fainelli]

On October 17, 2016 1:13:14 AM PDT, Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org> wrote:
>Hi Andrew,
>
>Thank you for code review and comments.
>
>On Fri, Oct 14, 2016 at 02:02:28PM +0200, Andrew Lunn wrote:
>> EXTERNAL EMAIL
>> 
>> 
>> > On Fri, Oct 14, 2016 at 05:10:33PM +0530, Raju Lakkaraju wrote:
>> > From: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
>> >
>> > VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
>> > onset of a potential link failure in < 100 usec for 100BASE-TX
>> > operation. FLF2 is supported through the MDINT (active low) pin.
>> 
>> Is the MDINT pin specific to this feature, or a general interrupt
>pin?
>> 
>
>MDINT pin is general interrupt. MDINT pin share the interrupt with
>FLF2 along with another 13 interrupts.
>
>> Device tree is used to describe the hardware. It should not really
>> describe software or configuration. But the borders are a bit
>> fluffly. Signal edge rates is near to hardware. This is a lot more
>> towards configuration. So i'm not sure a device tree property is the
>> correct way to describe this.
>> 
>> This is also a feature i know other PHYs support. The Marvell PHY has
>> a "Metro Ethernet" extension which allows it to report link failures
>> for 1000BASE-T in 10, 20 or 40ms, instead of the usual 750ms. So we
>> need a generic solution other PHYs can implement.
>> 
>> As with cable testing, i think it should be an ethtool option.
>
>I agree with you.
>I thought this is one time initialization either enable or disable.
>if customer need this feature, they can enable in DT.
>Do you want me to implement through IOCTL instead of Device tree?
>Do you have any other suggestions?

As indicated in the other email about speed downshift, we may want to utilize ethtool's ability to modify tunable parameters (small integer, boolean, values) and extend it to cover features offered by PHYs in a way that an user can dynamically turn these features on or off.

In fact, this looks a lot like netdev features (e.g: checksum offload), and there seems to be some commonality here between at least Marvell and Microsemi (for the faster link down reporting), so maybe we should start adding PHY features similar to netdev features?

-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Raju Lakkaraju]

Hi Florian,

Thank you for review comments.

On Mon, Oct 17, 2016 at 05:51:11AM -0700, Florian Fainelli wrote:
> EXTERNAL EMAIL
> 
> 
> On October 17, 2016 1:13:14 AM PDT, Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org> wrote:
> >Hi Andrew,
> >
> >Thank you for code review and comments.
> >
> >On Fri, Oct 14, 2016 at 02:02:28PM +0200, Andrew Lunn wrote:
> >> EXTERNAL EMAIL
> >>
> >>
> >> > On Fri, Oct 14, 2016 at 05:10:33PM +0530, Raju Lakkaraju wrote:
> >> > From: Raju Lakkaraju <Raju.Lakkaraju-dzo6w/eZyo2tG0bUXCXiUA@public.gmane.org>
> >> >
> >> > VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
> >> > onset of a potential link failure in < 100 usec for 100BASE-TX
> >> > operation. FLF2 is supported through the MDINT (active low) pin.
> >>
> >> Is the MDINT pin specific to this feature, or a general interrupt
> >pin?
> >>
> >
> >MDINT pin is general interrupt. MDINT pin share the interrupt with
> >FLF2 along with another 13 interrupts.
> >
> >> Device tree is used to describe the hardware. It should not really
> >> describe software or configuration. But the borders are a bit
> >> fluffly. Signal edge rates is near to hardware. This is a lot more
> >> towards configuration. So i'm not sure a device tree property is the
> >> correct way to describe this.
> >>
> >> This is also a feature i know other PHYs support. The Marvell PHY has
> >> a "Metro Ethernet" extension which allows it to report link failures
> >> for 1000BASE-T in 10, 20 or 40ms, instead of the usual 750ms. So we
> >> need a generic solution other PHYs can implement.
> >>
> >> As with cable testing, i think it should be an ethtool option.
> >
> >I agree with you.
> >I thought this is one time initialization either enable or disable.
> >if customer need this feature, they can enable in DT.
> >Do you want me to implement through IOCTL instead of Device tree?
> >Do you have any other suggestions?
> 
> As indicated in the other email about speed downshift, we may want to utilize ethtool's ability to modify tunable parameters (small integer, boolean, values) and extend it to cover features offered by PHYs in a way that an user can dynamically turn these features on or off.
> 
> In fact, this looks a lot like netdev features (e.g: checksum offload), and there seems to be some commonality here between at least Marvell and Microsemi (for the faster link down reporting), so maybe we should start adding PHY features similar to netdev features?
> 

Sure. 

I would like add one flag in phy_device structure:
u64 phy_features;

In phy_driver structure, i would like to add 2 function pointer as

int (*phy_featues_set)(struct phy_device *phydev);
int (*phy_featues_get)(struct phy_device *phydev);

All the PHY specific features i.e. Fast link failure -2, Downshift, Loopback etc
are the case in feature_set/feature_get functions.

Is it ok?

> --
> Florian

---
Thanks,
Raju.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Andrew Lunn]

> > In fact, this looks a lot like netdev features (e.g: checksum
> > offload), and there seems to be some commonality here between at
> > least Marvell and Microsemi (for the faster link down reporting),
> > so maybe we should start adding PHY features similar to netdev
> > features?

> Sure. 
> 
> I would like add one flag in phy_device structure:
> u64 phy_features;
> 
> In phy_driver structure, i would like to add 2 function pointer as
> 
> int (*phy_featues_set)(struct phy_device *phydev);
> int (*phy_featues_get)(struct phy_device *phydev);
> 
> All the PHY specific features i.e. Fast link failure -2, Downshift, Loopback etc
> are the case in feature_set/feature_get functions.

Please follow how the ethertool features are implemented. So you need
to extend net/core/ethtool.c to pass these call down, etc.

       Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Raju Lakkaraju]

Hi Andrew,

On Tue, Oct 18, 2016 at 12:58:20PM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> > > In fact, this looks a lot like netdev features (e.g: checksum
> > > offload), and there seems to be some commonality here between at
> > > least Marvell and Microsemi (for the faster link down reporting),
> > > so maybe we should start adding PHY features similar to netdev
> > > features?
> 
> > Sure.
> >
> > I would like add one flag in phy_device structure:
> > u64 phy_features;
> >
> > In phy_driver structure, i would like to add 2 function pointer as
> >
> > int (*phy_featues_set)(struct phy_device *phydev);
> > int (*phy_featues_get)(struct phy_device *phydev);
> >
> > All the PHY specific features i.e. Fast link failure -2, Downshift, Loopback etc
> > are the case in feature_set/feature_get functions.
> 
> Please follow how the ethertool features are implemented. So you need
> to extend net/core/ethtool.c to pass these call down, etc.
> 

Sure. I will do.
Do i need to change Ethtool application and submit along with 
downshift driver patch (i.e. ethtool.c, phy.c, cpsw.c and mscc.c changes) ? 
>        Andrew

---
Thanks,
Raju.

Re: [PATCH net-next 2/2] net: phy: Add Fast Link Failure - 2 set driver for Microsemi PHYs.

[Andrew Lunn]

> Do i need to change Ethtool application and submit along with 
> downshift driver patch (i.e. ethtool.c, phy.c, cpsw.c and mscc.c changes) ? 

Yes, please submit two patchset. One patchset for ethtool and its man
page and a second patchset for all the kernel changes.

     Thanks
	Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html