[PATCH net-next 0/9] DSA port configuration and status

[PATCH net-next 0/9] DSA port configuration and status

[Andrew Lunn]

This patchset allows various switch port settings to be configured and
port status to be sampled. Some of these patches have been posted
before.

The first three patches provide infrastructure for configuring a
switch ports link speed and duplex from a fixed_link phy.

Patch four then uses this infrastructure to allow the CPU and DSA
ports of a switch to be configured using a fixed-link property in the
device tree.

Patches five and six allow a phy-mode property to be specified in the
device tree, and allow this to be used for configuring RGMII delays.

Patches seven through nine allow link status, for example that of an
SFP module, to be read from a gpio.

Please don't merge these patches until Florian Fainelli has reviewed
them.

Andrew Lunn (8):
  dsa: mv88e6xxx: Allow speed/duplex of port to be configured
  phy: fixed_phy: Set supported speed in phydev
  net: dsa: Allow configuration of CPU & DSA port speeds/duplex
  net: dsa: Allow DSA and CPU ports to have a phy-mode property
  dsa: mv88e6xxx: Set the RGMII delay based on phy interface
  dsa: mv88e6xxx: Don't poll forced interfaces for state changes
  phy: fixed_phy: Add gpio to determine link up/down.
  phy: fixed_phy: Set phy capabilities even when link is down

Florian Fainelli (1):
  net: phy: Allow PHY devices to identify themselves as Ethernet
    switches, etc.

 .../devicetree/bindings/net/fixed-link.txt         | 14 ++++-
 Documentation/networking/stmmac.txt                |  2 +-
 arch/m68k/coldfire/m5272.c                         |  2 +-
 arch/mips/ar7/platform.c                           |  5 +-
 arch/mips/bcm47xx/setup.c                          |  2 +-
 drivers/net/dsa/mv88e6123_61_65.c                  |  1 +
 drivers/net/dsa/mv88e6131.c                        |  1 +
 drivers/net/dsa/mv88e6171.c                        |  1 +
 drivers/net/dsa/mv88e6352.c                        |  1 +
 drivers/net/dsa/mv88e6xxx.c                        | 73 ++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h                        |  4 ++
 drivers/net/ethernet/broadcom/genet/bcmmii.c       |  2 +-
 drivers/net/phy/fixed_phy.c                        | 45 ++++++++++---
 drivers/of/of_mdio.c                               | 13 +++-
 include/linux/phy.h                                | 12 ++++
 include/linux/phy_fixed.h                          |  8 ++-
 net/dsa/dsa.c                                      | 43 +++++++++++++
 17 files changed, 210 insertions(+), 19 deletions(-)

-- 
2.5.0

[PATCH net-next 1/9] net: phy: Allow PHY devices to identify themselves as Ethernet switches, etc.

[Andrew Lunn]

From: Florian Fainelli <f.fainelli@gmail.com>

Some Ethernet MAC drivers using the PHY library require the hardcoding
of link parameters when interfaced to a switch device, SFP module,
switch to switch port, etc. This has typically lead to various ad-hoc
implementations looking like this:

- using a "fixed PHY" emulated device, which will provide link
  indication towards the Ethernet MAC driver and hardware

- pretend there is no PHY and hardcode link parameters, ala mv643x_eth

Based on that, it is desireable to have the PHY drivers advertise the
correct link parameters, just like regular Ethernet PHYs towards their
CPU Ethernet MAC drivers, however, Ethernet MAC drivers should be able
to tell whether this link should be monitored or not. In the context
of an Ethernet switch, SFP module, switch to switch link, we do not
need to monitor this link since it should be always up.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: set is_pseudo_fixed_link before registering the phy
---
 drivers/net/phy/fixed_phy.c |  1 +
 include/linux/phy.h         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 479b93f9581c..1a400ef92d65 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -294,6 +294,7 @@ struct phy_device *fixed_phy_register(unsigned int irq,
 
 	of_node_get(np);
 	phy->dev.of_node = np;
+	phy->is_pseudo_fixed_link = true;
 
 	ret = phy_device_register(phy);
 	if (ret) {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e5fb1d415961..962387a192f1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -330,6 +330,7 @@ struct phy_c45_device_ids {
  * c45_ids: 802.3-c45 Device Identifers if is_c45.
  * is_c45:  Set to true if this phy uses clause 45 addressing.
  * is_internal: Set to true if this phy is internal to a MAC.
+ * 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.
  * state: state of the PHY for management purposes
@@ -368,6 +369,7 @@ struct phy_device {
 	struct phy_c45_device_ids c45_ids;
 	bool is_c45;
 	bool is_internal;
+	bool is_pseudo_fixed_link;
 	bool has_fixups;
 	bool suspended;
 
@@ -688,6 +690,16 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
 {
 	return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
 		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
+};
+
+/*
+ * phy_is_pseudo_fixed_link - Convenience function for testing if this
+ * PHY is the CPU port facing side of an Ethernet switch, or similar.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev)
+{
+	return phydev->is_pseudo_fixed_link;
 }
 
 /**
-- 
2.5.0

[PATCH net-next 2/9] dsa: mv88e6xxx: Allow speed/duplex of port to be configured

[Andrew Lunn]

The current code sets user ports to perform auto negotiation using the
phy. CPU and DSA ports are configured to full duplex and maximum speed
the switch supports.

There are however use cases where the CPU has a slower port, and when
user ports have SFP modules with fixed speed. In these cases, port
settings to be read from a fixed_phy devices. The switch driver then
needs to implement the adjust_link op, so the port settings can be
set.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6123_61_65.c |  1 +
 drivers/net/dsa/mv88e6131.c       |  1 +
 drivers/net/dsa/mv88e6171.c       |  1 +
 drivers/net/dsa/mv88e6352.c       |  1 +
 drivers/net/dsa/mv88e6xxx.c       | 58 +++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  2 ++
 6 files changed, 64 insertions(+)

diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
index 71a29a7ce538..3de2a6d73fdc 100644
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ b/drivers/net/dsa/mv88e6123_61_65.c
@@ -129,6 +129,7 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.adjust_link		= mv88e6xxx_adjust_link,
 #ifdef CONFIG_NET_DSA_HWMON
 	.get_temp		= mv88e6xxx_get_temp,
 #endif
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 32f4a08e9bc9..3e8386529965 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -182,6 +182,7 @@ struct dsa_switch_driver mv88e6131_switch_driver = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.adjust_link		= mv88e6xxx_adjust_link,
 };
 
 MODULE_ALIAS("platform:mv88e6085");
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 735f04cd83ee..d54b7400e8d8 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -108,6 +108,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.adjust_link		= mv88e6xxx_adjust_link,
 #ifdef CONFIG_NET_DSA_HWMON
 	.get_temp               = mv88e6xxx_get_temp,
 #endif
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 14b71779df99..1f5129c105fb 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -328,6 +328,7 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.adjust_link		= mv88e6xxx_adjust_link,
 	.set_eee		= mv88e6xxx_set_eee,
 	.get_eee		= mv88e6xxx_get_eee,
 #ifdef CONFIG_NET_DSA_HWMON
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 2ab3f9810593..7901db6503b4 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -14,6 +14,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
 #include <linux/if_bridge.h>
 #include <linux/jiffies.h>
 #include <linux/list.h>
@@ -560,6 +561,63 @@ static bool mv88e6xxx_6352_family(struct dsa_switch *ds)
 	return false;
 }
 
+/* We expect the switch to perform auto negotiation if there is a real
+ * phy. However, in the case of a fixed link phy, we force the port
+ * settings from the fixed link settings.
+ */
+void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
+			   struct phy_device *phydev)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	u32 ret, reg;
+
+	if (!phy_is_pseudo_fixed_link(phydev))
+		return;
+
+	mutex_lock(&ps->smi_mutex);
+
+	ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_PCS_CTRL);
+	if (ret < 0)
+		goto out;
+
+	reg = ret & ~(PORT_PCS_CTRL_LINK_UP |
+		      PORT_PCS_CTRL_FORCE_LINK |
+		      PORT_PCS_CTRL_DUPLEX_FULL |
+		      PORT_PCS_CTRL_FORCE_DUPLEX |
+		      PORT_PCS_CTRL_UNFORCED);
+
+	reg |= PORT_PCS_CTRL_FORCE_LINK;
+	if (phydev->link)
+			reg |= PORT_PCS_CTRL_LINK_UP;
+
+	if (mv88e6xxx_6065_family(ds) && phydev->speed > SPEED_100)
+		goto out;
+
+	switch (phydev->speed) {
+	case SPEED_1000:
+		reg |= PORT_PCS_CTRL_1000;
+		break;
+	case SPEED_100:
+		reg |= PORT_PCS_CTRL_100;
+		break;
+	case SPEED_10:
+		reg |= PORT_PCS_CTRL_10;
+		break;
+	default:
+		pr_info("Unknown speed");
+		goto out;
+	}
+
+	reg |= PORT_PCS_CTRL_FORCE_DUPLEX;
+	if (phydev->duplex == DUPLEX_FULL)
+		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
+
+	_mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg);
+
+out:
+	mutex_unlock(&ps->smi_mutex);
+}
+
 /* Must be called with SMI mutex held */
 static int _mv88e6xxx_stats_wait(struct dsa_switch *ds)
 {
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 72ca887feb0d..79003c55fe62 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -446,6 +446,8 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 				 uint64_t *data);
 int mv88e6xxx_get_sset_count(struct dsa_switch *ds);
 int mv88e6xxx_get_sset_count_basic(struct dsa_switch *ds);
