public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
@ 2009-07-14 13:35 Mike Frysinger
  2009-07-14 13:56 ` Wolfram Sang
  2011-04-16  3:13 ` Mike Frysinger
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2009-07-14 13:35 UTC (permalink / raw)
  To: linux-mtd; +Cc: Graf Yang

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(-)

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);
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
  2009-07-14 13:35 [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking Mike Frysinger
@ 2009-07-14 13:56 ` Wolfram Sang
  2009-07-14 14:14   ` Mike Frysinger
  2011-04-16  3:13 ` Mike Frysinger
  1 sibling, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2009-07-14 13:56 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Graf Yang, linux-mtd

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

> 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

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
  2009-07-14 13:56 ` Wolfram Sang
@ 2009-07-14 14:14   ` Mike Frysinger
  2009-07-14 14:32     ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2009-07-14 14:14 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-mtd, Graf Yang

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
  2009-07-14 14:14   ` Mike Frysinger
@ 2009-07-14 14:32     ` Wolfram Sang
  2009-07-14 14:37       ` Mike Frysinger
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2009-07-14 14:32 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Graf Yang, linux-mtd

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


> 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

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
  2009-07-14 14:32     ` Wolfram Sang
@ 2009-07-14 14:37       ` Mike Frysinger
  0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2009-07-14 14:37 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Graf Yang, linux-mtd

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking
  2009-07-14 13:35 [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking Mike Frysinger
  2009-07-14 13:56 ` Wolfram Sang
@ 2011-04-16  3:13 ` Mike Frysinger
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2011-04-16  3:13 UTC (permalink / raw)
  To: linux-mtd, David Woodhouse, Bityutskiy; +Cc: Graf Yang

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(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 4aaa88f..abe2571 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -2089,6 +2089,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);
 	mutex_unlock(&chip->mutex);
-- 
1.7.5.rc1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-04-16  3:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-14 13:35 [PATCH] [MTD] [CHIPS] cfi_cmdset_0001.c: put chip into read-array mode after unlocking Mike Frysinger
2009-07-14 13:56 ` Wolfram Sang
2009-07-14 14:14   ` Mike Frysinger
2009-07-14 14:32     ` Wolfram Sang
2009-07-14 14:37       ` Mike Frysinger
2011-04-16  3:13 ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox