Hello Ian, thank you for your proposal. Some comments below: On Sun, 8 Aug 2021 22:32:07 -0500 Ian Pilcher <arequipeno@xxxxxxxxx> wrote: > One thing that has not changed is that associations between block > devices and LEDs are still set via an attribute on the device, rather > than the LED. This is much simpler, as the device attribute only has > to handle a single value (the name of the associated LED), rather than > potentially handling multiple device names. It may be simpler, but it is in contrast to how the netdev trigger works, which already is in upstream for many years. I really think we should try to have similar sysfs ABIs here. (I understand that the netdev trigger is currently unable to handle multiple network interfaces - but it is possible to extend it so.) > I have modeled the interface for the /sys/block/<DEVICE>/led > attribute on the sysfs interface used for selecting a trigger. All > available LEDs (all LEDs associated with the blkdev trigger) are > shown when the attribute is read, with the currently selected LED > enclosed in square brackets ([]). I think it is reasonable to be able to set something like this: led0 : blink on activity on any of [sda, sdb, sdc] led1 : blink on activity on sda led2 : blink on activity on sdb led3 : blink on activity on sdc If I am reading your code correctly, it looks that only one LED can be configured for a block device. Is this true? If so, then the above configuration cannot be set. Also you are blinking the LED on any request to the block device. I would rather expect to be able to set the LED to blink on read and on write. (And possibly on other functions, like discard, or critical temperature, or error, ...) I would like to know what other people think about this. Marek
