Hi Eddie,
On 15 March 2017 at 01:56, Eddie Cai <eddie.cai.li...@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.li...@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?
>
> 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_ROCKUSB
> + help
> + Enables the command "rockusb" which is used to have U-Boot create a
> + Rockusb class device via USB.
Does this mean Rockchip-class device?
> +
> +config USB_FUNCTION_ROCKUSB
> + bool "Enable USB rockusb gadget"
> + help
> + This 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?
>
> 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.li...@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
> +
remove extra blank line
> +
> +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?
> +
> + 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.
> + printf("do_rockusb1\n");
Do you want this?
> + 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?
> + 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?
> + "rockusb <USB_controller> [<devtype>] <devnum> e.g. rockusb 0 mmc
> 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?
> @@ -0,0 +1,13 @@
> +/*
> + * (C) Copyright 2017
> + *
> + * Eddie Cai <eddie.cai.li...@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.
> +
> +#endif /* _ROCKUSB_H_ */
> --
> 2.7.4
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
