On Mon, 2020-10-19 at 23:36 +0200, Andrew Lunn wrote:
> >  static void axienet_mac_config(struct phylink_config *config,
> > unsigned int mode,
> >                            const struct phylink_link_state *state)
> >  {
> > -   /* nothing meaningful to do */
> > +   struct net_device *ndev = to_net_dev(config->dev);
> > +   struct axienet_local *lp = netdev_priv(ndev);
> > +   int ret;
> > +
> > +   switch (state->interface) {
> > +   case PHY_INTERFACE_MODE_SGMII:
> > +   case PHY_INTERFACE_MODE_1000BASEX:
> > +           ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode,
> > +                                            state->interface,
> > +                                            state->advertising);
> > +           if (ret < 0)
> > +                   netdev_warn(ndev, "Failed to configure PCS:
> > %d\n",
> > +                               ret);
> > +
> > +           /* Ensure isolate bit is cleared */
> > +           ret = mdiobus_modify(lp->pcs_phy->bus, lp->pcs_phy-
> > >addr,
> > +                                MII_BMCR, BMCR_ISOLATE, 0);
> 
> Hi Robert
> 
> That looks like a layering violation. Maybe move this into
> phylink_mii_c22_pcs_config(), it is accessing MII_BMCR anyway.

Could do - do we think there's any harm in just disabling BMCR_ISOLATE
in all cases in that function?

-- 
Robert Hancock
Senior Hardware Designer, Advanced Technologies 
www.calian.com

Reply via email to