cmd_mmc: add force_init parameter to init_mmc_device()
PATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
From: Stephen Warren <[email protected]>
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.
---
common/cmd_mmc.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
common/cmd_mmc.c +14 -12
@@ -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 ;
Signature · 1 line
1.8.1.5
PATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
This allows callers to inject
mmc->has_init= 0 between finding the MMC device, and callingmmc_init(), which forcesmmc_init()to rescan the HW. Future changes will use this feature.
Pantelis, does this series look good?
PATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
Hi, Stephen.
Your patch-set looks good to me.
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
Quoted 14 lines · Stephen Warren
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
This allows callers to inject
mmc->has_init= 0 between finding the MMC device, and callingmmc_init(), which forcesmmc_init()to rescan the HW. Future changes will use this feature.Pantelis, does this series look good?
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
Quoted 11 lines · Jaehoon Chung
Hi, Stephen.
Your patch-set looks good to me.
Acked-by Jaehoon Chung <[email protected]>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?
PATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
In reply to Stephen WarrenQuoted 8 lines · Stephen Warren
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
This allows callers to inject
mmc->has_init= 0 between finding the MMC device, and callingmmc_init(), which forcesmmc_init()to rescan the HW. Future changes will use this feature.Pantelis, does this series look good?
Any comments?
PATCH 2/3 cmd_mmc: Use init_mmc_device() from do_mmc_rescan()
In reply to Stephen WarrenFrom: Stephen Warren <[email protected]>
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.
--- common/cmd_mmc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
common/cmd_mmc.c +2 -8
@@ -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,
Signature · 1 line
1.8.1.5
Pantelis Antoniou
AppliedPATCH 2/3 cmd_mmc: Use init_mmc_device() from do_mmc_rescan()
Hi Stephen,
Quoted 38 lines · Stephen Warren
From: Stephen Warren <[email protected]>
The body of
init_mmc_device()is now identical to that ofdo_mmc_rescan()except for the error codes returned. Modifydo_mmc_rescan()to simply callinit_mmc_device()and convert the error codes, to avoid code duplication.Signed-off-by Stephen Warren <[email protected]>---
common/cmd_mmc.c| 10 ++--------1 file changed, 2 insertions(+), 8 deletions(-)common/cmd_mmc.c +2
-8@@ -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,Signature · 1 line
1.8.1.5
Looks good.
Thanks, applied.
-- Pantelis
PATCH 3/3 cmd_mmc: make mmc dev always re-probe the HW
In reply to Stephen WarrenFrom: Stephen Warren <[email protected]>
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.
--- common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/cmd_mmc.c +1 -1
@@ -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 ;
Signature · 1 line
1.8.1.5
Pantelis Antoniou
AppliedPATCH 3/3 cmd_mmc: make mmc dev always re-probe the HW
Hi Stephen,
Quoted 46 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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 detectedI 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 <[email protected]>---
common/cmd_mmc.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)common/cmd_mmc.c +1
-1@@ -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 ;Signature · 1 line
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
PATCH 3/3 cmd_mmc: make mmc dev always re-probe the HW
Quoted 18 lines · Pantelis Antoniou
Quoted 17 lines · Stephen Warren
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
...
Quoted 5 lines
Thanks, applied.
-- Pantelis
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.
PATCH 3/3 cmd_mmc: make mmc dev always re-probe the HW
Hi Stephen,
Quoted 28 lines · Stephen Warren
Quoted 18 lines · Pantelis Antoniou
Quoted 17 lines · Stephen Warren
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...
Quoted 5 lines
Thanks, applied.
-- Pantelis
Thanks very much for applying these.
Quoted 8 lines
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
Pantelis Antoniou
AppliedPATCH 1/3 cmd_mmc: add force_init parameter to init_mmc_device()
In reply to Stephen WarrenHi Stephen,
Quoted 136 lines · Stephen Warren
From: Stephen Warren <[email protected]>
This allows callers to inject
mmc->has_init= 0 between finding the MMC device, and callingmmc_init(), which forcesmmc_init()to rescan the HW. Future changes will use this feature.Signed-off-by Stephen Warren <[email protected]>---
common/cmd_mmc.c| 26 ++++++++++++++------------1 file changed, 14 insertions(+), 12 deletions(-)common/cmd_mmc.c +14
-12@@ -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 ;Signature · 1 line
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