[PATCH] Fix of broken state in CFI driver caused by FL_SHUTDOWN
Jörn Engel
joern at logfs.org
Fri Apr 4 05:34:00 EDT 2008
On Fri, 4 April 2008 10:06:59 +0100, Alexey Korolev wrote:
>
> CFI driver in 2.6.24 kernel is broken. Not so intensive read/write operations cause incomplete writes which lead to kernel panics in JFFS2.
> We investigated the issue - it is caused by bug in FL_SHUTDOWN parsing code. Sometimes chip returns -EIO as if it is in FL_SHUTDOWN state when it should wait in FL_PONT (error in order of conditions).
>
> The following patch fixes the problem.
> Could you please include it?
>
> Signed-off-by: Alexey Korolev <akorolev at infradead.org>
>
> diff -aurpp a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c 2008-02-11 08:51:11.000000000 +0300
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c 2008-04-04 12:46:24.000000000 +0400
> @@ -729,14 +729,14 @@ static int chip_ready (struct map_info *
> chip->state = FL_READY;
> return 0;
>
> + case FL_SHUTDOWN:
> + /* The machine is rebooting now,so no one can get chip anymore */
> + return -EIO;
> case FL_POINT:
> /* Only if there's no operation suspended... */
> if (mode == FL_READY && chip->oldstate == FL_READY)
> return 0;
Ouch! I've been bitten by these before. After I've carefully written
code that takes advantage of missing break; statements, a colleague of
mine reordered the code and missed those subtleties.
Could you add a comment like this where appropriate:
/* fall through */
It might even have prevented this bug if Kevin had seen such a comment
before writing his patch.
Reviewed-By: Joern Engel <joern at logfs.org>
>
> - case FL_SHUTDOWN:
> - /* The machine is rebooting now,so no one can get chip anymore */
> - return -EIO;
> default:
> sleep:
> set_current_state(TASK_UNINTERRUPTIBLE);
Jörn
--
When in doubt, punt. When somebody actually complains, go back and fix it...
The 90% solution is a good thing.
-- Rob Landley
More information about the linux-mtd
mailing list