ums: support block devices not MMC devices

---
v2: (The series) rebased onto latest u-boot-usb/master to pick up Mateusz's DFU changes.

 board/samsung/common/ums.c | 7 ++++---
 include/usb_mass_storage.h | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

board/samsung/common/ums.c +4 -3

index cebabe920a29..ffe63685835c 100644
@@ -7,12 +7,13 @@
 
 #include <common.h>
 #include <usb_mass_storage.h>
+#include <mmc.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;
+	block_dev_desc_t *block_dev = ums_dev->block_dev;
 	lbaint_t blkstart = start + ums_dev->start_sector;
 	int dev_num = block_dev->dev;

@@ -22,7 +23,7 @@ static int ums_read_sector(struct ums *ums_dev,
 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;
+	block_dev_desc_t *block_dev = ums_dev->block_dev;
 	lbaint_t blkstart = start + ums_dev->start_sector;
 	int dev_num = block_dev->dev;

@@ -45,7 +46,7 @@ static struct ums *ums_disk_init(struct mmc *mmc)
 		return NULL;
 	}
 
-	ums_dev.mmc = mmc;
+	ums_dev.block_dev = &mmc->block_dev;
 
 	if (ums_end_sector <= mmc_end_sector) {
 		ums_dev.start_sector = UMS_START_SECTOR;

include/usb_mass_storage.h +2 -2

index ed460644c1fe..e3eb1ebb2e47 100644
@@ -9,7 +9,7 @@
 #define __USB_MASS_STORAGE_H__
 
 #define SECTOR_SIZE		0x200
-#include <mmc.h>
+#include <part.h>
 #include <linux/usb/composite.h>
 
 #ifndef UMS_START_SECTOR

@@ -31,7 +31,7 @@ struct ums {
 	unsigned int start_sector;
 	unsigned int num_sectors;
 	const char *name;
-	struct mmc *mmc;
+	block_dev_desc_t *block_dev;
 };
 
 extern struct ums *ums;

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #1 lore raw

PATCH v2 2/8 ums: remove UMS_{NUM, START}_SECTORS + UMS_START_SECTOR

From: Stephen Warren <[email protected]>

These values aren't set anywhere at present, and hence have no effect. The concept of a single global offset/number of sectors to expose through USB Mass Storage doesn't even make sense in the face of multiple storage devices. Remove these defines to simplify the code.

Cc Lukasz Majewski <[email protected]>

---
 board/samsung/common/ums.c | 14 ++------------
 include/usb_mass_storage.h |  8 --------
 2 files changed, 2 insertions(+), 20 deletions(-)

board/samsung/common/ums.c +2 ~~-12~~

index ffe63685835c..1f7dda43629b 100644
@@ -39,7 +39,6 @@ static struct ums ums_dev = {
 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");

@@ -47,17 +46,8 @@ static struct ums *ums_disk_init(struct mmc *mmc)
 	}
 
 	ums_dev.block_dev = &mmc->block_dev;
-
-	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");
-	}
+	ums_dev.start_sector = 0;
+	ums_dev.num_sectors = mmc_end_sector;
 
 	printf("UMS: disk start sector: %#x, count: %#x\n",
 	       ums_dev.start_sector, ums_dev.num_sectors);

include/usb_mass_storage.h +0 -8

index e3eb1ebb2e47..2b959ec057e4 100644
@@ -12,14 +12,6 @@
 #include <part.h>
 #include <linux/usb/composite.h>
 
-#ifndef UMS_START_SECTOR
-#define UMS_START_SECTOR	0
-#endif
-
-#ifndef UMS_NUM_SECTORS
-#define UMS_NUM_SECTORS		0
-#endif
-
 /* Wait at maximum 60 seconds for cable connection */
 #define UMS_CABLE_READY_TIMEOUT	60

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #2 lore raw

PATCH v2 3/8 ums: remove error-checking of MMC device size

From: Stephen Warren <[email protected]>

There's no reason to believe that an MMC device will incorrectly report its capacity. Remove error checking of this value from ums_disk_init() to simplify it.

Cc Lukasz Majewski <[email protected]>

---
 board/samsung/common/ums.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

board/samsung/common/ums.c +1 -8

index 1f7dda43629b..42a49018f584 100644
@@ -38,16 +38,9 @@ static struct ums ums_dev = {
 
 static struct ums *ums_disk_init(struct mmc *mmc)
 {
-	uint64_t mmc_end_sector = mmc->capacity / SECTOR_SIZE;
-
-	if (!mmc_end_sector) {
-		error("MMC capacity is not valid");
-		return NULL;
-	}
-
 	ums_dev.block_dev = &mmc->block_dev;
 	ums_dev.start_sector = 0;
-	ums_dev.num_sectors = mmc_end_sector;
+	ums_dev.num_sectors = mmc->capacity / SECTOR_SIZE;
 
 	printf("UMS: disk start sector: %#x, count: %#x\n",
 	       ums_dev.start_sector, ums_dev.num_sectors);

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #3 lore raw

PATCH v2 4/8 ums: remove ums_disk_init()

From: Stephen Warren <[email protected]>

Now that ums_disk_init() is so simple, there's no need for it to be a separate function. Instead, just add it to the tail end of ums_init().

Cc Lukasz Majewski <[email protected]>

---
 board/samsung/common/ums.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

board/samsung/common/ums.c +7 ~~-10~~

index 42a49018f584..b35670d9966a 100644
@@ -36,8 +36,14 @@ static struct ums ums_dev = {
 	.name = "UMS disk",
 };
 
-static struct ums *ums_disk_init(struct mmc *mmc)
+struct ums *ums_init(unsigned int dev_num)
 {
+	struct mmc *mmc = NULL;
+
+	mmc = find_mmc_device(dev_num);
+	if (!mmc || mmc_init(mmc))
+		return NULL;
+
 	ums_dev.block_dev = &mmc->block_dev;
 	ums_dev.start_sector = 0;
 	ums_dev.num_sectors = mmc->capacity / SECTOR_SIZE;

@@ -47,12 +53,3 @@ static struct ums *ums_disk_init(struct mmc *mmc)
 
 	return &ums_dev;
 }
-
-struct ums *ums_init(unsigned int dev_num)
-{
-	struct mmc *mmc = find_mmc_device(dev_num);
-
-	if (!mmc || mmc_init(mmc))
-		return NULL;
-	return ums_disk_init(mmc);
-}

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #4 lore raw

PATCH v2 5/8 ums: move IO support code to common location

From: Stephen Warren <[email protected]>

There's nothing Samsung-/board-specfic about the implementation of ums_init(). Move the code into cmd_usb_mass_storage.c, so that it can be shared by any user of that command.

Cc Lukasz Majewski <[email protected]>

---
 board/samsung/common/Makefile |  1 -
 board/samsung/common/ums.c    | 55 -------------------------------------------
 common/cmd_usb_mass_storage.c | 46 ++++++++++++++++++++++++++++++++++++
 include/usb_mass_storage.h    |  1 -
 4 files changed, 46 insertions(+), 57 deletions(-)
 delete mode 100644 board/samsung/common/ums.c

board/samsung/common/Makefile +0 -1

index 7d2bb8c4a2f3..41d0cc381494 100644
@@ -7,7 +7,6 @@
 
 obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
 obj-$(CONFIG_THOR_FUNCTION) += thor.o
-obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
 obj-$(CONFIG_MISC_COMMON) += misc.o
 
 ifndef CONFIG_SPL_BUILD

board/samsung/common/ums.c deleted +0 ~~-55~~

55 lines not on this page · raw message

common/cmd_usb_mass_storage.c +46 -0

55 lines not on this page · raw message

include/usb_mass_storage.h +0 -1

index 2b959ec057e4..69b80cd1a367 100644
@@ -30,7 +30,6 @@ extern struct ums *ums;
 
 int fsg_init(struct ums *);
 void fsg_cleanup(void);
-struct ums *ums_init(unsigned int);
 int fsg_main_thread(void *);
 int fsg_add(struct usb_configuration *c);
 #endif /* __USB_MASS_STORAGE_H__ */

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #5 lore raw

PATCH v2 6/8 ums: use get_device() not find_mmc_device();

From: Stephen Warren <[email protected]>

get_device() is a generic routine that will support any type of block device. Use this instead of the type-specific find_mmc_device(), for future flexibility.

Cc Lukasz Majewski <[email protected]>

---
 common/cmd_usb_mass_storage.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

common/cmd_usb_mass_storage.c +14 ~~-11~~

index 9888ed4f1756..b5954fc5c6e5 100644
@@ -9,7 +9,6 @@
 #include <common.h>
 #include <command.h>
 #include <g_dnl.h>
-#include <mmc.h>
 #include <part.h>
 #include <usb.h>
 #include <usb_mass_storage.h>

@@ -40,17 +39,22 @@ static struct ums ums_dev = {
 	.name = "UMS disk",
 };
 
-struct ums *ums_init(unsigned int dev_num)
+struct ums *ums_init(const char *devtype, const char *devnum)
 {
-	struct mmc *mmc = NULL;
+	block_dev_desc_t *block_dev;
+	int ret;
 
-	mmc = find_mmc_device(dev_num);
-	if (!mmc || mmc_init(mmc))
+	ret = get_device(devtype, devnum, &block_dev);
+	if (ret < 0)
 		return NULL;
 
-	ums_dev.block_dev = &mmc->block_dev;
+	/* f_mass_storage.c assumes SECTOR_SIZE sectors */
+	if (block_dev->blksz != SECTOR_SIZE)
+		return NULL;
+
+	ums_dev.block_dev = block_dev;
 	ums_dev.start_sector = 0;
-	ums_dev.num_sectors = mmc->capacity / SECTOR_SIZE;
+	ums_dev.num_sectors = block_dev->lba;
 
 	printf("UMS: disk start sector: %#x, count: %#x\n",
 	       ums_dev.start_sector, ums_dev.num_sectors);

@@ -65,11 +69,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 		return CMD_RET_USAGE;
 
 	const char *usb_controller = argv[1];
-	const char *mmc_devstring  = argv[2];
-
-	unsigned int dev_num = simple_strtoul(mmc_devstring, NULL, 0);
+	const char *devtype = "mmc";
+	const char *devnum  = argv[2];
 
-	struct ums *ums = ums_init(dev_num);
+	struct ums *ums = ums_init(devtype, devnum);
 	if (!ums)
 		return CMD_RET_FAILURE;

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #6 lore raw

PATCH v2 7/8 ums: move all variable declarations to the start of the block

From: Stephen Warren <[email protected]>

It's easier to assign values to the variables inside an if statement body if the assignment and declaration are separate.

Cc Lukasz Majewski <[email protected]>

---
 common/cmd_usb_mass_storage.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

common/cmd_usb_mass_storage.c +16 -8

index b5954fc5c6e5..5e0211683c8f 100644
@@ -65,25 +65,33 @@ struct ums *ums_init(const char *devtype, const char *devnum)
 int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 			       int argc, char * const argv[])
 {
+	const char *usb_controller;
+	const char *devtype;
+	const char *devnum;
+	struct ums *ums;
+	unsigned int controller_index;
+	int rc;
+	int cable_ready_timeout __maybe_unused;
+
 	if (argc < 3)
 		return CMD_RET_USAGE;
 
-	const char *usb_controller = argv[1];
-	const char *devtype = "mmc";
-	const char *devnum  = argv[2];
+	usb_controller = argv[1];
+	devtype = "mmc";
+	devnum  = argv[2];
 
-	struct ums *ums = ums_init(devtype, devnum);
+	ums = ums_init(devtype, devnum);
 	if (!ums)
 		return CMD_RET_FAILURE;
 
-	unsigned int controller_index = (unsigned int)(simple_strtoul(
-					usb_controller,	NULL, 0));
+	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.");
 		return CMD_RET_FAILURE;
 	}
 
-	int rc = fsg_init(ums);
+	rc = fsg_init(ums);
 	if (rc) {
 		error("fsg_init failed");
 		return CMD_RET_FAILURE;

@@ -96,7 +104,7 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 	}
 
 	/* Timeout unit: seconds */
-	int cable_ready_timeout = UMS_CABLE_READY_TIMEOUT;
+	cable_ready_timeout = UMS_CABLE_READY_TIMEOUT;
 
 	if (!g_dnl_board_usb_cable_connected()) {
 		/*

Signature · 1 line

1.8.1.5

Stephen Warren

author

2014-05-05 16:40 UTC #7 lore raw

PATCH v2 8/8 ums: allow the user to specify the device type

From: Stephen Warren <[email protected]>

Allow an optional devtype parameter to the ums command, which specifies the type of the device to be exported. This could allow exporting a SATA or even another USB device.

Cc Lukasz Majewski <[email protected]>

---
 common/cmd_usb_mass_storage.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

common/cmd_usb_mass_storage.c +10 -4

index 5e0211683c8f..2c879ea0837c 100644
@@ -77,8 +77,13 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag,
 		return CMD_RET_USAGE;
 
 	usb_controller = argv[1];
-	devtype = "mmc";
-	devnum  = argv[2];
+	if (argc >= 4) {
+		devtype = argv[2];
+		devnum  = argv[3];
+	} else {
+		devtype = "mmc";
+		devnum  = argv[2];
+	}
 
 	ums = ums_init(devtype, devnum);
 	if (!ums)

@@ -152,7 +157,8 @@ exit:
 	return CMD_RET_SUCCESS;
 }
 
-U_BOOT_CMD(ums, CONFIG_SYS_MAXARGS, 1, do_usb_mass_storage,
+U_BOOT_CMD(ums, 4, 1, do_usb_mass_storage,
 	"Use the UMS [User Mass Storage]",
-	"ums <USB_controller> <mmc_dev>  e.g. ums 0 0"
+	"ums <USB_controller> [<devtype>] <devnum>  e.g. ums 0 mmc 0\n"
+	"    devtype defaults to mmc"
 );

Signature · 1 line

1.8.1.5

Marek Vasut

2014-05-05 17:45 UTC #8 lore raw

PATCH v2 1/8 ums: support block devices not MMC devices

Quoted 9 lines · Stephen Warren

From: Stephen Warren <[email protected]>
The USB Mass Storage function could equally well support a SATA device as support an MMC device. Update struct ums to contain a block device descriptor, not an MMC device descriptor.
Cc Lukasz Majewski <[email protected]>
Signed-off-by Stephen Warren <[email protected]>
Acked-by Przemyslaw Marczak <[email protected]>

Lukasz, I think this series is yours, right ? :)

Best regards,
Marek Vasut

Lukasz Majewski

2014-05-06 06:21 UTC #9 lore raw

PATCH v2 1/8 ums: support block devices not MMC devices

Hi Marek,

Quoted 7 lines · Stephen Warren

Quoted 6 lines
From: Stephen Warren <[email protected]>
The USB Mass Storage function could equally well support a SATA device as support an MMC device. Update struct ums to contain a block device descriptor, not an MMC device descriptor.

Quoted 5 lines

Quoted 3 lines
Cc Lukasz Majewski <[email protected]>
Signed-off-by Stephen Warren <[email protected]>
Acked-by Przemyslaw Marczak <[email protected]>
Lukasz, I think this series is yours, right ? :)

We feel a bit responsible for the gadget code :-).

Przemek (If he don't mind :-)) will look into the v2 of the UMS, since Stephen has sent it yesterday, and if nothing breaks I will pull them to u-boot-dfu branch.

Quoted 3 lines

Best regards,
Marek Vasut

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Przemyslaw Marczak

2014-05-06 08:28 UTC #10 lore raw

PATCH v2 1/8 ums: support block devices not MMC devices

Hi,

Quoted 13 lines · Lukasz Majewski

Hi Marek,
Quoted 11 lines · Stephen Warren
Quoted 9 lines
From: Stephen Warren <[email protected]>
The USB Mass Storage function could equally well support a SATA device as support an MMC device. Update struct ums to contain a block device descriptor, not an MMC device descriptor.
Cc Lukasz Majewski <[email protected]>
Signed-off-by Stephen Warren <[email protected]>
Acked-by Przemyslaw Marczak <[email protected]>

Quoted 14 lines

Quoted 1 line
Lukasz, I think this series is yours, right ? :)
We feel a bit responsible for the gadget code :-).
Przemek (If he don't mind :-)) will look into the v2 of the UMS, since Stephen has sent it yesterday, and if nothing breaks I will pull them to u-boot-dfu branch.
Quoted 3 lines
Best regards,
Marek Vasut

Patch number 7 can be applied with "am -3", beside this Trats2 is working fine.
Thanks

Signature · 4 lines

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

Lukasz Majewski

2014-05-06 09:21 UTC #11 lore raw

PATCH v2 1/8 ums: support block devices not MMC devices