Regression for NOR flash with multiple erase block regions

[Mathias Thore]

> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon at free-electrons.com]
> Sent: den 25 september 2017 10:14
> 
> On Mon, 25 Sep 2017 08:05:03 +0000
> Mathias Thore <Mathias.Thore at infinera.com> wrote:
> 
> > > -----Original Message-----
> > > From: Boris Brezillon [mailto:boris.brezillon at free-electrons.com]
> > > Sent: den 25 september 2017 09:30
> > >
> > > On Mon, 25 Sep 2017 06:28:18 +0000
> > > Mathias Thore <Mathias.Thore at infinera.com> wrote:
> > >
> > > > > From: Boris Brezillon
> > > > > [mailto:boris.brezillon at free-electrons.com]
> > > > > Sent: den 22 september 2017 22:05
> > > > >
> > > > > On Fri, 22 Sep 2017 18:27:42 +0000 Chris Packham
> > > > > <Chris.Packham at alliedtelesis.co.nz> wrote:
> > > > >
> > > > > > Hi Mathias,
> > > > > >
> > > > > > On 23/09/17 01:12, Mathias Thore wrote:
> > > > > > > Hello,
> > > > > > >
> > > > > > > Commit 1eeef2d7483a7e3f8d2dd2a5b9939b3b814dc549 included
> in
> > > > > > > Linux
> > > > > 4.13 (
> > > > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.g
> > > > > it/c
> > > > > ommit/d
> > > > >
> > >
> rivers/mtd/mtdpart.c?h=v4.13&id=1eeef2d7483a7e3f8d2dd2a5b9939b3b814
> > > > > dc549
> > > > > > > ) introduces a regression for NOR flash with multiple erase
> > > > > > > block regions of different sizes.
> > > > > > >
> > > > > > > Only the largest erase block size seems to be considered
> > > > > > > when determining if partitions are aligned. Partitions in
> > > > > > > smaller regions will be mounted as read-only. With Linux
> > > > > > > 4.12 and earlier, read/write access was available for these
> partitions.
> > > > >
> > > > > I don't understand how this could work before this patch? I
> > > > > mean, we were previously using mtd_mod_by_eb() to check part
> > > > > alignment and this functions is just returning the remainder of
> > > > > the off / erasesize division. So, assuming the erasesize of your
> > > > > NOR did not change between 4.12 and 4.13, I don't see how this
> > > > > commit could cause the regression you're describing here.
> > > >
> > > > Looking at the earlier code, in the call to mtd_mod_by_eb, the
> > > > parameter
> > > is slave->mtd. The slave mtd struct holds the correct erase size.
> > > The new code uses wr_alignment for all alignment tests, which comes
> > > from master/parent, and seems to always hold the largest possible erase
> size.
> > >
> > > Oh indeed! I didn't notice that the initial mtd_mod_by_eb() test was
> > > done against the slave dev and not the master one.
> > >
> > > Can you test the following patch and let me know it it solves your
> problem?
> >
> > The patch does not compile in my 4.13 tree. However,
> >
> > 	if (!(slave->mtd.flags & MTD_NO_ERASE))
> > 		wr_alignment = slave->mtd.erasesize; or
> > 	if (!(master->flags & MTD_NO_ERASE))
> > 		wr_alignment = slave->mtd.erasesize;
> >
> > do, and both work equally well to solve my problem.
> 
> Oops. I'll fix that and send a new version.
> 
> Thanks,
> 
> Boris
> 

Thank you for solving this problem!
/Mathias


> >
> > >
> > > --->8---
> > > From cdc8a5078ee330a645d1076a8289b411cffc4257 Mon Sep 17 00:00:00
> > > 2001
> > > From: Boris Brezillon <boris.brezillon at free-electrons.com>
> > > Date: Mon, 25 Sep 2017 09:14:00 +0200
> > > Subject: [PATCH] mtd: Fix partition alignment check on
> > > multi-erasesize devices
> > >
> > > Commit 1eeef2d7483a ("mtd: handle partitioning on devices with 0
> > > erasesize") introduced a regression on heterogeneous erase region
> devices.
> > > Alignment of the partition was tested against the master eraseblock
> > > size which can be bigger than the slave one, thus leading to some
> > > partitions being marked as read-only.
> > >
> > > Update wr_alignment to match this slave erasesize after this
> > > erasesize has been determined by picking the biggest erasesize of
> > > all the regions embedded in the MTD partition.
> > >
> > > Reported-by: Mathias Thore <Mathias.Thore at infinera.com>
> > > Fixes: 1eeef2d7483a ("mtd: handle partitioning on devices with 0
> > > erasesize")
> > > Cc: <stable at vger.kernel.org>
> > > Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
> > > ---
> > >  drivers/mtd/mtdpart.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index
> > > 5736b0c90b33..8802185b7729 100644
> > > --- a/drivers/mtd/mtdpart.c
> > > +++ b/drivers/mtd/mtdpart.c
> > > @@ -581,6 +581,14 @@ static struct mtd_part
> > > *allocate_partition(struct mtd_info *parent,
> > >  		slave->mtd.erasesize = parent->erasesize;
> > >  	}
> > >
> > > +	/*
> > > +	 * Slave erasesize might differ from the master one if the
> > > master
> > > +	 * exposes several regions with different erasesize. Adjust
> > > +	 * wr_alignment accordingly.
> > > +	 */
> > > +	if (!(slave->flags & MTD_NO_ERASE))
> > > +		wr_alignment = slave->erasesize;
> > > +
> > >  	tmp = slave->offset;
> > >  	remainder = do_div(tmp, wr_alignment);
> > >  	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {