Message ID | 20190522000628.13073-1-chris.packham@alliedtelesis.co.nz |
---|---|
State | Accepted |
Delegated to: | Miquel Raynal |
Headers | show |
Series | mtd: cfi_cmdset_0002: dynamically determine the max sectors | expand |
On 22.05.19 02:06, Chris Packham wrote: > Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to > remember the locked status for each sector so it can re-lock the > unaddressed sectors. Dynamically calculate the maximum number of sectors > rather than using a hardcoded value that is too small for larger chips. > > Tested with Spansion S29GL01GS11TFI flash device. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> This hardcoded limit always annoyed me at that time, so thanks for "fixing" this: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
Hi All, I think this may have got lost in the change of maintainer for mtd. On 22/05/19 12:06 PM, Chris Packham wrote: > Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to > remember the locked status for each sector so it can re-lock the > unaddressed sectors. Dynamically calculate the maximum number of sectors > rather than using a hardcoded value that is too small for larger chips. > > Tested with Spansion S29GL01GS11TFI flash device. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > drivers/mtd/chips/cfi_cmdset_0002.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index c8fa5906bdf9..a1a7d334aa82 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -2533,8 +2533,6 @@ struct ppb_lock { > int locked; > }; > > -#define MAX_SECTORS 512 > - > #define DO_XXLOCK_ONEBLOCK_LOCK ((void *)1) > #define DO_XXLOCK_ONEBLOCK_UNLOCK ((void *)2) > #define DO_XXLOCK_ONEBLOCK_GETLOCK ((void *)3) > @@ -2633,6 +2631,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > int i; > int sectors; > int ret; > + int max_sectors; > > /* > * PPB unlocking always unlocks all sectors of the flash chip. > @@ -2640,7 +2639,11 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > * first check the locking status of all sectors and save > * it for future use. > */ > - sect = kcalloc(MAX_SECTORS, sizeof(struct ppb_lock), GFP_KERNEL); > + max_sectors = 0; > + for (i = 0; i < mtd->numeraseregions; i++) > + max_sectors += regions[i].numblocks; > + > + sect = kcalloc(max_sectors, sizeof(struct ppb_lock), GFP_KERNEL); > if (!sect) > return -ENOMEM; > > @@ -2689,9 +2692,9 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > } > > sectors++; > - if (sectors >= MAX_SECTORS) { > + if (sectors >= max_sectors) { > printk(KERN_ERR "Only %d sectors for PPB locking supported!\n", > - MAX_SECTORS); > + max_sectors); > kfree(sect); > return -EINVAL; > } >
On Fri, 2019-06-14 at 03:23 +0000, Chris Packham wrote: > > > Hi All, > > I think this may have got lost in the change of maintainer for mtd. We need this too, ATM we have a local hack that just changes MAX_SECTORS to 1024 > > On 22/05/19 12:06 PM, Chris Packham wrote: > > Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to > > remember the locked status for each sector so it can re-lock the > > unaddressed sectors. Dynamically calculate the maximum number of sectors > > rather than using a hardcoded value that is too small for larger chips. > > > > Tested with Spansion S29GL01GS11TFI flash device. > > > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > > --- > > drivers/mtd/chips/cfi_cmdset_0002.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > > index c8fa5906bdf9..a1a7d334aa82 100644 > > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > > @@ -2533,8 +2533,6 @@ struct ppb_lock { > > int locked; > > }; > > > > -#define MAX_SECTORS 512 > > - > > #define DO_XXLOCK_ONEBLOCK_LOCK ((void *)1) > > #define DO_XXLOCK_ONEBLOCK_UNLOCK ((void *)2) > > #define DO_XXLOCK_ONEBLOCK_GETLOCK ((void *)3) > > @@ -2633,6 +2631,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > > int i; > > int sectors; > > int ret; > > + int max_sectors; > > > > /* > > * PPB unlocking always unlocks all sectors of the flash chip. > > @@ -2640,7 +2639,11 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > > * first check the locking status of all sectors and save > > * it for future use. > > */ > > - sect = kcalloc(MAX_SECTORS, sizeof(struct ppb_lock), GFP_KERNEL); > > + max_sectors = 0; > > + for (i = 0; i < mtd->numeraseregions; i++) > > + max_sectors += regions[i].numblocks; > > + > > + sect = kcalloc(max_sectors, sizeof(struct ppb_lock), GFP_KERNEL); > > if (!sect) > > return -ENOMEM; > > > > @@ -2689,9 +2692,9 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > > } > > > > sectors++; > > - if (sectors >= MAX_SECTORS) { > > + if (sectors >= max_sectors) { > > printk(KERN_ERR "Only %d sectors for PPB locking supported!\n", > > - MAX_SECTORS); > > + max_sectors); > > kfree(sect); > > return -EINVAL; > > } > > > > ______________________________________________________ > Linux MTD discussion mailing list > https://nam03.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-mtd%2F&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7C32742fa3a7134e77f21908d6f078e67b%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C636960799408384144&sdata=JidMNGuW7GdQO%2FA%2BBs8Q0mnqt%2BWlDUnjsbCRJIDkDvU%3D&reserved=0
On 14/06/19 8:53 AM, Chris Packham wrote: > Hi All, > > I think this may have got lost in the change of maintainer for mtd. > I do have this in my queue of patches for mtd/next and will forward to Miquel once he is back. > On 22/05/19 12:06 PM, Chris Packham wrote: >> Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to >> remember the locked status for each sector so it can re-lock the >> unaddressed sectors. Dynamically calculate the maximum number of sectors >> rather than using a hardcoded value that is too small for larger chips. >> >> Tested with Spansion S29GL01GS11TFI flash device. >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Acked-by: Vignesh Raghavendra <vigneshr@ti.com> Regards Vignesh >> --- >> drivers/mtd/chips/cfi_cmdset_0002.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c >> index c8fa5906bdf9..a1a7d334aa82 100644 >> --- a/drivers/mtd/chips/cfi_cmdset_0002.c >> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c >> @@ -2533,8 +2533,6 @@ struct ppb_lock { >> int locked; >> }; >> >> -#define MAX_SECTORS 512 >> - >> #define DO_XXLOCK_ONEBLOCK_LOCK ((void *)1) >> #define DO_XXLOCK_ONEBLOCK_UNLOCK ((void *)2) >> #define DO_XXLOCK_ONEBLOCK_GETLOCK ((void *)3) >> @@ -2633,6 +2631,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, >> int i; >> int sectors; >> int ret; >> + int max_sectors; >> >> /* >> * PPB unlocking always unlocks all sectors of the flash chip. >> @@ -2640,7 +2639,11 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, >> * first check the locking status of all sectors and save >> * it for future use. >> */ >> - sect = kcalloc(MAX_SECTORS, sizeof(struct ppb_lock), GFP_KERNEL); >> + max_sectors = 0; >> + for (i = 0; i < mtd->numeraseregions; i++) >> + max_sectors += regions[i].numblocks; >> + >> + sect = kcalloc(max_sectors, sizeof(struct ppb_lock), GFP_KERNEL); >> if (!sect) >> return -ENOMEM; >> >> @@ -2689,9 +2692,9 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, >> } >> >> sectors++; >> - if (sectors >= MAX_SECTORS) { >> + if (sectors >= max_sectors) { >> printk(KERN_ERR "Only %d sectors for PPB locking supported!\n", >> - MAX_SECTORS); >> + max_sectors); >> kfree(sect); >> return -EINVAL; >> } >> >
Hi, On 22-May-19 5:36 AM, Chris Packham wrote: > Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to > remember the locked status for each sector so it can re-lock the > unaddressed sectors. Dynamically calculate the maximum number of sectors > rather than using a hardcoded value that is too small for larger chips. > > Tested with Spansion S29GL01GS11TFI flash device. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git branch mtd/next. Regards Vignesh > drivers/mtd/chips/cfi_cmdset_0002.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index c8fa5906bdf9..a1a7d334aa82 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -2533,8 +2533,6 @@ struct ppb_lock { > int locked; > }; > > -#define MAX_SECTORS 512 > - > #define DO_XXLOCK_ONEBLOCK_LOCK ((void *)1) > #define DO_XXLOCK_ONEBLOCK_UNLOCK ((void *)2) > #define DO_XXLOCK_ONEBLOCK_GETLOCK ((void *)3) > @@ -2633,6 +2631,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > int i; > int sectors; > int ret; > + int max_sectors; > > /* > * PPB unlocking always unlocks all sectors of the flash chip. > @@ -2640,7 +2639,11 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > * first check the locking status of all sectors and save > * it for future use. > */ > - sect = kcalloc(MAX_SECTORS, sizeof(struct ppb_lock), GFP_KERNEL); > + max_sectors = 0; > + for (i = 0; i < mtd->numeraseregions; i++) > + max_sectors += regions[i].numblocks; > + > + sect = kcalloc(max_sectors, sizeof(struct ppb_lock), GFP_KERNEL); > if (!sect) > return -ENOMEM; > > @@ -2689,9 +2692,9 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, > } > > sectors++; > - if (sectors >= MAX_SECTORS) { > + if (sectors >= max_sectors) { > printk(KERN_ERR "Only %d sectors for PPB locking supported!\n", > - MAX_SECTORS); > + max_sectors); > kfree(sect); > return -EINVAL; > } >
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index c8fa5906bdf9..a1a7d334aa82 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -2533,8 +2533,6 @@ struct ppb_lock { int locked; }; -#define MAX_SECTORS 512 - #define DO_XXLOCK_ONEBLOCK_LOCK ((void *)1) #define DO_XXLOCK_ONEBLOCK_UNLOCK ((void *)2) #define DO_XXLOCK_ONEBLOCK_GETLOCK ((void *)3) @@ -2633,6 +2631,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, int i; int sectors; int ret; + int max_sectors; /* * PPB unlocking always unlocks all sectors of the flash chip. @@ -2640,7 +2639,11 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, * first check the locking status of all sectors and save * it for future use. */ - sect = kcalloc(MAX_SECTORS, sizeof(struct ppb_lock), GFP_KERNEL); + max_sectors = 0; + for (i = 0; i < mtd->numeraseregions; i++) + max_sectors += regions[i].numblocks; + + sect = kcalloc(max_sectors, sizeof(struct ppb_lock), GFP_KERNEL); if (!sect) return -ENOMEM; @@ -2689,9 +2692,9 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs, } sectors++; - if (sectors >= MAX_SECTORS) { + if (sectors >= max_sectors) { printk(KERN_ERR "Only %d sectors for PPB locking supported!\n", - MAX_SECTORS); + max_sectors); kfree(sect); return -EINVAL; }
Because PPB unlocking unlocks the whole chip cfi_ppb_unlock() needs to remember the locked status for each sector so it can re-lock the unaddressed sectors. Dynamically calculate the maximum number of sectors rather than using a hardcoded value that is too small for larger chips. Tested with Spansion S29GL01GS11TFI flash device. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- drivers/mtd/chips/cfi_cmdset_0002.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)