[U-Boot] [linux-sunxi] [RFC 11/11] mtd/nand: Sunxi NAND boot partition definitions

[Boris Brezillon]

On Sun, 14 Jun 2015 19:42:58 +0200
Michal Suchanek <hramrach at gmail.com> wrote:

> On 14 June 2015 at 14:18, Boris Brezillon
> <boris.brezillon at free-electrons.com> wrote:
> > On Sun, 14 Jun 2015 13:56:56 +0200
> > Michal Suchanek <hramrach at gmail.com> wrote:
> >
> >> On 14 June 2015 at 13:25, Boris Brezillon
> >> <boris.brezillon at free-electrons.com> wrote:
> >> > Hi Michal,
> >> >
> >> > On Sun, 7 Jun 2015 18:48:26 +0200
> >> > Michal Suchanek <hramrach at gmail.com> wrote:
> >> >
> >> >> Hello,
> >> >>
> >> >> On 5 June 2015 at 13:52, Roy Spliet <r.spliet at ultimaker.com> wrote:
> >> >> > Based on the default layout of the android image used at least on Olimex Lime
> >> >> >
> >> >> > Signed-off-by: Roy Spliet <r.spliet at ultimaker.com>
> >> >> > ---
> >> >> >  include/configs/sunxi-common.h | 9 +++++++++
> >> >> >  1 file changed, 9 insertions(+)
> >> >> >
> >> >> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> >> >> > index ec28c40..b38f2f5 100644
> >> >> > --- a/include/configs/sunxi-common.h
> >> >> > +++ b/include/configs/sunxi-common.h
> >> >> > @@ -404,8 +404,15 @@ extern int soft_i2c_gpio_scl;
> >> >> >  #define BOOT_TARGET_DEVICES_USB(func)
> >> >> >  #endif
> >> >> >
> >> >> > +#ifdef CONFIG_NAND
> >> >> > +#define BOOT_TARGET_DEVICES_NAND(func) func(NAND, nand , 0)
> >> >> > +#else
> >> >> > +#define BOOT_TARGET_DEVICES_NAND(func)
> >> >> > +#endif
> >> >> > +
> >> >> >  #define BOOT_TARGET_DEVICES(func) \
> >> >> >         BOOT_TARGET_DEVICES_MMC(func) \
> >> >> > +       BOOT_TARGET_DEVICES_NAND(func) \
> >> >> >         BOOT_TARGET_DEVICES_SCSI(func) \
> >> >> >         BOOT_TARGET_DEVICES_USB(func) \
> >> >> >         func(PXE, pxe, na) \
> >> >> > @@ -441,6 +448,8 @@ extern int soft_i2c_gpio_scl;
> >> >> >         MEM_LAYOUT_ENV_SETTINGS \
> >> >> >         "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \
> >> >> >         "console=ttyS0,115200\0" \
> >> >> > +       "mtdids=nand0=mtd2\0" \
> >> >> > +       "mtdparts=mtdparts=mtd2:0xffc00000 at 0x400000(nand0_main)\0" \
> >> >> >         BOOTENV
> >> >>
> >> >> From what I heard the nand boot partition size should be specified in
> >> >> nand pages rather than bytes because the boot rom loads a fixed number
> >> >> of pages and just uses the start of each page regardless of page size.
> >> >
> >> > AFAIK, the mtdparts format only allows you to express partition offsets
> >> > and sizes in bytes, and even if we had to change for something else, we
> >> > should choose NAND blocks rather than NAND pages.
> >> > The reason partitions should be block aligned in because the you can't
> >> > erase specific pages in a block, which means that if you define 2
> >> > partitions sharing the same block, you won't be able to update one
> >> > partition without potentially corrupting the other one.
> >>
> >> However, if the number of pages the boot0 partition takes up is not
> >> block aligned it means we cannot use the medium for booting and can
> >> just use 1 big partition anyway.
> >>
> >
> > Hm, I don't get your point.
> > You can have a boot0 partition taking one NAND block, and several other
> > partitions used for other purpose (though having a single UBI partition
> > is a better approach).
> 
> and how do you tell the kernel that these partitions follow the boot0
> taking up one block when you only count in bytes?

Oh come on! You just specify the appropriate size and offset in bytes.

> >
> >> >
> >> >>
> >> >> I did not find any document regarding the nand boot partition layout
> >> >> so I would like to see some input from somebody familiar with the
> >> >> driver.
> >> >
> >> > AFAIR, the mtd partition code checks for block alignment anyway, so you
> >> > shouldn't be allowed to create two partitions sharing the same block.
> >> >
> >> >>
> >> >> While it is fine for testing to hand-edit the environment the final
> >> >> nand support should have
> >> >>
> >> >> 1) way to express the boot partition size in nand pages
> >> >
> >> > Why should we add that ? The conversion from a number of blocks to a
> >> > number bytes is pretty straightforward (number_of_blocks *
> >> > block_size_in_bytes).
> >>
> >> Because the block size is not the same on all flash chips, obviously.
> >
> > And that's why partitions are defined in the board dts, and not the SoC
> > dtsi...
> >
> >>
> >> If there is only one block size that can ever be reasonably supported
> >> due to other constraints then it's fine to just hardcode it.
> >
> > I think we already had this discussion on the #linux-sunxi channel, and
> > you're trying to use a generic config from things that are really board
> > specific.
> > What's the problem with having different partition layout depending on
> > the board ?
> 
> Why do we have nand chip detection then?

There are a lot of other stuff detected from the NAND chip ID (or ONFI
or JEDEC standard), but remember that the partition stuff are supposed
to be common to all the MTD devices, and I'm not sure all of them can
use the erase size (or block size unit) to express their partitions
offsets/sizes.
Moreover, having the partition layout expressed in bytes is IMO clearer
than having it expressed in blocks.

> 
> We can just write the chip parameters in DT and have everything in one
> place at least. 

Yes, and we could also declare USB devices in the DT.

> 
> If we cannot use the detected parameters to infer the related values
> in DT then the detection is useless for DT based archs.

Again, you're not fair, the chip detection is used for a lot of things
(including detecting the NAND block size), and the fact that this
information is not used for partition definition, is really a minor
detail.

This being said, if you really want to change that, then propose a new
DT binding and a new formalism for the mtdparts parameter, and discuss
it with the MTD maintainers.
We don't need these changes for the sunxi NAND driver to work, so I'll
let you deal with that aspect.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com