Patchwork [U-Boot,3/3] ums: Extend ums to use all mmc devices.

login
register
mail settings
Submitter Przemyslaw Marczak
Date Aug. 20, 2013, 10:53 a.m.
Message ID <1376995992-20870-3-git-send-email-p.marczak@samsung.com>
Download mbox | patch
Permalink /patch/268489/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Przemyslaw Marczak - Aug. 20, 2013, 10:53 a.m.
This change allows using every mmc device instance with ums,
like eMMC or SD cards.

Example: ums <device_number> for mmc devices.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Marek Vasut <marek.vasut@gmail.com>
---
 common/cmd_usb_mass_storage.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Przemyslaw Marczak - Aug. 28, 2013, 7:40 a.m.
Hello Marek,

> This change allows using every mmc device instance with ums,
> like eMMC or SD cards.
>
> Example: ums <device_number> for mmc devices.

Could you review this patch, please?
>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Marek Vasut <marek.vasut@gmail.com>
> ---
>   common/cmd_usb_mass_storage.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
> index 33a4715..4181d3a 100644
> --- a/common/cmd_usb_mass_storage.c
> +++ b/common/cmd_usb_mass_storage.c
> @@ -14,6 +14,7 @@
>   int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
>   			       int argc, char * const argv[])
>   {
> +	struct mmc *mmc = NULL;
>   	char *ep;
>   	unsigned int dev_num = 0, offset = 0, part_size = 0;
>   	int rc;
> @@ -28,8 +29,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
>
>   	dev_num = (int)simple_strtoul(argv[1], &ep, 16);
>
> -	if (dev_num) {
> -		puts("\nSet eMMC device to 0! - e.g. ums 0\n");
> +	mmc = find_mmc_device(dev_num);
> +
> +	if (!mmc) {
> +		printf("\neMMC device: %d not found! Try ums 0.\n", dev_num);
>   		goto fail;
>   	}
>
>
Marek Vasut - Aug. 28, 2013, 8:44 a.m.
Dear Przemyslaw Marczak,

> This change allows using every mmc device instance with ums,
> like eMMC or SD cards.
> 
> Example: ums <device_number> for mmc devices.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Marek Vasut <marek.vasut@gmail.com>
> ---
>  common/cmd_usb_mass_storage.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
> index 33a4715..4181d3a 100644
> --- a/common/cmd_usb_mass_storage.c
> +++ b/common/cmd_usb_mass_storage.c
> @@ -14,6 +14,7 @@
>  int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
>  			       int argc, char * const argv[])
>  {
> +	struct mmc *mmc = NULL;
>  	char *ep;
>  	unsigned int dev_num = 0, offset = 0, part_size = 0;
>  	int rc;
> @@ -28,8 +29,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
> 
>  	dev_num = (int)simple_strtoul(argv[1], &ep, 16);
> 
> -	if (dev_num) {
> -		puts("\nSet eMMC device to 0! - e.g. ums 0\n");
> +	mmc = find_mmc_device(dev_num);
> +
> +	if (!mmc) {
> +		printf("\neMMC device: %d not found! Try ums 0.\n", dev_num);
>  		goto fail;
>  	}

You still use dev_num further in the code, why not convert the whole thing to 
use *mmc ?

Best regards,
Marek Vasut
Przemyslaw Marczak - Aug. 28, 2013, 10:59 a.m.
On 08/28/2013 10:44 AM, Marek Vasut wrote:
> You still use dev_num further in the code, why not convert the whole thing to
> use *mmc ?
>
> Best regards,
> Marek Vasut.
>
Hello,
I don't know exactly what do you mean.
I suppose that putting "mmc device number" as a parameter is easy to 
maintain at command code and also intuitive to use at prompt.
Do you prefer set mmc dev number by command "mmc dev <num>" ?
Should we depends one command on another? If yes, then what about e.g. 
fat or ext command set?

Thank you.
Marek Vasut - Aug. 28, 2013, 11:03 a.m.
Dear Przemyslaw Marczak,

> On 08/28/2013 10:44 AM, Marek Vasut wrote:
> > You still use dev_num further in the code, why not convert the whole
> > thing to use *mmc ?
> > 
> > Best regards,
> > Marek Vasut.
> 
> Hello,
> I don't know exactly what do you mean.
> I suppose that putting "mmc device number" as a parameter is easy to
> maintain at command code and also intuitive to use at prompt.
> Do you prefer set mmc dev number by command "mmc dev <num>" ?
> Should we depends one command on another? If yes, then what about e.g.
> fat or ext command set?

What I mean is ... for example, you pass dev_num to board_ums_init() . See the 
trats board, find_mmc_device() is called again in there. Why not just pass the 
*mmc directly?

Best regards,
Marek Vasut
Przemyslaw Marczak - Aug. 28, 2013, 11:21 a.m.
On 08/28/2013 01:03 PM, Marek Vasut wrote:
> Dear Przemyslaw Marczak,
>
>> On 08/28/2013 10:44 AM, Marek Vasut wrote:
>>> You still use dev_num further in the code, why not convert the whole
>>> thing to use *mmc ?
>>>
>>> Best regards,
>>> Marek Vasut.
>> Hello,
>> I don't know exactly what do you mean.
>> I suppose that putting "mmc device number" as a parameter is easy to
>> maintain at command code and also intuitive to use at prompt.
>> Do you prefer set mmc dev number by command "mmc dev <num>" ?
>> Should we depends one command on another? If yes, then what about e.g.
>> fat or ext command set?
> What I mean is ... for example, you pass dev_num to board_ums_init() . See the
> trats board, find_mmc_device() is called again in there. Why not just pass the
> *mmc directly?
>
> Best regards,
> Marek Vasut.
>
Oh, now I see. You are absolutely right, it was just code duplication.
I will change it and resend patch.
Thank you,
Marek Vasut - Aug. 28, 2013, 11:25 a.m.
Dear Przemyslaw Marczak,

> On 08/28/2013 01:03 PM, Marek Vasut wrote:
> > Dear Przemyslaw Marczak,
> > 
> >> On 08/28/2013 10:44 AM, Marek Vasut wrote:
> >>> You still use dev_num further in the code, why not convert the whole
> >>> thing to use *mmc ?
> >>> 
> >>> Best regards,
> >>> Marek Vasut.
> >> 
> >> Hello,
> >> I don't know exactly what do you mean.
> >> I suppose that putting "mmc device number" as a parameter is easy to
> >> maintain at command code and also intuitive to use at prompt.
> >> Do you prefer set mmc dev number by command "mmc dev <num>" ?
> >> Should we depends one command on another? If yes, then what about e.g.
> >> fat or ext command set?
> > 
> > What I mean is ... for example, you pass dev_num to board_ums_init() .
> > See the trats board, find_mmc_device() is called again in there. Why not
> > just pass the *mmc directly?
> > 
> > Best regards,
> > Marek Vasut.
> 
> Oh, now I see. You are absolutely right, it was just code duplication.
> I will change it and resend patch.

Try and see if *mmc can not be passed all around instead of the dev_num too.

Best regards,
Marek Vasut

Patch

diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index 33a4715..4181d3a 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -14,6 +14,7 @@ 
 int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 			       int argc, char * const argv[])
 {
+	struct mmc *mmc = NULL;
 	char *ep;
 	unsigned int dev_num = 0, offset = 0, part_size = 0;
 	int rc;
@@ -28,8 +29,10 @@  int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 
 	dev_num = (int)simple_strtoul(argv[1], &ep, 16);
 
-	if (dev_num) {
-		puts("\nSet eMMC device to 0! - e.g. ums 0\n");
+	mmc = find_mmc_device(dev_num);
+
+	if (!mmc) {
+		printf("\neMMC device: %d not found! Try ums 0.\n", dev_num);
 		goto fail;
 	}