Patchwork [MTD,CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking

login
register
mail settings
Submitter Mike Frysinger
Date July 14, 2009, 1:35 p.m.
Message ID <1247578544-6950-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/29766/
State New
Headers show

Comments

Mike Frysinger - July 14, 2009, 1:35 p.m.
From: Graf Yang <graf.yang@analog.com>

We should try and keep the flash in the default read-array mode when
possible so that we are much more likely to properly recover from
unexpected reboot events (such as watchdog expiring).  If we only unlock
the flash, it can stay in that state for a while, so put it into
read-array mode after successful unlocking.

URL: http://blackfin.uclinux.org/gf/tracker/4887
Signed-off-by: Graf Yang <graf.yang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/chips/cfi_cmdset_0001.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Wolfram Sang - July 14, 2009, 1:56 p.m.
> We should try and keep the flash in the default read-array mode when
> possible so that we are much more likely to properly recover from
> unexpected reboot events (such as watchdog expiring).  If we only unlock

Hmm, the right thing to do would be fixing the watchdog to reset the flash
correctly, no? As your patch makes non-working watchdogs less obvious, I fear
that lockups will then happen when it's too late (= devices are deployed).

Regards,

   Wolfram
Mike Frysinger - July 14, 2009, 2:14 p.m.
On Tue, Jul 14, 2009 at 09:56, Wolfram Sang wrote:
>> We should try and keep the flash in the default read-array mode when
>> possible so that we are much more likely to properly recover from
>> unexpected reboot events (such as watchdog expiring).  If we only unlock
>
> Hmm, the right thing to do would be fixing the watchdog to reset the flash
> correctly, no? As your patch makes non-working watchdogs less obvious, I fear
> that lockups will then happen when it's too late (= devices are deployed).

the CFI layers already go through steps to keep the memory in read
mode by default.  the only way the watchdog could reconfigure the
flash is if it were setup to execute software instead of kick the
hardware, and that makes the system significantly less reliable.
-mike
Wolfram Sang - July 14, 2009, 2:32 p.m.
> the CFI layers already go through steps to keep the memory in read
> mode by default.

Ah, okay. With this assumption, the patch makes sense. Still, the patch
description should refer more to the above statement and not to watchdogs IMHO.

> the only way the watchdog could reconfigure the
> flash is if it were setup to execute software instead of kick the
> hardware, and that makes the system significantly less reliable.

I meant fixing the watchdog in hardware: If the watchdog pulls the right
#RESET, the flash will be in read-mode by default. If not, this is an important
hardware issue.

Regards,

   Wolfram
Mike Frysinger - July 14, 2009, 2:37 p.m.
On Tue, Jul 14, 2009 at 10:32, Wolfram Sang wrote:
>> the CFI layers already go through steps to keep the memory in read
>> mode by default.
>
> Ah, okay. With this assumption, the patch makes sense. Still, the patch
> description should refer more to the above statement and not to watchdogs IMHO.

ok

>> the only way the watchdog could reconfigure the
>> flash is if it were setup to execute software instead of kick the
>> hardware, and that makes the system significantly less reliable.
>
> I meant fixing the watchdog in hardware: If the watchdog pulls the right
> #RESET, the flash will be in read-mode by default. If not, this is an important
> hardware issue.

you're assuming the hardware has this functionality in the first
place.  while it is still an issue that should be addressed in
hardware, it's not possible for board designers to do so ;).
-mike

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index c240454..d96cc99 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -2056,6 +2056,11 @@  static int __xipram do_xxlock_oneblock(struct map_info *map, struct flchip *chip
 		goto out;
 	}
 
+	if (chip->state == FL_STATUS) {
+		/* it goes ok, put chip into array mode */
+		map_write(map, CMD(0xff), adr);
+		chip->state = FL_READY;
+	}
 	xip_enable(map, chip, adr);
 out:	put_chip(map, chip, adr);
 	spin_unlock(chip->mutex);