netdev - RE: [PATCH] net: ethernet: xilinx_emaclite: keep protocol selector bits by reading ANAR

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 for Android: free password hash cracker in your pocket

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

Message-ID: <AE90C24D6B3A694183C094C60CF0A2F6026B7253@saturn3.aculab.com>
Date:	Wed, 29 May 2013 09:51:30 +0100
From:	"David Laight" <David.Laight@...LAB.COM>
To:	<renner@...-gmbh.de>, <netdev@...r.kernel.org>
Cc:	<davem@...emloft.net>
Subject: RE: [PATCH] net: ethernet: xilinx_emaclite: keep protocol selector bits by reading ANAR

> > David Laight <David.Laight@...LAB.COM> hat am 28. Mai 2013 um 18:22
> > geschrieben:
> >
> >
> > > This patch reads the PHY's MII_ADVERTISE register (ANAR) before modifying
> > > it.
> > > Hence, the protocol selector bits (4:0) which indicate IEEE 803.3u support
> > > are
> > > prevented from being cleared.
> > > ...
> > >                 /* Advertise only 10 and 100mbps full/half duplex speeds */
> > > -               phy_write(lp->phy_dev, MII_ADVERTISE, ADVERTISE_ALL);
> > > +               adv = phy_read(lp->phy_dev, MII_ADVERTISE);
> > > +               if (adv < 0)
> > > +                       return adv;
> > > +               phy_write(lp->phy_dev, MII_ADVERTISE, adv | ADVERTISE_ALL);
> >
> > Given the comment shouldn't this be removing some bits?
> >
> > It is a long time since I read the definitions of the ANAR (etc).
> > But I remember it having at least 5 bits defined for 10M and 100M
> > operation (including 100T4), so the reference to keeping bits (4:0)
> > seems confusing.
> >
> >    David
> >
> 
> David, thanks for your feedback!
> The bits I mentioned (the 5 least bits in ANAR) are the so-called protocol
> selection bits (at least that's what they call it at Intel, TI, Marvell, Maxim,
> etc.). The only value that seems actually to be used is 1 (0b00001) which means
> 802.3 / fast ethernet capability (maybe 0 for slower devices?). 1 is the
> default value for all the 10/100(/1000) PHYs I know of, in some PHYs it is even
> hardcoded (i.e. read-only). It must not be confused with ANAR bits 9:5 where
> the actual speed and duplex mode can be selected.
> ADVERTISE_ALL as defined in mii.h lacks this bit, in contrast to ADVERTISE_FULL
> where it is set through ADVERTISE_CSMA. Whether this is correct or not; at
> least it makes some sense to me to read what's actually written in these
> register bits and not to clear them. Otherwise some PHYs might fall back to
> 10-half as seen for the TI DP83640 which has been used to test this patch.
> I hope this explains the reason for this patch.

Part of my confusion was that I'd forgotten that the MII registers are 16bit!
(I said it had been a long time!)

However if you want to advertise 10M/100M HDX and FDX you need to unset
the other speed bits (100T4 was assigned a bit, and I'm sure there were
extra bits reserved for higher speeds - which also need clearing).
Given that the ANAR is defined by 803.x you shouldn't really have a problem!

My real bugbear is that none of the standard MII registers actually tells
you the mode the PHY is working in! Not sure how that got through the
standards body.

	David