[PATCH 2/2] leds: trigger: Introduce a MTD (NAND/NOR) trigger

[Ezequiel Garcia]

On 12 April 2016 at 15:27, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> Hi Ezequiel,
>
> On Tue, 12 Apr 2016 13:26:35 -0300
> Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> wrote:
>
>> This commit introduces a MTD trigger for flash (NAND/NOR) device
>> activity. The implementation is copied from IDE disk.
>>
>> This deprecates the "nand-disk" LED trigger, but for backwards
>> compatibility, we still keep the "nand-disk" trigger around.
>>
>> The motivation for deprecating the "nand-disk" LED trigger is that
>> it only works for NAND drivers, whereas the "mtd" LED trigger
>> is more generic (in fact, "nand-disk" currently only works for
>> certain NAND drivers).
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>
>> ---
>>  drivers/leds/trigger/Kconfig       |  8 +++++++
>>  drivers/leds/trigger/Makefile      |  1 +
>>  drivers/leds/trigger/ledtrig-mtd.c | 49 ++++++++++++++++++++++++++++++++++++++
>>  drivers/mtd/mtdcore.c              |  7 ++++++
>>  drivers/mtd/nand/nand_base.c       | 29 +---------------------
>>  include/linux/leds.h               |  6 +++++
>
> I'd suggest splitting this patch in 2, one adding ledtrig-mtd code, and
> another one make use of ledtrig_mtd_activity() and removing
> nand-trigger code.
>

Sure, that sounds good.

> IMHO, we should also factorize ledtrig-ide and ledtrig-mtd code
> (after all, you just copied the implementation from ledtrig-ide).
> How about renaming the ledtrig-ide into ledtrig-storage and
> ledtrig_ide_activity() into ledtrig_storage_activity()?
>
> You can then add a trigger named "storage" in addition to the existing
> "nand-disk" and "ide-disk" ones.
>

I'm not exactly following you: are you suggesting to suggest factorize
the code, or to meld the two triggers in a single "storage" trigger?

In the former, the code is so small, that I'm not sure it will be actually
reduced with some factorization. In the latter, wouldn't we loose
the capability to trigger on MTD and IDE activity independently?

> Anyway, this is just a suggestion, and I let leds maintainers decide
> whether this is appropriate or not.
>

Thanks for the reviewing Boris!

> Best Regards,
>
> Boris
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar