Message ID | 1387197646-14395-1-git-send-email-list-09_u-boot@tqsc.de |
---|---|
State | Accepted |
Delegated to: | Pantelis Antoniou |
Headers | show |
Hi Markus, You missed the code comment about the format of the if statement. Regards -- Pantelis On Dec 16, 2013, at 2:40 PM, Markus Niebel wrote: > From: Markus Niebel <Markus.Niebel@tqs.de> > > The eMMC and the SD-Card specifications describe the optional SET_DSR command. > During measurements at our lab we found that some cards implementing this feature > having really strong driver strengts per default. This can lead to voltage peaks > above the specification of the host on signal edges for data sent from a card to > the host. > > Since availability of a given card type may be shorter than the time a certain > hardware will be produced it is useful to have support for this command (Alternative > would be changing termination resistors and adapting the driver strength of the > host to the used card.) > > Following proposal for an implementation: > > - new field that reflects CSD field DSR_IMP in struct mmc > - new field for design specific DSR value in struct mmc > - board code can set DSR value in mmc struct just after registering an controller > - mmc_startup sends the the stored DSR value before selecting a card, if DSR_IMP is set > > Additionally the mmc command is extended to make is possible to play around with different > DSR values. > > The concept was tested on a i.MX53 based platform using a Micron eMMC card where the default > DSR is 0x0400 (12mA) but in our design 0x0100 (0x0100) were enough. To use this feature for > instance on a mx53loco one have to add a call to mmc_set_dsr() in board_mmc_init() after > calling fsl_esdhc_initialize() for the eMMC. > > Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de> > --- > changes for V2: > - extend documentation according to the comments from Pantelis Antoniou > > common/cmd_mmc.c | 23 +++++++++++++++++++++++ > drivers/mmc/mmc.c | 18 ++++++++++++++++++ > include/mmc.h | 3 +++ > 3 files changed, 44 insertions(+) > > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c > index 67a94a7..da5fef9 100644 > --- a/common/cmd_mmc.c > +++ b/common/cmd_mmc.c > @@ -340,6 +340,28 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > } > #endif /* CONFIG_SUPPORT_EMMC_BOOT */ > } > + > + else if (argc == 3 && strcmp(argv[1], "setdsr") == 0) { > + struct mmc *mmc = find_mmc_device(curr_device); > + u32 val = simple_strtoul(argv[2], NULL, 16); > + int ret; > + > + if (!mmc) { > + printf("no mmc device at slot %x\n", curr_device); > + return 1; > + } > + ret = mmc_set_dsr(mmc, val); > + printf("set dsr %s\n", (!ret) ? "OK, force rescan" : "ERROR"); > + if (!ret) { > + mmc->has_init = 0; > + if (mmc_init(mmc)) > + return 1; > + else > + return 0; > + } > + return ret; > + } > + > state = MMC_INVALID; > if (argc == 5 && strcmp(argv[1], "read") == 0) > state = MMC_READ; > @@ -423,5 +445,6 @@ U_BOOT_CMD( > "mmc bootpart <device num> <boot part size MB> <RPMB part size MB>\n" > " - change sizes of boot and RPMB partitions of specified device\n" > #endif > + "mmc setdsr - set DSR register value\n" > ); > #endif /* !CONFIG_GENERIC_MMC */ > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index e1461a9..c6a1c23 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -877,6 +877,7 @@ static int mmc_startup(struct mmc *mmc) > > mmc->tran_speed = freq * mult; > > + mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1); > mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf); > > if (IS_SD(mmc)) > @@ -907,6 +908,14 @@ static int mmc_startup(struct mmc *mmc) > if (mmc->write_bl_len > MMC_MAX_BLOCK_LEN) > mmc->write_bl_len = MMC_MAX_BLOCK_LEN; > > + if ((mmc->dsr_imp) && (0xffffffff != mmc->dsr)) { > + cmd.cmdidx = MMC_CMD_SET_DSR; > + cmd.cmdarg = (mmc->dsr & 0xffff) << 16; > + cmd.resp_type = MMC_RSP_NONE; > + if (mmc_send_cmd(mmc, &cmd, NULL)) > + printf("MMC: SET_DSR failed\n"); > + } > + > /* Select the card, and put it into Transfer Mode */ > if (!mmc_host_is_spi(mmc)) { /* cmd not supported in spi */ > cmd.cmdidx = MMC_CMD_SELECT_CARD; > @@ -1163,6 +1172,9 @@ static int mmc_send_if_cond(struct mmc *mmc) > > int mmc_register(struct mmc *mmc) > { > + /* Setup dsr related values */ > + mmc->dsr_imp = 0; > + mmc->dsr = 0xffffffff; > /* Setup the universal parts of the block interface just once */ > mmc->block_dev.if_type = IF_TYPE_MMC; > mmc->block_dev.dev = cur_dev_num++; > @@ -1280,6 +1292,12 @@ int mmc_init(struct mmc *mmc) > return err; > } > > +int mmc_set_dsr(struct mmc *mmc, u16 val) > +{ > + mmc->dsr = val; > + return 0; > +} > + > /* > * CPU and board-specific MMC initializations. Aliased function > * signals caller to move on > diff --git a/include/mmc.h b/include/mmc.h > index cb558da..facb819 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -262,6 +262,8 @@ struct mmc { > uint card_caps; > uint host_caps; > uint ocr; > + uint dsr; > + uint dsr_imp; > uint scr[2]; > uint csd[4]; > uint cid[4]; > @@ -304,6 +306,7 @@ int board_mmc_getcd(struct mmc *mmc); > int mmc_switch_part(int dev_num, unsigned int part_num); > int mmc_getcd(struct mmc *mmc); > int mmc_getwp(struct mmc *mmc); > +int mmc_set_dsr(struct mmc *mmc, u16 val); > void spl_mmc_load(void) __noreturn; > /* Function to change the size of boot partition and rpmb partitions */ > int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, > -- > 1.7.9.5 >
Hi Markus, On Dec 16, 2013, at 2:40 PM, Markus Niebel wrote: > From: Markus Niebel <Markus.Niebel@tqs.de> > > The eMMC and the SD-Card specifications describe the optional SET_DSR command. > During measurements at our lab we found that some cards implementing this feature > having really strong driver strengts per default. This can lead to voltage peaks > above the specification of the host on signal edges for data sent from a card to > the host. > > Since availability of a given card type may be shorter than the time a certain > hardware will be produced it is useful to have support for this command (Alternative > would be changing termination resistors and adapting the driver strength of the > host to the used card.) > > Following proposal for an implementation: > > - new field that reflects CSD field DSR_IMP in struct mmc > - new field for design specific DSR value in struct mmc > - board code can set DSR value in mmc struct just after registering an controller > - mmc_startup sends the the stored DSR value before selecting a card, if DSR_IMP is set > > Additionally the mmc command is extended to make is possible to play around with different > DSR values. > > The concept was tested on a i.MX53 based platform using a Micron eMMC card where the default > DSR is 0x0400 (12mA) but in our design 0x0100 (0x0100) were enough. To use this feature for > instance on a mx53loco one have to add a call to mmc_set_dsr() in board_mmc_init() after > calling fsl_esdhc_initialize() for the eMMC. > > Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de> > --- > changes for V2: > - extend documentation according to the comments from Pantelis Antoniou > > common/cmd_mmc.c | 23 +++++++++++++++++++++++ > drivers/mmc/mmc.c | 18 ++++++++++++++++++ > include/mmc.h | 3 +++ > 3 files changed, 44 insertions(+) > > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c > index 67a94a7..da5fef9 100644 > --- a/common/cmd_mmc.c > +++ b/common/cmd_mmc.c > @@ -340,6 +340,28 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > } > #endif /* CONFIG_SUPPORT_EMMC_BOOT */ > } > + > + else if (argc == 3 && strcmp(argv[1], "setdsr") == 0) { > + struct mmc *mmc = find_mmc_device(curr_device); > + u32 val = simple_strtoul(argv[2], NULL, 16); > + int ret; > + > + if (!mmc) { > + printf("no mmc device at slot %x\n", curr_device); > + return 1; > + } > + ret = mmc_set_dsr(mmc, val); > + printf("set dsr %s\n", (!ret) ? "OK, force rescan" : "ERROR"); > + if (!ret) { > + mmc->has_init = 0; > + if (mmc_init(mmc)) > + return 1; > + else > + return 0; > + } > + return ret; > + } > + > state = MMC_INVALID; > if (argc == 5 && strcmp(argv[1], "read") == 0) > state = MMC_READ; > @@ -423,5 +445,6 @@ U_BOOT_CMD( > "mmc bootpart <device num> <boot part size MB> <RPMB part size MB>\n" > " - change sizes of boot and RPMB partitions of specified device\n" > #endif > + "mmc setdsr - set DSR register value\n" > ); > #endif /* !CONFIG_GENERIC_MMC */ > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index e1461a9..c6a1c23 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -877,6 +877,7 @@ static int mmc_startup(struct mmc *mmc) > > mmc->tran_speed = freq * mult; > > + mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1); > mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf); > > if (IS_SD(mmc)) > @@ -907,6 +908,14 @@ static int mmc_startup(struct mmc *mmc) > if (mmc->write_bl_len > MMC_MAX_BLOCK_LEN) > mmc->write_bl_len = MMC_MAX_BLOCK_LEN; > > + if ((mmc->dsr_imp) && (0xffffffff != mmc->dsr)) { > + cmd.cmdidx = MMC_CMD_SET_DSR; > + cmd.cmdarg = (mmc->dsr & 0xffff) << 16; > + cmd.resp_type = MMC_RSP_NONE; > + if (mmc_send_cmd(mmc, &cmd, NULL)) > + printf("MMC: SET_DSR failed\n"); > + } > + > /* Select the card, and put it into Transfer Mode */ > if (!mmc_host_is_spi(mmc)) { /* cmd not supported in spi */ > cmd.cmdidx = MMC_CMD_SELECT_CARD; > @@ -1163,6 +1172,9 @@ static int mmc_send_if_cond(struct mmc *mmc) > > int mmc_register(struct mmc *mmc) > { > + /* Setup dsr related values */ > + mmc->dsr_imp = 0; > + mmc->dsr = 0xffffffff; > /* Setup the universal parts of the block interface just once */ > mmc->block_dev.if_type = IF_TYPE_MMC; > mmc->block_dev.dev = cur_dev_num++; > @@ -1280,6 +1292,12 @@ int mmc_init(struct mmc *mmc) > return err; > } > > +int mmc_set_dsr(struct mmc *mmc, u16 val) > +{ > + mmc->dsr = val; > + return 0; > +} > + > /* > * CPU and board-specific MMC initializations. Aliased function > * signals caller to move on > diff --git a/include/mmc.h b/include/mmc.h > index cb558da..facb819 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -262,6 +262,8 @@ struct mmc { > uint card_caps; > uint host_caps; > uint ocr; > + uint dsr; > + uint dsr_imp; > uint scr[2]; > uint csd[4]; > uint cid[4]; > @@ -304,6 +306,7 @@ int board_mmc_getcd(struct mmc *mmc); > int mmc_switch_part(int dev_num, unsigned int part_num); > int mmc_getcd(struct mmc *mmc); > int mmc_getwp(struct mmc *mmc); > +int mmc_set_dsr(struct mmc *mmc, u16 val); > void spl_mmc_load(void) __noreturn; > /* Function to change the size of boot partition and rpmb partitions */ > int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, > -- > 1.7.9.5 > Applied, thanks (although I have a few nitpicks about coding style in that file, will take a pass over the whole file and fix them all at some point). Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 67a94a7..da5fef9 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -340,6 +340,28 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif /* CONFIG_SUPPORT_EMMC_BOOT */ } + + else if (argc == 3 && strcmp(argv[1], "setdsr") == 0) { + struct mmc *mmc = find_mmc_device(curr_device); + u32 val = simple_strtoul(argv[2], NULL, 16); + int ret; + + if (!mmc) { + printf("no mmc device at slot %x\n", curr_device); + return 1; + } + ret = mmc_set_dsr(mmc, val); + printf("set dsr %s\n", (!ret) ? "OK, force rescan" : "ERROR"); + if (!ret) { + mmc->has_init = 0; + if (mmc_init(mmc)) + return 1; + else + return 0; + } + return ret; + } + state = MMC_INVALID; if (argc == 5 && strcmp(argv[1], "read") == 0) state = MMC_READ; @@ -423,5 +445,6 @@ U_BOOT_CMD( "mmc bootpart <device num> <boot part size MB> <RPMB part size MB>\n" " - change sizes of boot and RPMB partitions of specified device\n" #endif + "mmc setdsr - set DSR register value\n" ); #endif /* !CONFIG_GENERIC_MMC */ diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index e1461a9..c6a1c23 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -877,6 +877,7 @@ static int mmc_startup(struct mmc *mmc) mmc->tran_speed = freq * mult; + mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1); mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf); if (IS_SD(mmc)) @@ -907,6 +908,14 @@ static int mmc_startup(struct mmc *mmc) if (mmc->write_bl_len > MMC_MAX_BLOCK_LEN) mmc->write_bl_len = MMC_MAX_BLOCK_LEN; + if ((mmc->dsr_imp) && (0xffffffff != mmc->dsr)) { + cmd.cmdidx = MMC_CMD_SET_DSR; + cmd.cmdarg = (mmc->dsr & 0xffff) << 16; + cmd.resp_type = MMC_RSP_NONE; + if (mmc_send_cmd(mmc, &cmd, NULL)) + printf("MMC: SET_DSR failed\n"); + } + /* Select the card, and put it into Transfer Mode */ if (!mmc_host_is_spi(mmc)) { /* cmd not supported in spi */ cmd.cmdidx = MMC_CMD_SELECT_CARD; @@ -1163,6 +1172,9 @@ static int mmc_send_if_cond(struct mmc *mmc) int mmc_register(struct mmc *mmc) { + /* Setup dsr related values */ + mmc->dsr_imp = 0; + mmc->dsr = 0xffffffff; /* Setup the universal parts of the block interface just once */ mmc->block_dev.if_type = IF_TYPE_MMC; mmc->block_dev.dev = cur_dev_num++; @@ -1280,6 +1292,12 @@ int mmc_init(struct mmc *mmc) return err; } +int mmc_set_dsr(struct mmc *mmc, u16 val) +{ + mmc->dsr = val; + return 0; +} + /* * CPU and board-specific MMC initializations. Aliased function * signals caller to move on diff --git a/include/mmc.h b/include/mmc.h index cb558da..facb819 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -262,6 +262,8 @@ struct mmc { uint card_caps; uint host_caps; uint ocr; + uint dsr; + uint dsr_imp; uint scr[2]; uint csd[4]; uint cid[4]; @@ -304,6 +306,7 @@ int board_mmc_getcd(struct mmc *mmc); int mmc_switch_part(int dev_num, unsigned int part_num); int mmc_getcd(struct mmc *mmc); int mmc_getwp(struct mmc *mmc); +int mmc_set_dsr(struct mmc *mmc, u16 val); void spl_mmc_load(void) __noreturn; /* Function to change the size of boot partition and rpmb partitions */ int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize,