[PATCH 2/3] of_mdio: add new DT property 'managed' to specify the PHY management type

2015-07-16 Thread Stas Sergeev 

Currently the PHY management type is selected by the MAC driver arbitrary.
The decision is based on the presence of the fixed-link node and on a
will of the driver's authors.
This caused a regression recently, when mvneta driver suddenly started
to use the in-band status for auto-negotiation on fixed links.
It appears the auto-negotiation may not work when expected by the MAC driver.
Sebastien Rannou explains:
 Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
inband status) is connected to the switch through QSGMII, and in this context
we are on the media side of the PHY. 
https://lkml.org/lkml/2015/7/10/206

This patch introduces the new string property 'managed' that allows
the user to set the management type explicitly.
The supported values are:
auto - default. Uses either MDIO or nothing, depending on the presence
of the fixed-link node
in-band-status - use in-band status

Signed-off-by: Stas Sergeev s...@users.sourceforge.net

CC: Rob Herring robh...@kernel.org
CC: Pawel Moll pawel.m...@arm.com
CC: Mark Rutland mark.rutl...@arm.com
CC: Ian Campbell ijc+devicet...@hellion.org.uk
CC: Kumar Gala ga...@codeaurora.org
CC: Florian Fainelli f.faine...@gmail.com
CC: Grant Likely grant.lik...@linaro.org
CC: devicet...@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: netdev@vger.kernel.org
---
 Documentation/devicetree/bindings/net/ethernet.txt |  4 
 drivers/of/of_mdio.c   | 19 +--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt 
b/Documentation/devicetree/bindings/net/ethernet.txt
index 3fc3605..cb115a3 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -19,7 +19,11 @@ The following properties are common to the Ethernet 
controllers:
 - phy: the same as phy-handle property, not recommended for new bindings.
 - phy-device: the same as phy-handle property, not recommended for new
   bindings.
+- managed: string, specifies the PHY management type. Supported values are:
+  auto, in-band-status. auto is the default, it usess MDIO for
+  management if fixed-link is not specified.

 Child nodes of the Ethernet controller are typically the individual PHY devices
 connected via the MDIO bus (sometimes the MDIO bus controller is separate).
 They are described in the phy.txt file in this same directory.
+For non-MDIO PHY management see fixed-link.txt.
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1bd4305..5dc1ef95 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -262,7 +262,8 @@ EXPORT_SYMBOL(of_phy_attach);
 bool of_phy_is_fixed_link(struct device_node *np)
 {
struct device_node *dn;
-   int len;
+   int len, err;
+   const char *managed;

/* New binding */
dn = of_get_child_by_name(np, fixed-link);
@@ -271,6 +272,10 @@ bool of_phy_is_fixed_link(struct device_node *np)
return true;
}

+   err = of_property_read_string(np, managed, managed);
+   if (err == 0  strcmp(managed, auto) != 0)
+   return true;
+
/* Old binding */
if (of_get_property(np, fixed-link, len) 
len == (5 * sizeof(__be32)))
@@ -285,8 +290,18 @@ int of_phy_register_fixed_link(struct device_node *np)
struct fixed_phy_status status = {};
struct device_node *fixed_link_node;
const __be32 *fixed_link_prop;
-   int len;
+   int len, err;
struct phy_device *phy;
+   const char *managed;
+
+   err = of_property_read_string(np, managed, managed);
+   if (err == 0) {
+   if (strcmp(managed, in-band-status) == 0) {
+   /* status is zeroed, namely its .link member */
+   phy = fixed_phy_register(PHY_POLL, status, np);
+   return IS_ERR(phy) ? PTR_ERR(phy) : 0;
+   }
+   }

/* New binding */
fixed_link_node = of_get_child_by_name(np, fixed-link);
-- 
1.9.1
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] of_mdio: add new DT property 'managed' to specify the PHY management type

2015-07-14 Thread Stas Sergeev 

Currently the PHY management type is selected by the MAC driver arbitrary.
The decision is based on the presence of the fixed-link node and on a
will of the driver's authors.
This caused a regression recently, when mvneta driver suddenly started
to use the in-band status for auto-negotiation on fixed links.
It appears the auto-negotiation may not work when expected by the MAC driver.
Sebastien Rannou explains:
 Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
inband status) is connected to the switch through QSGMII, and in this context
we are on the media side of the PHY. 
https://lkml.org/lkml/2015/7/10/206

This patch introduces the new string property 'managed' that allows
the user to set the management type explicitly.
The supported values are:
auto - default. Uses either MDIO or nothing, depending on the presence
of the fixed-link node
mdio - use MDIO
in-band-status - use in-band status
none - use fixed-link description

Signed-off-by: Stas Sergeev s...@users.sourceforge.net