+void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
+			   struct phy_device *phydev);
 int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port);
 void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 			struct ethtool_regs *regs, void *_p);
-- 
2.5.0

Re: [PATCH net-next 2/9] dsa: mv88e6xxx: Allow speed/duplex of port to be configured

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> The current code sets user ports to perform auto negotiation using the
> phy. CPU and DSA ports are configured to full duplex and maximum speed
> the switch supports.
> 
> There are however use cases where the CPU has a slower port, and when
> user ports have SFP modules with fixed speed. In these cases, port
> settings to be read from a fixed_phy devices. The switch driver then
> needs to implement the adjust_link op, so the port settings can be
> set.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 3/9] phy: fixed_phy: Set supported speed in phydev

[Andrew Lunn]

Set the supported field of the phydev to indicate the speed features
of the phy. If the phy is never attached to a netdev, but used in an
adjust_link() function, the speed will be incorrectly evaluated to
10/half rather than the correct speed/duplex.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/fixed_phy.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 1a400ef92d65..7244336dee7a 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -296,6 +296,18 @@ struct phy_device *fixed_phy_register(unsigned int irq,
 	phy->dev.of_node = np;
 	phy->is_pseudo_fixed_link = true;
 
+	switch (status->speed) {
+	case SPEED_1000:
+		phy->supported = PHY_1000BT_FEATURES;
+		break;
+	case SPEED_100:
+		phy->supported = PHY_100BT_FEATURES;
+		break;
+	case SPEED_10:
+	default:
+		phy->supported = PHY_10BT_FEATURES;
+	}
+
 	ret = phy_device_register(phy);
 	if (ret) {
 		phy_device_free(phy);
-- 
2.5.0

Re: [PATCH net-next 3/9] phy: fixed_phy: Set supported speed in phydev

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> Set the supported field of the phydev to indicate the speed features
> of the phy. If the phy is never attached to a netdev, but used in an
> adjust_link() function, the speed will be incorrectly evaluated to
> 10/half rather than the correct speed/duplex.

As mentioned in patch 4, the behavior is not really specified before you
attach to the PHY device, and therefore configured by the PHY library,
still:

> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 4/9] net: dsa: Allow configuration of CPU & DSA port speeds/duplex

[Andrew Lunn]

By default, DSA and CPU ports are configured to the maximum speed the
switch supports. However there can be use cases where the peer devices
port is slower. Allow a fixed-link property to be used with the DSA
and CPU port in the device tree, and use this information to configure
the port.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/dsa.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 053eb2b8e682..afff17341b73 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -176,6 +176,35 @@ __ATTRIBUTE_GROUPS(dsa_hwmon);
 #endif /* CONFIG_NET_DSA_HWMON */
 
 /* basic switch operations **************************************************/
+static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device *master)
+{
+	struct dsa_chip_data *cd = ds->pd;
+	struct device_node *port_dn;
+	struct phy_device *phydev;
+	int ret, port;
+
+	for (port = 0; port < DSA_MAX_PORTS; port++) {
+		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
+			continue;
+
+		port_dn = cd->port_dn[port];
+		if (of_phy_is_fixed_link(port_dn)) {
+			ret = of_phy_register_fixed_link(port_dn);
+			if (ret) {
+				netdev_err(master,
+					   "failed to register fixed PHY\n");
+				return ret;
+			}
+			phydev = of_phy_find_device(port_dn);
+			genphy_config_init(phydev);
+			genphy_read_status(phydev);
+			if (ds->drv->adjust_link)
+				ds->drv->adjust_link(ds, port, phydev);
+		}
+	}
+	return 0;
+}
+
 static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 {
 	struct dsa_switch_driver *drv = ds->drv;
@@ -297,6 +326,14 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 		}
 	}
 
