netdev - Re: [PATCH net-next v2 2/2] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs

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:   Tue, 16 Feb 2021 16:52:13 +0000
From:   Robert Hancock <robert.hancock@...ian.com>
To:     "linux@...linux.org.uk" <linux@...linux.org.uk>
CC:     "bcm-kernel-feedback-list@...adcom.com" 
        <bcm-kernel-feedback-list@...adcom.com>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 2/2] net: phy: broadcom: Do not modify LED
 configuration for SFP module PHYs

On Sat, 2021-02-13 at 10:45 +0000, Russell King - ARM Linux admin wrote:
> On Fri, Feb 12, 2021 at 08:18:40PM -0600, Robert Hancock wrote:
> > +	if (!phydev->sfp_bus &&
> > +	    (!phydev->attached_dev || !phydev->attached_dev->sfp_bus)) {
> 
> First, do we want this to be repeated in every driver?
> 
> Second, are you sure this is the correct condition to be using for
> this?  phydev->sfp_bus is non-NULL when _this_ PHY has a SFP bus
> connected to its fibre side, it will never be set for a PHY on a
> SFP. The fact that it is non-NULL or NULL shouldn't have a bearing
> on whether we configure the LED register.

I think you're correct, the phydev->sfp_bus portion is probably not useful and
could be dropped. What we're really concerned about is whether this PHY is on
an SFP module or not. I'm not sure that a module-specific quirk makes sense
here since there are probably other models which have a similar design where
the LED outputs from the PHY are used for other purposes, and there's really no
benefit to playing with the LED outputs on SFP modules in any case, so it would
be safer to skip the LED reconfiguration for anything on an SFP. So we could
either have a condition for "!phydev->attached_dev || !phydev->attached_dev-
>sfp_bus" here and anywhere else that needs a similar check, or we do something
different, like have a specific flag to indicate that this PHY is on an SFP
module? What do people think is best here?

> 
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com