CC: Rob Herring robh...@kernel.org
CC: Pawel Moll pawel.m...@arm.com
CC: Mark Rutland mark.rutl...@arm.com
CC: Ian Campbell ijc+devicet...@hellion.org.uk
CC: Kumar Gala ga...@codeaurora.org
CC: Florian Fainelli f.faine...@gmail.com
CC: Grant Likely grant.lik...@linaro.org
CC: devicet...@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: netdev@vger.kernel.org
---
 Documentation/devicetree/bindings/net/ethernet.txt |  5 
 drivers/of/of_mdio.c   | 28 --
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt 
b/Documentation/devicetree/bindings/net/ethernet.txt
index 3fc3605..23743e9 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -19,7 +19,12 @@ The following properties are common to the Ethernet 
controllers:
 - phy: the same as phy-handle property, not recommended for new bindings.
 - phy-device: the same as phy-handle property, not recommended for new
   bindings.
+- managed: string, specifies the PHY management type. Supported values are:
+  auto, mdio, in-band-status, none. auto is the default, and it
+  sets the management type to either mdio or none, depending on the
+  presence of the fixed-link child node.

 Child nodes of the Ethernet controller are typically the individual PHY devices
 connected via the MDIO bus (sometimes the MDIO bus controller is separate).
 They are described in the phy.txt file in this same directory.
+For non-MDIO PHY management see fixed-link.txt.
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1bd4305..a460812 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -262,7 +262,16 @@ EXPORT_SYMBOL(of_phy_attach);
 bool of_phy_is_fixed_link(struct device_node *np)
 {
struct device_node *dn;
-   int len;
+   int len, m_err;
+   const char *managed;
+
+   m_err = of_property_read_string(np, managed, managed);
+   if (m_err == 0) {
+   if (strcmp(managed, none) == 0)
+   return true;
+   if (strcmp(managed, mdio) == 0)
+   return false;
+   }

/* New binding */
dn = of_get_child_by_name(np, fixed-link);
@@ -271,6 +280,9 @@ bool of_phy_is_fixed_link(struct device_node *np)
return true;
}

+   if (m_err == 0  strcmp(managed, auto) != 0)
+   return true;
+
/* Old binding */
if (of_get_property(np, fixed-link, len) 
len == (5 * sizeof(__be32)))
@@ -285,8 +297,20 @@ int of_phy_register_fixed_link(struct device_node *np)
struct fixed_phy_status status = {};
struct device_node *fixed_link_node;
const __be32 *fixed_link_prop;
-   int len;
+   int len, err;
struct phy_device *phy;
+   const char *managed;
+
+   err = of_property_read_string(np, managed, managed);
+   if (err == 0) {
+   if (strcmp(managed, mdio) == 0)
+   return -EINVAL;
+   if (strcmp(managed, in-band-status) == 0) {
+   status.link = 0;
+   phy = fixed_phy_register(PHY_POLL, status, np);
+   return IS_ERR(phy) ? PTR_ERR(phy) : 0;
+   }
+   }

/* New binding */
fixed_link_node = of_get_child_by_name(np, fixed-link);
-- 
1.9.1
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] of_mdio: add new DT property 'managed' to specify the PHY management type

2015-07-14 Thread Stas Sergeev 

14.07.2015 20:51, Florian Fainelli пишет:

On 14/07/15 10:13, Stas Sergeev wrote:

Currently the PHY management type is selected by the MAC driver arbitrary.
The decision is based on the presence of the fixed-link node and on a
will of the driver's authors.
This caused a regression recently, when mvneta driver suddenly started
to use the in-band status for auto-negotiation on fixed links.
It appears the auto-negotiation may not work when expected by the MAC driver.
Sebastien Rannou explains:
 Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
inband status) is connected to the switch through QSGMII, and in this context
we are on the media side of the PHY. 
https://lkml.org/lkml/2015/7/10/206

This patch introduces the new string property 'managed' that allows
the user to set the management type explicitly.
The supported values are:
auto - default. Uses either MDIO or nothing, depending on the presence
of the fixed-link node
mdio - use MDIO
in-band-status - use in-band status
none - use fixed-link description

I thought we agreed in the last thread that mdio was implied whenever
a proper PHY phandle to a device sitting on MDIO bus is used and that
auto did not make much sense unless we were also describing how to do
this auto-negotiation completely?

Exactly not:
---

If we would implement autoneg outside of the fixed-link,
then its semantic would likely be
autoneg = mdio | in-band | off


Right, if auto-negotiation was defined outside of fixed-link, that is
indeed how I would also specify this.
---

That's why I implemented it the way you see.
But as you changed your mind, I'll remove mdio.

As mentioned below, mdio is redundant with finding a phy-handle, 
and auto is not specific enough to be useful to a consumer of this 
information. 

I prefer to keep auto, as otherwise we'll have the default
value (when the option is not specified) never achievable
when the option _is_ specified, which is probably not very
good. But I can remove none instead, leaving only
in-band-status and auto. Ok?
Of course one can propose to completely ignore that option
in presence of MDIO, but I wonder why not to allow for
example to opt for in-band status even if you have MDIO?
So if we want this option to play nicely with MDIO, as opposed
to being entirely overridden, then auto still makes sense IMO.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] of_mdio: add new DT property 'managed' to specify the PHY management type