+	/* Perform configuration of the CPU and DSA ports */
+	ret = dsa_cpu_dsa_setup(ds, dst->master_netdev);
+	if (ret < 0) {
+		netdev_err(dst->master_netdev, "[%d] : can't configure CPU and DSA ports\n",
+			   index);
+		ret = 0;
+	}
+
 #ifdef CONFIG_NET_DSA_HWMON
 	/* If the switch provides a temperature sensor,
 	 * register with hardware monitoring subsystem.
-- 
2.5.0

Re: [PATCH net-next 4/9] net: dsa: Allow configuration of CPU & DSA port speeds/duplex

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> By default, DSA and CPU ports are configured to the maximum speed the
> switch supports. However there can be use cases where the peer devices
> port is slower. Allow a fixed-link property to be used with the DSA
> and CPU port in the device tree, and use this information to configure
> the port.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/dsa/dsa.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 053eb2b8e682..afff17341b73 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -176,6 +176,35 @@ __ATTRIBUTE_GROUPS(dsa_hwmon);
>  #endif /* CONFIG_NET_DSA_HWMON */
>  
>  /* basic switch operations **************************************************/
> +static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device *master)
> +{
> +	struct dsa_chip_data *cd = ds->pd;
> +	struct device_node *port_dn;
> +	struct phy_device *phydev;
> +	int ret, port;
> +
> +	for (port = 0; port < DSA_MAX_PORTS; port++) {
> +		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
> +			continue;
> +
> +		port_dn = cd->port_dn[port];
> +		if (of_phy_is_fixed_link(port_dn)) {
> +			ret = of_phy_register_fixed_link(port_dn);
> +			if (ret) {
> +				netdev_err(master,
> +					   "failed to register fixed PHY\n");
> +				return ret;
> +			}
> +			phydev = of_phy_find_device(port_dn);
> +			genphy_config_init(phydev);
> +			genphy_read_status(phydev);
> +			if (ds->drv->adjust_link)
> +				ds->drv->adjust_link(ds, port, phydev);

This kind of hack here because what you really need is just the link
parameters, but you cannot obtain such information without first
configuring the PHY up to a certain point in genphy_config_init(), and
then have genphy_read_status() copy these values in your phydev structure.

Maybe we should really consider something like this after all:

https://lkml.org/lkml/2015/8/5/490

Or maybe, we should really introduce this "cpu" network device after all
with a dropping xmit function, such that we get ethtool counters to work
on it, and we can also attach it to a PHY device to configure link
parameters?
-- 
Florian

Re: [PATCH net-next 4/9] net: dsa: Allow configuration of CPU & DSA port speeds/duplex

[Andrew Lunn]

> > +		port_dn = cd->port_dn[port];
> > +		if (of_phy_is_fixed_link(port_dn)) {
> > +			ret = of_phy_register_fixed_link(port_dn);
> > +			if (ret) {
> > +				netdev_err(master,
> > +					   "failed to register fixed PHY\n");
> > +				return ret;
> > +			}
> > +			phydev = of_phy_find_device(port_dn);
> > +			genphy_config_init(phydev);
> > +			genphy_read_status(phydev);
> > +			if (ds->drv->adjust_link)
> > +				ds->drv->adjust_link(ds, port, phydev);
> 
> This kind of hack here because what you really need is just the link
> parameters, but you cannot obtain such information without first
> configuring the PHY up to a certain point in genphy_config_init(), and
> then have genphy_read_status() copy these values in your phydev structure.
> 
> Maybe we should really consider something like this after all:
> 
> https://lkml.org/lkml/2015/8/5/490

Hi Florian

This half solves the problem. The nice thing about using the
fixed_link, is that i can just call the adjust_link function with it.
The fixed_phy_status cannot be passed directly to adjust_link. Some
code refactoring or duplication would be needed.
 
> Or maybe, we should really introduce this "cpu" network device after all
> with a dropping xmit function, such that we get ethtool counters to work
> on it, and we can also attach it to a PHY device to configure link
> parameters?

I keep humming and harring about this. I don't really like the idea of
having an interface which you cannot send/receive packets. Yet it
solves a number of problems like this, and gives you access to
statistics and registers in the usual way. If we do it for the CPU
port, we should also do it for the DSA ports. And we probably want the
call for up to return -ENOSUP, just to make it clear it cannot be used
for anything.

      Andrew

Re: [PATCH net-next 4/9] net: dsa: Allow configuration of CPU & DSA port speeds/duplex

[Florian Fainelli]

On 23/08/15 14:24, Andrew Lunn wrote:
>>> +		port_dn = cd->port_dn[port];
>>> +		if (of_phy_is_fixed_link(port_dn)) {
>>> +			ret = of_phy_register_fixed_link(port_dn);
>>> +			if (ret) {
>>> +				netdev_err(master,
>>> +					   "failed to register fixed PHY\n");
>>> +				return ret;
>>> +			}
>>> +			phydev = of_phy_find_device(port_dn);
>>> +			genphy_config_init(phydev);
>>> +			genphy_read_status(phydev);
>>> +			if (ds->drv->adjust_link)
>>> +				ds->drv->adjust_link(ds, port, phydev);
>>
>> This kind of hack here because what you really need is just the link
>> parameters, but you cannot obtain such information without first
>> configuring the PHY up to a certain point in genphy_config_init(), and
>> then have genphy_read_status() copy these values in your phydev structure.
>>
>> Maybe we should really consider something like this after all:
>>
>> https://lkml.org/lkml/2015/8/5/490
> 
> Hi Florian
> 
> This half solves the problem. The nice thing about using the
> fixed_link, is that i can just call the adjust_link function with it.
> The fixed_phy_status cannot be passed directly to adjust_link. Some
> code refactoring or duplication would be needed.

Right, and using an adjust_link callback seems a little cleaner anyway
since you get an abstracted PHY device to work with.

>  
>> Or maybe, we should really introduce this "cpu" network device after all
>> with a dropping xmit function, such that we get ethtool counters to work
>> on it, and we can also attach it to a PHY device to configure link
>> parameters?
> 
> I keep humming and harring about this. I don't really like the idea of
> having an interface which you cannot send/receive packets. Yet it
> solves a number of problems like this, and gives you access to
> statistics and registers in the usual way.

Right that would be my primary motivation and use case as well.

> If we do it for the CPU
> port, we should also do it for the DSA ports. And we probably want the
> call for up to return -ENOSUP, just to make it clear it cannot be used
> for anything.

We should definitively start a separate thread for this, as there might
be real uses cases that are not yet covered that would need a network
device.

Let's go ahead with your patch for now:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 4/9] net: dsa: Allow configuration of CPU & DSA port speeds/duplex

[Florian Fainelli]

Le 08/23/15 14:24, Andrew Lunn a écrit :
>>> +		port_dn = cd->port_dn[port];
>>> +		if (of_phy_is_fixed_link(port_dn)) {
>>> +			ret = of_phy_register_fixed_link(port_dn);
>>> +			if (ret) {
>>> +				netdev_err(master,
>>> +					   "failed to register fixed PHY\n");
>>> +				return ret;
>>> +			}
>>> +			phydev = of_phy_find_device(port_dn);
>>> +			genphy_config_init(phydev);
>>> +			genphy_read_status(phydev);
>>> +			if (ds->drv->adjust_link)
>>> +				ds->drv->adjust_link(ds, port, phydev);
>>
>> This kind of hack here because what you really need is just the link
>> parameters, but you cannot obtain such information without first
>> configuring the PHY up to a certain point in genphy_config_init(), and
>> then have genphy_read_status() copy these values in your phydev structure.
>>
>> Maybe we should really consider something like this after all:
>>
>> https://lkml.org/lkml/2015/8/5/490
> 
> Hi Florian
> 
> This half solves the problem. The nice thing about using the
> fixed_link, is that i can just call the adjust_link function with it.
> The fixed_phy_status cannot be passed directly to adjust_link. Some
> code refactoring or duplication would be needed.

BTW, this is really the reason why I was trying to push for having MDIO
connected switches as PHY devices, because then you get the PHY library
to calculate the advertised/supported intersection for you, and you
could even imagine re-negotiating the CPU port link with the Ethernet MAC.

Maybe I should repost these patches some day once I can get that working
with multiple switches in a tree ;)

>  
>> Or maybe, we should really introduce this "cpu" network device after all
>> with a dropping xmit function, such that we get ethtool counters to work
>> on it, and we can also attach it to a PHY device to configure link
>> parameters?
> 
> I keep humming and harring about this. I don't really like the idea of
> having an interface which you cannot send/receive packets. Yet it
> solves a number of problems like this, and gives you access to
> statistics and registers in the usual way. If we do it for the CPU
> port, we should also do it for the DSA ports. And we probably want the
> call for up to return -ENOSUP, just to make it clear it cannot be used
> for anything.
> 
>       Andrew
> 


-- 
Florian

[PATCH net-next 5/9] net: dsa: Allow DSA and CPU ports to have a phy-mode property

[Andrew Lunn]

It can be useful for DSA and CPU ports to have a phy-mode property, in
particular to specify RGMII delays. Parse the property and set it in
the fixed-link phydev.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/dsa.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index afff17341b73..76e3800765f8 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -181,7 +181,7 @@ static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device *master)
 	struct dsa_chip_data *cd = ds->pd;
 	struct device_node *port_dn;
 	struct phy_device *phydev;
-	int ret, port;
+	int ret, port, mode;
 
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
@@ -196,6 +196,12 @@ static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device *master)
 				return ret;
 			}
 			phydev = of_phy_find_device(port_dn);
+
+			mode = of_get_phy_mode(port_dn);
+			if (mode < 0)
+				mode = PHY_INTERFACE_MODE_NA;
+			phydev->interface = mode;
+
 			genphy_config_init(phydev);
 			genphy_read_status(phydev);
 			if (ds->drv->adjust_link)
-- 
2.5.0

Re: [PATCH net-next 5/9] net: dsa: Allow DSA and CPU ports to have a phy-mode property

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> It can be useful for DSA and CPU ports to have a phy-mode property, in
> particular to specify RGMII delays. Parse the property and set it in
> the fixed-link phydev.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 6/9] dsa: mv88e6xxx: Set the RGMII delay based on phy interface

[Andrew Lunn]

Some Marvell switches allow the RGMII Rx and Tx clock to be delayed
when the port is using RGMII. Have the adjust_link function look at
the phy interface type and enable this delay as requested.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
 drivers/net/dsa/mv88e6xxx.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 7901db6503b4..f5af368751b2 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
 	if (phydev->duplex == DUPLEX_FULL)
 		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
 
+	if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
+	    (port >= ps->num_ports - 2)) {
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+			reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK;
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+			reg |= PORT_PCS_CTRL_RGMII_DELAY_TXCLK;
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+			reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK |
+				PORT_PCS_CTRL_RGMII_DELAY_TXCLK);
+	}
 	_mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg);
 
 out:
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 79003c55fe62..9b6f3d9d5ae1 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -46,6 +46,8 @@
 #define PORT_STATUS_TX_PAUSED	BIT(5)
 #define PORT_STATUS_FLOW_CTRL	BIT(4)
 #define PORT_PCS_CTRL		0x01
