netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Heiner Kallweit <hkallweit1@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, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] net: phy: skip LED triggers on PHYs on SFP modules
Date: Wed, 13 Dec 2023 15:35:30 +0000	[thread overview]
Message-ID: <ZXnPQo7Nmh2PRrx+@shell.armlinux.org.uk> (raw)
In-Reply-To: <ac88a858-67fe-4932-a224-550d38454420@lunn.ch>

On Wed, Dec 13, 2023 at 11:47:50AM +0100, Andrew Lunn wrote:
> > > SFP LEDs are very unlikely to be on the front panel, since there is no
> > > such pins on the SFP cage.
> > > 
> > > Russell, in your collection of SFPs do you have any with LEDs?
> > 
> > I mean, aren't the led triggers generic so that it can trigger any
> > other LED to blink, and it's up to the user to decide ?
> 
> Correct. However, generic LEDs won't be registered via this code path,
> so the deadlock is not an issue. Only LED controllers in a PHY within
> an SFP, inside an SFP cage are the issue here. I don't have any Copper
> SFP modules, so i've no idea if they are physically big enough to have
> LEDs?

The ones I have, that is indeed the case - the RJ45 socket is the
absolute minimum size, with just enough metalwork around it to support
a RJ45 plug.

> I think it is all messy. Say the media converter has its LEDs
> connected to the front panel. You then get indications of activity on
> the front panel. I've never seen a fibre SFP with LEDs, and its an
> open question if Copper SFPs have LEDs.

A fibre module would only be able to repeat the information given via
RX_LOS and/or TX_FAULT if it had space to do so.

It's more normal in routers with SFP slots to see LEDs that are either
part of the SFP socket itself, or provided elsewhere.

> Another scenario could be a PHY
> which acts as a media switch, it can have an RJ-45 or an SFP cage,
> first to get link wins. In such a situation, i would put the LEDs on
> the front panel, since it would look odd for an empty RJ-45 socket
> LEDs to blink for SFP activity.

... and an example of this kind of setup would be the 88x3310 on
Macchiatobin - the LEDs are on the RJ45 socket, but they also
indicate for the status of the SFP connection if that is in use. I
don't remember off the top of my head what the LED configuration
register values allow one to select. We don't drive them because I
gave up on that idea - I don't believe that our LED framework is
"powerful" enough to be able to sensibly configure them... and I
personally use my own patches with register values in DT to
configure them to indicate sensibly.

However, from what I remember, configuring a LED to indicate for
1000M will mean that it will indicate whether the copper or fibre
interfaces are operating at 1000M.

-- 
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-12-13 15:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12  0:05 [PATCH net] net: phy: skip LED triggers on PHYs on SFP modules Daniel Golle
2023-12-12 14:35 ` Maxime Chevallier
2023-12-13  9:08   ` Andrew Lunn
2023-12-13 10:06     ` Maxime Chevallier
2023-12-13 10:47       ` Andrew Lunn
2023-12-13 15:35         ` Russell King (Oracle) [this message]
2023-12-13 15:27     ` Russell King (Oracle)
2023-12-13 17:12       ` Maxime Chevallier
2023-12-13 19:01       ` Daniel Golle
2023-12-13 20:23         ` Russell King (Oracle)
2023-12-14  9:48         ` Paolo Abeni
2023-12-14 16:52           ` Russell King (Oracle)
2023-12-15  9:46             ` Andrew Lunn
2023-12-15  9:59               ` Maxime Chevallier
2023-12-15 15:39                 ` Maxime Chevallier
2023-12-15  2:31 ` Jakub Kicinski
2023-12-15  2:54   ` Daniel Golle
2023-12-15  9:53   ` Andrew Lunn
2023-12-15 16:45 ` Russell King (Oracle)
2023-12-16  2:00 ` patchwork-bot+netdevbpf

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=ZXnPQo7Nmh2PRrx+@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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).