possible circular locking dependency detected in musb_dsps in gadget mode

[Tal Shorer]

when unregistering the configfs driver on my beaglebone black (running
linux-4.12-rc6), this happens:
root@beaglebone:/sys/kernel/config/usb_gadget/g_ecm|0 # echo > UDC
[   39.074784]
[   39.076361] ==
[   39.082815] WARNING: possible circular locking dependency detected
[   39.089272] 4.12.0-rc6 #3 Not tainted
[   39.093093] --
[   39.099547] sh/177 is trying to acquire lock:
[   39.104096]  (((>timer))){+.-...}, at: []
del_timer_sync+0x0/0xd0
[   39.111951]
[   39.111951] but task is already holding lock:
[   39.118040]  (&(>lock)->rlock){-.-...}, at: []
musb_gadget_stop+0x24/0x108 [musb_hdrc]
[   39.127805]
[   39.127805] which lock already depends on the new lock.
[   39.127805]
[   39.136346]
[   39.136346] the existing dependency chain (in reverse order) is:
[   39.144159]
[   39.144159] -> #1 (&(>lock)->rlock){-.-...}:
[   39.150726]otg_timer+0x80/0x194 [musb_dsps]
[   39.155813]call_timer_fn+0xb4/0x388
[   39.160182]expire_timers+0xe4/0x1f0
[   39.164545]run_timer_softirq+0x94/0x19c
[   39.169274]__do_softirq+0x140/0x510
[   39.173639]irq_exit+0xe4/0x160
[   39.177557]__handle_domain_irq+0x6c/0xe0
[   39.182380]__irq_svc+0x70/0x98
[   39.186293]__und_usr+0x58/0x78
[   39.190203]
[   39.190203] -> #0 (((>timer))){+.-...}:
[   39.196306]del_timer_sync+0x40/0xd0
[   39.200673]musb_stop+0x1c/0x60 [musb_hdrc]
[   39.205680]musb_gadget_stop+0x68/0x108 [musb_hdrc]
[   39.211414]usb_add_gadget_udc+0x70/0x90 [udc_core]
[   39.217134]usb_gadget_unregister_driver+0x64/0xd0 [udc_core]
[   39.223786]gadgets_make+0x278/0x29c [libcomposite]
[   39.229513]gadget_dev_desc_UDC_store+0x84/0xd0 [libcomposite]
[   39.236248]configfs_write_file+0xf4/0x188
[   39.241158]__vfs_write+0x1c/0x114
[   39.245340]vfs_write+0xa0/0x168
[   39.249345]SyS_write+0x3c/0x90
[   39.253264]ret_fast_syscall+0x0/0x1c
[   39.257718]
[   39.257718] other info that might help us debug this:
[   39.257718]
[   39.266063]  Possible unsafe locking scenario:
[   39.266063]
[   39.272244]CPU0CPU1
[   39.276963]
[   39.281684]   lock(&(>lock)->rlock);
[   39.285957]lock(((>timer)));
[   39.292413]lock(&(>lock)->rlock);
[   39.299319]   lock(((>timer)));
[   39.303144]
[   39.303144]  *** DEADLOCK ***
[   39.303144]
[   39.309331] 5 locks held by sh/177:
[   39.312972]  #0:  (sb_writers#7){.+.+.+}, at: []
vfs_write+0x148/0x168
[   39.320709]  #1:  (>mutex){+.+.+.}, at: []
configfs_write_file+0x2c/0x188
[   39.329434]  #2:  (>lock){+.+.+.}, at: []
gadget_dev_desc_UDC_store+0x48/0xd0 [libcomposite]
[   39.339538]  #3:  (udc_lock){+.+.+.}, at: []
usb_gadget_unregister_driver+0x28/0xd0 [udc_core]
[   39.349443]  #4:  (&(>lock)->rlock){-.-...}, at: []
musb_gadget_stop+0x24/0x108 [musb_hdrc]
[   39.359643]
[   39.359643] stack backtrace:
[   39.364201] CPU: 0 PID: 177 Comm: sh Not tainted 4.12.0-rc6 #3
[   39.370286] Hardware name: Generic AM33XX (Flattened Device Tree)
[   39.376663] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[   39.384761] [] (show_stack) from []
(dump_stack+0xac/0xe0)
[   39.392318] [] (dump_stack) from []
(print_circular_bug+0x1d0/0x308)
[   39.400763] [] (print_circular_bug) from []
(__lock_acquire+0x15b0/0x18c8)
[   39.409762] [] (__lock_acquire) from []
(lock_acquire+0xcc/0x238)
[   39.417944] [] (lock_acquire) from []
(del_timer_sync+0x40/0xd0)
[   39.426048] [] (del_timer_sync) from []
(musb_stop+0x1c/0x60 [musb_hdrc])
[   39.434955] [] (musb_stop [musb_hdrc]) from []
(musb_gadget_stop+0x68/0x108 [musb_hdrc])
[   39.445226] [] (musb_gadget_stop [musb_hdrc]) from
[] (usb_add_gadget_udc+0x70/0x90 [udc_core])
[   39.456131] [] (usb_add_gadget_udc [udc_core]) from
[] (usb_gadget_unregister_driver+0x64/0xd0 [udc_core])
[   39.468026] [] (usb_gadget_unregister_driver [udc_core])
from [] (gadgets_make+0x278/0x29c [libcomposite])
[   39.479930] [] (gadgets_make [libcomposite]) from
[] (gadget_dev_desc_UDC_store+0x84/0xd0 [libcomposite])
[   39.491755] [] (gadget_dev_desc_UDC_store [libcomposite])
from [] (configfs_write_file+0xf4/0x188)
[   39.502920] [] (configfs_write_file) from []
(__vfs_write+0x1c/0x114)
[   39.511455] [] (__vfs_write) from []
(vfs_write+0xa0/0x168)
[   39.519080] [] (vfs_write) from [] (SyS_write+0x3c/0x90)
[   39.526430] [] (SyS_write) from []
(ret_fast_syscall+0x0/0x1c)
root@beaglebone:/sys/kernel/config/usb_gadget/g_ecm|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

Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

On Wed, Jun 14, 2017 at 4:33 PM, Alan Cox  wrote:
>> That would cut it, but TIOCPKT is too coupled with having a linked tty.
>> I could make acm behave like a pty (accept TIOCPKT and issue the
>> ctrl_status bits), but for that I need n_tty to know that packet does
>> not always mean a linked tty is present, and that in case it isn't we
>> take our own ctrl_status bits instead of the link's. I could write a
>> small (inline?) function to fetch the correct ctrl_status bits and put
>> that in n_tty. Does that make sense?
>
> I think that makes sense, and I would do the job properly rather than do
> a hack with tty->link. Those hacks in the long term never work out the
> best approach.
>
> Alan
Ok, so I'm doing that and everything is great until I got to actually
modifying tty->termios.
I need to modify it from interrupt context (the usb_request's
complete() callback), but modifying the termios requires a
rw_semaphore I can't take.
I could queue_work() to do it, but then I'd have to flush the work
from another non-sleepable context in acm_disconnect() (which runs
under a spinlock).
I can't change the semaphore to a spinlock because some drivers that
use it actually wanna sleep while holding it.
Any ideas?
--
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/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

On Wed, Jun 14, 2017 at 11:20 AM, Tal Shorer <tal.sho...@gmail.com> wrote:
> On Wed, Jun 14, 2017 at 4:15 AM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote:
>> On Tue, 13 Jun 2017 09:52:07 +0300
>> Tal Shorer <tal.sho...@gmail.com> wrote:
>>
>>> If a tty driver wants to notify the user of some exceptional event,
>>> such as a usb cdc acm device set_line_coding event, it needs a way to
>>> modify the mask returned by poll() and possible also add wait queues.
>>> In order to do that, we allow the driver to supply a poll() callback
>>> of its own, which will be called in n_tty_poll().
>>>
>>> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
>>
>> You might be in any line discipline so you need to support that for each
>> ldisc that supports poll(). Also what do I do when I get an exception
>> event - what does it mean, how do I understand that, are you proposing a
>> standardized meaning ? Have you checked whether that conflicts with SuS
>> v3 or POSIX ? What will it do with existing code.
>>
>> At the very least it probably has to be something you turn on, and you
>> might then want to model it on the pty/tty interface logic and extend
>> TIOCPKT a shade.
> That would cut it, but TIOCPKT is too coupled with having a linked tty.
> I could make acm behave like a pty (accept TIOCPKT and issue the
> ctrl_status bits), but for that I need n_tty to know that packet does
> not always mean a linked tty is present, and that in case it isn't we
> take our own ctrl_status bits instead of the link's. I could write a
> small (inline?) function to fetch the correct ctrl_status bits and put
> that in n_tty. Does that make sense?
Or (maybe) better yet, I can do a hack and have the acm tty point to
itself as the link, which means n_tty doesn't have to change at all.
Any thoughts?
--
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/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

On Wed, Jun 14, 2017 at 4:15 AM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote:
> On Tue, 13 Jun 2017 09:52:07 +0300
> Tal Shorer <tal.sho...@gmail.com> wrote:
>
>> If a tty driver wants to notify the user of some exceptional event,
>> such as a usb cdc acm device set_line_coding event, it needs a way to
>> modify the mask returned by poll() and possible also add wait queues.
>> In order to do that, we allow the driver to supply a poll() callback
>> of its own, which will be called in n_tty_poll().
>>
>> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
>
> You might be in any line discipline so you need to support that for each
> ldisc that supports poll(). Also what do I do when I get an exception
> event - what does it mean, how do I understand that, are you proposing a
> standardized meaning ? Have you checked whether that conflicts with SuS
> v3 or POSIX ? What will it do with existing code.
>
> At the very least it probably has to be something you turn on, and you
> might then want to model it on the pty/tty interface logic and extend
> TIOCPKT a shade.
That would cut it, but TIOCPKT is too coupled with having a linked tty.
I could make acm behave like a pty (accept TIOCPKT and issue the
ctrl_status bits), but for that I need n_tty to know that packet does
not always mean a linked tty is present, and that in case it isn't we
take our own ctrl_status bits instead of the link's. I could write a
small (inline?) function to fetch the correct ctrl_status bits and put
that in n_tty. Does that make sense?
--
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: gadgetfs: how to wait for USB device initialization?

[Tal Shorer]

On Tue, Jun 13, 2017 at 7:02 PM, Tal Shorer <tal.sho...@gmail.com> wrote:
> On Tue, Jun 13, 2017 at 3:21 PM, Andrey Konovalov <andreyk...@google.com> 
> wrote:
>> Hi!
>>
>> I'm trying to use gadgetfs to fuzz USB device drivers by simply
>> connecting random devices for now.
>>
>> What I want to achieve right now is the following:
>>
>> 1. mount gadgetfs
>> 2. emulate connection of a new USB device
>> 3. wait for the device to finish initializing
>> 4. unmount gadgetfs
>> 5. goto 1
>>
>> The question is how do I wait for the device to finish initializing
>> (the corresponding USB driver to finish probing?) before unmounting
>> gadgetfs? As I understand, when I write device description to
>> "/dev/gadget/dummy_udc" the initialization happens asynchronously
>> after writing is done.
>>
>
> You can use inotify on the state file inside a udc's sysfs directory.
> It's awesome like that.
> Here's a sample code that doesn't do any cleanups because I'm lazy:
> https://gist.github.com/talshorer/61be1bc3472cc2a7b4866b9bd5a239ca
And I forgot usage >:
./wait_udc_configured /sys/devices/platform/dummy_udc.0/udc/dummy_udc.0/state
--
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: gadgetfs: how to wait for USB device initialization?

[Tal Shorer]

On Tue, Jun 13, 2017 at 3:21 PM, Andrey Konovalov  wrote:
> Hi!
>
> I'm trying to use gadgetfs to fuzz USB device drivers by simply
> connecting random devices for now.
>
> What I want to achieve right now is the following:
>
> 1. mount gadgetfs
> 2. emulate connection of a new USB device
> 3. wait for the device to finish initializing
> 4. unmount gadgetfs
> 5. goto 1
>
> The question is how do I wait for the device to finish initializing
> (the corresponding USB driver to finish probing?) before unmounting
> gadgetfs? As I understand, when I write device description to
> "/dev/gadget/dummy_udc" the initialization happens asynchronously
> after writing is done.
>

You can use inotify on the state file inside a udc's sysfs directory.
It's awesome like that.
Here's a sample code that doesn't do any cleanups because I'm lazy:
https://gist.github.com/talshorer/61be1bc3472cc2a7b4866b9bd5a239ca
--
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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Tue, Jun 13, 2017 at 12:19 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer wrote:
>> The user can issue USB_F_GET_LINE_CODING to get the current line coding
>> as set by the host (or the default if unset yet).
>>
>> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
>> ---
>>  Documentation/ioctl/ioctl-number.txt |  1 +
>>  drivers/usb/gadget/function/f_acm.c  | 19 +++
>>  include/uapi/linux/usb/f_acm.h   | 12 
>
> Where is this ioctl being called?  On the tty device?  If so, which one?
> The gadget driver's tty device node?  Or somewhere else?
On an acm ttyGS* fd, yes.
>
> confused at the different levels here, sorry.
>
> 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

[PATCH v2 2/8] usb: gadget: u_serial: propagate poll() to the next layer

[Tal Shorer]

In order for a serial function to add flags to the poll() mask of their
tty files, propagate the poll() callback to the next layer so it can
return a mask if it sees fit to do so.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 16 
 drivers/usb/gadget/function/u_serial.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 9b0805f..d466f58 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1025,6 +1025,21 @@ static int gs_break_ctl(struct tty_struct *tty, int 
duration)
return status;
 }
 
+static unsigned int gs_poll(struct tty_struct *tty, struct file *file,
+   poll_table *wait)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   unsigned int mask = 0;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->poll)
+   mask |= gser->poll(gser, file, wait);
+   spin_unlock_irq(>port_lock);
+   return mask;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1035,6 +1050,7 @@ static const struct tty_operations gs_tty_ops = {
.chars_in_buffer =  gs_chars_in_buffer,
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
+   .poll = gs_poll,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index c20210c..ce00840 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -12,6 +12,7 @@
 #ifndef __U_SERIAL_H
 #define __U_SERIAL_H
 
+#include 
 #include 
 #include 
 
@@ -50,6 +51,8 @@ struct gserial {
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
int (*send_break)(struct gserial *p, int duration);
+   unsigned int (*poll)(struct gserial *p, struct file *file,
+   poll_table *wait);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.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 3/8] usb: gadget: f_acm: validate set_line_coding requests

[Tal Shorer]

We shouldn't accept malformed set_line_coding requests.
All values were taken from table 17 (section 6.3.11) of the cdc1.2 spec
available at http://www.usb.org/developers/docs/devclass_docs/
The table is in the file PTSN120.pdf.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5e3828d..e023313 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -326,13 +326,22 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
struct usb_cdc_line_coding  *value = req->buf;
 
/* REVISIT:  we currently just remember this data.
-* If we change that, (a) validate it first, then
-* (b) update whatever hardware needs updating,
-* (c) worry about locking.  This is information on
-* the order of 9600-8-N-1 ... most of which means
-* nothing unless we control a real RS232 line.
-*/
-   acm->port_line_coding = *value;
+   * If we change that,
+   * (a) update whatever hardware needs updating,
+   * (b) worry about locking.  This is information on
+   * the order of 9600-8-N-1 ... most of which means
+   * nothing unless we control a real RS232 line.
+   */
+   dev_dbg(>gadget->dev,
+   "acm ttyGS%d set_line_coding: %d %d %d %d\n",
+   acm->port_num, le32_to_cpu(value->dwDTERate),
+   value->bCharFormat, value->bParityType,
+   value->bDataBits);
+   if (value->bCharFormat > 2 || value->bParityType > 4 ||
+   value->bDataBits < 5 || value->bDataBits > 8)
+   usb_ep_set_halt(ep);
+   else
+   acm->port_line_coding = *value;
}
 }
 
-- 
2.7.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 1/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

If a tty driver wants to notify the user of some exceptional event,
such as a usb cdc acm device set_line_coding event, it needs a way to
modify the mask returned by poll() and possible also add wait queues.
In order to do that, we allow the driver to supply a poll() callback
of its own, which will be called in n_tty_poll().

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/tty/n_tty.c| 2 ++
 include/linux/tty_driver.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..7af8c29 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2394,6 +2394,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, 
struct file *file,
tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
tty_write_room(tty) > 0)
mask |= POLLOUT | POLLWRNORM;
+   if (tty->ops->poll)
+   mask |= tty->ops->poll(tty, file, wait);
return mask;
 }
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index b742b5e..630ef03 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -243,6 +243,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct tty_struct;
 struct tty_driver;
@@ -285,6 +286,8 @@ struct tty_operations {
int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
int (*get_icount)(struct tty_struct *tty,
struct serial_icounter_struct *icount);
+   unsigned int (*poll)(struct tty_struct *tty, struct file *file,
+   poll_table *wait);
 #ifdef CONFIG_CONSOLE_POLL
int (*poll_init)(struct tty_driver *driver, int line, char *options);
int (*poll_get_char)(struct tty_driver *driver, int line);
-- 
2.7.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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 19 +++
 include/uapi/linux/usb/f_acm.h   | 12 
 3 files changed, 32 insertions(+)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex) Include FileComments
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
 0xCB   00-1F   CBM serial IEC bus  in development:

