netdev - Re: Decoupling phy_device from net_device (was "Re: [PATCH] net: dsa: fixed-link interface is reporting SPEED

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] [day] [month] [year] [list]

Message-ID: <20190506131057.GB15291@lunn.ch>
Date:   Mon, 6 May 2019 15:10:57 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Heiner Kallweit <hkallweit1@...il.com>, f.fainelli@...il.com,
        davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: Decoupling phy_device from net_device (was "Re: [PATCH] net:
 dsa: fixed-link interface is reporting SPEED_UNKNOWN")

On Mon, May 06, 2019 at 01:00:49AM +0300, Vladimir Oltean wrote:
> On 4/12/19 8:57 PM, Heiner Kallweit wrote:
> >On 12.04.2019 01:01, Vladimir Oltean wrote:
> >>With Heiner's recent patch "b6163f194c69 net: phy: improve
> >>genphy_read_status", the phydev->speed is now initialized by default to
> >>SPEED_UNKNOWN even for fixed PHYs. This is not necessarily bad, since it
> >>is not correct to call genphy_config_init() and genphy_read_status() for
> >>a fixed PHY.
> >>
> >What do you mean with "it is not correct"? Whether the calls are always
> >needed may be a valid question, but it's not forbidden to use these calls
> >with a fixed PHY. Actually in phylib polling mode genphy_read_status is
> >called every second also for a fixed PHY. swphy emulates all relevant
> >PHY registers.
> >
> >>This dates back all the way to "39b0c705195e net: dsa: Allow
> >>configuration of CPU & DSA port speeds/duplex" (discussion thread:
> >>https://www.spinics.net/lists/netdev/msg340862.html).
> >>
> >>I don't seem to understand why these calls were necessary back then, but
> >>removing these calls seemingly has no impact now apart from preventing
> >>the phydev->speed that was set in of_phy_register_fixed_link() from
> >>getting overwritten.

As Florian said, if you have patches, please post them and we will
consider them.

But i think we also need to take a step back and consider the big
picture. There has been a lot of work recently to support multi-G
PHYs. It is clear we soon need to make changes to fixed-link. It only
supports up to 1G. But we have use cases where we need multi-G fixed
links.

We could also consider making the tie to the MAC much stronger. We
have been encouraging MAC driver writers to make use of the
ndev->phylib pointer. We could even enforce that, and use
container_of() to determine the MAC associated to a PHY.

	Andrew