Message ID | 1247578544-6950-1-git-send-email-vapier@gentoo.org |
---|---|
State | New, archived |
Headers | show |
> 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
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
> 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
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
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);