date:20160105

We added a new error path to this function and we forgot to drop the
lock.

Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
Signed-off-by: Dan Carpenter 
---
v2: Felipe asked for this to be fixed a different way.

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index fb1fe96d..7d28944 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1163,24 +1163,25 @@ static void f_midi_unbind(struct usb_configuration *c, 
struct usb_function *f)
 
 static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
 {
-   struct f_midi *midi;
+   struct f_midi *midi = NULL;
struct f_midi_opts *opts;
-   int status, i;
+   int status;
+   int i = 0;
 
opts = container_of(fi, struct f_midi_opts, func_inst);
 
mutex_lock(&opts->lock);
/* sanity check */
if (opts->in_ports > MAX_PORTS || opts->out_ports > MAX_PORTS) {
-   mutex_unlock(&opts->lock);
-   return ERR_PTR(-EINVAL);
+   status = -EINVAL;
+   goto setup_fail;
}
 
/* allocate and initialize one new instance */
midi = kzalloc(sizeof(*midi), GFP_KERNEL);
if (!midi) {
-   mutex_unlock(&opts->lock);
-   return ERR_PTR(-ENOMEM);
+   status = -ENOMEM;
+   goto setup_fail;
}
 
for (i = 0; i < opts->in_ports; i++) {
@@ -1188,7 +1189,6 @@ static struct usb_function *f_midi_alloc(struct 
usb_function_instance *fi)
 
if (!port) {
status = -ENOMEM;
-   mutex_unlock(&opts->lock);
goto setup_fail;
}
 
@@ -1202,7 +1202,6 @@ static struct usb_function *f_midi_alloc(struct 
usb_function_instance *fi)
midi->id = kstrdup(opts->id, GFP_KERNEL);
if (opts->id && !midi->id) {
status = -ENOMEM;
-   mutex_unlock(&opts->lock);
goto setup_fail;
}
midi->in_ports = opts->in_ports;
@@ -1229,6 +1228,7 @@ static struct usb_function *f_midi_alloc(struct 
usb_function_instance *fi)
return &midi->func;
 
 setup_fail:
+   mutex_unlock(&opts->lock);
for (--i; i >= 0; i--)
kfree(midi->in_port[i]);
kfree(midi);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 04/11] usbip: kernel module for userspace URBs transmission

2016-01-05 Thread Bjørn Mork

Nobuo Iwata  writes:

> Originally, USB/IP transmits requests and response PDUs for preparation 
> to transfer URBs in user space, after the preparation, URBs are 
> transmitted in kernel space.
>
> To make easy to introduce application network protocols like WebSocket 
> and so on, the driver, usbip_ux.ko, forwards URBs to USB/IP user space 
> utilities. It's just like fuse driver for user space file system. 
> Then, utilities transfer URBs in user space.
>
> To do so, usbip_trx_ops makes send/receive functions pluggable. 
> kernel_sendmsg() and kernel_recvmsg() for kernel mode transfer can be 
> substituted by read/write methods to user space utilities.
>
> In the absence of usbip_ux.ko, original kernel space transferring is 
> valid. usbip_ux.ko replaces usbip_trx_ops in its init routine.
>
> A) Original - kernel space URBs transfer
>
> User+---+   1) import/export   +---+
> space   |uspipd,|<>|usbip, |
> |usbip  |  |usbipa |
> +---+---+  +---+---+
> |  |
> 2)Set sockfd|  |2)Set sockfd
>   thru sysfs|  |  thru sysfs
> V  V
> Kernel  +---+4)URBs+---+
> space   |usbip  |<>|vhci   |
> |host.ko|  |hcd.ko |
> +---+  +---+
> 3)link to kernel trx_ops   3)link to kernel trx_ops

AFAICS, usbip-host.ko and vhci-hcd.ko are running on different hosts,
likely on different hardware and kernel versions, so this picture is
impossible.

But I'm not going to be more difficult than necessary :) I can see what
you are getting at: There is a kernel TCP socket on each side of the
"4)URBs" channel, and you want to be able to replace it with something
else.

But I am still not convinced about this strategy.  There is nothing in
the old protocol preventing you from terminating a TCP session locally
on each host, and do whatever you like between these two TCP sessions.
You can implement a TCP<->websocket proxy in userspace on each side of
the link without hooking into the kernel at all, passing one end of each
session to the existing usbip kernel implementation.

Or am I missing something?

> B) New - user space URBs transfer
>
> User+---+1)import/export   +---+
> space   |uspipd,|<>|usbip, |
> +---|usbip  |<>|usbipa |---+
> 2)Set sockfd|+--+---+6)URBs+---+--+|2)Set sockfd
>   thru ioctl||  ^ ^   ||  thru ioctl
>   (right)   ||  |5)read/write 5)read/write|   ||  (left)
> 3)Set sockfd||  +---+  +--+   ||3)Set Sockfd
>   thru sysfs|+---+  | /dev/usbip-ux|  +---+|  thru sysfs
>   (left)VV  V  V  VV  (right)
> Kernel  +---+   +---+  +---+   +---+
> space   |usbip  |<->|usbip  |  |usbip  |<->|vhci   |
> |host.ko|5)send |ux.ko  |  |ux.ko  |5)send |hcd.ko |
> +---+  recv +---+  +---+  recv +---+
> 4)link to user trx_ops 4)link to user trx_ops

Thanks for trying to draw it up.  That really helps understaning. But
this got a little complicated  I assume you keep the protocol (the
"1) import/export" arrow) compatible, so that you can mix and match
(assuming fallback to TCP transport):

 a) old and new client/server versions, and
 b) clients/servers with and without usbip-ux.ko

Is this correct?

How about userspace<->kernel compatibility in each side:  Is a new
userspace required if usbip-ux.ko is loaded, or will the old ABI
continue to work as before?

> @@ -335,26 +336,28 @@ int usbip_recv(struct socket *sock, void *buf, int size)
>  
>   usbip_dbg_xmit("enter\n");
>  
> - if (!sock || !buf || !size) {
> - pr_err("invalid arg, sock %p buff %p size %d\n", sock, buf,
> -size);
> + if ((!ud->tcp_socket && !ud->ux) || !buf || !size) {
> + pr_err("invalid arg, sock %p ux %p buff %p size %d\n",
> + ud->tcp_socket, ud->ux, buf, size);
>   return -EINVAL;
>   }
>  
>   do {
> - sock->sk->sk_allocation = GFP_NOIO;
>   iov.iov_base= buf;
>   iov.iov_len = size;
> - msg.msg_name= NULL;
> - msg.msg_namelen = 0;
> - msg.msg_control = NULL;
> - msg.msg_controllen = 0;
> - msg.msg_flags  = MSG_NOSIGNAL;
> + if (usbip_trx_ops->mode == USBIP_TRX_MODE_KERNEL) {

Testing for mode before calling a mode dependent callback looks weird.
Why don't you do all the USBIP_

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread kbuild test robot

Hi Dan,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.4-rc8 next-20160105]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL but 
>> dereferenced.

vim +1233 drivers/usb/gadget/function/f_midi.c

e1e3d7ec Felipe F. Tonello 2015-12-01  1217  
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1218 ++opts->refcnt;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1219 
mutex_unlock(&opts->lock);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1220  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1221 midi->func.name 
= "gmidi function";
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1222 midi->func.bind 
= f_midi_bind;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1223 midi->func.unbind   
= f_midi_unbind;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1224 midi->func.set_alt  
= f_midi_set_alt;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1225 midi->func.disable  
= f_midi_disable;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1226 midi->func.free_func
= f_midi_free;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1227  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1228 return &midi->func;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1229  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1230  setup_fail:
39920a18 Dan Carpenter 2016-01-05  1231 
mutex_unlock(&opts->lock);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1232 for (--i; i >= 0; i--)
b85e9de9 Andrzej Pietrasiewicz 2014-10-16 @1233 
kfree(midi->in_port[i]);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1234 kfree(midi);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1235 return ERR_PTR(status);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1236  }
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1237  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1238  
DECLARE_USB_FUNCTION_INIT(midi, f_midi_alloc_inst, f_midi_alloc);

:: The code at line 1233 was first introduced by commit
:: b85e9de9e818de0dcbc50b7b4242192eb6194855 usb: gadget: f_midi: convert to 
new function interface with backward compatibility

:: TO: Andrzej Pietrasiewicz 
:: CC: Felipe Balbi 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usbip: vhci number of ports extension

2016-01-05 Thread Bjørn Mork

Nobuo Iwata  writes:

> This patch extends number of ports limitation in application (vhci) 
> side.
>
> To do so, vhci driver supports multiple host controllers. The number of 
> controllers can be specified as a module parameter 'num_controllers'. 
> The default is 1.
>
> ex) # insmod vhci_hcd.ko num_controllers=4
>
> Also, ports per controller is changed from 8 to USB_MAXCHILDREN (31). 
> It can be modified with VHCI_NPORTS flag at module compilation.

Why do these numbers have to be static?  Why can't we automatically add
new controllers when there are no more ports avilable?



> So number of ports supported by vhci is 'num_controllers' * 31.
>
> Sysfs structure is changes as following.
> BEFORE:
> /sys/devices/platform
> +-- vhci
> +-- status
> +-- attach
> +-- detach
> +-- usbip_debug
> AFTER: example for num_controllers=4
> /sys/devices/platform
> +-- vhci.0
> |   +-- nports
> |   +-- status.0
> |   +-- status.1
> |   +-- status.2
> |   +-- status.3
> |   +-- attach
> |   +-- detach
> |   +-- usbip_debug
> +-- vhci.1
> +-- vhci.2
> +-- vhci.3

Again:  Not sure the existing files can be changed?  Or can we do that
because the userspace implementation is part of the kernel source repo?

> +static ssize_t nports_show(struct device *dev, struct device_attribute *attr,
> +char *out)
> +{
> + char *s = out;
> + out += sprintf(out, "%d %d\n", VHCI_NPORTS, num_controllers);

Definitely not.  One value per file.  Note that the num_controllers will
be readable as a module parameter by default.



Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 0/4] Allow USB devices to remain runtime-suspended when sleeping

2016-01-05 Thread Rafael J. Wysocki

On Monday, January 04, 2016 06:27:18 PM Derek Basehore wrote:
> On Mon, Nov 02, 2015 at 02:50:40AM +0100, Rafael J. Wysocki wrote:
> > 
> > I've queued up this series for the second half of the v4.4 merge window.
> > 
> > Thanks,
> > Rafael
> > 
> > 
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> What's the status of this patch series? I haven't seen the patches
> posted for v4.4, plus there's the issue that Dan found that needs to be
> addressed.
> 
> Is there a new revision of the patch series being worked on?

Tomeu is not working on one AFAICS, but I may just revive his series.

Do you have a pointer to the Dan's report?

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Julia Lawall



On Tue, 5 Jan 2016, kbuild test robot wrote:

> Hi Dan,
>
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.4-rc8 next-20160105]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
> >> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL but 
> >> dereferenced.

It's a false positive for coccinelle, but I wonder if avoiding duplicating
the mutex_lock is really worth it?  There is a slightly subtle interaction
between the possibility of midi being NULL and the value of i that make it
all work.

julia


>
> vim +1233 drivers/usb/gadget/function/f_midi.c
>
> e1e3d7ec Felipe F. Tonello 2015-12-01  1217
> 6f1de344 Andrzej Pietrasiewicz 2014-10-16  1218   ++opts->refcnt;
> 6f1de344 Andrzej Pietrasiewicz 2014-10-16  1219   
> mutex_unlock(&opts->lock);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1220
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1221   midi->func.name 
> = "gmidi function";
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1222   midi->func.bind 
> = f_midi_bind;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1223   midi->func.unbind   
> = f_midi_unbind;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1224   midi->func.set_alt  
> = f_midi_set_alt;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1225   midi->func.disable  
> = f_midi_disable;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1226   midi->func.free_func
> = f_midi_free;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1227
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1228   return &midi->func;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1229
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1230  setup_fail:
> 39920a18 Dan Carpenter 2016-01-05  1231   
> mutex_unlock(&opts->lock);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1232   for (--i; i >= 0; i--)
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16 @1233   
> kfree(midi->in_port[i]);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1234   kfree(midi);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1235   return ERR_PTR(status);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1236  }
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1237
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1238  
> DECLARE_USB_FUNCTION_INIT(midi, f_midi_alloc_inst, f_midi_alloc);
>
> :: The code at line 1233 was first introduced by commit
> :: b85e9de9e818de0dcbc50b7b4242192eb6194855 usb: gadget: f_midi: convert 
> to new function interface with backward compatibility
>
> :: TO: Andrzej Pietrasiewicz 
> :: CC: Felipe Balbi 
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

It's a false positive, if midi is NULL then i starts as zero so it
won't go inside the for loop.  Smatch has the same false positive.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete

2016-01-05 Thread Michal Nazarewicz

On Tue, Jan 05 2016, Peter Chen wrote:
> Why -EINTR, the kernel-doc said it should return -ECONNRESET for
> active request, see include/linux/usb/gadget.h.

Because EINTR is what read returns to the user if the operation has been
interrupted by a signal, see ‘man 2 read’:

   EINTR The call was interrupted by a signal before any data was
   read; see signal(7).


