netdev - Re: [PATCH] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c989910aee122cfa9d29994d9ce650ce486442ca.camel@calian.com>
Date:   Mon, 19 Oct 2020 21:48:43 +0000
From:   Robert Hancock <robert.hancock@...ian.com>
To:     "andrew@...n.ch" <andrew@...n.ch>
CC:     "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "michal.simek@...inx.com" <michal.simek@...inx.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "radhey.shyam.pandey@...inx.com" <radhey.shyam.pandey@...inx.com>
Subject: Re: [PATCH] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX
 mode

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