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@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Marek Vasut marek.vasut@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,

On 09/04/2013 02:34 PM, Marek Vasut wrote:

Dear 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@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: Marek Vasut marek.vasut@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?

Why not just remove the dev_num field ? The UMS core can retrieve that information itself.

[...]

Best regards, Marek Vasut

Hello, I'm making little UMS code refactor now. I will consider your suggestion.

Regards,