Hi Simon,
Please find the responses below.
Thanks & Regards
Amarendra Reddy
On 27 January 2013 01:55, Simon Glass <s...@chromium.org> wrote:
> Hi Amar,
>
> On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra...@samsung.com> wrote:
> > This patch adds APIs to access(open / close) and to resize boot partiton
> of EMMC.
> >
> > Signed-off-by: Amar <amarendra...@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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