-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  ミハウ “mina86” ナザレヴイツ  (o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

On Tue, Jan 05, 2016 at 01:28:11PM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 5 Jan 2016, kbuild test robot wrote:
> 
> > Hi Dan,
> >
> > [auto build test WARNING on balbi-usb/next]
> > [also build test WARNING on v4.4-rc8 next-20160105]
> > [if your patch is applied to the wrong git tree, please drop us a note to 
> > help improving the system]
> >
> > url:
> > https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> >
> >
> > coccinelle warnings: (new ones prefixed by >>)
> >
> > >> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL but 
> > >> dereferenced.
> 
> It's a false positive for coccinelle, but I wonder if avoiding duplicating
> the mutex_lock is really worth it?

It's not the most beautiful code in the world.  I considered a bunch of
different ways to write it...  This is what Felipe Tonello wanted,
though.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Michal Nazarewicz

On Tue, Jan 05 2016, Dan Carpenter wrote:
> We added a new error path to this function and we forgot to drop the
> lock.
>
> Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
> Signed-off-by: Dan Carpenter 

Acked-by: Michal Nazarewicz 

> ---
> v2: Felipe asked for this to be fixed a different way.
>
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index fb1fe96d..7d28944 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1163,24 +1163,25 @@ static void f_midi_unbind(struct usb_configuration 
> *c, struct usb_function *f)
>  
>  static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
>  {
> - struct f_midi *midi;
> + struct f_midi *midi = NULL;
>   struct f_midi_opts *opts;
> - int status, i;
> + int status;
> + int i = 0;
>  
>   opts = container_of(fi, struct f_midi_opts, func_inst);
>  
>   mutex_lock(&opts->lock);
>   /* sanity check */
>   if (opts->in_ports > MAX_PORTS || opts->out_ports > MAX_PORTS) {
> - mutex_unlock(&opts->lock);
> - return ERR_PTR(-EINVAL);
> + status = -EINVAL;
> + goto setup_fail;
>   }
>  
>   /* allocate and initialize one new instance */
>   midi = kzalloc(sizeof(*midi), GFP_KERNEL);
>   if (!midi) {
> - mutex_unlock(&opts->lock);
> - return ERR_PTR(-ENOMEM);
> + status = -ENOMEM;
> + goto setup_fail;
>   }
>  
>   for (i = 0; i < opts->in_ports; i++) {
> @@ -1188,7 +1189,6 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>  
>   if (!port) {
>   status = -ENOMEM;
> - mutex_unlock(&opts->lock);
>   goto setup_fail;
>   }
>  
> @@ -1202,7 +1202,6 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>   midi->id = kstrdup(opts->id, GFP_KERNEL);
>   if (opts->id && !midi->id) {
>   status = -ENOMEM;
> - mutex_unlock(&opts->lock);
>   goto setup_fail;
>   }
>   midi->in_ports = opts->in_ports;
> @@ -1229,6 +1228,7 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>   return &midi->func;
>  
>  setup_fail:
> + mutex_unlock(&opts->lock);
>   for (--i; i >= 0; i--)
>   kfree(midi->in_port[i]);
>   kfree(midi);

How about some refactoring first:

 >8 
>From 81220372e4acce8f1ffee00338c24472469c1abe Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz 
Date: Tue, 5 Jan 2016 14:43:42 +0100
Subject: [PATCH 1/2] usb: gadget: f_midi: use flexible array member for
 gmidi_in_port elements

Reduce number of allocations, simplify memory management and reduce
memory usage by stacking the gmidi_in_port elements at the end of the
f_midi structure using a flexible array.

Also, observe that gmidi_in_port::midi pointer is *never* used for any
purpose so it can be safely removed.

Signed-off-by: Michal Nazarewicz 
---
 drivers/usb/gadget/function/f_midi.c | 42 
 1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 898a570..9338625 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
  * USB <- IN endpoint  <- rawmidi
  */
 struct gmidi_in_port {
-   struct f_midi *midi;
int active;
uint8_t cable;
uint8_t state;
@@ -78,7 +77,6 @@ struct f_midi {
 
struct snd_rawmidi_substream *in_substream[MAX_PORTS];
struct snd_rawmidi_substream *out_substream[MAX_PORTS];
-   struct gmidi_in_port*in_port[MAX_PORTS];
 
unsigned long   out_triggered;
struct tasklet_struct   tasklet;
@@ -87,6 +85,8 @@ struct f_midi {
int index;
char *id;
unsigned int buflen, qlen;
+
+   struct gmidi_in_portin_ports_array[/* in_ports */];
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -529,11 +529,11 @@ static void f_midi_transmit(struct f_midi *midi, struct 
usb_request *req)
req->length = 0;
req->complete = f_midi_complete;
 
-   for (i = 0; i < MAX_PORTS; i++) {
-   struct gmidi_in_port *port = midi->in_port[i];
+   for (i = 0; i < midi->in_ports; i++) {
+   struct gmidi_in_port *port = midi->in_ports_array + i;
struct snd_rawmidi_substream *substream = midi->in_substream[i];
 
-   if (!port || !port->active || !substream)
+   if (!port->active || !substream)
continue;
 
while (req->length + 3 < midi->buflen) {
@@ -568,12 +568,12

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

On Tue, Jan 05, 2016 at 02:55:31PM +0100, Michal Nazarewicz wrote:
> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream 
> *substream)
>  {
>   struct f_midi *midi = substream->rmidi->private_data;
>  
> - if (!midi->in_port[substream->number])
> + if (substream->number > midi->in_ports)

This is off by one.  It should be >= midi->in_ports.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: gadget: f_midi: use flexible array member for

2016-01-05 Thread kbuild test robot

Hi Michal,

[auto build test WARNING on v4.4-rc8]
[cannot apply to balbi-usb/next next-20160105]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Michal-Nazarewicz/usb-gadget-f_midi-use-flexible-array-member-for/20160105-215816
config: x86_64-randconfig-x016-01041832 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/usb/gadget/function/f_midi.c: In function 'f_midi_free':
>> drivers/usb/gadget/function/f_midi.c:1070:6: warning: unused variable 'i' 
>> [-Wunused-variable]
 int i;
 ^

vim +/i +1070 drivers/usb/gadget/function/f_midi.c

6f1de344 Andrzej Pietrasiewicz 2014-10-16  1054 opts->id = 
SNDRV_DEFAULT_STR1;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1055 opts->buflen = 256;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1056 opts->qlen = 32;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1057 opts->in_ports = 1;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1058 opts->out_ports = 1;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1059  
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1060 
config_group_init_type_name(&opts->func_inst.group, "",
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1061 
&midi_func_type);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1062  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1063 return &opts->func_inst;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1064  }
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1065  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1066  static void f_midi_free(struct 
usb_function *f)
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1067  {
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1068 struct f_midi *midi;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1069 struct f_midi_opts 
*opts;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16 @1070 int i;
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1071  
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1072 midi = func_to_midi(f);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1073 opts = 
container_of(f->fi, struct f_midi_opts, func_inst);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1074 kfree(midi->id);
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1075 mutex_lock(&opts->lock);
b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1076 kfree(midi);
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1077 --opts->refcnt;
6f1de344 Andrzej Pietrasiewicz 2014-10-16  1078 
mutex_unlock(&opts->lock);

:: The code at line 1070 was first introduced by commit
:: b85e9de9e818de0dcbc50b7b4242192eb6194855 usb: gadget: f_midi: convert to 
new function interface with backward compatibility

:: TO: Andrzej Pietrasiewicz 
:: CC: Felipe Balbi 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

2016-01-05 Thread Rob Herring

On Mon, Dec 21, 2015 at 2:33 AM, Peter Chen  wrote:
> On Fri, Dec 18, 2015 at 11:38 PM, Alan Stern  
> wrote:
>> On Fri, 18 Dec 2015, Peter Chen wrote:
>>
>>> On Fri, Dec 18, 2015 at 12:13 AM, Alan Stern  
>>> wrote:
>>
>>> > It's not clear (to me, anyway) how this is meant to work.  USB is a
>>> > completely discoverable bus: There is no way to represent devices
>>> > statically; they have to be discovered.  But a device can't be
>>> > discovered unless it is functional, so if an onboard hub (or any other
>>> > sort of USB device) requires power, clocks, or something similar in
>>> > order to function, it won't be discovered.  There won't be any device
>>> > structure to probe, and "forcing driver probe" won't accomplish
>>> > anything.
>>> >
>>> > It seems to me that the only way something like this could be made to
>>> > work is if the necessary actions are keyed off the host controller (and
>>> > in particular, _not_ the hub driver), presumably at the time the
>>> > controller is probed.

The problem with putting this in the the host controller driver is it
assumes the initialization sequence is generic enough to be described
in DT and that initialization is the only time we need DT data. The
MFD example is a perfect example of another case. Suspend/resume also
comes to mind. Even if we could put the control in both places, that
is poor design IMO. I'd rather just make it a requirement that the
bootloader do enough setup that devices can be discovered.

[...]

>>> +static int hub_of_children_register(struct usb_device *hdev)
>>> +{
>>> + struct device *dev;
>>> +
>>> + if (hdev->parent)
>>> + dev = &hdev->dev;
>>> + else
>>> + dev = bus_to_hcd(hdev->bus)->self.controller;
>>> +
>>> + if (!dev->of_node)
>>> + return 0;
>>
>> Suppose hdev->parent is not NULL (hdev isn't the root hub -- maybe it's
>> a 2nd-level hub).  Then how did hdev->dev->of_node get set?
>>
>
> Yes, the USB device doesn't know device node.

That should be a solvable problem...


> There are two problems which needs device tree to support, I have
> below solutions for them, please help to see if it is reasonable.
>
> 1. The USB device can't work without external clock, power, reset operation.
> At device tree, we have a node to describe external signals like clocks, gpios
> for all onboard devices under this controller. And this node is as phandle for
> host controller, see below:
>
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -108,6 +108,21 @@
> default-brightness-level = <7>;
> status = "okay";
> };
> +
> +   usbh1_pre_operation: usbh1_pre_operation {
> +   clocks = <&clks IMX6QDL_CLK_1>,
> +<&clks IMX6QDL_CLK_2>,
> +<&clks IMX6QDL_CLK_3>,
> +<&clks IMX6QDL_CLK_4>,
> +<&clks IMX6QDL_CLK_5>,
> +<&clks IMX6QDL_CLK_6>;
> +   reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>,
> + <&gpio4 7 GPIO_ACTIVE_LOW>,
> + <&gpio3 25 GPIO_ACTIVE_LOW>,
> + <&gpio3 27 GPIO_ACTIVE_LOW>,
> + <&gpio4 4 GPIO_ACTIVE_LOW>,
> + <&gpio4 6 GPIO_ACTIVE_LOW>;
> +   };
>  };
>
>  &clks {
> @@ -590,6 +605,8 @@
>  &usbh1 {
> vbus-supply = <®_usb_h1_vbus>;
> status = "okay";
> +
> +   devices-pre-operation = <&usbh1_pre_operation>
>  };

No. The binding should reflect the hardware connections. The hub is a
child of the USB controller/root hub. so the hub node had better be a
child of the controller in the DT. The clocks and reset are
connections to the hub, so they had better be in the hub's DT node.
There is really nothing more to discuss on the binding. Anything else
you are coming up with is working around kernel issues.


> At code, we add one API of_usb_pre_operation to get clocks and gpios through
> host controller's device node, and this API is called at usb_add_hcd,
> and opposite
> operation is called at usb_remove_hcd.
>
> This solution is almost the same with MMC power sequence solution.
> (see drivers/mmc/core/pwrseq.c)
>
> 2. There are MFD USB devices, which includes several interfaces under
> USB device,
> like i2c, gpios, etc. Due to lack of device tree support, USB
> class/device driver doesn't know
> which kinds of interfaces are needed for this board.

Are you talking about a device hard wired on the same board or
something like GPIOs on FTDI chip which could be hot-plugged in any
host (including non-DT based)?

For the hotplug case, we will need a way to associate a DT overlay
with the USB device and there may not even be a base DT to map the
overlay into. In this case, the USB device's driver will need to load
the overlay and trigger enumerating the child devices. Anyway, this is
a separate issue from

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, 
> >> __hc32 token)
> >>   return '/';
> >>  }
> >>
> >> -static void qh_lines(
> >> - struct ehci_hcd *ehci,
> >> - struct ehci_qh *qh,
> >> - char **nextp,
> >> - unsigned *sizep
> >> -)
> >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> >> + char **nextp, unsigned *sizep)
> >>  {
> >>   u32 scratch;
> >>   u32 hw_curr;
> >>
> >
> And about that style? Should be done?

You mean squeezing the function parameters into two lines?  That's 
okay.

However, the style in this file is to indent continuation lines by two
extra tab stops, not to line things up with an open paren on the first
line.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> 2016-01-04 18:01 GMT-03:00 Alan Stern :
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> >
> >> This patch fixes a coding style issue reported by checkpatch related to
> >> many leading tabs, removing a 'do while' loop and making use of goto tag 
> >> instead.
> >
> > This is highly questionable.  It's a big amount of code churn, nearly
> > impossible to verify visually, just to remove one level of indentation.
> > It also introduces an unnecessary backwards "goto", which seems like a
> > bad idea.
> After hear you I agree. I saw that others drivers uses similar
> structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
> would be the case in this file of moving code to a new function? If
> not, please disregard this patch.

Moving code into a new sub-function would be okay.

BTW, you don't need to post these patches to both linux-usb and LKML.  
linux-usb alone is good enough.  Nothing about the patches would be
especially interesting to a general Linux kernel programmer, so there's 
no point in bringing them to everybody's attention.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

2016-01-04 23:40 GMT-03:00 Joe Perches :
> On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
>> 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
>> > > > > > This patch fixes coding style issues reported by checkpatch 
>> > > > > > concerning
>> > > > > > to unnecessary space after a cast.
>> > > > > This is a case where checkpatch is wrong, IMO.  Casts should always 
>> > > > > be
>> > > > > followed by a space.  I will not accept this patch.
>
> Your choice, but most kernel code disagrees with you.
>
> measuring only kernel casts to a pointer, (because there are
> too many false positives otherwise) casts without a space
> are preferred ~3:1 over casts followed by a space.
>
> (without space)
> $ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)\w+" * 
> | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 36612
>
> (with space)
> $ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)[ 
> \t]\w+" * | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 13233

FWIW I really don't have a side here since it's not defined in Coding
Style. Please disregard this patch.

-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> 
> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct
> ehci_hcd *ehci, __hc32 token)
> > >>   return '/';
> > >>  }
> > >>
> > >> -static void qh_lines(
> > >> - struct ehci_hcd *ehci,
> > >> - struct ehci_qh *qh,
> > >> - char **nextp,
> > >> - unsigned *sizep
> > >> -)
> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > >> + char **nextp, unsigned *sizep)
> > >>  {
> > >>   u32 scratch;
> > >>   u32 hw_curr;
> > >>
> > >
> > And about that style? Should be done?
> 
> You mean squeezing the function parameters into two lines?  That's 
> okay.
> 
> However, the style in this file is to indent continuation lines by
> two
> extra tab stops, not to line things up with an open paren on the
> first
> line.
> 
> Alan Stern
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> 
> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd 
> > >> *ehci, __hc32 token)
> > >>   return '/';
> > >>  }
> > >>
> > >> -static void qh_lines(
> > >> - struct ehci_hcd *ehci,
> > >> - struct ehci_qh *qh,
> > >> - char **nextp,
> > >> - unsigned *sizep
> > >> -)
> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > >> + char **nextp, unsigned *sizep)
> > >>  {
> > >>   u32 scratch;
> > >>   u32 hw_curr;
> > >>
> > >
> > And about that style? Should be done?
> 
> You mean squeezing the function parameters into two lines?  That's 
> okay.
> 
> However, the style in this file is to indent continuation lines by two
> extra tab stops, not to line things up with an open paren on the first
> line.

It's not consistent.
It's a bit of a mix of 1 and 2 tabs, and some others.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

2016-01-05 12:23 GMT-03:00 Joe Perches :
> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
>> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>>
>> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd 
>> > >> *ehci, __hc32 token)
>> > >>   return '/';
>> > >>  }
>> > >>
>> > >> -static void qh_lines(
>> > >> - struct ehci_hcd *ehci,
>> > >> - struct ehci_qh *qh,
>> > >> - char **nextp,
>> > >> - unsigned *sizep
>> > >> -)
>> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>> > >> + char **nextp, unsigned *sizep)
>> > >>  {
>> > >>   u32 scratch;
>> > >>   u32 hw_curr;
>> > >>
>> > >
>> > And about that style? Should be done?
>>
>> You mean squeezing the function parameters into two lines?  That's
>> okay.
Yes. I'll change this patch to do only that squeezing.

>>
>> However, the style in this file is to indent continuation lines by two
>> extra tab stops, not to line things up with an open paren on the first
>> line.
I see. I used 3 tabs, reducing to 2.

>
> It's not consistent.
> It's a bit of a mix of 1 and 2 tabs, and some others.
I noticed it. Maybe to avoid the 80th column there are 1 tab indentations.

>
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

2016-01-05 12:27 GMT-03:00 Geyslan G. Bem :
> 2016-01-05 12:23 GMT-03:00 Joe Perches :
>> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
>>> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>>>
>>> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd 
>>> > >> *ehci, __hc32 token)
>>> > >>   return '/';
>>> > >>  }
>>> > >>
>>> > >> -static void qh_lines(
>>> > >> - struct ehci_hcd *ehci,
>>> > >> - struct ehci_qh *qh,
>>> > >> - char **nextp,
>>> > >> - unsigned *sizep
>>> > >> -)
>>> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>>> > >> + char **nextp, unsigned *sizep)
>>> > >>  {
>>> > >>   u32 scratch;
>>> > >>   u32 hw_curr;
>>> > >>
>>> > >
>>> > And about that style? Should be done?
>>>
>>> You mean squeezing the function parameters into two lines?  That's
>>> okay.
> Yes. I'll change this patch to do only that squeezing.
>
>>>
>>> However, the style in this file is to indent continuation lines by two
>>> extra tab stops, not to line things up with an open paren on the first
>>> line.
> I see. I used 3 tabs, reducing to 2.
>
>>
>> It's not consistent.
>> It's a bit of a mix of 1 and 2 tabs, and some others.
> I noticed it. Maybe to avoid the 80th column there are 1 tab indentations.

Others:

946
static struct debug_buffer *alloc_buffer(struct usb_bus *bus,
ssize_t (*fill_func)(struct debug_buffer *))

has 4 tabs on the second line.

986
static ssize_t debug_output(struct file *file, char __user *user_buf,
size_t len, loff_t *offset)

has 3 tabs and 4 spaces on the second line (aligned to open paren).

I think we should reduce them too to make the file consistent.

>
>>
>>
>
>
>
> --
> Regards,
>
> Geyslan G. Bem
> hackingbits.com



-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

2016-01-05 12:15 GMT-03:00 Alan Stern :
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> 2016-01-04 18:01 GMT-03:00 Alan Stern :
>> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>> >
>> >> This patch fixes a coding style issue reported by checkpatch related to
>> >> many leading tabs, removing a 'do while' loop and making use of goto tag 
>> >> instead.
>> >
>> > This is highly questionable.  It's a big amount of code churn, nearly
>> > impossible to verify visually, just to remove one level of indentation.
>> > It also introduces an unnecessary backwards "goto", which seems like a
>> > bad idea.
>> After hear you I agree. I saw that others drivers uses similar
>> structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
>> would be the case in this file of moving code to a new function? If
>> not, please disregard this patch.
>
> Moving code into a new sub-function would be okay.
Ok.

>
> BTW, you don't need to post these patches to both linux-usb and LKML.
> linux-usb alone is good enough.  Nothing about the patches would be
> especially interesting to a general Linux kernel programmer, so there's
> no point in bringing them to everybody's attention.
You're right. I used git send-email
--cc-cmd="scripts/get_maintainer.pl -i". Next patchset will not be
sent to LKML.
Tks.

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Add support for usbfs zerocopy.

On Tue, 5 Jan 2016, David Laight wrote:

> From: Steinar H. Gunderson [mailto:se...@google.com]
> > Sent: 26 November 2015 00:19
> 
> There is still a problem with this code, not sure how to fix it.
> 
> ...
> > +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
> > +{
> > +   struct usb_dev_state *ps = usbm->ps;
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(&ps->lock, flags);
> > +   --*count;
> > +   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
> ...
> > +   module_put(THIS_MODULE);
> > +   } else {
> > +   spin_unlock_irqrestore(&ps->lock, flags);
> > +   }
> > +}
> ...
> > +static void usbdev_vm_close(struct vm_area_struct *vma)
> > +{
> > +   struct usb_memory *usbm = vma->vm_private_data;
> > +
> > +   dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
> > +}
> 
> If the last reference to the module is for an mmap request
> (which is the only reason to reference count the module here)
> then the module_put() in dec_usb_memory_use_count()) returns
> back into freed memory.

Ooh, you're right.

> There isn't much the module can do about it either apart
> from using kthread_run() to call module_put_and_exit()
> and even that is somewhat racy (a sleep in the kthread
> is probably enough).
> 
> The only real solution is for mmap() itself to take the
> reference on the module.

I agree; the vm_operations_struct structure ought to have a .owner 
member.  But I don't feel like going through and changing all of them.

How do other drivers handle this problem?  Perhaps they don't face it
because they don't use DMA mapping to implement mmap.  A normal MMIO
sort of page mapping doesn't cause difficulties if you leave it in
place after the device is unregistered.

Or maybe I just don't understand the problem very well and the core 
kernel handles all of this for us.  I'll try asking.

kthread_run plus sleep and module_put_and_exit might turn out to be the
way to go. If anyone has a better suggestion, I'd like to hear it.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

On Tue, 5 Jan 2016, Rob Herring wrote:

> >>> > It's not clear (to me, anyway) how this is meant to work.  USB is a
> >>> > completely discoverable bus: There is no way to represent devices
> >>> > statically; they have to be discovered.  But a device can't be
> >>> > discovered unless it is functional, so if an onboard hub (or any other
> >>> > sort of USB device) requires power, clocks, or something similar in
> >>> > order to function, it won't be discovered.  There won't be any device
> >>> > structure to probe, and "forcing driver probe" won't accomplish
> >>> > anything.
> >>> >
> >>> > It seems to me that the only way something like this could be made to
> >>> > work is if the necessary actions are keyed off the host controller (and
> >>> > in particular, _not_ the hub driver), presumably at the time the
> >>> > controller is probed.
> 
> The problem with putting this in the the host controller driver is it
> assumes the initialization sequence is generic enough to be described
> in DT and that initialization is the only time we need DT data. The
> MFD example is a perfect example of another case. Suspend/resume also
> comes to mind. Even if we could put the control in both places, that
> is poor design IMO. I'd rather just make it a requirement that the
> bootloader do enough setup that devices can be discovered.

That would be okay with me.  It would make things a lot simpler (but it 
would also waste energy in situations where the devices weren't being 
used).

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Tue, 5 Jan 2016, Joe Perches wrote:

> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> > 
> > > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd 
> > > >> *ehci, __hc32 token)
> > > >>�� return '/';
> > > >>� }
> > > >>
> > > >> -static void qh_lines(
> > > >> - struct ehci_hcd *ehci,
> > > >> - struct ehci_qh *qh,
> > > >> - char **nextp,
> > > >> - unsigned *sizep
> > > >> -)
> > > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > > >> + char **nextp, unsigned *sizep)
> > > >>� {
> > > >>�� u32 scratch;
> > > >>�� u32 hw_curr;
> > > >>
> > > >
> > > And about that style? Should be done?
> > 
> > You mean squeezing the function parameters into two lines?� That's�
> > okay.
> > 
> > However, the style in this file is to indent continuation lines by two
> > extra tab stops, not to line things up with an open paren on the first
> > line.
> 
> It's not consistent.
> It's a bit of a mix of 1 and 2 tabs, and some others.

I know.  That's because the files were written by various people at 
various times and nobody tried to enforce a rigid consistent style.

I'm not even consistent all the time in the things that I write.  There 
are places (see drivers/usb/core/config.c) where I indented 
continuation lines by 4 spaces instead of 2 tab stops.  And there are 
places where a continuation of a continuation gets indented even 
farther.

Trying to come up with hard-and-fast rules for this sort of thing is 
pretty hopeless.  Even "Maximize readability" doesn't work too well, 
because different people find different things most readable.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
> Trying to come up with hard-and-fast rules for this sort of thing is 
> pretty hopeless.  Even "Maximize readability" doesn't work too well, 
> because different people find different things most readable.

Readability is mostly habituation.

Align to open parenthesis is relatively easy to implement,
but it can get out of hand too with long naming.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Does vm_operations_struct require a .owner field?

Hello:

Question: The vm_operations_struct structure contains lots of callback
pointers.  Is there any mechanism to prevent the callback routines and
the structure itself being unloaded from memory (if they are built into
modules) while the relevant VMAs are still in use?

Consider a simple example: A user program calls mmap(2) on a device
file.  Later on, the file is closed and the device driver's module is
unloaded.  But until munmap(2) is called or the user program exits, the
memory mapping and the corresponding VMA will remain in existence.  
(The man page for munmap specifically says "closing the file descriptor
does not unmap the region".)  Thus when the user program does do an
munmap(), the callback to vma->vm_ops->close will reference nonexistent
memory and cause an oops.

Normally this sort of thing is prevented by try_module_get(...->owner).  
But vm_operations_struct doesn't include a .owner field.

Am I missing something here?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

On Tue, 5 Jan 2016, Joe Perches wrote:

> On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
> > Trying to come up with hard-and-fast rules for this sort of thing is�
> > pretty hopeless.� Even "Maximize readability" doesn't work too well,�
> > because different people find different things most readable.
> 
> Readability is mostly habituation.

Indeed.

> Align to open parenthesis is relatively easy to implement,
> but it can get out of hand too with long naming.

That's why I dislike it.  Plus the fact that it results in
continuations of different lines being indented by different amounts.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add support for usbfs zerocopy.

2016-01-05 Thread Markus Rechberger

On Tue, Jan 5, 2016 at 4:57 PM, Alan Stern  wrote:
> On Tue, 5 Jan 2016, David Laight wrote:
>
>> From: Steinar H. Gunderson [mailto:se...@google.com]
>> > Sent: 26 November 2015 00:19
>>
>> There is still a problem with this code, not sure how to fix it.
>>
>> ...
>> > +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
>> > +{
>> > +   struct usb_dev_state *ps = usbm->ps;
>> > +   unsigned long flags;
>> > +
>> > +   spin_lock_irqsave(&ps->lock, flags);
>> > +   --*count;
>> > +   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
>> ...
>> > +   module_put(THIS_MODULE);
>> > +   } else {
>> > +   spin_unlock_irqrestore(&ps->lock, flags);
>> > +   }
>> > +}
>> ...
>> > +static void usbdev_vm_close(struct vm_area_struct *vma)
>> > +{
>> > +   struct usb_memory *usbm = vma->vm_private_data;
>> > +
>> > +   dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
>> > +}
>>
>> If the last reference to the module is for an mmap request
>> (which is the only reason to reference count the module here)
>> then the module_put() in dec_usb_memory_use_count()) returns
>> back into freed memory.
>
> Ooh, you're right.
>
>> There isn't much the module can do about it either apart
>> from using kthread_run() to call module_put_and_exit()
>> and even that is somewhat racy (a sleep in the kthread
>> is probably enough).
>>
>> The only real solution is for mmap() itself to take the
>> reference on the module.
>
> I agree; the vm_operations_struct structure ought to have a .owner
> member.  But I don't feel like going through and changing all of them.
>
> How do other drivers handle this problem?  Perhaps they don't face it
> because they don't use DMA mapping to implement mmap.  A normal MMIO
> sort of page mapping doesn't cause difficulties if you leave it in
> place after the device is unregistered.
>
> Or maybe I just don't understand the problem very well and the core
> kernel handles all of this for us.  I'll try asking.
>
> kthread_run plus sleep and module_put_and_exit might turn out to be the
> way to go. If anyone has a better suggestion, I'd like to hear it.
>

you might check the video4linux implementation, they do DMA mapping of
videoframes but not sure if they run into the
same problem somehow.

Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

On Mon, 4 Jan 2016, Joe Perches wrote:

> On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
> > 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
> > > > > > > This patch fixes coding style issues reported by checkpatch 
> > > > > > > concerning
> > > > > > > to unnecessary space after a cast.
> > > > > > This is a case where checkpatch is wrong, IMO.��Casts should always 
> > > > > > be
> > > > > > followed by a space.��I will not accept this patch.
> 
> Your choice, but most kernel code disagrees with you.
> 
> measuring only kernel casts to a pointer, (because there are
> too many false positives otherwise) casts without a space
> are preferred ~3:1 over casts followed by a space.

Then most kernel code is implicitly in violation of CodingStyle.
I'm referring to the section that says (admittedly, in a different 
context):

... all right-thinking people know that
(a) K&R are _right_ and (b) K&R are right.

K&R, both the first and second editions, is very consistent about 
always putting a space after a cast.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

On Mon, 4 Jan 2016, Joe Perches wrote:

> measuring only kernel casts to a pointer, (because there are
> too many false positives otherwise) casts without a space
> are preferred ~3:1 over casts followed by a space.
> 
> (without space)
> $�grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)\w+" * 
> | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 36612
> 
> (with space)
> $�grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)[ 
> \t]\w+" * | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 13233

It would be awfully nice if there was a reasonable way to express this
sort of computation that didn't require the reader to spend half an
hour of intense concentration and man-page reading in order to
decipher.

Just wishful thinking...

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] usb-misc: cleanup sisusbvga

The file

drivers/usb/misc/sisusbvga/sisusb.c

had many (192) coding style issues reported by checkpatch. This file also had a
problematic error path in the probe function that could result in dereferencing
a null pointer.

This patch series fix coding style issues and fix the problematic error path to
avoid the null pointer dereference.

Patch 1 is the biggest and fix only whitespace, tab and newline issues. I used

$ git diff -w --word-diff=porcelain drivers/usb/misc/sisusbvga/sisusb.c

to verify that this patch do not change any visible character. If the size of
this patch is a problem, please let me know in how many patches to split it.

Patch 2 follows fixing trivial coding style, mostly related to braces,
and parenthesis.

Patch 3 remove assignments from if tests.

Patch 4 remove calls to dev_err() on memory allocation failures and fix one
error path to avoid dereferencing a null pointer. The patch change the
problematic error path to cleanup previously allocated resources and abort the
probe with -ENOMEM instead of only calling dev_err() and continue with the
probe.

Patch 5 remove null tests before calls to kfree().

Peter Senna Tschudin (5):
  usb-misc: sisusbvga: Fix coding style: white space
  usb-misc: sisusbvga: Fix coding style: braces, parenthesis, comment
  usb-misc: sisusbvga: Fix coding style: remove assignment from if tests
  usb-misc: sisusbvga: Remove memory allocation logs and fix error path
  usb-misc: sisusbvga: Remove null test before calls to kfree()

 drivers/usb/misc/sisusbvga/sisusb.c | 1543 +--
 1 file changed, 752 insertions(+), 791 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] usb-misc: sisusbvga: Fix trivial coding style

From: Peter Senna Tschudin 

The file drivers/usb/misc/sisusbvga/sisusb.c contained coding style
issues reported by checkpatch. This patch fixes the following
errors:
 - 12 WARNING: braces {} are not necessary for single statement blocks
 - 4 ERROR: return is not a function, parentheses are not required
 - 3 ERROR: do not initialise statics to 0
 - 1 WARNING: else is not generally useful after a break or return
 - 1 WARNING: braces {} are not necessary for any arm of this statement
 - 1 WARNING: Block comments use * on subsequent lines

Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only.

 drivers/usb/misc/sisusbvga/sisusb.c | 88 +
 1 file changed, 41 insertions(+), 47 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index 8b754be..e6bb1ad 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -61,8 +61,8 @@
 /* Forward declarations / clean-up routines */
 
 #ifdef INCL_SISUSB_CON
