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  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:   Fri, 11 Sep 2020 14:52:11 +0200
From:   Marek Behún <marek.behun@....cz>
To:     Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc:     Andrew Lunn <andrew@...n.ch>, Pavel Machek <pavel@....cz>,
        netdev@...r.kernel.org, linux-leds@...r.kernel.org,
        Dan Murphy <dmurphy@...com>,
        Ondřej Jirman <megous@...ous.com>,
        Russell King <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support
 for LEDs controlled by Marvell PHYs

On Fri, 11 Sep 2020 09:12:01 +0200
Matthias Schiffer <matthias.schiffer@...tq-group.com> wrote:

> On Thu, 2020-09-10 at 17:00 +0200, Andrew Lunn wrote:
> > > I propose that at least these HW modes should be available (and
> > > documented) for ethernet PHY controlled LEDs:
> > >   mode to determine link on:
> > >     - `link`
> > >   mode for activity (these should blink):
> > >     - `activity` (both rx and tx), `rx`, `tx`
> > >   mode for link (on) and activity (blink)
> > >     - `link/activity`, maybe `link/rx` and `link/tx`
> > >   mode for every supported speed:
> > >     - `1Gbps`, `100Mbps`, `10Mbps`, ...
> > >   mode for every supported cable type:
> > >     - `copper`, `fiber`, ... (are there others?)  
> > 
> > In theory, there is AUI and BNC, but no modern device will have
> > these.
> >   
> > >   mode that allows the user to determine link speed
> > >     - `speed` (or maybe `linkspeed` ?)
> > >     - on some Marvell PHYs the speed can be determined by how fast
> > >       the LED is blinking (ie. 1Gbps blinks with default blinking
> > >       frequency, 100Mbps with half blinking frequeny of 1Gbps,
> > > 10Mbps
> > >       of half blinking frequency of 100Mbps)
> > >     - on other Marvell PHYs this is instead:
> > >       1Gpbs blinks 3 times, pause, 3 times, pause, ...
> > >       100Mpbs blinks 2 times, pause, 2 times, pause, ...
> > >       10Mpbs blinks 1 time, pause, 1 time, pause, ...
> > >     - we don't need to differentiate these modes with different
> > > names,
> > >       because the important thing is just that this mode allows
> > > the user to determine the speed from how the LED blinks
> > >   mode to just force blinking
> > >     - `blink`
> > > The nice thing is that all this can be documented and done in
> > > software
> > > as well.  
> > 
> > Have you checked include/dt-bindings/net/microchip-lan78xx.h and
> > mscc-phy-vsc8531.h ? If you are defining something generic, we need
> > to
> > make sure the majority of PHYs can actually do it. There is no
> > standardization in this area. I'm sure there is some similarity,
> > there
> > is only so many ways you can blink an LED, but i suspect we need a
> > mixture of standardized modes which we hope most PHYs implement, and
> > the option to support hardware specific modes.
> > 
> >     Andrew  
> 
> 
> FWIW, these are the LED HW trigger modes supported by the TI DP83867
> PHY:
> 
> - Receive Error
> - Receive Error or Transmit Error

Does somebody use this? I would just omit these.

> - Link established, blink for transmit or receive activity

`link/activity`

> - Full duplex

Not needed for now, I think.

> - 100/1000BT link established
> - 10/100BT link established

Disjunctive modes can go f*** themselves :)

> - 10BT link established
> - 100BT link established
> - 1000BT link established

`10Mbps`, `100Mbps`, `1Gbps`

> - Collision detected

Not needed for now.

> - Receive activity
> - Transmit activity

`rx/tx`

> - Receive or Transmit activity

`activity`

> - Link established

`link`

> 
> AFAIK, the "Link established, blink for transmit or receive activity"
> is the only trigger that involves blinking; all other modes simply
> make the LED light up when the condition is met. Setting the output
> level in software is also possible.
> 
> Regarding the option to emulate unsupported HW triggers in software,
> two questions come to my mind:
> 
> - Do all PHYs support manual setting of the LED level, or are the PHYs
> that can only work with HW triggers?
> - Is setting PHY registers always efficiently possible, or should SW
> triggers be avoided in certain cases? I'm thinking about setups like
> mdio-gpio. I guess this can only become an issue for triggers that
> blink.

The software trigger do not have to work with the LED connected to the
PHY. Any other LED on the system can be used. Only the information
about link and speed must come from the PHY, and kernel does have this
information already, either by polling or from interrupt.

> 
> 
> Kind regards,
> Matthias
> 

Powered by blists - more mailing lists