+#define PORT_PCS_CTRL_RGMII_DELAY_RXCLK	BIT(15)
+#define PORT_PCS_CTRL_RGMII_DELAY_TXCLK	BIT(14)
 #define PORT_PCS_CTRL_FC		BIT(7)
 #define PORT_PCS_CTRL_FORCE_FC		BIT(6)
 #define PORT_PCS_CTRL_LINK_UP		BIT(5)
-- 
2.5.0

Re: [PATCH net-next 6/9] dsa: mv88e6xxx: Set the RGMII delay based on phy interface

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> Some Marvell switches allow the RGMII Rx and Tx clock to be delayed
> when the port is using RGMII. Have the adjust_link function look at
> the phy interface type and enable this delay as requested.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
>  drivers/net/dsa/mv88e6xxx.h |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 7901db6503b4..f5af368751b2 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
>  	if (phydev->duplex == DUPLEX_FULL)
>  		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
>  
> +	if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
> +	    (port >= ps->num_ports - 2)) {

Are we positive that the last two ports of a switch are going to be
RGMII capable or is this something that should be moved to Device Tree /
platform data to account for different switch families? Maybe having a
bitmask of RGMII capable ports stored in "ps" would be good enough?

> +		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> +			reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK;
> +		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> +			reg |= PORT_PCS_CTRL_RGMII_DELAY_TXCLK;
> +		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> +			reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK |
> +				PORT_PCS_CTRL_RGMII_DELAY_TXCLK);
> +	}
>  	_mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg);
>  
>  out:
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 79003c55fe62..9b6f3d9d5ae1 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -46,6 +46,8 @@
>  #define PORT_STATUS_TX_PAUSED	BIT(5)
>  #define PORT_STATUS_FLOW_CTRL	BIT(4)
>  #define PORT_PCS_CTRL		0x01
> +#define PORT_PCS_CTRL_RGMII_DELAY_RXCLK	BIT(15)
> +#define PORT_PCS_CTRL_RGMII_DELAY_TXCLK	BIT(14)
>  #define PORT_PCS_CTRL_FC		BIT(7)
>  #define PORT_PCS_CTRL_FORCE_FC		BIT(6)
>  #define PORT_PCS_CTRL_LINK_UP		BIT(5)
> 


-- 
Florian

Re: [PATCH net-next 6/9] dsa: mv88e6xxx: Set the RGMII delay based on phy interface

