[PATCH v2 0/4] LED trigger on MTD activity

[PATCH v2 0/4] LED trigger on MTD activity

[Ezequiel Garcia]

After our recent discussion [1], here's a proper patchset.

The first patch is a minor cleanup which moves mtd_write_oob
out of mtd.h header.

The second patch introduces the LED MTD trigger.

The third patch removes the "nand-disk" LED trigger
from the NAND core.

The fourth patch completes the series by calling ledtrig_mtd_activity
on all MTD I/O activity paths.

Changes from v1:

  * Split the patches, suggested by Boris.

Feedback is welcome!

[1] http://www.spinics.net/lists/linux-leds/msg05906.html

Ezequiel Garcia (4):
  mtd: Uninline mtd_write_oob and move it to mtdcore.c
  leds: trigger: Introduce a MTD (NAND/NOR) trigger
  mtd: nand: Remove the "nand-disk" LED trigger
  mtd: Hook I/O activity to the MTD LED trigger

 drivers/leds/trigger/Kconfig       |  8 +++++++
 drivers/leds/trigger/Makefile      |  1 +
 drivers/leds/trigger/ledtrig-mtd.c | 45 ++++++++++++++++++++++++++++++++++++++
 drivers/mtd/mtdcore.c              | 19 ++++++++++++++++
 drivers/mtd/nand/nand_base.c       | 29 +-----------------------
 include/linux/leds.h               |  6 +++++
 include/linux/mtd/mtd.h            | 12 +---------
 7 files changed, 81 insertions(+), 39 deletions(-)
 create mode 100644 drivers/leds/trigger/ledtrig-mtd.c

-- 
2.7.0

[PATCH v2 1/4] mtd: Uninline mtd_write_oob and move it to mtdcore.c

[Ezequiel Garcia]

There's no reason for having mtd_write_oob inlined in mtd.h header.
Move it to mtdcore.c where it belongs.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/mtdcore.c   | 12 ++++++++++++
 include/linux/mtd/mtd.h | 12 +-----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 309625130b21..99d83f3331b0 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -997,6 +997,18 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 }
 EXPORT_SYMBOL_GPL(mtd_read_oob);
 
+int mtd_write_oob(struct mtd_info *mtd, loff_t to,
+				struct mtd_oob_ops *ops)
+{
+	ops->retlen = ops->oobretlen = 0;
+	if (!mtd->_write_oob)
+		return -EOPNOTSUPP;
+	if (!(mtd->flags & MTD_WRITEABLE))
+		return -EROFS;
+	return mtd->_write_oob(mtd, to, ops);
+}
+EXPORT_SYMBOL_GPL(mtd_write_oob);
+
 /*
  * Method to access the protection register area, present in some flash
  * devices. The user data is one time programmable but the factory data is read
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 771272187316..ef9fea4fc400 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -283,17 +283,7 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
 		    const u_char *buf);
 
 int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops);
-
-static inline int mtd_write_oob(struct mtd_info *mtd, loff_t to,
-				struct mtd_oob_ops *ops)
-{
-	ops->retlen = ops->oobretlen = 0;
-	if (!mtd->_write_oob)
-		return -EOPNOTSUPP;
-	if (!(mtd->flags & MTD_WRITEABLE))
-		return -EROFS;
-	return mtd->_write_oob(mtd, to, ops);
-}
+int mtd_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops);
 
 int mtd_get_fact_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
 			   struct otp_info *buf);
-- 
2.7.0

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

[Ezequiel Garcia]

This commit introduces a MTD trigger for flash (NAND/NOR) device
activity. The implementation is copied from IDE disk.

This trigger 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@vanguardiasur.com.ar>
--
NAND core is also registering a "nand-disk" trigger, which will
clash with the trigger registered in this patch. This won't
cause any issues, except that one of them won't get registered.
The LED trigger code is prepared to handle a trigger register failure.
---
 drivers/leds/trigger/Kconfig       |  8 +++++++
 drivers/leds/trigger/Makefile      |  1 +
 drivers/leds/trigger/ledtrig-mtd.c | 45 ++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h               |  6 +++++
 4 files changed, 60 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-mtd.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 5bda6a9b56bb..df6949a0ce13 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
 	  This allows LEDs to be controlled by IDE disk activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_MTD
+	bool "LED MTD (NAND/NOR) Trigger"
+	depends on MTD
+	depends on LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by MTD activity.
+	  If unsure, say N.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends on LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 1abf48dacf7e..446f4603a397 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_ONESHOT)	+= ledtrig-oneshot.o
 obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
+obj-$(CONFIG_LEDS_TRIGGER_MTD)		+= ledtrig-mtd.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
 obj-$(CONFIG_LEDS_TRIGGER_BACKLIGHT)	+= ledtrig-backlight.o
 obj-$(CONFIG_LEDS_TRIGGER_GPIO)		+= ledtrig-gpio.o
diff --git a/drivers/leds/trigger/ledtrig-mtd.c b/drivers/leds/trigger/ledtrig-mtd.c
new file mode 100644
index 000000000000..99b5b0a4d826
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-mtd.c
@@ -0,0 +1,45 @@
+/*
+ * LED MTD trigger
+ *
+ * Copyright 2016 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
+ *
+ * Based on LED IDE-Disk Activity Trigger
+ *
+ * Copyright 2006 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <rpurdie@openedhand.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_mtd);
+DEFINE_LED_TRIGGER(ledtrig_nand);
+
+void ledtrig_mtd_activity(void)
+{
+	unsigned long blink_delay = BLINK_DELAY;
+
+	led_trigger_blink_oneshot(ledtrig_mtd,
+				  &blink_delay, &blink_delay, 0);
+	led_trigger_blink_oneshot(ledtrig_nand,
+				  &blink_delay, &blink_delay, 0);
+}
+EXPORT_SYMBOL(ledtrig_mtd_activity);
+
+static int __init ledtrig_mtd_init(void)
+{
+	led_trigger_register_simple("mtd", &ledtrig_mtd);
+	led_trigger_register_simple("nand-disk", &ledtrig_nand);
+
+	return 0;
+}
+device_initcall(ledtrig_mtd_init);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index f203a8f89d30..19eb10278bea 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -329,6 +329,12 @@ extern void ledtrig_ide_activity(void);
 static inline void ledtrig_ide_activity(void) {}
 #endif
 
+#ifdef CONFIG_LEDS_TRIGGER_MTD
+extern void ledtrig_mtd_activity(void);
+#else
+static inline void ledtrig_mtd_activity(void) {}
+#endif
+
 #if defined(CONFIG_LEDS_TRIGGER_CAMERA) || defined(CONFIG_LEDS_TRIGGER_CAMERA_MODULE)
 extern void ledtrig_flash_ctrl(bool on);
 extern void ledtrig_torch_ctrl(bool on);
-- 
2.7.0

[PATCH v2 3/4] mtd: nand: Remove the "nand-disk" LED trigger

[Ezequiel Garcia]

This commit removes the "nand-disk" LED trigger from the
NAND code.

A trigger with the same name is already available selecting
LEDS_TRIGGER_MTD. Note that "nand-disk" trigger is being
deprecated in favor of the "mtd" trigger.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/nand_base.c | 29 +----------------------------
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index b6facac54fc0..8d6287ef3926 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -43,7 +43,6 @@
 #include <linux/mtd/nand_bch.h>
 #include <linux/interrupt.h>
 #include <linux/bitops.h>
-#include <linux/leds.h>
 #include <linux/io.h>
 #include <linux/mtd/partitions.h>
 #include <linux/of_mtd.h>
@@ -97,12 +96,6 @@ static int nand_get_device(struct mtd_info *mtd, int new_state);
 static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 			     struct mtd_oob_ops *ops);
 
-/*
- * For devices which display every fart in the system on a separate LED. Is
- * compiled away when LED support is disabled.
- */
-DEFINE_LED_TRIGGER(nand_led_trigger);
-
 static int check_offs_len(struct mtd_info *mtd,
 					loff_t ofs, uint64_t len)
 {
@@ -540,19 +533,16 @@ void nand_wait_ready(struct mtd_info *mtd)
 	if (in_interrupt() || oops_in_progress)
 		return panic_nand_wait_ready(mtd, timeo);
 
-	led_trigger_event(nand_led_trigger, LED_FULL);
 	/* Wait until command is processed or timeout occurs */
 	timeo = jiffies + msecs_to_jiffies(timeo);
 	do {
 		if (chip->dev_ready(mtd))
-			goto out;
+			return;
 		cond_resched();
 	} while (time_before(jiffies, timeo));
 
 	if (!chip->dev_ready(mtd))
 		pr_warn_ratelimited("timeout while waiting for chip to become ready\n");
-out:
-	led_trigger_event(nand_led_trigger, LED_OFF);
 }
 EXPORT_SYMBOL_GPL(nand_wait_ready);
 
