Hi Rafał, Rafał Miłecki <rafal@xxxxxxxxxx> wrote on Sat, 02 May 2020 21:36:24 +0200: > On 2020-05-02 20:04, Miquel Raynal wrote: > > Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote on Mon, 9 Mar 2020 > > 16:22:23 +0100: > > > >> Hi Rafał, > >> >> Rafał Miłecki <rafal@xxxxxxxxxx> wrote on Mon, 9 Mar 2020 16:08:12 > >> +0100: > >> >> > On 09.03.2020 15:22, Miquel Raynal wrote: > >> > > Rafał Miłecki <rafal@xxxxxxxxxx> wrote on Mon, 09 Mar 2020 15:19:10 > >> > > +0100: > >> > > > >> > >> On 2020-03-09 15:04, Miquel Raynal wrote: > >> > >>> Rafał Miłecki <zajec5@xxxxxxxxx> wrote on Mon, 9 Mar 2020 08:44:45 > >> > >>> +0100: > >> > >>> >>>> From: Rafał Miłecki <rafal@xxxxxxxxxx> > >> > >>>>>> This fixes check for partitions that don't start at beginning of their > >> > >>>> parents. Missing partition's offset in formula could result in forcing > >> > >>>> read-only incorrectly. > >> > >>>>>> Fixes: 6750f61a13a0 ("mtd: improve calculating partition boundaries >> when checking for alignment") > >> > >>>> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx> > >> > >>>> --- > >> > >>>> drivers/mtd/mtdpart.c | 2 +- > >> > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>>>>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > >> > >>>> index 7328c066c5ba..c683b432cc5e 100644 > >> > >>>> --- a/drivers/mtd/mtdpart.c > >> > >>>> +++ b/drivers/mtd/mtdpart.c > >> > >>>> @@ -524,7 +524,7 @@ static struct mtd_part *allocate_partition(struct >> mtd_info *parent, > >> > >>>> part->name); > >> > >>>> } > >> > >>>>>> - tmp = part_absolute_offset(parent) + slave->mtd.size; > >> > >>>> + tmp = part_absolute_offset(parent) + slave->offset + >> slave->mtd.size; > >> > >>> > >> > >>> I think you are doing the change at the wrong place, if you want to > >> > >>> check where the partition *starts* you should do it a few lines above. > >> > >>> But I think the check should be here as well, probably. > >> > >> > >> > >> The check where the partition *starts* is OK and I don't mean to change > >> > >> it. The bug is about calculating absolute *end* address of partition. > >> > > > >> > > Can you detail a little bit then? Because I don't see the issue anymore > >> > > even though I am convinced something is wrong here :) > >> > > >> > Please consider following partitions layout: > >> > * bcm47xxsflash > >> > ├─ boot 0x000000000000-0x000000040000 > >> > └┬ firmware 0x000000040000-0x000001000000 > >> > ├─ linux 0x00000000001c-0x00000018f800 > >> > └┬ container 0x00000018f800-0x000000fc0000 > >> > ├─ foo 0x000000000000-0x000000630800 > >> > └─ bar 0x000000630800-0x000000e30800 (size: 0x800000) > >> > > >> > > >> > Existing (correct) *start* calculation: > >> > bar start: 0 + 0x040000 + 0x18f800 + 0x630800 = 0x800000 > >> > > >> > Existing (wrong) end calculation: > >> > bar end: 0 + 0x040000 + 0x18f800 + 0x800000 = 0x9cf800 > >> > > >> > Fixed (correct) end calculation: > >> > bar end: 0 + 0x040000 + 0x18f800 + 0x630800 + 0x800000 = 0x1000000 > >> >> Ok I get it! I think mentioning "partitions that don't start at > >> beginning of their parents", despite being true, was misleading to me > >> as I understood "leaving extra space with the start of their parent". > >> >> I suppose you also have the issue with "container" too? > >> >> Anyway, I think the fix is fine. A better formulation for the commit > >> log would be welcome :) (maybe adding this example is a good idea!) > > > > I don't remember having applied this fix yet, would you mind resending > > this patch with an enhanced commit log (your example was a good one I > > think). > > It's not needed anymore. Please check discussion we got: > > -------- Original Message -------- > Subject: Re: [PATCH] mtd: fix calculating partition end address > Date: 2020-03-24 23:11 > > > > I would like to apply your fix this week, do you think you can rebase > > > and resend soon? > > > > It's not needed anymore as you fixed this bug in your commit reworking > > partitions. > > Great! Yeah sorry about that, I got confused while listing remaining patches :) ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/