> > > +static void rswitch_adjust_link(struct net_device *ndev) > > > +{ > > > + struct rswitch_device *rdev = netdev_priv(ndev); > > > + struct phy_device *phydev = ndev->phydev; > > > + > > > + if (phydev->link != rdev->etha->link) { > > > + phy_print_status(phydev); > > > + rdev->etha->link = phydev->link; > > > + } > > > > Given that the SERDES supports 100 and 1G, it seems odd you don't need > > to do anything here. > > Indeed. However, unfortunately, the current hardware cannot change the speed at runtime... > So, I'll add such comments here. Then you need to tell phylib about this. MAC drivers with limitations often call phy_set_max_speed() to remove higher speeds which the PHY can support, but the MAC cannot. You need to go further and remove lower speeds as well. The autoneg in the PHY should then only work for the speeds you actually support. > > > +static int rswitch_serdes_common_setting(void __iomem *addr0, > > > + enum rswitch_serdes_mode mode) > > > +{ > > > + switch (mode) { > > > + case SGMII: > > > + rswitch_serdes_write32(addr0, 0x0244, 0x180, 0x97); > > > + rswitch_serdes_write32(addr0, 0x01d0, 0x180, 0x60); > > > + rswitch_serdes_write32(addr0, 0x01d8, 0x180, 0x2200); > > > + rswitch_serdes_write32(addr0, 0x01d4, 0x180, 0); > > > + rswitch_serdes_write32(addr0, 0x01e0, 0x180, 0x3d); > > > > Please add #defines for all these magic numbers. > > I should have added comments before though, the datasheet also describes > such magic numbers like below... > Step S.4.1 bank 0x180 address = 0x0244 data = 0x00000097 > Step S.4.2 bank 0x180 address = 0x01d0 data = 0x00000060 > ... > > So, perhaps we can define like the followings: > #define SERDES_BANK_180 0x180 > > #define SERDES_STEP_S_4_1_ADDR 0x0244 > #define SERDES_STEP_S_4_1_DATA 0x00000097 Not really any better. Better to comment that you have no idea what any of this does, it is all black magic. Andrew