Re: mtd:nor: unlock enhancement for cmdset0002

[Honza PetrouÅ]

Sorry for self-answering, but I forgot to add mtd maintainers and
mtd-mailinglist.

/Honza

2017-05-11 12:11 GMT+02:00 Honza PetrouÅ <jpetrous@xxxxxxxxx>:
> Hi,
>
> I'm not so much experienced in MTD area, so please correct me, if I'm wrong.
>
> NOR flashes supported by cmdset0002 (AMD & Fujitsu Standard Vendor Command Set)
> are not able to unlock one particular eraseblock, the unlocking is
> done by unlock
> the whole chip. Because of that the driver (cfi_cmdset_0002.c) do it
> in free steps:
>   1) remember all locked eraseblocks
>   2) do chip unlock
>   3) lock all remaining eraseblocks (minus, of course ones which have to be
> enabled by current ioctl call).
>
> Unfortunately, in step 2 (unlocking chip) there is used unlocking over
> all eraseblocks
> which belong to requested unlock area. It means that chip unlock operation
> is doing multiple times, without any reason of doing so. It is enough to do
> unlock only on one eraseblock (as it already means full chip unlock). This way
> we can save programming time, on bigger parts even more dramatically
> (one chip unlock per request vs. [number of eraseblocks] * chip unlock).
>
> So I would like to ask, if my finding is correct.
>
> Thanks
> /Honza
>
> BTW, in current project we have one NOR chip, so when I applied
> the following patch (I know it has to be generalized, even now I
> understand that it can not work correctly in case of multiple NOR chips
> or with different eraseblock sizes, ... etc) I lowered unlocking for 20x
> on 64MB filesystem.
>
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 9dca881..36e45d2 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -2694,8 +2694,8 @@ static int __maybe_unused cfi_ppb_unlock(struct
> mtd_info *mtd, loff_t ofs,
>         }
>
>         /* Now unlock the whole chip */
> -       ret = cfi_varsize_frob(mtd, do_ppb_xxlock, ofs, len,
> -                              DO_XXLOCK_ONEBLOCK_UNLOCK);
> +       ret = do_ppb_xxlock(map, &cfi->chips[0], 0, regions[0].erasesize,
> +                                DO_XXLOCK_ONEBLOCK_UNLOCK);
>         if (ret) {
>                 kfree(sect);
>                 return ret;