diff mbox

[U-Boot] mmc: return meaningful error codes from mmc_select_hwpart

Message ID 1400870826-14589-1-git-send-email-swarren@wwwdotorg.org
State Accepted
Delegated to: Pantelis Antoniou
Headers show

Commit Message

Stephen Warren May 23, 2014, 6:47 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

Rather than just returning -1 everywhere, try to return something
meaningful from mmc_select_hwpart(). Note that most other MMC functions
don't do this, including functions called from mmc_select_hwpart(), so
I'm not sure how effective this will be. Still, it's one less place with
hard-coded -1.

Suggested-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/mmc/mmc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Stephen Warren May 30, 2014, 9:11 p.m. UTC | #1
On 05/23/2014 12:47 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Rather than just returning -1 everywhere, try to return something
> meaningful from mmc_select_hwpart(). Note that most other MMC functions
> don't do this, including functions called from mmc_select_hwpart(), so
> I'm not sure how effective this will be. Still, it's one less place with
> hard-coded -1.

Pantelis, does this look good?
Pantelis Antoniou June 2, 2014, 6:18 a.m. UTC | #2
Hi Stephen,

On May 23, 2014, at 9:47 PM, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Rather than just returning -1 everywhere, try to return something
> meaningful from mmc_select_hwpart(). Note that most other MMC functions
> don't do this, including functions called from mmc_select_hwpart(), so
> I'm not sure how effective this will be. Still, it's one less place with
> hard-coded -1.
> 
> Suggested-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> drivers/mmc/mmc.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 8b53ead98f80..221a293797e9 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -10,6 +10,7 @@
> #include <config.h>
> #include <common.h>
> #include <command.h>
> +#include <errno.h>
> #include <mmc.h>
> #include <part.h>
> #include <malloc.h>
> @@ -558,19 +559,19 @@ int mmc_select_hwpart(int dev_num, int hwpart)
> 	int ret;
> 
> 	if (!mmc)
> -		return -1;
> +		return -ENODEV;
> 
> 	if (mmc->part_num == hwpart)
> 		return 0;
> 
> 	if (mmc->part_config == MMCPART_NOAVAILABLE) {
> 		printf("Card doesn't support part_switch\n");
> -		return -1;
> +		return -EMEDIUMTYPE;
> 	}
> 
> 	ret = mmc_switch_part(dev_num, hwpart);
> 	if (ret)
> -		return -1;
> +		return ret;
> 
> 	mmc->part_num = hwpart;
> 
> -- 
> 1.8.1.5
> 

This does indeed look good.

Thanks

-- Pantelis

Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Stephen Warren June 10, 2014, 4 p.m. UTC | #3
On 06/02/2014 12:18 AM, Pantelis Antoniou wrote:
> On May 23, 2014, at 9:47 PM, Stephen Warren wrote:
>> Rather than just returning -1 everywhere, try to return something
>> meaningful from mmc_select_hwpart(). Note that most other MMC functions
>> don't do this, including functions called from mmc_select_hwpart(), so
>> I'm not sure how effective this will be. Still, it's one less place with
>> hard-coded -1.
...
> This does indeed look good.
...
> Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>

Are you intending to apply this patch, or were you expecting someone
else to?
Pantelis Antoniou June 10, 2014, 4:01 p.m. UTC | #4
On Jun 10, 2014, at 7:00 PM, Stephen Warren wrote:

> On 06/02/2014 12:18 AM, Pantelis Antoniou wrote:
>> On May 23, 2014, at 9:47 PM, Stephen Warren wrote:
>>> Rather than just returning -1 everywhere, try to return something
>>> meaningful from mmc_select_hwpart(). Note that most other MMC functions
>>> don't do this, including functions called from mmc_select_hwpart(), so
>>> I'm not sure how effective this will be. Still, it's one less place with
>>> hard-coded -1.
> ...
>> This does indeed look good.
> ...
>> Acked-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> Are you intending to apply this patch, or were you expecting someone
> else to?

I will apply the patch, but it will go out with the rest on the next
mmc pull request this week.

Regards

-- Pantelis
diff mbox

Patch

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 8b53ead98f80..221a293797e9 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -10,6 +10,7 @@ 
 #include <config.h>
 #include <common.h>
 #include <command.h>
+#include <errno.h>
 #include <mmc.h>
 #include <part.h>
 #include <malloc.h>
@@ -558,19 +559,19 @@  int mmc_select_hwpart(int dev_num, int hwpart)
 	int ret;
 
 	if (!mmc)
-		return -1;
+		return -ENODEV;
 
 	if (mmc->part_num == hwpart)
 		return 0;
 
 	if (mmc->part_config == MMCPART_NOAVAILABLE) {
 		printf("Card doesn't support part_switch\n");
-		return -1;
+		return -EMEDIUMTYPE;
 	}
 
 	ret = mmc_switch_part(dev_num, hwpart);
 	if (ret)
-		return -1;
+		return ret;
 
 	mmc->part_num = hwpart;