Thanks Gwendal!
We will test your changes.

Are you still experiencing the "request_firmware" failure?

Alex

> -----Original Message-----
> From: Gwendal Grignou [mailto:[email protected]]
> Sent: Thursday, July 10, 2014 4:08 PM
> To: Avi Shchislowski; [email protected]; [email protected]
> Cc: [email protected]; Alex Lemberg; [email protected];
> [email protected]; [email protected]; Yaniv Iarovici; Gwendal Grignou
> Subject: [PATCH] Fix on top of sandisk patches.
> 
> To apply on top of
> [RFC PATCH 1/1 v7]mmc: Support-FFU-for-eMMC-v5.0
> 
> Signed-off-by: Gwendal Grignou <[email protected]>
> ---
> 
> Avi,
> 
> As I mentioned earlier, I found several problems with your patch.
> Here is a patch that allows to go further.
> It is still not working reliably. The eMMC I am using has a problem being
> reseted after upgrade, and I am still hitting what appears to be memory
> corruptions. I believe the issues I am still having are due to the kernel 
> code,
> not the eMMC I am testing.
> 
> Gwendal.
> 
>  drivers/mmc/card/block.c |   3 +-
>  drivers/mmc/card/ffu.c   | 121 +++++++++++++++++++++++--------------------
> ----
>  drivers/mmc/core/mmc.c   |   8 ++++
>  include/linux/mmc/card.h |   1 +
>  include/linux/mmc/ffu.h  |  13 +----
>  5 files changed, 71 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index
> fc65a6c..9d866fb 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -504,8 +504,7 @@ static int mmc_blk_ioctl_cmd(struct block_device
> *bdev,
>       mmc_get_card(card);
> 
>       if (cmd.opcode == MMC_FFU_DOWNLOAD_OP) {
> -             err = mmc_ffu_download(card, &cmd , idata->buf,
> -                     idata->buf_bytes);
> +             err = mmc_ffu_download(card, idata->buf);
>               goto cmd_rel_host;
>       }
> 
> diff --git a/drivers/mmc/card/ffu.c b/drivers/mmc/card/ffu.c index
> 04e07a5..0e85b2b 100644
> --- a/drivers/mmc/card/ffu.c
> +++ b/drivers/mmc/card/ffu.c
> @@ -63,15 +63,13 @@ struct mmc_ffu_area {
> 
>  static void mmc_ffu_prepare_mrq(struct mmc_card *card,
>       struct mmc_request *mrq, struct scatterlist *sg, unsigned int sg_len,
> -     u32 arg, unsigned int blocks, unsigned int blksz, int write) {
> +     u32 arg, unsigned int blocks, unsigned int blksz) {
>       BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
> 
>       if (blocks > 1) {
> -             mrq->cmd->opcode = write ?
> -                     MMC_WRITE_MULTIPLE_BLOCK :
> MMC_READ_MULTIPLE_BLOCK;
> +             mrq->cmd->opcode = MMC_WRITE_MULTIPLE_BLOCK;
>       } else {
> -             mrq->cmd->opcode = write ? MMC_WRITE_BLOCK :
> -                     MMC_READ_SINGLE_BLOCK;
> +             mrq->cmd->opcode = MMC_WRITE_BLOCK;
>       }
> 
>       mrq->cmd->arg = arg;
> @@ -89,11 +87,12 @@ static void mmc_ffu_prepare_mrq(struct mmc_card
> *card,
> 
>       mrq->data->blksz = blksz;
>       mrq->data->blocks = blocks;
> -     mrq->data->flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
> +     mrq->data->flags = MMC_DATA_WRITE;
>       mrq->data->sg = sg;
>       mrq->data->sg_len = sg_len;
> 
> -     mmc_set_data_timeout(mrq->data, card); }
> +     mmc_set_data_timeout(mrq->data, card); }
> 
>  /*
>   * Checks that a normal transfer didn't have any errors @@ -153,7 +152,7
> @@ static int mmc_ffu_wait_busy(struct mmc_card *card) {
>   */
>  static int mmc_ffu_simple_transfer(struct mmc_card *card,
>       struct scatterlist *sg, unsigned int sg_len, u32 arg,
> -     unsigned int blocks, unsigned int blksz, int write) {
> +     unsigned int blocks, unsigned int blksz) {
>       struct mmc_request mrq = {0};
>       struct mmc_command cmd = {0};
>       struct mmc_command stop = {0};
> @@ -162,8 +161,7 @@ static int mmc_ffu_simple_transfer(struct mmc_card
> *card,
>       mrq.cmd = &cmd;
>       mrq.data = &data;
>       mrq.stop = &stop;
> -     mmc_ffu_prepare_mrq(card, &mrq, sg, sg_len, arg, blocks, blksz,
> -             write);
> +     mmc_ffu_prepare_mrq(card, &mrq, sg, sg_len, arg, blocks, blksz);
>       mmc_wait_for_req(card->host, &mrq);
> 
>       mmc_ffu_wait_busy(card);
> @@ -175,19 +173,17 @@ static int mmc_ffu_simple_transfer(struct
> mmc_card *card,
>   * Map memory into a scatterlist.
>   */
>  static unsigned int mmc_ffu_map_sg(struct mmc_ffu_mem *mem, int size,
> -     struct scatterlist *sglist, unsigned int max_segs,
> -     unsigned int max_seg_sz)
> +     struct scatterlist *sglist)
>  {
>       struct scatterlist *sg = sglist;
>       unsigned int i;
>       unsigned long sz = size;
> -     unsigned int sctr_len = 0;
>       unsigned long len;
> 
> -     sg_init_table(sglist, max_segs);
> +     sg_init_table(sglist, mem->cnt);
> 
> -     for (i = 0; i < mem->cnt && sz; i++, sz -= len) {
> -             len = PAGE_SIZE * (1 << mem->arr[i].order);
> +     for (i = 0; i < mem->cnt && sz; i++, sg++, sz -= len) {
> +             len = PAGE_SIZE << mem->arr[i].order;
> 
>               if (len > sz) {
>                       len = sz;
> @@ -195,12 +191,10 @@ static unsigned int mmc_ffu_map_sg(struct
> mmc_ffu_mem *mem, int size,
>               }
> 
>               sg_set_page(sg, mem->arr[i].page, len, 0);
> -             sg = sg_next(sg);
> -             sctr_len += 1;
>       }
>       sg_mark_end(sg);
> 
> -     return sctr_len;
> +     return mem->cnt;
>  }
> 
>  static void mmc_ffu_free_mem(struct mmc_ffu_mem *mem) { @@ -277,6
> +271,8 @@ static struct mmc_ffu_mem *mmc_ffu_alloc_mem(unsigned
> long min_sz,
>               mem->arr[mem->cnt].page = page;
>               mem->arr[mem->cnt].order = order;
>               mem->cnt += 1;
> +             pr_debug("FFU: cnt: %d - order %d\n", mem->cnt, order);
> +
>               if (max_page_cnt <= (1UL << order))
>                       break;
>               max_page_cnt -= 1UL << order;
> @@ -298,11 +294,11 @@ out_free:
>   * Copy the data to the allocated pages.
>   */
>  static int mmc_ffu_area_init(struct mmc_ffu_area *area, struct mmc_card
> *card,
> -     u8 *data, int size)
> +     const u8 *data, int size)
>  {
>       int ret;
>       int i;
> -     int length = 0;
> +     int length = 0, page_length;
> 
>       area->max_tfr = size;
> 
> @@ -323,20 +319,20 @@ static int mmc_ffu_area_init(struct mmc_ffu_area
> *area, struct mmc_card *card,
>                       goto out_free;
>               }
> 
> +             page_length = PAGE_SIZE << area->mem->arr[i].order;
>               memcpy(page_address(area->mem->arr[i].page), data +
> length,
> -                     min(size - length, (int)area->max_seg_sz));
> -             length += area->max_seg_sz;
> +                     min(size - length, page_length));
> +             length += page_length;
>       }
> 
>       area->sg = kmalloc(sizeof(struct scatterlist) * area->mem->cnt,
> -             GFP_KERNEL);
> +                        GFP_KERNEL);
>       if (!area->sg) {
>               ret = -ENOMEM;
>               goto out_free;
>       }
> 
> -     area->sg_len = mmc_ffu_map_sg(area->mem, size, area->sg,
> -             area->max_segs, area->mem->cnt);
> +     area->sg_len = mmc_ffu_map_sg(area->mem, size, area->sg);
> 
>       return 0;
> 
> @@ -345,7 +341,7 @@ out_free:
>       return ret;
>  }
> 
> -static int mmc_ffu_write(struct mmc_card *card, u8 *src, u32 arg,
> +static int mmc_ffu_write(struct mmc_card *card, const u8 *src, u32 arg,
>       int size)
>  {
>       int rc;
> @@ -370,7 +366,8 @@ static int mmc_ffu_write(struct mmc_card *card, u8
> *src, u32 arg,
>                       goto exit;
> 
>               rc = mmc_ffu_simple_transfer(card, area.sg, area.sg_len, arg,
> -                     max_tfr / CARD_BLOCK_SIZE, CARD_BLOCK_SIZE, 1);
> +                     max_tfr / CARD_BLOCK_SIZE, CARD_BLOCK_SIZE);
> +             mmc_ffu_area_cleanup(&area);
>               if (rc != 0)
>                       goto exit;
> 
> @@ -379,7 +376,6 @@ static int mmc_ffu_write(struct mmc_card *card, u8
> *src, u32 arg,
>       } while (size > 0);
> 
>  exit:
> -     mmc_ffu_area_cleanup(&area);
>       return rc;
>  }
> 
> @@ -406,47 +402,39 @@ exit:
>       return err;
>  }
> 
> -int mmc_ffu_download(struct mmc_card *card, struct mmc_command
> *cmd,
> -     u8 *data, int buf_bytes)
> +int mmc_ffu_download(struct mmc_card *card, const char *name)
>  {
>       u8 ext_csd[CARD_BLOCK_SIZE];
>       int err;
>       int ret;
> -     u8 *buf = NULL;
> +     u32 arg;
>       const struct firmware *fw;
> 
> -     /* Read the EXT_CSD */
> -     err = mmc_send_ext_csd(card, ext_csd);
> -     if (err) {
> -             pr_err("FFU: %s: error %d sending ext_csd\n",
> -                     mmc_hostname(card->host), err);
> +     pr_debug("FFU: %s uploading firmware %.20s to device\n",
> +              mmc_hostname(card->host), name);
> +
> +     if (strlen(name) > 512) {
> +             err = -EINVAL;
> +             pr_err("FFU: %s: %.20s is not a valid argument\n",
> +                    mmc_hostname(card->host), name);
>               goto exit;
>       }
> 
> -     /* check if card is eMMC 5.0 or higher */
> -     if (card->ext_csd.rev < 7)
> -             return -EINVAL;
> -
>       /* Check if FFU is supported */
> -     if (!FFU_SUPPORTED_MODE(ext_csd[EXT_CSD_SUPPORTED_MODE])
> ||
> -             FFU_CONFIG(ext_csd[EXT_CSD_FW_CONFIG])) {
> -             err = -EINVAL;
> +     if (!card->ext_csd.ffu_capable) {
> +             err = -EOPNOTSUPP;
>               pr_err("FFU: %s: error %d FFU is not supported\n",
>                       mmc_hostname(card->host), err);
>               goto exit;
>       }
> 
> -     /* setup FW data buffer */
> -     err = request_firmware(&fw, data, &card->dev);
> +     /* setup FW name buffer */
> +     mmc_put_card(card);
> +     err = request_firmware(&fw, name, &card->dev);
> +     mmc_get_card(card);
>       if (err) {
>               pr_err("Firmware request failed %d\n", err);
> -             goto exit_normal;
> -     }
> -
> -     buf = kmalloc(fw->size, GFP_KERNEL);
> -     if (buf == NULL) {
> -             pr_err("Allocating memory for firmware failed!\n");
> -             goto exit_normal;
> +             goto exit;
>       }
> 
>       if ((fw->size % CARD_BLOCK_SIZE)) {
> @@ -454,9 +442,16 @@ int mmc_ffu_download(struct mmc_card *card,
> struct mmc_command *cmd,
>                       mmc_hostname(card->host), fw->size);
>       }
> 
> -     memcpy(buf, fw->data, fw->size);
> +     /* Read the EXT_CSD */
> +     err = mmc_send_ext_csd(card, ext_csd);
> +     if (err) {
> +             pr_err("FFU: %s: error %d sending ext_csd\n",
> +                     mmc_hostname(card->host), err);
> +             goto exit;
> +     }
> 
>       /* set device to FFU mode */
> +     pr_debug("FFU: %s switch to FFU mode\n", mmc_hostname(card-
> >host));
>       err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_MODE_CONFIG,
>               MMC_FFU_MODE_SET, card->ext_csd.generic_cmd6_time);
>       if (err) {
> @@ -466,18 +461,18 @@ int mmc_ffu_download(struct mmc_card *card,
> struct mmc_command *cmd,
>       }
> 
>       /* set CMD ARG */
> -     cmd->arg = ext_csd[EXT_CSD_FFU_ARG] |
> +     arg = ext_csd[EXT_CSD_FFU_ARG] |
>               ext_csd[EXT_CSD_FFU_ARG + 1] << 8 |
>               ext_csd[EXT_CSD_FFU_ARG + 2] << 16 |
>               ext_csd[EXT_CSD_FFU_ARG + 3] << 24;
> 
> -     err = mmc_ffu_write(card, buf, cmd->arg, (int)fw->size);
> +     err = mmc_ffu_write(card, fw->data, arg, (int)fw->size);
> 
>  exit_normal:
>       release_firmware(fw);
> -     kfree(buf);
> 
>       /* host switch back to work in normal MMC Read/Write commands
> */
> +     pr_debug("FFU: %s switch to normal mode\n", mmc_hostname(card-
> >host));
>       ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>               EXT_CSD_MODE_CONFIG, MMC_FFU_MODE_NORMAL,
>               card->ext_csd.generic_cmd6_time);
> @@ -495,6 +490,8 @@ int mmc_ffu_install(struct mmc_card *card)
>       u32 ffu_data_len;
>       u32 timeout;
> 
> +     pr_debug("FFU: %s installing firmware to device\n",
> +              mmc_hostname(card->host));
>       err = mmc_send_ext_csd(card, ext_csd);
>       if (err) {
>               pr_err("FFU: %s: error %d sending ext_csd\n", @@ -503,9
> +500,8 @@ int mmc_ffu_install(struct mmc_card *card)
>       }
> 
>       /* Check if FFU is supported */
> -     if (!FFU_SUPPORTED_MODE(ext_csd[EXT_CSD_SUPPORTED_MODE])
> ||
> -             FFU_CONFIG(ext_csd[EXT_CSD_FW_CONFIG])) {
> -             err = -EINVAL;
> +     if (!card->ext_csd.ffu_capable) {
> +             err = -EOPNOTSUPP;
>               pr_err("FFU: %s: error %d FFU is not supported\n",
>                       mmc_hostname(card->host), err);
>               goto exit;
> @@ -514,19 +510,20 @@ int mmc_ffu_install(struct mmc_card *card)
>       /* check mode operation */
>       if (!FFU_FEATURES(ext_csd[EXT_CSD_FFU_FEATURES])) {
>               /* restart the eMMC */
> +             mmc_put_card(card);
>               err = mmc_ffu_restart(card);
> +             mmc_get_card(card);
>               if (err) {
>                       pr_err("FFU: %s: error %d FFU install:\n",
>                               mmc_hostname(card->host), err);
>               }
>       } else {
> -
>               ffu_data_len =
> ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG]|
>                       ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 1] <<
> 8 |
>                       ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 2] <<
> 16 |
>                       ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG + 3] <<
> 24;
> 
> -             if (!ffu_data_len) {
> +             if (ffu_data_len == 0) {
>                       err = -EPERM;
>                       return err;
>               }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> 4099424..ace9123 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -552,6 +552,14 @@ static int mmc_read_ext_csd(struct mmc_card
> *card, u8 *ext_csd)
>               card->ext_csd.data_sector_size = 512;
>       }
> 
> +     /* eMMC v5 or later */
> +     if (card->ext_csd.rev >= 7) {
> +             card->ext_csd.ffu_capable =
> +                     ((ext_csd[EXT_CSD_SUPPORTED_MODE] & 1) == 1)
> &&
> +                     ((ext_csd[EXT_CSD_FW_CONFIG] & 1) == 0);
> +     } else {
> +             card->ext_csd.ffu_capable = false;
> +     }
>  out:
>       return err;
>  }
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> 6a5c754..6a7de8d 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -87,6 +87,7 @@ struct mmc_ext_csd {
>       unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
>       unsigned int            boot_ro_lock;           /* ro lock support */
>       bool                    boot_ro_lockable;
> +     bool                    ffu_capable;  /* support Firmware Field
> Upgrade */
>       u8                      raw_exception_status;   /* 54 */
>       u8                      raw_partition_support;  /* 160 */
>       u8                      raw_rpmb_size_mult;     /* 168 */
> diff --git a/include/linux/mmc/ffu.h b/include/linux/mmc/ffu.h index
> f5dcedb..a0ade1b 100644
> --- a/include/linux/mmc/ffu.h
> +++ b/include/linux/mmc/ffu.h
> @@ -34,19 +34,10 @@
>  #define MMC_FFU_INSTALL_SET 0x1
> 
>  #ifdef CONFIG_MMC_FFU
> -#define MMC_FFU_ENABLE 0x0
> -#define MMC_FFU_CONFIG 0x1
> -#define MMC_FFU_SUPPORTED_MODES 0x1
>  #define MMC_FFU_FEATURES 0x1
> +#define FFU_FEATURES(ffu_features) (ffu_features & MMC_FFU_FEATURES)
> 
> -#define FFU_ENABLED(ffu_enable)      (ffu_enable & MMC_FFU_CONFIG)
> -#define FFU_SUPPORTED_MODE(ffu_sup_mode) \
> -     (ffu_sup_mode && MMC_FFU_SUPPORTED_MODES)
> -#define FFU_CONFIG(ffu_config) (ffu_config & MMC_FFU_CONFIG) -
> #define FFU_FEATURES(ffu_fetures) (ffu_fetures & MMC_FFU_FEATURES)
> -
> -int mmc_ffu_download(struct mmc_card *card, struct mmc_command
> *cmd,
> -     u8 *data, int buf_bytes);
> +int mmc_ffu_download(struct mmc_card *card, const char *name);
>  int mmc_ffu_install(struct mmc_card *card);  #else  static inline int
> mmc_ffu_download(struct mmc_card *card,
> --
> 2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to