netdev - Re: [PATCH net-next v2] net: dsa: mv88e6060: add phylink_get

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: Thu, 10 Aug 2023 18:38:29 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Sergei Antonov <saproj@...il.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2] net: dsa: mv88e6060: add phylink_get_caps
 implementation

On Thu, Aug 10, 2023 at 08:11:00PM +0300, Vladimir Oltean wrote:
> On Thu, Aug 10, 2023 at 05:52:41PM +0100, Russell King (Oracle) wrote:
> > I wonder whether we have any implementation using SNI mode. I couldn't
> > find anything in the in-kernel dts files for this driver, the only
> > dts we have is one that was posted on-list recently, and that was using
> > MII at 100Mbps:
> > 
> > https://lore.kernel.org/r/CABikg9zfGVEJsWf7eq=K5oKQozt86LLn-rzMaVmycekXkQEa8Q@mail.gmail.com
> > 
> > No one would be able to specify "sni" in their dts, so maybe for the
> > sake of simplicity, we shouldn't detect whether it's in SNI mode, and
> > just use MII, and limit the speed to just 10Mbps?
> 
> Based on the fact that "marvell,mv88e6060" is in
> dsa_switches_apply_workarounds[], it is technically possible that there
> exist boards which use the SNI mode but have no phy-mode and other
> phylink properties on the CPU port, and thus they work fine while
> skipping phylink. Of course, "possible" != "real".

What I meant is that there are no in-tree users of the Marvell 88E6060
DSA driver. It looks like it was contributed in 2008. Whether it had
users between the date that it was contributed and today I don't know.

All that I can see is that the only users of it are out-of-tree users,
which means we have the maintenance burden from the driver but no
apparent platforms that make use of it, and no way to test it (other
than if one of those out-of-tree users pops up, such as like last
month.)

I know that Arnd tends to strip out code that a platform uses when the
platform is removed, was there a reason that this got left behind,
assuming that it was used by a board?

> Maybe if we don't want to introduce PHY_INTERFACE_MODE_SNI for fear of a
> lack of real users, we could at least detect PortMode=0, and not
> populate supported_interfaces, leading to an intentional validation
> failure and a comment above that check, stating that phy-mode = "sni" is
> not yet implemented?

It would probably be better for mv88e6060_phylink_get_caps() to detect
it and print the warning, leaving supported_interfaces empty - which
will then cause phylink_create() to fail. Maybe that's what you meant,
but I interpreted it as modifying the check in phylink_create().

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!