[U-Boot] [PATCH 0/4] USB: UMS: code refactoring and usage improvement

[U-Boot] [PATCH 0/4] USB: UMS: code refactoring and usage improvement

[Przemyslaw Marczak]

Hello,
Please look at my little ums refactor.

Quick summary:
- move ums initialization code to Samsung common
- introduce board definable parameters: UMS_START_BLOCK and UMS_PART_SIZE
- remove ums unnecessary code
- move ums structures to just one generic ums structure
- fix ums capacity miscalculation
- add function usb_cable_connected() which depends on CONFIG_USB_CABLE_CHECK
- add ums exit feature by ctrl+c or cable detachment

Thank you,
Przemyslaw Marczak

Przemyslaw Marczak (4):
  usb: ums: move ums code from trats to Samsung common directory
  usb: ums: code refactoring to improve reusability at other boards.
  usb: ums: fix bug in partition capacity computation.
  usb: ums: add ums exit feature by ctrl+c or by detach usb cable

 board/samsung/common/Makefile       |    1 +
 board/samsung/common/ums.c          |   75 +++++++++++++++++++++++++++++++++++
 board/samsung/trats/trats.c         |   62 -----------------------------
 common/cmd_usb_mass_storage.c       |   48 +++++++++++-----------
 drivers/usb/gadget/f_mass_storage.c |   50 +++++++++++++++--------
 drivers/usb/gadget/storage_common.c |    3 +-
 include/configs/trats.h             |    2 -
 include/usb.h                       |   10 +++++
 include/usb_mass_storage.h          |   33 ++++++++-------
 9 files changed, 163 insertions(+), 121 deletions(-)
 create mode 100644 board/samsung/common/ums.c

-- 
1.7.9.5

[U-Boot] [PATCH 1/4] usb: ums: move ums code from trats to Samsung common directory

[Przemyslaw Marczak]

UMS init was implemented in trats board file but mostly it comprises
common code. Due to that it has been moved to common/ums.c to avoid
code duplication in the future.

Changes:
- move ums initialization code from trats to common/ums.c
- remove unused CONFIG_USB_GADGET_MASS_STORAGE from trats.h

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Minkyu Kang <mk7.kang@samsung.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
---
 board/samsung/common/Makefile |    1 +
 board/samsung/common/ums.c    |   66 +++++++++++++++++++++++++++++++++++++++++
 board/samsung/trats/trats.c   |   62 --------------------------------------
 include/configs/trats.h       |    2 --
 4 files changed, 67 insertions(+), 64 deletions(-)
 create mode 100644 board/samsung/common/ums.c

diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
index ad7564c..d122169 100644
--- a/board/samsung/common/Makefile
+++ b/board/samsung/common/Makefile
@@ -11,6 +11,7 @@ LIB	= $(obj)libsamsung.o
 
 COBJS-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
 COBJS-$(CONFIG_THOR_FUNCTION) += thor.o
+COBJS-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
 
 SRCS    := $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS-y))
diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
new file mode 100644
index 0000000..506f4b5
--- /dev/null
+++ b/board/samsung/common/ums.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics
+ * Lukasz Majewski <l.majewski@samsung.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <usb_mass_storage.h>
+#include <part.h>
+
+static int ums_read_sector(struct ums_device *ums_dev,
+			   ulong start, lbaint_t blkcnt, void *buf)
+{
+	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
+					       start + ums_dev->offset, blkcnt,
+					       buf) != blkcnt)
+		return -1;
+
+	return 0;
+}
+
+static int ums_write_sector(struct ums_device *ums_dev,
+			    ulong start, lbaint_t blkcnt, const void *buf)
+{
+	if (ums_dev->mmc->block_dev.block_write(ums_dev->dev_num,
+						start + ums_dev->offset, blkcnt,
+						buf) != blkcnt)
+		return -1;
+
+	return 0;
+}
+
+static void ums_get_capacity(struct ums_device *ums_dev,
+			     long long int *capacity)
+{
+	long long int tmp_capacity;
+
+	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
+				       * SECTOR_SIZE);
+	*capacity = ums_dev->mmc->capacity - tmp_capacity;
+}
+
+static struct ums_board_info ums_board = {
+	.read_sector = ums_read_sector,
+	.write_sector = ums_write_sector,
+	.get_capacity = ums_get_capacity,
+	.name = "UMS disk",
+};
+
+struct ums_board_info *board_ums_init(unsigned int dev_num, unsigned int offset,
+				      unsigned int part_size)
+{
+	struct mmc *mmc = NULL;
+
+	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.offset = offset;
+	ums_board.ums_dev.part_size = part_size;
+
+	return &ums_board;
+}
diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index 58d925f..db5828d 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -772,65 +772,3 @@ void init_panel_info(vidinfo_t *vid)
 
 	setenv("lcdinfo", "lcd=s6e8ax0");
 }
-
-#ifdef CONFIG_USB_GADGET_MASS_STORAGE
-static int ums_read_sector(struct ums_device *ums_dev,
-			   ulong start, lbaint_t blkcnt, void *buf)
-{
-	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
-			start + ums_dev->offset, blkcnt, buf) != blkcnt)
-		return -1;
-
-	return 0;
-}
-
-static int ums_write_sector(struct ums_device *ums_dev,
-			    ulong start, lbaint_t blkcnt, const void *buf)
-{
-	if (ums_dev->mmc->block_dev.block_write(ums_dev->dev_num,
-			start + ums_dev->offset, blkcnt, buf) != blkcnt)
-		return -1;
-
-	return 0;
-}
-
-static void ums_get_capacity(struct ums_device *ums_dev,
-			     long long int *capacity)
-{
-	long long int tmp_capacity;
-
-	tmp_capacity = (long long int) ((ums_dev->offset + ums_dev->part_size)
-					* SECTOR_SIZE);
-	*capacity = ums_dev->mmc->capacity - tmp_capacity;
-}
-
-static struct ums_board_info ums_board = {
-	.read_sector = ums_read_sector,
-	.write_sector = ums_write_sector,
-	.get_capacity = ums_get_capacity,
-	.name = "TRATS UMS disk",
-	.ums_dev = {
-		.mmc = NULL,
-		.dev_num = 0,
-		.offset = 0,
-		.part_size = 0.
-	},
-};
-
-struct ums_board_info *board_ums_init(unsigned int dev_num, 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.offset = offset;
-	ums_board.ums_dev.part_size = part_size;
-
-	return &ums_board;
-}
-#endif
diff --git a/include/configs/trats.h b/include/configs/trats.h
index f5bb6aa..3e67229 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -323,9 +323,7 @@
 #define CONFIG_SYS_VIDEO_LOGO_MAX_SIZE ((500 * 120 * 4) + (1 << 12))
 
 #define CONFIG_CMD_USB_MASS_STORAGE
-#if defined(CONFIG_CMD_USB_MASS_STORAGE)
 #define CONFIG_USB_GADGET_MASS_STORAGE
-#endif
 
 /* Pass open firmware flat tree */
 #define CONFIG_OF_LIBFDT    1
-- 
1.7.9.5

[U-Boot] [PATCH 2/4] usb: ums: code refactoring to improve reusability at other boards.

[Przemyslaw Marczak]

This patch introduces some cleanups to ums code. Changes:

ums common:
- introduce UMS_START_BLOCK and UMS_PART_SIZE as defined in usb_mass_storage.h
  both default values as 0 if board config do not define them

common cleanup changes:
- change name of struct "ums_board_info" to "ums"
- "ums_device" fields are moved to struct ums and "dev_num" is removed
- change function name: board_ums_init to ums_init
- remove "extern" prefixes from usb_mass_storage.h

cmd_usb_mass_storage:
- change error() to printf() if need to print info message
- change return values to command_ret_t type at ums command code
- add command usage string

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
---
 board/samsung/common/ums.c          |   37 ++++++++++++++++++-----------------
 common/cmd_usb_mass_storage.c       |   27 +++++++++++--------------
 drivers/usb/gadget/f_mass_storage.c |   26 ++++++++++++------------
 drivers/usb/gadget/storage_common.c |    3 +--
 include/usb_mass_storage.h          |   33 +++++++++++++++++--------------
 5 files changed, 62 insertions(+), 64 deletions(-)

diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
index 506f4b5..1f28590 100644
--- a/board/samsung/common/ums.c
+++ b/board/samsung/common/ums.c
@@ -9,30 +9,33 @@
 #include <usb_mass_storage.h>
 #include <part.h>
 
-static int ums_read_sector(struct ums_device *ums_dev,
+static int ums_read_sector(struct ums *ums_dev,
 			   ulong start, lbaint_t blkcnt, void *buf)
 {
-	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
-					       start + ums_dev->offset, blkcnt,
-					       buf) != blkcnt)
+	int dev_num = ums_dev->mmc->block_dev.dev;
+
+	if (ums_dev->mmc->block_dev.block_read(dev_num,
+					       start + ums_dev->offset,
+					       blkcnt, buf) != blkcnt)
 		return -1;
 
 	return 0;
 }
 
