> On Thu, Mar 26, 2020 at 03:51:19PM +0200, Florinel Iordache wrote: > > Add support for backplane kr generic driver including link training > > (ieee802.3ap/ba) and fixed equalization algorithm > > > > Signed-off-by: Florinel Iordache <florinel.iordache@xxxxxxx> > > +/* Read AN Link Status */ > > +static int is_an_link_up(struct phy_device *bpphy) { > > + struct backplane_phy_info *bp_phy = bpphy->priv; > > + int ret, val = 0; > > + > > + mutex_lock(&bp_phy->bpphy_lock); > > + > > + /* Read twice because Link_Status is LL (Latched Low) bit */ > > + val = phy_read_mmd(bpphy, MDIO_MMD_AN, bp_phy- > >bp_dev.mdio.an_status); > > + val = phy_read_mmd(bpphy, MDIO_MMD_AN, > > + bp_phy->bp_dev.mdio.an_status); > > Why not just > > val = phy_read_mmd(bpphy, MDIO_MMD_AN, MDIO_CTRL1); > > Or is your hardware not actually conformant to the standard? > > There has also been a lot of discussion of reading the status twice is correct or > not. Don't you care the link has briefly gone down and up again? > > Andrew This could be changed to use directly the MDIO_STAT1 in order to read AN status (and use MDIO_CTRL1 for writing the control register) but this is more flexible and more readable since we defined the structure kr_mdio_info that contains all registers offsets required by backplane driver like: LT(link training) registers, AN registers, PMD registers. So we wanted to put all these together to be clear that all these offsets are essential for backplane driver and they can be setup automatically by calling the function: backplane_setup_mdio_c45. + void backplane_setup_mdio_c45(struct backplane_kr_info *bpkr) + /* KX/KR AN registers: IEEE802.3 Clause 45 (MMD 7) */ + bpkr->mdio.an_control = MDIO_CTRL1; + bpkr->mdio.an_status = MDIO_STAT1; + bpkr->mdio.an_ad_ability_0 = MDIO_PMA_EXTABLE_10GBKR; + bpkr->mdio.an_ad_ability_1 = MDIO_PMA_EXTABLE_10GBKR + 1; + bpkr->mdio.an_lp_base_page_ability_1 = MDIO_PMA_EXTABLE_10GBKR + 4; This approach is more flexible because it lets open the possibility for extension on other non-standard devices (devices non-compliant with clause 45) to still use this driver for backplane operation. These non-standard devices will have just to define their particular registers offsets in structure kr_mdio_info and then the rest of the driver can be used without other modifications. Florin.