| Message ID | 1400870891-14655-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
On 05/23/2014 12:48 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Currently, "mmc dev 0" does not change the selected HW partition. I > think it makes more sense if "mmc dev 0" is an alias for "mmc dev 0 0", > i.e. that HW partition 0 (main data area) is always selected by default > if the user didn't request a specific partition. Otherwise, the following > happens, which feels wrong: > > Select HW partition 1 (boot0): > mmc dev 0 1 > > Doesn't change the HW partition, so it's still 1 (boot0): > mmc dev 0 > > With this patch, the second command above re-selects the main data area. > > Note that some MMC devices (i.e. SD cards) don't support HW partitions. > However, this patch still works, since mmc_start_init() sets the current > partition number to 0, and mmc_select_hwpart() succeeds if the requested > partition is already selected. Pantelis, does this series look good?
On 05/30/2014 03:11 PM, Stephen Warren wrote: > On 05/23/2014 12:48 PM, Stephen Warren wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> Currently, "mmc dev 0" does not change the selected HW partition. I >> think it makes more sense if "mmc dev 0" is an alias for "mmc dev 0 0", >> i.e. that HW partition 0 (main data area) is always selected by default >> if the user didn't request a specific partition. Otherwise, the following >> happens, which feels wrong: >> >> Select HW partition 1 (boot0): >> mmc dev 0 1 >> >> Doesn't change the HW partition, so it's still 1 (boot0): >> mmc dev 0 >> >> With this patch, the second command above re-selects the main data area. >> >> Note that some MMC devices (i.e. SD cards) don't support HW partitions. >> However, this patch still works, since mmc_start_init() sets the current >> partition number to 0, and mmc_select_hwpart() succeeds if the requested >> partition is already selected. > > Pantelis, does this series look good? Any comment on this series?
Hi Stephen, On May 23, 2014, at 9:48 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Currently, "mmc dev 0" does not change the selected HW partition. I > think it makes more sense if "mmc dev 0" is an alias for "mmc dev 0 0", > i.e. that HW partition 0 (main data area) is always selected by default > if the user didn't request a specific partition. Otherwise, the following > happens, which feels wrong: > > Select HW partition 1 (boot0): > mmc dev 0 1 > > Doesn't change the HW partition, so it's still 1 (boot0): > mmc dev 0 > > With this patch, the second command above re-selects the main data area. > > Note that some MMC devices (i.e. SD cards) don't support HW partitions. > However, this patch still works, since mmc_start_init() sets the current > partition number to 0, and mmc_select_hwpart() succeeds if the requested > partition is already selected. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > This series depends on "mmc: return meaningful error codes from > mmc_select_hwpart" (which I just sent) for context. > > common/cmd_mmc.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c > index eea337506c3f..9e6a26fe62a2 100644 > --- a/common/cmd_mmc.c > +++ b/common/cmd_mmc.c > @@ -403,7 +403,7 @@ static int do_mmc_part(cmd_tbl_t *cmdtp, int flag, > static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, > int argc, char * const argv[]) > { > - int dev, part = -1, ret; > + int dev, part = 0, ret; > struct mmc *mmc; > > if (argc == 1) { > @@ -426,13 +426,12 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, > if (!mmc) > return CMD_RET_FAILURE; > > - if (part != -1) { > - ret = mmc_select_hwpart(dev, part); > - printf("switch to partitions #%d, %s\n", > - part, (!ret) ? "OK" : "ERROR"); > - if (ret) > - return 1; > - } > + ret = mmc_select_hwpart(dev, part); > + printf("switch to partitions #%d, %s\n", > + part, (!ret) ? "OK" : "ERROR"); > + if (ret) > + return 1; > + > curr_device = dev; > if (mmc->part_config == MMCPART_NOAVAILABLE) > printf("mmc%d is current device\n", curr_device); > -- > 1.8.1.5 There's a small behavioral change (always selecting a hardware partition when selecting a device), but there shouldn't be any users that care about it. Thanks, applied. -- Pantelis Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index eea337506c3f..9e6a26fe62a2 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -403,7 +403,7 @@ static int do_mmc_part(cmd_tbl_t *cmdtp, int flag, static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - int dev, part = -1, ret; + int dev, part = 0, ret; struct mmc *mmc; if (argc == 1) { @@ -426,13 +426,12 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, if (!mmc) return CMD_RET_FAILURE; - if (part != -1) { - ret = mmc_select_hwpart(dev, part); - printf("switch to partitions #%d, %s\n", - part, (!ret) ? "OK" : "ERROR"); - if (ret) - return 1; - } + ret = mmc_select_hwpart(dev, part); + printf("switch to partitions #%d, %s\n", + part, (!ret) ? "OK" : "ERROR"); + if (ret) + return 1; + curr_device = dev; if (mmc->part_config == MMCPART_NOAVAILABLE) printf("mmc%d is current device\n", curr_device);