netdev - Re: [PATCH net-next] net: phy: fixed_phy: Fix fixed

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: <20190206222809.GA21303@archbook>
Date:   Wed, 6 Feb 2019 14:28:09 -0800
From:   Moritz Fischer <mdf@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Moritz Fischer <mdf@...nel.org>, netdev@...r.kernel.org,
        f.fainelli@...il.com, hkallweit1@...il.com, davem@...emloft.net
Subject: Re: [PATCH net-next] net: phy: fixed_phy: Fix fixed_phy not checking
 GPIO

Hi Andrew,

On Wed, Feb 06, 2019 at 10:59:05PM +0100, Andrew Lunn wrote:
> On Wed, Feb 06, 2019 at 10:10:40AM -0800, Moritz Fischer wrote:
> > Fix fixed_phy not checking GPIO if no link_update callback
> > is registered.
> > 
> > Signed-off-by: Moritz Fischer <mdf@...nel.org>
> > ---
> > 
> > Hi all,
> > 
> > I've been trying to figure out where exactly this broke,
> > it must've been somewhere when the file was refactored
> > in connection with phylink?
> 
> Hi Moritz
> 
> With a quick inspection, i also cannot see where it broken.
> 
> I think part of the issue is that all the current users have moved
> onto using phylink, and phylink polls the GPIO, rather than having
> fixed_link do it.

Yeah, I suspected at much :) I still feel we should fix fixed_phy as
long as there are still users for it.
> 
> I would prefer to understand exactly which change broke it. Without
> knowing how it broke, it is hard to say if this is the correct fix.

It might've been always this way. That being said I don't see why
one should've to implement an empty function (link_update) in my driver
just to read back the GPIO pin. Looking at the code it seems clear that
nothing else polls the GPIO, which doesn't seem right.

In my current understanding (correct me if I'm wrong), the link_update
callback would give the MAC a chance to update link parameters that
since we are a fixed phy we cannot read back from a PHY.

That seems conceptually independent from obtaining a link up/down info
from a GPIO pin. Wouldn't you agree?

> 
> What is your use-case? You Cc: the usb list. So a USB-Ethernet dongle?
> But then why fixed-link?

Not for this patch, did I ? I ran into this when testing nixge, but it
seemed unrelated with my changes.

Thanks,

Moritz