All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V10 1/4] usb: rockchip: add the rockusb gadget
Date: Thu, 30 Nov 2017 13:46:55 +0100	[thread overview]
Message-ID: <1e042fc0-e367-32d7-e60c-0a42b2e89d4c@gmail.com> (raw)
In-Reply-To: <1512023924-28841-2-git-send-email-eddie.cai.linux@gmail.com>

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@gmail.com>
> Reviewed-by: Simon Glass <sjg@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@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

  reply	other threads:[~2017-11-30 12:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30  6:38 [U-Boot] [PATCH V10 0/4]introduce Rockchip rockusb Eddie Cai
2017-11-30  6:38 ` [U-Boot] [PATCH V10 1/4] usb: rockchip: add the rockusb gadget Eddie Cai
2017-11-30 12:46   ` Marek Vasut [this message]
2017-11-30  6:38 ` [U-Boot] [PATCH V10 2/4] usb: rockchip: add rockusb command Eddie Cai
2017-11-30  6:38 ` [U-Boot] [PATCH V10 3/4] rockchip:usb: add a simple readme for rockusb Eddie Cai
2017-11-30  6:38 ` [U-Boot] [PATCH V10 4/4] rockchip: rk3288: enable rockusb support on rk3288 based device Eddie Cai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1e042fc0-e367-32d7-e60c-0a42b2e89d4c@gmail.com \
    --to=marek.vasut@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.