netdev - Re: [PATCH v2 0/7] alx: small fixes/cleanups

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: <1372526436.8183.3.camel@jlt4.sipsolutions.net>
Date:	Sat, 29 Jun 2013 19:20:36 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Johannes Stezenbach <js@...21.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH v2 0/7] alx: small fixes/cleanups

On Tue, 2013-06-25 at 23:01 +0200, Johannes Stezenbach wrote:
> On Tue, Jun 25, 2013 at 08:38:16PM +0200, Johannes Berg wrote:
> > Thanks to Ben's review mostly, here are some fixes/cleanups in
> > alx. I'm seriously considering removing WoWLAN as I have no use
> > for it and am not too motivated to debug it, anyone else want
> > to try debugging? :)
> 
> One thing I noticed is alx_select_powersaving_speed() will return 0
> when there is no link but leaves *speed uninitialized.

Yeah, that's a bit odd.

> Then __alx_shutdown() prints e.g.
>    alx 0000:03:00.0 eth0: wol: ctrl=3, speed=FFFF8801
> In this case the
>   alx_write_phy_reg(hw, ALX_MII_IER, 0);
> is skipped in alx_select_powersaving_speed().
> However, it should still be done in alx_pre_suspend()
> when WOL is disabled.  But it might be we need to move
> the alx_clear_phy_intr() in __alx_shutdown() down.
> 
> I'm also not sure why alx_get_phy_link() reads MII_BMSR
> twice, but the old driver from compat-wireless-3.6.8-1-snpc
> also does it, it seems deliberate.

Yeah, all this looks like magic to me...

> Unless someone beats me to it I will try to debug
> on the weekend.

The whole PCI(e) stuff there looks pretty odd to me and I can't seem to
fix it right now, so I'm removing it. It really looks like all the
wakeup_enable() etc. is in the wrong places? Hmmm. Well the code is
still in the history ;-)

> BTW, it is WoLAN not WoWLAN :-)

Heh, oops. Guess it shows what I usually work on ;-)

johannes

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