Re: [PATCH] mtd: Introduce CONFIG_MTD_RESERVE_END

Paul Bolle
2015-04-30T22:03:03Z

While you're discussing more substantial questions with Brian, I found
some nits.

On Wed, 2015-04-29 at 14:28 -0500, Ben Shelton wrote:
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig

> +config MTD_RESERVE_END
> + int "Reserved space at the end of an all remaining space partition"
> + depends on MTD_CMDLINE_PARTS = "y"

(The quotes are unneeded.)

MTD_CMDLINE_PARTS is tristate. Why does it need to be built-in for this
symbol?

> + default 0
> + ---help---
> + Specify an amount of reserved space at the end of the last MTD
> + partition when the size is specified with '-' to denote all
> + remaining space.
> +
> + This can be useful if, for example, the BBT is stored at the end
> + of the flash, and you don't want those blocks counted as part of
> + the last MTD partition. This is less heavyweight than reserving
> + the BBT blocks with a separate MTD partition. The BBT marks its
> + own blocks as bad blocks, which prevents an MTD driver such as
> + UBI from getting an accurate count of the actual bad blocks in
> + the MTD partition that contains the BBT.
> +
> + The value is specified in bytes. As an example, a typical BBT
> + reserves four erase blocks, and a typical erase block size is
> + 128kB. To reserve that much space at the end of the flash, the
> + value for this config option would be 524288.
> +
> + If unsure, use the default value of zero.

> --- a/drivers/mtd/cmdlinepart.c
> +++ b/drivers/mtd/cmdlinepart.c
> @@ -340,7 +340,8 @@ static int parse_cmdline_partitions(struct mtd_info *master,
> offset = part->parts[i].offset;
>
> if (part->parts[i].size == SIZE_REMAINING)
> - part->parts[i].size = master->size - offset;
> + part->parts[i].size = master->size - offset -
> + CONFIG_MTD_RESERVE_END;

I haven't tested this. (Quite often that means: I'm wrong.) But if
MTD_CMDLINE_PARTS is set to "m" I think MTD_RESERVE_END will not be set.
In that case CPP will helpfully set CONFIG_MTD_RESERVE_END to zero
(which is what you want). But it should also trigger this warning:
"CONFIG_MTD_RESERVE_END" is not defined [-Wundef]

>
> if (offset + part->parts[i].size > master->size) {
> printk(KERN_WARNING ERRP

Paul Bolle