Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

On Thu, 2020-09-10 at 17:00 +0200, Andrew Lunn wrote:

quoted

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.

quoted

  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
- Link established, blink for transmit or receive activity
- Full duplex
- 100/1000BT link established
- 10/100BT link established
- 10BT link established
- 100BT link established
- 1000BT link established
- Collision detected
- Receive activity
- Transmit activity
- Receive or Transmit activity
- Link established

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.


Kind regards,
Matthias