> > > + 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
Less readale. MDIO_STAT1 is well known. What does
bp_dev.mdio.an_status mean?
In general, core code should be KISS, and assume everything follows
the standard. We don't want to scatter workarounds for non-conformant
hardware in core code. Such workarounds should be in the drivers where
they are hidden away.
> + 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;
Please drop this, and use the #defines directly.
Andrew
