Re: [PATCH] usb: gadget: add Faraday fotg210_udc driver

From: Felipe Balbi
Date: Wed May 29 2013 - 19:56:21 EST


Hi,

On Wed, May 29, 2013 at 06:31:50PM +0000, Yuan-Hsin Chen wrote:
> Faraday fotg210 udc driver supports only Bulk transfer so far.
> fotg210 could be configured as an USB2.0 peripheral.
>
> This driver is tested with mass storage gadget driver on Faraday
> EVB a369.
>
> Signed-off-by: Yuan-Hsin Chen <yhchen@xxxxxxxxxxxxxxxx>

Run through checkpatch.pl --strict and fix all errors, warnings and all
checks which make sense.

WARNING: please write a paragraph that describes the config symbol fully
#81: FILE: drivers/usb/gadget/Kconfig:196:
+config USB_FOTG210_UDC

CHECK: braces {} should be used on all arms of this statement
#183: FILE: drivers/usb/gadget/fotg210-udc.c:76:
+ if (ep->epnum) {
[...]
+ } else
[...]

WARNING: line over 80 characters
#190: FILE: drivers/usb/gadget/fotg210-udc.c:83:
+static void fotg210_fifo_ep_mapping(struct fotg210_ep *ep, u32 epnum, u32 dir_in)

WARNING: line over 80 characters
#237: FILE: drivers/usb/gadget/fotg210-udc.c:130:
+static void fotg210_set_mps(struct fotg210_ep *ep, u32 epnum, u32 mps, u32 dir_in)

WARNING: line over 80 characters
#241: FILE: drivers/usb/gadget/fotg210-udc.c:134:
+ u32 offset = dir_in ? FOTG210_INEPMPSR(epnum) : FOTG210_OUTEPMPSR(epnum);

WARNING: line over 80 characters
#349: FILE: drivers/usb/gadget/fotg210-udc.c:242:
+static void fotg210_ep_free_request(struct usb_ep *_ep, struct usb_request *_req)

WARNING: Avoid CamelCase: <DMATFNR_ACC_Fn>
#372: FILE: drivers/usb/gadget/fotg210-udc.c:265:
+ value |= DMATFNR_ACC_Fn(ep->epnum - 1);

WARNING: line over 80 characters
#418: FILE: drivers/usb/gadget/fotg210-udc.c:311:
+ value = ioread32(ep->fotg210->reg + FOTG210_FIBCR(ep->epnum - 1));

WARNING: line over 80 characters
#420: FILE: drivers/usb/gadget/fotg210-udc.c:313:
+ iowrite32(value, ep->fotg210->reg + FOTG210_FIBCR(ep->epnum - 1));

WARNING: line over 80 characters
#441: FILE: drivers/usb/gadget/fotg210-udc.c:334:
+ length = ioread32(ep->fotg210->reg + FOTG210_FIBCR(ep->epnum - 1));

WARNING: Avoid CamelCase: <FIBCR_BCFx>
#442: FILE: drivers/usb/gadget/fotg210-udc.c:335:
+ length &= FIBCR_BCFx;

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#456: FILE: drivers/usb/gadget/fotg210-udc.c:349:
+ printk(KERN_DEBUG "dma_mapping_error\n");

CHECK: Alignment should match open parenthesis
#461: FILE: drivers/usb/gadget/fotg210-udc.c:354:
+ dma_sync_single_for_device(NULL, d, length,
+ ep->dir_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);

WARNING: line over 80 characters
#476: FILE: drivers/usb/gadget/fotg210-udc.c:369:
+static void fotg210_ep0_queue(struct fotg210_ep *ep, struct fotg210_request *req)