<mailto:michael.kl...@puffin.lb.shuttle.de>
+0xCD   10-1F   linux/usb/f_acm.h
 0xCD   01  linux/reiserfs_fs.h
 0xCF   02  fs/cifs/ioctl.c
 0xDB   00-0F   drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 188d314..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "u_serial.h"
 
@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int 
duration)
return acm_notify_serial_state(acm);
 }
 
+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+   struct f_acm*acm = port_to_acm(port);
+   int ret = -ENOIOCTLCMD;
+
+   switch (cmd) {
+   case USB_F_ACM_GET_LINE_CODING:
+   if (copy_to_user((__user void *)arg, >port_line_coding,
+   sizeof(acm->port_line_coding)))
+   ret = -EFAULT;
+   else
+   ret = 0;
+   break;
+   }
+   return ret;
+}
+
 /*-*/
 
 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
+   acm->port.ioctl = acm_ioctl;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include 
+#include 
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
-- 
2.7.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 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial

[Tal Shorer]

GetLineCoding and SetLineCoding are a cdc-acm thing. It doesn't make
sense to have that in the generic u_serial layer. Moreso, f_acm has its
own port_line_coding in its own struct and it uses that, while the one
in struct gserial is set once upon initialization and then never used.
Also, the initialized never-used values were invalid, with bDataBits
and bCharFormat having each other's value.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 22 ++
 drivers/usb/gadget/function/u_serial.h |  3 ---
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 8d9abf1..654d4a6 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -129,9 +129,6 @@ struct gs_port {
wait_queue_head_t   drain_wait; /* wait while writes drain */
boolwrite_busy;
wait_queue_head_t   close_wait;
-
-   /* REVISIT this state ... */
-   struct usb_cdc_line_coding port_line_coding;/* 8-N-1 etc */
 };
 
 static struct portmaster {
@@ -1314,7 +1311,7 @@ static void gserial_console_exit(void)
 #endif
 
 static int
-gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
+gs_port_alloc(unsigned port_num)
 {
struct gs_port  *port;
int ret = 0;
@@ -1343,7 +1340,6 @@ gs_port_alloc(unsigned port_num, struct 
usb_cdc_line_coding *coding)
INIT_LIST_HEAD(>write_pool);
 
port->port_num = port_num;
-   port->port_line_coding = *coding;
 
ports[port_num].port = port;
 out:
@@ -1392,18 +1388,12 @@ EXPORT_SYMBOL_GPL(gserial_free_line);
 
 int gserial_alloc_line(unsigned char *line_num)
 {
-   struct usb_cdc_line_coding  coding;
struct device   *tty_dev;
int ret;
int port_num;
 
-   coding.dwDTERate = cpu_to_le32(9600);
-   coding.bCharFormat = 8;
-   coding.bParityType = USB_CDC_NO_PARITY;
-   coding.bDataBits = USB_CDC_1_STOP_BITS;
-
for (port_num = 0; port_num < MAX_U_SERIAL_PORTS; port_num++) {
-   ret = gs_port_alloc(port_num, );
+   ret = gs_port_alloc(port_num);
if (ret == -EBUSY)
continue;
if (ret)
@@ -1491,11 +1481,6 @@ int gserial_connect(struct gserial *gser, u8 port_num)
gser->ioport = port;
port->port_usb = gser;
 
-   /* REVISIT unclear how best to handle this state...
-* we don't really couple it with the Linux TTY.
-*/
-   gser->port_line_coding = port->port_line_coding;
-
/* REVISIT if waiting on "carrier detect", signal. */
 
/* if it's already open, start I/O ... and notify the serial
@@ -1543,9 +1528,6 @@ void gserial_disconnect(struct gserial *gser)
/* tell the TTY glue not to do I/O here any more */
spin_lock_irqsave(>port_lock, flags);
 
-   /* REVISIT as above: how best to track this? */
-   port->port_line_coding = gser->port_line_coding;
-
port->port_usb = NULL;
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index 8d0901e..0549efe 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -44,9 +44,6 @@ struct gserial {
struct usb_ep   *in;
struct usb_ep   *out;
 
-   /* REVISIT avoid this CDC-ACM support harder ... */
-   struct usb_cdc_line_coding port_line_coding;/* 9600-8-N-1 etc */
-
/* notification callbacks */
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
-- 
2.7.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 5/8] usb: gadget: f_acm: initialize port_line_coding when creating an instance

[Tal Shorer]

Initialize acm->port_line_coding with something that makes sense so
that we can return a valid line coding if the host requests
GetLineCoding before requesting SetLineCoding

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index e023313..188d314 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.func.unbind = acm_unbind;
acm->port.func.free_func = acm_free_func;
 
+   /* initialize port_line_coding with something that makes sense */
+   acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
+   acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
+   acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
+   acm->port_line_coding.bDataBits = 8;
+
return >port.func;
 }
 
-- 
2.7.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 4/8] usb: gadget: u_serial: propagate ioctl() to the next layer

[Tal Shorer]

In order for a serial function to implement its own ioctl() calls,
propagate the ioctl() callback to the next layer so it can handle it if
it sees fit to do so.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 15 +++
 drivers/usb/gadget/function/u_serial.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index d466f58..8d9abf1 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1040,6 +1040,20 @@ static unsigned int gs_poll(struct tty_struct *tty, 
struct file *file,
return mask;
 }
 
+static int gs_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long 
arg)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   int ret = -ENOIOCTLCMD;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->ioctl)
+   ret = gser->ioctl(gser, cmd, arg);
+   spin_unlock_irq(>port_lock);
+   return ret;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1051,6 +1065,7 @@ static const struct tty_operations gs_tty_ops = {
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
.poll = gs_poll,
+   .ioctl =gs_ioctl,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index ce00840..8d0901e 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -53,6 +53,7 @@ struct gserial {
int (*send_break)(struct gserial *p, int duration);
unsigned int (*poll)(struct gserial *p, struct file *file,
poll_table *wait);
+   int (*ioctl)(struct gserial *p, unsigned int cmd, unsigned long arg);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.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 7/8] usb: gadget: f_acm: notify the user on SetLineCoding

[Tal Shorer]

Notify the user with a POLLPRI event when the host issues a
SetLineCoding request so that they can act upon it, for example by
configuring the line coding on a real serial port.

The event is cleared when the user reads the line coding using
USB_F_ACM_GET_LINE_CODING ioctl()

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 38 ++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5feea7c..0983999 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -58,6 +58,9 @@ struct f_acm {
struct usb_request  *notify_req;
 
struct usb_cdc_line_coding  port_line_coding;   /* 8-N-1 etc */
+   /* we have a SetLineCoding request that the user haven't read yet */
+   bool set_line_coding_pending;
+   wait_queue_head_t set_line_coding_waitq;
 
/* SetControlLineState request -- CDC 1.1 section 6.2.14 (INPUT) */
u16 port_handshake_bits;
@@ -326,23 +329,19 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
} else {
struct usb_cdc_line_coding  *value = req->buf;
 
-   /* REVISIT:  we currently just remember this data.
-   * If we change that,
-   * (a) update whatever hardware needs updating,
-   * (b) worry about locking.  This is information on
-   * the order of 9600-8-N-1 ... most of which means
-   * nothing unless we control a real RS232 line.
-   */
dev_dbg(>gadget->dev,
"acm ttyGS%d set_line_coding: %d %d %d %d\n",
acm->port_num, le32_to_cpu(value->dwDTERate),
value->bCharFormat, value->bParityType,
value->bDataBits);
if (value->bCharFormat > 2 || value->bParityType > 4 ||
-   value->bDataBits < 5 || value->bDataBits > 8)
+   value->bDataBits < 5 || value->bDataBits > 8) {
usb_ep_set_halt(ep);
-   else
+   } else {
acm->port_line_coding = *value;
+   acm->set_line_coding_pending = true;
+   wake_up_interruptible(>set_line_coding_waitq);
+   }
}
 }
 
@@ -598,6 +597,19 @@ static void acm_disconnect(struct gserial *port)
acm_notify_serial_state(acm);
 }
 
+static unsigned int acm_poll(struct gserial *port, struct file *file,
+   poll_table *wait)
+{
+   unsigned int mask = 0;
+   struct f_acm *acm = port_to_acm(port);
+
+   poll_wait(file, >set_line_coding_waitq, wait);
+   if (acm->set_line_coding_pending)
+   mask |= POLLPRI;
+   return mask;
+}
+
+
 static int acm_send_break(struct gserial *port, int duration)
 {
struct f_acm*acm = port_to_acm(port);
@@ -620,10 +632,12 @@ static int acm_ioctl(struct gserial *port, unsigned int 
cmd, unsigned long arg)
switch (cmd) {
case USB_F_ACM_GET_LINE_CODING:
if (copy_to_user((__user void *)arg, >port_line_coding,
-   sizeof(acm->port_line_coding)))
+   sizeof(acm->port_line_coding))) {
ret = -EFAULT;
-   else
+   } else {
ret = 0;
+   acm->set_line_coding_pending = false;
+   }
break;
}
return ret;
@@ -763,11 +777,13 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(>lock);
+   init_waitqueue_head(>set_line_coding_waitq);
 
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
acm->port.ioctl = acm_ioctl;
+   acm->port.poll = acm_poll;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
-- 
2.7.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 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests

[Tal Shorer]

I'm currently working on a project where I'd like to have an omap board
running linux be a usb-to-uart converter (using f_acm), and I've ran
into an issue: there's no way for the application to know if the host
has issued a SetLineCoding requests (after which parity/baudrate should
be changed to match the host's request).

This series adds the support necessary to achieve that:
- Allowing tty drivers to supply a poll() function to notify the user of
driver-specific events.
- Propagating poll() and ioctl() from u_serial to the next layer (f_acm)
in this case.
- Let the user read the current line coding set by the host (via an
ioctl() call).
- Notify the user when there's a pending SetLineCoding request they
haven't read yet

The last patch also removes up the port_line_config field from
struct gserial. It made no sense to have there (and had a REVISIT
comment at every turn), it was never used and it was initialized with
invalid values.

Changes from v1:
- patch 5 was messed up, which made patch 6 also messed up. fixed both
  of these.

Tal Shorer (8):
  tty: add a poll() callback in struct tty_operations
  usb: gadget: u_serial: propagate poll() to the next layer
  usb: gadget: f_acm: validate set_line_coding requests
  usb: gadget: u_serial: propagate ioctl() to the next layer
  usb: gadget: f_acm: initialize port_line_coding when creating an
instance
  usb: gadget: f_acm: add an ioctl to get the current line coding
  usb: gadget: f_acm: notify the user on SetLineCoding
  usb: gadget: u_serial: remove port_line_config from struct gserial

 Documentation/ioctl/ioctl-number.txt   |  1 +
 drivers/tty/n_tty.c|  2 ++
 drivers/usb/gadget/function/f_acm.c| 66 +-
 drivers/usb/gadget/function/u_serial.c | 53 ---
 drivers/usb/gadget/function/u_serial.h |  7 ++--
 include/linux/tty_driver.h |  3 ++
 include/uapi/linux/usb/f_acm.h | 12 +++
 7 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

--
2.7.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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Mon, Jun 12, 2017 at 9:02 PM, Tal Shorer <tal.sho...@gmail.com> wrote:
> On Mon, Jun 12, 2017 at 8:26 PM, Tal Shorer <tal.sho...@gmail.com> wrote:
>> The user can issue USB_F_GET_LINE_CODING to get the current line coding
>> as set by the host (or the default if unset yet).
>>
>> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
>> ---
>
>> @@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct 
>> usb_function_instance *fi)
>> acm->port.func.free_func = acm_free_func;
>>
>> /* initialize port_line_coding with something that makes sense */
>> -   coding.dwDTERate = cpu_to_le32(9600);
>> -   coding.bCharFormat = USB_CDC_1_STOP_BITS;
>> -   coding.bParityType = USB_CDC_NO_PARITY;
>> -   coding.bDataBits = 8;
>> +   acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
>> +   acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
>> +   acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
>> +   acm->port_line_coding.bDataBits = 8;
>>
>> return >port.func;
>>  }
> This hunk was messed up somehow and will not apply. I can resend a v2 if 
> necessary, but the correct patch is as follows:
Actually, it shouldn't even be in this patch. I somehow meshed 5 and 6 together.
The correct PATCH 6/8 is as follows:

>From d7d56ceb8020f10623a04b4634f93e4cc678454d Mon Sep 17 00:00:00 2001
From: Tal Shorer <tal.sho...@gmail.com>
Date: Mon, 12 Jun 2017 19:40:56 +0300
Subject: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line
 coding

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++
 include/uapi/linux/usb/f_acm.h   | 12 
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex) Include FileComments
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
 0xCB   00-1F   CBM serial IEC bus  in development:

<mailto:michael.kl...@puffin.lb.shuttle.de>
+0xCD   10-1F   linux/usb/f_acm.h
 0xCD   01  linux/reiserfs_fs.h
 0xCF   02  fs/cifs/ioctl.c
 0xDB   00-0F   drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "u_serial.h"

@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int 
duration)
return acm_notify_serial_state(acm);
 }

+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+   struct f_acm*acm = port_to_acm(port);
+   int ret = -ENOIOCTLCMD;
+
+   switch (cmd) {
+   case USB_F_ACM_GET_LINE_CODING:
+   if (copy_to_user((__user void *)arg, >port_line_coding,
+   sizeof(acm->port_line_coding)))
+   ret = -EFAULT;
+   else
+   ret = 0;
+   break;
+   }
+   return ret;
+}
+
 /*-*/

 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
+   acm->port.ioctl = acm_ioctl;

acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include 
+#include 
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
--
2.7.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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Mon, Jun 12, 2017 at 8:26 PM, Tal Shorer <tal.sho...@gmail.com> wrote:
> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).
>
> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
> ---

> @@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct 
> usb_function_instance *fi)
> acm->port.func.free_func = acm_free_func;
>
> /* initialize port_line_coding with something that makes sense */
> -   coding.dwDTERate = cpu_to_le32(9600);
> -   coding.bCharFormat = USB_CDC_1_STOP_BITS;
> -   coding.bParityType = USB_CDC_NO_PARITY;
> -   coding.bDataBits = 8;
> +   acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
> +   acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
> +   acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
> +   acm->port_line_coding.bDataBits = 8;
>
> return >port.func;
>  }
This hunk was messed up somehow and will not apply. I can resend a v2 if 
necessary, but the correct patch is as follows:

From: Tal Shorer <tal.sho...@gmail.com>
Subject: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line
 coding

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++
 include/uapi/linux/usb/f_acm.h   | 12 
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex) Include FileComments
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
 0xCB   00-1F   CBM serial IEC bus  in development:

<mailto:michael.kl...@puffin.lb.shuttle.de>
+0xCD   10-1F   linux/usb/f_acm.h
 0xCD   01  linux/reiserfs_fs.h
 0xCF   02  fs/cifs/ioctl.c
 0xDB   00-0F   drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "u_serial.h"

@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int 
duration)
return acm_notify_serial_state(acm);
 }

+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+   struct f_acm*acm = port_to_acm(port);
+   int ret = -ENOIOCTLCMD;
+
+   switch (cmd) {
+   case USB_F_ACM_GET_LINE_CODING:
+   if (copy_to_user((__user void *)arg, >port_line_coding,
+   sizeof(acm->port_line_coding)))
+   ret = -EFAULT;
+   else
+   ret = 0;
+   break;
+   }
+   return ret;
+}
+
 /*-*/

 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
+   acm->port.ioctl = acm_ioctl;

acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.func.unbind = acm_unbind;
acm->port.func.free_func = acm_free_func;

+   /* initialize port_line_coding with something that makes sense */
+   coding.dwDTERate = cpu_to_le32(9600);
+   coding.bCharFormat = USB_CDC_1_STOP_BITS;
+   coding.bParityType = USB_CDC_NO_PARITY;
+   coding.bDataBits = 8;
+
return >port.func;
 }

diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include 
+#include 
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
--
2.7.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 1/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

If a tty driver wants to notify the user of some exceptional event,
such as a usb cdc acm device set_line_coding event, it needs a way to
modify the mask returned by poll() and possible also add wait queues.
In order to do that, we allow the driver to supply a poll() callback
of its own, which will be called in n_tty_poll().

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/tty/n_tty.c| 2 ++
 include/linux/tty_driver.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..7af8c29 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2394,6 +2394,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, 
struct file *file,
tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
tty_write_room(tty) > 0)
mask |= POLLOUT | POLLWRNORM;
+   if (tty->ops->poll)
+   mask |= tty->ops->poll(tty, file, wait);
return mask;
 }
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index b742b5e..630ef03 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -243,6 +243,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct tty_struct;
 struct tty_driver;
@@ -285,6 +286,8 @@ struct tty_operations {
int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
int (*get_icount)(struct tty_struct *tty,
struct serial_icounter_struct *icount);
+   unsigned int (*poll)(struct tty_struct *tty, struct file *file,
+   poll_table *wait);
 #ifdef CONFIG_CONSOLE_POLL
int (*poll_init)(struct tty_driver *driver, int line, char *options);
int (*poll_get_char)(struct tty_driver *driver, int line);
-- 
2.7.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 2/8] usb: gadget: u_serial: propagate poll() to the next layer

[Tal Shorer]

