[U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified

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);

From: Stephen Warren swarren@nvidia.com
Currently, get_device()/get_dev_hwpart() for MMC devices does not select an explicit HW partition unless the user explicitly requests one, i.e. by requesting device "mmc 0.0" rather than just "mmc 0". I think it makes more sense if the default is to select HW partition 0 (main data area) 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
Attempts to access SW partition 1 on HW partition 1 (boot0), rather than SW partition 1 on HW partition 0 (main data area): ls mmc 0:1 /
With this patch, the second command above re-selects the main data area.
Many device types don't support HW partitions at all, so if HW partition 0 is selected (either explicitly or as the default) and there's no select_hwpart function, we simply skip attempting to select a HW partition.
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 --- disk/part.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 2827089d8d5f..b3097e32f0eb 100644 --- a/disk/part.c +++ b/disk/part.c @@ -86,7 +86,7 @@ block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart) block_dev_desc_t *dev_desc = reloc_get_dev(dev); if (!dev_desc) return NULL; - if (hwpart == -1) + if (hwpart == 0 && !select_hwpart) return dev_desc; if (!select_hwpart) return NULL; @@ -102,7 +102,7 @@ block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart)
block_dev_desc_t *get_dev(const char *ifname, int dev) { - return get_dev_hwpart(ifname, dev, -1); + return get_dev_hwpart(ifname, dev, 0); } #else block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart) @@ -460,7 +460,7 @@ int get_device(const char *ifname, const char *dev_hwpart_str, hwpart_str++; } else { dev_str = dev_hwpart_str; - hwpart = -1; + hwpart = 0; }
dev = simple_strtoul(dev_str, &ep, 16);

Hi Stephen,
On May 23, 2014, at 9:48 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Currently, get_device()/get_dev_hwpart() for MMC devices does not select an explicit HW partition unless the user explicitly requests one, i.e. by requesting device "mmc 0.0" rather than just "mmc 0". I think it makes more sense if the default is to select HW partition 0 (main data area) 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
Attempts to access SW partition 1 on HW partition 1 (boot0), rather than SW partition 1 on HW partition 0 (main data area): ls mmc 0:1 /
With this patch, the second command above re-selects the main data area.
Many device types don't support HW partitions at all, so if HW partition 0 is selected (either explicitly or as the default) and there's no select_hwpart function, we simply skip attempting to select a HW partition.
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
disk/part.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 2827089d8d5f..b3097e32f0eb 100644 --- a/disk/part.c +++ b/disk/part.c @@ -86,7 +86,7 @@ block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart) block_dev_desc_t *dev_desc = reloc_get_dev(dev); if (!dev_desc) return NULL;
if (hwpart == -1)
if (hwpart == 0 && !select_hwpart) return dev_desc; if (!select_hwpart) return NULL;
@@ -102,7 +102,7 @@ block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart)
block_dev_desc_t *get_dev(const char *ifname, int dev) {
- return get_dev_hwpart(ifname, dev, -1);
- return get_dev_hwpart(ifname, dev, 0);
} #else block_dev_desc_t *get_dev_hwpart(const char *ifname, int dev, int hwpart) @@ -460,7 +460,7 @@ int get_device(const char *ifname, const char *dev_hwpart_str, hwpart_str++; } else { dev_str = dev_hwpart_str;
hwpart = -1;
hwpart = 0;
}
dev = simple_strtoul(dev_str, &ep, 16);
-- 1.8.1.5
Same comment for part 1, slight behavioral change but with no known side-effects.
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com

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
participants (2)
-
Pantelis Antoniou
-
Stephen Warren