[U-Boot] [PATCH] ums: Add "dev num" parameter. Check mmc device before do ums init.

[Przemyslaw Marczak]

Hello Marek,
Thank you for reply.

On 09/04/2013 12:26 AM, Marek Vasut wrote:
> Dear Przemyslaw Marczak,
>
>> This change allows using every mmc device instance with ums, like eMMC
>> or SD cards. Now MMC device is checked before ums is inited.
>>
>> Example of use: ums <device_number> for mmc devices.
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>> CC: Marek Vasut <marek.vasut at gmail.com>
>> ---
>>   board/samsung/trats/trats.c   |   12 +++---------
>>   common/cmd_usb_mass_storage.c |   30 ++++++++++++++----------------
>>   include/usb_mass_storage.h    |    4 ++--
>>   3 files changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
>> index 7f61d17..b7f7b05 100644
>> --- a/board/samsung/trats/trats.c
>> +++ b/board/samsung/trats/trats.c
>> @@ -816,17 +816,11 @@ static struct ums_board_info ums_board = {
>>   	},
>>   };
>>
>> -struct ums_board_info *board_ums_init(unsigned int dev_num, unsigned int
>> offset, -				      unsigned int part_size)
>> +struct ums_board_info *board_ums_init(struct mmc *mmc, unsigned int
>> offset, +					unsigned int part_size)
>>   {
>> -	struct mmc *mmc;
>> -
>> -	mmc = find_mmc_device(dev_num);
>> -	if (!mmc)
>> -		return NULL;
>> -
>>   	ums_board.ums_dev.mmc = mmc;
>> -	ums_board.ums_dev.dev_num = dev_num;
>> +	ums_board.ums_dev.dev_num = mmc->block_dev.dev;
>
> You already pass "mmc", why pass mmc->block_dev.dev too? Is it not a little
> redundant?
>

You are right, it is little redundant but pointer to this structure is 
returned so we expect that structure fields were proper filled, right?

>>   	ums_board.ums_dev.offset = offset;
>>   	ums_board.ums_dev.part_size = part_size;
>>
>> diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
>> index 33a4715..62c1308 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;
>> @@ -21,29 +22,29 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
>>   	struct ums_board_info *ums_info;
>>   	static char *s = "ums";
>>
>> -	if (argc < 2) {
>> -		printf("usage: ums <dev> - e.g. ums 0\n");
>> -		return 0;
>> -	}
>> +	if (argc < 2)
>> +		return CMD_RET_USAGE;
>
> UMS works on MMC devices only right now?
>

Yes, it is.

>>   	dev_num = (int)simple_strtoul(argv[1], &ep, 16);
>>
>> -	if (dev_num) {
>> -		puts("\nSet eMMC device to 0! - e.g. ums 0\n");
>> -		goto fail;
>> +	mmc = find_mmc_device(dev_num);
>> +	if (!mmc) {
>> +		printf("UMS error: invalid mmc device num: %d.\n", dev_num);
>> +		return CMD_RET_FAILURE;
>>   	}
>>
>>   	board_usb_init();
>> -	ums_info = board_ums_init(dev_num, offset, part_size);
>>
>> +	ums_info = board_ums_init(mmc, offset, part_size);
>>   	if (!ums_info) {
>> -		printf("MMC: %d -> NOT available\n", dev_num);
>> -		goto fail;
>> +		printf("UMS is not supported on this board.\n");
>
> puts()
>

Right, I will change it in patch v2.

>> +		return CMD_RET_FAILURE;
>>   	}
>> +
>>   	rc = fsg_init(ums_info);
>>   	if (rc) {
>>   		printf("cmd ums: fsg_init failed\n");
>> -		goto fail;
>> +		return CMD_RET_FAILURE;
>>   	}
>>
>>   	g_dnl_register(s);
>> @@ -61,13 +62,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
>>   	}
>>   exit:
>>   	g_dnl_unregister();
>> -	return 0;
>> -
>> -fail:
>> -	return -1;
>> +	return CMD_RET_SUCCESS;
>>   }
>>
>>   U_BOOT_CMD(ums, CONFIG_SYS_MAXARGS, 1, do_usb_mass_storage,
>>   	"Use the UMS [User Mass Storage]",
>> -	"ums - User Mass Storage Gadget"
>> +	"ums <dev> e.g. ums 0"
>>   );
>> diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
>> index 35cdcc3..0f94f31 100644
>> --- a/include/usb_mass_storage.h
>> +++ b/include/usb_mass_storage.h
>> @@ -34,8 +34,8 @@ extern void board_usb_init(void);
>>
>>   extern int fsg_init(struct ums_board_info *);
>>   extern void fsg_cleanup(void);
>> -extern struct ums_board_info *board_ums_init(unsigned int,
>> -					     unsigned int, unsigned int);
>> +extern struct ums_board_info *board_ums_init(struct mmc *, unsigned int,
>> +						unsigned int);
>>   extern int usb_gadget_handle_interrupts(void);
>>   extern int fsg_main_thread(void *);
>
> Best regards,
> Marek Vasut
>

Thank you,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com