@@ -885,8 +875,6 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	int status;
 	unsigned long timeo = 400;
 
-	led_trigger_event(nand_led_trigger, LED_FULL);
-
 	/*
 	 * Apply this short delay always to ensure that we do wait tWB in any
 	 * case on any machine.
@@ -910,7 +898,6 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
 			cond_resched();
 		} while (time_before(jiffies, timeo));
 	}
-	led_trigger_event(nand_led_trigger, LED_OFF);
 
 	status = (int)chip->read_byte(mtd);
 	/* This can happen if in case of timeout or buggy dev_ready */
@@ -4474,20 +4461,6 @@ void nand_release(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(nand_release);
 
-static int __init nand_base_init(void)
-{
-	led_trigger_register_simple("nand-disk", &nand_led_trigger);
-	return 0;
-}
-
-static void __exit nand_base_exit(void)
-{
-	led_trigger_unregister_simple(nand_led_trigger);
-}
-
-module_init(nand_base_init);
-module_exit(nand_base_exit);
-
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Steven J. Hill <sjhill@realitydiluted.com>");
 MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
-- 
2.7.0

[PATCH v2 4/4] mtd: Hook I/O activity to the MTD LED trigger

[Ezequiel Garcia]

Now that we've added the MTD LED trigger, we need
to call each I/O path to ledtrig_mtd_activity.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/mtdcore.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 99d83f3331b0..bee180bd11e7 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -40,6 +40,7 @@
 #include <linux/slab.h>
 #include <linux/reboot.h>
 #include <linux/kconfig.h>
+#include <linux/leds.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -862,6 +863,7 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 		mtd_erase_callback(instr);
 		return 0;
 	}
+	ledtrig_mtd_activity();
 	return mtd->_erase(mtd, instr);
 }
 EXPORT_SYMBOL_GPL(mtd_erase);
@@ -925,6 +927,7 @@ int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
 	if (!len)
 		return 0;
 
