Re: [RFC PATCH 2/2] leds: trigger: implement block trigger

Pavel Machek
2021-05-03T10:11:39Z

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

Hi!

Please trim the emails you are responding to.

> >+MODULE_AUTHOR("Enzo Matsumiya <ematsumiya@suse.de>");
> >+MODULE_DESCRIPTION("LED block trigger");
> >+MODULE_LICENSE("GPL v2");
> >+
> >
> As already commented on, this for_each_blk() construct is not a good idea.
> Infact, I guess it would be better if you could invert the logic:
> Not having the block trigger enumerating all devices, but rather let the
> devices register with the block trigger.
> That would have the benefit that one could choose which block device should
> be handled by the LED trigger subsystem, _and_ you would avoid the need for
> a for_each_blk() construct.
> Thing is, I don't think that all block devices should be handled by the LED
> trigger; eg for things like 'loop' or 'ramdisk' it is very
> >questionable.

> Downside is that you would need to modify the drivers, but realistically
> there are only very few drivers which should be modified; I would go for
> nvme-pci and the sd driver for starters. Maybe floppy, but arguably that can
> omitted as one has a very good audio indicator for floppy accesses
> :-)

And we already have disk activity trigger. Maybe NVMe and SD needs to
be modified to use it?

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]