[U-Boot] [PATCH V5 07/10] MMC: APIs to support resize of EMMC boot partition

Amarendra Reddy amar.lavanuru at gmail.com
Mon Jan 28 10:34:03 CET 2013


Hi Simon,

Please find the responses below.

Thanks & Regards
Amarendra Reddy

On 27 January 2013 01:55, Simon Glass <sjg at chromium.org> wrote:

> Hi Amar,
>
> On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra.xt at samsung.com> wrote:
> > This patch adds APIs to access(open / close) and to resize boot partiton
> of EMMC.
> >
> > Signed-off-by: Amar <amarendra.xt at samsung.com>
> > ---
> > Changes since V1:
> >         New patch.
> >
> > Changes since V2:
> >         1)Updation of commit message and resubmition of proper patch set.
> >
> > Changes since V3:
> >         No change.
> >
> > Changes since V4:
> >         1)Replaced the functions mmc_boot_open() & mmc_boot_close() with
> a
> >         single function mmc_boot_part_access().
> >
> >  drivers/mmc/mmc.c | 117
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/mmc.h     |  24 +++++++++++
> >  2 files changed, 141 insertions(+)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index 72e8ce6..c152934 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -1327,3 +1327,120 @@ int mmc_initialize(bd_t *bis)
> >
> >         return 0;
> >  }
> > +
> > +int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long
> bootsize,
> > +                               unsigned long rpmbsize)
> > +{
> > +       int err;
> > +       struct mmc_cmd cmd;
> > +
> > +       /* Only use this command for raw EMMC moviNAND */
> > +       /* Enter backdoor mode */
>
> Should probably join these two comments.
> Ok.
> > +       cmd.cmdidx = MMC_CMD_RES_MAN;
> > +       cmd.resp_type = MMC_RSP_R1b;
> > +       cmd.cmdarg = MMC_CMD62_ARG1;
> > +
> > +       err = mmc_send_cmd(mmc, &cmd, NULL);
> > +       if (err) {
> > +               debug("mmc_boot_partition_size_change: Error1 = %d\n",
> err);
> > +               return err;
> > +       }
> > +
> > +       /* Boot partition changing mode */
> > +       cmd.cmdidx = MMC_CMD_RES_MAN;
> > +       cmd.resp_type = MMC_RSP_R1b;
> > +       cmd.cmdarg = MMC_CMD62_ARG2;
> > +
> > +       err = mmc_send_cmd(mmc, &cmd, NULL);
> > +       if (err) {
> > +               debug("mmc_boot_partition_size_change: Error2 = %d\n",
> err);
> > +               return err;
> > +       }
> > +       /* boot partition size is multiple of 128KB */
> > +       bootsize = ((bootsize*1024)/128);
>
> space around * and /
> plus remove outer ()
>
Ok.

>
> > +
> > +       /* Arg: boot partition size */
> > +       cmd.cmdidx = MMC_CMD_RES_MAN;
> > +       cmd.resp_type = MMC_RSP_R1b;
> > +       cmd.cmdarg = bootsize;
> > +
> > +       err = mmc_send_cmd(mmc, &cmd, NULL);
> > +       if (err) {
> > +               debug("mmc_boot_partition_size_change: Error3 = %d\n",
> err);
> > +               return err;
> > +       }
> > +       /* RPMB partition size is multiple of 128KB */
> > +       rpmbsize = ((rpmbsize*1024)/128);
>
> and here
>
Ok.

>
> > +       /* Arg: RPMB partition size */
> > +       cmd.cmdidx = MMC_CMD_RES_MAN;
> > +       cmd.resp_type = MMC_RSP_R1b;
> > +       cmd.cmdarg = rpmbsize;
> > +
> > +       err = mmc_send_cmd(mmc, &cmd, NULL);
> > +       if (err) {
> > +               debug("mmc_boot_partition_size_change: Error4 = %d\n",
> err);
> > +               return err;
> > +       }
> > +       return 0;
> > +}
> > +
> > +/* This function shall form and send the commands to open / close the
>
> /*
>  * This function...
>
> Ok.

>  > + * boot partition specified by user.
> > + *
> > + * ack: 0x0 - No boot acknowledge sent (default)
> > + *     0x1 - Boot acknowledge sent during boot operation
> > + * part_num: User selects boot data that will be sent to master
> > + *     0x0 - Device not boot enabled (default)
> > + *     0x1 - Boot partition 1 enabled for boot
> > + *     0x2 - Boot partition 2 enabled for boot
> > + * access: User selects partitions to access
> > + *     0x0 : No access to boot partition (default)
> > + *     0x1 : R/W boot partition 1
> > + *     0x2 : R/W boot partition 2
> > + *     0x3 : R/W Replay Protected Memory Block (RPMB)
> > + */
> > +int mmc_boot_part_access(struct mmc *mmc, u32 ack, u32 part_num, u32
> access)
> > +{
> > +       int err;
> > +       struct mmc_cmd cmd;
> > +
> > +       /* Boot ack enable, boot partition enable , boot partition
> access */
> > +       cmd.cmdidx = MMC_CMD_SWITCH;
> > +       cmd.resp_type = MMC_RSP_R1b;
> > +
> > +       cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24 |
> > +                       EXT_CSD_PART_CONF << 16 |
> > +                       (EXT_CSD_BOOT_ACK(ack) |
> > +                       EXT_CSD_BOOT_PART_NUM(part_num) |
> > +                       EXT_CSD_PARTITION_ACCESS(access)) << 8);
>
> You can remove outer (). Might be nice to put () around the <<
> sub-expressions:
>
>        cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
>                        (EXT_CSD_PART_CONF << 16) |
>
> Ok.

