Hi Akinobu,
Thank you for the update. I have one more remark to this patch
below.
On 8/10/19 7:55 PM, Akinobu Mita wrote:
> This allows LEDs to be controlled by block device activity.
>
> We already have ledtrig-disk (LED disk activity trigger), but the lower
> level disk drivers need to utilize ledtrig_disk_activity() to make the
> LED blink.
>
> The LED block device trigger doesn't require the lower level drivers to
> have any instrumentation. The activity is collected by polling the disk
> stats.
>
> Example:
>
> echo block-nvme0n1 > /sys/class/leds/diy/trigger
>
> Cc: Frank Steiner <fsteiner-mail1@xxxxxxxxxxxxxx>
> Cc: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
> Cc: Pavel Machek <pavel@xxxxxx>
> Cc: Dan Murphy <dmurphy@xxxxxx>
> Cc: Jens Axboe <axboe@xxxxxxxxx>
> Cc: "James E.J. Bottomley" <jejb@xxxxxxxxxxxxx>
> Cc: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxxx>
> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
> ---
> * v3
> - Add ABI documentation
> - Add more detail to Kconfig help text
>
> .../ABI/testing/sysfs-class-led-trigger-blk | 37 ++++
> block/genhd.c | 2 +
> drivers/leds/trigger/Kconfig | 9 +
> drivers/leds/trigger/Makefile | 1 +
> drivers/leds/trigger/ledtrig-blk.c | 225 +++++++++++++++++++++
> include/linux/genhd.h | 3 +
> include/linux/leds.h | 27 +++
> 7 files changed, 304 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-blk
> create mode 100644 drivers/leds/trigger/ledtrig-blk.c
>
[...]
> +EXPORT_SYMBOL_GPL(ledtrig_blk_unregister);
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 8b5330d..b2c934e 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -17,6 +17,7 @@
> #include <linux/percpu-refcount.h>
> #include <linux/uuid.h>
> #include <linux/blk_types.h>
> +#include <linux/leds.h>
> #include <asm/local.h>
>
> #ifdef CONFIG_BLOCK
> @@ -219,6 +220,8 @@ struct gendisk {
> int node_id;
> struct badblocks *bb;
> struct lockdep_map lockdep_map;
> +
> + struct ledtrig_blk led;
Please rename this property to led_trig to reflect
its purpose accurately.
> };
>
> static inline struct gendisk *part_to_disk(struct hd_struct *part)
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 9b2bf57..395fa61 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -517,4 +517,31 @@ static inline void ledtrig_audio_set(enum led_audio type,
> }
> #endif
>
> +struct gendisk;
> +
> +#ifdef CONFIG_LEDS_TRIGGER_BLOCK
> +
> +struct ledtrig_blk {
> + struct led_trigger trig;
> +};
> +
> +int ledtrig_blk_register(struct gendisk *disk);
> +void ledtrig_blk_unregister(struct gendisk *disk);
> +
> +#else
> +
> +struct ledtrig_blk {
> +};
> +
> +static inline int ledtrig_blk_register(struct gendisk *disk)
> +{
> + return 0;
> +}
> +
> +static inline void ledtrig_blk_unregister(struct gendisk *disk)
> +{
> +}
> +
> +#endif /* CONFIG_LEDS_TRIGGER_BLOCK */
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
