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