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

2013-09-04 Thread 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.marc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
CC: Marek Vasut marek.va...@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 RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-09-04 Thread Marek Vasut
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.marc...@samsung.com
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  CC: Marek Vasut marek.va...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-09-04 Thread Przemyslaw Marczak

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.marc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
CC: Marek Vasut marek.va...@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,
--
Przemyslaw Marczak
Samsung RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-09-03 Thread 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.marc...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
CC: Marek Vasut marek.va...@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;
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;
 
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);
+   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 *);
 
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-09-03 Thread Marek Vasut
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.marc...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 CC: Marek Vasut marek.va...@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?

   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?

   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()

 + 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot