[U-Boot] [PATCH V10 1/4] usb: rockchip: add the rockusb gadget

Marek Vasut marek.vasut at gmail.com
Thu Nov 30 12:46:55 UTC 2017


On 11/30/2017 07:38 AM, Eddie Cai wrote:
> this patch implement rockusb protocol on the device side. this is based on USB
> download gadget infrastructure. the rockusb function implements the rd, wl, rid
> commands. it can work with rkdeveloptool
> 
> Signed-off-by: Eddie Cai <eddie.cai.linux at gmail.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Changes in v10:
> -fix build error
> 
> Changes in v9:
> -fix compile error
> 
> Changes in v8:
> -none
> 
> Changes in v7:
> -none
> 
> Changes in v6:
> -move some data to f_rockusb structure
> 
> Changes in v5:
> -fix build error when build non-rockchip board
> -fix checkpatch error
> 
> Changes in v4:
> -use enum instead of macro define
> -move some structure define and macro to f_rockusb.h
> -add some function comment as Simon required
> -address other comment from Simon
> -fix build error as Lukasz point out
> 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h | 132 +++++
>  drivers/usb/gadget/Kconfig                     |   8 +
>  drivers/usb/gadget/Makefile                    |   1 +
>  drivers/usb/gadget/f_rockusb.c                 | 691 +++++++++++++++++++++++++
>  4 files changed, 832 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-rockchip/f_rockusb.h
>  create mode 100644 drivers/usb/gadget/f_rockusb.c
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
> new file mode 100644
> index 0000000..c207a78
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
> @@ -0,0 +1,132 @@
> +/*
> + * (C) Copyright 2017
> + *
> + * Eddie Cai <eddie.cai.linux at gmail.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _F_ROCKUSB_H_
> +#define _F_ROCKUSB_H_
> +#include <blk.h>
> +
> +#define ROCKUSB_VERSION		"0.1"
> +
> +#define ROCKUSB_INTERFACE_CLASS	0xff
> +#define ROCKUSB_INTERFACE_SUB_CLASS	0x06
> +#define ROCKUSB_INTERFACE_PROTOCOL	0x05
> +
> +#define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_2_0  (0x0200)
> +#define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1  (0x0040)
> +#define TX_ENDPOINT_MAXIMUM_PACKET_SIZE      (0x0040)

Remove the parenthesis around the values

> +#define EP_BUFFER_SIZE			4096
> +/*
> + * EP_BUFFER_SIZE must always be an integral multiple of maxpacket size
> + * (64 or 512 or 1024), else we break on certain controllers like DWC3
> + * that expect bulk OUT requests to be divisible by maxpacket size.
> + */
> +
> +#define CONFIG_ROCKUSB_BUF_ADDR		CONFIG_SYS_LOAD_ADDR

Why is this not dynamically allocated ? Using default load address for
this looks pretty weird.

> +#define CONFIG_ROCKUSB_BUF_SIZE		0x08000000
> +
> +#define RKUSB_STATUS_IDLE			0
> +#define RKUSB_STATUS_CMD			1
> +#define RKUSB_STATUS_RXDATA			2
> +#define RKUSB_STATUS_TXDATA			3
> +#define RKUSB_STATUS_CSW			4
> +#define RKUSB_STATUS_RXDATA_PREPARE		5
> +#define RKUSB_STATUS_TXDATA_PREPARE		6
Fix all the other checkpatch errors ...

$ ./scripts/checkpatch.pl
/tmp/0001-usb-rockchip-add-the-rockusb-gadget.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#6:
this patch implement rockusb protocol on the device side. this is based
on USB

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48:
new file mode 100644

WARNING: Missing a blank line after declarations
#328: FILE: drivers/usb/gadget/f_rockusb.c:107:
+       struct f_rockusb *f_rkusb = rockusb_func;
+       if (!f_rkusb) {

WARNING: Missing a blank line after declarations
#352: FILE: drivers/usb/gadget/f_rockusb.c:131:
+       int status = req->status;
+       if (!status)

CHECK: Alignment should match open parenthesis
#447: FILE: drivers/usb/gadget/f_rockusb.c:226:
+static int rockusb_set_alt(struct usb_function *f,
+                           unsigned interface, unsigned alt)

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#447: FILE: drivers/usb/gadget/f_rockusb.c:226:
+                           unsigned interface, unsigned alt)

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#447: FILE: drivers/usb/gadget/f_rockusb.c:226:
+                           unsigned interface, unsigned alt)

WARNING: Missing a blank line after declarations
#523: FILE: drivers/usb/gadget/f_rockusb.c:302:
+       struct f_rockusb *f_rkusb = get_rkusb();
+       f_rkusb->rockusb_dev_type = dev_type;

WARNING: Prefer using '"%s...", __func__' to using
'rx_handler_dl_image', this function's name, in a string
#625: FILE: drivers/usb/gadget/f_rockusb.c:404:
+               printf("rx_handler_dl_image blk_get_dev\n");

CHECK: Alignment should match open parenthesis
#627: FILE: drivers/usb/gadget/f_rockusb.c:406:
+               f_rkusb->download_desc =
blk_get_dev(f_rkusb->rockusb_dev_type,
+                               f_rkusb->rockusb_dev_index);

CHECK: spaces preferred around that '/' (ctx:VxV)
#653: FILE: drivers/usb/gadget/f_rockusb.c:432:
+               int blks = 0, blkcnt = f_rkusb->download_size/512;
                                                             ^

WARNING: Missing a blank line after declarations
#654: FILE: drivers/usb/gadget/f_rockusb.c:433:
+               int blks = 0, blkcnt = f_rkusb->download_size/512;
+               printf("download %d bytes finished, start writing to lba
%x\n",

CHECK: Alignment should match open parenthesis
#661: FILE: drivers/usb/gadget/f_rockusb.c:440:
+                       printf("failed writing to device %s: %d\n",
+                             f_rkusb->rockusb_dev_type,

WARNING: Prefer using '"%s...", __func__' to using 'cb_read_storage_id',
this function's name, in a string
#699: FILE: drivers/usb/gadget/f_rockusb.c:478:
+       printf("cb_read_storage_id\n");

WARNING: Comparisons should place the constant on the right side of the test
#719: FILE: drivers/usb/gadget/f_rockusb.c:498:
+       if ((0 == f_rkusb->download_size) ||

WARNING: Prefer using '"%s...", __func__' to using
'rkusb_set_reboot_flag', this function's name, in a string
#733: FILE: drivers/usb/gadget/f_rockusb.c:512:
+       printf("rkusb_set_reboot_flag: %d\n", f_rkusb->reboot_flag);

WARNING: ENOSYS means 'invalid syscall nr' and nothing else
#734: FILE: drivers/usb/gadget/f_rockusb.c:513:
+       return -ENOSYS;

WARNING: Missing a blank line after declarations
#875: FILE: drivers/usb/gadget/f_rockusb.c:654:
+       void (*func_cb)(struct usb_ep *ep, struct usb_request *req) = NULL;
+       ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,

WARNING: Missing a blank line after declarations
#901: FILE: drivers/usb/gadget/f_rockusb.c:680:
+                       u8 *buf = (u8 *)req->buf;
+                       buf[req->actual] = 0;

total: 0 errors, 15 warnings, 4 checks, 844 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or
--fix-inplace.

/tmp/0001-usb-rockchip-add-the-rockusb-gadget.patch has style problems,
please review.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
PREFER_ETHER_ADDR_COPY USLEEP_RANGE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list