[U-Boot] [PATCH v3 7/9] dfu:mmc: raw data write fix
Jaehoon Chung
jh80.chung at samsung.com
Fri Jan 10 06:03:54 CET 2014
Hi, Mateusz,
This patch should be separated with dfu and mmc.
On 01/09/2014 11:31 PM, Mateusz Zalega wrote:
> When user attempted to perform a raw write using DFU (vide
> dfu_fill_entity_mmc) with MMC interface not initialized before,
> get_mmc_blk_size() reported invalid (zero) block size - it wasn't
> possible to write ie. a new u-boot image.
>
> This commit fixes that by initializing device in get_mmc_blk_size() when
> needed.
>
> Tested on Samsung Goni.
>
> v2 changes:
> - code cleanup
> - minor dfu_alt_info format change
>
> v3 changes:
> - moved invalid block length check to mmc core
> - removed redundant 'has_init' check
>
> Change-Id: Icb50bb9f805a9a78848acd19f682fad474cb9082
> Signed-off-by: Mateusz Zalega <m.zalega at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Reviewed-by: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Minkyu Kang <mk7.kang at samsung.com>
> ---
> drivers/dfu/dfu_mmc.c | 106 ++++++++++++++++++++++++++-----------------
> drivers/mmc/mmc.c | 13 ++++--
> include/configs/am335x_evm.h | 8 ++--
> include/configs/trats.h | 2 +-
> include/configs/trats2.h | 2 +-
> include/dfu.h | 5 --
> 6 files changed, 80 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index f942758..075e4cd 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -168,66 +168,88 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
> return ret;
> }
>
> +/*
> + * @param s Parameter string containing space-separated arguments:
> + * 1st:
> + * raw (raw read/write)
> + * fat (files)
> + * ext4 (^)
> + * part (partition image)
> + * 2nd and 3rd:
> + * lba_start and lba_size, for raw write
> + * mmc_dev and mmc_part, for filesystems and part
> + */
> 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;
> - st = strsep(&s, " ");
> - if (!strcmp(st, "mmc")) {
> - dfu->layout = DFU_RAW_ADDR;
> - dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16);
> - dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16);
> - dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num);
> - } else if (!strcmp(st, "fat")) {
> - dfu->layout = DFU_FS_FAT;
> - } else if (!strcmp(st, "ext4")) {
> - dfu->layout = DFU_FS_EXT4;
> - } else if (!strcmp(st, "part")) {
> + const char *argv[3];
> + const char **parg = argv;
> + for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
> + *parg = strsep(&s, " ");
> + if (*parg == NULL) {
> + error("Invalid number of arguments.\n");
> + return -ENODEV;
> + }
> + }
>
> - dfu->layout = DFU_RAW_ADDR;
> + const char *entity_type = argv[0];
> + /*
> + * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8,
> + * with default 10.
> + */
> + size_t second_arg = simple_strtoul(argv[1], NULL, 0);
> + size_t third_arg = simple_strtoul(argv[2], NULL, 0);
>
> - dev = simple_strtoul(s, &s, 10);
> - s++;
> - part = simple_strtoul(s, &s, 10);
> + struct mmc *mmc = find_mmc_device(dfu->dev_num);
> + if (mmc == NULL) {
> + error("Couldn't find MMC device no. %d.\n", dfu->dev_num);
> + return -ENODEV;
> + }
>
> - mmc = find_mmc_device(dev);
> - if (mmc == NULL || mmc_init(mmc)) {
> - printf("%s: could not find mmc device #%d!\n",
> - __func__, dev);
> - return -ENODEV;
> - }
> + if (mmc_init(mmc)) {
> + error("Couldn't init MMC device.\n");
> + return -ENODEV;
> + }
>
> - 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);
> + if (!strcmp(entity_type, "raw")) {
> + dfu->layout = DFU_RAW_ADDR;
> + dfu->data.mmc.lba_start = second_arg;
> + dfu->data.mmc.lba_size = third_arg;
> + dfu->data.mmc.lba_blk_size = mmc->read_bl_len;
> + } else if (!strcmp(entity_type, "part")) {
> + disk_partition_t partinfo;
> + block_dev_desc_t *blk_dev = &mmc->block_dev;
> + int mmcdev = second_arg;
> + int mmcpart = third_arg;
> +
> + if (get_partition_info(blk_dev, mmcpart, &partinfo) != 0) {
> + error("Couldn't find part #%d on mmc device #%d\n",
> + mmcpart, mmcdev);
> return -ENODEV;
> }
>
> - dfu->data.mmc.lba_start = partinfo.start;
> - dfu->data.mmc.lba_size = partinfo.size;
> - dfu->data.mmc.lba_blk_size = partinfo.blksz;
> -
> + dfu->layout = DFU_RAW_ADDR;
> + dfu->data.mmc.lba_start = partinfo.start;
> + dfu->data.mmc.lba_size = partinfo.size;
> + dfu->data.mmc.lba_blk_size = partinfo.blksz;
> + } else if (!strcmp(entity_type, "fat")) {
> + dfu->layout = DFU_FS_FAT;
> + } else if (!strcmp(entity_type, "ext4")) {
> + dfu->layout = DFU_FS_EXT4;
> } else {
> - printf("%s: Memory layout (%s) not supported!\n", __func__, st);
> + error("Memory layout (%s) not supported!\n", entity_type);
> return -ENODEV;
> }
>
> - if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
> - dfu->data.mmc.dev = simple_strtoul(s, &s, 10);
> - dfu->data.mmc.part = simple_strtoul(++s, &s, 10);
> + /* if it's NOT a raw write */
> + if (strcmp(entity_type, "raw")) {
> + dfu->data.mmc.dev = second_arg;
> + dfu->data.mmc.part = third_arg;
> }
>
> + dfu->dev_type = DFU_DEV_MMC;
> dfu->read_medium = dfu_read_medium_mmc;
> dfu->write_medium = dfu_write_medium_mmc;
> dfu->flush_medium = dfu_flush_medium_mmc;
> -
> - /* initial state */
> dfu->inited = 0;
>
> return 0;
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index e1461a9..f2fa230 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -15,6 +15,7 @@
> #include <malloc.h>
> #include <linux/list.h>
> #include <div64.h>
> +#include <errno.h>
> #include "mmc_private.h"
>
> /* Set block count limit because of 16 bit register limit on some hardware*/
> @@ -1266,17 +1267,23 @@ static int mmc_complete_init(struct mmc *mmc)
>
> int mmc_init(struct mmc *mmc)
> {
> + if (mmc->has_init)
> + return 0;
> +
What difference before?
> int err = IN_PROGRESS;
> unsigned start = get_timer(0);
>
> - if (mmc->has_init)
> - return 0;
> if (!mmc->init_in_progress)
> err = mmc_start_init(mmc);
> -
Need not to change.
> if (!err || err == IN_PROGRESS)
> err = mmc_complete_init(mmc);
> +
Ditto.
> debug("%s: %d, time %lu\n", __func__, err, get_timer(start));
> +
> + if (!mmc->read_bl_len || !mmc->write_bl_len) {
> + error("invalid block length\n");
> + return -ENODEV;
> + }
I know mmc->read_bl_len and write_bl_len is set at init time.
Why this condition needs?
> return err;
> }
>
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index 8af4d6a..d76962f 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -312,10 +312,10 @@
> "boot part 0 1;" \
> "rootfs part 0 2;" \
> "MLO fat 0 1;" \
> - "MLO.raw mmc 100 100;" \
> - "u-boot.img.raw mmc 300 400;" \
> - "spl-os-args.raw mmc 80 80;" \
> - "spl-os-image.raw mmc 900 2000;" \
> + "MLO.raw mmc 0x100 0x100;" \
> + "u-boot.img.raw mmc 0x300 0x400;" \
> + "spl-os-args.raw mmc 0x80 0x80;" \
> + "spl-os-image.raw mmc 0x900 0x2000;" \
> "spl-os-args fat 0 1;" \
> "spl-os-image fat 0 1;" \
> "u-boot.img fat 0 1;" \
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index 6cd15c2..ed3b278 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -140,7 +140,7 @@
> "name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>
> #define CONFIG_DFU_ALT \
> - "u-boot mmc 80 400;" \
> + "u-boot raw 0x80 0x400;" \
> "uImage ext4 0 2;" \
> "exynos4210-trats.dtb ext4 0 2;" \
> ""PARTS_ROOT" part 0 5\0"
> diff --git a/include/configs/trats2.h b/include/configs/trats2.h
> index 5d86a3d..a22be63 100644
> --- a/include/configs/trats2.h
> +++ b/include/configs/trats2.h
> @@ -169,7 +169,7 @@
> "name="PARTS_UMS",size=-,uuid=${uuid_gpt_"PARTS_UMS"}\0" \
>
> #define CONFIG_DFU_ALT \
> - "u-boot mmc 80 800;" \
> + "u-boot mmc 0x80 0x800;" \
u-boot mmc? u-boot raw? what's correct?
Best Regards,
Jaehoon Chung
> "uImage ext4 0 2;" \
> "exynos4412-trats2.dtb ext4 0 2;" \
> ""PARTS_ROOT" part 0 5\0"
> diff --git a/include/dfu.h b/include/dfu.h
> index f973426..f2e83db 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -64,11 +64,6 @@ struct ram_internal_data {
> unsigned int size;
> };
>
> -static inline unsigned int get_mmc_blk_size(int dev)
> -{
> - return find_mmc_device(dev)->read_bl_len;
> -}
> -
> #define DFU_NAME_SIZE 32
> #define DFU_CMD_BUF_SIZE 128
> #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE
>
More information about the U-Boot
mailing list