In order for a serial function to add flags to the poll() mask of their
tty files, propagate the poll() callback to the next layer so it can
return a mask if it sees fit to do so.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 16 
 drivers/usb/gadget/function/u_serial.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 9b0805f..d466f58 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1025,6 +1025,21 @@ static int gs_break_ctl(struct tty_struct *tty, int 
duration)
return status;
 }
 
+static unsigned int gs_poll(struct tty_struct *tty, struct file *file,
+   poll_table *wait)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   unsigned int mask = 0;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->poll)
+   mask |= gser->poll(gser, file, wait);
+   spin_unlock_irq(>port_lock);
+   return mask;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1035,6 +1050,7 @@ static const struct tty_operations gs_tty_ops = {
.chars_in_buffer =  gs_chars_in_buffer,
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
+   .poll = gs_poll,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index c20210c..ce00840 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -12,6 +12,7 @@
 #ifndef __U_SERIAL_H
 #define __U_SERIAL_H
 
+#include 
 #include 
 #include 
 
@@ -50,6 +51,8 @@ struct gserial {
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
int (*send_break)(struct gserial *p, int duration);
+   unsigned int (*poll)(struct gserial *p, struct file *file,
+   poll_table *wait);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.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 5/8] usb: gadget: f_acm: initialize port_line_coding when creating an instance

[Tal Shorer]

Initialize acm->port_line_coding with something that makes sense so
that we can return a valid line coding if the host requests
GetLineCoding before requesting SetLineCoding

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index e023313..b7a1466 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.func.unbind = acm_unbind;
acm->port.func.free_func = acm_free_func;
 
+   /* initialize port_line_coding with something that makes sense */
+   coding.dwDTERate = cpu_to_le32(9600);
+   coding.bCharFormat = USB_CDC_1_STOP_BITS;
+   coding.bParityType = USB_CDC_NO_PARITY;
+   coding.bDataBits = 8;
+
return >port.func;
 }
 
-- 
2.7.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 7/8] usb: gadget: f_acm: notify the user on SetLineCoding

[Tal Shorer]

Notify the user with a POLLPRI event when the host issues a
SetLineCoding request so that they can act upon it, for example by
configuring the line coding on a real serial port.

The event is cleared when the user reads the line coding using
USB_F_ACM_GET_LINE_CODING ioctl()

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 38 ++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5feea7c..0983999 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -58,6 +58,9 @@ struct f_acm {
struct usb_request  *notify_req;
 
struct usb_cdc_line_coding  port_line_coding;   /* 8-N-1 etc */
+   /* we have a SetLineCoding request that the user haven't read yet */
+   bool set_line_coding_pending;
+   wait_queue_head_t set_line_coding_waitq;
 
/* SetControlLineState request -- CDC 1.1 section 6.2.14 (INPUT) */
u16 port_handshake_bits;
@@ -326,23 +329,19 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
} else {
struct usb_cdc_line_coding  *value = req->buf;
 
-   /* REVISIT:  we currently just remember this data.
-   * If we change that,
-   * (a) update whatever hardware needs updating,
-   * (b) worry about locking.  This is information on
-   * the order of 9600-8-N-1 ... most of which means
-   * nothing unless we control a real RS232 line.
-   */
dev_dbg(>gadget->dev,
"acm ttyGS%d set_line_coding: %d %d %d %d\n",
acm->port_num, le32_to_cpu(value->dwDTERate),
value->bCharFormat, value->bParityType,
value->bDataBits);
if (value->bCharFormat > 2 || value->bParityType > 4 ||
-   value->bDataBits < 5 || value->bDataBits > 8)
+   value->bDataBits < 5 || value->bDataBits > 8) {
usb_ep_set_halt(ep);
-   else
+   } else {
acm->port_line_coding = *value;
+   acm->set_line_coding_pending = true;
+   wake_up_interruptible(>set_line_coding_waitq);
+   }
}
 }
 
@@ -598,6 +597,19 @@ static void acm_disconnect(struct gserial *port)
acm_notify_serial_state(acm);
 }
 
+static unsigned int acm_poll(struct gserial *port, struct file *file,
+   poll_table *wait)
+{
+   unsigned int mask = 0;
+   struct f_acm *acm = port_to_acm(port);
+
+   poll_wait(file, >set_line_coding_waitq, wait);
+   if (acm->set_line_coding_pending)
+   mask |= POLLPRI;
+   return mask;
+}
+
+
 static int acm_send_break(struct gserial *port, int duration)
 {
struct f_acm*acm = port_to_acm(port);
@@ -620,10 +632,12 @@ static int acm_ioctl(struct gserial *port, unsigned int 
cmd, unsigned long arg)
switch (cmd) {
case USB_F_ACM_GET_LINE_CODING:
if (copy_to_user((__user void *)arg, >port_line_coding,
-   sizeof(acm->port_line_coding)))
+   sizeof(acm->port_line_coding))) {
ret = -EFAULT;
-   else
+   } else {
ret = 0;
+   acm->set_line_coding_pending = false;
+   }
break;
}
return ret;
@@ -763,11 +777,13 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(>lock);
+   init_waitqueue_head(>set_line_coding_waitq);
 
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
acm->port.ioctl = acm_ioctl;
+   acm->port.poll = acm_poll;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
-- 
2.7.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 3/8] usb: gadget: f_acm: validate set_line_coding requests

[Tal Shorer]

We shouldn't accept malformed set_line_coding requests.
All values were taken from table 17 (section 6.3.11) of the cdc1.2 spec
available at http://www.usb.org/developers/docs/devclass_docs/
The table is in the file PTSN120.pdf.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5e3828d..e023313 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -326,13 +326,22 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
struct usb_cdc_line_coding  *value = req->buf;
 
/* REVISIT:  we currently just remember this data.
-* If we change that, (a) validate it first, then
-* (b) update whatever hardware needs updating,
-* (c) worry about locking.  This is information on
-* the order of 9600-8-N-1 ... most of which means
-* nothing unless we control a real RS232 line.
-*/
-   acm->port_line_coding = *value;
+   * If we change that,
+   * (a) update whatever hardware needs updating,
+   * (b) worry about locking.  This is information on
+   * the order of 9600-8-N-1 ... most of which means
+   * nothing unless we control a real RS232 line.
+   */
+   dev_dbg(>gadget->dev,
+   "acm ttyGS%d set_line_coding: %d %d %d %d\n",
+   acm->port_num, le32_to_cpu(value->dwDTERate),
+   value->bCharFormat, value->bParityType,
+   value->bDataBits);
+   if (value->bCharFormat > 2 || value->bParityType > 4 ||
+   value->bDataBits < 5 || value->bDataBits > 8)
+   usb_ep_set_halt(ep);
+   else
+   acm->port_line_coding = *value;
}
 }
 
-- 
2.7.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 4/8] usb: gadget: u_serial: propagate ioctl() to the next layer

[Tal Shorer]

In order for a serial function to implement its own ioctl() calls,
propagate the ioctl() callback to the next layer so it can handle it if
it sees fit to do so.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 15 +++
 drivers/usb/gadget/function/u_serial.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index d466f58..8d9abf1 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1040,6 +1040,20 @@ static unsigned int gs_poll(struct tty_struct *tty, 
struct file *file,
return mask;
 }
 
+static int gs_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long 
arg)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   int ret = -ENOIOCTLCMD;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->ioctl)
+   ret = gser->ioctl(gser, cmd, arg);
+   spin_unlock_irq(>port_lock);
+   return ret;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1051,6 +1065,7 @@ static const struct tty_operations gs_tty_ops = {
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
.poll = gs_poll,
+   .ioctl =gs_ioctl,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index ce00840..8d0901e 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -53,6 +53,7 @@ struct gserial {
int (*send_break)(struct gserial *p, int duration);
unsigned int (*poll)(struct gserial *p, struct file *file,
poll_table *wait);
+   int (*ioctl)(struct gserial *p, unsigned int cmd, unsigned long arg);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.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 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++
 include/uapi/linux/usb/f_acm.h   | 12 
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex) Include FileComments
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
 0xCB   00-1F   CBM serial IEC bus  in development:

<mailto:michael.kl...@puffin.lb.shuttle.de>
+0xCD   10-1F   linux/usb/f_acm.h
 0xCD   01  linux/reiserfs_fs.h
 0xCF   02  fs/cifs/ioctl.c
 0xDB   00-0F   drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "u_serial.h"
 
@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int 
duration)
return acm_notify_serial_state(acm);
 }
 
+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+   struct f_acm*acm = port_to_acm(port);
+   int ret = -ENOIOCTLCMD;
+
+   switch (cmd) {
+   case USB_F_ACM_GET_LINE_CODING:
+   if (copy_to_user((__user void *)arg, >port_line_coding,
+   sizeof(acm->port_line_coding)))
+   ret = -EFAULT;
+   else
+   ret = 0;
+   break;
+   }
+   return ret;
+}
+
 /*-*/
 
 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
+   acm->port.ioctl = acm_ioctl;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
@@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.func.free_func = acm_free_func;
 
/* initialize port_line_coding with something that makes sense */
-   coding.dwDTERate = cpu_to_le32(9600);
-   coding.bCharFormat = USB_CDC_1_STOP_BITS;
-   coding.bParityType = USB_CDC_NO_PARITY;
-   coding.bDataBits = 8;
+   acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
+   acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
+   acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
+   acm->port_line_coding.bDataBits = 8;
 
return >port.func;
 }
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include 
+#include 
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
-- 
2.7.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 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial

[Tal Shorer]

