[U-Boot] Getting rid of board_mtdparts_default()

[Boris Brezillon]

On Fri, 16 Nov 2018 23:30:41 +0100
Enric Balletbo Serra <eballetbo at gmail.com> wrote:

> Missatge de Boris Brezillon <boris.brezillon at bootlin.com> del dia dj.,
> 15 de nov. 2018 a les 23:40:
> >
> > On Thu, 15 Nov 2018 23:18:46 +0100
> > Enric Balletbo Serra <eballetbo at gmail.com> wrote:
> >  
> > > Hello Boris,
> > >
> > > Missatge de Boris Brezillon <boris.brezillon at bootlin.com> del dia dj.,
> > > 15 de nov. 2018 a les 16:58:  
> > > >
> > > > Hello Enric,
> > > >
> > > > Miquel and I recently reworked drivers/mtd/mtdpart.c to get MTD
> > > > partitions exposed as MTD devices (as is done in Linux) instead of
> > > > having yet another abstraction to handle them (see what's currently
> > > > done in cmd/{mtdparts,nand,sf,...}.c).
> > > >
> > > > This lead us to duplicate the mtdparts/mtdids parsing logic we found in
> > > > cmd/mtdparts.c, and while doing that we noticed that one function is
> > > > only implemented by igep boards: board_mtdparts_default().
> > > >
> > > > We'd like to get rid of this function if possible in order to simplify
> > > > the mtdparts/mtdids retrieval logic, and there might be an easy
> > > > solution to do that: use the CONFIG_{MTDPARTS,MTDIDS}_DEFAULT config
> > > > options
> > > >
> > > > CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
> > > > CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
> > > >
> > > > It seems those defaults were built at runtime, and the only thing I'm
> > > > not sure about is whether 512k for the SPL is always good. Looks like
> > > > 4 eraseblocks are reserved for the SPL [1], but is the eraseblock size
> > > > always 128k?  
> > >
> > > No, there are boards in the field that doesn't have and eraseblock
> > > size of 128k, as far as I can remember there are at least boards with
> > > an eraseblock of 64k, I don't remember though, boards with and
> > > eraseblock size greater than 128k. About the 4 eraseblocks, there is a
> > > reason for this., the blocks are reserved for the SPL because you can
> > > flash 4 times (once per block) the SPL, the ROM code looks for an
> > > image in the first four blocks and it detects the validity status of
> > > these blocks. So, if the first one is corrupted, boots from the
> > > second, if the second is corrupted looks for the third, etc.
> > >
> > > Said this, I think that the main question is if it's fine to get rid
> > > of this code. Ideally, we should cover all the cases, so assuming that
> > > there aren't devices with an eraseblock size greater than 128k,
> > > reserve 512k seems a good number. There is, though, a corner case
> > > where this number is not fine. There are some boards that came with a
> > > OneNand(DDP), the ROM code is only capable to read the odd blocks (1,
> > > 3, 5, 7...). So, in this case, you need to reserve 896k (128k NA 128k
> > > NA 128k NA 128k ). Fwiw that was also not covered with current code.
> > >
> > > To be honest, I wouldn't worry about this latest case, and in my
> > > opinion, a fixed size of 512k is enough. Also, it's common now that
> > > the first few blocks of NAND flash are guaranteed to be safe for the
> > > first N erase cycles. So, I'm fine with the proposed fixed 512k for
> > > SPL. Btw, good job.  
> >
> > Okay, so next question is, does anyone rely on the default
> > partitioning? If that's the case we're screwed because using something
> > different will break things (UBI will be smaller than expected and
> > complain that blocks are missing).  
> 
> I don't rely on the default partitioning and the few people I know
> also don't rely on it. I can't talk for everyone as is more than 3
> year I left from the company that makes the IGEP, although I am pretty
> sure nobody relies on it. That partioning is the most common, so my
> suggestion is to go ahead and let's see if somebody complains.

Okay, I'll send a patch soon.

Thanks,

Boris