* [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified
@ 2014-05-23 18:48 Stephen Warren
2014-05-23 18:48 ` [U-Boot] [PATCH 2/2] disk: " Stephen Warren
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stephen Warren @ 2014-05-23 18:48 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] disk: default to HW partition 0 if not specified
2014-05-23 18:48 [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified Stephen Warren
@ 2014-05-23 18:48 ` Stephen Warren
2014-06-12 11:23 ` Pantelis Antoniou
2014-05-30 21:11 ` [U-Boot] [PATCH 1/2] cmd_mmc: " Stephen Warren
2014-06-12 11:21 ` Pantelis Antoniou
2 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2014-05-23 18:48 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified
2014-05-23 18:48 [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified Stephen Warren
2014-05-23 18:48 ` [U-Boot] [PATCH 2/2] disk: " Stephen Warren
@ 2014-05-30 21:11 ` Stephen Warren
2014-06-10 16:00 ` Stephen Warren
2014-06-12 11:21 ` Pantelis Antoniou
2 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2014-05-30 21:11 UTC (permalink / raw)
To: u-boot
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified
2014-05-30 21:11 ` [U-Boot] [PATCH 1/2] cmd_mmc: " Stephen Warren
@ 2014-06-10 16:00 ` Stephen Warren
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-06-10 16:00 UTC (permalink / raw)
To: u-boot
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified
2014-05-23 18:48 [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified Stephen Warren
2014-05-23 18:48 ` [U-Boot] [PATCH 2/2] disk: " Stephen Warren
2014-05-30 21:11 ` [U-Boot] [PATCH 1/2] cmd_mmc: " Stephen Warren
@ 2014-06-12 11:21 ` Pantelis Antoniou
2 siblings, 0 replies; 6+ messages in thread
From: Pantelis Antoniou @ 2014-06-12 11:21 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] disk: default to HW partition 0 if not specified
2014-05-23 18:48 ` [U-Boot] [PATCH 2/2] disk: " Stephen Warren
@ 2014-06-12 11:23 ` Pantelis Antoniou
0 siblings, 0 replies; 6+ messages in thread
From: Pantelis Antoniou @ 2014-06-12 11:23 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-12 11:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-23 18:48 [U-Boot] [PATCH 1/2] cmd_mmc: default to HW partition 0 if not specified Stephen Warren
2014-05-23 18:48 ` [U-Boot] [PATCH 2/2] disk: " Stephen Warren
2014-06-12 11:23 ` Pantelis Antoniou
2014-05-30 21:11 ` [U-Boot] [PATCH 1/2] cmd_mmc: " Stephen Warren
2014-06-10 16:00 ` Stephen Warren
2014-06-12 11:21 ` Pantelis Antoniou
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.