Hi Simon
2017-03-20 10:29 GMT+08:00 Simon Glass sjg@chromium.org:
Hi Eddie,
On 15 March 2017 at 01:56, Eddie Cai eddie.cai.linux@gmail.com wrote:
this patch add rockusb command. the usage is rockusb <USB_controller> [<devtype>] <devnum> e.g. rockusb 0 mmc 0
Signed-off-by: Eddie Cai eddie.cai.linux@gmail.com
cmd/Kconfig | 12 +++++++++ cmd/Makefile | 1 + cmd/rockusb.c | 79 ++++++++++++++++++++++++++++++
+++++++++++++++++++++++++
include/rockusb.h | 13 +++++++++ 4 files changed, 105 insertions(+) create mode 100644 cmd/rockusb.c create mode 100644 include/rockusb.h
Can you please add some documentation on how to use this command and what host tools you need?
Sure, I will add README.rockusb in next version
diff --git a/cmd/Kconfig b/cmd/Kconfig index ef53156..dac2cc0 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -475,6 +475,18 @@ config CMD_DFU Enables the command "dfu" which is used to have U-Boot create
a DFU
class device via USB.+config CMD_ROCKUSB
bool "rockusb"select USB_FUNCTION_ROCKUSBhelpEnables the command "rockusb" which is used to have U-Bootcreate a
Rockusb class device via USB.Does this mean Rockchip-class device?
Yes, This is vendor specific class.
+config USB_FUNCTION_ROCKUSB
bool "Enable USB rockusb gadget"helpThis enables the USB part of the rockusb gadget.config CMD_USB_MASS_STORAGE bool "UMS usb mass storage" help diff --git a/cmd/Makefile b/cmd/Makefile index f13bb8c..f5f1663 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -141,6 +141,7 @@ endif
obj-$(CONFIG_CMD_USB) += usb.o disk.o obj-$(CONFIG_CMD_FASTBOOT) += fastboot.o +obj-$(CONFIG_CMD_ROCKUSB) += rockusb.o obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o
Can you put it in alphabetical order if possible?
Sure, I will try to put it in alphabetical order in next version
obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o diff --git a/cmd/rockusb.c b/cmd/rockusb.c new file mode 100644 index 0000000..f5bd86e --- /dev/null +++ b/cmd/rockusb.c @@ -0,0 +1,79 @@ +/*
- Copyright (C) 2017 Eddie Cai eddie.cai.linux@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <command.h> +#include <console.h> +#include <g_dnl.h> +#include <usb.h> +#include <rockusb.h>
Should go above usb.h
done
remove extra blank line
done
+static int do_rockusb(cmd_tbl_t *cmdtp, int flag, int argc, char *const
argv[])
+{
int controller_index, dev_index;char *usb_controller;char *devtype;char *devnum;int ret;if (argc < 2)return CMD_RET_USAGE;usb_controller = argv[1];controller_index = simple_strtoul(usb_controller, NULL, 0);if (argc >= 4) {devtype = argv[2];devnum = argv[3];} else {devtype = "mmc";devnum = argv[2];}dev_index = simple_strtoul(devnum, NULL, 0);rockusb_dev_init(devtype, dev_index);Can this fail, e.g. if index is out of range?
I will add a return value in next version
ret = board_usb_init(controller_index, USB_INIT_DEVICE);if (ret) {error("USB init failed: %d", ret);return CMD_RET_FAILURE;}g_dnl_clear_detach();ret = g_dnl_register("usb_dnl_rockusb");if (ret)return ret;Commands should return either 0 or CMD_RET_FAILURE.
OK, i will fix it in next version.
printf("do_rockusb1\n");Do you want this?
Oops, i will remove it in next version.
if (!g_dnl_board_usb_cable_connected()) {puts("\rUSB cable not detected.\n" \"Command exit.\n");ret = CMD_RET_FAILURE;goto exit;}while (1) {if (g_dnl_detach())break;if (ctrlc())break;usb_gadget_handle_interrupts(controller_index);}printf("do_rockusb2\n");Do you want this?
i will remove it in next version.
ret = CMD_RET_SUCCESS;+exit:
g_dnl_unregister();g_dnl_clear_detach();board_usb_cleanup(controller_index, USB_INIT_DEVICE);return ret;+}
+U_BOOT_CMD(rockusb, 4, 1, do_rockusb,
"Use the ROCKUSB",Can you add a little more here?
sure
"rockusb <USB_controller> [<devtype>] <devnum> e.g. rockusb 0mmc 0\n"
" devtype defaults to mmc"+); diff --git a/include/rockusb.h b/include/rockusb.h new file mode 100644 index 0000000..cdea63d --- /dev/null +++ b/include/rockusb.h
Can this go in include/asm instead?
do you mean include/asm-generic ? may i know why ?
@@ -0,0 +1,13 @@ +/*
- (C) Copyright 2017
- Eddie Cai eddie.cai.linux@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef _ROCKUSB_H_ +#define _ROCKUSB_H_
+void rockusb_dev_init(char *dev_type, int dev_index);
Please add a function comment.
OK, i will add a function comment in next version .
+#endif /* _ROCKUSB_H_ */
2.7.4
Regards, Simon