[U-Boot] [PATCH 0/4] USB: UMS: code refactoring and usage improvement
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
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
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.
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.
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.
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
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 = {
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
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?
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.
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
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
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
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
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
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
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
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
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");
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;
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
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
participants (3)
-
Lukasz Majewski -
Marek Vasut -
Przemyslaw Marczak