Re: Numonyx NOR and chip->mutex bug?

[Joakim Tjernlund <joakim.tjernlund@transmode.se>]

>
> Stefan Bigler <stefan.bigler@keymile.com> wrote on 2011/02/04 12:04:40:
> >
> > Hi Jocke
> >
> > The code does the same, but is much nicer :-)
> > My test is also working again.
> >
> > Im on the way to find out why our old P30 (130nm) Flash worked
> > without the fix.
>
> Stefan,
>
> I think the bug lies in
> static int inval_cache_and_wait_for_operation(
>       struct map_info *map, struct flchip *chip,
>       unsigned long cmd_adr, unsigned long inval_adr, int inval_len,
>       unsigned int chip_op_time, unsigned int chip_op_time_max)
> {
>    struct cfi_private *cfi = map->fldrv_priv;
>    map_word status, status_OK = CMD(0x80);
>    int chip_state = chip->state;
>    unsigned int timeo, sleep_time, reset_timeo;
>
>    mutex_unlock(&chip->mutex);
>    if (inval_len)
>       INVALIDATE_CACHED_RANGE(map, inval_adr, inval_len);
>    mutex_lock(&chip->mutex);
>
> Here we drop the lock and take it again. Someone may suspend the
> erase and do a read/write instead and put the chip some other state.
> Later we blindly do:
>    for (;;) {
>       status = map_read(map, cmd_adr);
>       if (map_word_andequal(map, status, status_OK, status_OK))
>          break;
> But we don't know what we are reading really.
>
> To test my theory, just remove the
>    mutex_unlock(&chip->mutex);
>    if (inval_len)
>       INVALIDATE_CACHED_RANGE(map, inval_adr, inval_len);
>    mutex_lock(&chip->mutex);
> and see what happens. This assumes that INVALIDATE_CACHED_RANGE is a NOP
> in your setup though.
>
>  Jocke

Noticed this too:
	if (sleep_time >= 1000000/HZ) {
			/*
			 * Half of the normal delay still remaining
			 * can be performed with a sleeping delay instead
			 * of busy waiting.
			 */
			msleep(sleep_time/1000);
			timeo -= sleep_time;
			sleep_time = 1000000/HZ;
	} ...

This is broken and may cause needless delays, possibly underflow timeo
too.

 Jocke