netdev - Re: [PATCH v7 09/11] leds: trigger: netdev: add additional hardware only triggers

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]

Message-ID: <63a30297.050a0220.46b0f.7533@mx.google.com>
Date:   Wed, 21 Dec 2022 13:56:53 +0100
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jonathan Corbet <corbet@....net>, Pavel Machek <pavel@....cz>,
        John Crispin <john@...ozen.org>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-leds@...r.kernel.org,
        Tim Harvey <tharvey@...eworks.com>,
        Alexander Stein <alexander.stein@...tq-group.com>,
        Rasmus Villemoes <rasmus.villemoes@...vas.dk>
Subject: Re: [PATCH v7 09/11] leds: trigger: netdev: add additional hardware
 only triggers

On Wed, Dec 21, 2022 at 01:12:16AM +0100, Andrew Lunn wrote:
> On Thu, Dec 15, 2022 at 05:35:47PM +0000, Russell King (Oracle) wrote:
> > On Thu, Dec 15, 2022 at 12:54:36AM +0100, Christian Marangi wrote:
> > > Add additional hardware only triggers commonly supported by switch LEDs.
> > > 
> > > Additional modes:
> > > link_10: LED on with link up AND speed 10mbps
> > > link_100: LED on with link up AND speed 100mbps
> > > link_1000: LED on with link up AND speed 1000mbps
> > > half_duplex: LED on with link up AND half_duplex mode
> > > full_duplex: LED on with link up AND full duplex mode
> > 
> > Looking at Marvell 88e151x, I don't think this is usable there.
> > We have the option of supporting link_1000 on one of the LEDs,
> > link_100 on another, and link_10 on the other. It's rather rare
> > for all three leds to be wired though.
> 
> The 88e151x will need to enumerate what it actually supports from the
> above list, per LED. I also think we can carefully expand the list
> above, adding a few more modes. We just need to ensure what is added
> is reasonably generic, modes we expect multiple PHY to support. What
> we need to avoid is adding every single mode a PHY supports, but no
> other PHY has.
> 
> > This is also a PHY where "activity" mode is supported (illuminated
> > or blinking if any traffic is transmitted or received) but may not
> > support individual directional traffic in hardware. However, it
> > does support forcing the LED on or off, so software mode can handle
> > those until the user selects a combination of modes that are
> > supported in the hardware.
> > 
> > > Additional blink interval modes:
> > > blink_2hz: LED blink on any even at 2Hz (250ms)
> > > blink_4hz: LED blink on any even at 4Hz (125ms)
> > > blink_8hz: LED blink on any even at 8Hz (62ms)
> > 
> > This seems too restrictive. For example, Marvell 88e151x supports
> > none of these, but does support 42, 84, 170, 340, 670ms.
> 
> I would actually drop this whole idea of being able to configure the
> blink period. It seems like it is going to cause problems. I expect
> most PHYs actual share the period across multiple LEDs, which you
> cannot easily model here.
> 
> So i would have the driver hard coded to pick a frequency at thats' it.
> 

Yes I think "for now" it's the only way and just drop blink
configuration support.

-- 
	Ansuel