On Thu, Nov 11, 2021 at 03:16:08AM +0100, Marek Behún wrote:

On Thu, 11 Nov 2021 02:34:52 +0100
Ansuel Smith [off-list ref] wrote:

quoted

This is another attempt in adding support for PHY LEDs. Most of the
times Switch/PHY have connected multiple LEDs that are controlled by HW
based on some rules/event. Currently we lack any support for a generic
way to control the HW part and normally we either never implement the
feature or only add control for brightness or hw blink.

This is based on Marek idea of providing some API to cled but use a
different implementation that in theory should be more generilized.

The current idea is:
- LED driver implement 3 API (hw_control_status/start/stop).
  They are used to put the LED in hardware mode and to configure the
  various trigger.
- We have hardware triggers that are used to expose to userspace the
  supported hardware mode and set the hardware mode on trigger
  activation.
- We can also have triggers that both support hardware and software mode.
- The LED driver will declare each supported hardware blink mode and
  communicate with the trigger all the supported blink modes that will
  be available by sysfs.
- A trigger will use blink_set to configure the blink mode to active
  in hardware mode.
- On hardware trigger activation, only the hardware mode is enabled but
  the blink modes are not configured. The LED driver should reset any
  link mode active by default.

Each LED driver will have to declare explicit support for the offload
trigger (or return not supported error code) as we the trigger_data that
the LED driver will elaborate and understand what is referring to (based
on the current active trigger).

I posted a user for this new implementation that will benefit from this
and will add a big feature to it. Currently qca8k can have up to 3 LEDs
connected to each PHY port and we have some device that have only one of
them connected and the default configuration won't work for that.

I also posted the netdev trigger expanded with the hardware support.

More polish is required but this is just to understand if I'm taking
the correct path with this implementation hoping we find a correct
implementation and we start working on the ""small details""

Hello Ansuel,

besides other things, I am still against the idea of the
`hardware-phy-activity` trigger: I think that if the user wants the LED
to indicate network device's link status or activity, it should always
be done via the existing netdev trigger, and with that trigger only.

Yes, I know that netdev trigger does not currently support indicating
different link modes, only whether the link is up (in any mode). That
should be solved by extending the netdev trigger.

I am going to try to revive my last attempt and send my proposal again.
Hope you don't mind.

Marek

Honestly... It's a bit sad.
The netdev trigger have its limitation and I see introducing an
additional trigger a practical way to correctly support some
strange/specific PHY.
I implemented both idea: expand netdev and introduce a dedicated
trigger and still this is problematic.
Is having an additional trigger for the specific task that bad?

I don't care as long as the feature is implemented but again
pretty sad how this LEDs proposal went.

-- 
	Ansuel