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 <ansuelsmth@xxxxxxxxx> wrote: > > > 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