2015-07-14 Thread Florian Fainelli 
On 14/07/15 10:13, Stas Sergeev wrote:
 
 Currently the PHY management type is selected by the MAC driver arbitrary.
 The decision is based on the presence of the fixed-link node and on a
 will of the driver's authors.
 This caused a regression recently, when mvneta driver suddenly started
 to use the in-band status for auto-negotiation on fixed links.
 It appears the auto-negotiation may not work when expected by the MAC driver.
 Sebastien Rannou explains:
  Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
 a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
 inband status) is connected to the switch through QSGMII, and in this context
 we are on the media side of the PHY. 
 https://lkml.org/lkml/2015/7/10/206
 
 This patch introduces the new string property 'managed' that allows
 the user to set the management type explicitly.
 The supported values are:
 auto - default. Uses either MDIO or nothing, depending on the presence
 of the fixed-link node
 mdio - use MDIO
 in-band-status - use in-band status
 none - use fixed-link description

I thought we agreed in the last thread that mdio was implied whenever
a proper PHY phandle to a device sitting on MDIO bus is used and that
auto did not make much sense unless we were also describing how to do
this auto-negotiation completely?

The way I see it, the only thing that is needed at this point is an
in-band-status property which you rightfully placed at the Ethernet
MAC DT node level, this is fine, however, I disagree with how the values
are enforced, see below:

 
 Signed-off-by: Stas Sergeev s...@users.sourceforge.net
 
 CC: Rob Herring robh...@kernel.org
 CC: Pawel Moll pawel.m...@arm.com
 CC: Mark Rutland mark.rutl...@arm.com
 CC: Ian Campbell ijc+devicet...@hellion.org.uk
 CC: Kumar Gala ga...@codeaurora.org
 CC: Florian Fainelli f.faine...@gmail.com
 CC: Grant Likely grant.lik...@linaro.org
 CC: devicet...@vger.kernel.org
 CC: linux-ker...@vger.kernel.org
 CC: netdev@vger.kernel.org
 ---
  Documentation/devicetree/bindings/net/ethernet.txt |  5 
  drivers/of/of_mdio.c   | 28 
 --
  2 files changed, 31 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/net/ethernet.txt 
 b/Documentation/devicetree/bindings/net/ethernet.txt
 index 3fc3605..23743e9 100644
 --- a/Documentation/devicetree/bindings/net/ethernet.txt
 +++ b/Documentation/devicetree/bindings/net/ethernet.txt
 @@ -19,7 +19,12 @@ The following properties are common to the Ethernet 
 controllers:
  - phy: the same as phy-handle property, not recommended for new bindings.
  - phy-device: the same as phy-handle property, not recommended for new
bindings.
 +- managed: string, specifies the PHY management type. Supported values are:
 +  auto, mdio, in-band-status, none. auto is the default, and it
 +  sets the management type to either mdio or none, depending on the
 +  presence of the fixed-link child node.

As mentioned below, mdio is redundant with finding a phy-handle, and
auto is not specific enough to be useful to a consumer of this
information.

 
  Child nodes of the Ethernet controller are typically the individual PHY 
 devices
  connected via the MDIO bus (sometimes the MDIO bus controller is separate).
  They are described in the phy.txt file in this same directory.
 +For non-MDIO PHY management see fixed-link.txt.
 diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
 index 1bd4305..a460812 100644
 --- a/drivers/of/of_mdio.c
 +++ b/drivers/of/of_mdio.c
 @@ -262,7 +262,16 @@ EXPORT_SYMBOL(of_phy_attach);
  bool of_phy_is_fixed_link(struct device_node *np)
  {
   struct device_node *dn;
 - int len;
 + int len, m_err;
 + const char *managed;
 +
 + m_err = of_property_read_string(np, managed, managed);
 + if (m_err == 0) {
 + if (strcmp(managed, none) == 0)
 + return true;
 + if (strcmp(managed, mdio) == 0)
 + return false;
 + }

managed = mdio on a fixed link is by definition not something remotely
correct, there should be a proper PHY driver for this, and therefore a
different representation: a PHY phandle to a PHY node on a MDIO bus. I
do not think enforcing this has been incorrect provides much value,
since you ought to write correct DT in the first place.

 
   /* New binding */
   dn = of_get_child_by_name(np, fixed-link);
 @@ -271,6 +280,9 @@ bool of_phy_is_fixed_link(struct device_node *np)
   return true;
   }
 
 + if (m_err == 0  strcmp(managed, auto) != 0)
 + return true;
 +
   /* Old binding */
   if (of_get_property(np, fixed-link, len) 
   len == (5 * sizeof(__be32)))
 @@ -285,8 +297,20 @@ int of_phy_register_fixed_link(struct device_node *np)
   struct fixed_phy_status status = {};
   struct device_node *fixed_link_node;
   const __be32 *fixed_link_prop;
 - int len;
 + int len,