Hi Sascha,
only some small nitpicks.
On Thu, Feb 13, 2014 at 11:25:28AM +0100, Sascha Hauer wrote:
> Embed the partition information in struct mtd_info. This makes the
> mtd partition code simpler.
>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> ---
> drivers/mtd/partition.c | 75 ++++++++++++++++++++-----------------------------
> include/linux/mtd/mtd.h | 5 +++-
> 2 files changed, 34 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/mtd/partition.c b/drivers/mtd/partition.c
> index 85f486d..7431fc1 100644
> --- a/drivers/mtd/partition.c
> +++ b/drivers/mtd/partition.c
> @@ -4,29 +4,19 @@
> #include <linux/err.h>
> #include <linux/mtd/mtd.h>
>
...
> + struct mtd_info *part;
> int start = 0, end = 0, i;
>
> - slave = xzalloc(sizeof(*slave));
> - slave_mtd = &slave->mtd;
> + part = xzalloc(sizeof(*part));
>
> - memcpy(slave_mtd, mtd, sizeof(*slave));
> + memcpy(part, mtd, sizeof(*part));
Maybe we should fixup this commit with patch:
[PATCH 6/8] mtd: partition: only copy selected fields to partition
>
> /*
> * find the number of eraseregions the partition includes.
> @@ -118,26 +101,28 @@ struct mtd_info *mtd_add_partition(struct mtd_info *mtd, off_t offset, size_t si
> end = i;
> }
>
> - slave_mtd->numeraseregions = end - start;
> + part->numeraseregions = end - start;
>
> - slave_mtd->read = mtd_part_read;
> - slave_mtd->write = mtd_part_write;
> - slave_mtd->erase = mtd_part_erase;
> - slave_mtd->block_isbad = mtd->block_isbad ? mtd_part_block_isbad : NULL;
> - slave_mtd->block_markbad = mtd->block_markbad ? mtd_part_block_markbad : NULL;
> - slave_mtd->size = size;
> - slave_mtd->name = strdup(name);
> + part->read = mtd_part_read;
> + part->write = mtd_part_write;
> + part->erase = mtd_part_erase;
> + part->block_isbad = mtd->block_isbad ? mtd_part_block_isbad : NULL;
> + part->block_markbad = mtd->block_markbad ? mtd_part_block_markbad : NULL;
> + part->size = size;
> + part->name = strdup(name);
>
> - slave->offset = offset;
> - slave->master = mtd;
> + part->master_offset = offset;
> + part->master = mtd;
>
> - return slave_mtd;
> + return part;
> }
>
> -void mtd_del_partition(struct mtd_info *mtd)
> +int mtd_del_partition(struct mtd_info *part)
> {
> - struct mtd_part *part = PART(mtd);
> + if (!part->master)
> + return -EINVAL;
>
> - free(mtd->name);
I think this should be free(part->name);
...
Yea I know it's only a bootloader but I can't stop to say something to
this. :-(
> free(part);
> +
> + return 0;
> }
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 402e497..347443b 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -217,6 +217,9 @@ struct mtd_info {
> /* If true erasing bad blocks is allowed, this is set via a device parameter */
> bool allow_erasebad;
> int p_allow_erasebad;
> +
> + struct mtd_info *master;
> + uint64_t master_offset;
Currently we have only u_int32_t like for total mtd size. Do you
plan a support for mtd devices which a greater than 4GB? :-)
- Alex
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
