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 |
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
> > +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 --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);