CHECK: braces {} should be used on all arms of this statement
#483: FILE: drivers/usb/gadget/fotg210-udc.c:376:
+ if (req->req.length) {
[...]
+ } else
[...]

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#486: FILE: drivers/usb/gadget/fotg210-udc.c:379:
+ printk(KERN_DEBUG "%s : req->req.length = 0x%x\n",

CHECK: Alignment should match open parenthesis
#487: FILE: drivers/usb/gadget/fotg210-udc.c:380:
+ printk(KERN_DEBUG "%s : req->req.length = 0x%x\n",
+ __func__, req->req.length);

CHECK: braces {} should be used on all arms of this statement
#492: FILE: drivers/usb/gadget/fotg210-udc.c:385:
+ if (!req->req.length)
[...]
+ else {
[...]

WARNING: line over 80 characters
#495: FILE: drivers/usb/gadget/fotg210-udc.c:388:
+ u32 value = ioread32(ep->fotg210->reg + FOTG210_DMISGR0);

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#680: FILE: drivers/usb/gadget/fotg210-udc.c:573:
+ printk(KERN_DEBUG " 0x%x\n", data);

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#691: FILE: drivers/usb/gadget/fotg210-udc.c:584:
+ printk(KERN_DEBUG " 0x%x\n", data);

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#696: FILE: drivers/usb/gadget/fotg210-udc.c:589:
+ printk(KERN_DEBUG " 0x%x\n", data);

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#702: FILE: drivers/usb/gadget/fotg210-udc.c:595:
+ printk(KERN_DEBUG " 0x%x\n", data);

WARNING: Prefer netdev_dbg(netdev, ... then dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
#741: FILE: drivers/usb/gadget/fotg210-udc.c:634:
+ printk(KERN_DEBUG "request error!!\n");

WARNING: line over 80 characters
#744: FILE: drivers/usb/gadget/fotg210-udc.c:637:
+static void fotg210_set_address(struct fotg210_udc *fotg210, struct usb_ctrlrequest *ctrl)

WARNING: Avoid CamelCase: <ctrl->wValue>
#746: FILE: drivers/usb/gadget/fotg210-udc.c:639:
+ if (ctrl->wValue >= 0x0100)

CHECK: braces {} should be used on all arms of this statement
#746: FILE: drivers/usb/gadget/fotg210-udc.c:639:
+ if (ctrl->wValue >= 0x0100)
[...]
+ else {
[...]

WARNING: line over 80 characters
#754: FILE: drivers/usb/gadget/fotg210-udc.c:647:
+static void fotg210_set_feature(struct fotg210_udc *fotg210, struct usb_ctrlrequest *ctrl)

WARNING: Avoid CamelCase: <ctrl->bRequestType>
#756: FILE: drivers/usb/gadget/fotg210-udc.c:649:
+ switch (ctrl->bRequestType & USB_RECIP_MASK) {

WARNING: Avoid CamelCase: <ctrl->wIndex>
#765: FILE: drivers/usb/gadget/fotg210-udc.c:658:
+ epnum = le16_to_cpu(ctrl->wIndex) & USB_ENDPOINT_NUMBER_MASK;

WARNING: line over 80 characters
#779: FILE: drivers/usb/gadget/fotg210-udc.c:672:
+static void fotg210_clear_feature(struct fotg210_udc *fotg210, struct usb_ctrlrequest *ctrl)

WARNING: line over 80 characters
#821: FILE: drivers/usb/gadget/fotg210-udc.c:714:
+static void fotg210_get_status(struct fotg210_udc *fotg210, struct usb_ctrlrequest *ctrl)

WARNING: line over 80 characters
#836: FILE: drivers/usb/gadget/fotg210-udc.c:729:
+ fotg210_is_epnstall(fotg210->ep[epnum]) << USB_ENDPOINT_HALT;

WARNING: line over 80 characters
#854: FILE: drivers/usb/gadget/fotg210-udc.c:747:
+static int fotg210_setup_packet(struct fotg210_udc *fotg210, struct usb_ctrlrequest *ctrl)

CHECK: braces {} should be used on all arms of this statement
#870: FILE: drivers/usb/gadget/fotg210-udc.c:763:
+ if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD) {
[...]
+ } else
[...]

WARNING: Avoid CamelCase: <ctrl->bRequest>
#871: FILE: drivers/usb/gadget/fotg210-udc.c:764:
+ switch (ctrl->bRequest) {

CHECK: braces {} should be used on all arms of this statement
#902: FILE: drivers/usb/gadget/fotg210-udc.c:795:
+ if (!list_empty(&ep->queue) && !ep->dir_in) {
[...]
+ } else
[...]

CHECK: braces {} should be used on all arms of this statement
#921: FILE: drivers/usb/gadget/fotg210-udc.c:814:
+ if ((!list_empty(&ep->queue)) && (ep->dir_in)) {
[...]
+ } else
[...]

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#989: FILE: drivers/usb/gadget/fotg210-udc.c:882:
+ printk(KERN_INFO"fotg210 udc reset\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#995: FILE: drivers/usb/gadget/fotg210-udc.c:888:
+ printk(KERN_INFO"fotg210 udc suspend\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1001: FILE: drivers/usb/gadget/fotg210-udc.c:894:
+ printk(KERN_INFO"fotg210 udc resume\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1007: FILE: drivers/usb/gadget/fotg210-udc.c:900:
+ printk(KERN_INFO"fotg210 iso sequence error\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1013: FILE: drivers/usb/gadget/fotg210-udc.c:906:
+ printk(KERN_INFO"fotg210 iso sequence abort\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1020: FILE: drivers/usb/gadget/fotg210-udc.c:913:
+ printk(KERN_INFO"fotg210 transferred 0 byte\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1027: FILE: drivers/usb/gadget/fotg210-udc.c:920:
+ printk(KERN_INFO"fotg210 received 0 byte\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1047: FILE: drivers/usb/gadget/fotg210-udc.c:940:
+ printk(KERN_INFO "fotg210 CX command abort\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1051: FILE: drivers/usb/gadget/fotg210-udc.c:944:
+ printk(KERN_INFO "fotg210 ep0 setup\n");

WARNING: line over 80 characters
#1054: FILE: drivers/usb/gadget/fotg210-udc.c:947:
+ if (fotg210->driver->setup(&fotg210->gadget, &ctrl) < 0)

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1060: FILE: drivers/usb/gadget/fotg210-udc.c:953:
+ printk(KERN_INFO "fotg210 cmd end\n");

WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1070: FILE: drivers/usb/gadget/fotg210-udc.c:963:
+ printk(KERN_INFO "fotg210 ep0 fail\n");

CHECK: Alignment should match open parenthesis
#1129: FILE: drivers/usb/gadget/fotg210-udc.c:1022:
+ iowrite32(GMIR_MHC_INT | GMIR_MOTG_INT | GMIR_INT_POLARITY,
+ fotg210->reg + FOTG210_GMIR);

WARNING: space prohibited before semicolon
#1237: FILE: drivers/usb/gadget/fotg210-udc.c:1130:
+ for (i = 0; i < FOTG210_MAX_NUM_EP ; i++) {

CHECK: Alignment should match open parenthesis
#1243: FILE: drivers/usb/gadget/fotg210-udc.c:1136:
+ list_add_tail(&fotg210->ep[i]->ep.ep_list,
+ &fotg210->gadget.ep_list);

WARNING: kfree(NULL) is safe this check is probably not required
#1291: FILE: drivers/usb/gadget/fotg210-udc.c:1184:
+ if (fotg210)
+ kfree(fotg210);

CHECK: No space is necessary after a cast
#1298: FILE: drivers/usb/gadget/fotg210-udc.c:1191:
+ .driver = {
+ .name = (char *) udc_name,

WARNING: line over 80 characters
#1477: FILE: drivers/usb/gadget/fotg210.h:162:
+#define EPMAP_FIFONO(ep, dir) ((((ep) - 1) << ((ep) - 1) * 8) << ((dir) ? 0 : 4))

WARNING: line over 80 characters
#1478: FILE: drivers/usb/gadget/fotg210.h:163:
+#define EPMAP_FIFONOMSK(ep, dir) ((3 << ((ep) - 1) * 8) << ((dir) ? 0 : 4))

CHECK: spinlock_t definition without comment
#1549: FILE: drivers/usb/gadget/fotg210.h:234:
+ spinlock_t lock;

total: 0 errors, 45 warnings, 13 checks, 1472 lines checked

Your patch has style problems, please review.

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

--
balbi

Attachment: signature.asc
Description: Digital signature