[Andrew Lunn]

On Sun, Aug 23, 2015 at 11:44:01AM -0700, Florian Fainelli wrote:
> Le 08/23/15 02:46, Andrew Lunn a écrit :
> > Some Marvell switches allow the RGMII Rx and Tx clock to be delayed
> > when the port is using RGMII. Have the adjust_link function look at
> > the phy interface type and enable this delay as requested.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
> >  drivers/net/dsa/mv88e6xxx.h |  2 ++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> > index 7901db6503b4..f5af368751b2 100644
> > --- a/drivers/net/dsa/mv88e6xxx.c
> > +++ b/drivers/net/dsa/mv88e6xxx.c
> > @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
> >  	if (phydev->duplex == DUPLEX_FULL)
> >  		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
> >  
> > +	if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
> > +	    (port >= ps->num_ports - 2)) {
> 
> Are we positive that the last two ports of a switch are going to be
> RGMII capable or is this something that should be moved to Device Tree /
> platform data to account for different switch families? Maybe having a
> bitmask of RGMII capable ports stored in "ps" would be good enough?

Hi Florian

For these two families, this is correct. And it is a property of the
switch, not the board, so should not be in DT. Other families are
different. Older ones are Fast Ethernet only. Some don't have any
RGMII ports, etc. It could be with time, this condition gets messy, at
which point, a bitmask in ps would make sense. But is it justified
now?

Thanks
      Andrew

Re: [PATCH net-next 6/9] dsa: mv88e6xxx: Set the RGMII delay based on phy interface

[Florian Fainelli]

On 23/08/15 14:10, Andrew Lunn wrote:
> On Sun, Aug 23, 2015 at 11:44:01AM -0700, Florian Fainelli wrote:
>> Le 08/23/15 02:46, Andrew Lunn a écrit :
>>> Some Marvell switches allow the RGMII Rx and Tx clock to be delayed
>>> when the port is using RGMII. Have the adjust_link function look at
>>> the phy interface type and enable this delay as requested.
>>>
>>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>>> ---
>>>  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
>>>  drivers/net/dsa/mv88e6xxx.h |  2 ++
>>>  2 files changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
>>> index 7901db6503b4..f5af368751b2 100644
>>> --- a/drivers/net/dsa/mv88e6xxx.c
>>> +++ b/drivers/net/dsa/mv88e6xxx.c
>>> @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
>>>  	if (phydev->duplex == DUPLEX_FULL)
>>>  		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
>>>  
>>> +	if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
>>> +	    (port >= ps->num_ports - 2)) {
>>
>> Are we positive that the last two ports of a switch are going to be
>> RGMII capable or is this something that should be moved to Device Tree /
>> platform data to account for different switch families? Maybe having a
>> bitmask of RGMII capable ports stored in "ps" would be good enough?
> 
> Hi Florian
> 
> For these two families, this is correct. And it is a property of the
> switch, not the board, so should not be in DT. Other families are
> different. Older ones are Fast Ethernet only. Some don't have any
> RGMII ports, etc. It could be with time, this condition gets messy, at
> which point, a bitmask in ps would make sense. But is it justified
> now?

Sure, I think for now this patch is good as-is, I was mostly curious
whether the assumption about the last 2 ports of the switch being RGMII
would hold for a while, and it looks like it will. With that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 7/9] dsa: mv88e6xxx: Don't poll forced interfaces for state changes

[Andrew Lunn]

