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 <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> Reviewed-by: Lukasz Majewski <[email protected]>
> Cc: Minkyu Kang <[email protected]>
> ---
>  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
> 

_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to