> > +
> > +       err = mmc_send_cmd(mmc, &cmd, NULL);
> > +       if (err) {
> > +               if (access) {
> > +                       debug("mmc boot partition#%d open failure:"
> > +                                       "Error1 = %d\n", part_num, err);
> > +               } else {
> > +                       debug("mmc boot partition#%d close failure:"
> > +                                       "Error = %d\n", part_num, err);
> > +               }
> > +               return err;
> > +       }
> > +
> > +       if (access) {
> > +               /* 4bit transfer mode at booting time. */
> > +               cmd.cmdidx = MMC_CMD_SWITCH;
> > +               cmd.resp_type = MMC_RSP_R1b;
> > +
> > +               cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24|
>
> Same here. Also space before |, and below
>
Ok.

>
> > +                               EXT_CSD_BOOT_BUS_WIDTH << 16|
> > +                               ((1<<0) << 8));
> > +
> > +               err = mmc_send_cmd(mmc, &cmd, NULL);
> > +               if (err) {
> > +                       debug("mmc boot partition#%d open failure:"
> > +                                       "Error2 = %d\n", part_num, err);
> > +                       return err;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > diff --git a/include/mmc.h b/include/mmc.h
> > index a13e2bd..fcd9c54 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -86,6 +86,11 @@
> >  #define MMC_CMD_APP_CMD                        55
> >  #define MMC_CMD_SPI_READ_OCR           58
> >  #define MMC_CMD_SPI_CRC_ON_OFF         59
> > +#define MMC_CMD_RES_MAN                        62
> > +
> > +#define MMC_CMD62_ARG1                 0xefac62ec
> > +#define MMC_CMD62_ARG2                 0xcbaea7
> > +
> >
> >  #define SD_CMD_SEND_RELATIVE_ADDR      3
> >  #define SD_CMD_SWITCH_FUNC             6
> > @@ -153,6 +158,7 @@
> >   */
> >  #define EXT_CSD_PARTITIONING_SUPPORT   160     /* RO */
> >  #define EXT_CSD_ERASE_GROUP_DEF                175     /* R/W */
> > +#define EXT_CSD_BOOT_BUS_WIDTH         177
> >  #define EXT_CSD_PART_CONF              179     /* R/W */
> >  #define EXT_CSD_BUS_WIDTH              183     /* R/W */
> >  #define EXT_CSD_HS_TIMING              185     /* R/W */
> > @@ -177,6 +183,16 @@
> >  #define EXT_CSD_BUS_WIDTH_4    1       /* Card is in 4 bit mode */
> >  #define EXT_CSD_BUS_WIDTH_8    2       /* Card is in 8 bit mode */
> >
> > +#define EXT_CSD_BOOT_ACK_ENABLE                        (1<<6)
> > +#define EXT_CSD_BOOT_PARTITION_ENABLE          (1<<3)
> > +#define EXT_CSD_PARTITION_ACCESS_ENABLE                (1<<0)
> > +#define EXT_CSD_PARTITION_ACCESS_DISABLE       (0<<0)
> > +
> > +#define EXT_CSD_BOOT_ACK(x)            (x<<6)
> > +#define EXT_CSD_BOOT_PART_NUM(x)       (x<<3)
> > +#define EXT_CSD_PARTITION_ACCESS(x)    (x<<0)
> > +
> > +
> >  #define R1_ILLEGAL_COMMAND             (1 << 22)
> >  #define R1_APP_CMD                     (1 << 5)
> >
> > @@ -201,6 +217,11 @@
> >  #define PART_ACCESS_MASK       (0x7)
> >  #define PART_SUPPORT           (0x1)
> >
> > +/* The number of MMC physical partitions.  These consist of:
> > + * boot partitions (2), general purpose partitions (4) in MMC v4.4.
> > + */
> > +#define MMC_NUM_BOOT_PARTITION 2
> > +
> >  struct mmc_cid {
> >         unsigned long psn;
> >         unsigned short oid;
> > @@ -275,6 +296,9 @@ int board_mmc_getcd(struct mmc *mmc);
> >  int mmc_switch_part(int dev_num, unsigned int part_num);
> >  int mmc_getcd(struct mmc *mmc);
> >  void spl_mmc_load(void) __noreturn;
>
> Could you please add full comments for these functions?
>
Ok.

>
> > +int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long
> bootsize,
> > +                                       unsigned long rpmbsize);
> > +int mmc_boot_part_access(struct mmc *mmc, u32 ack, u32 part_num, u32
> access);
> >
> >  #ifdef CONFIG_GENERIC_MMC
> >  #define mmc_host_is_spi(mmc)   ((mmc)->host_caps & MMC_MODE_SPI)
> > --
> > 1.8.0
> >
>
> Regards,
> Simon
>  _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>


More information about the U-Boot mailing list