+	ledtrig_mtd_activity();
 	/*
 	 * In the absence of an error, drivers return a non-negative integer
 	 * representing the maximum number of bitflips that were corrected on
@@ -949,6 +952,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
 		return -EROFS;
 	if (!len)
 		return 0;
+	ledtrig_mtd_activity();
 	return mtd->_write(mtd, to, len, retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_write);
@@ -982,6 +986,8 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 	ops->retlen = ops->oobretlen = 0;
 	if (!mtd->_read_oob)
 		return -EOPNOTSUPP;
+
+	ledtrig_mtd_activity();
 	/*
 	 * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
 	 * similar to mtd->_read(), returning a non-negative integer
@@ -1005,6 +1011,7 @@ int mtd_write_oob(struct mtd_info *mtd, loff_t to,
 		return -EOPNOTSUPP;
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
+	ledtrig_mtd_activity();
 	return mtd->_write_oob(mtd, to, ops);
 }
 EXPORT_SYMBOL_GPL(mtd_write_oob);
-- 
2.7.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 0/4] LED trigger on MTD activity

[Boris Brezillon]

On Tue, 12 Apr 2016 17:46:38 -0300
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:

> After our recent discussion [1], here's a proper patchset.
> 
> The first patch is a minor cleanup which moves mtd_write_oob
> out of mtd.h header.
> 
> The second patch introduces the LED MTD trigger.
> 
> The third patch removes the "nand-disk" LED trigger
> from the NAND core.
> 
> The fourth patch completes the series by calling ledtrig_mtd_activity
> on all MTD I/O activity paths.

LGTM.

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> 
> Changes from v1:
> 
>   * Split the patches, suggested by Boris.
> 
> Feedback is welcome!
> 
> [1] http://www.spinics.net/lists/linux-leds/msg05906.html
> 
> Ezequiel Garcia (4):
>   mtd: Uninline mtd_write_oob and move it to mtdcore.c
>   leds: trigger: Introduce a MTD (NAND/NOR) trigger
>   mtd: nand: Remove the "nand-disk" LED trigger
>   mtd: Hook I/O activity to the MTD LED trigger
> 
>  drivers/leds/trigger/Kconfig       |  8 +++++++
>  drivers/leds/trigger/Makefile      |  1 +
>  drivers/leds/trigger/ledtrig-mtd.c | 45 ++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/mtdcore.c              | 19 ++++++++++++++++
>  drivers/mtd/nand/nand_base.c       | 29 +-----------------------
>  include/linux/leds.h               |  6 +++++
>  include/linux/mtd/mtd.h            | 12 +---------
>  7 files changed, 81 insertions(+), 39 deletions(-)
>  create mode 100644 drivers/leds/trigger/ledtrig-mtd.c
> 



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

Re: [PATCH v2 0/4] LED trigger on MTD activity

[Jacek Anaszewski]

Hi Ezequiel,

Thanks for the update. From the previous discussion I've inferred
that it has been agreed on the whole patch to go through the LED tree.

Applied whole patch set to the for-next branch of linux-leds.git,
with acks from Boris, then.

Thanks,
Jacek Anaszewski

On 04/12/2016 10:46 PM, Ezequiel Garcia wrote:
> After our recent discussion [1], here's a proper patchset.
>
> The first patch is a minor cleanup which moves mtd_write_oob
> out of mtd.h header.
>
> The second patch introduces the LED MTD trigger.
>
> The third patch removes the "nand-disk" LED trigger
> from the NAND core.
>
> The fourth patch completes the series by calling ledtrig_mtd_activity
> on all MTD I/O activity paths.
>
> Changes from v1:
>
>    * Split the patches, suggested by Boris.
>
> Feedback is welcome!
>
> [1] http://www.spinics.net/lists/linux-leds/msg05906.html
>
> Ezequiel Garcia (4):
>    mtd: Uninline mtd_write_oob and move it to mtdcore.c
>    leds: trigger: Introduce a MTD (NAND/NOR) trigger
>    mtd: nand: Remove the "nand-disk" LED trigger
>    mtd: Hook I/O activity to the MTD LED trigger
>
>   drivers/leds/trigger/Kconfig       |  8 +++++++
>   drivers/leds/trigger/Makefile      |  1 +
>   drivers/leds/trigger/ledtrig-mtd.c | 45 ++++++++++++++++++++++++++++++++++++++
>   drivers/mtd/mtdcore.c              | 19 ++++++++++++++++
>   drivers/mtd/nand/nand_base.c       | 29 +-----------------------
>   include/linux/leds.h               |  6 +++++
>   include/linux/mtd/mtd.h            | 12 +---------
>   7 files changed, 81 insertions(+), 39 deletions(-)
>   create mode 100644 drivers/leds/trigger/ledtrig-mtd.c
>

Re: [PATCH v2 0/4] LED trigger on MTD activity

[Boris Brezillon]

Hi Jacek,

On Wed, 13 Apr 2016 10:36:22 +0200
Jacek Anaszewski <j.anaszewski@samsung.com> wrote:

> Hi Ezequiel,
> 
> Thanks for the update. From the previous discussion I've inferred
> that it has been agreed on the whole patch to go through the LED tree.
> 
> Applied whole patch set to the for-next branch of linux-leds.git,
> with acks from Boris, then.

Can you provide an immutable branch containing those changes?

Thanks,

Boris

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

Re: [PATCH v2 0/4] LED trigger on MTD activity

[Jacek Anaszewski]

Hi Boris,

On 04/13/2016 08:39 PM, Boris Brezillon wrote:
> Hi Jacek,
>
> On Wed, 13 Apr 2016 10:36:22 +0200
> Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
>
>> Hi Ezequiel,
>>
>> Thanks for the update. From the previous discussion I've inferred
>> that it has been agreed on the whole patch to go through the LED tree.
>>
>> Applied whole patch set to the for-next branch of linux-leds.git,
>> with acks from Boris, then.
>
> Can you provide an immutable branch containing those changes?

No problem, I've created mtd-nand-trigger branch in the repository

git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v2 0/4] LED trigger on MTD activity

[Boris Brezillon]

Hi Jacek,

On Thu, 14 Apr 2016 09:45:53 +0200
Jacek Anaszewski <j.anaszewski@samsung.com> wrote:

> Hi Boris,
> 
> On 04/13/2016 08:39 PM, Boris Brezillon wrote:
> > Hi Jacek,
> >
> > On Wed, 13 Apr 2016 10:36:22 +0200
> > Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> >
> >> Hi Ezequiel,
> >>
> >> Thanks for the update. From the previous discussion I've inferred
> >> that it has been agreed on the whole patch to go through the LED tree.
> >>
> >> Applied whole patch set to the for-next branch of linux-leds.git,
> >> with acks from Boris, then.
> >
> > Can you provide an immutable branch containing those changes?
> 
> No problem, I've created mtd-nand-trigger branch in the repository
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git
> 

I've merged this branch into nand/next.

Thanks,

Boris

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