From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dwOXg-0007Wf-Ry for linux-mtd@lists.infradead.org; Mon, 25 Sep 2017 08:15:08 +0000 Date: Mon, 25 Sep 2017 10:14:25 +0200 From: Boris Brezillon To: Mathias Thore Cc: Chris Packham , "linux-mtd@lists.infradead.org" Subject: Re: Regression for NOR flash with multiple erase block regions Message-ID: <20170925101425.4389f713@bbrezillon> In-Reply-To: References: <4762f230bbd3437990b9f5baa61ab095@sv-ex13-prd1.infinera.com> <74af973f5ae542c3b53240c95949628c@svr-chch-ex1.atlnz.lc> <20170922220455.04f89f08@bbrezillon> <20170925093004.0e0af91e@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, 25 Sep 2017 08:05:03 +0000 Mathias Thore wrote: > > -----Original Message----- > > From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com] > > Sent: den 25 september 2017 09:30 > > > > On Mon, 25 Sep 2017 06:28:18 +0000 > > Mathias Thore wrote: > > > > > > From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com] > > > > Sent: den 22 september 2017 22:05 > > > > > > > > On Fri, 22 Sep 2017 18:27:42 +0000 > > > > Chris Packham 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.git/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 > > > > > --->8--- > > From cdc8a5078ee330a645d1076a8289b411cffc4257 Mon Sep 17 00:00:00 > > 2001 > > From: Boris Brezillon > > 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 > > Fixes: 1eeef2d7483a ("mtd: handle partitioning on devices with 0 erasesize") > > Cc: > > Signed-off-by: Boris Brezillon > > --- > > 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) {