GetLineCoding and SetLineCoding are a cdc-acm thing. It doesn't make
sense to have that in the generic u_serial layer. Moreso, f_acm has its
own port_line_coding in its own struct and it uses that, while the one
in struct gserial is set once upon initialization and then never used.
Also, the initialized never-used values were invalid, with bDataBits
and bCharFormat having each other's value.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 22 ++
 drivers/usb/gadget/function/u_serial.h |  3 ---
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 8d9abf1..654d4a6 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -129,9 +129,6 @@ struct gs_port {
wait_queue_head_t   drain_wait; /* wait while writes drain */
boolwrite_busy;
wait_queue_head_t   close_wait;
-
-   /* REVISIT this state ... */
-   struct usb_cdc_line_coding port_line_coding;/* 8-N-1 etc */
 };
 
 static struct portmaster {
@@ -1314,7 +1311,7 @@ static void gserial_console_exit(void)
 #endif
 
 static int
-gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
+gs_port_alloc(unsigned port_num)
 {
struct gs_port  *port;
int ret = 0;
@@ -1343,7 +1340,6 @@ gs_port_alloc(unsigned port_num, struct 
usb_cdc_line_coding *coding)
INIT_LIST_HEAD(>write_pool);
 
port->port_num = port_num;
-   port->port_line_coding = *coding;
 
ports[port_num].port = port;
 out:
@@ -1392,18 +1388,12 @@ EXPORT_SYMBOL_GPL(gserial_free_line);
 
 int gserial_alloc_line(unsigned char *line_num)
 {
-   struct usb_cdc_line_coding  coding;
struct device   *tty_dev;
int ret;
int port_num;
 
-   coding.dwDTERate = cpu_to_le32(9600);
-   coding.bCharFormat = 8;
-   coding.bParityType = USB_CDC_NO_PARITY;
-   coding.bDataBits = USB_CDC_1_STOP_BITS;
-
for (port_num = 0; port_num < MAX_U_SERIAL_PORTS; port_num++) {
-   ret = gs_port_alloc(port_num, );
+   ret = gs_port_alloc(port_num);
if (ret == -EBUSY)
continue;
if (ret)
@@ -1491,11 +1481,6 @@ int gserial_connect(struct gserial *gser, u8 port_num)
gser->ioport = port;
port->port_usb = gser;
 
-   /* REVISIT unclear how best to handle this state...
-* we don't really couple it with the Linux TTY.
-*/
-   gser->port_line_coding = port->port_line_coding;
-
/* REVISIT if waiting on "carrier detect", signal. */
 
/* if it's already open, start I/O ... and notify the serial
@@ -1543,9 +1528,6 @@ void gserial_disconnect(struct gserial *gser)
/* tell the TTY glue not to do I/O here any more */
spin_lock_irqsave(>port_lock, flags);
 
-   /* REVISIT as above: how best to track this? */
-   port->port_line_coding = gser->port_line_coding;
-
port->port_usb = NULL;
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index 8d0901e..0549efe 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -44,9 +44,6 @@ struct gserial {
struct usb_ep   *in;
struct usb_ep   *out;
 
-   /* REVISIT avoid this CDC-ACM support harder ... */
-   struct usb_cdc_line_coding port_line_coding;/* 9600-8-N-1 etc */
-
/* notification callbacks */
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
-- 
2.7.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 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests

[Tal Shorer]

I'm currently working on a project where I'd like to have an omap board
running linux be a usb-to-uart converter (using f_acm), and I've ran
into an issue: there's no way for the application to know if the host
has issued a SetLineCoding requests (after which parity/baudrate should
be changed to match the host's request).

This series adds the support necessary to achieve that:
- Allowing tty drivers to supply a poll() function to notify the user of
driver-specific events.
- Propagating poll() and ioctl() from u_serial to the next layer (f_acm)
in this case.
- Let the user read the current line coding set by the host (via an
ioctl() call).
- Notify the user when there's a pending SetLineCoding request they
haven't read yet

The last patch also removes up the port_line_config field from
struct gserial. It made no sense to have there (and had a REVISIT
comment at every turn), it was never used and it was initialized with
invalid values.

Tal Shorer (8):
  tty: add a poll() callback in struct tty_operations
  usb: gadget: u_serial: propagate poll() to the next layer
  usb: gadget: f_acm: validate set_line_coding requests
  usb: gadget: u_serial: propagate ioctl() to the next layer
  usb: gadget: f_acm: initialize port_line_coding when creating an
instance
  usb: gadget: f_acm: add an ioctl to get the current line coding
  usb: gadget: f_acm: notify the user on SetLineCoding
  usb: gadget: u_serial: remove port_line_config from struct gserial

 Documentation/ioctl/ioctl-number.txt   |  1 +
 drivers/tty/n_tty.c|  2 ++
 drivers/usb/gadget/function/f_acm.c| 66 +-
 drivers/usb/gadget/function/u_serial.c | 53 ---
 drivers/usb/gadget/function/u_serial.h |  7 ++--
 include/linux/tty_driver.h |  3 ++
 include/uapi/linux/usb/f_acm.h | 12 +++
 7 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

--
2.7.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: Piping f_acm to real hardware

[Tal Shorer]

On Mon, Jun 12, 2017 at 4:34 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Mon, Jun 12, 2017 at 04:00:56PM +0300, Tal Shorer wrote:
>> On Mon, Jun 12, 2017 at 2:42 PM, Greg KH <gre...@linuxfoundation.org> wrote:
>> > On Mon, Jun 12, 2017 at 01:17:18PM +0300, Tal Shorer wrote:
>> >> On Mon, Jun 12, 2017 at 9:48 AM, Greg KH <gre...@linuxfoundation.org> 
>> >> wrote:
>> >> > On Sun, Jun 11, 2017 at 11:41:02PM +0300, Tal Shorer wrote:
>> >> >> I'm currently working on a project where I'd like to have an omap board
>> >> >> running linux be a usb-to-uart converter (using f_acm). I have an
>> >> >> application that holds both the ttyGS* and the ttyO* port (the physical
>> >> >> uart port) open, polls for readable data, and writes any incoming data
>> >> >> to the other side.
>> >> >>
>> >> >> I'd also like the host to be able to configure baudrate, parity, etc.
>> >> >> My thought on how to achieve this is to modify how the ttyGS* ports
>> >> >> behave by adding a POLLPRI event when set_line_coding is received. In
>> >> >> order to do that several steps will have to be taken:
>> >> >> - one of:
>> >> >> 1. add a poll() callback to struct tty_operations and call it in
>> >> >> n_tty_poll(). u_serial and f_acm will implement required
>> >> >> callbacks to allow returning POLLPRI when necessary.
>> >> >> 2. add a flag to struct tty_struct that indicates an exceptional
>> >> >> condition and make n_tty_poll() return POLLPRI if that flag is
>> >> >> set. this will also require a new wait queue to allow us to 
>> >> >> wait
>> >> >> for such an event.
>> >> >> - after receiving SET_LINE_CODING (and verifying it), set whatever flag
>> >> >> is necessary to wake the user with POLLPRI.
>> >> >> - implement the ioctl() callback in u_serial and f_acm to allow the
>> >> >> user to get the line coding set by the host, with which it can
>> >> >> configure the hardware port accordingly.
>> >> >>
>> >> >> Does my approach make sense? Assuming I do that, which tree should such
>> >> >> a series be sent to? usb-gadget? tty?
>> >> >
>> >> > Why not just do it all in userspace, a simple "loopback" program should
>> >> > be able to handle this.  If not, look into the 'serdev' interface now in
>> >> > the kernel, that might help out a lot with what you want to do here.
>> >> >
>> >> > And I really doubt you need to touch the tty core or ldisc for this,
>> >> > that seems a bit extreme.
>> >> >
>> >> > good luck!
>> >> >
>> >> > greg k-h
>> >>
>> >> I'm not sure what you mean by "loopback", but if you mean an
>> >> application that poll()s the two sides, reads from one side and writes
>> >> to the other, I'm already doing that. My issue is that:
>> >> 1. f_acm doesn't do anything with set_line_coding.
>> >> 2. even if it did, the tty core has no easy way to notify the user of 
>> >> that.
>> >
>> > Ah, I understand now, thanks.  Yeah, that's a problem :)
>> >
>> > Good luck!
>> >
>> > greg k-h
>> Which of the two approaches do you think is cleaner? I tend to favour
>> the first. Should this be split into two series, one for tty core and
>> one for the gadget stuff or should this be one long series? If one
>> series, which tree should I submit this to?
>
> One long series is fine, post to both lists and let's see how it works
> out...
>
> greg k-h
Alright, so I'm about to add a new ioctl for ttyGS* devices when used
with acm, which means I should it to a uapi header file somewhere.
include/uapi/linux/usb/cdc.h makes the most sense from the existing
ones, but it's currently only definitions and I see the value in
keeping it that way. Should I create a new uapi file, maybe called
include/uapi/linux/usb/f_acm.h or use the existing one?
--
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: Piping f_acm to real hardware

[Tal Shorer]

On Mon, Jun 12, 2017 at 2:42 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Mon, Jun 12, 2017 at 01:17:18PM +0300, Tal Shorer wrote:
>> On Mon, Jun 12, 2017 at 9:48 AM, Greg KH <gre...@linuxfoundation.org> wrote:
>> > On Sun, Jun 11, 2017 at 11:41:02PM +0300, Tal Shorer wrote:
>> >> I'm currently working on a project where I'd like to have an omap board
>> >> running linux be a usb-to-uart converter (using f_acm). I have an
>> >> application that holds both the ttyGS* and the ttyO* port (the physical
>> >> uart port) open, polls for readable data, and writes any incoming data
>> >> to the other side.
>> >>
>> >> I'd also like the host to be able to configure baudrate, parity, etc.
>> >> My thought on how to achieve this is to modify how the ttyGS* ports
>> >> behave by adding a POLLPRI event when set_line_coding is received. In
>> >> order to do that several steps will have to be taken:
>> >> - one of:
>> >> 1. add a poll() callback to struct tty_operations and call it in
>> >> n_tty_poll(). u_serial and f_acm will implement required
>> >> callbacks to allow returning POLLPRI when necessary.
>> >> 2. add a flag to struct tty_struct that indicates an exceptional
>> >> condition and make n_tty_poll() return POLLPRI if that flag is
>> >> set. this will also require a new wait queue to allow us to wait
>> >> for such an event.
>> >> - after receiving SET_LINE_CODING (and verifying it), set whatever flag
>> >> is necessary to wake the user with POLLPRI.
>> >> - implement the ioctl() callback in u_serial and f_acm to allow the
>> >> user to get the line coding set by the host, with which it can
>> >> configure the hardware port accordingly.
>> >>
>> >> Does my approach make sense? Assuming I do that, which tree should such
>> >> a series be sent to? usb-gadget? tty?
>> >
>> > Why not just do it all in userspace, a simple "loopback" program should
>> > be able to handle this.  If not, look into the 'serdev' interface now in
>> > the kernel, that might help out a lot with what you want to do here.
>> >
>> > And I really doubt you need to touch the tty core or ldisc for this,
>> > that seems a bit extreme.
>> >
>> > good luck!
>> >
>> > greg k-h
>>
>> I'm not sure what you mean by "loopback", but if you mean an
>> application that poll()s the two sides, reads from one side and writes
>> to the other, I'm already doing that. My issue is that:
>> 1. f_acm doesn't do anything with set_line_coding.
>> 2. even if it did, the tty core has no easy way to notify the user of that.
>
> Ah, I understand now, thanks.  Yeah, that's a problem :)
>
> Good luck!
>
> greg k-h
Which of the two approaches do you think is cleaner? I tend to favour
the first. Should this be split into two series, one for tty core and
one for the gadget stuff or should this be one long series? If one
series, which tree should I submit this to?
--
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: Piping f_acm to real hardware

[Tal Shorer]

On Mon, Jun 12, 2017 at 9:48 AM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Sun, Jun 11, 2017 at 11:41:02PM +0300, Tal Shorer wrote:
>> I'm currently working on a project where I'd like to have an omap board
>> running linux be a usb-to-uart converter (using f_acm). I have an
>> application that holds both the ttyGS* and the ttyO* port (the physical
>> uart port) open, polls for readable data, and writes any incoming data
>> to the other side.
>>
>> I'd also like the host to be able to configure baudrate, parity, etc.
>> My thought on how to achieve this is to modify how the ttyGS* ports
>> behave by adding a POLLPRI event when set_line_coding is received. In
>> order to do that several steps will have to be taken:
>> - one of:
>> 1. add a poll() callback to struct tty_operations and call it in
>> n_tty_poll(). u_serial and f_acm will implement required
>> callbacks to allow returning POLLPRI when necessary.
>> 2. add a flag to struct tty_struct that indicates an exceptional
>> condition and make n_tty_poll() return POLLPRI if that flag is
>> set. this will also require a new wait queue to allow us to wait
>> for such an event.
>> - after receiving SET_LINE_CODING (and verifying it), set whatever flag
>> is necessary to wake the user with POLLPRI.
>> - implement the ioctl() callback in u_serial and f_acm to allow the
>> user to get the line coding set by the host, with which it can
>> configure the hardware port accordingly.
>>
>> Does my approach make sense? Assuming I do that, which tree should such
>> a series be sent to? usb-gadget? tty?
>
> Why not just do it all in userspace, a simple "loopback" program should
> be able to handle this.  If not, look into the 'serdev' interface now in
> the kernel, that might help out a lot with what you want to do here.
>
> And I really doubt you need to touch the tty core or ldisc for this,
> that seems a bit extreme.
>
> good luck!
>
> greg k-h

I'm not sure what you mean by "loopback", but if you mean an
application that poll()s the two sides, reads from one side and writes
to the other, I'm already doing that. My issue is that:
1. f_acm doesn't do anything with set_line_coding.
2. even if it did, the tty core has no easy way to notify the user of that.

As to serdev, I looked into it and it really could do the job if we
redesigned u_serial to be a serdev driver. I don't think we want that
since sometimes the user might not want to use real hardware (obex
comes to mind easily but it could be any of them really).

I think this choice should be left to the user, but then the user
needs a way to know they should update the hardware's line coding
--
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

Piping f_acm to real hardware

[Tal Shorer]

I'm currently working on a project where I'd like to have an omap board
running linux be a usb-to-uart converter (using f_acm). I have an
application that holds both the ttyGS* and the ttyO* port (the physical
uart port) open, polls for readable data, and writes any incoming data
to the other side.

I'd also like the host to be able to configure baudrate, parity, etc.
My thought on how to achieve this is to modify how the ttyGS* ports
behave by adding a POLLPRI event when set_line_coding is received. In
order to do that several steps will have to be taken:
- one of:
1. add a poll() callback to struct tty_operations and call it in
n_tty_poll(). u_serial and f_acm will implement required
callbacks to allow returning POLLPRI when necessary.
2. add a flag to struct tty_struct that indicates an exceptional
condition and make n_tty_poll() return POLLPRI if that flag is
set. this will also require a new wait queue to allow us to wait
for such an event.
- after receiving SET_LINE_CODING (and verifying it), set whatever flag
is necessary to wake the user with POLLPRI.
- implement the ioctl() callback in u_serial and f_acm to allow the
user to get the line coding set by the host, with which it can
configure the hardware port accordingly.

Does my approach make sense? Assuming I do that, which tree should such
a series be sent to? usb-gadget? tty?
--
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 v3] usb: hcd.h: construct hub class request constants from simpler constants

[Tal Shorer]

On Fri, Nov 18, 2016 at 2:17 PM, Tal Shorer <tal.sho...@gmail.com> wrote:
> Currently, each hub class request constant is defined by a line like:
> #define ClearHubFeature (0x2000 | USB_REQ_CLEAR_FEATURE)
>
> The "magic" number for the high byte is one of 0x20, 0xa0, 0x23, 0xa3.
> The 0x80 bit that changes inditace USB_DIR_IN, and the 0x03 that
> pops up is the difference between USB_RECIP_DEVICE (0x00) and
> USB_RECIP_OTHER (0x03). The constant 0x20 bit is USB_TYPE_CLASS.
>
> This patch eliminates those magic numbers by defining a macro to help
> construct these hub class request from simpler constants.
> Note that USB_RT_HUB is defined as (USB_TYPE_CLASS | USB_RECIP_DEVICE)
> and that USB_RT_PORT is defined as (USB_TYPE_CLASS | USB_RECIP_OTHER).
>
> Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
> ---
>  include/linux/usb/hcd.h | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 66fc137..40edf6a 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -566,21 +566,22 @@ extern void usb_ep0_reinit(struct usb_device *);
> ((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
>
>  /* class requests from the USB 2.0 hub spec, table 11-15 */
> +#define HUB_CLASS_REQ(dir, type, request) dir) | (type)) << 8) | 
> (request))
>  /* GetBusState and SetHubDescriptor are optional, omitted */
> -#define ClearHubFeature(0x2000 | USB_REQ_CLEAR_FEATURE)
> -#define ClearPortFeature   (0x2300 | USB_REQ_CLEAR_FEATURE)
> -#define GetHubDescriptor   (0xa000 | USB_REQ_GET_DESCRIPTOR)
> -#define GetHubStatus   (0xa000 | USB_REQ_GET_STATUS)
> -#define GetPortStatus  (0xa300 | USB_REQ_GET_STATUS)
> -#define SetHubFeature  (0x2000 | USB_REQ_SET_FEATURE)
> -#define SetPortFeature (0x2300 | USB_REQ_SET_FEATURE)
> +#define ClearHubFeatureHUB_CLASS_REQ(USB_DIR_OUT, 
> USB_RT_HUB, USB_REQ_CLEAR_FEATURE)
> +#define ClearPortFeature   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
> USB_REQ_CLEAR_FEATURE)
> +#define GetHubDescriptor   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, 
> USB_REQ_GET_DESCRIPTOR)
> +#define GetHubStatus   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, 
> USB_REQ_GET_STATUS)
> +#define GetPortStatus  HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
> USB_REQ_GET_STATUS)
> +#define SetHubFeature  HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
> USB_REQ_SET_FEATURE)
> +#define SetPortFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
> USB_REQ_SET_FEATURE)
>
>
>  /*-*/
>
>  /* class requests from USB 3.1 hub spec, table 10-7 */
> -#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
> -#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
> +#define SetHubDepthHUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
> HUB_SET_DEPTH)
> +#define GetPortErrorCount  HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
> HUB_GET_PORT_ERR_COUNT)
>
>  /*
>   * Generic bandwidth allocation constants/support
> --
> 2.5.0
>
ping?
--
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] usb: hcd.h: construct hub class request constants from simpler constants

[Tal Shorer]

Currently, each hub class request constant is defined by a line like:
#define ClearHubFeature (0x2000 | USB_REQ_CLEAR_FEATURE)

The "magic" number for the high byte is one of 0x20, 0xa0, 0x23, 0xa3.
The 0x80 bit that changes inditace USB_DIR_IN, and the 0x03 that
pops up is the difference between USB_RECIP_DEVICE (0x00) and
USB_RECIP_OTHER (0x03). The constant 0x20 bit is USB_TYPE_CLASS.

This patch eliminates those magic numbers by defining a macro to help
construct these hub class request from simpler constants.
Note that USB_RT_HUB is defined as (USB_TYPE_CLASS | USB_RECIP_DEVICE)
and that USB_RT_PORT is defined as (USB_TYPE_CLASS | USB_RECIP_OTHER).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 include/linux/usb/hcd.h | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 66fc137..40edf6a 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -566,21 +566,22 @@ extern void usb_ep0_reinit(struct usb_device *);
((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
 
 /* class requests from the USB 2.0 hub spec, table 11-15 */
+#define HUB_CLASS_REQ(dir, type, request) dir) | (type)) << 8) | (request))
 /* GetBusState and SetHubDescriptor are optional, omitted */
-#define ClearHubFeature(0x2000 | USB_REQ_CLEAR_FEATURE)
-#define ClearPortFeature   (0x2300 | USB_REQ_CLEAR_FEATURE)
-#define GetHubDescriptor   (0xa000 | USB_REQ_GET_DESCRIPTOR)
-#define GetHubStatus   (0xa000 | USB_REQ_GET_STATUS)
-#define GetPortStatus  (0xa300 | USB_REQ_GET_STATUS)
-#define SetHubFeature  (0x2000 | USB_REQ_SET_FEATURE)
-#define SetPortFeature (0x2300 | USB_REQ_SET_FEATURE)
+#define ClearHubFeatureHUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
USB_REQ_CLEAR_FEATURE)
+#define ClearPortFeature   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
USB_REQ_CLEAR_FEATURE)
+#define GetHubDescriptor   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, 
USB_REQ_GET_DESCRIPTOR)
+#define GetHubStatus   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, 
USB_REQ_GET_STATUS)
+#define GetPortStatus  HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
USB_REQ_GET_STATUS)
+#define SetHubFeature  HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
USB_REQ_SET_FEATURE)
+#define SetPortFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
USB_REQ_SET_FEATURE)
 
 
 /*-*/
 
 /* class requests from USB 3.1 hub spec, table 10-7 */
-#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
+#define SetHubDepthHUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
HUB_SET_DEPTH)
+#define GetPortErrorCount  HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
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 v2] usb: hcd.h: construct hub class request conastants from simpler constants

[Tal Shorer]

Currently, each hub class request constant is defined by a line like:

The "magic" number for the high byte is one of 0x20, 0xa0, 0x23, 0xa3.
The 0x80 bit that changes inditace USB_DIR_IN, and the 0x03 that
pops up is the difference between USB_RECIP_DEVICE (0x00) and
USB_RECIP_OTHER (0x03). The constant 0x20 bit is USB_TYPE_CLASS.

This patch eliminates those magic numbers by defining a macro to help
construct these hub class request from simpler constants.
Note that USB_RT_HUB is defined as (USB_TYPE_CLASS | USB_RECIP_DEVICE)
and that USB_RT_PORT is defined as (USB_TYPE_CLASS | USB_RECIP_OTHER).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 include/linux/usb/hcd.h | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 66fc137..76159fb 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -566,21 +566,22 @@ extern void usb_ep0_reinit(struct usb_device *);
((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
 
 /* class requests from the USB 2.0 hub spec, table 11-15 */
+#define HUB_CLASS_REQ(dir, type, request) dir) | (type)) << 8) | (request))
 /* GetBusState and SetHubDescriptor are optional, omitted */
-#define ClearHubFeature(0x2000 | USB_REQ_CLEAR_FEATURE)
-#define ClearPortFeature   (0x2300 | USB_REQ_CLEAR_FEATURE)
-#define GetHubDescriptor   (0xa000 | USB_REQ_GET_DESCRIPTOR)
-#define GetHubStatus   (0xa000 | USB_REQ_GET_STATUS)
-#define GetPortStatus  (0xa300 | USB_REQ_GET_STATUS)
-#define SetHubFeature  (0x2000 | USB_REQ_SET_FEATURE)
-#define SetPortFeature (0x2300 | USB_REQ_SET_FEATURE)
+#define ClearHubFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
USB_REQ_CLEAR_FEATURE)
+#define ClearPortFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
USB_REQ_CLEAR_FEATURE)
+#define GetHubDescriptor HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, 
USB_REQ_GET_DESCRIPTOR)
+#define GetHubStatus HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, USB_REQ_GET_STATUS)
+#define GetPortStatus HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
USB_REQ_GET_STATUS)
+#define SetHubFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, 
USB_REQ_SET_FEATURE)
+#define SetPortFeature HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, 
USB_REQ_SET_FEATURE)
 
 
 /*-*/
 
 /* class requests from USB 3.1 hub spec, table 10-7 */
-#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
+#define SetHubDepth HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, HUB_SET_DEPTH)
+#define GetPortErrorCount HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, 
HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
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] usb: hcd.h: construct hub class request conastants from simpler constants

[Tal Shorer]

Currently, each hub class request constant is defined by a line like:

Where the "magic" number for the high byte is one of 0x20, 0xa0, 0x23,
0xa3.
The 0x80 bit that changes inditace USB_DIR_IN, and the 0x03 that
pops up is the difference between USB_RECIP_DEVICE (0x00) and
USB_RECIP_OTHER (0x03). The constant 0x20 bit is USB_TYPE_CLASS.

This patch eliminates those magic numbers by defining a macro to help
construct these hub class request from simpler constants.
Note that USB_RT_HUB is defined as (USB_TYPE_CLASS | USB_RECIP_DEVICE)
and that USB_RT_PORT is defined as (USB_TYPE_CLASS | USB_RECIP_OTHER).

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 include/linux/usb/hcd.h | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 66fc137..2405853 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -566,21 +566,29 @@ extern void usb_ep0_reinit(struct usb_device *);
((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
 
 /* class requests from the USB 2.0 hub spec, table 11-15 */
+#define HUB_CLASS_REQ(dir, type, request) dir) | (type)) << 8) | (request))
 /* GetBusState and SetHubDescriptor are optional, omitted */
-#define ClearHubFeature(0x2000 | USB_REQ_CLEAR_FEATURE)
-#define ClearPortFeature   (0x2300 | USB_REQ_CLEAR_FEATURE)
-#define GetHubDescriptor   (0xa000 | USB_REQ_GET_DESCRIPTOR)
-#define GetHubStatus   (0xa000 | USB_REQ_GET_STATUS)
-#define GetPortStatus  (0xa300 | USB_REQ_GET_STATUS)
-#define SetHubFeature  (0x2000 | USB_REQ_SET_FEATURE)
-#define SetPortFeature (0x2300 | USB_REQ_SET_FEATURE)
+#define ClearHubFeature \
+   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, USB_REQ_CLEAR_FEATURE)
+#define ClearPortFeature \
+   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, USB_REQ_CLEAR_FEATURE)
+#define GetHubDescriptor \
+   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, USB_REQ_GET_DESCRIPTOR)
+#define GetHubStatus HUB_CLASS_REQ(USB_DIR_IN, USB_RT_HUB, USB_REQ_GET_STATUS)
+#define GetPortStatus \
+   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, USB_REQ_GET_STATUS)
+#define SetHubFeature \
+   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, USB_REQ_SET_FEATURE)
+#define SetPortFeature \
+   HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, USB_REQ_SET_FEATURE)
 
 
 /*-*/
 
 /* class requests from USB 3.1 hub spec, table 10-7 */
