[U-Boot] [PATCH 7/9] Add a partition type target

[Marek Vasut]

Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:48 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> Dealing with raw block numbers with the dfu is very annoying.
> >> Introduce a partition method.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto at antoniou-consulting.com>
> >> ---
> >> drivers/dfu/dfu_mmc.c | 29 +++++++++++++++++++++++++++++
> >> 1 file changed, 29 insertions(+)
> >> 
> >> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> >> index 5d504df..3733b21 100644
> >> --- a/drivers/dfu/dfu_mmc.c
> >> +++ b/drivers/dfu/dfu_mmc.c
> >> @@ -153,6 +153,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu,
> >> void *buf, long *len)
> >> 
> >> int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
> >> {
> >> +	int dev, part;
> >> +	struct mmc *mmc;
> >> +	block_dev_desc_t *blk_dev;
> >> +	disk_partition_t partinfo;
> >> 
> >> 	char *st;
> >> 	
> >> 	dfu->dev_type = DFU_DEV_MMC;
> >> 
> >> @@ -166,8 +170,33 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu,
> >> char *s) dfu->layout = DFU_FS_FAT;
> >> 
> >> 	} else if (!strcmp(st, "ext4")) {
> >> 	
> >> 		dfu->layout = DFU_FS_EXT4;
> >> 
> >> +	} else if (!strcmp(st, "part")) {
> >> +
> >> +		dfu->layout = DFU_RAW_ADDR;
> >> +
> >> +		dev = simple_strtoul(s, &s, 10);
> >> +		part = simple_strtoul(++s, &s, 10);
> > 
> > ++s ... this is unreadable and definitelly prone to breakage.
> 
> Just following what the existing code does. Look around this line.
> Perhaps we should modify that too.

Yes, that'd be really nice ... but in a separate patch please.

> >> +
> >> +		mmc = find_mmc_device(dev);
> >> +		if (mmc == NULL || mmc_init(mmc)) {
> >> +			printf("%s: could not find mmc device #%d!\n", __func__,
> > 
> > dev);
> > 
> >> +			return -1;
> >> +		}
> >> +
> >> +		blk_dev = &mmc->block_dev;
> >> +		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
> >> +			printf("%s: could not find partition #%d on mmc device
> > 
> > #%d!\n",
> > 
> >> +					__func__, part, dev);
> >> +			return -1;
> > 
> > Try using regular errno.h ... fix all around.
> 
> The whole file doesn't use errno.h at all. Changing this single thing will
> stick out like a sore thumb.

:-(

> >> +		}
> >> +
> >> +		dfu->data.mmc.lba_start = partinfo.start;
> >> +		dfu->data.mmc.lba_size = partinfo.size;
> >> +		dfu->data.mmc.lba_blk_size = partinfo.blksz;
> >> +
> >> 
> >> 	} else {
> >> 	
> >> 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
> >> 
> >> +		return -1;
> > 
> > This is new ... does it fit into this patch at all?
> 
> It should, it's an error condition and the code just blindly continued.

Even more :-((

> >> 	}
> >> 	
> >> 	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis