> +static void ksz9477_phy_setup(struct ksz_device *dev, int port,
> + struct phy_device *phy)
> +{
> + /* ETHTOOL_LINK_MODE_Pause_BIT and ETHTOOL_LINK_MODE_Asym_Pause_BIT
> + * can be removed to disable flow control when rate limiting is used.
> + */
> + if (port < dev->phy_port_cnt) {
> + /* The MAC actually cannot run in 1000 half-duplex mode. */
> + phy_remove_link_mode(phy,
> + ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
> + }
Hi Tristram
The comment about pause does not seem to match the code.
> +}
> +
> static void ksz9477_port_setup(struct ksz_device *dev, int port, bool
> cpu_port)
> {
> u8 data8;
> @@ -1151,6 +1164,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 +1312,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 8a5111f..a57bda7 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2,7 +2,7 @@
> /*
> * Microchip switch driver main logic
> *
> - * Copyright (C) 2017-2018 Microchip Technology Inc.
> + * Copyright (C) 2017-2019 Microchip Technology Inc.
> */
>
> #include <linux/delay.h>
> @@ -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);
> + }
It is not very easy to understand what on_ports, live_ports and
link_just_down are all about. Could you simplify this, and make the
code symmetric? ksz_enable_port does not touch any of these, but
ksz_disable_port does?
grep live_ports *
ksz9477.c: dev->live_ports = dev->host_mask;
ksz9477.c: dev->live_ports |= (1 << port);
ksz_common.c: dev->live_ports &= ~(1 << port);
ksz_priv.h: u16 live_ports;
So live_ports is not actually used for anything.
> + p->phydev = *phydev;
This last statement seems like it belongs in phy_setup, which gets
called as part of port_enable.