-static int sisusb_first_vc = 0;
-static int sisusb_last_vc = 0;
+static int sisusb_first_vc;
+static int sisusb_last_vc;
 module_param_named(first, sisusb_first_vc, int, 0);
 module_param_named(last, sisusb_last_vc, int, 0);
 MODULE_PARM_DESC(first, "Number of first console to take over (1 - 
MAX_NR_CONSOLES)");
@@ -762,7 +762,7 @@ static int sisusb_write_mem_bulk(struct sisusb_usb_data 
*sisusb, u32 addr,
 {
struct sisusb_packet packet;
int  ret = 0;
-   static int msgcount = 0;
+   static int msgcount;
u8   swap8, fromkern = kernbuffer ? 1 : 0;
u16  swap16;
u32  swap32, flag = (length >> 28) & 1;
@@ -1129,12 +1129,10 @@ static int sisusb_read_mem_bulk(struct sisusb_usb_data 
*sisusb, u32 addr,
if (!ret) {
(*bytes_read)++;
if (userbuffer) {
-   if (put_user(buf[0], (u8 __user 
*)userbuffer)) {
+   if (put_user(buf[0], (u8 __user 
*)userbuffer))
return -EFAULT;
-   }
-   } else {
+   } else
kernbuffer[0] = buf[0];
-   }
}
return ret;
 
@@ -1268,13 +1266,13 @@ static int sisusb_setidxregmask(struct sisusb_usb_data 
*sisusb,
 int sisusb_setidxregor(struct sisusb_usb_data *sisusb, int port,
u8 index, u8 myor)
 {
-   return(sisusb_setidxregandor(sisusb, port, index, 0xff, myor));
+   return sisusb_setidxregandor(sisusb, port, index, 0xff, myor);
 }
 
 int sisusb_setidxregand(struct sisusb_usb_data *sisusb, int port,
u8 idx, u8 myand)
 {
-   return(sisusb_setidxregandor(sisusb, port, idx, myand, 0x00));
+   return sisusb_setidxregandor(sisusb, port, idx, myand, 0x00);
 }
 
 /* Write/read video ram */
@@ -1282,27 +1280,27 @@ int sisusb_setidxregand(struct sisusb_usb_data *sisusb, 
int port,
 #ifdef INCL_SISUSB_CON
 int sisusb_writeb(struct sisusb_usb_data *sisusb, u32 adr, u8 data)
 {
-   return(sisusb_write_memio_byte(sisusb, SISUSB_TYPE_MEM, adr, data));
+   return sisusb_write_memio_byte(sisusb, SISUSB_TYPE_MEM, adr, data);
 }
 
 int sisusb_readb(struct sisusb_usb_data *sisusb, u32 adr, u8 *data)
 {
-   return(sisusb_read_memio_byte(sisusb, SISUSB_TYPE_MEM, adr, data));
+   return sisusb_read_memio_byte(sisusb, SISUSB_TYPE_MEM, adr, data);
 }
 
 int sisusb_copy_memory(struct sisusb_usb_data *sisusb, char *src,
u32 dest, int length, size_t *bytes_written)
 {
-   return(sisusb_write_mem_bulk(sisusb, dest, src, length,
-   NULL, 0, bytes_written));
+   return sisusb_write_mem_bulk(sisusb, dest, src, length,
+   NULL, 0, bytes_written);
 }
 
 #ifdef SISUSBENDIANTEST
 int sisusb_read_memory(struct sisusb_usb_data *sisusb, char *dest,
u32 src, int length, size_t *bytes_written)
 {
-   return(sisusb_read_mem_bulk(sisusb, src, dest, length,
-   NULL, bytes_written));
+   return sisusb_read_mem_bulk(sisusb, src, dest, length,
+   NULL, bytes_written);
 }
 #endif
 #endif
@@ -1818,18 +1816,18 @@ static int sisusb_set_default_mode(struct 
sisusb_usb_data *sisusb,
SETIREGAND(SISSR, 0x37, 0xfe);
SETREG(SISMISCW, 0xef); /* sync */
SETIREG(SISCR, 0x11, 0x00); /* crtc */
-   for (j = 0x00, i = 0; i <= 7; i++, j++) {
+   for (j = 0x00, i = 0; i <= 7; i++, j++)
SETIREG(SISCR, j, crtcdata[i]);
-   }
-   for (j = 0x10; i <= 10; i++, j++) {
+
+   for (j = 0x10; i <= 10; i++, j++)
SETIREG(SISCR, j, crtcdata[i]);
-   }
-   for (j = 0x15; i <= 12; i++, j++) {
+
+   for (j

[PATCH 3/5] usb-misc: sisusbvga: Remove assignments from if tests

From: Peter Senna Tschudin 

The file drivers/usb/misc/sisusbvga/sisusb.c had 6 assignments inside if
tests. This patch move the assignement outside the test.

Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only.

 drivers/usb/misc/sisusbvga/sisusb.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index e6bb1ad..6cce4fb 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -1378,7 +1378,8 @@ static int sisusb_clear_vram(struct sisusb_usb_data 
*sisusb,
return 0;
 
/* allocate free buffer/urb and clear the buffer */
-   if ((i = sisusb_alloc_outbuf(sisusb)) < 0)
+   i = sisusb_alloc_outbuf(sisusb);
+   if (i < 0)
return -EBUSY;
 
memset(sisusb->obuf[i], 0, sisusb->obufsize);
@@ -3067,7 +3068,8 @@ static int sisusb_probe(struct usb_interface *intf,
 
/* Allocate buffers */
sisusb->ibufsize = SISUSB_IBUF_SIZE;
-   if (!(sisusb->ibuf = kmalloc(SISUSB_IBUF_SIZE, GFP_KERNEL))) {
+   sisusb->ibuf = kmalloc(SISUSB_IBUF_SIZE, GFP_KERNEL);
+   if (!sisusb->ibuf) {
dev_err(&sisusb->sisusb_dev->dev, "Failed to allocate memory 
for input buffer");
retval = -ENOMEM;
goto error_2;
@@ -3076,7 +3078,8 @@ static int sisusb_probe(struct usb_interface *intf,
sisusb->numobufs = 0;
sisusb->obufsize = SISUSB_OBUF_SIZE;
for (i = 0; i < NUMOBUFS; i++) {
-   if (!(sisusb->obuf[i] = kmalloc(SISUSB_OBUF_SIZE, GFP_KERNEL))) 
{
+   sisusb->obuf[i] = kmalloc(SISUSB_OBUF_SIZE, GFP_KERNEL);
+   if (!sisusb->obuf[i]) {
if (i == 0) {
dev_err(&sisusb->sisusb_dev->dev, "Failed to 
allocate memory for output buffer\n");
retval = -ENOMEM;
@@ -3088,7 +3091,8 @@ static int sisusb_probe(struct usb_interface *intf,
}
 
/* Allocate URBs */
-   if (!(sisusb->sisurbin = usb_alloc_urb(0, GFP_KERNEL))) {
+   sisusb->sisurbin = usb_alloc_urb(0, GFP_KERNEL);
+   if (!sisusb->sisurbin) {
dev_err(&sisusb->sisusb_dev->dev, "Failed to allocate URBs\n");
retval = -ENOMEM;
goto error_3;
@@ -3096,7 +3100,8 @@ static int sisusb_probe(struct usb_interface *intf,
sisusb->completein = 1;
 
for (i = 0; i < sisusb->numobufs; i++) {
-   if (!(sisusb->sisurbout[i] = usb_alloc_urb(0, GFP_KERNEL))) {
+   sisusb->sisurbout[i] = usb_alloc_urb(0, GFP_KERNEL);
+   if (!sisusb->sisurbout[i]) {
dev_err(&sisusb->sisusb_dev->dev,
"Failed to allocate URBs\n");
retval = -ENOMEM;
@@ -3112,7 +3117,8 @@ static int sisusb_probe(struct usb_interface *intf,
 
 #ifdef INCL_SISUSB_CON
/* Allocate our SiS_Pr */
-   if (!(sisusb->SiS_Pr = kmalloc(sizeof(struct SiS_Private), 
GFP_KERNEL))) {
+   sisusb->SiS_Pr = kmalloc(sizeof(struct SiS_Private), GFP_KERNEL);
+   if (!sisusb->SiS_Pr) {
dev_err(&sisusb->sisusb_dev->dev, "Failed to allocate 
SiS_Pr\n");
}
 #endif
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] usb-misc: sisusbvga: Remove kmalloc logs and fix error path

From: Peter Senna Tschudin 

This patch remove four calls to dev_err() from sisusb_probe() as
reporting memory allocation failures is redundant:

 - Remove a call to dev_err() that was reporting unsuccesful call to
   kzalloc().

 - Remove two calls to dev_err() that were reporting unsuccesful calls
   to kmalloc().

 - Remove a call to dev_err() that was reporting unsuccesful call to
   kmalloc(), and replace it by code that clean up previously allocated
   resources and abort the probe with -ENOMEM. Before this modification
   sisusb->SiS_Pr could be dereferenced even it was null.

Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only.

 drivers/usb/misc/sisusbvga/sisusb.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index 6cce4fb..875e365 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -3040,10 +3040,9 @@ static int sisusb_probe(struct usb_interface *intf,
 
/* Allocate memory for our private */
sisusb = kzalloc(sizeof(*sisusb), GFP_KERNEL);
-   if (!sisusb) {
-   dev_err(&dev->dev, "Failed to allocate memory for private 
data\n");
+   if (!sisusb)
return -ENOMEM;
-   }
+
kref_init(&sisusb->kref);
 
mutex_init(&(sisusb->lock));
@@ -3070,7 +3069,6 @@ static int sisusb_probe(struct usb_interface *intf,
sisusb->ibufsize = SISUSB_IBUF_SIZE;
sisusb->ibuf = kmalloc(SISUSB_IBUF_SIZE, GFP_KERNEL);
if (!sisusb->ibuf) {
-   dev_err(&sisusb->sisusb_dev->dev, "Failed to allocate memory 
for input buffer");
retval = -ENOMEM;
goto error_2;
}
@@ -3081,7 +3079,6 @@ static int sisusb_probe(struct usb_interface *intf,
sisusb->obuf[i] = kmalloc(SISUSB_OBUF_SIZE, GFP_KERNEL);
if (!sisusb->obuf[i]) {
if (i == 0) {
-   dev_err(&sisusb->sisusb_dev->dev, "Failed to 
allocate memory for output buffer\n");
retval = -ENOMEM;
goto error_3;
}
@@ -3119,7 +3116,8 @@ static int sisusb_probe(struct usb_interface *intf,
/* Allocate our SiS_Pr */
sisusb->SiS_Pr = kmalloc(sizeof(struct SiS_Private), GFP_KERNEL);
if (!sisusb->SiS_Pr) {
-   dev_err(&sisusb->sisusb_dev->dev, "Failed to allocate 
SiS_Pr\n");
+   retval = -ENOMEM;
+   goto error_4;
}
 #endif
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] usb-misc: sisusbvga: Remove null test before kfree()

From: Peter Senna Tschudin 

This patch removes null test before calls to kfree() as kfree() can
handle null pointers safely.

Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only.

 drivers/usb/misc/sisusbvga/sisusb.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index 875e365..8d0b29a 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -76,15 +76,11 @@ static void sisusb_free_buffers(struct sisusb_usb_data 
*sisusb)
int i;
 
for (i = 0; i < NUMOBUFS; i++) {
-   if (sisusb->obuf[i]) {
-   kfree(sisusb->obuf[i]);
-   sisusb->obuf[i] = NULL;
-   }
-   }
-   if (sisusb->ibuf) {
-   kfree(sisusb->ibuf);
-   sisusb->ibuf = NULL;
+   kfree(sisusb->obuf[i]);
+   sisusb->obuf[i] = NULL;
}
+   kfree(sisusb->ibuf);
+   sisusb->ibuf = NULL;
 }
 
 static void sisusb_free_urbs(struct sisusb_usb_data *sisusb)
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] usb-misc: sisusbvga: Fix coding style: white space

From: Peter Senna Tschudin 

This patch change only spaces, tabs, and newlines.

The file drivers/usb/misc/sisusbvga/sisusb.c contained entire statements
using 4 spaces for the last level of indetantion, and many other space
and tab related issues. This patch fixes the following issues:
 - 83 ERROR: space required after that ','
 - 20 ERROR: trailing statements should be on next line
 - 13 WARNING: line over 80 characters
 - 13 ERROR: switch and case should be at the same indent
 - 8 WARNING: please, no spaces at the start of a line
 - 4 WARNING: suspect code indent for conditional statements
 - 3 WARNING: Missing a blank line after declarations
 - 3 ERROR: space required before the open parenthesis '('
 - 1 ERROR: space prohibited before that ',' (ctx:WxW)
 - 1 ERROR: code indent should use tabs where possible

This patch also fixes function declaration style and continuation lines.

 3 lines over 80 chars are left as the fix would split the cast and
varible name in different lines, which makes the code confusing to read.

Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only.

The command:

$ git diff -w --word-diff=porcelain drivers/usb/misc/sisusbvga/sisusb.c

helps reviewing this patch as it simplifies the visualization of
changes, making it easier to verify that only white spaces, tabs and
newlines are added or removed by this patch.
 drivers/usb/misc/sisusbvga/sisusb.c | 1425 +--
 1 file changed, 696 insertions(+), 729 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c 
b/drivers/usb/misc/sisusbvga/sisusb.c
index 8efbaba..8b754be 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -71,8 +71,7 @@ MODULE_PARM_DESC(last, "Number of last console to take over 
(1 - MAX_NR_CONSOLES
 
 static struct usb_driver sisusb_driver;
 
-static void
-sisusb_free_buffers(struct sisusb_usb_data *sisusb)
+static void sisusb_free_buffers(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -88,8 +87,7 @@ sisusb_free_buffers(struct sisusb_usb_data *sisusb)
}
 }
 
-static void
-sisusb_free_urbs(struct sisusb_usb_data *sisusb)
+static void sisusb_free_urbs(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -108,8 +106,7 @@ sisusb_free_urbs(struct sisusb_usb_data *sisusb)
 /* out-urb management */
 
 /* Return 1 if all free, 0 otherwise */
-static int
-sisusb_all_free(struct sisusb_usb_data *sisusb)
+static int sisusb_all_free(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -124,8 +121,7 @@ sisusb_all_free(struct sisusb_usb_data *sisusb)
 }
 
 /* Kill all busy URBs */
-static void
-sisusb_kill_all_busy(struct sisusb_usb_data *sisusb)
+static void sisusb_kill_all_busy(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -141,20 +137,17 @@ sisusb_kill_all_busy(struct sisusb_usb_data *sisusb)
 }
 
 /* Return 1 if ok, 0 if error (not all complete within timeout) */
-static int
-sisusb_wait_all_out_complete(struct sisusb_usb_data *sisusb)
+static int sisusb_wait_all_out_complete(struct sisusb_usb_data *sisusb)
 {
int timeout = 5 * HZ, i = 1;
 
-   wait_event_timeout(sisusb->wait_q,
-   (i = sisusb_all_free(sisusb)),
-timeout);
+   wait_event_timeout(sisusb->wait_q, (i = sisusb_all_free(sisusb)),
+   timeout);
 
return i;
 }
 
-static int
-sisusb_outurb_available(struct sisusb_usb_data *sisusb)
+static int sisusb_outurb_available(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -168,20 +161,17 @@ sisusb_outurb_available(struct sisusb_usb_data *sisusb)
return -1;
 }
 
-static int
-sisusb_get_free_outbuf(struct sisusb_usb_data *sisusb)
+static int sisusb_get_free_outbuf(struct sisusb_usb_data *sisusb)
 {
int i, timeout = 5 * HZ;
 
wait_event_timeout(sisusb->wait_q,
-   ((i = sisusb_outurb_available(sisusb)) >= 0),
-   timeout);
+   ((i = sisusb_outurb_available(sisusb)) >= 0), timeout);
 
return i;
 }
 
-static int
-sisusb_alloc_outbuf(struct sisusb_usb_data *sisusb)
+static int sisusb_alloc_outbuf(struct sisusb_usb_data *sisusb)
 {
int i;
 
@@ -193,8 +183,7 @@ sisusb_alloc_outbuf(struct sisusb_usb_data *sisusb)
return i;
 }
 
-static void
-sisusb_free_outbuf(struct sisusb_usb_data *sisusb, int index)
+static void sisusb_free_outbuf(struct sisusb_usb_data *sisusb, int index)
 {
if ((index >= 0) && (index < sisusb->numobufs))
sisusb->urbstatus[index] &= ~SU_URB_ALLOC;
@@ -202,8 +191,7 @@ sisusb_free_outbuf(struct sisusb_usb_data *sisusb, int 
index)
 
 /* completion callback */
 
-static void
-sisusb_bulk_completeout(struct urb *urb)
+static void sisusb_bulk_completeout(struct urb *urb)
 {
struct sisusb_urb_context *context = urb->context;
struct sisusb_usb_data *sisusb;
@@ -225,9 +213,9 @@ sisusb_bulk_completeout(struct urb *urb)
wake

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

2016-01-05 13:29 GMT-03:00 Alan Stern :
> On Tue, 5 Jan 2016, Joe Perches wrote:
>
>> On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
>> > Trying to come up with hard-and-fast rules for this sort of thing is
>> > pretty hopeless.  Even "Maximize readability" doesn't work too well,
>> > because different people find different things most readable.
>>
>> Readability is mostly habituation.
>
> Indeed.

Ditto.

>
>> Align to open parenthesis is relatively easy to implement,
>> but it can get out of hand too with long naming.

It's easy and it's consistent (the place of sequential lines).

>
> That's why I dislike it.  Plus the fact that it results in
> continuations of different lines being indented by different amounts.

The different amounts is the price to pay for that indentation, but it
will mostly make the style consistent and easy as Joe stated. I'm not
claiming for that changing, just debating. :-D
But IMHO standardization could make things easier.

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Add support for usbfs zerocopy.

2016-01-05 Thread David Laight

From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: 05 January 2016 15:58
> On Tue, 5 Jan 2016, David Laight wrote:
> 
> > From: Steinar H. Gunderson [mailto:se...@google.com]
> > > Sent: 26 November 2015 00:19
> >
> > There is still a problem with this code, not sure how to fix it.
> >
> > ...
> > > +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
> > > +{
> > > + struct usb_dev_state *ps = usbm->ps;
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&ps->lock, flags);
> > > + --*count;
> > > + if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
> > ...
> > > + module_put(THIS_MODULE);
> > > + } else {
> > > + spin_unlock_irqrestore(&ps->lock, flags);
> > > + }
> > > +}
> > ...
> > > +static void usbdev_vm_close(struct vm_area_struct *vma)
> > > +{
> > > + struct usb_memory *usbm = vma->vm_private_data;
> > > +
> > > + dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
> > > +}
> >
> > If the last reference to the module is for an mmap request
> > (which is the only reason to reference count the module here)
> > then the module_put() in dec_usb_memory_use_count()) returns
> > back into freed memory.
> 
> Ooh, you're right.

In fact every call of module_put(THIS_MODULE) is probably either buggy
or unnecessary.

> > There isn't much the module can do about it either apart
> > from using kthread_run() to call module_put_and_exit()
> > and even that is somewhat racy (a sleep in the kthread
> > is probably enough).
> >
> > The only real solution is for mmap() itself to take the
> > reference on the module.
> 
> I agree; the vm_operations_struct structure ought to have a .owner
> member.  But I don't feel like going through and changing all of them.

The .owner from the device can be copied into the 'vm_area_struct',
vm_operations_struct doesn't need changing.

> How do other drivers handle this problem?  Perhaps they don't face it
> because they don't use DMA mapping to implement mmap.  A normal MMIO
> sort of page mapping doesn't cause difficulties if you leave it in
> place after the device is unregistered.

It can cause issues.
If you remove a PCIe device (echo 1 >/sys/devices/pci*/*/remove) and don't
invalidate any corresponding mmap vma then the same bus addresses could
be assigned to a different card if a rescan is done.

I think that zap_page_range() could be called with parameters taken
from the vma in order to cause the .fault function be called.
However zap_page_range() isn't exported in mainline kernels.

I'm trying to get the same effect by calling remap_pfn_range() to
replace the dma/io page with a vmalloc()ed one - but so far have
only have multiple oops and system lockups.
It might be because I'm trying to remap pages for the wrong process
(or I might just have the args to remap_pfn_range() wrong).

There is a further problem that it is basically impossible to remove
timing windows between an application unmap() and a card removal event
because the required locks can't be obtained in the same order in
both cases.

> Or maybe I just don't understand the problem very well and the core
> kernel handles all of this for us.  I'll try asking.
> 
> kthread_run plus sleep and module_put_and_exit might turn out to be the
> way to go. If anyone has a better suggestion, I'd like to hear it.

If you know the device is open you can directly call module_put()
(with a mutex/sema held to block the close) since you know it
can't be the last reference.

David

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/16] usb: host: ehci-dbg: cleanup and refactoring

This patchset removes all errors reported by checkpatch in addition to
some refactoring.

v2:
 - changes:
   * usb: host: ehci-dbg: fix up function definitions
 - adds:
   * usb: host: ehci-dbg: use scnprintf() in qh_lines()
 - removes:
   * usb: host: ehci-dbg: fix unsigned comparison
   * usb: host: ehci-dbg: remove unnecessary space after cast

Geyslan G. Bem (16):
  usb: host: ehci-dbg: remove space before open parenthesis
  usb: host: ehci-dbg: remove space before open square bracket
  usb: host: ehci-dbg: use C89-style comments
  usb: host: ehci-dbg: move trailing statements to next line
  usb: host: ehci-dbg: fix up closing parenthesis
  usb: host: ehci-dbg: put spaces around operators
  usb: host: ehci-dbg: use scnprintf() in qh_lines()
  usb: host: ehci-dbg: fix up function definitions
  usb: host: ehci-dbg: use a blank line after struct declarations
  usb: host: ehci-dbg: convert macro to inline function
  usb: host: ehci-dbg: add blank line after declarations
  usb: host: ehci-dbg: remove blank line before close brace
  usb: host: ehci-dbg: replace sizeof operand
  usb: host: ehci-dbg: enclose conditional blocks with braces
  usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size
  usb: host: ehci-dbg: add function output_buf_tds_dir()

 drivers/usb/host/ehci-dbg.c | 473 +++-
 1 file changed, 252 insertions(+), 221 deletions(-)

-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/16] usb: host: ehci-dbg: remove space before open parenthesis

This patch fixes coding style issues reported by checkpatch. The vast
majority of changes in this patch are removing spaces before opening
parenthesis, but in some cases, a few additional changes are made to fix
other coding style issues.

These additional changes are:

 - Spaces around >> on line 50.
 - On line 55 a call to ehci_dbg reduced to a single line.
 - sizeof operands surrounded with parenthesis on lines 877, 883, 889
   and 901.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 105 warnings about spaces before opening
parenthesis. After there are still 6. These lines concern to macros that
will be modified to inline functions in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 199 ++--
 1 file changed, 99 insertions(+), 100 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b7d623f..fcbbdfa 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -24,41 +24,40 @@
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
-static void dbg_hcs_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcs_params);
 
-   ehci_dbg (ehci,
+   ehci_dbg(ehci,
"%s hcs_params 0x%x dbg=%d%s cc=%d pcc=%d%s%s ports=%d\n",
label, params,
-   HCS_DEBUG_PORT (params),
-   HCS_INDICATOR (params) ? " ind" : "",
-   HCS_N_CC (params),
-   HCS_N_PCC (params),
-   HCS_PORTROUTED (params) ? "" : " ordered",
-   HCS_PPC (params) ? "" : " !ppc",
-   HCS_N_PORTS (params)
+   HCS_DEBUG_PORT(params),
+   HCS_INDICATOR(params) ? " ind" : "",
+   HCS_N_CC(params),
+   HCS_N_PCC(params),
+   HCS_PORTROUTED(params) ? "" : " ordered",
+   HCS_PPC(params) ? "" : " !ppc",
+   HCS_N_PORTS(params)
);
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
-   if (HCS_PORTROUTED (params)) {
+   if (HCS_PORTROUTED(params)) {
int i;
char buf [46], tmp [7], byte;
 
buf[0] = 0;
-   for (i = 0; i < HCS_N_PORTS (params); i++) {
+   for (i = 0; i < HCS_N_PORTS(params); i++) {
// FIXME MIPS won't readb() ...
-   byte = readb (&ehci->caps->portroute[(i>>1)]);
+   byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
strcat(buf, tmp);
}
-   ehci_dbg (ehci, "%s portroute %s\n",
-   label, buf);
+   ehci_dbg(ehci, "%s portroute %s\n", label, buf);
}
 }
 #else
 
-static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
@@ -68,19 +67,19 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci, 
char *label) {}
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
  * */
-static void dbg_hcc_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
 
-   if (HCC_ISOC_CACHE (params)) {
-   ehci_dbg (ehci,
+   if (HCC_ISOC_CACHE(params)) {
+   ehci_dbg(ehci,
"%s hcc_params %04x caching frame %s%s%s\n",
label, params,
HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024",
HCC_CANPARK(params) ? " park" : "",
HCC_64BIT_ADDR(params) ? " 64 bit addr" : "");
} else {
-   ehci_dbg (ehci,
+   ehci_dbg(ehci,
"%s hcc_params %04x thresh %d uframes %s%s%s%s%s%s%s\n",
label,
params,
@@ -97,14 +96,14 @@ static void dbg_hcc_params (struct ehci_hcd *ehci, char 
*label)
 }
 #else
 
-static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
 static void __maybe_unused
-dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
+dbg_qtd(const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
 {
ehci_dbg(ehci, "%s td %p n%08x %08x t%08x p0=%08x\n", label, qtd,
hc32_to_cpup(ehci, &qtd->hw_next),
@@ -120,22 +119,22 @@ dbg_qtd (const char *label, struct ehci_hcd *ehci, struct 
ehci_qtd *qtd)

[PATCH v2 05/16] usb: host: ehci-dbg: fix up closing parenthesis

This patch puts the closing parenthesis at the statement end removing
unnecessary "new line".

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c409e4f..3b423e1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -35,8 +35,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
HCS_N_PCC(params),
HCS_PORTROUTED(params) ? "" : " ordered",
HCS_PPC(params) ? "" : " !ppc",
-   HCS_N_PORTS(params)
-   );
+   HCS_N_PORTS(params));
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
@@ -189,8 +188,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, 
u32 status)
(status & STS_FLR) ? " FLR" : "",
(status & STS_PCD) ? " PCD" : "",
(status & STS_ERR) ? " ERR" : "",
-   (status & STS_INT) ? " INT" : ""
-   );
+   (status & STS_INT) ? " INT" : "");
 }
 
 static int __maybe_unused
@@ -205,8 +203,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
(enable & STS_FLR) ? " FLR" : "",
(enable & STS_PCD) ? " PCD" : "",
(enable & STS_ERR) ? " ERR" : "",
-   (enable & STS_INT) ? " INT" : ""
-   );
+   (enable & STS_INT) ? " INT" : "");
 }
 
 static const char *const fls_strings[] = { "1024", "512", "256", "??" };
@@ -232,8 +229,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
(command & CMD_PSE) ? " Periodic" : "",
fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
-   (command & CMD_RUN) ? "RUN" : "HALT"
-   );
+   (command & CMD_RUN) ? "RUN" : "HALT");
 }
 
 static int
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/16] usb: host: ehci-dbg: put spaces around operators

This patch fixes coding style issues reported by checkpatch concerning
to missing spaces around operators.

There is an additional change on line 49 that removes unnecessary
parentheses around ternary operands.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 3b423e1..980ca55 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -46,7 +46,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
-   ((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
+   (i & 0x1) ? byte & 0xf : (byte >> 4) & 0xf);
strcat(buf, tmp);
}
ehci_dbg(ehci, "%s portroute %s\n", label, buf);
@@ -257,14 +257,14 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
label, label[0] ? " " : "", port, status,
-   status>>25,/*device address */
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
+   status >> 25, /*device address */
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_NYET ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_NYET ?
" NYET" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_STALL ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_STALL ?
" STALL" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ERR ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ERR ?
" ERR" : "",
(status & PORT_POWER) ? " POWER" : "",
(status & PORT_OWNER) ? " OWNER" : "",
@@ -846,7 +846,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer 
*buf)
if (dev_is_pci(hcd->self.controller)) {
struct pci_dev  *pdev;
u32 offset, cap, cap2;
-   unsignedcount = 256/4;
+   unsignedcount = 256 / 4;
 
pdev = to_pci_dev(ehci_to_hcd(ehci)->self.controller);
offset = HCC_EXT_CAPS(ehci_readl(ehci,
@@ -1058,7 +1058,7 @@ static int debug_periodic_open(struct inode *inode, 
struct file *file)
if (!buf)
return -ENOMEM;
 
-   buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8)*PAGE_SIZE;
+   buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8) * PAGE_SIZE;
file->private_data = buf;
return 0;
 }
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 04/16] usb: host: ehci-dbg: move trailing statements to next line

This patch fixes coding style issues reported by checkpatch concerning
to switch case statements. There are few additional changes made to fix
other coding styles issues.

These additional changes are:

 - The compound statement "({...})" on line 474 is pulled out from
   snprintf parameters.

 - On line 723 the constant "0x03" is moved to right.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 14 warnings about trailing statements that
should be on next line. After there are still 4. These lines concern to
a macro that will be modified to inline function in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 +++--
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index df9f598..c409e4f 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -243,10 +243,18 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
 
/* signaling state */
switch (status & (3 << 10)) {
-   case 0 << 10: sig = "se0"; break;
-   case 1 << 10: sig = "k"; break; /* low speed */
-   case 2 << 10: sig = "j"; break;
-   default: sig = "?"; break;
+   case 0 << 10:
+   sig = "se0";
+   break;
+   case 1 << 10: /* low speed */
+   sig = "k";
+   break;
+   case 2 << 10:
+   sig = "j";
+   break;
+   default:
+   sig = "?";
+   break;
}
 
return scnprintf(buf, len,
@@ -451,6 +459,8 @@ static void qh_lines(
 
/* hc may be modifying the list as we read it ... */
list_for_each(entry, &qh->qtd_list) {
+   char *type;
+
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
@@ -464,16 +474,24 @@ static void qh_lines(
else if (td->hw_alt_next != list_end)
mark = '/';
}
+   switch ((scratch >> 8) & 0x03) {
+   case 0:
+   type = "out";
+   break;
+   case 1:
+   type = "in";
+   break;
+   case 2:
+   type = "setup";
+   break;
+   default:
+   type = "?";
+   break;
+   }
temp = snprintf(next, size,
"\n\t%p%c%s len=%d %08x urb %p"
" [td %08x buf[0] %08x]",
-   td, mark, ({ char *tmp;
-switch ((scratch>>8)&0x03) {
-case 0: tmp = "out"; break;
-case 1: tmp = "in"; break;
-case 2: tmp = "setup"; break;
-default: tmp = "?"; break;
-} tmp;}),
+   td, mark, type,
(scratch >> 16) & 0x7fff,
scratch,
td->urb,
@@ -702,11 +720,15 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
&p.qh->qtd_list,
qtd_list) {
temp++;
-   switch (0x03 & (hc32_to_cpu(
-   ehci,
-   qtd->hw_token) >> 8)) {
-   case 0: type = "out"; continue;
-   case 1: type = "in"; continue;
+   switch ((hc32_to_cpu(ehci,
+   qtd->hw_token) >> 8)
+   & 0x03) {
+   case 0:
+   type = "out";
+   continue;
+   case 1:
+   type = "in";
+   continue;
}
}
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/16] usb: host: ehci-dbg: use C89-style comments

This patch fixes coding style issues reported by checkpatch.

Coding style demands usage of C89-style comments and a specific format
when it's multiline.

This also removes the Free Software Foundation address because FSF can
change it again.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 52bf3fe..df9f598 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -11,16 +11,14 @@
  * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
  * for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 /* this file is part of ehci-hcd.c */
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCSPARAMS register
+/*
+ * check the values in the HCSPARAMS register
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
@@ -46,7 +44,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
-   // FIXME MIPS won't readb() ...
+   /* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
@@ -63,10 +61,11 @@ static inline void dbg_hcs_params(struct ehci_hcd *ehci, 
char *label) {}
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCCPARAMS register
+/*
+ * check the values in the HCCPARAMS register
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
- * */
+ */
 static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
@@ -515,7 +514,8 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 
*next = 0;
 
-   /* dumps a snapshot of the async schedule.
+   /*
+* dumps a snapshot of the async schedule.
 * usually empty except for long-term bulk reads, or head.
 * one QH per line, and TDs we know about
 */
@@ -647,7 +647,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
size -= temp;
next += temp;
 
-   /* dump a snapshot of the periodic schedule.
+   /*
+* dump a snapshot of the periodic schedule.
 * iso changes, interrupt usually doesn't.
 */
spin_lock_irqsave(&ehci->lock, flags);
@@ -861,7 +862,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer 
*buf)
}
 #endif
 
-   // FIXME interpret both types of params
+   /* FIXME interpret both types of params */
i = ehci_readl(ehci, &ehci->caps->hcs_params);
temp = scnprintf(next, size, "structural params 0x%08x\n", i);
size -= temp;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/16] usb: host: ehci-dbg: remove space before open square bracket

This patch fixes coding style issues reported by checkpatch. The only
change in this patch that isn't just removing spaces before opening
square brackets is at line 213 where the initialization of fls_strings[]
is placed in same line.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 20 warnings about spaces before open square
bracket. After there are still 3. These lines concern to macros that
will be modified to inline functions in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 33 -
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index fcbbdfa..52bf3fe 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -42,7 +42,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
-   char buf [46], tmp [7], byte;
+   char buf[46], tmp[7], byte;
 
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
@@ -109,8 +109,8 @@ dbg_qtd(const char *label, struct ehci_hcd *ehci, struct 
ehci_qtd *qtd)
hc32_to_cpup(ehci, &qtd->hw_next),
hc32_to_cpup(ehci, &qtd->hw_alt_next),
hc32_to_cpup(ehci, &qtd->hw_token),
-   hc32_to_cpup(ehci, &qtd->hw_buf [0]));
-   if (qtd->hw_buf [1])
+   hc32_to_cpup(ehci, &qtd->hw_buf[0]));
+   if (qtd->hw_buf[1])
ehci_dbg(ehci, "  p1=%08x p2=%08x p3=%08x p4=%08x\n",
hc32_to_cpup(ehci, &qtd->hw_buf[1]),
hc32_to_cpup(ehci, &qtd->hw_buf[2]),
@@ -179,7 +179,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, 
u32 status)
 {
return scnprintf(buf, len,
"%s%sstatus %04x%s%s%s%s%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", status,
+   label, label[0] ? " " : "", status,
(status & STS_PPCE_MASK) ? " PPCE" : "",
(status & STS_ASS) ? " Async" : "",
(status & STS_PSS) ? " Periodic" : "",
@@ -199,7 +199,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
 {
return scnprintf(buf, len,
"%s%sintrenable %02x%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", enable,
+   label, label[0] ? " " : "", enable,
(enable & STS_PPCE_MASK) ? " PPCE" : "",
(enable & STS_IAA) ? " IAA" : "",
(enable & STS_FATAL) ? " FATAL" : "",
@@ -210,8 +210,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
);
 }
 
-static const char *const fls_strings [] =
-{ "1024", "512", "256", "??" };
+static const char *const fls_strings[] = { "1024", "512", "256", "??" };
 
 static int
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
@@ -219,7 +218,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
return scnprintf(buf, len,
"%s%scommand %07x %s%s%s%s%s%s=%d ithresh=%d%s%s%s%s "
"period=%s%s %s",
-   label, label [0] ? " " : "", command,
+   label, label[0] ? " " : "", command,
(command & CMD_HIRD) ? " HIRD" : "",
(command & CMD_PPCEE) ? " PPCEE" : "",
(command & CMD_FSP) ? " FSP" : "",
@@ -232,7 +231,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
(command & CMD_IAAD) ? " IAAD" : "",
(command & CMD_ASE) ? " Async" : "",
(command & CMD_PSE) ? " Periodic" : "",
-   fls_strings [(command >> 2) & 0x3],
+   fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
(command & CMD_RUN) ? "RUN" : "HALT"
);
@@ -254,7 +253,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
return scnprintf(buf, len,
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", port, status,
+   label, label[0] ? " " : "", port, status,
status>>25,/*device address */
(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
@@ -653,10 +652,10 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
 */
spin_lock_irqsave(&ehci->lock, flags);
for (i = 0; i < ehci->periodic_size; i++) {
-   p = ehci->pshadow [i];
+   p = ehci->pshadow[i];
if (likely(!p.ptr))
continue;
-   tag = Q_NEXT_TYPE(ehci, ehci->periodic [i]);
+

[PATCH v2 08/16] usb: host: ehci-dbg: fix up function definitions

This patch indents not empty functions to have the opening brace at the
beginning of the next line and body conforming coding style.

This also makes the function definition consistent with the file coding
style aligning parameters in sequential lines and indenting them with
two tabs.

Signed-off-by: Geyslan G. Bem 
---

Notes:
v2:
 - removes changes to empty functions
 - fix up function header indentation (2 tabs)

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 97a0582..44ca226 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -288,19 +288,27 @@ dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh 
*qh)
 
 static inline int __maybe_unused
 dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
-{ return 0; }
+{
+   return 0;
+}
 
 #endif /* CONFIG_DYNAMIC_DEBUG */
 
@@ -404,12 +412,8 @@ static inline char token_mark(struct ehci_hcd *ehci, 
__hc32 token)
return '/';
 }
 
-static void qh_lines(
-   struct ehci_hcd *ehci,
-   struct ehci_qh *qh,
-   char **nextp,
-   unsigned *sizep
-)
+static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
+   char **nextp, unsigned *sizep)
 {
u32 scratch;
u32 hw_curr;
@@ -957,7 +961,7 @@ done:
 }
 
 static struct debug_buffer *alloc_buffer(struct usb_bus *bus,
-   ssize_t (*fill_func)(struct debug_buffer *))
+   ssize_t (*fill_func)(struct debug_buffer *))
 {
struct debug_buffer *buf;
 
@@ -997,7 +1001,7 @@ out:
 }
 
 static ssize_t debug_output(struct file *file, char __user *user_buf,
-   size_t len, loff_t *offset)
+   size_t len, loff_t *offset)
 {
struct debug_buffer *buf = file->private_data;
int ret = 0;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/16] usb: host: ehci-dbg: use scnprintf() in qh_lines()

This patch replaces two snprintf() calls with scnprintf() in qh_lines()
and hence removes the unneeded sequential truncation tests.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 980ca55..97a0582 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -484,7 +484,7 @@ static void qh_lines(
type = "?";
break;
}
-   temp = snprintf(next, size,
+   temp = scnprintf(next, size,
"\n\t%p%c%s len=%d %08x urb %p"
" [td %08x buf[0] %08x]",
td, mark, type,
@@ -493,17 +493,13 @@ static void qh_lines(
td->urb,
(u32) td->qtd_dma,
hc32_to_cpup(ehci, &td->hw_buf[0]));
-   if (size < temp)
-   temp = size;
size -= temp;
next += temp;
if (temp == size)
goto done;
}
 
-   temp = snprintf(next, size, "\n");
-   if (size < temp)
-   temp = size;
+   temp = scnprintf(next, size, "\n");
size -= temp;
next += temp;
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/16] usb: host: ehci-dbg: add blank line after declarations

This patch fixes coding style issues reported by checkpatch concerning
to missing line after variable declarations.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 9efb2d1..8c55d9a 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1071,6 +1071,7 @@ static int debug_bandwidth_open(struct inode *inode, 
struct file *file)
 static int debug_periodic_open(struct inode *inode, struct file *file)
 {
struct debug_buffer *buf;
+
buf = alloc_buffer(inode->i_private, fill_periodic_buffer);
if (!buf)
return -ENOMEM;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/16] usb: host: ehci-dbg: use a blank line after struct declarations

This patch fixes coding style issues reported by checkpatch concerning
to missing line after struct declarations.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 44ca226..7e07014 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -357,6 +357,7 @@ static const struct file_operations debug_async_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_bandwidth_fops = {
.owner  = THIS_MODULE,
.open   = debug_bandwidth_open,
@@ -364,6 +365,7 @@ static const struct file_operations debug_bandwidth_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_periodic_fops = {
.owner  = THIS_MODULE,
.open   = debug_periodic_open,
@@ -371,6 +373,7 @@ static const struct file_operations debug_periodic_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_registers_fops = {
.owner  = THIS_MODULE,
.open   = debug_registers_open,
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/16] usb: host: ehci-dbg: convert macro to inline function

This patch converts macros into inline functions since the usage of
second is encouraged by Coding Style instead of the first.

Macros converted to functions:
 - dbg_status
 - dbg_cmd
 - dbg_port
 - speed_char

The size after changes remains the same.

Before:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

After:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

Signed-off-by: Geyslan G. Bem 
---

Notes:
The comment

/* functions have the "wrong" filename when they're output... */

was removed because after changes the file remained the same size and
the symbols of the new inline functions were not created, so they were
properly inlined.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 -
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 7e07014..9efb2d1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -312,23 +312,31 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
 
 #endif /* CONFIG_DYNAMIC_DEBUG */
 
-/* functions have the "wrong" filename when they're output... */
-#define dbg_status(ehci, label, status) { \
-   char _buf [80]; \
-   dbg_status_buf (_buf, sizeof _buf, label, status); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_status(struct ehci_hcd *ehci, const char *label, u32 status)
+{
+   char buf[80];
+
+   dbg_status_buf(buf, sizeof(buf), label, status);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_cmd(ehci, label, command) { \
-   char _buf [80]; \
-   dbg_command_buf (_buf, sizeof _buf, label, command); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_cmd(struct ehci_hcd *ehci, const char *label, u32 command)
+{
+   char buf[80];
+
+   dbg_command_buf(buf, sizeof(buf), label, command);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_port(ehci, label, port, status) { \
-   char _buf [80]; \
-   dbg_port_buf (_buf, sizeof _buf, label, port, status); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_port(struct ehci_hcd *ehci, const char *label, int port, u32 status)
+{
+   char buf[80];
+
+   dbg_port_buf(buf, sizeof(buf), label, port, status);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
 /*-*/
@@ -393,13 +401,19 @@ struct debug_buffer {
size_t alloc_size;
 };
 
-#define speed_char(info1) ({ char tmp; \
-   switch (info1 & (3 << 12)) { \
-   case QH_FULL_SPEED: tmp = 'f'; break; \
-   case QH_LOW_SPEED:  tmp = 'l'; break; \
-   case QH_HIGH_SPEED: tmp = 'h'; break; \
-   default: tmp = '?'; break; \
-   } tmp; })
+static inline char speed_char(u32 info1)
+{
+   switch (info1 & (3 << 12)) {
+   case QH_FULL_SPEED:
+   return 'f';
+   case QH_LOW_SPEED:
+   return 'l';
+   case QH_HIGH_SPEED:
+   return 'h';
+   default:
+   return '?';
+   }
+}
 
 static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
 {
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 12/16] usb: host: ehci-dbg: remove blank line before close brace

This patch fixes coding style issue reported by checkpatch concerning to
an unnecessary line before close brace.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 8c55d9a..37dac75 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1038,7 +1038,6 @@ static ssize_t debug_output(struct file *file, char 
__user *user_buf,
 
 out:
return ret;
-
 }
 
 static int debug_close(struct inode *inode, struct file *file)
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 13/16] usb: host: ehci-dbg: replace sizeof operand

This patch fixes a coding style issue reported by checkpatch concerning
to usage of sizeof operand as a variable instead the type.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 37dac75..22e6d4c 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -982,7 +982,7 @@ static struct debug_buffer *alloc_buffer(struct usb_bus 
*bus,
 {
struct debug_buffer *buf;
 
-   buf = kzalloc(sizeof(struct debug_buffer), GFP_KERNEL);
+   buf = kzalloc(sizeof(*buf), GFP_KERNEL);
 
if (buf) {
buf->bus = bus;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 14/16] usb: host: ehci-dbg: enclose conditional blocks with braces

This patch fixes coding style issues reported by checkpatch concerning
to conditional blocks without braces.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only

 drivers/usb/host/ehci-dbg.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 22e6d4c..760b1d6 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -481,11 +481,11 @@ static void qh_lines(struct ehci_hcd *ehci, struct 
ehci_qh *qh,
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
-   if (hw_curr == td->qtd_dma)
+   if (hw_curr == td->qtd_dma) {
mark = '*';
-   else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma))
+   } else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma)) {
mark = '+';
-   else if (QTD_LENGTH(scratch)) {
+   } else if (QTD_LENGTH(scratch)) {
if (td->hw_alt_next == ehci->async->hw->hw_alt_next)
mark = '#';
else if (td->hw_alt_next != list_end)
@@ -758,8 +758,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
 
if (seen_count < DBG_SCHED_LIMIT)
seen[seen_count++].qh = p.qh;
-   } else
+   } else {
temp = 0;
+   }
tag = Q_NEXT_TYPE(ehci, hw->hw_next);
p = p.qh->qh_next;
break;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 15/16] usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size

This patch fixes a coding style issue reported by checkpatch related to
kmalloc_array usage.

On the same line the sizeof operand was enclosed in parentheses.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 760b1d6..da41334 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -664,7 +664,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
unsignedi;
__hc32  tag;
 
-   seen = kmalloc(DBG_SCHED_LIMIT * sizeof *seen, GFP_ATOMIC);
+   seen = kmalloc_array(DBG_SCHED_LIMIT, sizeof(*seen), GFP_ATOMIC);
if (!seen)
return 0;
seen_count = 0;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 16/16] usb: host: ehci-dbg: add function output_buf_tds_dir()

This patch fixes a coding style issue reported by checkpatch related to
too many leading tabs.

This moves part of the fill_periodic_buffer() to the new function
output_buf_tds_dir().

Because it's inline, the file size has not changed.

Before:
  text  data  bssdec   hex  filename
 3692081   12  37013  9095  drivers/usb/host/ehci-hcd.o

After:
  text  data  bssdec   hex  filename
 3692081   12  37013  9095  drivers/usb/host/ehci-hcd.o

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 62 +
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index da41334..207c7eb 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -652,6 +652,33 @@ static ssize_t fill_bandwidth_buffer(struct debug_buffer 
*buf)
return next - buf->output_buf;
 }
 
+static unsigned output_buf_tds_dir(char *buf, struct ehci_hcd *ehci,
+   struct ehci_qh_hw *hw, struct ehci_qh *qh, unsigned size)
+{
+   u32 scratch = hc32_to_cpup(ehci, &hw->hw_info1);
+   struct ehci_qtd *qtd;
+   char*type = "";
+   unsignedtemp = 0;
+
+   /* count tds, get ep direction */
+   list_for_each_entry(qtd, &qh->qtd_list, qtd_list) {
+   temp++;
+   switch ((hc32_to_cpu(ehci, qtd->hw_token) >> 8) & 0x03) {
+   case 0:
+   type = "out";
+   continue;
+   case 1:
+   type = "in";
+   continue;
+   }
+   }
+
+   return scnprintf(next, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
+   speed_char(scratch), scratch & 0x007f,
+   (scratch >> 8) & 0x000f, type, qh->ps.usecs,
+   qh->ps.c_usecs, temp, 0x7ff & (scratch >> 16));
+}
+
 #define DBG_SCHED_LIMIT 64
 static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 {
@@ -722,39 +749,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
}
/* show more info the first time around */
if (temp == seen_count) {
-   u32 scratch = hc32_to_cpup(ehci,
-   &hw->hw_info1);
-   struct ehci_qtd *qtd;
-   char*type = "";
-
-   /* count tds, get ep direction */
-   temp = 0;
-   list_for_each_entry(qtd,
-   &p.qh->qtd_list,
-   qtd_list) {
-   temp++;
-   switch ((hc32_to_cpu(ehci,
-   qtd->hw_token) >> 8)
-   & 0x03) {
-   case 0:
-   type = "out";
-   continue;
-   case 1:
-   type = "in";
-   continue;
-   }
-   }
-
-   temp = scnprintf(next, size,
-   " (%c%d ep%d%s "
-   "[%d/%d] q%d p%d)",
-   speed_char (scratch),
-   scratch & 0x007f,
-   (scratch >> 8) & 0x000f, type,
-   p.qh->ps.usecs,
-   p.qh->ps.c_usecs,
-   temp,
-   0x7ff & (scratch >> 16));
+   temp = output_buf_tds_dir(next, ehci,
+   hw, p.qh, size);
 
if (seen_count < DBG_SCHED_LIMIT)
seen[seen_count++].qh = p.qh;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] usb-misc: cleanup sisusbvga

On Tue, 2016-01-05 at 17:54 +0100, Peter Senna Tschudin wrote:
[]
> Patch 1 is the biggest and fix only whitespace, tab and newline issues. I used
> 
> $ git diff -w --word-diff=porcelain drivers/usb/misc/sisusbvga/sisusb.c
> 
> to verify that this patch do not change any visible character. If the size of
> this patch is a problem, please let me know in how many patches to split it.

Thanks.

Maybe for future series, but not one in particular unless
you feel like respinning it:

As porcelain is meant for script consumption,  I think a good
mechanism
for human review is to separate patch 1 into 2 patches.

1: Only horizontal line whitespace changes
   git diff -w shouldn't show any changes and
   git blame -w would not show any change either.
2: Only vertical line changes additions/reformatting

This makes it easy to use git diff -w on patch 1 and
patch 2 can be more easily scanned visually.

It's also nice to use objdiff and show no object changes.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/16] usb: host: ehci-dbg: cleanup and refactoring

On Tue, 5 Jan 2016, Geyslan G. Bem wrote:

> This patchset removes all errors reported by checkpatch in addition to
> some refactoring.
> 
> v2:
>  - changes:
>* usb: host: ehci-dbg: fix up function definitions
>  - adds:
>* usb: host: ehci-dbg: use scnprintf() in qh_lines()
>  - removes:
>* usb: host: ehci-dbg: fix unsigned comparison
>* usb: host: ehci-dbg: remove unnecessary space after cast
> 
> Geyslan G. Bem (16):
>   usb: host: ehci-dbg: remove space before open parenthesis
>   usb: host: ehci-dbg: remove space before open square bracket
>   usb: host: ehci-dbg: use C89-style comments
>   usb: host: ehci-dbg: move trailing statements to next line
>   usb: host: ehci-dbg: fix up closing parenthesis
>   usb: host: ehci-dbg: put spaces around operators
>   usb: host: ehci-dbg: use scnprintf() in qh_lines()
>   usb: host: ehci-dbg: fix up function definitions
>   usb: host: ehci-dbg: use a blank line after struct declarations
>   usb: host: ehci-dbg: convert macro to inline function
>   usb: host: ehci-dbg: add blank line after declarations
>   usb: host: ehci-dbg: remove blank line before close brace
>   usb: host: ehci-dbg: replace sizeof operand
>   usb: host: ehci-dbg: enclose conditional blocks with braces
>   usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size
>   usb: host: ehci-dbg: add function output_buf_tds_dir()
> 
>  drivers/usb/host/ehci-dbg.c | 473 
> +++-
>  1 file changed, 252 insertions(+), 221 deletions(-)

Acked-by: Alan Stern 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [argyllcms] [Colormunki] dispread: Error - new_disprd failed with 'Instrument Access Failed'

2016-01-05 Thread Niccolò Belli

Hi Graeme,
Here is the USBMon trace of the timeout:
https://sourceforge.net/p/dispcalgui/bug-reports/_discuss/thread/73a10891/735c/attachment/1.mon.out.txt
I also uploaded a video on youtube to show when I get the timeout:
https://www.youtube.com/watch?v=_W7j8j4pKKQ&feature=youtu.be
It manages to calibrate the display and to read all the patches to make the
profile, but when it finally wants to access the instrument once again for
the final verification step it fails.

I CCed Alan Stern as you suggested.

Thanks,
Niccolò

On martedì 5 gennaio 2016 03:17:01 CET, Graeme Gill wrote:

Niccolò Belli wrote:
I'm using dispcalgui 3.0.6.0 with Argyllcms 1.8.3 on Archlinux
x64. I get this error while
trying to calibrate my monitor (not immediately but after a
while). My laptop only has
usb3 ports.

Hi,
I'm not sure there is much I can do about this sort of problem -
it appears to be the result of some interaction between the Linux
USB hub driver, hub hardware and ColorMunki hardware. The most
direct way of looking for an explanation would be to capture
a USBMon trace of the timeout, and then report the problem to the
linux-usb list, and see if Alan Stern is willing to take a look at it.

As a workaround you could try using a different USB port, or trying
running it through a USB hub.

Graeme Gill.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 16/16] usb: host: ehci-dbg: add function output_buf_tds_dir()

2016-01-05 Thread kbuild test robot

Hi Geyslan,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on next-20160105]
[cannot apply to v4.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Geyslan-G-Bem/usb-host-ehci-dbg-cleanup-and-refactoring/20160106-024114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: x86_64-rhel (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/usb/host/ehci-hcd.c:139:0:
   drivers/usb/host/ehci-dbg.c: In function 'output_buf_tds_dir':
>> drivers/usb/host/ehci-dbg.c:676:19: error: 'next' undeclared (first use in 
>> this function)
 return scnprintf(next, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
  ^
   drivers/usb/host/ehci-dbg.c:676:19: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/usb/host/ehci-dbg.c:680:1: warning: control reaches end of non-void 
>> function [-Wreturn-type]
}
^

vim +/next +676 drivers/usb/host/ehci-dbg.c

   670  case 1:
   671  type = "in";
   672  continue;
   673  }
   674  }
   675  
 > 676  return scnprintf(next, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
   677  speed_char(scratch), scratch & 0x007f,
   678  (scratch >> 8) & 0x000f, type, qh->ps.usecs,
   679  qh->ps.c_usecs, temp, 0x7ff & (scratch >> 16));
 > 680  }
   681  
   682  #define DBG_SCHED_LIMIT 64
   683  static ssize_t fill_periodic_buffer(struct debug_buffer *buf)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH v2 16/16] usb: host: ehci-dbg: add function output_buf_tds_dir()

2016-01-05 16:23 GMT-03:00 kbuild test robot :
> Hi Geyslan,
>
> [auto build test ERROR on usb/usb-testing]
> [also build test ERROR on next-20160105]
> [cannot apply to v4.4-rc8]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Geyslan-G-Bem/usb-host-ehci-dbg-cleanup-and-refactoring/20160106-024114
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> config: x86_64-rhel (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All error/warnings (new ones prefixed by >>):
>
>In file included from drivers/usb/host/ehci-hcd.c:139:0:
>drivers/usb/host/ehci-dbg.c: In function 'output_buf_tds_dir':
>>> drivers/usb/host/ehci-dbg.c:676:19: error: 'next' undeclared (first use in 
>>> this function)
>  return scnprintf(next, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
Bad. Must be buf, resending.

>   ^
>drivers/usb/host/ehci-dbg.c:676:19: note: each undeclared identifier is 
> reported only once for each function it appears in
>>> drivers/usb/host/ehci-dbg.c:680:1: warning: control reaches end of non-void 
>>> function [-Wreturn-type]
> }
> ^
>
> vim +/next +676 drivers/usb/host/ehci-dbg.c
>
>670  case 1:
>671  type = "in";
>672  continue;
>673  }
>674  }
>675
>  > 676  return scnprintf(next, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
>677  speed_char(scratch), scratch & 0x007f,
>678  (scratch >> 8) & 0x000f, type, qh->ps.usecs,
>679  qh->ps.c_usecs, temp, 0x7ff & (scratch >> 
> 16));
>  > 680  }
>681
>682  #define DBG_SCHED_LIMIT 64
>683  static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation



-- 
Regards,

Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 04/16] usb: host: ehci-dbg: move trailing statements to next line

This patch fixes coding style issues reported by checkpatch concerning
to switch case statements. There are few additional changes made to fix
other coding styles issues.

These additional changes are:

 - The compound statement "({...})" on line 474 is pulled out from
   snprintf parameters.

 - On line 723 the constant "0x03" is moved to right.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 14 warnings about trailing statements that
should be on next line. After there are still 4. These lines concern to
a macro that will be modified to inline function in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 +++--
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index df9f598..c409e4f 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -243,10 +243,18 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
 
/* signaling state */
switch (status & (3 << 10)) {
-   case 0 << 10: sig = "se0"; break;
-   case 1 << 10: sig = "k"; break; /* low speed */
-   case 2 << 10: sig = "j"; break;
-   default: sig = "?"; break;
+   case 0 << 10:
+   sig = "se0";
+   break;
+   case 1 << 10: /* low speed */
+   sig = "k";
+   break;
+   case 2 << 10:
+   sig = "j";
+   break;
+   default:
+   sig = "?";
+   break;
}
 
return scnprintf(buf, len,
@@ -451,6 +459,8 @@ static void qh_lines(
 
/* hc may be modifying the list as we read it ... */
list_for_each(entry, &qh->qtd_list) {
+   char *type;
+
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
@@ -464,16 +474,24 @@ static void qh_lines(
else if (td->hw_alt_next != list_end)
mark = '/';
}
+   switch ((scratch >> 8) & 0x03) {
+   case 0:
+   type = "out";
+   break;
+   case 1:
+   type = "in";
+   break;
+   case 2:
+   type = "setup";
+   break;
+   default:
+   type = "?";
+   break;
+   }
temp = snprintf(next, size,
"\n\t%p%c%s len=%d %08x urb %p"
" [td %08x buf[0] %08x]",
-   td, mark, ({ char *tmp;
-switch ((scratch>>8)&0x03) {
-case 0: tmp = "out"; break;
-case 1: tmp = "in"; break;
-case 2: tmp = "setup"; break;
-default: tmp = "?"; break;
-} tmp;}),
+   td, mark, type,
(scratch >> 16) & 0x7fff,
scratch,
td->urb,
@@ -702,11 +720,15 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
&p.qh->qtd_list,
qtd_list) {
temp++;
-   switch (0x03 & (hc32_to_cpu(
-   ehci,
-   qtd->hw_token) >> 8)) {
-   case 0: type = "out"; continue;
-   case 1: type = "in"; continue;
+   switch ((hc32_to_cpu(ehci,
+   qtd->hw_token) >> 8)
+   & 0x03) {
+   case 0:
+   type = "out";
+   continue;
+   case 1:
+   type = "in";
+   continue;
}
}
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 00/16] usb: host: ehci-dbg: cleanup and refactoring

This patchset removes all errors reported by checkpatch in addition to
some refactoring.

v2:
 - changes:
   * usb: host: ehci-dbg: fix up function definitions
 - adds:
   * usb: host: ehci-dbg: use scnprintf() in qh_lines()
 - removes:
   * usb: host: ehci-dbg: fix unsigned comparison
   * usb: host: ehci-dbg: remove unnecessary space after cast

v3:
 - fixes:
  * usb: host: ehci-dbg: add function output_buf_tds_dir()

Geyslan G. Bem (16):
  usb: host: ehci-dbg: remove space before open parenthesis
  usb: host: ehci-dbg: remove space before open square bracket
  usb: host: ehci-dbg: use C89-style comments
  usb: host: ehci-dbg: move trailing statements to next line
  usb: host: ehci-dbg: fix up closing parenthesis
  usb: host: ehci-dbg: put spaces around operators
  usb: host: ehci-dbg: use scnprintf() in qh_lines()
  usb: host: ehci-dbg: fix up function definitions
  usb: host: ehci-dbg: use a blank line after struct declarations
  usb: host: ehci-dbg: convert macro to inline function
  usb: host: ehci-dbg: add blank line after declarations
  usb: host: ehci-dbg: remove blank line before close brace
  usb: host: ehci-dbg: replace sizeof operand
  usb: host: ehci-dbg: enclose conditional blocks with braces
  usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size
  usb: host: ehci-dbg: add function output_buf_tds_dir()

 drivers/usb/host/ehci-dbg.c | 473 +++-
 1 file changed, 252 insertions(+), 221 deletions(-)

-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 05/16] usb: host: ehci-dbg: fix up closing parenthesis

This patch puts the closing parenthesis at the statement end removing
unnecessary "new line".

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c409e4f..3b423e1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -35,8 +35,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
HCS_N_PCC(params),
HCS_PORTROUTED(params) ? "" : " ordered",
HCS_PPC(params) ? "" : " !ppc",
-   HCS_N_PORTS(params)
-   );
+   HCS_N_PORTS(params));
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
@@ -189,8 +188,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, 
u32 status)
(status & STS_FLR) ? " FLR" : "",
(status & STS_PCD) ? " PCD" : "",
(status & STS_ERR) ? " ERR" : "",
-   (status & STS_INT) ? " INT" : ""
-   );
+   (status & STS_INT) ? " INT" : "");
 }
 
 static int __maybe_unused
@@ -205,8 +203,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
(enable & STS_FLR) ? " FLR" : "",
(enable & STS_PCD) ? " PCD" : "",
(enable & STS_ERR) ? " ERR" : "",
-   (enable & STS_INT) ? " INT" : ""
-   );
+   (enable & STS_INT) ? " INT" : "");
 }
 
 static const char *const fls_strings[] = { "1024", "512", "256", "??" };
@@ -232,8 +229,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
(command & CMD_PSE) ? " Periodic" : "",
fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
-   (command & CMD_RUN) ? "RUN" : "HALT"
-   );
+   (command & CMD_RUN) ? "RUN" : "HALT");
 }
 
 static int
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 03/16] usb: host: ehci-dbg: use C89-style comments

This patch fixes coding style issues reported by checkpatch.

Coding style demands usage of C89-style comments and a specific format
when it's multiline.

This also removes the Free Software Foundation address because FSF can
change it again.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 52bf3fe..df9f598 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -11,16 +11,14 @@
  * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
  * for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 /* this file is part of ehci-hcd.c */
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCSPARAMS register
+/*
+ * check the values in the HCSPARAMS register
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
@@ -46,7 +44,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
-   // FIXME MIPS won't readb() ...
+   /* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
@@ -63,10 +61,11 @@ static inline void dbg_hcs_params(struct ehci_hcd *ehci, 
char *label) {}
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCCPARAMS register
+/*
+ * check the values in the HCCPARAMS register
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
- * */
+ */
 static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
@@ -515,7 +514,8 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 
*next = 0;
 
-   /* dumps a snapshot of the async schedule.
+   /*
+* dumps a snapshot of the async schedule.
 * usually empty except for long-term bulk reads, or head.
 * one QH per line, and TDs we know about
 */
@@ -647,7 +647,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
size -= temp;
next += temp;
 
-   /* dump a snapshot of the periodic schedule.
+   /*
+* dump a snapshot of the periodic schedule.
 * iso changes, interrupt usually doesn't.
 */
spin_lock_irqsave(&ehci->lock, flags);
@@ -861,7 +862,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer 
*buf)
}
 #endif
 
-   // FIXME interpret both types of params
+   /* FIXME interpret both types of params */
i = ehci_readl(ehci, &ehci->caps->hcs_params);
temp = scnprintf(next, size, "structural params 0x%08x\n", i);
size -= temp;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 02/16] usb: host: ehci-dbg: remove space before open square bracket

This patch fixes coding style issues reported by checkpatch. The only
change in this patch that isn't just removing spaces before opening
square brackets is at line 213 where the initialization of fls_strings[]
is placed in same line.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 20 warnings about spaces before open square
bracket. After there are still 3. These lines concern to macros that
will be modified to inline functions in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 33 -
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index fcbbdfa..52bf3fe 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -42,7 +42,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
-   char buf [46], tmp [7], byte;
+   char buf[46], tmp[7], byte;
 
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
@@ -109,8 +109,8 @@ dbg_qtd(const char *label, struct ehci_hcd *ehci, struct 
ehci_qtd *qtd)
hc32_to_cpup(ehci, &qtd->hw_next),
hc32_to_cpup(ehci, &qtd->hw_alt_next),
hc32_to_cpup(ehci, &qtd->hw_token),
-   hc32_to_cpup(ehci, &qtd->hw_buf [0]));
-   if (qtd->hw_buf [1])
+   hc32_to_cpup(ehci, &qtd->hw_buf[0]));
+   if (qtd->hw_buf[1])
ehci_dbg(ehci, "  p1=%08x p2=%08x p3=%08x p4=%08x\n",
hc32_to_cpup(ehci, &qtd->hw_buf[1]),
hc32_to_cpup(ehci, &qtd->hw_buf[2]),
@@ -179,7 +179,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, 
u32 status)
 {
return scnprintf(buf, len,
"%s%sstatus %04x%s%s%s%s%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", status,
+   label, label[0] ? " " : "", status,
(status & STS_PPCE_MASK) ? " PPCE" : "",
(status & STS_ASS) ? " Async" : "",
(status & STS_PSS) ? " Periodic" : "",
@@ -199,7 +199,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
 {
return scnprintf(buf, len,
"%s%sintrenable %02x%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", enable,
+   label, label[0] ? " " : "", enable,
(enable & STS_PPCE_MASK) ? " PPCE" : "",
(enable & STS_IAA) ? " IAA" : "",
(enable & STS_FATAL) ? " FATAL" : "",
@@ -210,8 +210,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, 
u32 enable)
);
 }
 
-static const char *const fls_strings [] =
-{ "1024", "512", "256", "??" };
+static const char *const fls_strings[] = { "1024", "512", "256", "??" };
 
 static int
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
@@ -219,7 +218,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
return scnprintf(buf, len,
"%s%scommand %07x %s%s%s%s%s%s=%d ithresh=%d%s%s%s%s "
"period=%s%s %s",
-   label, label [0] ? " " : "", command,
+   label, label[0] ? " " : "", command,
(command & CMD_HIRD) ? " HIRD" : "",
(command & CMD_PPCEE) ? " PPCEE" : "",
(command & CMD_FSP) ? " FSP" : "",
@@ -232,7 +231,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, 
u32 command)
(command & CMD_IAAD) ? " IAAD" : "",
(command & CMD_ASE) ? " Async" : "",
(command & CMD_PSE) ? " Periodic" : "",
-   fls_strings [(command >> 2) & 0x3],
+   fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
(command & CMD_RUN) ? "RUN" : "HALT"
);
@@ -254,7 +253,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
return scnprintf(buf, len,
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
-   label, label [0] ? " " : "", port, status,
+   label, label[0] ? " " : "", port, status,
status>>25,/*device address */
(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
@@ -653,10 +652,10 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
 */
spin_lock_irqsave(&ehci->lock, flags);
for (i = 0; i < ehci->periodic_size; i++) {
-   p = ehci->pshadow [i];
+   p = ehci->pshadow[i];
if (likely(!p.ptr))
continue;
-   tag = Q_NEXT_TYPE(ehci, ehci->periodic [i]);
+

[PATCH v3 01/16] usb: host: ehci-dbg: remove space before open parenthesis

This patch fixes coding style issues reported by checkpatch. The vast
majority of changes in this patch are removing spaces before opening
parenthesis, but in some cases, a few additional changes are made to fix
other coding style issues.

These additional changes are:

 - Spaces around >> on line 50.
 - On line 55 a call to ehci_dbg reduced to a single line.
 - sizeof operands surrounded with parenthesis on lines 877, 883, 889
   and 901.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Before this patch there are 105 warnings about spaces before opening
parenthesis. After there are still 6. These lines concern to macros that
will be modified to inline functions in sequential patches.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 199 ++--
 1 file changed, 99 insertions(+), 100 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b7d623f..fcbbdfa 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -24,41 +24,40 @@
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
-static void dbg_hcs_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcs_params);
 
-   ehci_dbg (ehci,
+   ehci_dbg(ehci,
"%s hcs_params 0x%x dbg=%d%s cc=%d pcc=%d%s%s ports=%d\n",
label, params,
-   HCS_DEBUG_PORT (params),
-   HCS_INDICATOR (params) ? " ind" : "",
-   HCS_N_CC (params),
-   HCS_N_PCC (params),
-   HCS_PORTROUTED (params) ? "" : " ordered",
-   HCS_PPC (params) ? "" : " !ppc",
-   HCS_N_PORTS (params)
+   HCS_DEBUG_PORT(params),
+   HCS_INDICATOR(params) ? " ind" : "",
+   HCS_N_CC(params),
+   HCS_N_PCC(params),
+   HCS_PORTROUTED(params) ? "" : " ordered",
+   HCS_PPC(params) ? "" : " !ppc",
+   HCS_N_PORTS(params)
);
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
-   if (HCS_PORTROUTED (params)) {
+   if (HCS_PORTROUTED(params)) {
int i;
char buf [46], tmp [7], byte;
 
buf[0] = 0;
-   for (i = 0; i < HCS_N_PORTS (params); i++) {
+   for (i = 0; i < HCS_N_PORTS(params); i++) {
// FIXME MIPS won't readb() ...
-   byte = readb (&ehci->caps->portroute[(i>>1)]);
+   byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
strcat(buf, tmp);
}
-   ehci_dbg (ehci, "%s portroute %s\n",
-   label, buf);
+   ehci_dbg(ehci, "%s portroute %s\n", label, buf);
}
 }
 #else
 
-static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
@@ -68,19 +67,19 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci, 
char *label) {}
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
  * */
-static void dbg_hcc_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
 
-   if (HCC_ISOC_CACHE (params)) {
-   ehci_dbg (ehci,
+   if (HCC_ISOC_CACHE(params)) {
+   ehci_dbg(ehci,
"%s hcc_params %04x caching frame %s%s%s\n",
label, params,
HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024",
HCC_CANPARK(params) ? " park" : "",
HCC_64BIT_ADDR(params) ? " 64 bit addr" : "");
} else {
-   ehci_dbg (ehci,
+   ehci_dbg(ehci,
"%s hcc_params %04x thresh %d uframes %s%s%s%s%s%s%s\n",
label,
params,
@@ -97,14 +96,14 @@ static void dbg_hcc_params (struct ehci_hcd *ehci, char 
*label)
 }
 #else
 
-static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
 static void __maybe_unused
-dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
+dbg_qtd(const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
 {
ehci_dbg(ehci, "%s td %p n%08x %08x t%08x p0=%08x\n", label, qtd,
hc32_to_cpup(ehci, &qtd->hw_next),
@@ -120,22 +119,22 @@ dbg_qtd (const char *label, struct ehci_hcd *ehci, struct 
ehci_qtd *qtd)

[PATCH v3 07/16] usb: host: ehci-dbg: use scnprintf() in qh_lines()

This patch replaces two snprintf() calls with scnprintf() in qh_lines()
and hence removes the unneeded sequential truncation tests.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 980ca55..97a0582 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -484,7 +484,7 @@ static void qh_lines(
type = "?";
break;
}
-   temp = snprintf(next, size,
+   temp = scnprintf(next, size,
"\n\t%p%c%s len=%d %08x urb %p"
" [td %08x buf[0] %08x]",
td, mark, type,
@@ -493,17 +493,13 @@ static void qh_lines(
td->urb,
(u32) td->qtd_dma,
hc32_to_cpup(ehci, &td->hw_buf[0]));
-   if (size < temp)
-   temp = size;
size -= temp;
next += temp;
if (temp == size)
goto done;
}
 
-   temp = snprintf(next, size, "\n");
-   if (size < temp)
-   temp = size;
+   temp = scnprintf(next, size, "\n");
size -= temp;
next += temp;
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 11/16] usb: host: ehci-dbg: add blank line after declarations

This patch fixes coding style issues reported by checkpatch concerning
to missing line after variable declarations.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 9efb2d1..8c55d9a 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1071,6 +1071,7 @@ static int debug_bandwidth_open(struct inode *inode, 
struct file *file)
 static int debug_periodic_open(struct inode *inode, struct file *file)
 {
struct debug_buffer *buf;
+
buf = alloc_buffer(inode->i_private, fill_periodic_buffer);
if (!buf)
return -ENOMEM;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 10/16] usb: host: ehci-dbg: convert macro to inline function

This patch converts macros into inline functions since the usage of
second is encouraged by Coding Style instead of the first.

Macros converted to functions:
 - dbg_status
 - dbg_cmd
 - dbg_port
 - speed_char

The size after changes remains the same.

Before:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

After:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

Signed-off-by: Geyslan G. Bem 
---

Notes:
The comment

/* functions have the "wrong" filename when they're output... */

was removed because after changes the file remained the same size and
the symbols of the new inline functions were not created, so they were
properly inlined.

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 -
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 7e07014..9efb2d1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -312,23 +312,31 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
 
 #endif /* CONFIG_DYNAMIC_DEBUG */
 
-/* functions have the "wrong" filename when they're output... */
-#define dbg_status(ehci, label, status) { \
-   char _buf [80]; \
-   dbg_status_buf (_buf, sizeof _buf, label, status); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_status(struct ehci_hcd *ehci, const char *label, u32 status)
+{
+   char buf[80];
+
+   dbg_status_buf(buf, sizeof(buf), label, status);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_cmd(ehci, label, command) { \
-   char _buf [80]; \
-   dbg_command_buf (_buf, sizeof _buf, label, command); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_cmd(struct ehci_hcd *ehci, const char *label, u32 command)
+{
+   char buf[80];
+
+   dbg_command_buf(buf, sizeof(buf), label, command);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_port(ehci, label, port, status) { \
-   char _buf [80]; \
-   dbg_port_buf (_buf, sizeof _buf, label, port, status); \
-   ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_port(struct ehci_hcd *ehci, const char *label, int port, u32 status)
+{
+   char buf[80];
+
+   dbg_port_buf(buf, sizeof(buf), label, port, status);
+   ehci_dbg(ehci, "%s\n", buf);
 }
 
 /*-*/
@@ -393,13 +401,19 @@ struct debug_buffer {
size_t alloc_size;
 };
 
-#define speed_char(info1) ({ char tmp; \
-   switch (info1 & (3 << 12)) { \
-   case QH_FULL_SPEED: tmp = 'f'; break; \
-   case QH_LOW_SPEED:  tmp = 'l'; break; \
-   case QH_HIGH_SPEED: tmp = 'h'; break; \
-   default: tmp = '?'; break; \
-   } tmp; })
+static inline char speed_char(u32 info1)
+{
+   switch (info1 & (3 << 12)) {
+   case QH_FULL_SPEED:
+   return 'f';
+   case QH_LOW_SPEED:
+   return 'l';
+   case QH_HIGH_SPEED:
+   return 'h';
+   default:
+   return '?';
+   }
+}
 
 static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
 {
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 09/16] usb: host: ehci-dbg: use a blank line after struct declarations

This patch fixes coding style issues reported by checkpatch concerning
to missing line after struct declarations.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 44ca226..7e07014 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -357,6 +357,7 @@ static const struct file_operations debug_async_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_bandwidth_fops = {
.owner  = THIS_MODULE,
.open   = debug_bandwidth_open,
@@ -364,6 +365,7 @@ static const struct file_operations debug_bandwidth_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_periodic_fops = {
.owner  = THIS_MODULE,
.open   = debug_periodic_open,
@@ -371,6 +373,7 @@ static const struct file_operations debug_periodic_fops = {
.release= debug_close,
.llseek = default_llseek,
 };
+
 static const struct file_operations debug_registers_fops = {
.owner  = THIS_MODULE,
.open   = debug_registers_open,
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 15/16] usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size

This patch fixes a coding style issue reported by checkpatch related to
kmalloc_array usage.

On the same line the sizeof operand was enclosed in parentheses.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 760b1d6..da41334 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -664,7 +664,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
unsignedi;
__hc32  tag;
 
-   seen = kmalloc(DBG_SCHED_LIMIT * sizeof *seen, GFP_ATOMIC);
+   seen = kmalloc_array(DBG_SCHED_LIMIT, sizeof(*seen), GFP_ATOMIC);
if (!seen)
return 0;
seen_count = 0;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 16/16] usb: host: ehci-dbg: add function output_buf_tds_dir()

This patch fixes a coding style issue reported by checkpatch related to
too many leading tabs.

This moves part of the fill_periodic_buffer() to the new function
output_buf_tds_dir().

Because it's inline, the file size has not changed.

Before:
  text  data  bssdec   hex  filename
 3692081   12  37013  9095  drivers/usb/host/ehci-hcd.o

After:
  text  data  bssdec   hex  filename
 3692081   12  37013  9095  drivers/usb/host/ehci-hcd.o

Signed-off-by: Geyslan G. Bem 
---

Notes:
v3:
 - fix first parameter of scnprintf on line 676 (buf)

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 62 +
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index da41334..d2c0711 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -652,6 +652,33 @@ static ssize_t fill_bandwidth_buffer(struct debug_buffer 
*buf)
return next - buf->output_buf;
 }
 
+static unsigned output_buf_tds_dir(char *buf, struct ehci_hcd *ehci,
+   struct ehci_qh_hw *hw, struct ehci_qh *qh, unsigned size)
+{
+   u32 scratch = hc32_to_cpup(ehci, &hw->hw_info1);
+   struct ehci_qtd *qtd;
+   char*type = "";
+   unsignedtemp = 0;
+
+   /* count tds, get ep direction */
+   list_for_each_entry(qtd, &qh->qtd_list, qtd_list) {
+   temp++;
+   switch ((hc32_to_cpu(ehci, qtd->hw_token) >> 8) & 0x03) {
+   case 0:
+   type = "out";
+   continue;
+   case 1:
+   type = "in";
+   continue;
+   }
+   }
+
+   return scnprintf(buf, size, " (%c%d ep%d%s [%d/%d] q%d p%d)",
+   speed_char(scratch), scratch & 0x007f,
+   (scratch >> 8) & 0x000f, type, qh->ps.usecs,
+   qh->ps.c_usecs, temp, 0x7ff & (scratch >> 16));
+}
+
 #define DBG_SCHED_LIMIT 64
 static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 {
@@ -722,39 +749,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
}
/* show more info the first time around */
if (temp == seen_count) {
-   u32 scratch = hc32_to_cpup(ehci,
-   &hw->hw_info1);
-   struct ehci_qtd *qtd;
-   char*type = "";
-
-   /* count tds, get ep direction */
-   temp = 0;
-   list_for_each_entry(qtd,
-   &p.qh->qtd_list,
-   qtd_list) {
-   temp++;
-   switch ((hc32_to_cpu(ehci,
-   qtd->hw_token) >> 8)
-   & 0x03) {
-   case 0:
-   type = "out";
-   continue;
-   case 1:
-   type = "in";
-   continue;
-   }
-   }
-
-   temp = scnprintf(next, size,
-   " (%c%d ep%d%s "
-   "[%d/%d] q%d p%d)",
-   speed_char (scratch),
-   scratch & 0x007f,
-   (scratch >> 8) & 0x000f, type,
-   p.qh->ps.usecs,
-   p.qh->ps.c_usecs,
-   temp,
-   0x7ff & (scratch >> 16));
+   temp = output_buf_tds_dir(next, ehci,
+   hw, p.qh, size);
 
if (seen_count < DBG_SCHED_LIMIT)
seen[seen_count++].qh = p.qh;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 06/16] usb: host: ehci-dbg: put spaces around operators

This patch fixes coding style issues reported by checkpatch concerning
to missing spaces around operators.

There is an additional change on line 49 that removes unnecessary
parentheses around ternary operands.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 3b423e1..980ca55 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -46,7 +46,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
-   ((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
+   (i & 0x1) ? byte & 0xf : (byte >> 4) & 0xf);
strcat(buf, tmp);
}
ehci_dbg(ehci, "%s portroute %s\n", label, buf);
@@ -257,14 +257,14 @@ dbg_port_buf(char *buf, unsigned len, const char *label, 
int port, u32 status)
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
label, label[0] ? " " : "", port, status,
-   status>>25,/*device address */
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
+   status >> 25, /*device address */
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_NYET ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_NYET ?
" NYET" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_STALL ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_STALL ?
" STALL" : "",
-   (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ERR ?
+   (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ERR ?
" ERR" : "",
(status & PORT_POWER) ? " POWER" : "",
(status & PORT_OWNER) ? " OWNER" : "",
@@ -846,7 +846,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer 
*buf)
if (dev_is_pci(hcd->self.controller)) {
struct pci_dev  *pdev;
u32 offset, cap, cap2;
-   unsignedcount = 256/4;
+   unsignedcount = 256 / 4;
 
pdev = to_pci_dev(ehci_to_hcd(ehci)->self.controller);
offset = HCC_EXT_CAPS(ehci_readl(ehci,
@@ -1058,7 +1058,7 @@ static int debug_periodic_open(struct inode *inode, 
struct file *file)
if (!buf)
return -ENOMEM;
 
-   buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8)*PAGE_SIZE;
+   buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8) * PAGE_SIZE;
file->private_data = buf;
return 0;
 }
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 14/16] usb: host: ehci-dbg: enclose conditional blocks with braces

This patch fixes coding style issues reported by checkpatch concerning
to conditional blocks without braces.

Signed-off-by: Geyslan G. Bem 
---

Notes:
Tested by compilation only

 drivers/usb/host/ehci-dbg.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 22e6d4c..760b1d6 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -481,11 +481,11 @@ static void qh_lines(struct ehci_hcd *ehci, struct 
ehci_qh *qh,
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
-   if (hw_curr == td->qtd_dma)
+   if (hw_curr == td->qtd_dma) {
mark = '*';
-   else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma))
+   } else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma)) {
mark = '+';
-   else if (QTD_LENGTH(scratch)) {
+   } else if (QTD_LENGTH(scratch)) {
if (td->hw_alt_next == ehci->async->hw->hw_alt_next)
mark = '#';
else if (td->hw_alt_next != list_end)
@@ -758,8 +758,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer 
*buf)
 
if (seen_count < DBG_SCHED_LIMIT)
seen[seen_count++].qh = p.qh;
-   } else
+   } else {
temp = 0;
+   }
tag = Q_NEXT_TYPE(ehci, hw->hw_next);
p = p.qh->qh_next;
break;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 13/16] usb: host: ehci-dbg: replace sizeof operand

This patch fixes a coding style issue reported by checkpatch concerning
to usage of sizeof operand as a variable instead the type.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 37dac75..22e6d4c 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -982,7 +982,7 @@ static struct debug_buffer *alloc_buffer(struct usb_bus 
*bus,
 {
struct debug_buffer *buf;
 
-   buf = kzalloc(sizeof(struct debug_buffer), GFP_KERNEL);
+   buf = kzalloc(sizeof(*buf), GFP_KERNEL);
 
if (buf) {
buf->bus = bus;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 08/16] usb: host: ehci-dbg: fix up function definitions

This patch indents not empty functions to have the opening brace at the
beginning of the next line and body conforming coding style.

This also makes the function definition consistent with the file coding
style aligning parameters in sequential lines and indenting them with
two tabs.

Signed-off-by: Geyslan G. Bem 
---

Notes:
v2:
 - removes changes to empty functions
 - fix up function header indentation (2 tabs)

Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 97a0582..44ca226 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -288,19 +288,27 @@ dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh 
*qh)
 
 static inline int __maybe_unused
 dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
-{ return 0; }
+{
+   return 0;
+}
 
 static inline int __maybe_unused
 dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
-{ return 0; }
+{
+   return 0;
+}
 
 #endif /* CONFIG_DYNAMIC_DEBUG */
 
@@ -404,12 +412,8 @@ static inline char token_mark(struct ehci_hcd *ehci, 
__hc32 token)
return '/';
 }
 
-static void qh_lines(
-   struct ehci_hcd *ehci,
-   struct ehci_qh *qh,
-   char **nextp,
-   unsigned *sizep
-)
+static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
+   char **nextp, unsigned *sizep)
 {
u32 scratch;
u32 hw_curr;
@@ -957,7 +961,7 @@ done:
 }
 
 static struct debug_buffer *alloc_buffer(struct usb_bus *bus,
-   ssize_t (*fill_func)(struct debug_buffer *))
+   ssize_t (*fill_func)(struct debug_buffer *))
 {
struct debug_buffer *buf;
 
@@ -997,7 +1001,7 @@ out:
 }
 
 static ssize_t debug_output(struct file *file, char __user *user_buf,
-   size_t len, loff_t *offset)
+   size_t len, loff_t *offset)
 {
struct debug_buffer *buf = file->private_data;
int ret = 0;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [argyllcms] [Colormunki] dispread: Error - new_disprd failed with 'Instrument Access Failed'

On Tue, 5 Jan 2016, [iso-8859-1] Niccol� Belli wrote:

> Hi Graeme,
> Here is the USBMon trace of the timeout: 
> https://sourceforge.net/p/dispcalgui/bug-reports/_discuss/thread/73a10891/735c/attachment/1.mon.out.txt
> I also uploaded a video on youtube to show when I get the timeout: 
> https://www.youtube.com/watch?v=_W7j8j4pKKQ&feature=youtu.be
> It manages to calibrate the display and to read all the patches to make the 
> profile, but when it finally wants to access the instrument once again for 
> the final verification step it fails.
> I CCed Alan Stern as you suggested.

The only thing that looked suspicious in the usbmon trace was the way 
the program issued Clear-Halt requests for endpoints 1, 2, and 3 each 
time it started up.  Such things can lead to toggle mismatch errors, 
depending on the device.

In general it's a good idea never to use Clear-Halt unless you really 
do need to clear an endpoint's Halted feature.  If those requests were 
removed from the program, perhaps the timeout won't occur.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 12/16] usb: host: ehci-dbg: remove blank line before close brace

This patch fixes coding style issue reported by checkpatch concerning to
an unnecessary line before close brace.

Signed-off-by: Geyslan G. Bem 
---
 drivers/usb/host/ehci-dbg.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 8c55d9a..37dac75 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1038,7 +1038,6 @@ static ssize_t debug_output(struct file *file, char 
__user *user_buf,
 
 out:
return ret;
-
 }
 
 static int debug_close(struct inode *inode, struct file *file)
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I NEED YOUR IMMEDIATE ATTENTION ON MY PROPOSAL

2016-01-05 Thread Mr. Wang Zhiqiang




I am Mr. Wang Zhiqiang working with Wing Lung Bank Hong Kong; I have a
highly mutual and legitimate Bequest for you to handle with me. This fund
was deposited in our bank by an Oil Magnate who lived in Hong Kong for
Twenty Eight years. He died along with his family during the Tsunami which
occurred Dec 2004. You can contact me for more details if interested.

Best regards,
Mr. Wang Zhiqiang
Private Email:wangzhiqian...@yahoo.com.hk

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: dwc2: regression on bcm2835 after commit 09c96980dc72

2016-01-05 Thread John Youn

On 1/2/2016 4:00 PM, Stefan Wahren wrote:
> Hi Remi,
> 
>> Remi Pommarel  hat am 2. Januar 2016 um 14:55 geschrieben:
>>
>>
>> Hi Stefan, Hi John,
>>
>> On Mon, Dec 28, 2015 at 3:16:47AM -0800, Stefan Wharen wrote:
> ...
>>
>> I came up with the same patch, but it does not seem to fix everything. Indeed
>> 263b7fb557f797d9d4d1dcf93fb6bb2efc3f1d46 (previous patch in patchset) seems
>> to break USB ethernet also.
> 
> yes i can confirm that this patch is also a regression. I get the following
> 
> 256 invalid for host_nperio_tx_fifo_size. Check HW configuration.
> 512 invalid for host_perio_tx_fifo_size. Check HW configuration.
> 
> without changing any parameter above.
> 
>>
>> My guess is that bcm2835 still need dwc2_core_reset_and_force_dr_mode()
>> before dwc2_get_hwparams().
> 
> I agree. IMHO this patch should be reverted.
> 
>>
>> Do you have any idea ?
>>
>> Regards
>>
>> --
>> Remi
> 

Ok, I'll get these fixes in.

I'm not totally sure what the problem is. I'll see if I can get a
raspberry pi to test it out.

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Felipe Ferreri Tonello

Hi Dan,

On 05/01/16 12:44, Dan Carpenter wrote:
> On Tue, Jan 05, 2016 at 01:28:11PM +0100, Julia Lawall wrote:
>>
>>
>> On Tue, 5 Jan 2016, kbuild test robot wrote:
>>
>>> Hi Dan,
>>>
>>> [auto build test WARNING on balbi-usb/next]
>>> [also build test WARNING on v4.4-rc8 next-20160105]
>>> [if your patch is applied to the wrong git tree, please drop us a note to 
>>> help improving the system]
>>>
>>> url:
>>> https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
>>>
>>>
>>> coccinelle warnings: (new ones prefixed by >>)
>>>
>>>>> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL but 
>>>>> dereferenced.
>>
>> It's a false positive for coccinelle, but I wonder if avoiding duplicating
>> the mutex_lock is really worth it?
> 
> It's not the most beautiful code in the world.  I considered a bunch of
> different ways to write it...  This is what Felipe Tonello wanted,
> though.

This case is not a matter of been pretty but a matter of been less error
prone.

What would you suggest?

Thanks,
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: Does vm_operations_struct require a .owner field?

2016-01-05 Thread Kirill A. Shutemov

On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> Hello:
> 
> Question: The vm_operations_struct structure contains lots of callback
> pointers.  Is there any mechanism to prevent the callback routines and
> the structure itself being unloaded from memory (if they are built into
> modules) while the relevant VMAs are still in use?
> 
> Consider a simple example: A user program calls mmap(2) on a device
> file.  Later on, the file is closed and the device driver's module is
> unloaded.  But until munmap(2) is called or the user program exits, the
> memory mapping and the corresponding VMA will remain in existence.  
> (The man page for munmap specifically says "closing the file descriptor
> does not unmap the region".)  Thus when the user program does do an
> munmap(), the callback to vma->vm_ops->close will reference nonexistent
> memory and cause an oops.
> 
> Normally this sort of thing is prevented by try_module_get(...->owner).  
> But vm_operations_struct doesn't include a .owner field.
> 
> Am I missing something here?

mmap(2) takes reference of the file, therefore the file is not closed from
kernel POV until vma is gone and you cannot unload relevant module.
See get_file() in mmap_region().

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Julia Lawall



On Tue, 5 Jan 2016, Felipe Ferreri Tonello wrote:

> Hi Dan,
> 
> On 05/01/16 12:44, Dan Carpenter wrote:
> > On Tue, Jan 05, 2016 at 01:28:11PM +0100, Julia Lawall wrote:
> >>
> >>
> >> On Tue, 5 Jan 2016, kbuild test robot wrote:
> >>
> >>> Hi Dan,
> >>>
> >>> [auto build test WARNING on balbi-usb/next]
> >>> [also build test WARNING on v4.4-rc8 next-20160105]
> >>> [if your patch is applied to the wrong git tree, please drop us a note to 
> >>> help improving the system]
> >>>
> >>> url:
> >>> https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
> >>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> >>>
> >>>
> >>> coccinelle warnings: (new ones prefixed by >>)
> >>>
> >>>>> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL 
> >>>>> but dereferenced.
> >>
> >> It's a false positive for coccinelle, but I wonder if avoiding duplicating
> >> the mutex_lock is really worth it?
> > 
> > It's not the most beautiful code in the world.  I considered a bunch of
> > different ways to write it...  This is what Felipe Tonello wanted,
> > though.
> 
> This case is not a matter of been pretty but a matter of been less error
> prone.
> 
> What would you suggest?

I thought to be a little less subtle about midi, it would be easier to 
keep the first few mutex_unlocks up where they were.  But up to you.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Does vm_operations_struct require a .owner field?

On Tue, 5 Jan 2016, Kirill A. Shutemov wrote:

> On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> > Hello:
> > 
> > Question: The vm_operations_struct structure contains lots of callback
> > pointers.  Is there any mechanism to prevent the callback routines and
> > the structure itself being unloaded from memory (if they are built into
> > modules) while the relevant VMAs are still in use?
> > 
> > Consider a simple example: A user program calls mmap(2) on a device
> > file.  Later on, the file is closed and the device driver's module is
> > unloaded.  But until munmap(2) is called or the user program exits, the
> > memory mapping and the corresponding VMA will remain in existence.  
> > (The man page for munmap specifically says "closing the file descriptor
> > does not unmap the region".)  Thus when the user program does do an
> > munmap(), the callback to vma->vm_ops->close will reference nonexistent
> > memory and cause an oops.
> > 
> > Normally this sort of thing is prevented by try_module_get(...->owner).  
> > But vm_operations_struct doesn't include a .owner field.
> > 
> > Am I missing something here?
> 
> mmap(2) takes reference of the file, therefore the file is not closed from
> kernel POV until vma is gone and you cannot unload relevant module.
> See get_file() in mmap_region().

Thank you.  So it looks like I was worried about nothing.

Steinar, you can remove the try_module_get/module_put lines from your
patch.  Also, the list_del() and comment in usbdev_release() aren't 
needed -- at that point we know the memory_list has to be empty since 
there can't be any outstanding URBs or VMA references.  If you take 
those things out then the patch should be ready for merging.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

On Tue, Jan 05, 2016 at 08:51:18PM +, Felipe Ferreri Tonello wrote:
> This case is not a matter of been pretty but a matter of been less error
> prone.
> 
> What would you suggest?

Normally it's better to unwind in the reverse order from how we
allocated so it would be:

lock
allocate midi
allocate ports

free ports
free midi
unlock

We could move the midi allocation outside the lock, but we can't move
ports allocation.  And also we want to drop the lock as soon as we can
so it's better to do that early like my patch does instead of after the
frees.  It's less symetric that way and thus more error prone but it's
better for performance.

Anyway, I don't think it really matters, this is a minor thing.

Also I hope that Smatch will be able to avoid that false positive about
the midi dereference by the end of 2016. :)

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 0/4] Allow USB devices to remain runtime-suspended when sleeping

2016-01-05 Thread dbasehore .

On Tue, Jan 5, 2016 at 4:48 AM, Rafael J. Wysocki  wrote:
> On Monday, January 04, 2016 06:27:18 PM Derek Basehore wrote:
>> On Mon, Nov 02, 2015 at 02:50:40AM +0100, Rafael J. Wysocki wrote:
>> >
>> > I've queued up this series for the second half of the v4.4 merge window.
>> >
>> > Thanks,
>> > Rafael
>> >
>> >
>> > ___
>> > linux-arm-kernel mailing list
>> > linux-arm-ker...@lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>> What's the status of this patch series? I haven't seen the patches
>> posted for v4.4, plus there's the issue that Dan found that needs to be
>> addressed.
>>
>> Is there a new revision of the patch series being worked on?
>
> Tomeu is not working on one AFAICS, but I may just revive his series.
>
> Do you have a pointer to the Dan's report?
>
> Thanks,
> Rafael
>

It was a reply to the second patch in the series. Here's a link to it
https://lkml.org/lkml/2015/11/10/107
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Add support for usbfs zerocopy.

Add a new interface for userspace to preallocate memory that can be
used with usbfs. This gives two primary benefits:

 - Zerocopy; data no longer needs to be copied between the userspace
   and the kernel, but can instead be read directly by the driver from
   userspace's buffers. This works for all kinds of transfers (even if
   nonsensical for control and interrupt transfers); isochronous also
   no longer need to memset() the buffer to zero to avoid leaking kernel data.

 - Once the buffers are allocated, USB transfers can no longer fail due to
   memory fragmentation; previously, long-running programs could run into
   problems finding a large enough contiguous memory chunk, especially on
   embedded systems or at high rates.

Memory is allocated by using mmap() against the usbfs file descriptor,
and similarly deallocated by munmap(). Once memory has been allocated,
using it as pointers to a bulk or isochronous operation means you will
automatically get zerocopy behavior. Note that this also means you cannot
modify outgoing data until the transfer is complete. The same holds for
data on the same cache lines as incoming data; DMA modifying them at the
same time could lead to your changes being overwritten.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

Largely based on a patch by Markus Rechberger with some updates. The original
patch can be found at:

  http://sundtek.de/support/devio_mmap_v0.4.diff

Signed-off-by: Steinar H. Gunderson 
Signed-off-by: Markus Rechberger 
Acked-by: Alan Stern 
---
 drivers/usb/core/devio.c  | 227 +-
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 203 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..0238c78 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -69,6 +70,7 @@ struct usb_dev_state {
spinlock_t lock;/* protects the async urb lists */
struct list_head async_pending;
struct list_head async_completed;
+   struct list_head memory_list;
wait_queue_head_t wait; /* wake up if a request completed */
unsigned int discsignr;
struct pid *disc_pid;
@@ -79,6 +81,17 @@ struct usb_dev_state {
u32 disabled_bulk_eps;
 };
 
+struct usb_memory {
+   struct list_head memlist;
+   int vma_use_count;
+   int urb_use_count;
+   u32 size;
+   void *mem;
+   dma_addr_t dma_handle;
+   unsigned long vm_start;
+   struct usb_dev_state *ps;
+};
+
 struct async {
struct list_head asynclist;
struct usb_dev_state *ps;
@@ -89,6 +102,7 @@ struct async {
void __user *userbuffer;
void __user *userurb;
struct urb *urb;
+   struct usb_memory *usbm;
unsigned int mem_usage;
int status;
u32 secid;
@@ -157,6 +171,111 @@ static int connected(struct usb_dev_state *ps)
ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
+{
+   struct usb_dev_state *ps = usbm->ps;
+   unsigned long flags;
+
+   spin_lock_irqsave(&ps->lock, flags);
+   --*count;
+   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
+   list_del(&usbm->memlist);
+   spin_unlock_irqrestore(&ps->lock, flags);
+
+   usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+   usbm->dma_handle);
+   usbfs_decrease_memory_usage(
+   usbm->size + sizeof(struct usb_memory));
+   kfree(usbm);
+   } else {
+   spin_unlock_irqrestore(&ps->lock, flags);
+   }
+}
+
+static void usbdev_vm_open(struct vm_area_struct *vma)
+{
+   struct usb_memory *usbm = vma->vm_private_data;
+   unsigned long flags;
+
+   spin_lock_irqsave(&usbm->ps->lock, flags);
+   ++usbm->vma_use_count;
+   spin_unlock_irqrestore(&usbm->ps->lock, flags);
+}
+
+static void usbdev_vm_close(struct vm_area_struct *vma)
+{
+   struct usb_memory *usbm = vma->vm_private_data;
+
+   dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
+}
+
+struct vm_operations_struct usbdev_vm_ops = {
+   .open = usbdev_vm_open,
+   .close = usbdev_vm_close
+};
+
+static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
+{
+   struct usb_memory *usbm = NULL;
+   struct usb_dev_state *ps = file->private_data;
+   size_t size = vma->vm_end - vma->vm_start;
+   void *mem;
+   unsigned long flags;
+   dma_addr_t dma_handle;
+   int ret;
+
+   ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
+   if (ret)
+   goto error;
+
+   usbm = k

Re: Does vm_operations_struct require a .owner field?

On Tue, Jan 05, 2016 at 04:31:09PM -0500, Alan Stern wrote:
> Thank you.  So it looks like I was worried about nothing.
> 
> Steinar, you can remove the try_module_get/module_put lines from your
> patch.  Also, the list_del() and comment in usbdev_release() aren't 
> needed -- at that point we know the memory_list has to be empty since 
> there can't be any outstanding URBs or VMA references.  If you take 
> those things out then the patch should be ready for merging.

Good, thanks. Did so, compiled, testing it still works, sending :-)

/* Steinar */
-- 
Software Engineer, Google Switzerland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Michal Nazarewicz

On Wed, Jan 06 2016, Dan Carpenter wrote:
> On Tue, Jan 05, 2016 at 08:51:18PM +, Felipe Ferreri Tonello wrote:
>> This case is not a matter of been pretty but a matter of been less error
>> prone.
>> 
>> What would you suggest?
>
> Normally it's better to unwind in the reverse order from how we
> allocated so it would be:
>
>   lock
>   allocate midi
>   allocate ports
>
>   free ports
>   free midi
>   unlock
>
> We could move the midi allocation outside the lock, but we can't move
> ports allocation.

I still think the easiest way to go is getting rid of ports allocation
all together.  With that gone, it’s trivial to initialise midi to NULL
and then we can do kfree whenever we want.

> And also we want to drop the lock as soon as we can
> so it's better to do that early like my patch does instead of after the
> frees.  It's less symetric that way and thus more error prone but it's
> better for performance.
>
> Anyway, I don't think it really matters, this is a minor thing.
>
> Also I hope that Smatch will be able to avoid that false positive about
> the midi dereference by the end of 2016. :)

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  ミハウ “mina86” ナザレヴイツ  (o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add support for usbfs zerocopy.

2016-01-05 Thread Greg Kroah-Hartman

On Thu, Nov 26, 2015 at 01:19:13AM +0100, Steinar H. Gunderson wrote:
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:

Please 'version' your patches, so that I have a chance to figure out
what patch is what, and what changed between patches.

otherwise the odds of me picking the "wrong" one is _very_ high...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add support for usbfs zerocopy.

On Tue, Jan 05, 2016 at 04:11:43PM -0800, Greg Kroah-Hartman wrote:
>> Add a new interface for userspace to preallocate memory that can be
>> used with usbfs. This gives two primary benefits:
> Please 'version' your patches, so that I have a chance to figure out
> what patch is what, and what changed between patches.
> 
> otherwise the odds of me picking the "wrong" one is _very_ high...

OK. I won't make any attempt at reconstructing the history, but I'll resend
the one I just sent you as v2, ie. --reroll-count=2.

Somehow it feels like there should be a way to integrate this better into my
MUA, but hopefully this is soon all done anyway. :-)

/* Steinar */
-- 
Software Engineer, Google Switzerland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Add support for usbfs zerocopy.