linux-kernel - Re: [PATCH net-next v3 1/9] net: phy: micrel: move phy reg offsets to common header

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: <YK+34eadXNOR4f1D@lunn.ch>
Date:   Thu, 27 May 2021 17:16:49 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Oleksij Rempel <o.rempel@...gutronix.de>,
        Woojung Huh <woojung.huh@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Michael Grzeschik <m.grzeschik@...gutronix.de>,
        kernel@...gutronix.de, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH net-next v3 1/9] net: phy: micrel: move phy reg offsets
 to common header

> >  	switch (reg) {
> > -	case PHY_REG_CTRL:
> > +	case MII_BMCR:
> >  		ksz_pread8(dev, p, regs[P_NEG_RESTART_CTRL], &restart);
> >  		ksz_pread8(dev, p, regs[P_SPEED_STATUS], &speed);
> >  		ksz_pread8(dev, p, regs[P_FORCE_CTRL], &ctrl);
> >  		if (restart & PORT_PHY_LOOPBACK)
> > -			data |= PHY_LOOPBACK;
> > +			data |= BMCR_LOOPBACK;
> >  		if (ctrl & PORT_FORCE_100_MBIT)
> > -			data |= PHY_SPEED_100MBIT;
> > +			data |= BMCR_SPEED100;
> >  		if (ksz_is_ksz88x3(dev)) {
> >  			if ((ctrl & PORT_AUTO_NEG_ENABLE))
> > -				data |= PHY_AUTO_NEG_ENABLE;
> > +				data |= BMCR_ANENABLE;
> >  		} else {
> >  			if (!(ctrl & PORT_AUTO_NEG_DISABLE))
> > -				data |= PHY_AUTO_NEG_ENABLE;
> > +				data |= BMCR_ANENABLE;
> >  		}
> >  		if (restart & PORT_POWER_DOWN)
> > -			data |= PHY_POWER_DOWN;
> > +			data |= BMCR_PDOWN;
> >  		if (restart & PORT_AUTO_NEG_RESTART)
> > -			data |= PHY_AUTO_NEG_RESTART;
> > +			data |= BMCR_ANRESTART;
> >  		if (ctrl & PORT_FORCE_FULL_DUPLEX)
> > -			data |= PHY_FULL_DUPLEX;
> > +			data |= BMCR_FULLDPLX;
> >  		if (speed & PORT_HP_MDIX)
> > -			data |= PHY_HP_MDIX;
> > +			data |= KSZ886X_BMCR_HP_MDIX;
> >  		if (restart & PORT_FORCE_MDIX)
> > -			data |= PHY_FORCE_MDIX;
> > +			data |= KSZ886X_BMCR_FORCE_MDI;
> >  		if (restart & PORT_AUTO_MDIX_DISABLE)
> > -			data |= PHY_AUTO_MDIX_DISABLE;
> > +			data |= KSZ886X_BMCR_DISABLE_AUTO_MDIX;
> >  		if (restart & PORT_TX_DISABLE)
> > -			data |= PHY_TRANSMIT_DISABLE;
> > +			data |= KSZ886X_BMCR_DISABLE_TRANSMIT;
> >  		if (restart & PORT_LED_OFF)
> > -			data |= PHY_LED_DISABLE;
> > +			data |= KSZ886X_BMCR_DISABLE_LED;
> >  		break;
> 
> I am deeply confused as to what this function is doing. It is reading
> the 8-bit port registers P_NEG_RESTART_CTRL, P_SPEED_STATUS and
> P_FORCE_CTRL and stitching them into a 16-bit "MII_BMCR"?

Sort of. Take a look at the datasheet for the ksz8841. It has clause
22 like registers which it exports to a PHY driver. It puts MDIX
control into the bottom of the BMCR. So this DSA driver is emulating
the ksz8841 so it can share the PHY driver.

    Andrew