-#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
+#define SetHubDepth HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, HUB_SET_DEPTH)
+#define GetPortErrorCount \
+   HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
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 v3 2/8] usb: ulpi: add new api functions, {read|write}_dev()

[Tal Shorer]

Add these two new api callbacks to struct ulpi_ops. These are different
than read, write in that they pass the parent device directly instead
of via the ops argument.
They are intended to replace the old api functions.

If the new api callbacks are missing, revert to calling the old ones
as before.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 8 ++--
 include/linux/ulpi/interface.h | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index c6ce92b..15e4a14 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,13 +21,17 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   return ulpi->ops->read(ulpi->ops, addr);
+   if (!ulpi->ops->read_dev)
+   return ulpi->ops->read(ulpi->ops, addr);
+   return ulpi->ops->read_dev(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   return ulpi->ops->write(ulpi->ops, addr, val);
+   if (!ulpi->ops->write_dev)
+   return ulpi->ops->write(ulpi->ops, addr, val);
+   return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
 
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index 4de8ab4..d8189d0 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -15,6 +15,8 @@ struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
+   int (*read_dev)(struct device *dev, u8 addr);
+   int (*write_dev)(struct device *dev, u8 addr, u8 val);
 };
 
 struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
-- 
2.7.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 0/8] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

struct ulpi_ops is defined as follows:

struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
};

Upon calling ulpi_register_interface(), the struct device argument is
put inside the struct ulpi_ops argument's dev field. Later, when
calling the actual read()/write() operations, the struct ulpi_ops is
passed to them and they use the stored device to access whatever
private data they need.

This means that if one wishes to reuse the same oprations for multiple
interfaces (e.g if we have multiple instances of the same controller),
any but the last interface registered will not operate properly (and
the one that does work will be at the mercy of the others to not mess
it up).

I understand that barely any driver uses this bus right now, but I
suppose it's there to be used at some point. We might as well fix the
design here before we hit this bug.

This series fixes this by passing the given struct device directly to
the operation functions via ulpi->dev.parent in ulpi_read() and
ulpi_write(). It also changes the operations struct to be constant
since now nobody has a reason to modify it.

Changes from v1:
 * Split the actual api change into multiple patch as per Felipe Balbi's
   suggestion. The series now first adds the new api, then migrates
   everything to use and only then removes the old api.

Changes from v2:
 * Merge patches 2 and 3 (now patch 2)
 * Merge patches 5 and 6 (now patch 4)
 * Remove comment documenting the removed dev field in struct ulpi_ops

Tal Shorer (8):
  usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()
  usb: ulpi: add new api functions, {read|write}_dev()
  usb: dwc3: ulpi: use new api
  usb: ulpi: remove calls to old api callbacks
  usb: ulpi: rename operations {read|write}_dev to simply {read|write}
  usb: ulpi: remove "dev" field from struct ulpi_ops
  usb: ulpi: make ops struct constant
  usb: dwc3: ulpi: make dwc3_ulpi_ops constant

 drivers/usb/common/ulpi.c  | 11 ++-
 drivers/usb/dwc3/ulpi.c| 10 +-
 include/linux/ulpi/driver.h|  2 +-
 include/linux/ulpi/interface.h |  9 -
 4 files changed, 16 insertions(+), 16 deletions(-)

--
2.7.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 7/8] usb: ulpi: make ops struct constant

[Tal Shorer]

None of the core ulpi functions perform any changes to the operations
struct, and logically as a struct that contains function pointers
there's no reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 3 ++-
 include/linux/ulpi/driver.h| 2 +-
 include/linux/ulpi/interface.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index d005c15..8b31770 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -203,7 +203,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
  * Allocates and registers a ULPI device and an interface for it. Called from
  * the USB controller that provides the ULPI interface.
  */
-struct ulpi *ulpi_register_interface(struct device *dev, struct ulpi_ops *ops)
+struct ulpi *ulpi_register_interface(struct device *dev,
+const struct ulpi_ops *ops)
 {
struct ulpi *ulpi;
int ret;
diff --git a/include/linux/ulpi/driver.h b/include/linux/ulpi/driver.h
index 80b36ca..a7af21a 100644
--- a/include/linux/ulpi/driver.h
+++ b/include/linux/ulpi/driver.h
@@ -15,7 +15,7 @@ struct ulpi_ops;
  */
 struct ulpi {
struct ulpi_device_id id;
-   struct ulpi_ops *ops;
+   const struct ulpi_ops *ops;
struct device dev;
 };
 
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index cdedac8..a2011a9 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -16,7 +16,7 @@ struct ulpi_ops {
int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
-struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
+struct ulpi *ulpi_register_interface(struct device *, const struct ulpi_ops *);
 void ulpi_unregister_interface(struct ulpi *);
 
 #endif /* __LINUX_ULPI_INTERFACE_H */
-- 
2.7.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 5/8] usb: ulpi: rename operations {read|write}_dev to simply {read|write}

[Tal Shorer]

With the removal of the old {read|write} operations, we can now safely
rename the new api operations {read|write}_dev to use the shorter and
clearer names {read|write}, respectively.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 4 ++--
 drivers/usb/dwc3/ulpi.c| 4 ++--
 include/linux/ulpi/interface.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index d682cf2..da17a74 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,13 +21,13 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   return ulpi->ops->read_dev(ulpi->dev.parent, addr);
+   return ulpi->ops->read(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
+   return ulpi->ops->write(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
 
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 94eeb7a..51ac939 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -66,8 +66,8 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 
val)
 }
 
 static struct ulpi_ops dwc3_ulpi_ops = {
-   .read_dev = dwc3_ulpi_read,
-   .write_dev = dwc3_ulpi_write,
+   .read = dwc3_ulpi_read,
+   .write = dwc3_ulpi_write,
 };
 
 int dwc3_ulpi_init(struct dwc3 *dwc)
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index 71f3c99..ac3cd80 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -13,8 +13,8 @@ struct ulpi;
  */
 struct ulpi_ops {
struct device *dev;
-   int (*read_dev)(struct device *dev, u8 addr);
-   int (*write_dev)(struct device *dev, u8 addr, u8 val);
+   int (*read)(struct device *dev, u8 addr);
+   int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
 struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
-- 
2.7.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 4/8] usb: ulpi: remove calls to old api callbacks

[Tal Shorer]

Now that all users use the new api callbacks, remove the old api
callbacks and force new interface drivers to use the new api.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 4 
 include/linux/ulpi/interface.h | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 15e4a14..d682cf2 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,16 +21,12 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   if (!ulpi->ops->read_dev)
-   return ulpi->ops->read(ulpi->ops, addr);
return ulpi->ops->read_dev(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   if (!ulpi->ops->write_dev)
-   return ulpi->ops->write(ulpi->ops, addr, val);
return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index d8189d0..71f3c99 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -13,8 +13,6 @@ struct ulpi;
  */
 struct ulpi_ops {
struct device *dev;
-   int (*read)(struct ulpi_ops *ops, u8 addr);
-   int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
int (*read_dev)(struct device *dev, u8 addr);
int (*write_dev)(struct device *dev, u8 addr, u8 val);
 };
-- 
2.7.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 8/8] usb: dwc3: ulpi: make dwc3_ulpi_ops constant

[Tal Shorer]

ulpi_register_interface() accepts a const struct ulpi_ops and dwc3
doesn't perform any changes to this struct at runtime, so there's no
reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/dwc3/ulpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 51ac939..bd86f84 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -65,7 +65,7 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 
val)
return dwc3_ulpi_busyloop(dwc);
 }
 
-static struct ulpi_ops dwc3_ulpi_ops = {
+static const struct ulpi_ops dwc3_ulpi_ops = {
.read = dwc3_ulpi_read,
.write = dwc3_ulpi_write,
 };
-- 
2.7.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 3/8] usb: dwc3: ulpi: use new api

[Tal Shorer]

The old read, write callbacks in struct ulpi_ops have been deprecated
in favor of new callbacks that pass the parent device directly.
Replace the used callbacks in dwc3's ulpi component with the new api.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/dwc3/ulpi.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index ec004c6..94eeb7a 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -35,9 +35,9 @@ static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
return -ETIMEDOUT;
 }
 
-static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
+static int dwc3_ulpi_read(struct device *dev, u8 addr)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
int ret;
 
@@ -53,9 +53,9 @@ static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
return DWC3_GUSB2PHYACC_DATA(reg);
 }
 
-static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
+static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 val)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
 
reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
@@ -66,8 +66,8 @@ static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 
val)
 }
 
 static struct ulpi_ops dwc3_ulpi_ops = {
-   .read = dwc3_ulpi_read,
-   .write = dwc3_ulpi_write,
+   .read_dev = dwc3_ulpi_read,
+   .write_dev = dwc3_ulpi_write,
 };
 
 int dwc3_ulpi_init(struct dwc3 *dwc)
-- 
2.7.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 6/8] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

Operations now use ulpi->dev.parent directly instead of via the
ulpi_ops struct, making this field unused. Remove it.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 1 -
 include/linux/ulpi/interface.h | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index da17a74..d005c15 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -213,7 +213,6 @@ struct ulpi *ulpi_register_interface(struct device *dev, 
struct ulpi_ops *ops)
return ERR_PTR(-ENOMEM);
 
ulpi->ops = ops;
-   ops->dev = dev;
 
ret = ulpi_register(dev, ulpi);
if (ret) {
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index ac3cd80..cdedac8 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -4,15 +4,14 @@
 #include 
 
 struct ulpi;
+struct device;
 
 /**
  * struct ulpi_ops - ULPI register access
- * @dev: the interface provider
  * @read: read operation for ULPI register access
  * @write: write operation for ULPI register access
  */
 struct ulpi_ops {
-   struct device *dev;
int (*read)(struct device *dev, u8 addr);
int (*write)(struct device *dev, u8 addr, u8 val);
 };
-- 
2.7.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 1/8] usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()

[Tal Shorer]

Once ulpi operations use the parent device directly, this will be
needed during the operations used in ulpi_register() itself, so set
the parent field before calling any ulpi operations.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index e04a34e..c6ce92b 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -157,6 +157,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
 {
int ret;
 
+   ulpi->dev.parent = dev; /* needed early for ops */
+
/* Test the interface */
ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
if (ret < 0)
@@ -175,7 +177,6 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
-   ulpi->dev.parent = dev;
ulpi->dev.bus = _bus;
ulpi->dev.type = _dev_type;
dev_set_name(>dev, "%s.ulpi", dev_name(dev));
-- 
2.7.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 v2 00/10] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

On Tue, Aug 9, 2016 at 5:04 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> On Mon, Aug 01, 2016 at 09:15:48PM +0300, Tal Shorer wrote:
>> struct ulpi_ops is defined as follows:
>>
>> struct ulpi_ops {
>> struct device *dev;
>> int (*read)(struct ulpi_ops *ops, u8 addr);
>> int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
>> };
>>
>> Upon calling ulpi_register_interface(), the struct device argument is
>> put inside the struct ulpi_ops argument's dev field. Later, when
>> calling the actual read()/write() operations, the struct ulpi_ops is
>> passed to them and they use the stored device to access whatever
>> private data they need.
>>
>> This means that if one wishes to reuse the same oprations for multiple
>> interfaces (e.g if we have multiple instances of the same controller),
>> any but the last interface registered will not operate properly (and
>> the one that does work will be at the mercy of the others to not mess
>> it up).
>>
>> I understand that barely any driver uses this bus right now, but I
>> suppose it's there to be used at some point. We might as well fix the
>> design here before we hit this bug.
>>
>> This series fixes this by passing the given struct device directly to
>> the operation functions via ulpi->dev.parent in ulpi_read() and
>> ulpi_write(). It also changes the operations struct to be constant
>> since now nobody has a reason to modify it.
>>
>> Changes from v1:
>>  * Split the actual api change into multiple patch as per Felipe Balbi's
>>suggestion. The series now first adds the new api, then migrates
>>everything to use and only then removes the old api.
>>
>> Tal Shorer (10):
>>   usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()
>>   usb: ulpi: add new api functions, {read|write}_dev()
>>   usb: ulpi: use new api functions if available
>>   usb: dwc3: ulpi: use new api
>>   usb: ulpi: remove calls to old api callbacks
>>   usb: ulpi: remove old api callbacks from struct ulpi_ops
>>   usb: ulpi: rename operations {read|write}_dev to simply {read|write}
>>   usb: ulpi: remove "dev" field from struct ulpi_ops
>>   usb: ulpi: make ops struct constant
>>   usb: dwc3: ulpi: make dwc3_ulpi_ops constant
>
> I'd like to get Heikki's ack for this series...
Anything to do on my end except waiting?
--
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/10] usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()

[Tal Shorer]

Once ulpi operations use the parent device directly, this will be
needed during the operations used in ulpi_register() itself, so set
the parent field before calling any ulpi operations.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 01c0c04..d1f419c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -156,6 +156,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
 {
int ret;
 
+   ulpi->dev.parent = dev; /* needed early for ops */
+
/* Test the interface */
ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
if (ret < 0)
@@ -174,7 +176,6 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
-   ulpi->dev.parent = dev;
ulpi->dev.bus = _bus;
ulpi->dev.type = _dev_type;
dev_set_name(>dev, "%s.ulpi", dev_name(dev));
-- 
2.7.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/10] usb: ulpi: remove old api callbacks from struct ulpi_ops

[Tal Shorer]

The old api callbacks, read() and write(), are not referenced anywhere.
Remove them.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 include/linux/ulpi/interface.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index d8189d0..71f3c99 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -13,8 +13,6 @@ struct ulpi;
  */
 struct ulpi_ops {
struct device *dev;
-   int (*read)(struct ulpi_ops *ops, u8 addr);
-   int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
int (*read_dev)(struct device *dev, u8 addr);
int (*write_dev)(struct device *dev, u8 addr, u8 val);
 };
-- 
2.7.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/10] usb: dwc3: ulpi: use new api

[Tal Shorer]

The old read, write callbacks in struct ulpi_ops have been deprecated
in favor of new callbacks that pass the parent device directly.
Replace the used callbacks in dwc3's ulpi component with the new api.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/dwc3/ulpi.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index ec004c6..94eeb7a 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -35,9 +35,9 @@ static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
return -ETIMEDOUT;
 }
 
-static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
+static int dwc3_ulpi_read(struct device *dev, u8 addr)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
int ret;
 
@@ -53,9 +53,9 @@ static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
return DWC3_GUSB2PHYACC_DATA(reg);
 }
 
-static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
+static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 val)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
 
reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
@@ -66,8 +66,8 @@ static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 
val)
 }
 
 static struct ulpi_ops dwc3_ulpi_ops = {
-   .read = dwc3_ulpi_read,
-   .write = dwc3_ulpi_write,
+   .read_dev = dwc3_ulpi_read,
+   .write_dev = dwc3_ulpi_write,
 };
 
 int dwc3_ulpi_init(struct dwc3 *dwc)
-- 
2.7.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/10] usb: ulpi: use new api functions if available

[Tal Shorer]

If the registered has the new api callbacks {read|write}_dev, call
these instead of the deprecated read, write functions. If the
registered does not support the new callbacks, revert to calling the
old ones as before.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index d1f419c..a877ddb 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,13 +21,17 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   return ulpi->ops->read(ulpi->ops, addr);
+   if (!ulpi->ops->read_dev)
+   return ulpi->ops->read(ulpi->ops, addr);
+   return ulpi->ops->read_dev(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   return ulpi->ops->write(ulpi->ops, addr, val);
+   if (!ulpi->ops->write_dev)
+   return ulpi->ops->write(ulpi->ops, addr, val);
+   return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
 
-- 
2.7.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/10] usb: ulpi: add new api functions, {read|write}_dev()

[Tal Shorer]

Add these two new api callbacks to struct ulpi_ops. These are different
than read, write in that they pass the parent device directly instead
of via the ops argument.
They are intended to replace the old api functions.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 include/linux/ulpi/interface.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index 4de8ab4..d8189d0 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -15,6 +15,8 @@ struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
+   int (*read_dev)(struct device *dev, u8 addr);
+   int (*write_dev)(struct device *dev, u8 addr, u8 val);
 };
 
 struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
-- 
2.7.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 00/10] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

struct ulpi_ops is defined as follows:

struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
};

Upon calling ulpi_register_interface(), the struct device argument is
put inside the struct ulpi_ops argument's dev field. Later, when
calling the actual read()/write() operations, the struct ulpi_ops is
passed to them and they use the stored device to access whatever
private data they need.

