linux-kernel - Re: [RFC PATCH v2 3/5] leds: trigger: add offload-phy-activity trigger

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:   Mon, 8 Nov 2021 16:19:12 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.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
Subject: Re: [RFC PATCH v2 3/5] leds: trigger: add offload-phy-activity
 trigger

On Mon, Nov 08, 2021 at 03:17:33PM +0100, Andrew Lunn wrote:
> On Mon, Nov 08, 2021 at 01:24:58AM +0100, Ansuel Smith wrote:
> > Add Offload Trigger for PHY Activity. This special trigger is used to
> > configure and expose the different HW trigger that are provided by the
> > PHY. Each offload trigger can be configured by sysfs and on trigger
> > activation the offload mode is enabled.
> > 
> > This currently implement these hw triggers:
> >   - blink_tx: Blink LED on tx packet receive
> >   - blink_rx: Blink LED on rx packet receive
> >   - blink_collision: Blink LED on collision detection
> 
> When did you last see a collision? Do you really have a 1/2 duplex
> link? Just because the PHY can, does not mean we should support
> it. Lets restrict this to the most useful modes.
>

Ok will drop this. In my case (qca8k) I also never see a device using it
so I agree on the fact that should be dropped.

> >   - link_10m: Keep LED on with 10m link speed
> >   - link_100m: Keep LED on with 100m link speed
> >   - link_1000m: Keep LED on with 1000m link speed
> >   - half_duplex: Keep LED on with half duplex link
> >   - full_duplex: Keep LED on with full duplex link
> >   - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
> >   - power_on_reset: Keep LED on with switch reset
> 
> >   - blink_2hz: Set blink speed at 2hz for every blink event
> >   - blink_4hz: Set blink speed at 4hz for every blink event
> >   - blink_8hz: Set blink speed at 8hz for every blink event
> 
> These seems like attributes, not blink modes. They need to be
> specified somehow differently, or not at all. Do we really need them?
> 

Sorry I didn't update the commit. In sysfs they are exposed as option
like the power_on_reset and linkup_over. So they are option on how the
LED behave on the event.

> >   - blink_auto: Set blink speed at 2hz for 10m link speed,
> >       4hz for 100m and 8hz for 1000m
> 
> Another attribute, and one i've not seen any other PHY do.
> 

Yes we can consider dropping this but I think the other 3 should be
keeped.

> 	Andrew

-- 
	Ansuel