[U-Boot] [PATCH 1/3] cmd_mmc: add force_init parameter to init_mmc_device()
From: Stephen Warren swarren@nvidia.com
This allows callers to inject mmc->has_init = 0 between finding the MMC device, and calling mmc_init(), which forces mmc_init() to rescan the HW. Future changes will use this feature.
Signed-off-by: Stephen Warren swarren@nvidia.com ---
common/cmd_mmc.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 9e6a26fe62a2..6741ebee3bca 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -92,7 +92,7 @@ static void print_mmcinfo(struct mmc *mmc)
printf("Bus Width: %d-bit\n", mmc->bus_width); } -static struct mmc *init_mmc_device(int dev) +static struct mmc *init_mmc_device(int dev, bool force_init) { struct mmc *mmc; mmc = find_mmc_device(dev); @@ -100,6 +100,8 @@ static struct mmc *init_mmc_device(int dev) printf("no mmc device at slot %x\n", dev); return NULL; } + if (force_init) + mmc->has_init = 0; if (mmc_init(mmc)) return NULL; return mmc; @@ -117,7 +119,7 @@ static int do_mmcinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } }
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -247,7 +249,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag, if (flag == CMD_FLAG_REPEAT && !cp->repeatable) return CMD_RET_SUCCESS;
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -292,7 +294,7 @@ static int do_mmc_read(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[2], NULL, 16); cnt = simple_strtoul(argv[3], NULL, 16);
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -320,7 +322,7 @@ static int do_mmc_write(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[2], NULL, 16); cnt = simple_strtoul(argv[3], NULL, 16);
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -348,7 +350,7 @@ static int do_mmc_erase(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[1], NULL, 16); cnt = simple_strtoul(argv[2], NULL, 16);
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -387,7 +389,7 @@ static int do_mmc_part(cmd_tbl_t *cmdtp, int flag, block_dev_desc_t *mmc_dev; struct mmc *mmc;
- mmc = init_mmc_device(curr_device); + mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -422,7 +424,7 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev); + mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -462,7 +464,7 @@ static int do_mmc_bootbus(cmd_tbl_t *cmdtp, int flag, reset = simple_strtoul(argv[3], NULL, 10); mode = simple_strtoul(argv[4], NULL, 10);
- mmc = init_mmc_device(dev); + mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -487,7 +489,7 @@ static int do_mmc_boot_resize(cmd_tbl_t *cmdtp, int flag, bootsize = simple_strtoul(argv[2], NULL, 10); rpmbsize = simple_strtoul(argv[3], NULL, 10);
- mmc = init_mmc_device(dev); + mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -520,7 +522,7 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, part_num = simple_strtoul(argv[3], NULL, 10); access = simple_strtoul(argv[4], NULL, 10);
- mmc = init_mmc_device(dev); + mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -555,7 +557,7 @@ static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev); + mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
From: Stephen Warren swarren@nvidia.com
The body of init_mmc_device() is now identical to that of do_mmc_rescan() except for the error codes returned. Modify do_mmc_rescan() to simply call init_mmc_device() and convert the error codes, to avoid code duplication.
Signed-off-by: Stephen Warren swarren@nvidia.com --- common/cmd_mmc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 6741ebee3bca..6c8db2e78c4f 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -371,16 +371,10 @@ static int do_mmc_rescan(cmd_tbl_t *cmdtp, int flag, { struct mmc *mmc;
- mmc = find_mmc_device(curr_device); - if (!mmc) { - printf("no mmc device at slot %x\n", curr_device); + mmc = init_mmc_device(curr_device, true); + if (!mmc) return CMD_RET_FAILURE; - } - - mmc->has_init = 0;
- if (mmc_init(mmc)) - return CMD_RET_FAILURE; return CMD_RET_SUCCESS; } static int do_mmc_part(cmd_tbl_t *cmdtp, int flag,
Hi Stephen,
On May 23, 2014, at 10:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The body of init_mmc_device() is now identical to that of do_mmc_rescan() except for the error codes returned. Modify do_mmc_rescan() to simply call init_mmc_device() and convert the error codes, to avoid code duplication.
Signed-off-by: Stephen Warren swarren@nvidia.com
common/cmd_mmc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 6741ebee3bca..6c8db2e78c4f 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -371,16 +371,10 @@ static int do_mmc_rescan(cmd_tbl_t *cmdtp, int flag, { struct mmc *mmc;
- mmc = find_mmc_device(curr_device);
- if (!mmc) {
printf("no mmc device at slot %x\n", curr_device);
- mmc = init_mmc_device(curr_device, true);
- if (!mmc) return CMD_RET_FAILURE;
}
mmc->has_init = 0;
if (mmc_init(mmc))
return CMD_RET_FAILURE;return CMD_RET_SUCCESS;
} static int do_mmc_part(cmd_tbl_t *cmdtp, int flag, -- 1.8.1.5
Looks good.
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com
From: Stephen Warren swarren@nvidia.com
Currently, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1" - Execute: mmc dev 1 - This fails, since there is no card - User plugs in an SD card - Execute: mmc dev 1 - This still fails, since the HW isn't reprobed.
With this change, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1" - Execute: mmc dev 1 - This fails, since there is no card - User plugs in an SD card - Execute: mmc dev 1 - The newly present SD card is detected
I know that "mmc rescan" will force the HW to be reprobed, but I feel it makes more sense if "mmc dev" always reprobes the HW after selecting the current MMC device. This allows scripts to just execute "mmc dev", and not have to also execute "mmc rescan" to check for media presense.
Signed-off-by: Stephen Warren swarren@nvidia.com --- common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 6c8db2e78c4f..1e40983c757b 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -418,7 +418,7 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev, false); + mmc = init_mmc_device(dev, true); if (!mmc) return CMD_RET_FAILURE;
Hi Stephen,
On May 23, 2014, at 10:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Currently, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- This still fails, since the HW isn't reprobed.
With this change, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- The newly present SD card is detected
I know that "mmc rescan" will force the HW to be reprobed, but I feel it makes more sense if "mmc dev" always reprobes the HW after selecting the current MMC device. This allows scripts to just execute "mmc dev", and not have to also execute "mmc rescan" to check for media presense.
Signed-off-by: Stephen Warren swarren@nvidia.com
common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 6c8db2e78c4f..1e40983c757b 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -418,7 +418,7 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev, false);
- mmc = init_mmc_device(dev, true); if (!mmc) return CMD_RET_FAILURE;
-- 1.8.1.5
Behavioral change that no one could possibly depend on, so I'll just go ahead and get it in.
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com
On 06/12/2014 05:31 AM, Pantelis Antoniou wrote:
On May 23, 2014, at 10:24 PM, Stephen Warren wrote:
Currently, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- This still fails, since the HW isn't reprobed.
With this change, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- The newly present SD card is detected
...
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com
Thanks very much for applying these.
I'm puzzled why you write Acked-by in the emails, and add it to the commit descriptions when you apply them? FWIW for reference: Acked-by as used by the Linux kernel is usually only used when giving permission to a different maintainer to apply the patches, rather than taking them through the usual tree. Signed-off-by is the tag usually used when applying commits yourself, although there's an unresolved question re: whether adding s-o-b (or presumably anything at all) to commits when applying them is appropriate behaviour for U-Boot.
Hi Stephen,
On Jun 12, 2014, at 7:19 PM, Stephen Warren wrote:
On 06/12/2014 05:31 AM, Pantelis Antoniou wrote:
On May 23, 2014, at 10:24 PM, Stephen Warren wrote:
Currently, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- This still fails, since the HW isn't reprobed.
With this change, U-Boot behaves as follows:
- Begin with no SD card inserted in "mmc 1"
- Execute: mmc dev 1
- This fails, since there is no card
- User plugs in an SD card
- Execute: mmc dev 1
- The newly present SD card is detected
...
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com
Thanks very much for applying these.
I'm puzzled why you write Acked-by in the emails, and add it to the commit descriptions when you apply them? FWIW for reference: Acked-by as used by the Linux kernel is usually only used when giving permission to a different maintainer to apply the patches, rather than taking them through the usual tree. Signed-off-by is the tag usually used when applying commits yourself, although there's an unresolved question re: whether adding s-o-b (or presumably anything at all) to commits when applying them is appropriate behaviour for U-Boot.
Patchwork adds the Acked-by: line in the patches, and when I apply the patch I know for sure that I have actually reviewed the patch, and didn't just apply it blindly.
Just a way to not let things slip in.
Regards
-- Pantelis
On 05/23/2014 01:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This allows callers to inject mmc->has_init = 0 between finding the MMC device, and calling mmc_init(), which forces mmc_init() to rescan the HW. Future changes will use this feature.
Pantelis, does this series look good?
Hi, Stephen.
Your patch-set looks good to me.
Acked-by: Jaehoon Chung jh80.chung@samsung.com
If card didn't insert, can we display the other log instead of the below log?
#mmc rescan #mmc dev 1 "Card did not respond to voltage select!"
Best Regards, Jaehoon Chung
On 05/31/2014 06:12 AM, Stephen Warren wrote:
On 05/23/2014 01:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This allows callers to inject mmc->has_init = 0 between finding the MMC device, and calling mmc_init(), which forces mmc_init() to rescan the HW. Future changes will use this feature.
Pantelis, does this series look good?
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 06/01/2014 10:59 PM, Jaehoon Chung wrote:
Hi, Stephen.
Your patch-set looks good to me.
Acked-by: Jaehoon Chung jh80.chung@samsung.com
If card didn't insert, can we display the other log instead of the below log?
#mmc rescan #mmc dev 1 "Card did not respond to voltage select!"
Some SD slots have a "card detect" GPIO or similar, so it might be possible to do that. Of course, not all slots have that, and I don't know how to differentiate "card not present" from "command timed out". Is there a defined way to do that?
On 05/30/2014 03:12 PM, Stephen Warren wrote:
On 05/23/2014 01:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This allows callers to inject mmc->has_init = 0 between finding the MMC device, and calling mmc_init(), which forces mmc_init() to rescan the HW. Future changes will use this feature.
Pantelis, does this series look good?
Any comments?
Hi Stephen,
On May 23, 2014, at 10:24 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This allows callers to inject mmc->has_init = 0 between finding the MMC device, and calling mmc_init(), which forces mmc_init() to rescan the HW. Future changes will use this feature.
Signed-off-by: Stephen Warren swarren@nvidia.com
common/cmd_mmc.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 9e6a26fe62a2..6741ebee3bca 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -92,7 +92,7 @@ static void print_mmcinfo(struct mmc *mmc)
printf("Bus Width: %d-bit\n", mmc->bus_width); } -static struct mmc *init_mmc_device(int dev) +static struct mmc *init_mmc_device(int dev, bool force_init) { struct mmc *mmc; mmc = find_mmc_device(dev); @@ -100,6 +100,8 @@ static struct mmc *init_mmc_device(int dev) printf("no mmc device at slot %x\n", dev); return NULL; }
- if (force_init)
mmc->has_init = 0;@@ -117,7 +119,7 @@ static int do_mmcinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } }
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -247,7 +249,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag, if (flag == CMD_FLAG_REPEAT && !cp->repeatable) return CMD_RET_SUCCESS;
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -292,7 +294,7 @@ static int do_mmc_read(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[2], NULL, 16); cnt = simple_strtoul(argv[3], NULL, 16);
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -320,7 +322,7 @@ static int do_mmc_write(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[2], NULL, 16); cnt = simple_strtoul(argv[3], NULL, 16);
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -348,7 +350,7 @@ static int do_mmc_erase(cmd_tbl_t *cmdtp, int flag, blk = simple_strtoul(argv[1], NULL, 16); cnt = simple_strtoul(argv[2], NULL, 16);
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -387,7 +389,7 @@ static int do_mmc_part(cmd_tbl_t *cmdtp, int flag, block_dev_desc_t *mmc_dev; struct mmc *mmc;
- mmc = init_mmc_device(curr_device);
- mmc = init_mmc_device(curr_device, false); if (!mmc) return CMD_RET_FAILURE;
@@ -422,7 +424,7 @@ static int do_mmc_dev(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev);
- mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -462,7 +464,7 @@ static int do_mmc_bootbus(cmd_tbl_t *cmdtp, int flag, reset = simple_strtoul(argv[3], NULL, 10); mode = simple_strtoul(argv[4], NULL, 10);
- mmc = init_mmc_device(dev);
- mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -487,7 +489,7 @@ static int do_mmc_boot_resize(cmd_tbl_t *cmdtp, int flag, bootsize = simple_strtoul(argv[2], NULL, 10); rpmbsize = simple_strtoul(argv[3], NULL, 10);
- mmc = init_mmc_device(dev);
- mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -520,7 +522,7 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, part_num = simple_strtoul(argv[3], NULL, 10); access = simple_strtoul(argv[4], NULL, 10);
- mmc = init_mmc_device(dev);
- mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
@@ -555,7 +557,7 @@ static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, return CMD_RET_USAGE; }
- mmc = init_mmc_device(dev);
- mmc = init_mmc_device(dev, false); if (!mmc) return CMD_RET_FAILURE;
-- 1.8.1.5
Looks fine. I have a slight preference for not using bools in C code, but I see it's already creeping in the codebase. I'll apply it as is, and if we have any complaints by people using really ancient compilers I'll convert to int.
Thanks, applied.
-- Pantelis
Acked-by: Pantelis Antoniou panto@antoniou-consulting.com
participants (3)
-
Jaehoon Chung -
Pantelis Antoniou -
Stephen Warren