This means that if one wishes to reuse the same oprations for multiple
interfaces (e.g if we have multiple instances of the same controller),
any but the last interface registered will not operate properly (and
the one that does work will be at the mercy of the others to not mess
it up).

I understand that barely any driver uses this bus right now, but I
suppose it's there to be used at some point. We might as well fix the
design here before we hit this bug.

This series fixes this by passing the given struct device directly to
the operation functions via ulpi->dev.parent in ulpi_read() and
ulpi_write(). It also changes the operations struct to be constant
since now nobody has a reason to modify it.

Changes from v1:
 * Split the actual api change into multiple patch as per Felipe Balbi's
   suggestion. The series now first adds the new api, then migrates
   everything to use and only then removes the old api.

Tal Shorer (10):
  usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()
  usb: ulpi: add new api functions, {read|write}_dev()
  usb: ulpi: use new api functions if available
  usb: dwc3: ulpi: use new api
  usb: ulpi: remove calls to old api callbacks
  usb: ulpi: remove old api callbacks from struct ulpi_ops
  usb: ulpi: rename operations {read|write}_dev to simply {read|write}
  usb: ulpi: remove "dev" field from struct ulpi_ops
  usb: ulpi: make ops struct constant
  usb: dwc3: ulpi: make dwc3_ulpi_ops constant

 drivers/usb/common/ulpi.c  | 11 ++-
 drivers/usb/dwc3/ulpi.c| 10 +-
 include/linux/ulpi/driver.h|  2 +-
 include/linux/ulpi/interface.h |  8 
 4 files changed, 16 insertions(+), 15 deletions(-)

-- 
2.7.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/10] usb: ulpi: make ops struct constant

[Tal Shorer]

None of the core ulpi functions perform any changes to the operations
struct, and logically as a struct that contains function pointers
there's no reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 3 ++-
 include/linux/ulpi/driver.h| 2 +-
 include/linux/ulpi/interface.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0439e96..d4ff6df 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -202,7 +202,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
  * Allocates and registers a ULPI device and an interface for it. Called from
  * the USB controller that provides the ULPI interface.
  */
-struct ulpi *ulpi_register_interface(struct device *dev, struct ulpi_ops *ops)
+struct ulpi *ulpi_register_interface(struct device *dev,
+const struct ulpi_ops *ops)
 {
struct ulpi *ulpi;
int ret;
diff --git a/include/linux/ulpi/driver.h b/include/linux/ulpi/driver.h
index 388f6e0..a44408f 100644
--- a/include/linux/ulpi/driver.h
+++ b/include/linux/ulpi/driver.h
@@ -15,7 +15,7 @@ struct ulpi_ops;
  */
 struct ulpi {
struct ulpi_device_id id;
-   struct ulpi_ops *ops;
+   const struct ulpi_ops *ops;
struct device dev;
 };
 
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index ee6e35a..43b0738 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -17,7 +17,7 @@ struct ulpi_ops {
int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
-struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
+struct ulpi *ulpi_register_interface(struct device *, const struct ulpi_ops *);
 void ulpi_unregister_interface(struct ulpi *);
 
 #endif /* __LINUX_ULPI_INTERFACE_H */
-- 
2.7.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/10] usb: ulpi: rename operations {read|write}_dev to simply {read|write}

[Tal Shorer]

With the removal of the old {read|write} operations, we can now safely
rename the new api operations {read|write}_dev to use the shorter and
clearer names {read|write}, respectively.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 4 ++--
 drivers/usb/dwc3/ulpi.c| 4 ++--
 include/linux/ulpi/interface.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 43142c3..fdaed7c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,13 +21,13 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   return ulpi->ops->read_dev(ulpi->dev.parent, addr);
+   return ulpi->ops->read(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
+   return ulpi->ops->write(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
 
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 94eeb7a..51ac939 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -66,8 +66,8 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 
val)
 }
 
 static struct ulpi_ops dwc3_ulpi_ops = {
-   .read_dev = dwc3_ulpi_read,
-   .write_dev = dwc3_ulpi_write,
+   .read = dwc3_ulpi_read,
+   .write = dwc3_ulpi_write,
 };
 
 int dwc3_ulpi_init(struct dwc3 *dwc)
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index 71f3c99..ac3cd80 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -13,8 +13,8 @@ struct ulpi;
  */
 struct ulpi_ops {
struct device *dev;
-   int (*read_dev)(struct device *dev, u8 addr);
-   int (*write_dev)(struct device *dev, u8 addr, u8 val);
+   int (*read)(struct device *dev, u8 addr);
+   int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
 struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
-- 
2.7.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 05/10] usb: ulpi: remove calls to old api callbacks

[Tal Shorer]

Now that all users use the new api callbacks, remove the calls to the
old api functions and force new interface drivers to use the new api.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index a877ddb..43142c3 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,16 +21,12 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   if (!ulpi->ops->read_dev)
-   return ulpi->ops->read(ulpi->ops, addr);
return ulpi->ops->read_dev(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   if (!ulpi->ops->write_dev)
-   return ulpi->ops->write(ulpi->ops, addr, val);
return ulpi->ops->write_dev(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
-- 
2.7.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 08/10] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

Operations now use ulpi->dev.parent directly instead of via the
ulpi_ops struct, making this field unused. Remove it.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 1 -
 include/linux/ulpi/interface.h | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index fdaed7c..0439e96 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -212,7 +212,6 @@ struct ulpi *ulpi_register_interface(struct device *dev, 
struct ulpi_ops *ops)
return ERR_PTR(-ENOMEM);
 
ulpi->ops = ops;
-   ops->dev = dev;
 
ret = ulpi_register(dev, ulpi);
if (ret) {
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index ac3cd80..ee6e35a 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -4,6 +4,7 @@
 #include 
 
 struct ulpi;
+struct device;
 
 /**
  * struct ulpi_ops - ULPI register access
@@ -12,7 +13,6 @@ struct ulpi;
  * @write: write operation for ULPI register access
  */
 struct ulpi_ops {
-   struct device *dev;
int (*read)(struct device *dev, u8 addr);
int (*write)(struct device *dev, u8 addr, u8 val);
 };
-- 
2.7.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/10] usb: dwc3: ulpi: make dwc3_ulpi_ops constant

[Tal Shorer]

ulpi_register_interface() accepts a const struct ulpi_ops and dwc3
doesn't perform any changes to this struct at runtime, so there's no
reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/dwc3/ulpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 51ac939..bd86f84 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -65,7 +65,7 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 
val)
return dwc3_ulpi_busyloop(dwc);
 }
 
-static struct ulpi_ops dwc3_ulpi_ops = {
+static const struct ulpi_ops dwc3_ulpi_ops = {
.read = dwc3_ulpi_read,
.write = dwc3_ulpi_write,
 };
-- 
2.7.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 4/5] usb: ulpi: make ops struct constant

[Tal Shorer]

None of the core ulpi functions perform any changes to the operations
struct, and logically as a struct that contains function pointers
there's no reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 3 ++-
 include/linux/ulpi/driver.h| 2 +-
 include/linux/ulpi/interface.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0439e96..d4ff6df 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -202,7 +202,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
  * Allocates and registers a ULPI device and an interface for it. Called from
  * the USB controller that provides the ULPI interface.
  */
-struct ulpi *ulpi_register_interface(struct device *dev, struct ulpi_ops *ops)
+struct ulpi *ulpi_register_interface(struct device *dev,
+const struct ulpi_ops *ops)
 {
struct ulpi *ulpi;
int ret;
diff --git a/include/linux/ulpi/driver.h b/include/linux/ulpi/driver.h
index 388f6e0..a44408f 100644
--- a/include/linux/ulpi/driver.h
+++ b/include/linux/ulpi/driver.h
@@ -15,7 +15,7 @@ struct ulpi_ops;
  */
 struct ulpi {
struct ulpi_device_id id;
-   struct ulpi_ops *ops;
+   const struct ulpi_ops *ops;
struct device dev;
 };
 
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index ee6e35a..43b0738 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -17,7 +17,7 @@ struct ulpi_ops {
int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
-struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
+struct ulpi *ulpi_register_interface(struct device *, const struct ulpi_ops *);
 void ulpi_unregister_interface(struct ulpi *);
 
 #endif /* __LINUX_ULPI_INTERFACE_H */
-- 
2.7.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 0/5] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

struct ulpi_ops is defined as follows:

struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
};

Upon calling ulpi_register_interface(), the struct device argument is
put inside the struct ulpi_ops argument's dev field. Later, when
calling the actual read()/write() operations, the struct ulpi_ops is
passed to them and they use the stored device to access whatever
private data they need.

This means that if one wishes to reuse the same oprations for multiple
interfaces (e.g if we have multiple instances of the same controller),
any but the last interface registered will not operate properly (and
the one that does work will be at the mercy of the others to not mess
it up).

I understand that barely any driver uses this bus right now, but I
suppose it's there to be used at some point. We might as well fix the
design here before we hit this bug.

This series fixes this by passing the given struct device directly to
the operation functions via ulpi->dev.parent in ulpi_read() and
ulpi_write(). It also changes the operations struct to be constant
since now nobody has a reason to modify it.

Tal Shorer (5):
  usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()
  usb: ulpi: change operations api to pass parent device directly
  usb: ulpi: remove "dev" field from struct ulpi_ops
  usb: ulpi: make ops struct constant
  usb: dwc3: ulpi: make dwc3_ulpi_ops constant

 drivers/usb/common/ulpi.c  | 11 ++-
 drivers/usb/dwc3/ulpi.c| 10 +-
 include/linux/ulpi/driver.h|  2 +-
 include/linux/ulpi/interface.h |  8 
 4 files changed, 16 insertions(+), 15 deletions(-)

--
2.7.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 5/5] usb: dwc3: ulpi: make dwc3_ulpi_ops constant

[Tal Shorer]

ulpi_register_interface() accepts a const struct ulpi_ops and dwc3
doesn't perform any changes to this struct at runtime, so there's no
reason it shouldn't be constant.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/dwc3/ulpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 51ac939..bd86f84 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -65,7 +65,7 @@ static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 
val)
return dwc3_ulpi_busyloop(dwc);
 }
 
-static struct ulpi_ops dwc3_ulpi_ops = {
+static const struct ulpi_ops dwc3_ulpi_ops = {
.read = dwc3_ulpi_read,
.write = dwc3_ulpi_write,
 };
-- 
2.7.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 1/5] usb: ulpi: move setting of ulpi->dev parent up in ulpi_register()

[Tal Shorer]

Once ulpi operations use the parent device directly, this will be
needed during the operations used in ulpi_register() itself, so set
the parent field before calling any ulpi operations.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 01c0c04..d1f419c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -156,6 +156,8 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
 {
int ret;
 
+   ulpi->dev.parent = dev; /* needed early for ops */
+
/* Test the interface */
ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
if (ret < 0)
@@ -174,7 +176,6 @@ static int ulpi_register(struct device *dev, struct ulpi 
*ulpi)
ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
-   ulpi->dev.parent = dev;
ulpi->dev.bus = _bus;
ulpi->dev.type = _dev_type;
dev_set_name(>dev, "%s.ulpi", dev_name(dev));
-- 
2.7.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 3/5] usb: ulpi: remove "dev" field from struct ulpi_ops

[Tal Shorer]

Operations now use ulpi->dev.parent directly instead of via the
ulpi_ops struct, making this field unused. Remove it.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 1 -
 include/linux/ulpi/interface.h | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index fdaed7c..0439e96 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -212,7 +212,6 @@ struct ulpi *ulpi_register_interface(struct device *dev, 
struct ulpi_ops *ops)
return ERR_PTR(-ENOMEM);
 
ulpi->ops = ops;
-   ops->dev = dev;
 
ret = ulpi_register(dev, ulpi);
if (ret) {
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index ac3cd80..ee6e35a 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -4,6 +4,7 @@
 #include 
 
 struct ulpi;
+struct device;
 
 /**
  * struct ulpi_ops - ULPI register access
@@ -12,7 +13,6 @@ struct ulpi;
  * @write: write operation for ULPI register access
  */
 struct ulpi_ops {
-   struct device *dev;
int (*read)(struct device *dev, u8 addr);
int (*write)(struct device *dev, u8 addr, u8 val);
 };
-- 
2.7.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 2/5] usb: ulpi: change operations api to pass parent device directly

[Tal Shorer]

struct ulpi_ops is defined as follows:

struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
};

Upon calling ulpi_register_interface(), the struct device argument is
put inside the struct ulpi_ops argument's dev field. Later, when
calling the actual read()/write() operations, the struct ulpi_ops is
passed to them and they use the stored device to access whatever
private data they need.

This means that if one wishes to reuse the same oprations for multiple
interfaces (e.g if we have multiple instances of the same controller),
any but the last interface registered will not operate properly (and
the one that does work will be at the mercy of the others to not mess
it up).

Fix this by passing the given struct device directly to the operation
functions (ulpi->dev.parent) in ulpi_read() and ulpi_write() instead
of via the ops argument

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/common/ulpi.c  | 4 ++--
 drivers/usb/dwc3/ulpi.c| 8 
 include/linux/ulpi/interface.h | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index d1f419c..fdaed7c 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -21,13 +21,13 @@
 
 int ulpi_read(struct ulpi *ulpi, u8 addr)
 {
-   return ulpi->ops->read(ulpi->ops, addr);
+   return ulpi->ops->read(ulpi->dev.parent, addr);
 }
 EXPORT_SYMBOL_GPL(ulpi_read);
 
 int ulpi_write(struct ulpi *ulpi, u8 addr, u8 val)
 {
-   return ulpi->ops->write(ulpi->ops, addr, val);
+   return ulpi->ops->write(ulpi->dev.parent, addr, val);
 }
 EXPORT_SYMBOL_GPL(ulpi_write);
 
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index ec004c6..51ac939 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -35,9 +35,9 @@ static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
return -ETIMEDOUT;
 }
 
-static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
+static int dwc3_ulpi_read(struct device *dev, u8 addr)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
int ret;
 
@@ -53,9 +53,9 @@ static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
return DWC3_GUSB2PHYACC_DATA(reg);
 }
 
-static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
+static int dwc3_ulpi_write(struct device *dev, u8 addr, u8 val)
 {
-   struct dwc3 *dwc = dev_get_drvdata(ops->dev);
+   struct dwc3 *dwc = dev_get_drvdata(dev);
u32 reg;
 
reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
diff --git a/include/linux/ulpi/interface.h b/include/linux/ulpi/interface.h
index 4de8ab4..ac3cd80 100644
--- a/include/linux/ulpi/interface.h
+++ b/include/linux/ulpi/interface.h
@@ -13,8 +13,8 @@ struct ulpi;
  */
 struct ulpi_ops {
struct device *dev;
-   int (*read)(struct ulpi_ops *ops, u8 addr);
-   int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
+   int (*read)(struct device *dev, u8 addr);
+   int (*write)(struct device *dev, u8 addr, u8 val);
 };
 
 struct ulpi *ulpi_register_interface(struct device *, struct ulpi_ops *);
-- 
2.7.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

Possible design problem with struct ulpi_ops's dev field

[Tal Shorer]

struct ulpi_ops is defined as follows:

struct ulpi_ops {
struct device *dev;
int (*read)(struct ulpi_ops *ops, u8 addr);
int (*write)(struct ulpi_ops *ops, u8 addr, u8 val);
};

Upon calling ulpi_register_interface(), the struct device argument is
put inside the struct ulpi_ops argument's dev field. Later, when
calling the actual read()/write() operations, the struct ulpi_ops is
passed to them and they use the stored device to access whatever
private data they need.

This means that if one wishes to reuse the same oprations for multiple
interfaces (e.g if we have multiple instances of the same controller),
any but the last interface registered will not operate properly (and
the one that does work will be at the mercy of the others to not mess
it up).

I understand that barely any driver uses this bus right now, but I
suppose it's there to be used at some point. We might as well fix the
design here before we hit this bug.

I would like to create a patch series to fix this by passing the given
struct device directly to the operation functions via ulpi->dev.parent
in ulpi_read() and ulpi_write(), but I'm not sure about the correct
order of the patches that make such a series. Do I change the api and
all users in one patch? one patch for api and one for each user?

Any comments and guidelines are welcome :)

Respectfully,
Tal Shorer
--
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/1] usb: musb: gadget: nuke endpoint before setting its descriptor to NULL

[Tal Shorer]

Some functions, such as f_sourcesink, rely on an endpoint's desc
field during their requests' complete() callback, so clear it only
_after_ nuking all requests to avoid NULL pointer dereference.

Signed-off-by: Tal Shorer <tal.sho...@gmail.com>
---
 drivers/usb/musb/musb_gadget.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 87bd578..152865b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1164,12 +1164,12 @@ static int musb_gadget_disable(struct usb_ep *ep)
musb_writew(epio, MUSB_RXMAXP, 0);
}
 
-   musb_ep->desc = NULL;
-   musb_ep->end_point.desc = NULL;
-
/* abort all pending DMA and requests */
nuke(musb_ep, -ESHUTDOWN);
 
+   musb_ep->desc = NULL;
+   musb_ep->end_point.desc = NULL;
+
schedule_work(>irq_work);
 
spin_unlock_irqrestore(&(musb->lock), flags);
-- 
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 0/1] usb: musb: nuke endpoint before setting its descriptor to NULL

[Tal Shorer]

