netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Sergei Antonov <saproj@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next v2] net: dsa: mv88e6060: add phylink_get_caps implementation
Date: Thu, 10 Aug 2023 18:38:29 +0100	[thread overview]
Message-ID: <ZNUglYF2Xy63l4aZ@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230810171100.dvnsjgjo67hax4ld@skbuf>

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!

  reply	other threads:[~2023-08-10 17:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-09 14:46 [PATCH net-next v2] net: dsa: mv88e6060: add phylink_get_caps implementation Russell King (Oracle)
2023-08-10 16:44 ` Vladimir Oltean
2023-08-10 16:52   ` Russell King (Oracle)
2023-08-10 17:11     ` Vladimir Oltean
2023-08-10 17:38       ` Russell King (Oracle) [this message]
2023-08-10 18:17         ` Sergei Antonov
2023-08-10 18:31           ` Vladimir Oltean
2023-08-10 18:27         ` Vladimir Oltean
2023-08-10 18:09 ` Sergei Antonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNUglYF2Xy63l4aZ@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=saproj@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).