Patchwork [PATCHv2,1/2] MTD: cfi_cmdset_0002: Add reboot notifier for AMD flashes

login
register
mail settings
Submitter Kevin Cernekee
Date April 15, 2010, 12:57 a.m.
Message ID <a00f98bfcda7db841108605ebab8d07363f4aa8b@localhost.localdomain>
Download mbox | patch
Permalink /patch/50206/
State New
Headers show

Comments

Kevin Cernekee - April 15, 2010, 12:57 a.m.
Ensure that the flash device is in a quiescent state before rebooting.
The implementation is closely modeled after the cfe_cmdset_0001 reboot
notifier, commit 963a6fb0a0d336d0513083b7e4b5c3ff9d6d2061 .

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |   56 +++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)
Wolfram Sang - April 15, 2010, 2:53 a.m.
On Wed, Apr 14, 2010 at 05:57:33PM -0700, Kevin Cernekee wrote:
> Ensure that the flash device is in a quiescent state before rebooting.
> The implementation is closely modeled after the cfe_cmdset_0001 reboot
> notifier, commit 963a6fb0a0d336d0513083b7e4b5c3ff9d6d2061 .
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>

Yes, if this is present for Intel, it should be present for AMD, IMHO.

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

Still, the correct solution is to wire the correct reset-line to the flashes.
Above patch will not help if a watchdog kicks in. Been there :(

Regards,

   Wolfram
Artem Bityutskiy - April 27, 2010, 1:07 p.m.
On Wed, 2010-04-14 at 17:57 -0700, Kevin Cernekee wrote:
> Ensure that the flash device is in a quiescent state before rebooting.
> The implementation is closely modeled after the cfe_cmdset_0001 reboot
> notifier, commit 963a6fb0a0d336d0513083b7e4b5c3ff9d6d2061 .
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c |   56 +++++++++++++++++++++++++++++++++++
>  1 files changed, 56 insertions(+), 0 deletions(-)

This patch conflicts with the patch from Stefani, which is sitting in my
l2-mtd-2.6.git tree
(http://git.infradead.org/users/dedekind/l2-mtd-2.6.git). Could you
please amend it and re-send?

The conflicting patch is:

Author: Stefani Seibold <stefani@seibold.net>
Date:   Sun Apr 18 22:46:44 2010 +0200

    mtd: fix a huge latency problem in the MTD CFI and LPDDR flash drivers.
    
    The use of a memcpy() during a spinlock operation will cause very long
    thread context switch delays if the flash chip bandwidth is low and the
    data to be copied large, because a spinlock will disable preemption.
    
    For example: A flash with 6,5 MB/s bandwidth will cause under ubifs,
    which request sometimes 128 KB (the flash erase size), a preemption delay of
    20 milliseconds. High priority threads will not be served during this
    time, regardless whether this threads access the flash or not. This behavior
    breaks real time.
    
    The patch changes all the use of spin_lock operations for xxxx->mutex
    into mutex operations, which is exact what the name says and means.
    
    I have checked the code of the drivers and there is no use of atomic
    pathes like interrupt or timers. The mtdoops facility will also not be used
    by this drivers. So it is dave to replace the spin_lock against mutex.
    
    There is no performance regression since the mutex is normally not
    acquired.
    
    Changelog:
     06.03.2010 First release
     26.03.2010 Fix mutex[1] issue and tested it for compile failure
    
    Signed-off-by: Stefani Seibold <stefani@seibold.net>
    Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Jamie Lokier - April 27, 2010, 1:47 p.m.
Wolfram Sang wrote:
> On Wed, Apr 14, 2010 at 05:57:33PM -0700, Kevin Cernekee wrote:
> > Ensure that the flash device is in a quiescent state before rebooting.
> > The implementation is closely modeled after the cfe_cmdset_0001 reboot
> > notifier, commit 963a6fb0a0d336d0513083b7e4b5c3ff9d6d2061 .
> > 
> > Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> 
> Yes, if this is present for Intel, it should be present for AMD, IMHO.
> 
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> Still, the correct solution is to wire the correct reset-line to the flashes.
> Above patch will not help if a watchdog kicks in. Been there :(

Isn't the patch intended to handle _software_ resets where the hard reset
line isn't toggled?

-- Jamie

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index ea2a7f6..f40d308 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -32,6 +32,7 @@ 
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 #include <linux/mtd/compatmac.h>
 #include <linux/mtd/map.h>
 #include <linux/mtd/mtd.h>
@@ -56,6 +57,7 @@  static int cfi_amdstd_erase_varsize(struct mtd_info *, struct erase_info *);
 static void cfi_amdstd_sync (struct mtd_info *);
 static int cfi_amdstd_suspend (struct mtd_info *);
 static void cfi_amdstd_resume (struct mtd_info *);
+static int cfi_amdstd_reboot(struct notifier_block *, unsigned long, void *);
 static int cfi_amdstd_secsi_read (struct mtd_info *, loff_t, size_t, size_t *, u_char *);
 
 static void cfi_amdstd_destroy(struct mtd_info *);
@@ -351,6 +353,8 @@  struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 	mtd->name    = map->name;
 	mtd->writesize = 1;
 
+	mtd->reboot_notifier.notifier_call = cfi_amdstd_reboot;
+
 	if (cfi->cfi_mode==CFI_MODE_CFI){
 		unsigned char bootloc;
 		/*
@@ -487,6 +491,7 @@  static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 #endif
 
 	__module_get(THIS_MODULE);
+	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;
 
  setup_err:
@@ -628,6 +633,10 @@  static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr
 		chip->state = FL_READY;
 		return 0;
 
+	case FL_SHUTDOWN:
+		/* The machine is rebooting */
+		return -EIO;
+
 	case FL_POINT:
 		/* Only if there's no operation suspended... */
 		if (mode == FL_READY && chip->oldstate == FL_READY)
@@ -1918,11 +1927,58 @@  static void cfi_amdstd_resume(struct mtd_info *mtd)
 	}
 }
 
+
+/*
+ * Ensure that the flash device is put back into read array mode before
+ * unloading the driver or rebooting.  On some systems, rebooting while
+ * the flash is in query/program/erase mode will prevent the CPU from
+ * fetching the bootloader code, requiring a hard reset or power cycle.
+ */
+static int cfi_amdstd_reset(struct mtd_info *mtd)
+{
+	struct map_info *map = mtd->priv;
+	struct cfi_private *cfi = map->fldrv_priv;
+	int i, ret;
+	struct flchip *chip;
+
+	for (i = 0; i < cfi->numchips; i++) {
+
+		chip = &cfi->chips[i];
+
+		spin_lock(chip->mutex);
+
+		ret = get_chip(map, chip, chip->start, FL_SHUTDOWN);
+		if (!ret) {
+			map_write(map, CMD(0xF0), chip->start);
+			chip->state = FL_SHUTDOWN;
+			put_chip(map, chip, chip->start);
+		}
+
+		spin_unlock(chip->mutex);
+	}
+
+	return 0;
+}
+
+
+static int cfi_amdstd_reboot(struct notifier_block *nb, unsigned long val,
+			       void *v)
+{
+	struct mtd_info *mtd;
+
+	mtd = container_of(nb, struct mtd_info, reboot_notifier);
+	cfi_amdstd_reset(mtd);
+	return NOTIFY_DONE;
+}
+
+
 static void cfi_amdstd_destroy(struct mtd_info *mtd)
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
 
+	cfi_amdstd_reset(mtd);
+	unregister_reboot_notifier(&mtd->reboot_notifier);
 	kfree(cfi->cmdset_priv);
 	kfree(cfi->cfiq);
 	kfree(cfi);