Hello,
I compiled and installed linux-4.6-rc3 on my beagle bone black and
noticed that when I unload a gadget using f_sourcesink (namely
g_zero), a kernel panic occurs:
[   12.531504] Unable to handle kernel NULL pointer dereference at virtual 
address 0005
[   12.540100] pgd = de6a4000
[   12.542984] [0005] *pgd=9e702831, *pte=, *ppte=
[   12.549713] Internal error: Oops: 17 [#1] SMP ARM
[   12.554713] Modules linked in: usb_f_ss_lb g_zero(-) libcomposite musb_dsps 
musb_hdrc cppi41 udc_core usbcore omap_rng rng_core musb_am335x rtc_omap 
omap_wdt cpufreq_dt thermal_sys leds_gpio hwmon led_class
[   12.574519] CPU: 0 PID: 139 Comm: modprobe Not tainted 4.6.0-rc3 #3
[   12.581165] Hardware name: Generic AM33XX (Flattened Device Tree)
[   12.587632] task: de65e400 ti: de6ce000 task.ti: de6ce000
[   12.593391] PC is at source_sink_free_instance+0x24/0xfc [usb_f_ss_lb]
[   12.600327] LR is at source_sink_complete+0x174/0x480 [usb_f_ss_lb]
[   12.606980] pc : []lr : []psr: 8093
[   12.606980] sp : de6cfe40  ip : bf0a4074  fp : 
[   12.619141] r10: de685d80  r9 : dd00a000  r8 : 2093
[   12.624688] r7 : de685d80  r6 : dd123000  r5 :   r4 : dd1804e8
[   12.631609] r3 :   r2 : bf0d2900  r1 : de5a4280  r0 : dd123000
[   12.638536] Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
none
[   12.646196] Control: 10c5387d  Table: 9e6a4019  DAC: 0051
[   12.652292] Process modprobe (pid: 139, stack limit = 0xde6ce218)
[   12.658758] Stack: (0xde6cfe40 to 0xde6d)
[   12.663409] fe40: dd1804e8 dd1804e8 de5a4280 dd123000 de685d80 2093 
dd00a000 e0b72410
[   12.672096] fe60:  bf0d6d2c 6093 dd180010 dd180010 dd1804e8 
0001 de5a4280
[   12.680781] fe80: dd180010 dd1804e8 0001 bf09900c c0c0965c bf0ad7ec 
e0b72410 dd180010
[   12.689467] fea0: dd1804e8 dd180538 dd180010 bf0b2c84 2093 bf0adf28 
dd1804e8 de685d80
[   12.698152] fec0: dd180f0c dd1804e8 c0107984 de6ce000  bf0d68f8 
dd102740 de6cff00
[   12.706838] fee0: c0107984 de685d80 dd180fec bf0d7aa8 dd180f0c dd123000 
de685d80 
[   12.715523] ff00: 0081 bf0d7b00 dd180fec bf0c4bec bf0ca000 bf0c4c18 
de685d80 de685ddc
[   12.724209] ff20: 6013 bf0c51d4 dd175800 dd1811b0 0080 bf09974c 
bf0d2a10 dd175800
[   12.732895] ff40: bf0d2a10 bf099814 bf0d247c bf0d2ac0 000af6d0 c01c5350 
 657a5f67
[   12.741579] ff60: 6f72 c0cc5d9c de65e400  de6ce000  
 c0155754
[   12.750264] ff80:  de6ce000 b6f4348c 000af6b0 0001 001078bc 
b6f4348c 000af6b0
[   12.758949] ffa0: 0001 c01077e0 b6f4348c 000af6b0 000af6d0 0080 
0001 
[   12.767633] ffc0: b6f4348c 000af6b0 0001 0081 000af638  
000af6b0 
[   12.776319] ffe0: b6eed3e8 beb1bb70 0001b160 b6eed3f4 a010 000af6d0 
9fdf6861 9fdf6c61
[   12.785029] [] (source_sink_free_instance [usb_f_ss_lb]) from 
[] (source_sink_complete+0x174/0x480 [usb_f_ss_lb])
[   12.797779] [] (source_sink_complete [usb_f_ss_lb]) from 
[] (usb_gadget_giveback_request+0xc/0x10 [udc_core])
[   12.810188] [] (usb_gadget_giveback_request [udc_core]) from 
[] (musb_g_giveback+0x118/0x614 [musb_hdrc])
[   12.822218] [] (musb_g_giveback [musb_hdrc]) from [] 
(musb_gadget_disable+0x130/0x1d8 [musb_hdrc])
[   12.833590] [] (musb_gadget_disable [musb_hdrc]) from [] 
(sourcesink_get_alt+0x3c/0x78 [usb_f_ss_lb])
[   12.845227] [] (sourcesink_get_alt [usb_f_ss_lb]) from 
[] (disable_endpoints+0x24/0x84 [usb_f_ss_lb])
[   12.856864] [] (disable_endpoints [usb_f_ss_lb]) from [] 
(disable_endpoints+0x7c/0x84 [usb_f_ss_lb])
[   12.868447] [] (disable_endpoints [usb_f_ss_lb]) from [] 
(config_ep_by_speed+0x28c/0x3ac [libcomposite])
[   12.880379] [] (config_ep_by_speed [libcomposite]) from 
[] (composite_disconnect+0x2c/0x54 [libcomposite])
[   12.892484] [] (composite_disconnect [libcomposite]) from 
[] (usb_gadget_state_work+0x90/0xf4 [udc_core])
[   12.904488] [] (usb_gadget_state_work [udc_core]) from 
[] (usb_gadget_unregister_driver+0x64/0xc4 [udc_core])
[   12.916868] [] (usb_gadget_unregister_driver [udc_core]) from 
[] (SyS_delete_module+0x11c/0x1e4)
[   12.928048] [] (SyS_delete_module) from [] 
(ret_fast_syscall+0x0/0x1c)
[   12.936829] Code: e5905080 e5933024 e3550002 e592a01c (e5d39005)
[   12.943314] ---[ end trace 87c865532163a167 ]---

I traced the issue to f_sourcesink trying to use a struct usb_ep's
dest field after it's set to NULL by musb_gadget.c

This patch fixes this problem by moving the clearing of ep->desc to
occur after calling the complete() callbacks for all requests.

Tal Shorer (1):
  usb: musb: gadget: nuke endpoint before setting its descriptor to NULL

 drivers/usb/musb/musb_gadget.c | 6 +++---
 1 file changed, 3 insertions(+), 3 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://vg

Values for SetHubDepth and GetPortErrorCount

[Tal Shorer]

In include/linux/usb/hcd.h, we have

/* class requests from USB 3.0 hub spec, table 10-5 */
#define SetHubDepth (0x3000 | HUB_SET_DEPTH)
#define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT)

However, from the usb 3.1 spec I downloaded at
http://www.usb.org/developers/docs/,
table 10-5 appears to be unrelated (Table 10-5. Enhanced SuperSpeed
Hub Descriptor).
Table 10-7 (Table 10-7. Hub Class Requests) lists the two values as:
Request bmRequestType bRequest wValue wIndex wLength Data
SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of
Link Errors on this port

It appears the correct bmRequestType values should be:
#define SetHubDepth (0x2000 | HUB_SET_DEPTH)
#define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT)

Which are the bmRequestType values of SetHubFeature and GetPortStatus
(and friends).

These lines were introduced in
commit 0eadcc09203349b11ca477ec367079b23d32ab91
Author: Tatyana Brokhman tlin...@codeaurora.org
Date:   Mon Nov 1 18:18:24 2010 +0200

usb: USB3.0 ch11 definitions

Adding hub SuperSpeed usb definitions as defined by ch10 of the USB3.0
spec.

Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
Signed-off-by: Greg Kroah-Hartman gre...@suse.de

The values are only used by dummy_hcd which does nothing with them and
by max3421-hcd which returns an error.
Can anyone clarify what I'm missing or if the values are actually wrong?
--
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: Values for SetHubDepth and GetPortErrorCount

[Tal Shorer]

On Fri, Aug 7, 2015 at 8:31 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Aug 07, 2015 at 07:56:37PM +0300, Tal Shorer wrote:
 On Fri, Aug 7, 2015 at 7:37 PM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Aug 07, 2015 at 12:49:39PM +0300, Tal Shorer wrote:
  In include/linux/usb/hcd.h, we have
 
  /* class requests from USB 3.0 hub spec, table 10-5 */
  #define SetHubDepth (0x3000 | HUB_SET_DEPTH)
  #define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT)
 
  However, from the usb 3.1 spec I downloaded at
  http://www.usb.org/developers/docs/,
  table 10-5 appears to be unrelated (Table 10-5. Enhanced SuperSpeed
  Hub Descriptor).
 
  Um, look at the comment you copied 3.0 hub spec, vs. the spec you
  downloaded and referred to 3.1 spec.  Things might have changed :)
 
 Yeah, my bad.
  Table 10-7 (Table 10-7. Hub Class Requests) lists the two values as:
  Request bmRequestType bRequest wValue wIndex wLength Data
  SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
  GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of
  Link Errors on this port
 
  It appears the correct bmRequestType values should be:
  #define SetHubDepth (0x2000 | HUB_SET_DEPTH)
  #define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT)
 
  Which are the bmRequestType values of SetHubFeature and GetPortStatus
  (and friends).
 
  These lines were introduced in
  commit 0eadcc09203349b11ca477ec367079b23d32ab91
  Author: Tatyana Brokhman tlin...@codeaurora.org
  Date:   Mon Nov 1 18:18:24 2010 +0200
 
  usb: USB3.0 ch11 definitions
 
  Adding hub SuperSpeed usb definitions as defined by ch10 of the USB3.0
  spec.
 
  Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
  Signed-off-by: Greg Kroah-Hartman gre...@suse.de
 
  The values are only used by dummy_hcd which does nothing with them and
  by max3421-hcd which returns an error.
  Can anyone clarify what I'm missing or if the values are actually wrong?
 
  Can you compare the 3.1 to the 3.0 spec and see if they changed things
  here?
 
  thanks,
 
  greg k-h

 USB 3.0 spec download from 
 http://www.usb.org/developers/docs/documents_archive/
 Table 10-6. Hub Class Requests
 GetPortErrorCount 1000B GET_PORT_ERR_COUNT Zero Port Two Number of
 Link Errors on this port
 SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None

 It appears it wasn't changed from 3.0 to 3.1.
 Looking at the code in hub.c, it appears root hubs are never called
 with SetHubDepth.

 1049 if (hdev-parent  hub_is_superspeed(hdev)) {
 1050 ret = usb_control_msg(hdev,
 usb_sndctrlpipe(hdev, 0),
 1051 HUB_SET_DEPTH, USB_RT_HUB,
 1052 hdev-level - 1, 0, NULL, 0,
 1053 USB_CTRL_SET_TIMEOUT);

 There are no more uses of HUB_SET_DEPTH except for this one and the one in 
 hcd.h

 tal@tal-H87-D3H:~/Dev/linux|0 $ git grep HUB_SET_DEPTH
 drivers/usb/core/hub.c: HUB_SET_DEPTH,
 USB_RT_HUB,
 include/linux/usb/hcd.h:#define SetHubDepth (0x3000 | 
 HUB_SET_DEPTH)
 include/uapi/linux/usb/ch11.h:#define HUB_SET_DEPTH 12

 People use SetHubDepth today.

 And HUB_GET_PORT_ERR_COUNT is never used by the hub driver:

 tal@tal-H87-D3H:~/Dev/linux|1 $ git grep HUB_GET_PORT_ERR_COUNT
 include/linux/usb/hcd.h:#define GetPortErrorCount   (0x8000 |
 HUB_GET_PORT_ERR_COUNT)
 include/uapi/linux/usb/ch11.h:#define HUB_GET_PORT_ERR_COUNT13

 So even if the constant was right, root hubs using it would never
 reach the code handling it.

 But again, GetPortErrorCount is being used.
Yes, but with the current implementation of hub.c, these uses will never
run. I don't really think it's a good idea to delete these uses, since
this feature might be implemented in the future.

 Would you prefer to eliminate these completely or make them right in
 case anyone uses them in the future?

 We should fix these if they were incorrect.

 Care to make up a patch?
Sure, I'll send it shortly.

 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

[Patch v2] usb: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

From the usb 3.1 spec available at http://www.usb.org/developers/docs/
table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
GetPortErrorCount as:

Request bmRequestType bRequest wValue wIndex wLength Data
SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of Link
Errors on this port

Fix these two values to match the spec.

Signed-off-by: Tal Shorer tal.sho...@gmail.com
---
 include/linux/usb/hcd.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..d2784c1 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -564,9 +564,9 @@ extern void usb_ep0_reinit(struct usb_device *);
 
 /*-*/
 
-/* class requests from USB 3.0 hub spec, table 10-5 */
-#define SetHubDepth(0x3000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0x8000 | HUB_GET_PORT_ERR_COUNT)
+/* class requests from USB 3.1 hub spec, table 10-7 */
+#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
+#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
2.4.6

--
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: Values for SetHubDepth and GetPortErrorCount

[Tal Shorer]

On Fri, Aug 7, 2015 at 7:37 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Aug 07, 2015 at 12:49:39PM +0300, Tal Shorer wrote:
 In include/linux/usb/hcd.h, we have

 /* class requests from USB 3.0 hub spec, table 10-5 */
 #define SetHubDepth (0x3000 | HUB_SET_DEPTH)
 #define GetPortErrorCount (0x8000 | HUB_GET_PORT_ERR_COUNT)

 However, from the usb 3.1 spec I downloaded at
 http://www.usb.org/developers/docs/,
 table 10-5 appears to be unrelated (Table 10-5. Enhanced SuperSpeed
 Hub Descriptor).

 Um, look at the comment you copied 3.0 hub spec, vs. the spec you
 downloaded and referred to 3.1 spec.  Things might have changed :)

Yeah, my bad.
 Table 10-7 (Table 10-7. Hub Class Requests) lists the two values as:
 Request bmRequestType bRequest wValue wIndex wLength Data
 SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
 GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of
 Link Errors on this port

 It appears the correct bmRequestType values should be:
 #define SetHubDepth (0x2000 | HUB_SET_DEPTH)
 #define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT)

 Which are the bmRequestType values of SetHubFeature and GetPortStatus
 (and friends).

 These lines were introduced in
 commit 0eadcc09203349b11ca477ec367079b23d32ab91
 Author: Tatyana Brokhman tlin...@codeaurora.org
 Date:   Mon Nov 1 18:18:24 2010 +0200

 usb: USB3.0 ch11 definitions

 Adding hub SuperSpeed usb definitions as defined by ch10 of the USB3.0
 spec.

 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 Signed-off-by: Greg Kroah-Hartman gre...@suse.de

 The values are only used by dummy_hcd which does nothing with them and
 by max3421-hcd which returns an error.
 Can anyone clarify what I'm missing or if the values are actually wrong?

 Can you compare the 3.1 to the 3.0 spec and see if they changed things
 here?

 thanks,

 greg k-h

USB 3.0 spec download from http://www.usb.org/developers/docs/documents_archive/
Table 10-6. Hub Class Requests
GetPortErrorCount 1000B GET_PORT_ERR_COUNT Zero Port Two Number of
Link Errors on this port
SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None

It appears it wasn't changed from 3.0 to 3.1.
Looking at the code in hub.c, it appears root hubs are never called
with SetHubDepth.

1049 if (hdev-parent  hub_is_superspeed(hdev)) {
1050 ret = usb_control_msg(hdev,
usb_sndctrlpipe(hdev, 0),
1051 HUB_SET_DEPTH, USB_RT_HUB,
1052 hdev-level - 1, 0, NULL, 0,
1053 USB_CTRL_SET_TIMEOUT);

There are no more uses of HUB_SET_DEPTH except for this one and the one in hcd.h

tal@tal-H87-D3H:~/Dev/linux|0 $ git grep HUB_SET_DEPTH
drivers/usb/core/hub.c: HUB_SET_DEPTH,
USB_RT_HUB,
include/linux/usb/hcd.h:#define SetHubDepth (0x3000 | HUB_SET_DEPTH)
include/uapi/linux/usb/ch11.h:#define HUB_SET_DEPTH 12

And HUB_GET_PORT_ERR_COUNT is never used by the hub driver:

tal@tal-H87-D3H:~/Dev/linux|1 $ git grep HUB_GET_PORT_ERR_COUNT
include/linux/usb/hcd.h:#define GetPortErrorCount   (0x8000 |
HUB_GET_PORT_ERR_COUNT)
include/uapi/linux/usb/ch11.h:#define HUB_GET_PORT_ERR_COUNT13

So even if the constant was right, root hubs using it would never
reach the code handling it.


Would you prefer to eliminate these completely or make them right in
case anyone uses them in the future?
--
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] usb: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

From the usb 3.1 spec available at http://www.usb.org/developers/docs/
table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
GetPortErrorCount as:

Request bmRequestType bRequest wValue wIndex wLength Data
SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of Link
Errors on this port

Fix these two values to match the spec.

Signed-off-by: Tal Shorer tal.sho...@gmail.com
---
 include/linux/usb/hcd.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..6a24416 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -564,9 +564,9 @@ extern void usb_ep0_reinit(struct usb_device *);
 
 /*-*/
 
