netdev - Re: [PATCH v6 1/6] net: phy: at803x: Export at803x_debug_reg

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]

Date:   Fri, 11 May 2018 15:22:39 -0700
From:   Paul Burton <paul.burton@...s.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Darren Hart <dvhart@...ux.intel.com>, <netdev@...r.kernel.org>,
        <linux-mips@...ux-mips.org>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v6 1/6] net: phy: at803x: Export at803x_debug_reg_mask()

Hi Andrew,

On Fri, May 11, 2018 at 09:24:46PM +0200, Andrew Lunn wrote:
> > I could reorder the probe function a little to initialize the PHY before
> > performing the MAC reset, drop this patch and the AR803X hibernation
> > stuff from patch 2 if you like. But again, I can't actually test the
> > result on the affected hardware.
> 
> Hi Paul
> 
> I don't like a MAC driver poking around in PHY registers.
> 
> So if you can rearrange the code, that would be great.
> 
>    Thanks
> 	Andrew

Sure, I'll give it a shot.

After digging into it I see 2 ways to go here:

  1) We could just always reset the PHY before we reset the MAC. That
     would give us a window of however long the PHY takes to enter its
     low power state & stop providing the RX clock during which we'd
     need the MAC reset to complete. In the case of the AR8031 that's
     "about 10 seconds" according to its data sheet. In this particular
     case that feels like plenty, but it does also feel a bit icky to
     rely on the timing chosen by the PHY manufacturer to line up with
     that of the MAC reset.

  2) We could introduce a couple of new phy_* functions to disable &
     enable low power states like the AR8031's hibernation feature, by
     calling new function pointers in struct phy_driver. Then pch_gbe &
     other MACs could call those to have the PHY driver disable
     hibernation at times where we know we'll need the RX clock and
     re-enable it afterwards.

I'm currently leaning towards option 2. How does that sound to you? Or
can you see another way to handle this?

Thanks,
    Paul