(Artem, any opinions, since you had the most opinion last time this came
up?)
On Mon, Jul 25, 2016 at 12:31:39PM +0200, Richard Weinberger wrote:
> Am 25.07.2016 um 11:46 schrieb Rajeev Kumar:
> > If the master mtd does not have any slave mtd partitions,
> > and its numeraseregions is one(only has one erease block), and
> > we attach the master mtd with : ubiattach -m 0 -d 0
> >
> > We will meet the error:
> > -------------------------------------------------------
> > root ~$ ubiattach /dev/ubi_ctrl -m 0 -d 0
> > UBI: attaching mtd0 to ubi0
> > UBI error: io_init: multiple regions, not implemented
> > ubiattach: error!: cannot attach mtd0
> > error 22 (Invalid argument)
> > -------------------------------------------------------
> >
> > In fact, if there is only one "erase block", we should not
> > prevent the attach.
> >
> > This patch is tested against 3.14 kernel and only build test is
> > performed against current upstream master branch.
>
> The more interesting question is, why is ->numeraseregions not 0?
>
> The comment in the header says:
> /* Data for variable erase regions. If numeraseregions is zero,
> * it means that the whole device has erasesize as given above.
> */
>
> So, if your MTD erase regions with the same size, it should be 0.
>
> IIRC we had such a discussion already on linux-mtd and it was not clear
> whether numeraseregions of 0 and 1 are equal or not.
I think 0 and 1 are essentially equal. But there's some potential room
for error (e.g., if the driver doesn't configure mtd->erasesize ==
mtd->eraseregions[0].erasesize, and similar). Also, I see some
problematic code like this in cfi_cmdset_0001.c:
mtd->numeraseregions = cfi->cfiq->NumEraseRegions * cfi->numchips;
So, if there are two chips, but both have a single erase region, with
the same erasesize, we'll still end up with ->numeraseregions == 2. We
can't hack all MTD users to start searching the eraseregions[] array to
see if the device is actually uniform, even if it reports
numeraseregions > 0.
I'm inclined, then, to outlaw numeraseregions == 1 (to make it simpler for MTD
users to handle), and put code either in drivers or in mtdcore.c to be slightly
more intelligent (e.g., if the driver left numeraseregions == 1, then just do
some sanity checking and re-set numeraseregions to 0). It might be good to move
code like this from cfi_cmdset_0001.c into mtdcore.c at the same time too:
if (offset != devsize) {
/* Argh */
printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
goto setup_err;
}
BTW, Rajeev, what devices are you testing? Just curious.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html