[PATCH V2] mtd:nor:timeout:fix do_write_buffer() timeout error

[Christian Riesch]

Hi,
I tried to test this version of your patch, but cannot apply it. On
which kernel version does it apply?

On Mon, Jun 23, 2014 at 4:50 AM, Bean Huo 霍斌斌 (beanhuo)
<beanhuo at micron.com> wrote:
> The size of the buffer program has been increased from 256 to 512 , 2ms maximum timeout can not adapt to all the different vendor's norflash, There maximum timeout information in the CFI area,so the best way is to choose the result calculated according to timeout field of struct cfi_ident that probed from norflash's CFI aera.This is also a standard defined by CFI.
>
> Without this change, if the size of buffer program is 512 or bigger than 256, due to timeout is the shorter than that the chip required,do_write_buffer sometimes fails.
>
> Tested with Micron JS28F512M29EWx and Micron MT28EW512ABA flash devices.
>
> Signed-off-by: bean huo <beanhuo at micron.com>
> ---
> changes v1->v2:Deleted unused parameters in this patch (word_write_time_max and erase_time_max).
> Using usecs_to_jiffies instead of msecs_to_jiffies for convert timeout value into jiffies.
>
>  drivers/mtd/chips/cfi_cmdset_0002.c |   27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index e21fde9..22e0b93 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -628,10 +628,24 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
>                 cfi->chips[i].word_write_time = 1<<cfi->cfiq->WordWriteTimeoutTyp;
>                 cfi->chips[i].buffer_write_time = 1<<cfi->cfiq->BufWriteTimeoutTyp;
>                 cfi->chips[i].erase_time = 1<<cfi->cfiq->BlockEraseTimeoutTyp;
> +               /*
> +                * We first calculate the timeout max according to timeout
> +                * field of struct cfi_ident that probed from chip's CFI
> +                * aera,If haven't probed this information,we will specify
> +                * a default value,and the time unit is us.
> +                */
> +               if (cfi->cfiq->BufWriteTimeoutTyp &&
> +                       cfi->cfiq->BufWriteTimeoutMax){
> +                       cfi->chips[i].buffer_write_time_max =
> +                               1<<(cfi->cfiq->BufWriteTimeoutTyp +
> +                                       cfi->cfiq->BufWriteTimeoutMax);
> +                       } else {
> +               /* specify maximum timeout for buffer program 2000us */
> +                               cfi->chips[i].buffer_write_time_max = 2000;
> +                               }
>                 cfi->chips[i].ref_point_counter = 0;
>                 init_waitqueue_head(&(cfi->chips[i].wq));
>         }
> -
>         map->fldrv = &cfi_amdstd_chipdrv;
>
>         return cfi_amdstd_setup(mtd);
> @@ -1462,8 +1476,15 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,  {
>         struct cfi_private *cfi = map->fldrv_priv;
>         unsigned long timeo = jiffies + HZ;
> -       /* see comments in do_write_oneword() regarding uWriteTimeo. */
> -       unsigned long uWriteTimeout = ( HZ / 1000 ) + 1;
> +       /* The size of the buffer program has been increased from 256 to 512,
> +        * 2ms maximum timeout can not adapt to all the different vendor's
> +        * norflash.There maximum timeout information in the CFI area,so the
> +        * best way is to choose the result calculated according to timeout
> +        * field of struct cfi_ident that probed from norflash's CFI aera,see
> +        * comments in cfi_cmdset_0002().uWriteTimeout is used for timeout step,
> +        * it must be concerted into jiffies.

I think the reference to the old 2ms timeout only confuses readers,
since they don't know the old code. Putting this into the commit
message is sufficient. I think you could delete this comment here.

> +        */
> +       unsigned long uWriteTimeout =

Trailing whitespace, please remove that. Please run your patch through
scripts/checkpatch.pl to detect such errors.

Christian

> +       usecs_to_jiffies(chip->buffer_write_time_max);
>         int ret = -EIO;
>         unsigned long cmd_adr;
>         int z, words;
> --
> 1.7.9.5
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/