linux-kernel - Re: [PATCH v5] usb: gadget: Add UDC driver for Aeroflex Gaisler GRUSBDC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20131223171122.GI28679@saruman.home>
Date:	Mon, 23 Dec 2013 11:11:22 -0600
From:	Felipe Balbi <balbi@...com>
To:	Andreas Larsson <andreas@...sler.com>
CC:	Felipe Balbi <balbi@...com>,
	Robert Baldyga <r.baldyga@...sung.com>,
	<linux-usb@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<software@...sler.com>
Subject: Re: [PATCH v5] usb: gadget: Add UDC driver for Aeroflex Gaisler
 GRUSBDC

Hi,

On Mon, Dec 23, 2013 at 12:57:38PM +0100, Andreas Larsson wrote:
> This adds an UDC driver for GRUSBDC USB Device Controller cores available in the
> GRLIB VHDL IP core library. The driver only supports DMA mode.
> 
> Signed-off-by: Andreas Larsson <andreas@...sler.com>

You have a few Sparse Warnings:

drivers/usb/gadget/gr_udc.c:1255:13: warning: incorrect type in argument 1 (different address spaces)
drivers/usb/gadget/gr_udc.c:1255:13:    expected void [noderef] <asn:2>*<noident>
drivers/usb/gadget/gr_udc.c:1255:13:    got unsigned int *<noident>
drivers/usb/gadget/gr_udc.c:1284:16: warning: incorrect type in argument 1 (different address spaces)
drivers/usb/gadget/gr_udc.c:1284:16:    expected void [noderef] <asn:2>*<noident>
drivers/usb/gadget/gr_udc.c:1284:16:    got unsigned int *<noident>
drivers/usb/gadget/gr_udc.c:1136:28: warning: context imbalance in 'gr_ep0_setup' - unexpected unlock
drivers/usb/gadget/gr_udc.c:1052:1: warning: "/*" within comment [-Wcomment]
 /* Must be called with dev->lock held and irqs disabled

Two of them I fixed below:

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index ee220f3..5eb36af 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -218,7 +218,7 @@ config USB_FOTG210_UDC
 
 config USB_GR_UDC
        tristate "Aeroflex Gaisler GRUSBDC USB Peripheral Controller Driver"
-       depends on OF && HAS_DMA
+       depends on HAS_DMA
        help
           Select this to support Aeroflex Gaisler GRUSBDC cores from the GRLIB
 	  VHDL IP core library.
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 4f99a61..9ac378a 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -1049,9 +1049,11 @@ static void gr_ep0out_requeue(struct gr_udc *dev)
 /*
  * The main function dealing with setup requests on ep0.
  *
-/* Must be called with dev->lock held and irqs disabled
+ * Must be called with dev->lock held and irqs disabled
  */
 static void gr_ep0_setup(struct gr_udc *dev, struct gr_request *req)
+	__releases(&dev->lock)
+	__acquires(&dev->lock)
 {
 	union {
 		struct usb_ctrlrequest ctrl;

The other two are seem like they would require a much larger code
change. Why are you passing integers as arguments to IO access
functions ? You should be passing void __iomem *.

Dropping for now.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)