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 1dwNqQ-0000YA-7c for linux-mtd@lists.infradead.org; Mon, 25 Sep 2017 07:30:26 +0000 Date: Mon, 25 Sep 2017 09:30:04 +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: <20170925093004.0e0af91e@bbrezillon> In-Reply-To: References: <4762f230bbd3437990b9f5baa61ab095@sv-ex13-prd1.infinera.com> <74af973f5ae542c3b53240c95949628c@svr-chch-ex1.atlnz.lc> <20170922220455.04f89f08@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 > >=20 > > On Fri, 22 Sep 2017 18:27:42 +0000 > > Chris Packham wrote: > > =20 > > > Hi Mathias, > > > > > > On 23/09/17 01:12, Mathias Thore wrote: =20 > > > > Hello, > > > > > > > > Commit 1eeef2d7483a7e3f8d2dd2a5b9939b3b814dc549 included in Linux = =20 > > 4.13 ( =20 > > > > =20 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/comm= it/d > > rivers/mtd/mtdpart.c?h=3Dv4.13&id=3D1eeef2d7483a7e3f8d2dd2a5b9939b3b814 > > dc549 =20 > > > > ) 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/wri= te > > > > access was available for these partitions. =20 > >=20 > > 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. =20 >=20 > 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 c= ode uses wr_alignment for all alignment tests, which comes from master/pare= nt, 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? --->8--- =46rom 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 devi= ces 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 =3D parent->erasesize; } =20 + /* + * 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 =3D slave->erasesize; + tmp =3D slave->offset; remainder =3D do_div(tmp, wr_alignment); if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {