netdev - Re: [PATCH] net: ibm_newemac: Don't start autonegotiation when disabled in BMCR (genmii)

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: <1311078570.25044.421.camel@pasglop>
Date:	Tue, 19 Jul 2011 22:29:30 +1000
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Stefan Roese <sr@...x.de>
Cc:	linuxppc-dev@...abs.org, netdev@...r.kernel.org
Subject: Re: [PATCH] net: ibm_newemac: Don't start autonegotiation when
 disabled in BMCR (genmii)

On Tue, 2011-07-19 at 13:59 +0200, Stefan Roese wrote:
> Hi Ben,
> 
> On Tuesday 19 July 2011 13:35:22 Benjamin Herrenschmidt wrote:
> > On Tue, 2011-07-19 at 12:50 +0200, Stefan Roese wrote:
> > > As noticed on a custom 440GX board using the Micrel KSZ8041 PHY in
> > > fiber mode, a strapped fixed PHY configuration will currently restart
> > > the autonegotiation. This patch checks the BMCR_ANENABLE bit and
> > > skips this autonegotiation if its disabled.
> > 
> > Won't that just break aneg on everything else ?
> > 
> > IE, most other PHYs rely on ANENABLE being set further down this same
> > function (especially if the FW doesn't do it but even then, we may reset
> > PHYs along the way etc...)
> 
> If aneg is enabled for a PHY (e.g. not strapped to fixed configuration), I 
> don't see how this patch will change the current aneg behaviour. Perhaps I'm 
> missing something, but I tested this on some boards with aneg enabled (Sequoia 
> etc). And I didn't notice any problems.

But is aneg always enabled via straps ? The whole point of this function
is that aneg can be enabled or disabled via the ethtool API (thus
overriding whatever strapping)... I may be missing something but this
patch looks like it would break this no ?

> > This is something that really a case where the device-tree should
> > indicate that aneg shall not be performed and from there don't call
> > setup_aneg at all.
> 
> I feel that this BMCR_ANENABLE bit should be evaluated, but I have no strong 
> preference here. If you prefer that this should be handled via a new dt 
> property (phy-aneg = "disabled" ?), I can implement it this way. Just let me 
> know.

Don't we already have some bindings for PHY with a fixed setting ? I
don't remember off hand, we need to dbl check.

I don't like looking at BMCR because BCMR is a -control- register,
something that we can (and will) whack with anything ourselves, which
can be reset or reconfigured and I have learned to never trust board
straps :-)

Cheers,
Ben.


--
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