Re: [U-Boot] [U-Boot 2/3] cmd: add rockusb command
Hi Eddie,
> this patch add rockusb command. the usage is
> rockusb []
> e.g. rockusb 0 mmc 0
>
> Signed-off-by: Eddie Cai
> ---
> 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
>
> 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.
> +
> +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
>
> 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 000..f5bd86e
> --- /dev/null
> +++ b/cmd/rockusb.c
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright (C) 2017 Eddie Cai
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +
Please remove the extra blank here. (also run your patches
through ./scripts/checkpatch.pl before submission.
> +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];
> + }
I would prefer to force the whole command syntax support:
"rockchip 0 mmc 0" to comply with other commands (like thor, dfu).
Instead of "rockchip 0 0" and imply the mmc.
> + dev_index = simple_strtoul(devnum, NULL, 0);
> + rockusb_dev_init(devtype, dev_index);
> +
> + 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;
> + printf("do_rockusb1\n");
this should be either removed or
rewritten as "debug(...)".
> + 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");
> + 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",
> + "rockusb [] e.g. rockusb
> 0 mmc 0\n"
> + "devtype defaults to mmc"
[] should be mandatory.
> +);
> diff --git a/include/rockusb.h b/include/rockusb.h
> new file mode 100644
> index 000..cdea63d
> --- /dev/null
> +++ b/include/rockusb.h
> @@ -0,0 +1,13 @@
> +/*
> + * (C) Copyright 2017
> + *
> + * Eddie Cai
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#ifndef _ROCKUSB_H_
> +#define _ROCKUSB_H_
> +
> +void rockusb_dev_init(char *dev_type, int dev_index);
> +
> +#endif /* _ROCKUSB_H_ */
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot 2/3] cmd: add rockusb command
Hi Eddie, On 20 March 2017 at 02:14, Eddie Cai wrote: > Hi Simon > > 2017-03-20 10:29 GMT+08:00 Simon Glass : >> >> Hi Eddie, >> >> On 15 March 2017 at 01:56, Eddie Cai wrote: >> > >> > this patch add rockusb command. the usage is >> > rockusb [] >> > e.g. rockusb 0 mmc 0 >> > >> > Signed-off-by: Eddie Cai >> > --- >> > 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 >> [..] >> >> > + "rockusb [] 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 000..cdea63d >> > --- /dev/null >> > +++ b/include/rockusb.h >> >> Can this go in include/asm instead? > > do you mean include/asm-generic ? may i know why ? No I was thinking of arch/arm/include/asm/arch-rockchip. Because this is a rockchip-specific file, isn't it? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot 2/3] cmd: add rockusb command
Hi Simon
2017-03-20 10:29 GMT+08:00 Simon Glass :
> Hi Eddie,
>
> On 15 March 2017 at 01:56, Eddie Cai wrote:
> >
> > this patch add rockusb command. the usage is
> > rockusb []
> > e.g. rockusb 0 mmc 0
> >
> > Signed-off-by: Eddie Cai
> > ---
> > 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_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?
>
Yes, This is vendor specific class.
>
> > +
> > +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?
>
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 000..f5bd86e
> > --- /dev/null
> > +++ b/cmd/rockusb.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright (C) 2017 Eddie Cai
> > + *
> > + * SPDX-License-Identifier:GPL-2.0+
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> 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 [] e.g. rockusb 0
> mmc 0\n
Re: [U-Boot] [U-Boot 2/3] cmd: add rockusb command
Hi Eddie,
On 15 March 2017 at 01:56, Eddie Cai wrote:
>
> this patch add rockusb command. the usage is
> rockusb []
> e.g. rockusb 0 mmc 0
>
> Signed-off-by: Eddie Cai
> ---
> 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 000..f5bd86e
> --- /dev/null
> +++ b/cmd/rockusb.c
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright (C) 2017 Eddie Cai
> + *
> + * SPDX-License-Identifier:GPL-2.0+
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
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 [] 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 000..cdea63d
> --- /dev/null
> +++ b/include/rockusb.h
Can this go in include/asm instead?
> @@ -0,0 +1,13 @@
> +/*
> + * (C) Copyright 2017
> + *
> + * Eddie Cai
> + *
> + * 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
[U-Boot] [U-Boot 2/3] cmd: add rockusb command
this patch add rockusb command. the usage is
rockusb []
e.g. rockusb 0 mmc 0
Signed-off-by: Eddie Cai
---
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
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.
+
+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
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 000..f5bd86e
--- /dev/null
+++ b/cmd/rockusb.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2017 Eddie Cai
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+
+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);
+
+ 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;
+ printf("do_rockusb1\n");
+ 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");
+ 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",
+ "rockusb [] 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 000..cdea63d
--- /dev/null
+++ b/include/rockusb.h
@@ -0,0 +1,13 @@
+/*
+ * (C) Copyright 2017
+ *
+ * Eddie Cai
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+#ifndef _ROCKUSB_H_
+#define _ROCKUSB_H_
+
+void rockusb_dev_init(char *dev_type, int dev_index);
+
+#endif /* _ROCKUSB_H_ */
--
2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
