Hi Patrick,

Two comments below.

Patrick Uiterwijk <patr...@puiterwijk.org> writes:

> +static int mv88e6xxx_power_on_serdes(struct dsa_switch *ds)

Since this function assumes the SMI lock is already held, its name
should be prefixed with _ by convention (_mv88e6xxx_power_on_serdes).

> +{
> +     int ret;
> +
> +     ret = _mv88e6xxx_phy_page_read(ds, REG_FIBER_SERDES, PAGE_FIBER_SERDES,
> +                                    MII_BMCR);
> +     if (ret < 0)
> +             return ret;
> +
> +     if (ret & BMCR_PDOWN) {
> +             ret = ret & ~BMCR_PDOWN;

                ret &= ~BMCR_PDOWN;

> +             ret = _mv88e6xxx_phy_page_write(ds, REG_FIBER_SERDES,
> +                                             PAGE_FIBER_SERDES, MII_BMCR,
> +                                             ret);
> +     }
> +
> +     return ret;
> +}

Thanks,
Vivien

Reply via email to