Hi Tristram
> +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.
> + */
> + phy->advertising = phy->supported;
> + }
This looks a bit odd. phy_probe() does the same:
/* Start out supporting everything. Eventually,
* a controller will attach, and may modify one
* or both of these values
*/
phydev->supported = phydrv->features;
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
You don't modify anything here, so i don't see why it is needed.
> +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) {
> + p->speed = phydev->speed;
> + p->duplex = phydev->duplex;
> + p->flow_ctrl = phydev->pause;
> + p->link_up = 1;
> + dev->live_ports |= (1 << port) & dev->on_ports;
> + } else if (p->link_up) {
> + p->link_up = 0;
> + p->link_down = 1;
> + dev->live_ports &= ~(1 << port);
> + }
The link_down looks odd. If you are setting link_up to 1, should
link_down also be set to 0? Can it be both up and down at the same
time?
Andrew
