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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <635022046.487018.1369769745682.open-xchange@email.1und1.de>
Date:	Tue, 28 May 2013 21:35:45 +0200 (CEST)
From:	"renner@...-gmbh.de" <renner@...-gmbh.de>
To:	netdev@...r.kernel.org, David Laight <David.Laight@...LAB.COM>
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.

Jens.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html