| Message ID | 1400605052-14714-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Rejected |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
Hi, Stephen. i didn't apply your patch. Which repository do you use? Well, if you want to check, can be used "if (mmc_init(mmc))". And i'm not sure whether this code is really need or not. Best Regards, Jaehoon Chung On 05/21/2014 01:57 AM, Stephen Warren wrote: > From: Bryan Wu <cooloney@gmail.com> > > mmc dev ${devnum} will return 0 as success even if there is no card > inserted. Booting script like tegra-common-post.h will call mmc dev > ${devnum} to check the mmc device status, it always return 0 even if > mmc_init() fails. > > Check mmc_init() return value let mmc dev command return failure > when mmc_init() fails. > > Signed-off-by: Bryan Wu <pengw@nvidia.com> > [swarren: Always return 1 on error; hush doesn't appear to support > arbitrary return values. Fixed compilation error due to multiple > declaration of ret variable.] > Tested-by: Stephen Warren <swarren@nvidia.com> > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > v2: > * Always return 1 on error; hush doesn't appear to support > arbitrary return values. > * Fixed compilation error due to multiple declaration of ret variable. > --- > common/cmd_mmc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c > index 572aa895fa1c..2e564572f74d 100644 > --- a/common/cmd_mmc.c > +++ b/common/cmd_mmc.c > @@ -217,7 +217,10 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > return 1; > } > > - mmc_init(mmc); > + ret = mmc_init(mmc); > + if (ret) > + return 1; > + > if (part != -1) { > ret = mmc_select_hwpart(dev, part); > printf("switch to partitions #%d, %s\n", >
On 05/20/2014 11:40 PM, Jaehoon Chung wrote: > Hi, Stephen. > > i didn't apply your patch. Which repository do you use? It's based on u-boot.git master branch. The latest u-boot-mmc.git master branch is already included in that branch, and it looks like some changes have been applied to cmd_mmc.c in u-boot/master that aren't in u-boot-mmc/master. > Well, if you want to check, can be used "if (mmc_init(mmc))". > > And i'm not sure whether this code is really need or not. Why not? This code is required to solve the problem described in the commit description: > On 05/21/2014 01:57 AM, Stephen Warren wrote: >> From: Bryan Wu <cooloney@gmail.com> >> >> mmc dev ${devnum} will return 0 as success even if there is no card >> inserted. Booting script like tegra-common-post.h will call mmc dev >> ${devnum} to check the mmc device status, it always return 0 even if >> mmc_init() fails.
On 05/22/2014 01:18 AM, Stephen Warren wrote: > On 05/20/2014 11:40 PM, Jaehoon Chung wrote: >> Hi, Stephen. >> >> i didn't apply your patch. Which repository do you use? > > It's based on u-boot.git master branch. The latest u-boot-mmc.git master > branch is already included in that branch, and it looks like some > changes have been applied to cmd_mmc.c in u-boot/master that aren't in > u-boot-mmc/master. I have pulled the latest u-boot.git, but it didn't apply this patch. If i missed something, let me know plz. > >> Well, if you want to check, can be used "if (mmc_init(mmc))". >> >> And i'm not sure whether this code is really need or not. > > Why not? This code is required to solve the problem described in the > commit description: I will try to reproduce the problem described in the commit-msg. Because, i didn't reproduce it, so i'm not sure. But to control the return value, it's reasonable, right? Best Regards, Jaehoon Chung > >> On 05/21/2014 01:57 AM, Stephen Warren wrote: >>> From: Bryan Wu <cooloney@gmail.com> >>> >>> mmc dev ${devnum} will return 0 as success even if there is no card >>> inserted. Booting script like tegra-common-post.h will call mmc dev >>> ${devnum} to check the mmc device status, it always return 0 even if >>> mmc_init() fails. > > >
On 05/21/2014 07:41 PM, Jaehoon Chung wrote: > On 05/22/2014 01:18 AM, Stephen Warren wrote: >> On 05/20/2014 11:40 PM, Jaehoon Chung wrote: >>> Hi, Stephen. >>> >>> i didn't apply your patch. Which repository do you use? >> >> It's based on u-boot.git master branch. The latest u-boot-mmc.git master >> branch is already included in that branch, and it looks like some >> changes have been applied to cmd_mmc.c in u-boot/master that aren't in >> u-boot-mmc/master. > > I have pulled the latest u-boot.git, but it didn't apply this patch. > If i missed something, let me know plz. Ah, I guess I hadn't noticed there's an interaction (context changes) with some other MMC-related patches that I sent: http://patchwork.ozlabs.org/patch/346771/ [U-Boot,1/4] cmd_part: fix type in part command help text http://patchwork.ozlabs.org/patch/346770/ [U-Boot,2/4] disk: support devices with HW partitions http://patchwork.ozlabs.org/patch/346768/ [U-Boot,3/4] mmc: provide a select_hwpart implementation for get_device() http://patchwork.ozlabs.org/patch/346769/ [U-Boot,4/4] cmd_mmc: use new mmc_select_hwpart() function So, you can either apply those first, or use "git am -3" rather than "git am", plus declare "int ret"; patch 4/4 above does that. >>> Well, if you want to check, can be used "if (mmc_init(mmc))". >>> >>> And i'm not sure whether this code is really need or not. >> >> Why not? This code is required to solve the problem described in the >> commit description: > > I will try to reproduce the problem described in the commit-msg. > Because, i didn't reproduce it, so i'm not sure. > But to control the return value, it's reasonable, right? Yes, I think so.
On 05/20/2014 10:57 AM, Stephen Warren wrote: > From: Bryan Wu <cooloney@gmail.com> > > mmc dev ${devnum} will return 0 as success even if there is no card > inserted. Booting script like tegra-common-post.h will call mmc dev > ${devnum} to check the mmc device status, it always return 0 even if > mmc_init() fails. > > Check mmc_init() return value let mmc dev command return failure > when mmc_init() fails. For the email archive record, it looks like this same logic has already been implemented as part of: 1fd93c6e7d8a eMMC: cmd_mmc.c adds the 'rpmb' sub-command for the 'mmc' command ... which was just recently committed into u-boot-mmc/master. (See the code in init_mmc_device)
Hi Stephen, On May 23, 2014, at 9:59 PM, Stephen Warren wrote: > On 05/20/2014 10:57 AM, Stephen Warren wrote: >> From: Bryan Wu <cooloney@gmail.com> >> >> mmc dev ${devnum} will return 0 as success even if there is no card >> inserted. Booting script like tegra-common-post.h will call mmc dev >> ${devnum} to check the mmc device status, it always return 0 even if >> mmc_init() fails. >> >> Check mmc_init() return value let mmc dev command return failure >> when mmc_init() fails. > > For the email archive record, it looks like this same logic has already > been implemented as part of: > > 1fd93c6e7d8a eMMC: cmd_mmc.c adds the 'rpmb' sub-command for the 'mmc' > command > > ... which was just recently committed into u-boot-mmc/master. > > (See the code in init_mmc_device) Similar functionality is in master already. Dropping the patch. -- Pantelis
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 572aa895fa1c..2e564572f74d 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -217,7 +217,10 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; } - mmc_init(mmc); + ret = mmc_init(mmc); + if (ret) + return 1; + if (part != -1) { ret = mmc_select_hwpart(dev, part); printf("switch to partitions #%d, %s\n",