-static int ums_write_sector(struct ums_device *ums_dev,
+static int ums_write_sector(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf)
 {
-	if (ums_dev->mmc->block_dev.block_write(ums_dev->dev_num,
-						start + ums_dev->offset, blkcnt,
-						buf) != blkcnt)
+	int dev_num = ums_dev->mmc->block_dev.dev;
+
+	if (ums_dev->mmc->block_dev.block_write(dev_num,
+						start + ums_dev->offset,
+						blkcnt, buf) != blkcnt)
 		return -1;
 
 	return 0;
 }
 
-static void ums_get_capacity(struct ums_device *ums_dev,
-			     long long int *capacity)
+static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
 {
 	long long int tmp_capacity;
 
@@ -41,15 +44,16 @@ static void ums_get_capacity(struct ums_device *ums_dev,
 	*capacity = ums_dev->mmc->capacity - tmp_capacity;
 }
 
-static struct ums_board_info ums_board = {
+static struct ums ums_dev = {
 	.read_sector = ums_read_sector,
 	.write_sector = ums_write_sector,
 	.get_capacity = ums_get_capacity,
 	.name = "UMS disk",
+	.offset = UMS_START_BLOCK,
+	.part_size = UMS_PART_SIZE,
 };
 
-struct ums_board_info *board_ums_init(unsigned int dev_num, unsigned int offset,
-				      unsigned int part_size)
+struct ums *ums_init(unsigned int dev_num)
 {
 	struct mmc *mmc = NULL;
 
@@ -57,10 +61,7 @@ struct ums_board_info *board_ums_init(unsigned int dev_num, unsigned int offset,
 	if (!mmc)
 		return NULL;
 
-	ums_board.ums_dev.mmc = mmc;
-	ums_board.ums_dev.dev_num = dev_num;
-	ums_board.ums_dev.offset = offset;
-	ums_board.ums_dev.part_size = part_size;
+	ums_dev.mmc = mmc;
 
-	return &ums_board;
+	return &ums_dev;
 }
diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index f583caf..f6ceba7 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -22,28 +22,26 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 
 	unsigned int dev_num = (unsigned int)(simple_strtoul(mmc_devstring,
 				NULL, 0));
-	if (dev_num) {
-		error("Set eMMC device to 0! - e.g. ums 0");
-		goto fail;
-	}
+	if (dev_num)
+		return CMD_RET_USAGE;
 
 	unsigned int controller_index = (unsigned int)(simple_strtoul(
 					usb_controller,	NULL, 0));
 	if (board_usb_init(controller_index, USB_INIT_DEVICE)) {
 		error("Couldn't init USB controller.");
-		goto fail;
+		return CMD_RET_FAILURE;
 	}
 
-	struct ums_board_info *ums_info = board_ums_init(dev_num, 0, 0);
-	if (!ums_info) {
-		error("MMC: %d -> NOT available", dev_num);
-		goto fail;
+	struct ums *ums = ums_init(dev_num);
+	if (!ums) {
+		printf("MMC: %u no such device\n", dev_num);
+		return CMD_RET_FAILURE;
 	}
 
-	int rc = fsg_init(ums_info);
+	int rc = fsg_init(ums);
 	if (rc) {
 		error("fsg_init failed");
-		goto fail;
+		return CMD_RET_FAILURE;
 	}
 
 	g_dnl_register("ums");
@@ -62,13 +60,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]",
-	"<USB_controller> <mmc_dev>"
+	"ums <USB_controller> <mmc_dev>  e.g. ums 0 0"
 );
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index b34068a..9560deb 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -444,7 +444,7 @@ static void set_bulk_out_req_length(struct fsg_common *common,
 
 /*-------------------------------------------------------------------------*/
 
-struct ums_board_info			*ums_info;
+struct ums *ums;
 struct fsg_common *the_fsg_common;
 
 static int fsg_set_halt(struct fsg_dev *fsg, struct usb_ep *ep)
@@ -761,10 +761,10 @@ static int do_read(struct fsg_common *common)
 
 		/* Perform the read */
 		nread = 0;
-		rc = ums_info->read_sector(&(ums_info->ums_dev),
-					   file_offset / SECTOR_SIZE,
-					   amount / SECTOR_SIZE,
-					   (char __user *)bh->buf);
+		rc = ums->read_sector(ums,
+				      file_offset / SECTOR_SIZE,
+				      amount / SECTOR_SIZE,
+				      (char __user *)bh->buf);
 		if (rc)
 			return -EIO;
 		nread = amount;
@@ -934,7 +934,7 @@ static int do_write(struct fsg_common *common)
 			amount = bh->outreq->actual;
 
 			/* Perform the write */
-			rc = ums_info->write_sector(&(ums_info->ums_dev),
+			rc = ums->write_sector(ums,
 					       file_offset / SECTOR_SIZE,
 					       amount / SECTOR_SIZE,
 					       (char __user *)bh->buf);
@@ -1049,10 +1049,10 @@ static int do_verify(struct fsg_common *common)
 
 		/* Perform the read */
 		nread = 0;
-		rc = ums_info->read_sector(&(ums_info->ums_dev),
-					   file_offset / SECTOR_SIZE,
-					   amount / SECTOR_SIZE,
-					   (char __user *)bh->buf);
+		rc = ums->read_sector(ums,
+				      file_offset / SECTOR_SIZE,
+				      amount / SECTOR_SIZE,
+				      (char __user *)bh->buf);
 		if (rc)
 			return -EIO;
 		nread = amount;
@@ -1103,7 +1103,7 @@ static int do_inquiry(struct fsg_common *common, struct fsg_buffhd *bh)
 	buf[4] = 31;		/* Additional length */
 				/* No special options */
 	sprintf((char *) (buf + 8), "%-8s%-16s%04x", (char*) vendor_id ,
-			ums_info->name, (u16) 0xffff);
+			ums->name, (u16) 0xffff);
 
 	return 36;
 }
@@ -2758,9 +2758,9 @@ int fsg_add(struct usb_configuration *c)
 	return fsg_bind_config(c->cdev, c, fsg_common);
 }
 
-int fsg_init(struct ums_board_info *ums)
+int fsg_init(struct ums *ums_dev)
 {
-	ums_info = ums;
+	ums = ums_dev;
 
 	return 0;
 }
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 866e7c7..c2c5424 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -275,7 +275,6 @@ struct rw_semaphore { int i; };
 #define ETOOSMALL	525
 
 #include <usb_mass_storage.h>
-extern struct ums_board_info		*ums_info;
 
 /*-------------------------------------------------------------------------*/
 
@@ -581,7 +580,7 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
 	/* R/W if we can, R/O if we must */
 	ro = curlun->initially_ro;
 
-	ums_info->get_capacity(&(ums_info->ums_dev), &size);
+	ums->get_capacity(ums, &size);
 	if (size < 0) {
 		printf("unable to find file size: %s\n", filename);
 		rc = (int) size;
diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
index 13f535c..292e471 100644
--- a/include/usb_mass_storage.h
+++ b/include/usb_mass_storage.h
@@ -9,32 +9,35 @@
 #define __USB_MASS_STORAGE_H__
 
 #define SECTOR_SIZE		0x200
-
 #include <mmc.h>
 #include <linux/usb/composite.h>
 
-struct ums_device {
-	struct mmc *mmc;
-	int dev_num;
-	int offset;
-	int part_size;
-};
+#ifndef UMS_START_BLOCK
+#define UMS_START_BLOCK		0
+#endif
 
-struct ums_board_info {
-	int (*read_sector)(struct ums_device *ums_dev,
+#ifndef UMS_PART_SIZE
+#define UMS_PART_SIZE		0
+#endif
+
+struct ums {
+	int (*read_sector)(struct ums *ums_dev,
 			   ulong start, lbaint_t blkcnt, void *buf);
-	int (*write_sector)(struct ums_device *ums_dev,
+	int (*write_sector)(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf);
-	void (*get_capacity)(struct ums_device *ums_dev,
+	void (*get_capacity)(struct ums *ums_dev,
 			     long long int *capacity);
 	const char *name;
-	struct ums_device ums_dev;
+	struct mmc *mmc;
+	int offset;
+	int part_size;
 };
 
-int fsg_init(struct ums_board_info *);
+extern struct ums *ums;
+
+int fsg_init(struct ums *);
 void fsg_cleanup(void);
-struct ums_board_info *board_ums_init(unsigned int, unsigned int,
-				      unsigned int);
+struct ums *ums_init(unsigned int);
 int fsg_main_thread(void *);
 
 #ifdef CONFIG_USB_GADGET_MASS_STORAGE
-- 
1.7.9.5

[U-Boot] [PATCH 3/4] usb: ums: fix bug in partition capacity computation.

[Przemyslaw Marczak]

Before this change ums disk capacity was miscalculated because
of integer overflow.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
---
 board/samsung/common/ums.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
index 1f28590..6c4e6c4 100644
--- a/board/samsung/common/ums.c
+++ b/board/samsung/common/ums.c
@@ -37,11 +37,19 @@ static int ums_write_sector(struct ums *ums_dev,
 
 static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
 {
-	long long int tmp_capacity;
+	int64_t mmc_capacity = (int64_t)ums_dev->mmc->capacity;
+	int64_t ums_capacity = (int64_t)ums_dev->part_size * SECTOR_SIZE;
+	int64_t ums_offset = (int64_t)ums_dev->offset * SECTOR_SIZE;
 
-	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
-				       * SECTOR_SIZE);
-	*capacity = ums_dev->mmc->capacity - tmp_capacity;
+	if (ums_capacity && ((ums_capacity + ums_offset) < mmc_capacity))
+		*capacity = ums_capacity;
+	else
+		*capacity = mmc_capacity - ums_offset;
+
+	printf("UMS: partition capacity: %#llx blocks\n"
+	       "UMS: partition start block: %#x\n",
+	       *capacity / SECTOR_SIZE,
+	       ums_dev->offset);
 }
 
 static struct ums ums_dev = {
-- 
1.7.9.5

[U-Boot] [PATCH 4/4] usb: ums: add ums exit feature by ctrl+c or by detach usb cable

[Przemyslaw Marczak]

This patch allows exiting from UMS mode to u-boot prompt
by detaching usb cable or by pressing ctrl+c.

Add new config: CONFIG_USB_CABLE_CHECK. If defined then board
file should provide function: usb_cable_connected() (include/usb.h)
that return 1 if cable is connected and 0 otherwise.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
---
 common/cmd_usb_mass_storage.c       |   21 +++++++++++++--------
 drivers/usb/gadget/f_mass_storage.c |   24 +++++++++++++++++++++---
 include/usb.h                       |   10 ++++++++++
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index f6ceba7..9224d3c 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -5,6 +5,7 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <errno.h>
 #include <common.h>
 #include <command.h>
 #include <g_dnl.h>
@@ -47,16 +48,20 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 	g_dnl_register("ums");
 
 	while (1) {
-		/* Handle control-c and timeouts */
-		if (ctrlc()) {
-			error("The remote end did not respond in time.");
-			goto exit;
-		}
-
 		usb_gadget_handle_interrupts();
-		/* Check if USB cable has been detached */
-		if (fsg_main_thread(NULL) == EIO)
+
+		rc = fsg_main_thread(NULL);
+		if (rc) {
+			/* Check I/O error */
+			if (rc == -EIO)
+				printf("\rCheck USB cable connection\n");
+
+			/* Check CTRL+C */
+			if (rc == -EPIPE)
+				printf("\rCTRL+C - Operation aborted\n");
+
 			goto exit;
+		}
 	}
 exit:
 	g_dnl_unregister();
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 9560deb..a49572d 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -245,6 +245,7 @@
 #include <config.h>
 #include <malloc.h>
 #include <common.h>
+#include <usb.h>
 
 #include <linux/err.h>
 #include <linux/usb/ch9.h>
@@ -678,6 +679,18 @@ static int sleep_thread(struct fsg_common *common)
 			k++;
 		}
 
+		if (k == 10) {
+			/* Handle CTRL+C */
+			if (ctrlc())
+				return -EPIPE;
+#ifdef CONFIG_USB_CABLE_CHECK
+			/* Check cable connection */
+			if (!usb_cable_connected())
+				return -EIO;
+#endif
+			k = 0;
+		}
+
 		usb_gadget_handle_interrupts();
 	}
 	common->thread_wakeup_needed = 0;
@@ -2389,6 +2402,7 @@ static void handle_exception(struct fsg_common *common)
 
 int fsg_main_thread(void *common_)
 {
+	int ret;
 	struct fsg_common	*common = the_fsg_common;
 	/* The main loop */
 	do {
@@ -2398,12 +2412,16 @@ int fsg_main_thread(void *common_)
 		}
 
 		if (!common->running) {
-			sleep_thread(common);
+			ret = sleep_thread(common);
+			if (ret)
+				return ret;
+
 			continue;
 		}
 
-		if (get_next_command(common))
-			continue;
+		ret = get_next_command(common);
+		if (ret)
+			return ret;
 
 		if (!exception_in_progress(common))
 			common->state = FSG_STATE_DATA_PHASE;
diff --git a/include/usb.h b/include/usb.h
index 17fb68c..8c1789f 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -197,6 +197,16 @@ int board_usb_init(int index, enum board_usb_init_type init);
  */
 int board_usb_cleanup(int index, enum board_usb_init_type init);
 
+/*
+ * If CONFIG_USB_CABLE_CHECK is set then this function
+ * should be defined in board file.
+ *
+ * @return 1 if cable is connected and 0 otherwise.
+ */
+#ifdef CONFIG_USB_CABLE_CHECK
+int usb_cable_connected(void);
+#endif
+
 #ifdef CONFIG_USB_STORAGE
 
 #define USB_MAX_STOR_DEV 5
-- 
1.7.9.5

[U-Boot] [PATCH 1/4] usb: ums: move ums code from trats to Samsung common directory

[Marek Vasut]

Dear Przemyslaw Marczak,

> UMS init was implemented in trats board file but mostly it comprises
> common code. Due to that it has been moved to common/ums.c to avoid
> code duplication in the future.
> 
> Changes:
> - move ums initialization code from trats to common/ums.c
> - remove unused CONFIG_USB_GADGET_MASS_STORAGE from trats.h
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  board/samsung/common/Makefile |    1 +
>  board/samsung/common/ums.c    |   66
> +++++++++++++++++++++++++++++++++++++++++ board/samsung/trats/trats.c   | 
>  62 -------------------------------------- include/configs/trats.h       |
>    2 --
>  4 files changed, 67 insertions(+), 64 deletions(-)
>  create mode 100644 board/samsung/common/ums.c
> 
> diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
> index ad7564c..d122169 100644
> --- a/board/samsung/common/Makefile
> +++ b/board/samsung/common/Makefile
> @@ -11,6 +11,7 @@ LIB	= $(obj)libsamsung.o
> 
>  COBJS-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>  COBJS-$(CONFIG_THOR_FUNCTION) += thor.o
> +COBJS-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
> 
>  SRCS    := $(COBJS-y:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS-y))
> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
> new file mode 100644
> index 0000000..506f4b5
> --- /dev/null
> +++ b/board/samsung/common/ums.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2013 Samsung Electronics
> + * Lukasz Majewski <l.majewski@samsung.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <usb_mass_storage.h>
> +#include <part.h>
> +
> +static int ums_read_sector(struct ums_device *ums_dev,
> +			   ulong start, lbaint_t blkcnt, void *buf)
> +{
> +	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
> +					       start + ums_dev->offset, blkcnt,
> +					       buf) != blkcnt)

This looks like hell.

typeT block_dev = ums_dev->mmc;
int ret;

ret = block_dev->block_read(....);

return ret;

Is it necessary to return -1? Why ?

Please fix the whole thing in-place first, then rebase this migration patch on 
top of the fix.

[U-Boot] [PATCH 3/4] usb: ums: fix bug in partition capacity computation.

[Marek Vasut]

Dear Przemyslaw Marczak,

> Before this change ums disk capacity was miscalculated because
> of integer overflow.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  board/samsung/common/ums.c |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
> index 1f28590..6c4e6c4 100644
> --- a/board/samsung/common/ums.c
> +++ b/board/samsung/common/ums.c
> @@ -37,11 +37,19 @@ static int ums_write_sector(struct ums *ums_dev,
> 
>  static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
>  {
> -	long long int tmp_capacity;
> +	int64_t mmc_capacity = (int64_t)ums_dev->mmc->capacity;

Why are these casts here?

> +	int64_t ums_capacity = (int64_t)ums_dev->part_size * SECTOR_SIZE;
> +	int64_t ums_offset = (int64_t)ums_dev->offset * SECTOR_SIZE;

And here all around? And why are these values signed, can there ever be negative 
value in them?

> -	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
> -				       * SECTOR_SIZE);
> -	*capacity = ums_dev->mmc->capacity - tmp_capacity;
> +	if (ums_capacity && ((ums_capacity + ums_offset) < mmc_capacity))
> +		*capacity = ums_capacity;
> +	else
> +		*capacity = mmc_capacity - ums_offset;

Urgh, what exactly does this code achieve again?

> +	printf("UMS: partition capacity: %#llx blocks\n"
> +	       "UMS: partition start block: %#x\n",
> +	       *capacity / SECTOR_SIZE,
> +	       ums_dev->offset);
>  }
> 
>  static struct ums ums_dev = {

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/4] usb: ums: add ums exit feature by ctrl+c or by detach usb cable

[Marek Vasut]

Dear Przemyslaw Marczak,

> This patch allows exiting from UMS mode to u-boot prompt
> by detaching usb cable or by pressing ctrl+c.
> 
> Add new config: CONFIG_USB_CABLE_CHECK. If defined then board
> file should provide function: usb_cable_connected() (include/usb.h)
> that return 1 if cable is connected and 0 otherwise.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> ---

[...]

> @@ -678,6 +679,18 @@ static int sleep_thread(struct fsg_common *common)
>  			k++;
>  		}
> 
> +		if (k == 10) {
> +			/* Handle CTRL+C */
> +			if (ctrlc())
> +				return -EPIPE;
> +#ifdef CONFIG_USB_CABLE_CHECK

Please document this newly added option in README.

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/4] usb: ums: move ums code from trats to Samsung common directory

[Przemyslaw Marczak]

Hello Marek,
Thank you for fast reply.

On 10/17/2013 07:39 PM, Marek Vasut wrote:
> Dear Przemyslaw Marczak,
>
>> UMS init was implemented in trats board file but mostly it comprises
>> common code. Due to that it has been moved to common/ums.c to avoid
>> code duplication in the future.
>>
>> Changes:
>> - move ums initialization code from trats to common/ums.c
>> - remove unused CONFIG_USB_GADGET_MASS_STORAGE from trats.h
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Minkyu Kang <mk7.kang@samsung.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> ---
>>   board/samsung/common/Makefile |    1 +
>>   board/samsung/common/ums.c    |   66
>> +++++++++++++++++++++++++++++++++++++++++ board/samsung/trats/trats.c   |
>>   62 -------------------------------------- include/configs/trats.h       |
>>     2 --
>>   4 files changed, 67 insertions(+), 64 deletions(-)
>>   create mode 100644 board/samsung/common/ums.c
>>
>> diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
>> index ad7564c..d122169 100644
>> --- a/board/samsung/common/Makefile
>> +++ b/board/samsung/common/Makefile
>> @@ -11,6 +11,7 @@ LIB	= $(obj)libsamsung.o
>>
>>   COBJS-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>   COBJS-$(CONFIG_THOR_FUNCTION) += thor.o
>> +COBJS-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>>
>>   SRCS    := $(COBJS-y:.o=.c)
>>   OBJS	:= $(addprefix $(obj),$(COBJS-y))
>> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
>> new file mode 100644
>> index 0000000..506f4b5
>> --- /dev/null
>> +++ b/board/samsung/common/ums.c
>> @@ -0,0 +1,66 @@
>> +/*
>> + * Copyright (C) 2013 Samsung Electronics
>> + * Lukasz Majewski <l.majewski@samsung.com>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <usb_mass_storage.h>
>> +#include <part.h>
>> +
>> +static int ums_read_sector(struct ums_device *ums_dev,
>> +			   ulong start, lbaint_t blkcnt, void *buf)
>> +{
>> +	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
>> +					       start + ums_dev->offset, blkcnt,
>> +					       buf) != blkcnt)
>
> This looks like hell.
>
> typeT block_dev = ums_dev->mmc;
> int ret;
>
> ret = block_dev->block_read(....);
>
> return ret;

Ok, you're right - I will fix it in next patch set.

>
> Is it necessary to return -1? Why ?
>

It's only because of ums gadged driver design but it is easy to simplify.

> Please fix the whole thing in-place first, then rebase this migration patch on
> top of the fix.
>

OK, migration patch will be at the top.

And one more. Do you want me to make it applicable to usb-next or 
usb-master tree?

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

[U-Boot] [PATCH 1/4] usb: ums: move ums code from trats to Samsung common directory

[Marek Vasut]

Dear Przemyslaw Marczak,

> Hello Marek,
> Thank you for fast reply.
> 
> On 10/17/2013 07:39 PM, Marek Vasut wrote:
> > Dear Przemyslaw Marczak,
> > 
> >> UMS init was implemented in trats board file but mostly it comprises
> >> common code. Due to that it has been moved to common/ums.c to avoid
> >> code duplication in the future.
> >> 
> >> Changes:
> >> - move ums initialization code from trats to common/ums.c
> >> - remove unused CONFIG_USB_GADGET_MASS_STORAGE from trats.h
> >> 
> >> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> Cc: Minkyu Kang <mk7.kang@samsung.com>
> >> Cc: Lukasz Majewski <l.majewski@samsung.com>
> >> ---
> >> 
> >>   board/samsung/common/Makefile |    1 +
> >>   board/samsung/common/ums.c    |   66
> >> 
> >> +++++++++++++++++++++++++++++++++++++++++ board/samsung/trats/trats.c  
> >> |
> >> 
> >>   62 -------------------------------------- include/configs/trats.h     
> >>    |
> >>   
> >>     2 --
> >>   
> >>   4 files changed, 67 insertions(+), 64 deletions(-)
> >>   create mode 100644 board/samsung/common/ums.c
> >> 
> >> diff --git a/board/samsung/common/Makefile
> >> b/board/samsung/common/Makefile index ad7564c..d122169 100644
> >> --- a/board/samsung/common/Makefile
> >> +++ b/board/samsung/common/Makefile
> >> @@ -11,6 +11,7 @@ LIB	= $(obj)libsamsung.o
> >> 
> >>   COBJS-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
> >>   COBJS-$(CONFIG_THOR_FUNCTION) += thor.o
> >> 
> >> +COBJS-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
> >> 
> >>   SRCS    := $(COBJS-y:.o=.c)
> >>   OBJS	:= $(addprefix $(obj),$(COBJS-y))
> >> 
> >> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
> >> new file mode 100644
> >> index 0000000..506f4b5
> >> --- /dev/null
> >> +++ b/board/samsung/common/ums.c
> >> @@ -0,0 +1,66 @@
> >> +/*
> >> + * Copyright (C) 2013 Samsung Electronics
> >> + * Lukasz Majewski <l.majewski@samsung.com>
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <usb_mass_storage.h>
> >> +#include <part.h>
> >> +
> >> +static int ums_read_sector(struct ums_device *ums_dev,
> >> +			   ulong start, lbaint_t blkcnt, void *buf)
> >> +{
> >> +	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
> >> +					       start + ums_dev->offset, blkcnt,
> >> +					       buf) != blkcnt)
> > 
> > This looks like hell.
> > 
> > typeT block_dev = ums_dev->mmc;
> > int ret;
> > 
> > ret = block_dev->block_read(....);
> > 
> > return ret;
> 
> Ok, you're right - I will fix it in next patch set.
> 
> > Is it necessary to return -1? Why ?
> 
> It's only because of ums gadged driver design but it is easy to simplify.

A proper errno.h patch would be good here.

> > Please fix the whole thing in-place first, then rebase this migration
> > patch on top of the fix.
> 
> OK, migration patch will be at the top.
> 
> And one more. Do you want me to make it applicable to usb-next or
> usb-master tree?

-next please.

[U-Boot] [PATCH 3/4] usb: ums: fix bug in partition capacity computation.

[Przemyslaw Marczak]

Hi Marek,

On 10/17/2013 07:41 PM, Marek Vasut wrote:
> Dear Przemyslaw Marczak,
>
>> Before this change ums disk capacity was miscalculated because
>> of integer overflow.
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>   board/samsung/common/ums.c |   16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
>> index 1f28590..6c4e6c4 100644
>> --- a/board/samsung/common/ums.c
>> +++ b/board/samsung/common/ums.c
>> @@ -37,11 +37,19 @@ static int ums_write_sector(struct ums *ums_dev,
>>
>>   static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
>>   {
>> -	long long int tmp_capacity;
>> +	int64_t mmc_capacity = (int64_t)ums_dev->mmc->capacity;
>
> Why are these casts here?
>
>> +	int64_t ums_capacity = (int64_t)ums_dev->part_size * SECTOR_SIZE;
>> +	int64_t ums_offset = (int64_t)ums_dev->offset * SECTOR_SIZE;
>
> And here all around? And why are these values signed, can there ever be negative
> value in them?
>

I tried to fix it without changes in ums driver because it works fine. 
Of course capacity can't be a negative value.

When we set some offset and some part size we have an integer overflow 
at this line, just before cast to long long int:
>> -	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
>> -				       * SECTOR_SIZE);
>> -	*capacity = ums_dev->mmc->capacity - tmp_capacity;
In the best case of overflow - ums partition capacity will have the same 
value as mmc cap, but if offset was set, then the partition size will be 
exceeded.

>> +	if (ums_capacity && ((ums_capacity + ums_offset) < mmc_capacity))
>> +		*capacity = ums_capacity;
>> +	else
>> +		*capacity = mmc_capacity - ums_offset;
>
> Urgh, what exactly does this code achieve again?

This code above avoids situation when tmp_capacity value is bigger  than 
real mmc capacity. I don't check next the offset but this is also the 
reason why I put printf here. I assume that developer should know how to 
define UMS_START_BLOCK and UMS_PART_SIZE if no, some information will be 
printed.

>
>> +	printf("UMS: partition capacity: %#llx blocks\n"
>> +	       "UMS: partition start block: %#x\n",
>> +	       *capacity / SECTOR_SIZE,
>> +	       ums_dev->offset);
>>   }
>>
>>   static struct ums ums_dev = {
>
> Best regards,
> Marek Vasut
>

In summary I will change signed variables to unsigned here and few in 
the ums gadget driver.
Moreover now I think that it will be better to replace part_size from 
the struct ums_dev with part_blk_num and compute its value at ums_init 
function. And then pointer to ums_get_capacity is not needed in ums 
structure.

What do you think about this?

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

[U-Boot] [PATCH 3/4] usb: ums: fix bug in partition capacity computation.

[Marek Vasut]

Dear Przemyslaw Marczak,

> Hi Marek,
> 
> On 10/17/2013 07:41 PM, Marek Vasut wrote:
> > Dear Przemyslaw Marczak,
> > 
> >> Before this change ums disk capacity was miscalculated because
> >> of integer overflow.
> >> 
> >> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> ---
> >> 
> >>   board/samsung/common/ums.c |   16 ++++++++++++----
> >>   1 file changed, 12 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
> >> index 1f28590..6c4e6c4 100644
> >> --- a/board/samsung/common/ums.c
> >> +++ b/board/samsung/common/ums.c
> >> @@ -37,11 +37,19 @@ static int ums_write_sector(struct ums *ums_dev,
> >> 
> >>   static void ums_get_capacity(struct ums *ums_dev, long long int
> >>   *capacity) {
> >> 
> >> -	long long int tmp_capacity;
> >> +	int64_t mmc_capacity = (int64_t)ums_dev->mmc->capacity;
> > 
> > Why are these casts here?
> > 
> >> +	int64_t ums_capacity = (int64_t)ums_dev->part_size * SECTOR_SIZE;
> >> +	int64_t ums_offset = (int64_t)ums_dev->offset * SECTOR_SIZE;
> > 
> > And here all around? And why are these values signed, can there ever be
> > negative value in them?
> 
> I tried to fix it without changes in ums driver because it works fine.
> Of course capacity can't be a negative value.
> 
> When we set some offset and some part size we have an integer overflow
> 
> at this line, just before cast to long long int:
> >> -	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
> >> -				       * SECTOR_SIZE);
> >> -	*capacity = ums_dev->mmc->capacity - tmp_capacity;
> 
> In the best case of overflow - ums partition capacity will have the same
> value as mmc cap, but if offset was set, then the partition size will be
> exceeded.
> 
> >> +	if (ums_capacity && ((ums_capacity + ums_offset) < mmc_capacity))
> >> +		*capacity = ums_capacity;
> >> +	else
> >> +		*capacity = mmc_capacity - ums_offset;
> > 
> > Urgh, what exactly does this code achieve again?
> 
> This code above avoids situation when tmp_capacity value is bigger  than
> real mmc capacity. I don't check next the offset but this is also the
> reason why I put printf here. I assume that developer should know how to
> define UMS_START_BLOCK and UMS_PART_SIZE if no, some information will be
> printed.
> 
> >> +	printf("UMS: partition capacity: %#llx blocks\n"
> >> +	       "UMS: partition start block: %#x\n",
> >> +	       *capacity / SECTOR_SIZE,
> >> +	       ums_dev->offset);
> >> 
> >>   }
> >>   
> >>   static struct ums ums_dev = {
> > 
> > Best regards,
> > Marek Vasut
> 
> In summary I will change signed variables to unsigned here and few in
> the ums gadget driver.
> Moreover now I think that it will be better to replace part_size from
> the struct ums_dev with part_blk_num and compute its value at ums_init
> function. And then pointer to ums_get_capacity is not needed in ums
> structure.
> 
> What do you think about this?

I think the first screaming thing here is ... why is this all multiplied by 
SECTOR_SIZE before doing the comparisons and stuffs ? You can do that later 
(that does mean do it later, yes).

Try this:

u64 mmc_cap = ums_dev->mmc->capacity / SECTOR_SIZE;
u64 ums_start = ums_dev->offset;
u64 ums_end = ums_start + ums_dev->part_size;

/* Start past MMC size. */
if (ums_start >= mmc_cap)
	return -EINVAL;

/* End past MMC size. */
if (ums_end > mmc_cap) {
	puts("UMS region larger than MMC device, capping\n");
	ums_end = mmc_cap;
}

*capacity = (ums_end - ums_start) * SECTOR_SIZE;

Does this work? You'd need to add debug.

[U-Boot] [PATCH 3/4] usb: ums: fix bug in partition capacity computation.

[Przemyslaw Marczak]

Hello Marek,

On 10/19/2013 02:57 AM, Marek Vasut wrote:
> Dear Przemyslaw Marczak,
>
>> Hi Marek,
>>
>> On 10/17/2013 07:41 PM, Marek Vasut wrote:
>>> Dear Przemyslaw Marczak,
>>>
>>>> Before this change ums disk capacity was miscalculated because
>>>> of integer overflow.
>>>>
>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> ---
>>>>
>>>>    board/samsung/common/ums.c |   16 ++++++++++++----
>>>>    1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
>>>> index 1f28590..6c4e6c4 100644
>>>> --- a/board/samsung/common/ums.c
>>>> +++ b/board/samsung/common/ums.c
>>>> @@ -37,11 +37,19 @@ static int ums_write_sector(struct ums *ums_dev,
>>>>
>>>>    static void ums_get_capacity(struct ums *ums_dev, long long int
>>>>    *capacity) {
>>>>
>>>> -	long long int tmp_capacity;
>>>> +	int64_t mmc_capacity = (int64_t)ums_dev->mmc->capacity;
>>>
>>> Why are these casts here?
>>>
>>>> +	int64_t ums_capacity = (int64_t)ums_dev->part_size * SECTOR_SIZE;
>>>> +	int64_t ums_offset = (int64_t)ums_dev->offset * SECTOR_SIZE;
>>>
>>> And here all around? And why are these values signed, can there ever be
>>> negative value in them?
>>
>> I tried to fix it without changes in ums driver because it works fine.
>> Of course capacity can't be a negative value.
>>
>> When we set some offset and some part size we have an integer overflow
>>
>> at this line, just before cast to long long int:
>>>> -	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
>>>> -				       * SECTOR_SIZE);
>>>> -	*capacity = ums_dev->mmc->capacity - tmp_capacity;
>>
>> In the best case of overflow - ums partition capacity will have the same
>> value as mmc cap, but if offset was set, then the partition size will be
>> exceeded.
>>
>>>> +	if (ums_capacity && ((ums_capacity + ums_offset) < mmc_capacity))
>>>> +		*capacity = ums_capacity;
>>>> +	else
>>>> +		*capacity = mmc_capacity - ums_offset;
>>>
>>> Urgh, what exactly does this code achieve again?
>>
>> This code above avoids situation when tmp_capacity value is bigger  than
>> real mmc capacity. I don't check next the offset but this is also the
>> reason why I put printf here. I assume that developer should know how to
>> define UMS_START_BLOCK and UMS_PART_SIZE if no, some information will be
>> printed.
>>
>>>> +	printf("UMS: partition capacity: %#llx blocks\n"
>>>> +	       "UMS: partition start block: %#x\n",
>>>> +	       *capacity / SECTOR_SIZE,
>>>> +	       ums_dev->offset);
>>>>
>>>>    }
>>>>
>>>>    static struct ums ums_dev = {
>>>
>>> Best regards,
>>> Marek Vasut
>>
>> In summary I will change signed variables to unsigned here and few in
>> the ums gadget driver.
>> Moreover now I think that it will be better to replace part_size from
>> the struct ums_dev with part_blk_num and compute its value at ums_init
>> function. And then pointer to ums_get_capacity is not needed in ums
>> structure.
>>
>> What do you think about this?
>
> I think the first screaming thing here is ... why is this all multiplied by
> SECTOR_SIZE before doing the comparisons and stuffs ? You can do that later
> (that does mean do it later, yes).

You're right, but actually we don't need to use real card capacity but 
only sector count. Patch v2 will include this.

>
> Try this:
>
> u64 mmc_cap = ums_dev->mmc->capacity / SECTOR_SIZE;
> u64 ums_start = ums_dev->offset;
> u64 ums_end = ums_start + ums_dev->part_size;
>
> /* Start past MMC size. */
> if (ums_start >= mmc_cap)
> 	return -EINVAL;
>
> /* End past MMC size. */
> if (ums_end > mmc_cap) {
> 	puts("UMS region larger than MMC device, capping\n");
> 	ums_end = mmc_cap;
> }
>
> *capacity = (ums_end - ums_start) * SECTOR_SIZE;
>
> Does this work? You'd need to add debug.
>

It will only work if UMS_PART_SIZE and UMS_START_BLOCK are set 
correctly. In default case when both values are defined as 0 - function 
returns null capacity but we don't want this.

Patch v2 will include cases for default, valid and bad definitions of 
UMS_PART_SIZE and UMS_START_BLOCK. I will also remove unnecessary code 
around capacity validation from ums gadget driver.
Next patch set will be send soon.

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

[U-Boot] [PATCH v2 0/5] USB: UMS: code refactoring and usage improvement

[Przemyslaw Marczak]

Quick summary:
- introduce board definable parameters: UMS_START_BLOCK and UMS_PART_SIZE
- remove ums unnecessary code
- move ums structures to just one generic ums structure
- add mmc device num as one of ums parameter
- fix ums capacity miscalculation
- move ums initialization code to Samsung common
- add function usb_cable_connected() which depends on CONFIG_USB_CABLE_CHECK
- add ums exit feature by ctrl+c or cable detachment

Thank you,
Przemyslaw Marczak

Przemyslaw Marczak (5):
  usb: ums: code refactoring to improve reusability on other boards.
  usb: ums: allows using every mmc device with ums.
  usb: ums: fix disk capacity miscalculation and code cleanup
  usb: ums: move ums code from trats to Samsung common directory
  usb: ums: add ums exit feature by ctrl+c or by detach usb cable

 README                              |    7 ++++
 board/samsung/common/Makefile       |    1 +
 board/samsung/common/ums.c          |   76 +++++++++++++++++++++++++++++++++++
 board/samsung/trats/trats.c         |   62 ----------------------------
 common/cmd_usb_mass_storage.c       |   51 +++++++++++------------
 drivers/usb/gadget/f_mass_storage.c |   67 +++++++++++++++++++-----------
 drivers/usb/gadget/storage_common.c |   27 ++-----------
 include/configs/trats.h             |    2 -
 include/usb.h                       |   10 +++++
 include/usb_mass_storage.h          |   33 +++++++--------
 10 files changed, 180 insertions(+), 156 deletions(-)
 create mode 100644 board/samsung/common/ums.c

-- 
1.7.9.5

[U-Boot] [PATCH v2 1/5] usb: ums: code refactoring to improve reusability on other boards.

[Przemyslaw Marczak]

This patch introduces some cleanups to ums code. Changes:

ums common:
- introduce UMS_START_SECTOR and UMS_NUM_SECTORS as defined in
  usb_mass_storage.h both default values as 0 if board config
  doesn't define them

common cleanup changes:
- change name of struct "ums_board_info" to "ums"
- "ums_device" fields are moved to struct ums and "dev_num" is removed
- change function name: board_ums_init to ums_init
- remove "extern" prefixes from usb_mass_storage.h

cmd_usb_mass_storage:
- change error() to printf() if need to print info message
- change return values to command_ret_t type at ums command code
- add command usage string

Changes v2:
ums common:
- always returns number of read/write sectors
- coding style clean-up
ums gadget:
- calculate amount of read/write from device returned value.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
---
 board/samsung/trats/trats.c         |   51 +++++++++++++++--------------------
 common/cmd_usb_mass_storage.c       |   27 ++++++++-----------
 drivers/usb/gadget/f_mass_storage.c |   43 ++++++++++++++---------------
 drivers/usb/gadget/storage_common.c |    3 +--
 include/usb_mass_storage.h          |   33 ++++++++++++-----------
 5 files changed, 73 insertions(+), 84 deletions(-)

diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index 58d925f..0b58b00 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -774,63 +774,54 @@ void init_panel_info(vidinfo_t *vid)
 }
 
 #ifdef CONFIG_USB_GADGET_MASS_STORAGE
-static int ums_read_sector(struct ums_device *ums_dev,
+static int ums_read_sector(struct ums *ums_dev,
 			   ulong start, lbaint_t blkcnt, void *buf)
 {
-	if (ums_dev->mmc->block_dev.block_read(ums_dev->dev_num,
-			start + ums_dev->offset, blkcnt, buf) != blkcnt)
-		return -1;
+	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
+	lbaint_t blkstart = start + ums_dev->offset;
+	int dev_num = block_dev->dev;
 
-	return 0;
+	return block_dev->block_read(dev_num, blkstart, blkcnt, buf);
 }
 
-static int ums_write_sector(struct ums_device *ums_dev,
+static int ums_write_sector(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf)
 {
-	if (ums_dev->mmc->block_dev.block_write(ums_dev->dev_num,
-			start + ums_dev->offset, blkcnt, buf) != blkcnt)
-		return -1;
+	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
+	lbaint_t blkstart = start + ums_dev->offset;
+	int dev_num = block_dev->dev;
 
-	return 0;
+	return block_dev->block_write(dev_num, blkstart, blkcnt, buf);
 }
 
-static void ums_get_capacity(struct ums_device *ums_dev,
-			     long long int *capacity)
+static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
 {
 	long long int tmp_capacity;
 
-	tmp_capacity = (long long int) ((ums_dev->offset + ums_dev->part_size)
-					* SECTOR_SIZE);
+	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
+				       * SECTOR_SIZE);
 	*capacity = ums_dev->mmc->capacity - tmp_capacity;
 }
 
-static struct ums_board_info ums_board = {
+static struct ums ums_dev = {
 	.read_sector = ums_read_sector,
 	.write_sector = ums_write_sector,
 	.get_capacity = ums_get_capacity,
-	.name = "TRATS UMS disk",
-	.ums_dev = {
-		.mmc = NULL,
-		.dev_num = 0,
-		.offset = 0,
-		.part_size = 0.
-	},
+	.name = "UMS disk",
+	.offset = UMS_START_SECTOR,
+	.part_size = UMS_NUM_SECTORS,
 };
 
-struct ums_board_info *board_ums_init(unsigned int dev_num, unsigned int offset,
-				      unsigned int part_size)
+struct ums *ums_init(unsigned int dev_num)
 {
-	struct mmc *mmc;
+	struct mmc *mmc = NULL;
 
 	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.offset = offset;
-	ums_board.ums_dev.part_size = part_size;
+	ums_dev.mmc = mmc;
 
-	return &ums_board;
+	return &ums_dev;
 }
 #endif
diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index f583caf..f6ceba7 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -22,28 +22,26 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 
 	unsigned int dev_num = (unsigned int)(simple_strtoul(mmc_devstring,
 				NULL, 0));
-	if (dev_num) {
-		error("Set eMMC device to 0! - e.g. ums 0");
-		goto fail;
-	}
+	if (dev_num)
+		return CMD_RET_USAGE;
 
 	unsigned int controller_index = (unsigned int)(simple_strtoul(
 					usb_controller,	NULL, 0));
 	if (board_usb_init(controller_index, USB_INIT_DEVICE)) {
 		error("Couldn't init USB controller.");
-		goto fail;
+		return CMD_RET_FAILURE;
 	}
 
-	struct ums_board_info *ums_info = board_ums_init(dev_num, 0, 0);
-	if (!ums_info) {
-		error("MMC: %d -> NOT available", dev_num);
-		goto fail;
+	struct ums *ums = ums_init(dev_num);
+	if (!ums) {
+		printf("MMC: %u no such device\n", dev_num);
+		return CMD_RET_FAILURE;
 	}
 
-	int rc = fsg_init(ums_info);
+	int rc = fsg_init(ums);
 	if (rc) {
 		error("fsg_init failed");
-		goto fail;
+		return CMD_RET_FAILURE;
 	}
 
 	g_dnl_register("ums");
@@ -62,13 +60,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]",
-	"<USB_controller> <mmc_dev>"
+	"ums <USB_controller> <mmc_dev>  e.g. ums 0 0"
 );
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index b34068a..3b77047 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -444,7 +444,7 @@ static void set_bulk_out_req_length(struct fsg_common *common,
 
 /*-------------------------------------------------------------------------*/
 
-struct ums_board_info			*ums_info;
+struct ums *ums;
 struct fsg_common *the_fsg_common;
 
 static int fsg_set_halt(struct fsg_dev *fsg, struct usb_ep *ep)
@@ -760,14 +760,14 @@ static int do_read(struct fsg_common *common)
 		}
 
 		/* Perform the read */
-		nread = 0;
-		rc = ums_info->read_sector(&(ums_info->ums_dev),
-					   file_offset / SECTOR_SIZE,
-					   amount / SECTOR_SIZE,
-					   (char __user *)bh->buf);
-		if (rc)
+		rc = ums->read_sector(ums,
+				      file_offset / SECTOR_SIZE,
+				      amount / SECTOR_SIZE,
+				      (char __user *)bh->buf);
+		if (!rc)
 			return -EIO;
-		nread = amount;
+
+		nread = rc * SECTOR_SIZE;
 
 		VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
 				(unsigned long long) file_offset,
@@ -934,13 +934,13 @@ static int do_write(struct fsg_common *common)
 			amount = bh->outreq->actual;
 
 			/* Perform the write */
-			rc = ums_info->write_sector(&(ums_info->ums_dev),
+			rc = ums->write_sector(ums,
 					       file_offset / SECTOR_SIZE,
 					       amount / SECTOR_SIZE,
 					       (char __user *)bh->buf);
-			if (rc)
+			if (!rc)
 				return -EIO;
-			nwritten = amount;
+			nwritten = rc * SECTOR_SIZE;
 
 			VLDBG(curlun, "file write %u @ %llu -> %d\n", amount,
 					(unsigned long long) file_offset,
@@ -962,6 +962,8 @@ static int do_write(struct fsg_common *common)
 
 			/* If an error occurred, report it and its position */
 			if (nwritten < amount) {
+				printf("nwritten:%d amount:%d\n", nwritten,
+				       amount);
 				curlun->sense_data = SS_WRITE_ERROR;
 				curlun->info_valid = 1;
 				break;
@@ -1048,14 +1050,13 @@ static int do_verify(struct fsg_common *common)
 		}
 
 		/* Perform the read */
-		nread = 0;
-		rc = ums_info->read_sector(&(ums_info->ums_dev),
-					   file_offset / SECTOR_SIZE,
-					   amount / SECTOR_SIZE,
-					   (char __user *)bh->buf);
-		if (rc)
+		rc = ums->read_sector(ums,
+				      file_offset / SECTOR_SIZE,
+				      amount / SECTOR_SIZE,
+				      (char __user *)bh->buf);
+		if (!rc)
 			return -EIO;
-		nread = amount;
+		nread = rc * SECTOR_SIZE;
 
 		VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
 				(unsigned long long) file_offset,
@@ -1103,7 +1104,7 @@ static int do_inquiry(struct fsg_common *common, struct fsg_buffhd *bh)
 	buf[4] = 31;		/* Additional length */
 				/* No special options */
 	sprintf((char *) (buf + 8), "%-8s%-16s%04x", (char*) vendor_id ,
-			ums_info->name, (u16) 0xffff);
+			ums->name, (u16) 0xffff);
 
 	return 36;
 }
@@ -2758,9 +2759,9 @@ int fsg_add(struct usb_configuration *c)
 	return fsg_bind_config(c->cdev, c, fsg_common);
 }
 
-int fsg_init(struct ums_board_info *ums)
+int fsg_init(struct ums *ums_dev)
 {
-	ums_info = ums;
+	ums = ums_dev;
 
 	return 0;
 }
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 866e7c7..c2c5424 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -275,7 +275,6 @@ struct rw_semaphore { int i; };
 #define ETOOSMALL	525
 
 #include <usb_mass_storage.h>
-extern struct ums_board_info		*ums_info;
 
 /*-------------------------------------------------------------------------*/
 
@@ -581,7 +580,7 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
 	/* R/W if we can, R/O if we must */
 	ro = curlun->initially_ro;
 
-	ums_info->get_capacity(&(ums_info->ums_dev), &size);
+	ums->get_capacity(ums, &size);
 	if (size < 0) {
 		printf("unable to find file size: %s\n", filename);
 		rc = (int) size;
diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
index 13f535c..674ca70 100644
--- a/include/usb_mass_storage.h
+++ b/include/usb_mass_storage.h
@@ -9,32 +9,35 @@
 #define __USB_MASS_STORAGE_H__
 
 #define SECTOR_SIZE		0x200
-
 #include <mmc.h>
 #include <linux/usb/composite.h>
 
-struct ums_device {
-	struct mmc *mmc;
-	int dev_num;
-	int offset;
-	int part_size;
-};
+#ifndef UMS_START_SECTOR
+#define UMS_START_SECTOR	0
+#endif
 
-struct ums_board_info {
-	int (*read_sector)(struct ums_device *ums_dev,
+#ifndef UMS_NUM_SECTORS
+#define UMS_NUM_SECTORS		0
+#endif
+
+struct ums {
+	int (*read_sector)(struct ums *ums_dev,
 			   ulong start, lbaint_t blkcnt, void *buf);
-	int (*write_sector)(struct ums_device *ums_dev,
+	int (*write_sector)(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf);
-	void (*get_capacity)(struct ums_device *ums_dev,
+	void (*get_capacity)(struct ums *ums_dev,
 			     long long int *capacity);
 	const char *name;
-	struct ums_device ums_dev;
+	struct mmc *mmc;
+	int offset;
+	int part_size;
 };
 
-int fsg_init(struct ums_board_info *);
+extern struct ums *ums;
+
+int fsg_init(struct ums *);
 void fsg_cleanup(void);
-struct ums_board_info *board_ums_init(unsigned int, unsigned int,
-				      unsigned int);
+struct ums *ums_init(unsigned int);
 int fsg_main_thread(void *);
 
 #ifdef CONFIG_USB_GADGET_MASS_STORAGE
-- 
1.7.9.5

[U-Boot] [PATCH v2 2/5] usb: ums: allows using every mmc device with ums.

[Przemyslaw Marczak]

Before this change ums command only allowed use of mmc 0.
Now this argument can be set.

Changes:
- remove mmc device number checking because it is always positive number
- remove printing "no such device" - it is done by find_mmc_device()

Change-Id: I767e45151ad515c7bef19e6c13087374f5e23c11
Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
---
 common/cmd_usb_mass_storage.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index f6ceba7..4d3bbd8 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -20,10 +20,11 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 	const char *usb_controller = argv[1];
 	const char *mmc_devstring  = argv[2];
 
-	unsigned int dev_num = (unsigned int)(simple_strtoul(mmc_devstring,
-				NULL, 0));
-	if (dev_num)
-		return CMD_RET_USAGE;
+	unsigned int dev_num = simple_strtoul(mmc_devstring, NULL, 0);
+
+	struct ums *ums = ums_init(dev_num);
+	if (!ums)
+		return CMD_RET_FAILURE;
 
 	unsigned int controller_index = (unsigned int)(simple_strtoul(
 					usb_controller,	NULL, 0));
@@ -32,12 +33,6 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 		return CMD_RET_FAILURE;
 	}
 
-	struct ums *ums = ums_init(dev_num);
-	if (!ums) {
-		printf("MMC: %u no such device\n", dev_num);
-		return CMD_RET_FAILURE;
-	}
-
 	int rc = fsg_init(ums);
 	if (rc) {
 		error("fsg_init failed");
-- 
1.7.9.5

[U-Boot] [PATCH v2 3/5] usb: ums: fix disk capacity miscalculation and code cleanup

[Przemyslaw Marczak]

This patch prevents:
- ums disk capacity miscalculation because of integer overflow

Changes v2:
- Prevents passing zero size disk capacity to ums gadget driver
- Change function ums_get_capacity() to ums_disk_init() and do ums disk
  initialization before gadget init
- Remove unnecessary code from mass storage driver

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
---
 board/samsung/trats/trats.c         |   49 +++++++++++++++++++++++------------
 drivers/usb/gadget/storage_common.c |   26 +++----------------
 include/usb_mass_storage.h          |    6 ++---
 3 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index 0b58b00..7e0e31e 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -778,7 +778,7 @@ static int ums_read_sector(struct ums *ums_dev,
 			   ulong start, lbaint_t blkcnt, void *buf)
 {
 	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
-	lbaint_t blkstart = start + ums_dev->offset;
+	lbaint_t blkstart = start + ums_dev->start_sector;
 	int dev_num = block_dev->dev;
 
 	return block_dev->block_read(dev_num, blkstart, blkcnt, buf);
@@ -788,30 +788,47 @@ static int ums_write_sector(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf)
 {
 	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
-	lbaint_t blkstart = start + ums_dev->offset;
+	lbaint_t blkstart = start + ums_dev->start_sector;
 	int dev_num = block_dev->dev;
 
 	return block_dev->block_write(dev_num, blkstart, blkcnt, buf);
 }
 
-static void ums_get_capacity(struct ums *ums_dev, long long int *capacity)
-{
-	long long int tmp_capacity;
-
-	tmp_capacity = (long long int)((ums_dev->offset + ums_dev->part_size)
-				       * SECTOR_SIZE);
-	*capacity = ums_dev->mmc->capacity - tmp_capacity;
-}
-
 static struct ums ums_dev = {
 	.read_sector = ums_read_sector,
 	.write_sector = ums_write_sector,
-	.get_capacity = ums_get_capacity,
 	.name = "UMS disk",
-	.offset = UMS_START_SECTOR,
-	.part_size = UMS_NUM_SECTORS,
 };
 
+static struct ums *ums_disk_init(struct mmc *mmc)
+{
+	uint64_t mmc_end_sector = mmc->capacity / SECTOR_SIZE;
+	uint64_t ums_end_sector = UMS_NUM_SECTORS + UMS_START_SECTOR;
+
+	if (!mmc_end_sector) {
+		error("MMC capacity is not valid");
+		return NULL;
+	}
+
+	ums_dev.mmc = mmc;
+
+	if (ums_end_sector <= mmc_end_sector) {
+		ums_dev.start_sector = UMS_START_SECTOR;
+		if (UMS_NUM_SECTORS)
+			ums_dev.num_sectors = UMS_NUM_SECTORS;
+		else
+			ums_dev.num_sectors = mmc_end_sector - UMS_START_SECTOR;
+	} else {
+		ums_dev.num_sectors = mmc_end_sector;
+		puts("UMS: defined bad disk parameters. Using default.\n");
+	}
+
+	printf("UMS: disk start sector: %#x, count: %#x\n",
+	       ums_dev.start_sector, ums_dev.num_sectors);
+
+	return &ums_dev;
+}
+
 struct ums *ums_init(unsigned int dev_num)
 {
 	struct mmc *mmc = NULL;
@@ -820,8 +837,6 @@ struct ums *ums_init(unsigned int dev_num)
 	if (!mmc)
 		return NULL;
 
-	ums_dev.mmc = mmc;
-
-	return &ums_dev;
+	return ums_disk_init(mmc);
 }
 #endif
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index c2c5424..02803df 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -572,36 +572,16 @@ static struct usb_gadget_strings	fsg_stringtab = {
 static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
 {
 	int				ro;
-	int				rc = -EINVAL;
-	loff_t				size;
-	loff_t				num_sectors;
-	loff_t				min_sectors;
 
 	/* R/W if we can, R/O if we must */
 	ro = curlun->initially_ro;
 
-	ums->get_capacity(ums, &size);
-	if (size < 0) {
-		printf("unable to find file size: %s\n", filename);
-		rc = (int) size;
-		goto out;
-	}
-	num_sectors = size >> 9;	/* File size in 512-byte blocks */
-	min_sectors = 1;
-	if (num_sectors < min_sectors) {
-		printf("file too small: %s\n", filename);
-		rc = -ETOOSMALL;
-		goto out;
-	}
-
 	curlun->ro = ro;
-	curlun->file_length = size;
-	curlun->num_sectors = num_sectors;
+	curlun->file_length = ums->num_sectors << 9;
+	curlun->num_sectors = ums->num_sectors;
 	debug("open backing file: %s\n", filename);
-	rc = 0;
 
-out:
-	return rc;
+	return 0;
 }
 
 static void fsg_lun_close(struct fsg_lun *curlun)
diff --git a/include/usb_mass_storage.h b/include/usb_mass_storage.h
index 674ca70..9df3adc 100644
--- a/include/usb_mass_storage.h
+++ b/include/usb_mass_storage.h
@@ -25,12 +25,10 @@ struct ums {
 			   ulong start, lbaint_t blkcnt, void *buf);
 	int (*write_sector)(struct ums *ums_dev,
 			    ulong start, lbaint_t blkcnt, const void *buf);
-	void (*get_capacity)(struct ums *ums_dev,
-			     long long int *capacity);
+	unsigned int start_sector;
+	unsigned int num_sectors;
 	const char *name;
 	struct mmc *mmc;
-	int offset;
-	int part_size;
 };
 
 extern struct ums *ums;
-- 
1.7.9.5

[U-Boot] [PATCH v2 4/5] usb: ums: move ums code from trats to Samsung common directory

[Przemyslaw Marczak]

UMS init was implemented in trats board file but mostly it comprises
common code. Due to that it has been moved to common/ums.c to avoid
code duplication in the future.

Changes:
- move ums initialization code from trats to common/ums.c
- remove unused CONFIG_USB_GADGET_MASS_STORAGE from trats.h

Changes v2:
- move this patch at the top of code cleanups patches

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Minkyu Kang <mk7.kang@samsung.com>
---
 board/samsung/common/Makefile |    1 +
 board/samsung/common/ums.c    |   76 +++++++++++++++++++++++++++++++++++++++++
 board/samsung/trats/trats.c   |   68 ------------------------------------
 include/configs/trats.h       |    2 --
 4 files changed, 77 insertions(+), 70 deletions(-)
 create mode 100644 board/samsung/common/ums.c

diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
index ad7564c..d122169 100644
--- a/board/samsung/common/Makefile
+++ b/board/samsung/common/Makefile
@@ -11,6 +11,7 @@ LIB	= $(obj)libsamsung.o
 
 COBJS-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
 COBJS-$(CONFIG_THOR_FUNCTION) += thor.o
+COBJS-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
 
 SRCS    := $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS-y))
diff --git a/board/samsung/common/ums.c b/board/samsung/common/ums.c
new file mode 100644
index 0000000..dc155ad
--- /dev/null
+++ b/board/samsung/common/ums.c
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics
+ * Lukasz Majewski <l.majewski@samsung.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <usb_mass_storage.h>
+#include <part.h>
+
+static int ums_read_sector(struct ums *ums_dev,
+			   ulong start, lbaint_t blkcnt, void *buf)
+{
+	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
+	lbaint_t blkstart = start + ums_dev->start_sector;
+	int dev_num = block_dev->dev;
+
+	return block_dev->block_read(dev_num, blkstart, blkcnt, buf);
+}
+
+static int ums_write_sector(struct ums *ums_dev,
+			    ulong start, lbaint_t blkcnt, const void *buf)
+{
+	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
+	lbaint_t blkstart = start + ums_dev->start_sector;
+	int dev_num = block_dev->dev;
+
+	return block_dev->block_write(dev_num, blkstart, blkcnt, buf);
+}
+
+static struct ums ums_dev = {
+	.read_sector = ums_read_sector,
+	.write_sector = ums_write_sector,
+	.name = "UMS disk",
+};
+
+static struct ums *ums_disk_init(struct mmc *mmc)
+{
+	uint64_t mmc_end_sector = mmc->capacity / SECTOR_SIZE;
+	uint64_t ums_end_sector = UMS_NUM_SECTORS + UMS_START_SECTOR;
+
+	if (!mmc_end_sector) {
+		error("MMC capacity is not valid");
+		return NULL;
+	}
+
+	ums_dev.mmc = mmc;
+
+	if (ums_end_sector <= mmc_end_sector) {
+		ums_dev.start_sector = UMS_START_SECTOR;
+		if (UMS_NUM_SECTORS)
+			ums_dev.num_sectors = UMS_NUM_SECTORS;
+		else
+			ums_dev.num_sectors = mmc_end_sector - UMS_START_SECTOR;
+	} else {
+		ums_dev.num_sectors = mmc_end_sector;
+		puts("UMS: defined bad disk parameters. Using default.\n");
+	}
+
+	printf("UMS: disk start sector: %#x, count: %#x\n",
+	       ums_dev.start_sector, ums_dev.num_sectors);
+
+	return &ums_dev;
+}
+
+struct ums *ums_init(unsigned int dev_num)
+{
+	struct mmc *mmc = NULL;
+
+	mmc = find_mmc_device(dev_num);
+	if (!mmc)
+		return NULL;
+
+	return ums_disk_init(mmc);
+}
diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index 7e0e31e..db5828d 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -772,71 +772,3 @@ void init_panel_info(vidinfo_t *vid)
 
 	setenv("lcdinfo", "lcd=s6e8ax0");
 }
-
-#ifdef CONFIG_USB_GADGET_MASS_STORAGE
-static int ums_read_sector(struct ums *ums_dev,
-			   ulong start, lbaint_t blkcnt, void *buf)
-{
-	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
-	lbaint_t blkstart = start + ums_dev->start_sector;
-	int dev_num = block_dev->dev;
-
-	return block_dev->block_read(dev_num, blkstart, blkcnt, buf);
-}
-
-static int ums_write_sector(struct ums *ums_dev,
-			    ulong start, lbaint_t blkcnt, const void *buf)
-{
-	block_dev_desc_t *block_dev = &ums_dev->mmc->block_dev;
-	lbaint_t blkstart = start + ums_dev->start_sector;
-	int dev_num = block_dev->dev;
-
-	return block_dev->block_write(dev_num, blkstart, blkcnt, buf);
-}
-
-static struct ums ums_dev = {
-	.read_sector = ums_read_sector,
-	.write_sector = ums_write_sector,
-	.name = "UMS disk",
-};
-
-static struct ums *ums_disk_init(struct mmc *mmc)
-{
-	uint64_t mmc_end_sector = mmc->capacity / SECTOR_SIZE;
-	uint64_t ums_end_sector = UMS_NUM_SECTORS + UMS_START_SECTOR;
-
-	if (!mmc_end_sector) {
-		error("MMC capacity is not valid");
-		return NULL;
-	}
-
-	ums_dev.mmc = mmc;
-
-	if (ums_end_sector <= mmc_end_sector) {
-		ums_dev.start_sector = UMS_START_SECTOR;
-		if (UMS_NUM_SECTORS)
-			ums_dev.num_sectors = UMS_NUM_SECTORS;
-		else
-			ums_dev.num_sectors = mmc_end_sector - UMS_START_SECTOR;
-	} else {
-		ums_dev.num_sectors = mmc_end_sector;
-		puts("UMS: defined bad disk parameters. Using default.\n");
-	}
-
-	printf("UMS: disk start sector: %#x, count: %#x\n",
-	       ums_dev.start_sector, ums_dev.num_sectors);
-
-	return &ums_dev;
-}
-
-struct ums *ums_init(unsigned int dev_num)
-{
-	struct mmc *mmc = NULL;
-
-	mmc = find_mmc_device(dev_num);
-	if (!mmc)
-		return NULL;
-
-	return ums_disk_init(mmc);
-}
-#endif
diff --git a/include/configs/trats.h b/include/configs/trats.h
index f5bb6aa..3e67229 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -323,9 +323,7 @@
 #define CONFIG_SYS_VIDEO_LOGO_MAX_SIZE ((500 * 120 * 4) + (1 << 12))
 
 #define CONFIG_CMD_USB_MASS_STORAGE
-#if defined(CONFIG_CMD_USB_MASS_STORAGE)
 #define CONFIG_USB_GADGET_MASS_STORAGE
-#endif
 
 /* Pass open firmware flat tree */
 #define CONFIG_OF_LIBFDT    1
-- 
1.7.9.5

[U-Boot] [PATCH v2 5/5] usb: ums: add ums exit feature by ctrl+c or by detach usb cable

[Przemyslaw Marczak]

This patch allows exiting from UMS mode to u-boot prompt
by detaching usb cable or by pressing ctrl+c.

Add new config: CONFIG_USB_CABLE_CHECK. If defined then board
file should provide function: usb_cable_connected() (include/usb.h)
that return 1 if cable is connected and 0 otherwise.

Changes v2:
- add a note to the README

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Marek Vasut <marex@denx.de>
---
 README                              |    7 +++++++
 common/cmd_usb_mass_storage.c       |   21 +++++++++++++--------
 drivers/usb/gadget/f_mass_storage.c |   24 +++++++++++++++++++++---
 include/usb.h                       |   10 ++++++++++
 4 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/README b/README
index cee8e2f..75329b1 100644
--- a/README
+++ b/README
@@ -1362,6 +1362,13 @@ The following options need to be configured:
 			for your device
 			- CONFIG_USBD_PRODUCTID 0xFFFF
 
+		Some USB device drivers may need to check USB cable attachment.
+		In this case you can enable following config in BoardName.h:
+			CONFIG_USB_CABLE_CHECK
+			This enables function definition:
+			- usb_cable_connected() in include/usb.h
+			Implementation of this function is board-specific.
+
 - ULPI Layer Support:
 		The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
 		the generic ULPI layer. The generic layer accesses the ULPI PHY
diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index 4d3bbd8..99487f4 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -5,6 +5,7 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <errno.h>
 #include <common.h>
 #include <command.h>
 #include <g_dnl.h>
@@ -42,16 +43,20 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 	g_dnl_register("ums");
 
 	while (1) {
-		/* Handle control-c and timeouts */
-		if (ctrlc()) {
-			error("The remote end did not respond in time.");
-			goto exit;
-		}
-
 		usb_gadget_handle_interrupts();
-		/* Check if USB cable has been detached */
-		if (fsg_main_thread(NULL) == EIO)
+
+		rc = fsg_main_thread(NULL);
+		if (rc) {
+			/* Check I/O error */
+			if (rc == -EIO)
+				printf("\rCheck USB cable connection\n");
+
+			/* Check CTRL+C */
+			if (rc == -EPIPE)
+				printf("\rCTRL+C - Operation aborted\n");
+
 			goto exit;
+		}
 	}
 exit:
 	g_dnl_unregister();
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 3b77047..d290a56 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -245,6 +245,7 @@
 #include <config.h>
 #include <malloc.h>
 #include <common.h>
+#include <usb.h>
 
 #include <linux/err.h>
 #include <linux/usb/ch9.h>
@@ -678,6 +679,18 @@ static int sleep_thread(struct fsg_common *common)
 			k++;
 		}
 
+		if (k == 10) {
+			/* Handle CTRL+C */
+			if (ctrlc())
+				return -EPIPE;
+#ifdef CONFIG_USB_CABLE_CHECK
+			/* Check cable connection */
+			if (!usb_cable_connected())
+				return -EIO;
+#endif
+			k = 0;
+		}
+
 		usb_gadget_handle_interrupts();
 	}
 	common->thread_wakeup_needed = 0;
@@ -2390,6 +2403,7 @@ static void handle_exception(struct fsg_common *common)
 
 int fsg_main_thread(void *common_)
 {
+	int ret;
 	struct fsg_common	*common = the_fsg_common;
 	/* The main loop */
 	do {
@@ -2399,12 +2413,16 @@ int fsg_main_thread(void *common_)
 		}
 
 		if (!common->running) {
-			sleep_thread(common);
+			ret = sleep_thread(common);
+			if (ret)
+				return ret;
+
 			continue;
 		}
 
-		if (get_next_command(common))
-			continue;
+		ret = get_next_command(common);
+		if (ret)
+			return ret;
 
 		if (!exception_in_progress(common))
 			common->state = FSG_STATE_DATA_PHASE;
diff --git a/include/usb.h b/include/usb.h
index 17fb68c..8c1789f 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -197,6 +197,16 @@ int board_usb_init(int index, enum board_usb_init_type init);
  */
 int board_usb_cleanup(int index, enum board_usb_init_type init);
 
+/*
+ * If CONFIG_USB_CABLE_CHECK is set then this function
+ * should be defined in board file.
+ *
+ * @return 1 if cable is connected and 0 otherwise.
+ */
+#ifdef CONFIG_USB_CABLE_CHECK
+int usb_cable_connected(void);
+#endif
+
 #ifdef CONFIG_USB_STORAGE
 
 #define USB_MAX_STOR_DEV 5
-- 
1.7.9.5

[U-Boot] [PATCH v2 1/5] usb: ums: code refactoring to improve reusability on other boards.

[Marek Vasut]

Dear Przemyslaw Marczak,

> This patch introduces some cleanups to ums code. Changes:
> 
> ums common:
> - introduce UMS_START_SECTOR and UMS_NUM_SECTORS as defined in
>   usb_mass_storage.h both default values as 0 if board config
>   doesn't define them
> 
> common cleanup changes:
> - change name of struct "ums_board_info" to "ums"
> - "ums_device" fields are moved to struct ums and "dev_num" is removed
> - change function name: board_ums_init to ums_init
> - remove "extern" prefixes from usb_mass_storage.h
> 
> cmd_usb_mass_storage:
> - change error() to printf() if need to print info message
> - change return values to command_ret_t type at ums command code
> - add command usage string
> 
> Changes v2:
> ums common:
> - always returns number of read/write sectors
> - coding style clean-up
> ums gadget:
> - calculate amount of read/write from device returned value.

Hi Lukasz, can I get ack/nak on the series? Thanks!

Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/5] usb: ums: code refactoring to improve reusability on other boards.

[Lukasz Majewski]

Hi Marek,

> Dear Przemyslaw Marczak,
> 
> > This patch introduces some cleanups to ums code. Changes:
> > 
> > ums common:
> > - introduce UMS_START_SECTOR and UMS_NUM_SECTORS as defined in
> >   usb_mass_storage.h both default values as 0 if board config
> >   doesn't define them
> > 
> > common cleanup changes:
> > - change name of struct "ums_board_info" to "ums"
> > - "ums_device" fields are moved to struct ums and "dev_num" is
> > removed
> > - change function name: board_ums_init to ums_init
> > - remove "extern" prefixes from usb_mass_storage.h
> > 
> > cmd_usb_mass_storage:
> > - change error() to printf() if need to print info message
> > - change return values to command_ret_t type at ums command code
> > - add command usage string
> > 
> > Changes v2:
> > ums common:
> > - always returns number of read/write sectors
> > - coding style clean-up
> > ums gadget:
> > - calculate amount of read/write from device returned value.
> 
> Hi Lukasz, can I get ack/nak on the series? Thanks!

For the whole series:

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

> 
> Best regards,
> Marek Vasut



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH v2 1/5] usb: ums: code refactoring to improve reusability on other boards.

[Marek Vasut]

Hi Lukasz, Przemyslaw,

> Hi Marek,
> 
> > Dear Przemyslaw Marczak,
> > 
> > > This patch introduces some cleanups to ums code. Changes:
> > > 
> > > ums common:
> > > - introduce UMS_START_SECTOR and UMS_NUM_SECTORS as defined in
> > > 
> > >   usb_mass_storage.h both default values as 0 if board config
> > >   doesn't define them
> > > 
> > > common cleanup changes:
> > > - change name of struct "ums_board_info" to "ums"
> > > - "ums_device" fields are moved to struct ums and "dev_num" is
> > > removed
> > > - change function name: board_ums_init to ums_init
> > > - remove "extern" prefixes from usb_mass_storage.h
> > > 
> > > cmd_usb_mass_storage:
> > > - change error() to printf() if need to print info message
> > > - change return values to command_ret_t type at ums command code
> > > - add command usage string
> > > 
> > > Changes v2:
> > > ums common:
> > > - always returns number of read/write sectors
> > > - coding style clean-up
> > > ums gadget:
> > > - calculate amount of read/write from device returned value.
> > 
> > Hi Lukasz, can I get ack/nak on the series? Thanks!
> 
> For the whole series:
> 
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>

Applied all. Thanks!

Best regards,
Marek Vasut