-/* class requests from USB 3.0 hub spec, table 10-5 */
-#define SetHubDepth(0x3000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0x8000 | HUB_GET_PORT_ERR_COUNT)
+/* class requests from USB 3.1 hub spec, table 10-7 */
+#define SetHubDepth (0x2000 | HUB_SET_DEPTH)
+#define GetPortErrorCount (0xa300 | HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
2.4.6

--
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: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

Please ignore v2, I missed that you

On Fri, Aug 7, 2015 at 9:13 PM, Tal Shorer tal.sho...@gmail.com wrote:
 From the usb 3.1 spec available at http://www.usb.org/developers/docs/
 table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
 GetPortErrorCount as:

 Request bmRequestType bRequest wValue wIndex wLength Data
 SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
 GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of Link
 Errors on this port

 Fix these two values to match the spec.

 Signed-off-by: Tal Shorer tal.sho...@gmail.com
 ---
  include/linux/usb/hcd.h | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
 index c9aa779..d2784c1 100644
 --- a/include/linux/usb/hcd.h
 +++ b/include/linux/usb/hcd.h
 @@ -564,9 +564,9 @@ extern void usb_ep0_reinit(struct usb_device *);

  /*-*/

 -/* class requests from USB 3.0 hub spec, table 10-5 */
 -#define SetHubDepth(0x3000 | HUB_SET_DEPTH)
 -#define GetPortErrorCount  (0x8000 | HUB_GET_PORT_ERR_COUNT)
 +/* class requests from USB 3.1 hub spec, table 10-7 */
 +#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
 +#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)

  /*
   * Generic bandwidth allocation constants/support
 --
 2.4.6

--
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] usb: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

From the usb 3.1 spec available at http://www.usb.org/developers/docs/
table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
GetPortErrorCount as:

Request bmRequestType bRequest wValue wIndex wLength Data
SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of Link 
Errors on this port

Fix these two values to match the spec.

Signed-off-by: Tal Shorer tal.sho...@gmail.com
---
 include/linux/usb/hcd.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..d2784c1 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -564,9 +564,9 @@ extern void usb_ep0_reinit(struct usb_device *);
 
 /*-*/
 
-/* class requests from USB 3.0 hub spec, table 10-5 */
-#define SetHubDepth(0x3000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0x8000 | HUB_GET_PORT_ERR_COUNT)
+/* class requests from USB 3.1 hub spec, table 10-7 */
+#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
+#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
2.4.6

--
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 v3] usb: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

On Fri, Aug 7, 2015 at 9:40 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Aug 07, 2015 at 09:16:00PM +0300, Tal Shorer wrote:
 From the usb 3.1 spec available at http://www.usb.org/developers/docs/
 table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
 GetPortErrorCount as:

 Request bmRequestType bRequest wValue wIndex wLength Data
 SetHubDepth 0010B SET_HUB_DEPTH Hub Depth Zero Zero None
 GetPortErrorCount 10100011B GET_PORT_ERR_COUNT Zero Port Two Number of Link 
 Errors on this port

 Still does not look correct to me, how about you?


Not sure what you mean. This is a simple copy-paste from the spec document.
Do you want me to put quotation marks around the multi-word Data column?
--
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 v4] usb: hcd.h: Fix the values of SetHubDepth and GetPortErrorCount to match USB 3.1 specification

[Tal Shorer]

From the usb 3.1 spec available at http://www.usb.org/developers/docs/
table 10-7 (Hub Class Requests) specifies the values for SetHubDepth and
GetPortErrorCount as:

Request bmRequestType   bRequestwValue  
wIndex  wLength Data
SetHubDepth 0010B   SET_HUB_DEPTH   Hub Depth   
ZeroZeroNone
GetPortErrorCount   10100011B   GET_PORT_ERR_COUNT  Zero
PortTwo Number of Link Errors on this port

Fix these two values to match the spec.

Signed-off-by: Tal Shorer tal.sho...@gmail.com
---
 include/linux/usb/hcd.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..d2784c1 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -564,9 +564,9 @@ extern void usb_ep0_reinit(struct usb_device *);
 
 /*-*/
 
-/* class requests from USB 3.0 hub spec, table 10-5 */
-#define SetHubDepth(0x3000 | HUB_SET_DEPTH)
-#define GetPortErrorCount  (0x8000 | HUB_GET_PORT_ERR_COUNT)
+/* class requests from USB 3.1 hub spec, table 10-7 */
+#define SetHubDepth(0x2000 | HUB_SET_DEPTH)
+#define GetPortErrorCount  (0xa300 | HUB_GET_PORT_ERR_COUNT)
 
 /*
  * Generic bandwidth allocation constants/support
-- 
2.4.6

--
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: common code for CDC drivers

[Tal Shorer]

As someone who has cdc-acm enabled and usbnet disabled on my project,
I'd rather you don't break my configuration :

On Fri, Jun 26, 2015 at 4:40 PM, Oliver Neukum oneu...@suse.com wrote:
 Hi,

 I am looking at all the CDC drivers. They all have a parser for
 the extra CDC headers. If I unify this would it be acceptable
 for cdc-acm to depend on usbnet? Or should I put the code into
 a header?

 Regards
 Oliver


 --
 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
--
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: common code for CDC drivers

[Tal Shorer]

It won't break, but this will make me compile something I don't need
and may not want.
Wouldn't it make more sense to put this unified code in a new file
(maybe drivers/usb/class/cdc.c) and have all related drivers depend on
that instead of on usbnet?
The less code that is compiled but never runs, the happier I am.

On Fri, Jun 26, 2015 at 7:59 PM, Greg KH g...@kroah.com wrote:
 On Fri, Jun 26, 2015 at 07:47:47PM +0300, Tal Shorer wrote:
 As someone who has cdc-acm enabled and usbnet disabled on my project,
 I'd rather you don't break my configuration :

 configurations will not break, they will be updated to pull in usbnet if
 needed.

 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] usb: ulpi: don't register drivers if bus doesn't exist

[Tal Shorer]

Why do we even need that? If you take patch that makes ulpi_init a
subsys_initcall you won't have this problem, and no additional weird
hacks and errors will be needed

On Sun, May 24, 2015 at 11:09 AM, Sudip Mukherjee
sudipm.mukher...@gmail.com wrote:
 On Sun, May 24, 2015 at 12:19:48AM -0700, Greg KH wrote:
 On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote:
  ULPI registers it's bus at module_init so if the bus fails to register, the
  module will fail to load and all will be well in the world.
 
  However, if the ULPI code is built-in rather than a module, the bus
  initialization may fail but we'd still try to register drivers later onto
  a non-existant bus, which will panic the kernel.
 
  Fix that by checking that the bus was indeed initialized before trying to
  register drivers on top of it.
 
  Signed-off-by: Sasha Levin sasha.le...@oracle.com
  ---
   drivers/usb/common/ulpi.c |4 
   1 file changed, 4 insertions(+)
 
  diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
  index 0e6f968..0b0a5e7 100644
  --- a/drivers/usb/common/ulpi.c
  +++ b/drivers/usb/common/ulpi.c
  @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
  if (!drv-probe)
  return -EINVAL;
 
  +   /* Was the bus registered successfully? */
  +   if (!ulpi_bus.p)
  +   return -ENODEV;

 Ick, no, don't go mucking around in the bus internals like this, that's
 not ok.  You should either know the bus is registered, or something is
 really wrong with the design here.
 can't we use a variable which can be initialized to 1 in ulpi_init() if
 the bus registers successfully and later check that?

 regards
 sudip

 greg k-h
 --
 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
--
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: don't build PCI quirks if USB support isn't configured

[Tal Shorer]

Why is drivers/usb/Makefile even read when CONFIG_USB is not set?

Won't it make more sense to throw the line
obj-$(CONFIG_PCI) += usb/
from drivers/Makfile?

On Wed, Apr 22, 2015 at 7:11 PM, Alan Stern st...@rowland.harvard.edu wrote:
 The USB PCI quirks code gets built into the kernel whenever CONFIG_PCI
 is enabled, even if CONFIG_USB is not set.  This can cause unnecessary
 messages to show up in the kernel log, such as CONFIG_USB_XHCI_HCD is
 turned off, defaulting to EHCI (which makes no sense when the kernel
 has been configured without host-side USB support).

 This patch addresses the problem by building pci-quirks.o only when
 CONFIG_PCI and CONFIG_USB are both enabled.

 Signed-off-by: Alan Stern st...@rowland.harvard.edu
 Reported-by: Toralf Förster toralf.foers...@gmx.de

 ---

 Ver. 2: My earlier approach, removing the dependency from CONFIG_PCI
 to drivers/usb/host/, doesn't work when CONFIG_PCI=y and CONFIG_USB=m.
 This is because pci-quirks.o will not be available to be linked into
 the kernel image, since nothing will cause make to descend into the
 host/ directory when building the main kernel.


 [as1778b]


  drivers/usb/host/Makefile |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 Index: usb-4.0/drivers/usb/host/Makefile
 ===
 --- usb-4.0.orig/drivers/usb/host/Makefile
 +++ usb-4.0/drivers/usb/host/Makefile
 @@ -24,7 +24,9 @@ endif

  obj-$(CONFIG_USB_WHCI_HCD) += whci/

 -obj-$(CONFIG_PCI)  += pci-quirks.o
 +ifneq ($(CONFIG_USB), )
 +   obj-$(CONFIG_PCI)   += pci-quirks.o
 +endif

  obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
  obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o

 --
 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
--
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 4/4] Documentation: ABI: Fix documentation for mass_storage function

[Tal Shorer]

On Wed, Apr 8, 2015 at 3:06 PM, Krzysztof Opasiak k.opas...@samsung.com wrote:
 Luns in mass storage function are identified using their id.
 While creating lun's directory user cannot choose any arbitrary
 name other than arabic numeral from 1 to FSG_MAX_LUNS.

 Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
 ---
  .../ABI/testing/configfs-usb-gadget-mass-storage   |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

 diff --git a/Documentation/ABI/testing/configfs-usb-gadget-mass-storage 
 b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
 index 9931fb0..0b54280 100644
 --- a/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
 +++ b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
 @@ -11,10 +11,15 @@ Description:
 are 2..4. Available only if
 CONFIG_USB_GADGET_DEBUG_FILES is set.

 -What:  /config/usb-gadget/gadget/functions/mass_storage.name/lun.name
 +What:  /config/usb-gadget/gadget/functions/mass_storage.name/lun.id
  Date:  Oct 2013
  KernelVersion: 3.13
  Description:
 +   id - arabic numeral from 1 to FSG_MAX_LUNS
I think decimal number or decimal value would be easier to understand.
 +   (which is 8 by default) - 1. LUNs should be numbered 
 contiguously.
 +   lun.0 is reserved for default lun which appears while creating
 +   mass_storage.name directory and cannot be removed by the user.
 +
 The attributes:

 file- The path to the backing file for the LUN.
 --
 1.7.9.5

 --
 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
--
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: Question: drivers/usb/serial/generic.c: usb_serial_generic_read_bulk_callback()

[Tal Shorer]

Wild guess, you don't see the same behavior on your desktop machine
because it's a different hcd that sets the return value.
What happens when you

On Tue, Apr 7, 2015 at 9:57 PM, Mark Enstone mark.enst...@silabs.com wrote:
 linux-usb,

 Apologies if I've come at this the wrong way, I've read MAINTAINERS and 
 usb-serial.txt to get your name.


 Kernel 3.15 added logic to usb_serial_generic_read_bulk_callback() to 
 resubmit read URBs in the general/default fail case (for failures other than 
 urb-status = ENOENT, ECONNRESET, ESHUTDOWN. Also EPIPE).

 On Raspberry Pi, running a 3.18 kernel, I am seeing -EPROTO as the 
 urb-status  in usb_serial_generic_read_bulk_callback()  when the last CP210X 
 USB-UART device is removed from a hub (the hub remains attached to the 
 Raspberry Pi).  ((removing a non-last CP210X from the hub, does not return 
 EPROTO, does return one of the specifically identified errnos)).  The URB is 
 retried, which fails again with EPROTO, is retried, fails in the same way 
 again, etc. forever.

 I am not seeing this same behavior on a similarly configured Dell Optiplex 
 desktop.  Thus far, only on RPi.


 Question: have you seen such behavior? Had you considered adding EPROTO to 
 the list of urb-status errnos failed rather than retried?   If I do fail, 
 rather than retry, the -EPROTO-failed URB, I get my expected results.  I do 
 not know what the correct approach is, hoped you could opine:  should I add 
 EPROTO as a fail rather than as a retry error in 
 usb_serial_generic_read_bulk_callback() (this seems to fix my problem)?  Or 
 should I investigate why/where the EPROTO is coming from vs. why it is not, 
 say, ENOENT (it does not seem unreasonable to retry an EPROTO IFF EPROTO is 
 considered transient, however my test case it seems to get stuck at EPROTO)?

 Thoughts appreciated, thank you,

 ~Mark

 Mark Enstone
 We are stuck with technology when what we really want is just stuff that 
 works --- DNA

 --
 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
--
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: Question: drivers/usb/serial/generic.c: usb_serial_generic_read_bulk_callback()

[Tal Shorer]

Stupid mail client sent my message mid-writing. Sorry.
What happens when you remove a last-non-CP210X device from the hub?

On Tue, Apr 7, 2015 at 10:39 PM, Tal Shorer tal.sho...@gmail.com wrote:
 Wild guess, you don't see the same behavior on your desktop machine
 because it's a different hcd that sets the return value.
 What happens when you

 On Tue, Apr 7, 2015 at 9:57 PM, Mark Enstone mark.enst...@silabs.com wrote:
 linux-usb,

 Apologies if I've come at this the wrong way, I've read MAINTAINERS and 
 usb-serial.txt to get your name.


 Kernel 3.15 added logic to usb_serial_generic_read_bulk_callback() to 
 resubmit read URBs in the general/default fail case (for failures other than 
 urb-status = ENOENT, ECONNRESET, ESHUTDOWN. Also EPIPE).

 On Raspberry Pi, running a 3.18 kernel, I am seeing -EPROTO as the 
 urb-status  in usb_serial_generic_read_bulk_callback()  when the last 
 CP210X USB-UART device is removed from a hub (the hub remains attached to 
 the Raspberry Pi).  ((removing a non-last CP210X from the hub, does not 
 return EPROTO, does return one of the specifically identified errnos)).  The 
 URB is retried, which fails again with EPROTO, is retried, fails in the same 
 way again, etc. forever.

 I am not seeing this same behavior on a similarly configured Dell Optiplex 
 desktop.  Thus far, only on RPi.


 Question: have you seen such behavior? Had you considered adding EPROTO to 
 the list of urb-status errnos failed rather than retried?   If I do fail, 
 rather than retry, the -EPROTO-failed URB, I get my expected results.  I do 
 not know what the correct approach is, hoped you could opine:  should I add 
 EPROTO as a fail rather than as a retry error in 
 usb_serial_generic_read_bulk_callback() (this seems to fix my problem)?  Or 
 should I investigate why/where the EPROTO is coming from vs. why it is not, 
 say, ENOENT (it does not seem unreasonable to retry an EPROTO IFF EPROTO is 
 considered transient, however my test case it seems to get stuck at 
 EPROTO)?

 Thoughts appreciated, thank you,

 ~Mark

 Mark Enstone
 We are stuck with technology when what we really want is just stuff that 
 works --- DNA

 --
 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
--
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: musb: add softconnect for host mode

[Tal Shorer]

Mostly out of curiosity, why a debugfs interface and not an attribute in sysfs?
--
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

Why is USB_DT_HUB not used?

[Tal Shorer]

As far as I can grep, the only hcd that uses the named constants
USB_DT_HUB and USB_DT_SS_HUB is xhci.
even dummy_hcd which uses USB_DT_SS_HUB to check that uscore asked for
a superspeed hub descriptor still uses the numeric value when filling
the buffer.
Was this just overlooked? Will a patch series to fix this be welcome?
--
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] usb: gadget: f_mass_storage: use defined constant instead of numeric value

[Tal Shorer]

replace numeric value with TYPE_NO_LUN (defined in scsi/scsi.h)

Signed-off-by: Tal Shorer tal.sho...@gmail.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c 
b/drivers/usb/gadget/function/f_mass_storage.c
index 811929c..6d5ca2b 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -1085,7 +1085,7 @@ static int do_inquiry(struct fsg_common *common, struct 
fsg_buffhd *bh)
if (!curlun) {  /* Unsupported LUNs are okay */
common-bad_lun_okay = 1;
memset(buf, 0, 36);
-   buf[0] = 0x7f;  /* Unsupported, no device-type */
+   buf[0] = TYPE_NO_LUN;   /* Unsupported, no device-type */
buf[4] = 31;/* Additional length */
return 36;
}
-- 
2.2.2

--
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] usb: gadget: f_mass_storage: use defined constant instead of numeric value

[Tal Shorer]

Signed-off-by: Tal Shorer tal.sho...@gmail.com
---
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c 
b/drivers/usb/gadget/function/f_mass_storage.c
index 811929c..6d5ca2b 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -1085,7 +1085,7 @@ static int do_inquiry(struct fsg_common *common, struct 
fsg_buffhd *bh)
if (!curlun) {  /* Unsupported LUNs are okay */
common-bad_lun_okay = 1;
memset(buf, 0, 36);
-   buf[0] = 0x7f;  /* Unsupported, no device-type */
+   buf[0] = TYPE_NO_LUN;   /* Unsupported, no device-type */
buf[4] = 31;/* Additional length */
return 36;
}
-- 
2.2.2

--
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