diff mbox series

[RFC,1/6] net: dsa: microchip: Prepare PHY for proper advertisement

Message ID 1543880097-7106-2-git-send-email-Tristram.Ha@microchip.com
State RFC, archived
Delegated to: David Miller
Headers show
Series net: dsa: microchip: Modify KSZ9477 DSA driver to support different tail tag formats | expand

Commit Message

Tristram.Ha@microchip.com Dec. 3, 2018, 11:34 p.m. UTC
From: Tristram Ha <Tristram.Ha@microchip.com>

Prepare PHY for proper advertisement and get link status for the port.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Woojung Huh <Woojung.Huh@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 12 ++++++++++++
 drivers/net/dsa/microchip/ksz_common.c | 17 +++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  2 ++
 drivers/net/dsa/microchip/ksz_priv.h   |  2 ++
 4 files changed, 33 insertions(+)

Comments

Andrew Lunn Dec. 5, 2018, 5:40 p.m. UTC | #1
On Mon, Dec 03, 2018 at 03:34:52PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Prepare PHY for proper advertisement and get link status for the port.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> Reviewed-by: Woojung Huh <Woojung.Huh@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz9477.c    | 12 ++++++++++++
>  drivers/net/dsa/microchip/ksz_common.c | 17 +++++++++++++++++
>  drivers/net/dsa/microchip/ksz_common.h |  2 ++
>  drivers/net/dsa/microchip/ksz_priv.h   |  2 ++
>  4 files changed, 33 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 0684657..98aa25e 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -969,6 +969,16 @@ static void ksz9477_port_mirror_del(struct dsa_switch *ds, int port,
>  			     PORT_MIRROR_SNIFFER, false);
>  }
>  
> +static void ksz9477_phy_setup(struct ksz_device *dev, int port,
> +			      struct phy_device *phy)
> +{
> +	if (port < dev->phy_port_cnt) {
> +		/* SUPPORTED_Asym_Pause and SUPPORTED_Pause can be removed to
> +		 * disable flow control when rate limiting is used.
> +		 */
> +	}

Hi Tristram

Is this meant to be a TODO comment?

What is supposed to happen here is that all forms of pause are disable
by default. The MAC driver needs to enable what it supports by calling
phy_support_sym_pause() or phy_support_asym_pause().

Ah, is this because there is not a real PHY driver?

Thanks
	Andrew
Tristram.Ha@microchip.com Dec. 6, 2018, 8:31 p.m. UTC | #2
> > +static void ksz9477_phy_setup(struct ksz_device *dev, int port,
> > +			      struct phy_device *phy)
> > +{
> > +	if (port < dev->phy_port_cnt) {
> > +		/* SUPPORTED_Asym_Pause and SUPPORTED_Pause can be
> removed to
> > +		 * disable flow control when rate limiting is used.
> > +		 */
> > +	}
> 
> Hi Tristram
> 
> Is this meant to be a TODO comment?
> 
> What is supposed to happen here is that all forms of pause are disable
> by default. The MAC driver needs to enable what it supports by calling
> phy_support_sym_pause() or phy_support_asym_pause().
> 
> Ah, is this because there is not a real PHY driver?

The kernel has been changed so I am not sure about the current behavior.
I would like to turn on flow control by default.  Before I just assigned
"supported" to "advertising."  I know linkmode_copy is being used now for that.
But last time I checked "advertising" is already the same as "supported."

There will be a situation that flow control should not be turned on as the switch
uses bandwidth control to limit outgoing traffic.

The issue is actually becoming more complex as KSZ9477 has a variant which
does not support gigabit speed, although the same PHY device id is being used.
That means the driver has to fake it by returning a different id and also registering
a different PHY driver to handle that.  Marketing also likes to display the correct chip
name during kernel booting so that users do not get confused.
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 0684657..98aa25e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -969,6 +969,16 @@  static void ksz9477_port_mirror_del(struct dsa_switch *ds, int port,
 			     PORT_MIRROR_SNIFFER, false);
 }
 
+static void ksz9477_phy_setup(struct ksz_device *dev, int port,
+			      struct phy_device *phy)
+{
+	if (port < dev->phy_port_cnt) {
+		/* SUPPORTED_Asym_Pause and SUPPORTED_Pause can be removed to
+		 * disable flow control when rate limiting is used.
+		 */
+	}
+}
+
 static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 {
 	u8 data8;
@@ -1151,6 +1161,7 @@  static int ksz9477_setup(struct dsa_switch *ds)
 	.setup			= ksz9477_setup,
 	.phy_read		= ksz9477_phy_read16,
 	.phy_write		= ksz9477_phy_write16,
+	.adjust_link		= ksz_adjust_link,
 	.port_enable		= ksz_enable_port,
 	.port_disable		= ksz_disable_port,
 	.get_strings		= ksz9477_get_strings,
@@ -1298,6 +1309,7 @@  static void ksz9477_switch_exit(struct ksz_device *dev)
 	.get_port_addr = ksz9477_get_port_addr,
 	.cfg_port_member = ksz9477_cfg_port_member,
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
+	.phy_setup = ksz9477_phy_setup,
 	.port_setup = ksz9477_port_setup,
 	.shutdown = ksz9477_reset_switch,
 	.detect = ksz9477_switch_detect,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9705808..39adc57 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -61,6 +61,22 @@  int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
 }
 EXPORT_SYMBOL_GPL(ksz_phy_write16);
 
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *p = &dev->ports[port];
+
+	if (phydev->link) {
+		dev->live_ports |= (1 << port) & dev->on_ports;
+	} else if (p->phydev.link) {
+		p->link_just_down = 1;
+		dev->live_ports &= ~(1 << port);
+	}
+	p->phydev = *phydev;
+}
+EXPORT_SYMBOL_GPL(ksz_adjust_link);
+
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
 {
 	struct ksz_device *dev = ds->priv;
@@ -238,6 +254,7 @@  int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 
 	/* setup slave port */
 	dev->dev_ops->port_setup(dev, port, false);
+	dev->dev_ops->phy_setup(dev, port, phy);
 
 	/* port_stp_state_set() will be called after to enable the port so
 	 * there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 2dd832d..206b313 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -13,6 +13,8 @@ 
 
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg);
 int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val);
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev);
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br);
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index a38ff08..fcb75a8 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -135,6 +135,8 @@  struct ksz_dev_ops {
 	u32 (*get_port_addr)(int port, int offset);
 	void (*cfg_port_member)(struct ksz_device *dev, int port, u8 member);
 	void (*flush_dyn_mac_table)(struct ksz_device *dev, int port);
+	void (*phy_setup)(struct ksz_device *dev, int port,
+			  struct phy_device *phy);
 	void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port);
 	void (*r_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 *val);
 	void (*w_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 val);