Hi, thanks for the new series. Am Donnerstag, 15. Dezember 2022, 00:54:27 CET schrieb Christian Marangi: > This is another attempt on adding this feature on LEDs, hoping this is > the right time and someone finally notice this. Unfortunately I'm out of office from next week on, so there is only limited feedback from my side. > 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. I'm a bit confused about that blink mode is supposed to mean. I don't know what to implement for blink_set. Reading qca8k_cled_blink_set it seems to just configure the blink interval for the corresponding LED. Unfortunately that's not possible for all PHYs. In my case, DP83867, I can configure the blink interval only globally. I'm not sure how this will fit into this LED trigger. > 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. My DP83867 proof of concept implementation also supports several LEDs, but my actual hardware also only has 2 of them attached (LED0 and LED2). Best regards, Alexander > The netdev trigger is expanded and it does now support hardware only > triggers. > The idea is to use hardware mode when a device_name is not defined. > An additional sysfs entry is added to give some info about the available > trigger modes supported in the current configuration. > > > It was reported that at least 3 other switch family would benefits by > this as they all lack support for a generic way to setup their leds and > netdev team NACK each try to add special code to support LEDs present > on switch in favor of a generic solution. > > v7: > - Rebase on top of net-next (for qca8k changes) > - Fix some typo in commit description > - Fix qca8k leds documentation warning > - Remove RFC tag > v6: > - Back to RFC. > - Drop additional trigger > - Rework netdev trigger to support common modes used by switch and > hardware only triggers > - Refresh qca8k leds logic and driver > v5: > - Move out of RFC. (no comments from Andrew this is the right path?) > - Fix more spelling mistake (thx Randy) > - Fix error reported by kernel test bot > - Drop the additional HW_CONTROL flag. It does simplify CONFIG > handling and hw control should be available anyway to support > triggers as module. > v4: > - Rework implementation and drop hw_configure logic. > We now expand blink_set. > - Address even more spelling mistake. (thx a lot Randy) > - Drop blink option and use blink_set delay. > - Rework phy-activity trigger to actually make the groups dynamic. > v3: > - Rework start/stop as Andrew asked. > - Introduce more logic to permit a trigger to run in hardware mode. > - Add additional patch with netdev hardware support. > - Use test_bit API to check flag passed to hw_control_configure. > - Added a new cmd to hw_control_configure to reset any active blink_mode. > - Refactor all the patches to follow this new implementation. > v2: > - Fix spelling mistake (sorry) > - Drop patch 02 "permit to declare supported offload triggers". > Change the logic, now the LED driver declare support for them > using the configure_offload with the cmd TRIGGER_SUPPORTED. > - Rework code to follow this new implementation. > - Update Documentation to better describe how this offload > implementation work. > > Christian Marangi (11): > leds: add support for hardware driven LEDs > leds: add function to configure hardware controlled LED > leds: trigger: netdev: drop NETDEV_LED_MODE_LINKUP from mode > leds: trigger: netdev: rename and expose NETDEV trigger enum modes > leds: trigger: netdev: convert device attr to macro > leds: trigger: netdev: add hardware control support > leds: trigger: netdev: use mutex instead of spinlocks > leds: trigger: netdev: add available mode sysfs attr > leds: trigger: netdev: add additional hardware only triggers > net: dsa: qca8k: add LEDs support > dt-bindings: net: dsa: qca8k: add LEDs definition example > > .../devicetree/bindings/net/dsa/qca8k.yaml | 24 ++ > Documentation/leds/leds-class.rst | 53 +++ > drivers/leds/Kconfig | 11 + > drivers/leds/led-class.c | 27 ++ > drivers/leds/led-triggers.c | 29 ++ > drivers/leds/trigger/ledtrig-netdev.c | 385 ++++++++++++----- > drivers/net/dsa/qca/Kconfig | 9 + > drivers/net/dsa/qca/Makefile | 1 + > drivers/net/dsa/qca/qca8k-8xxx.c | 4 + > drivers/net/dsa/qca/qca8k-leds.c | 406 ++++++++++++++++++ > drivers/net/dsa/qca/qca8k.h | 62 +++ > include/linux/leds.h | 103 ++++- > 12 files changed, 1015 insertions(+), 99 deletions(-) > create mode 100644 drivers/net/dsa/qca/qca8k-leds.c