When polling for link status, don't consider ports which have a forced
link. Such ports don't monitor their phy or may not even have a phy.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index f5af368751b2..537bd1f74fbf 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -395,6 +395,7 @@ void mv88e6xxx_poll_link(struct dsa_switch *ds)
 	for (i = 0; i < DSA_MAX_PORTS; i++) {
 		struct net_device *dev;
 		int uninitialized_var(port_status);
+		int pcs_ctrl;
 		int link;
 		int speed;
 		int duplex;
@@ -404,6 +405,10 @@ void mv88e6xxx_poll_link(struct dsa_switch *ds)
 		if (dev == NULL)
 			continue;
 
+		pcs_ctrl = mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_PCS_CTRL);
+		if (pcs_ctrl < 0 || pcs_ctrl & PORT_PCS_CTRL_FORCE_LINK)
+			continue;
+
 		link = 0;
 		if (dev->flags & IFF_UP) {
 			port_status = mv88e6xxx_reg_read(ds, REG_PORT(i),
-- 
2.5.0

Re: [PATCH net-next 7/9] dsa: mv88e6xxx: Don't poll forced interfaces for state changes

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> When polling for link status, don't consider ports which have a forced
> link. Such ports don't monitor their phy or may not even have a phy.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 8/9] phy: fixed_phy: Add gpio to determine link up/down.

[Andrew Lunn]

An SFP module may have a link up/down status pin which can be
connection to a GPIO line of the host. Add support for reading such an
GPIO in the fixed_phy driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../devicetree/bindings/net/fixed-link.txt         | 14 +++++++++++-
 Documentation/networking/stmmac.txt                |  2 +-
 arch/m68k/coldfire/m5272.c                         |  2 +-
 arch/mips/ar7/platform.c                           |  5 +++--
 arch/mips/bcm47xx/setup.c                          |  2 +-
 drivers/net/ethernet/broadcom/genet/bcmmii.c       |  2 +-
 drivers/net/phy/fixed_phy.c                        | 26 +++++++++++++++++++---
 drivers/of/of_mdio.c                               | 13 ++++++++---
 include/linux/phy_fixed.h                          |  8 +++++--
 9 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index 82bf7e0f47b6..ec5d889fe3d8 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -17,6 +17,8 @@ properties:
   enabled.
 * 'asym-pause' (boolean, optional), to indicate that asym_pause should
   be enabled.
+* 'link-gpios' ('gpio-list', optional), to indicate if a gpio can be read
+  to determine if the link is up.
 
 Old, deprecated 'fixed-link' binding:
 
@@ -30,7 +32,7 @@ Old, deprecated 'fixed-link' binding:
   - e: asymmetric pause configuration: 0 for no asymmetric pause, 1 for
     asymmetric pause
 
-Example:
+Examples:
 
 ethernet@0 {
 	...
@@ -40,3 +42,13 @@ ethernet@0 {
 	};
 	...
 };
+
+ethernet@1 {
+	...
+	fixed-link {
+	      speed = <1000>;
+	      pause;
+	      link-gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+	};
+	...
+};
diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
index 2903b1cf4d70..d64a14714236 100644
--- a/Documentation/networking/stmmac.txt
+++ b/Documentation/networking/stmmac.txt
@@ -254,7 +254,7 @@ static struct fixed_phy_status stmmac0_fixed_phy_status = {
 
 During the board's device_init we can configure the first
 MAC for fixed_link by calling:
-  fixed_phy_add(PHY_POLL, 1, &stmmac0_fixed_phy_status));)
+  fixed_phy_add(PHY_POLL, 1, &stmmac0_fixed_phy_status, -1);
 and the second one, with a real PHY device attached to the bus,
 by using the stmmac_mdio_bus_data structure (to provide the id, the
 reset procedure etc).
diff --git a/arch/m68k/coldfire/m5272.c b/arch/m68k/coldfire/m5272.c
index b15219ed22bf..c525e4c08f84 100644
--- a/arch/m68k/coldfire/m5272.c
+++ b/arch/m68k/coldfire/m5272.c
@@ -126,7 +126,7 @@ static struct fixed_phy_status nettel_fixed_phy_status __initdata = {
 static int __init init_BSP(void)
 {
 	m5272_uarts_init();
-	fixed_phy_add(PHY_POLL, 0, &nettel_fixed_phy_status);
+	fixed_phy_add(PHY_POLL, 0, &nettel_fixed_phy_status, -1);
 	return 0;
 }
 
diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
index be9ff1673ded..298b97715d5f 100644
--- a/arch/mips/ar7/platform.c
+++ b/arch/mips/ar7/platform.c
@@ -679,7 +679,8 @@ static int __init ar7_register_devices(void)
 	}
 
 	if (ar7_has_high_cpmac()) {
-		res = fixed_phy_add(PHY_POLL, cpmac_high.id, &fixed_phy_status);
+		res = fixed_phy_add(PHY_POLL, cpmac_high.id,
+				    &fixed_phy_status, -1);
 		if (!res) {
 			cpmac_get_mac(1, cpmac_high_data.dev_addr);
 
@@ -692,7 +693,7 @@ static int __init ar7_register_devices(void)
 	} else
 		cpmac_low_data.phy_mask = 0xffffffff;
 
-	res = fixed_phy_add(PHY_POLL, cpmac_low.id, &fixed_phy_status);
+	res = fixed_phy_add(PHY_POLL, cpmac_low.id, &fixed_phy_status, -1);
 	if (!res) {
 		cpmac_get_mac(0, cpmac_low_data.dev_addr);
 		res = platform_device_register(&cpmac_low);
diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index 98c075f81795..17503a05938e 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -263,7 +263,7 @@ static int __init bcm47xx_register_bus_complete(void)
 	bcm47xx_leds_register();
 	bcm47xx_workarounds();
 
-	fixed_phy_add(PHY_POLL, 0, &bcm47xx_fixed_phy_status);
+	fixed_phy_add(PHY_POLL, 0, &bcm47xx_fixed_phy_status, -1);
 	return 0;
 }
 device_initcall(bcm47xx_register_bus_complete);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b3679ad1c1c7..c8affad76f36 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -585,7 +585,7 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
 			.asym_pause = 0,
 		};
 
-		phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
+		phydev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
 		if (!phydev || IS_ERR(phydev)) {
 			dev_err(kdev, "failed to register fixed PHY device\n");
 			return -ENODEV;
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 7244336dee7a..14ab80899e77 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -22,6 +22,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/gpio.h>
 
 #define MII_REGS_NUM 29
 
@@ -38,6 +39,7 @@ struct fixed_phy {
 	struct fixed_phy_status status;
 	int (*link_update)(struct net_device *, struct fixed_phy_status *);
 	struct list_head node;
+	int link_gpio;
 };
 
 static struct platform_device *pdev;
@@ -52,6 +54,9 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	u16 lpagb = 0;
 	u16 lpa = 0;
 
+	if (gpio_is_valid(fp->link_gpio))
+		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
+
 	if (!fp->status.link)
 		goto done;
 	bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
@@ -215,7 +220,8 @@ int fixed_phy_update_state(struct phy_device *phydev,
 EXPORT_SYMBOL(fixed_phy_update_state);
 
 int fixed_phy_add(unsigned int irq, int phy_addr,
-		  struct fixed_phy_status *status)
+		  struct fixed_phy_status *status,
+		  int link_gpio)
 {
 	int ret;
 	struct fixed_mdio_bus *fmb = &platform_fmb;
@@ -231,15 +237,26 @@ int fixed_phy_add(unsigned int irq, int phy_addr,
 
 	fp->addr = phy_addr;
 	fp->status = *status;
+	fp->link_gpio = link_gpio;
+
+	if (gpio_is_valid(fp->link_gpio)) {
+		ret = gpio_request_one(fp->link_gpio, GPIOF_DIR_IN,
+				       "fixed-link-gpio-link");
+		if (ret)
+			goto err_regs;
+	}
 
 	ret = fixed_phy_update_regs(fp);
 	if (ret)
-		goto err_regs;
+		goto err_gpio;
 
 	list_add_tail(&fp->node, &fmb->phys);
 
 	return 0;
 
+err_gpio:
+	if (gpio_is_valid(fp->link_gpio))
+		gpio_free(fp->link_gpio);
 err_regs:
 	kfree(fp);
 	return ret;
@@ -254,6 +271,8 @@ void fixed_phy_del(int phy_addr)
 	list_for_each_entry_safe(fp, tmp, &fmb->phys, node) {
 		if (fp->addr == phy_addr) {
 			list_del(&fp->node);
+			if (gpio_is_valid(fp->link_gpio))
+				gpio_free(fp->link_gpio);
 			kfree(fp);
 			return;
 		}
@@ -266,6 +285,7 @@ static DEFINE_SPINLOCK(phy_fixed_addr_lock);
 
 struct phy_device *fixed_phy_register(unsigned int irq,
 				      struct fixed_phy_status *status,
+				      int link_gpio,
 				      struct device_node *np)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
@@ -282,7 +302,7 @@ struct phy_device *fixed_phy_register(unsigned int irq,
 	phy_addr = phy_fixed_addr++;
 	spin_unlock(&phy_fixed_addr_lock);
 
-	ret = fixed_phy_add(PHY_POLL, phy_addr, status);
+	ret = fixed_phy_add(PHY_POLL, phy_addr, status, link_gpio);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 7c8c23cc6896..1350fa25cdb0 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -16,6 +16,7 @@
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/module.h>
@@ -294,6 +295,7 @@ 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 link_gpio;
 	int len, err;
 	struct phy_device *phy;
 	const char *managed;
@@ -302,7 +304,7 @@ int of_phy_register_fixed_link(struct device_node *np)
 	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);
+			phy = fixed_phy_register(PHY_POLL, &status, -1, np);
 			return IS_ERR(phy) ? PTR_ERR(phy) : 0;
 		}
 	}
@@ -318,8 +320,13 @@ int of_phy_register_fixed_link(struct device_node *np)
 		status.pause = of_property_read_bool(fixed_link_node, "pause");
 		status.asym_pause = of_property_read_bool(fixed_link_node,
 							  "asym-pause");
+		link_gpio = of_get_named_gpio_flags(fixed_link_node,
+						    "link-gpios", 0, NULL);
 		of_node_put(fixed_link_node);
-		phy = fixed_phy_register(PHY_POLL, &status, np);
+		if (link_gpio == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		phy = fixed_phy_register(PHY_POLL, &status, link_gpio, np);
 		return IS_ERR(phy) ? PTR_ERR(phy) : 0;
 	}
 
@@ -331,7 +338,7 @@ int of_phy_register_fixed_link(struct device_node *np)
 		status.speed = be32_to_cpu(fixed_link_prop[2]);
 		status.pause = be32_to_cpu(fixed_link_prop[3]);
 		status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
-		phy = fixed_phy_register(PHY_POLL, &status, np);
+		phy = fixed_phy_register(PHY_POLL, &status, -1, np);
 		return IS_ERR(phy) ? PTR_ERR(phy) : 0;
 	}
 
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index fe5732d53eda..2400d2ea4f34 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -13,9 +13,11 @@ struct device_node;
 
 #if IS_ENABLED(CONFIG_FIXED_PHY)
 extern int fixed_phy_add(unsigned int irq, int phy_id,
-			 struct fixed_phy_status *status);
+			 struct fixed_phy_status *status,
+			 int link_gpio);
 extern struct phy_device *fixed_phy_register(unsigned int irq,
 					     struct fixed_phy_status *status,
+					     int link_gpio,
 					     struct device_node *np);
 extern void fixed_phy_del(int phy_addr);
 extern int fixed_phy_set_link_update(struct phy_device *phydev,
@@ -26,12 +28,14 @@ extern int fixed_phy_update_state(struct phy_device *phydev,
 			   const struct fixed_phy_status *changed);
 #else
 static inline int fixed_phy_add(unsigned int irq, int phy_id,
-				struct fixed_phy_status *status)
+				struct fixed_phy_status *status,
+				int link_gpio)
 {
 	return -ENODEV;
 }
 static inline struct phy_device *fixed_phy_register(unsigned int irq,
 						struct fixed_phy_status *status,
+						int gpio_link,
 						struct device_node *np)
 {
 	return ERR_PTR(-ENODEV);
-- 
2.5.0

Re: [PATCH net-next 8/9] phy: fixed_phy: Add gpio to determine link up/down.

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> An SFP module may have a link up/down status pin which can be
> connection to a GPIO line of the host. Add support for reading such an
> GPIO in the fixed_phy driver.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even when link is down

[Andrew Lunn]

What features a phy supports is masked in genphy_config_init() by
looking at the PHYs BMSR register.

If the link is down, fixed_phy_update_regs() will only set the auto-
negotiation capable bit in BMSR. Thus genphy_config_init() comes to
the conclusion the PHY can only perform 10/Half, and masks out the
higher speed features. If however the link it up, BMSR is set to
indicate the speed the PHY is capable of auto-negotiating, and
genphy_config_init() does not mask out the high speed features.

To fix this, when the link is down, have fixed_phy_update_regs() leave
the link status and auto-negotiation complete bit unset, but set all
the other bits depending on the fixed phy speed.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/fixed_phy.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 14ab80899e77..ff4869222c27 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -57,9 +57,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	if (gpio_is_valid(fp->link_gpio))
 		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
 
-	if (!fp->status.link)
-		goto done;
-	bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
+	if (fp->status.link)
+		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
 
 	if (fp->status.duplex) {
 		bmcr |= BMCR_FULLDPLX;
@@ -111,7 +110,6 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	if (fp->status.asym_pause)
 		lpa |= LPA_PAUSE_ASYM;
 
-done:
 	fp->regs[MII_PHYSID1] = 0;
 	fp->regs[MII_PHYSID2] = 0;
 
-- 
2.5.0

Re: [PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even when link is down

[Florian Fainelli]

Le 08/23/15 02:47, Andrew Lunn a écrit :
> What features a phy supports is masked in genphy_config_init() by
> looking at the PHYs BMSR register.
> 
> If the link is down, fixed_phy_update_regs() will only set the auto-
> negotiation capable bit in BMSR. Thus genphy_config_init() comes to
> the conclusion the PHY can only perform 10/Half, and masks out the
> higher speed features. If however the link it up, BMSR is set to
> indicate the speed the PHY is capable of auto-negotiating, and
> genphy_config_init() does not mask out the high speed features.
> 
> To fix this, when the link is down, have fixed_phy_update_regs() leave
> the link status and auto-negotiation complete bit unset, but set all
> the other bits depending on the fixed phy speed.

This kinds of revert what Staas did in commit
868a4215be9a6d80548ccb74763b883dc99d32a2 ("net: phy: fixed_phy: handle
link-down case"). When the link is down, it does not seem to me like we
can rely on the previous speed and duplex parameters to be considered valid.

Your change does fix a valid use case though... humm.

> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/fixed_phy.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 14ab80899e77..ff4869222c27 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -57,9 +57,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>  	if (gpio_is_valid(fp->link_gpio))
>  		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
>  
> -	if (!fp->status.link)
> -		goto done;
> -	bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
> +	if (fp->status.link)
> +		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>  
>  	if (fp->status.duplex) {
>  		bmcr |= BMCR_FULLDPLX;
> @@ -111,7 +110,6 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>  	if (fp->status.asym_pause)
>  		lpa |= LPA_PAUSE_ASYM;
>  
> -done:
>  	fp->regs[MII_PHYSID1] = 0;
>  	fp->regs[MII_PHYSID2] = 0;
>  
> 


-- 
Florian

Re: [PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even when link is down

[Andrew Lunn]

On Sun, Aug 23, 2015 at 11:40:07AM -0700, Florian Fainelli wrote:
> Le 08/23/15 02:47, Andrew Lunn a écrit :
> > What features a phy supports is masked in genphy_config_init() by
> > looking at the PHYs BMSR register.
> > 
> > If the link is down, fixed_phy_update_regs() will only set the auto-
> > negotiation capable bit in BMSR. Thus genphy_config_init() comes to
> > the conclusion the PHY can only perform 10/Half, and masks out the
> > higher speed features. If however the link it up, BMSR is set to
> > indicate the speed the PHY is capable of auto-negotiating, and
> > genphy_config_init() does not mask out the high speed features.
> > 
> > To fix this, when the link is down, have fixed_phy_update_regs() leave
> > the link status and auto-negotiation complete bit unset, but set all
> > the other bits depending on the fixed phy speed.
> 
> This kinds of revert what Staas did in commit
> 868a4215be9a6d80548ccb74763b883dc99d32a2 ("net: phy: fixed_phy: handle
> link-down case"). When the link is down, it does not seem to me like we
> can rely on the previous speed and duplex parameters to be considered valid.

I must admit to not spending the time needed to understand what all
these bits mean. Are they what we are advertising we are capable off,
or what we have negotiated.

We definitely should not be configuring the phy layer on what we have
negotiated in the past. But what we are advertising should be a good
indication of what we are capable of. Maybe this function needs to set
the advertised features separate from negotiated features? If the link
is down, leave the negotiated at 10/Half, but set the advertised
features based on the fixed-link settings?

	 Andrew

Re: [PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even when link is down

[Andrew Lunn]

On Sun, Aug 23, 2015 at 11:40:07AM -0700, Florian Fainelli wrote:
> Le 08/23/15 02:47, Andrew Lunn a écrit :
> > What features a phy supports is masked in genphy_config_init() by
> > looking at the PHYs BMSR register.
> > 
> > If the link is down, fixed_phy_update_regs() will only set the auto-
> > negotiation capable bit in BMSR. Thus genphy_config_init() comes to
> > the conclusion the PHY can only perform 10/Half, and masks out the
> > higher speed features. If however the link it up, BMSR is set to
> > indicate the speed the PHY is capable of auto-negotiating, and
> > genphy_config_init() does not mask out the high speed features.
> > 
> > To fix this, when the link is down, have fixed_phy_update_regs() leave
> > the link status and auto-negotiation complete bit unset, but set all
> > the other bits depending on the fixed phy speed.
> 
> This kinds of revert what Staas did in commit
> 868a4215be9a6d80548ccb74763b883dc99d32a2 ("net: phy: fixed_phy: handle
> link-down case"). When the link is down, it does not seem to me like we
> can rely on the previous speed and duplex parameters to be considered valid.
> 
> Your change does fix a valid use case though... humm.

Hi Florian

I took at look at Staas fix, and read a bit about what the different
bits mean. I've reworked the patch. I now always set the local phy
capabilities in BMSR, but only set the negotiated speed and link
partner capabilities if the link it up. I also don't error out on
speed=0, unless the link is up.

This works for my use case, and hopefully also Staas.

I will post the new version when we have come to a conclusion about
other open issues.

      Andrew

Re: [PATCH net-next 0/9] DSA port configuration and status

[Florian Fainelli]

Le 08/23/15 02:46, Andrew Lunn a écrit :
> This patchset allows various switch port settings to be configured and
> port status to be sampled. Some of these patches have been posted
> before.
> 
> The first three patches provide infrastructure for configuring a
> switch ports link speed and duplex from a fixed_link phy.
> 
> Patch four then uses this infrastructure to allow the CPU and DSA
> ports of a switch to be configured using a fixed-link property in the
> device tree.

Your changes look very clean and neat!

Since we keep seeing changes around the fixed PHY driver, I am starting
to wonder whether we need something more lightweight than a full fledged
fixed PHY to retrieve link parameters and manage them outside of the PHY
library.

One of the promises of the PHY library is that you will get a number of
things for free, whether you deal with a real PHY device or a fixed PHY
one: ethtool reporting, adjust_link to configure the Ethernet MAC with
detected/hardcoded link settings, automatic carrier settings etc...
Which would have to be repeated in individual drivers utilizing a more
lightweight fixed link code...

> 
> Patches five and six allow a phy-mode property to be specified in the
> device tree, and allow this to be used for configuring RGMII delays.
> 
> Patches seven through nine allow link status, for example that of an
> SFP module, to be read from a gpio.
> 
> Please don't merge these patches until Florian Fainelli has reviewed
> them.
> 
> Andrew Lunn (8):
>   dsa: mv88e6xxx: Allow speed/duplex of port to be configured
>   phy: fixed_phy: Set supported speed in phydev
>   net: dsa: Allow configuration of CPU & DSA port speeds/duplex
>   net: dsa: Allow DSA and CPU ports to have a phy-mode property
>   dsa: mv88e6xxx: Set the RGMII delay based on phy interface
>   dsa: mv88e6xxx: Don't poll forced interfaces for state changes
>   phy: fixed_phy: Add gpio to determine link up/down.
>   phy: fixed_phy: Set phy capabilities even when link is down
> 
> Florian Fainelli (1):
>   net: phy: Allow PHY devices to identify themselves as Ethernet
>     switches, etc.
> 
>  .../devicetree/bindings/net/fixed-link.txt         | 14 ++++-
>  Documentation/networking/stmmac.txt                |  2 +-
>  arch/m68k/coldfire/m5272.c                         |  2 +-
>  arch/mips/ar7/platform.c                           |  5 +-
>  arch/mips/bcm47xx/setup.c                          |  2 +-
>  drivers/net/dsa/mv88e6123_61_65.c                  |  1 +
>  drivers/net/dsa/mv88e6131.c                        |  1 +
>  drivers/net/dsa/mv88e6171.c                        |  1 +
>  drivers/net/dsa/mv88e6352.c                        |  1 +
>  drivers/net/dsa/mv88e6xxx.c                        | 73 ++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx.h                        |  4 ++
>  drivers/net/ethernet/broadcom/genet/bcmmii.c       |  2 +-
>  drivers/net/phy/fixed_phy.c                        | 45 ++++++++++---
>  drivers/of/of_mdio.c                               | 13 +++-
>  include/linux/phy.h                                | 12 ++++
>  include/linux/phy_fixed.h                          |  8 ++-
>  net/dsa/dsa.c                                      | 43 +++++++++++++
>  17 files changed, 210 insertions(+), 19 deletions(-)
> 


-- 
Florian

Re: [PATCH net-next 0/9] DSA port configuration and status

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 23 Aug 2015 11:46:51 +0200

> This patchset allows various switch port settings to be configured and
> port status to be sampled. Some of these patches have been posted
> before.
> 
> The first three patches provide infrastructure for configuring a
> switch ports link speed and duplex from a fixed_link phy.
> 
> Patch four then uses this infrastructure to allow the CPU and DSA
> ports of a switch to be configured using a fixed-link property in the
> device tree.
> 
> Patches five and six allow a phy-mode property to be specified in the
> device tree, and allow this to be used for configuring RGMII delays.
> 
> Patches seven through nine allow link status, for example that of an
> SFP module, to be read from a gpio.
> 
> Please don't merge these patches until Florian Fainelli has reviewed
> them.

It looks to me like there will be at least one more revision to this
series, so I'm not applying this version.

Re: [PATCH net-next 0/9] DSA port configuration and status

[Andrew Lunn]

> It looks to me like there will be at least one more revision to this
> series, so I'm not applying this version.

Correct, and thanks.

	 Andrew