RE: [PATCH v13 03/10] usbip: exporting devices: new connect operation

[fx IWATA NOBUO]

Hello,

> Can client use the same import over vpn connection

Yes.
Using VPN, the firewall problem doesn't need to be concerned.

To build proprietary virtual network in internet, VPN can be used.
The goal #1 will give flexibility for various kind of internet or
cloud service.

Using VPN, goal #2 "To improve usability of operations" can be useful.

> or some sort of authorized mechanism to access to the server?

Yes.
USB/IP already has TCP wrapper option.
When using with SSL or WebSocket(wss) tunneling proxy, client and
Server authentication can be used.

> I want know a clear use-case definition of why the current client pull
> model to import devices doesn't work. I understand firewalls pose some
> issues. However, it doesn't sound right that server initiates export
> and contacts the client.

Does the response to 00/00 comments and answers above make clear?

> What happens when client doesn't have access to the information on the
> server.

I couldn't catch this question especially 'the information'.

> Also the above doesn't look right without email address.

OK. I will add email address.

> Please don't add new Copyright to an existing file.

Do you mean not to add to existing files if it doesn't have certain
changes?

If so, I will check and fix again.

Here's an example of copyright in a file.

 * Copyright (C) 2011 matt mooney 
 *   2005-2007 Takahiro Hirofuchi
 * Copyright (C) 2015-2016 Samsung Electronics
 *   Igor Kotrasinski 
 *   Krzysztof Opasiak 

Ver.1 of this patch set is older than Samsung's vudc patches so I added
above regarding the year order.

Please, let me know if it's not good. I will move it below.

Thank you,

nobuo.iwata
//
> -Original Message-
> From: Shuah Khan [mailto:shuah...@samsung.com]
> Sent: Thursday, November 24, 2016 6:52 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO; Shuah Khan
> Subject: Re: [PATCH v13 03/10] usbip: exporting devices: new connect
> operation
> 
> On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> > Implementation of new connect operation. This is linked as a part of
> > usbip commnad. With this patch, usbip command has following operations.
> >
> >  bind
> >  unbind
> >  list (local/remote)
> >  attach
> >  detach
> >  port
> >  connect ... this patch
> 
> I am finding it hard to follow these changes without the server and client
> side details. A few comments below:
> 
> I want know a clear use-case definition of why the current client pull model
> to import devices doesn't work. I understand firewalls pose some issues.
> However, it doesn't sound right that server initiates export and contacts
> the client. What happens when client doesn't have access to the information
> on the server.
> 
> Can client use the same import over vpn connection or some sort of authorized
> mechanism to access to the server?
> 
> >
> > In device side node, this binds a device internally, sends an export
> > request and receives export response. The definition of the request
> > and response are defined in original code:
> > tools/usb/usbip/usbip_network.h but was not used. They are corresponding
> to NEW-3 in following diagram.
> >
> > EXISTING) - invites devices from application(vhci)-side
> >  +--+
> +--+
> >  device--+ STUB | | application/VHCI |
> >  +--+
> +--+
> >  1) usbipd ... start daemon
> >  = = =
> >  2) usbip list --local
> >  3) usbip bind
> > <--- list bound devices ---   4) usbip list --remote
> > <--- import a device --   5) usbip attach
> >  = = =
> >X disconnected 6) usbip detach
> >  7) usbip unbind
> >
> > NEW) - dedicates devices from device(stb)-side
> >  +--+
> +--+
> >  device--+ STUB | | application/VHCI |
> >  +--+
> +--+
> >   1) usbipa ... start
> > daemon  = = =
> >  2) usbip list --local
> >  3) usbip connect --- export a device -->
> >  = = =
> >  4) usbip disconnect  --- un-export a device --->
> >
> > Signed-off-by: Nobuo Iwata 
> > ---
> >  tools/usb/usbip/src/Makefile.am |   3 +-
> >  tools/usb/usbip/src/usbip.c |   9 +-
> >  tools/usb/usbip/src/usbip.h |   5 +-
> >  tools/usb/usbip/src/usbip_connect.c | 228
> > 
> >  4 files changed, 242 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/usb/usbip/src/Makefile.am
> > b/tools/usb/usbip/src/Makefile.am index e81a4eb..0947476 100644
> > --- a/tools/usb/usbip/src/Makefile.am
> > +++ b/tools/usb/usbip/src/Makefile.am
> > @@ -6,6 +6,7 @@ sbin_PROGRAMS := usbip usbipd
> >
> >  usbip_SOURCES := usbip.h utils.h 

xhci halted endpoint is not cleared

[David]

I encounter the following problem with a USB3 camera device using
kernel version 4.8.10 but also with previous kernel versions.

The camera device is transmitting image data via endpoint 0x82 to the
host. If the data from the device could not be transmitted in time the
device is going to stall the endpoint. When the stalled endpoint
condition is detected by our userspace driver we cancel all previously
submitted URBs. Afterwards a 'Clear Feature' is submitted to clear the
halt condition for this endpoint and then the transfer of the image
data is restarted.

With the enabled xhci module debug messages I see the following
messages and the transfer is successfully resumed:

---
kernel: xhci_hcd :00:14.0: Stalled endpoint
kernel: xhci_hcd :00:14.0: Cleaning up stalled endpoint ring
---

But sometimes I run into the problem that there could no longer any
data via endpoint 0x82 be received. If I refer to the debug message
from the xhci module I see the warning that I'm queueing URBs to an
already halted endpoint. But also the message that the reset of the
callback has been previously called.

If I compare the debug message to the successful message I see the
following output:

---
kernel: xhci_hcd :00:14.0: Stalled endpoint
kernel: xhci_hcd :00:14.0: event_trb is a no-op TRB. Skip it
---

Leading to the following message when the URBs are submitted.

kernel: xhci_hcd :00:14.0: Endpoint 0x82 ep reset callback called
kernel: xhci_hcd :00:14.0: WARN halted endpoint, queueing URB anyway.
kernel: xhci_hcd :00:14.0: WARN halted endpoint, queueing URB anyway.

Is this an error of the xhci module? I do think that cleaning up the
stalled endpoint is something which should be done by the xhci module.
If I understand the flow correctly the endpoint is cleaned up
immediately but in this case it is skipped. Is this a correct
behaviour? What have to be done to correctly clean up the stalled
endpoint?

Furthermore enabling the debug messages of the xhci module seems to
reduce the chance of this error to occur.

Here is an excerpt from the xhci debug messages of this stall condition:

---
kernel: xhci_hcd :00:14.0: Finding endpoint context
kernel: xhci_hcd :00:14.0: Cycle state = 0x0
kernel: xhci_hcd :00:14.0: New dequeue segment = 8fb02406c8c0 (virtual)
kernel: xhci_hcd :00:14.0: New dequeue pointer = 0x2dd985de0 (DMA)
kernel: xhci_hcd :00:14.0: Set TR Deq Ptr cmd, new deq seg =
8fb02406c8c0 (0x2dd985000 dma), new deq ptr = 8fb05d985de0
(0x2dd985de0 dma), new cycle = 0
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Successful Set TR Deq Ptr cmd, deq = @2dd985de0
kernel: xhci_hcd :00:14.0: Cancel URB 8faffb76f600, dev 5, ep
0x82, starting at offset 0x2dd985de0
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Stopped on Transfer TRB
kernel: xhci_hcd :00:14.0: Removing canceled TD starting at
0x2dd985de0 (dma).
kernel: xhci_hcd :00:14.0: Finding endpoint context
kernel: xhci_hcd :00:14.0: Cycle state = 0x0
kernel: xhci_hcd :00:14.0: New dequeue segment = 8fb02406c8c0 (virtual)
kernel: xhci_hcd :00:14.0: New dequeue pointer = 0x2dd985df0 (DMA)
kernel: xhci_hcd :00:14.0: Set TR Deq Ptr cmd, new deq seg =
8fb02406c8c0 (0x2dd985000 dma), new deq ptr = 8fb05d985df0
(0x2dd985df0 dma), new cycle = 0
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Cancel URB 8faffb76fa80, dev 5, ep
0x82, starting at offset 0x2dd985df0
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Successful Set TR Deq Ptr cmd, deq = @2dd985df0
kernel: xhci_hcd :00:14.0: Removing canceled TD starting at
0x2dd985df0 (dma).
kernel: xhci_hcd :00:14.0: TRB to noop at offset 0x2dd985df0
kernel: xhci_hcd :00:14.0: Cancel URB 8fb08f5b6f00, dev 5, ep
0x82, starting at offset 0x2dd985e00
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Removing canceled TD starting at
0x2dd985e00 (dma).
kernel: xhci_hcd :00:14.0: TRB to noop at offset 0x2dd985e00
kernel: xhci_hcd :00:14.0: Stalled endpoint
kernel: xhci_hcd :00:14.0: event_trb is a no-op TRB. Skip it
kernel: xhci_hcd :00:14.0: ep 0x81 - asked for 4096 bytes, 4088
bytes untransferred
kernel: xhci_hcd :00:14.0: Cancel URB 8fb08f5b63c0, dev 5, ep
0x82, starting at offset 0x2dd985e10
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Removing canceled TD starting at
0x2dd985e10 (dma).
kernel: xhci_hcd :00:14.0: TRB to noop at offset 0x2dd985e10
kernel: xhci_hcd :00:14.0: Cancel URB 8fb08f5b6840, dev 5, ep
0x82, starting at offset 0x2dd985e20
kernel: xhci_hcd :00:14.0: // Ding dong!
kernel: xhci_hcd :00:14.0: Removing canceled TD starting at
0x2dd985e20 (dma).
kernel: xhci_hcd :00:14.0: TRB to noop at offset 0x2dd985e20
kernel: xhci_hcd :00:14.0: Cancel URB 8fb08f5b60c0, dev 5, ep
0x82, starting 

RE: [PATCH v13 02/10] usbip: exporting devices: modifications to host side libraries

[fx IWATA NOBUO]

Hello,

> This doesn't look like a simple change to rename and reuse an unused
> function. This patch does lot more and is changing the user interface.
> Looks like instead of taking an integer value for device lookup, you
> are changing it to char *.
> 
> Any reason why you have to change the user interface to go to char
> *busid?
> 
> I would like to see a good explanation why this user interface change
> is necessary.

I can't figure out why the second argument was int because it was
unused.

usbip_get_device() is a stub-side (usb-host) function.
As you know, only port number in vhci-side can be int.
Others are bus-id.
Furthermore, the unused code searches list node position. It doesn't
make sense.

Here, this patch set needs to find bound device with bus-id in
stub-side.

I may add explanation above to change log.
Or I can change new function name like usbip_find_device() and leave the
unused function once.

Please, let me know how should I do.

Best Regards,

nobuo.iwata
//
> -Original Message-
> From: Shuah Khan [mailto:shuah...@samsung.com]
> Sent: Thursday, November 24, 2016 6:02 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO; Shuah Khan
> Subject: Re: [PATCH v13 02/10] usbip: exporting devices: modifications to
> host side libraries
> 
> On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> > usbip_get_device() method in usbip_host_driver_ops was not used. It is
> > modified as a function to find an exported device for new operations
> > 'connect' and 'disconnect'.
> >
> > bind and unbind function are exported to reuse by new connect and
> > disconnect operation.
> 
> This doesn't look like a simple change to rename and reuse an unused function.
> This patch does lot more and is changing the user interface.
> Looks like instead of taking an integer value for device lookup, you are
> changing it to char *.
> 
> Any reason why you have to change the user interface to go to char *busid?
> 
> I would like to see a good explanation why this user interface change is
> necessary.
> 
> thanks,
> -- Shuah
> 
> >
> > Here, connect and disconnect is NEW-3 and NEW-4 respactively in
> > diagram below.
> >
> > EXISTING) - invites devices from application(vhci)-side
> >  +--+
> +--+
> >  device--+ STUB | | application/VHCI |
> >  +--+
> +--+
> >  1) usbipd ... start daemon
> >  = = =
> >  2) usbip list --local
> >  3) usbip bind
> > <--- list bound devices ---   4) usbip list --remote
> > <--- import a device --   5) usbip attach
> >  = = =
> >X disconnected 6) usbip detach
> >  7) usbip unbind
> >
> > NEW) - dedicates devices from device(stb)-side
> >  +--+
> +--+
> >  device--+ STUB | | application/VHCI |
> >  +--+
> +--+
> >   1) usbipa ... start
> > daemon  = = =
> >  2) usbip list --local
> >  3) usbip connect --- export a device -->
> >  = = =
> >  4) usbip disconnect  --- un-export a device --->
> >
> >  Bind and unbind are done in connect and disconnect internally.
> >
> > Signed-off-by: Nobuo Iwata 
> > ---
> >  tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++
> > tools/usb/usbip/libsrc/usbip_host_common.h | 8 
> >  tools/usb/usbip/src/usbip.h| 3 +++
> >  tools/usb/usbip/src/usbip_bind.c   | 4 ++--
> >  tools/usb/usbip/src/usbip_unbind.c | 4 ++--
> >  5 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
> > b/tools/usb/usbip/libsrc/usbip_host_common.c
> > index 9d41522..6a98d6c 100644
> > --- a/tools/usb/usbip/libsrc/usbip_host_common.c
> > +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
> > @@ -256,17 +256,15 @@ int usbip_export_device(struct
> > usbip_exported_device *edev, int sockfd)  }
> >
> >  struct usbip_exported_device *usbip_generic_get_device(
> > -   struct usbip_host_driver *hdriver, int num)
> > +   struct usbip_host_driver *hdriver, char *busid)
> >  {
> > struct list_head *i;
> > struct usbip_exported_device *edev;
> > -   int cnt = 0;
> >
> > list_for_each(i, >edev_list) {
> > edev = list_entry(i, struct usbip_exported_device, node);
> > -   if (num == cnt)
> > +   if (!strncmp(busid, edev->udev.busid,
> SYSFS_BUS_ID_SIZE))
> > return edev;
> > -   cnt++;
> > }
> >
> > return NULL;
> > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h
> > b/tools/usb/usbip/libsrc/usbip_host_common.h
> > index a64b803..f9a9def 100644
> > --- a/tools/usb/usbip/libsrc/usbip_host_common.h
> > +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
> > @@ -38,7 +38,7 @@ struct 

Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Guenter Roeck]

Hello Heikki,

On 11/22/2016 06:11 AM, Heikki Krogerus wrote:
[ ... ]

+
+struct typec_port *typec_register_port(struct device *dev,
+  const struct typec_capability *cap)
+{
+   struct typec_port *port;
+   int ret;
+   int id;
+
+   port = kzalloc(sizeof(*port), GFP_KERNEL);
+   if (!port)
+   return ERR_PTR(-ENOMEM);
+
+   id = ida_simple_get(_index_ida, 0, 0, GFP_KERNEL);
+   if (id < 0) {
+   kfree(port);
+   return ERR_PTR(id);
+   }
+
+   port->prefer_role = TYPEC_NO_PREFERRED_ROLE;
+


Following up on this:

In our implementation, the default preferred role is determined by the
low level driver (as, in my understanding, is suggested by the standard).
This means that the ABI will report "no preferred role", unless user space
overwrites it, even though there _is_ in fact a preferred role, and the
low level driver will execute try.src or try.snk based on that role.

It might make sense to add the preferred role to struct typec_capability
and get the initial value from there. If a low level driver does not want
to specify it, it can easily set its value to TYPEC_NO_PREFERRED_ROLE.

Thanks,
Guenter

--
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 v13 00/10] usbip: exporting devices

[fx IWATA NOBUO]

Hello,

> Can you elaborate on the use-case a bit more? What does it mean to
> "Connection from device side is needed"?

I'd like to update ending part of use case as following.
---
Firewall, proxy, or router in front of internet usually blocks
connections from internet regarding all TCP ports. They opens some
ports, usually HTTP(80) and HTTPS(443), for connection from inside.
In combination with WebSocket proxy, USB/IP can establish connection
from inside of the firewall.

Enterprise/SOHO/Home   Firewall/Proxy/Router   Internet

EXISTING)
APP# usbip attach ---(passed)> DEV# usbipd
DEV# usbipd (blocked)|| <- APP# usbip attach

NEW)
DEV# usbip connect --(passed)> DEV# usbipa
APP# usbipa (blocked)|| <- APP# usbip connect

Attach operation can invite devices in internet but cannot invite
devices from internet. On the other hand, connect operation can dedicate
devices to internet but cannot dedicate devices in internet.
---

> I would like to see server side and client side operations clearly.
> It would help me understand the use-case you are trying to add.

It's shown in README and manual page added by patch 9/10 as below.

app:# insmod usbip-core.ko
app:# insmod vhci-hcd.ko

app:# usbipa -D
- Start usbip daemon.

dev:# (Physically attach your USB device.)

dev:# insmod usbip-core.ko
dev:# insmod usbip-host.ko

dev:# usbip list -l
- List driver assignments for USB devices and their busid.

dev:# usbip connect --remote  --busid 
- Bind usbip-host.ko to the device with .
- The USB device of  is connected to remote host!

dev:# usbip disconnect --remote  --busid 
- The USB device with  is disconnected from remote host.
- Unbind usbip-host.ko from the device.

> I do have some concerns about security on client side. User sits on
> the client side and it should be a pull from client side as opposed to
> push from server side.

Connection level consideration is described in "5. Security
 Consideration". As you know, USB/IP already has TCP wrapper option.
With SSL or WebSocket(wss) tunneling proxy, client authentication can
be applied using the proxies.

I didn't write device level consideration. It's same as local device
plug-and-play. I will add description below in cover letter and README.

---
Udev rules can allow only known devices. To identify a device is remote,
the local bus-id (KERNEL parameter in the rule) will be found the last
in column of /sys/devices/platform/vhci_hcd/status[.N]. When device is
found, the port number in USB/IP can be found in the first column of the
matched line. The udev can finish the connection using detach operation
with the port number.
---

> It sounds like this patch series adds push from server side.

Yes.
But it's only the initiation request and response.
URBs direction is just same as before. Applications send submit
and unlink, devices process them.

Your comment is at goal #1. I'm also considering goal #2.

Thanks for reviewing,

nobuo.iwata
//
> -Original Message-
> From: Shuah Khan [mailto:shuah...@samsung.com]
> Sent: Thursday, November 24, 2016 1:43 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO; Shuah Khan; Shuah Khan
> Subject: Re: [PATCH v13 00/10] usbip: exporting devices
> 
> On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> > Dear all,
> >
> > This series of patches adds exporting device operation to USB/IP.
> >
> > NOTE:
> > This patch set modifies only userspace codes in tools/usb/usbip.
> >
> > 1. Goal
> >
> > 1-1) To give flexibility to direction of connection Using USB/IP in
> > internet, there can be two cases.
> > a) an application is inside firewall and devices are outside.
> > b) devices are inside firewall and an application is inside.
> > In case-a, import works because the connection is from inside.
> > In case-b, import doesn't works because the connection is from outside.
> > Connection from device side is needed. This patch set adds the
> > direction of connection establishment.
> >
> 
> Can you elaborate on the use-case a bit more? What does it mean to "Connection
> from device side is needed"?
> 
> I would like to see server side and client side operations clearly.
> It would help me understand the use-case you are trying to add.
> 
> I do have some concerns about security on client side. User sits on the
> client side and it should be a pull from client side as opposed to push
> from server side.
> 
> It sounds like this patch series adds push from server side.
> 
> thanks,
> -- Shuah
> 
> > NOTE:
> > Directions of URBs requests and responses are not changed. Only
> > direction of connection establishment initiated with usbip command is
> > added to exsiting one.
> >
> > 1-2) To improve usability of operations When two Linux machines are in
> > a small distance, it's OK to bind (makes
> > importable) 

Re: [PATCH 0/2] usb: musb: fix bogus rx endpoint interrupt

[Bin Liu]

On Wed, Nov 23, 2016 at 10:43:32PM -0600, Bin Liu wrote:
> Hi,
> 
> This fixes a long standing musb bogus rx interrupt problem. I am not sure on
> other platforms, but on AM335x with CPPI DMA enabled, occasionally any of the
> following kernel messages shows up from musb driver. (The endpoint number is
> random, of cause.)
> 
> musb_host_rx 1853: BOGUS RX2 ready, csr , count 0
> 
> musb_host_rx 1936: RX3 dma busy, csr 2020
> 
> In one of the test cases with FT4232H I observed the issue when sometimes
> closing the uart port. It turns out the issue is that during handling urb
> dequeue, the controller is still receiving data while the CPPI channel is
> already aborted, then musb core generates endpoint rx interrupt, instead of
> rx dma interrupt in normal rx transfer.
rx dma interrupt _as_ in normal rx transfer.

> 
> Based on the inline comments, the fix is to call the new platform ops to clear
> the rx ep interrupt in musb_cleanup_urb().
> 
> I am not sure if this fix should be back ported to all stable trees, but I
> only tested up to v4.1, so cc'd stable v4.1+.
> 
> Regards,
> -Bin.
> ---
> 
> Bin Liu (2):
>   usb: musb: core: add clear_ep_rxintr() to musb_platform_ops
>   usb: musb: dsps: implement clear_ep_rxintr() callback
> 
>  drivers/usb/musb/musb_core.h |  7 +++
>  drivers/usb/musb/musb_dsps.c | 12 
>  drivers/usb/musb/musb_host.c | 10 --
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> -- 
> 1.9.1
> 
> --
> 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

[PATCH 1/2] usb: musb: core: add clear_ep_rxintr() to musb_platform_ops

[Bin Liu]

During dma teardown for dequque urb, musb might generate bogus rx ep
interrupt even when the rx fifo is flushed. As mentioned in the current
inline comment, clearing ep interrupt in the teardown path avoids the
bogus interrupt.

Before this change, any of the follow log messages could happen when
musb load is high.

musb_host_rx 1853: BOGUS RX2 ready, csr , count 0

musb_host_rx 1936: RX3 dma busy, csr 2020

cc: sta...@vger.kernel.org # 4.1+
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_core.h |  7 +++
 drivers/usb/musb/musb_host.c | 10 --
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index ed5e354b0891..deb64fe896b2 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -216,6 +216,7 @@ struct musb_platform_ops {
void(*pre_root_reset_end)(struct musb *musb);
void(*post_root_reset_end)(struct musb *musb);
int (*phy_callback)(enum musb_vbus_id_status status);
+   void(*clear_ep_rxintr)(struct musb *musb, int epnum);
 };
 
 /*
@@ -619,6 +620,12 @@ static inline void 
musb_platform_post_root_reset_end(struct musb *musb)
musb->ops->post_root_reset_end(musb);
 }
 
+static inline void musb_platform_clear_ep_rxintr(struct musb *musb, int epnum)
+{
+   if (musb->ops->clear_ep_rxintr)
+   musb->ops->clear_ep_rxintr(musb, epnum);
+}
+
 /*
  * gets the "dr_mode" property from DT and converts it into musb_mode
  * if the property is not found or not recognized returns MUSB_OTG
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 30d394b3f2f6..613009abd766 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2374,12 +2374,11 @@ static int musb_cleanup_urb(struct urb *urb, struct 
musb_qh *qh)
int is_in = usb_pipein(urb->pipe);
int status = 0;
u16 csr;
+   struct dma_channel  *dma = NULL;
 
musb_ep_select(regs, hw_end);
 
if (is_dma_capable()) {
-   struct dma_channel  *dma;
-
dma = is_in ? ep->rx_channel : ep->tx_channel;
if (dma) {
status = ep->musb->dma_controller->channel_abort(dma);
@@ -2395,10 +2394,9 @@ static int musb_cleanup_urb(struct urb *urb, struct 
musb_qh *qh)
/* giveback saves bulk toggle */
csr = musb_h_flush_rxfifo(ep, 0);
 
-   /* REVISIT we still get an irq; should likely clear the
-* endpoint's irq status here to avoid bogus irqs.
-* clearing that status is platform-specific...
-*/
+   /* clear the endpoint's irq status here to avoid bogus irqs */
+   if (is_dma_capable() && dma)
+   musb_platform_clear_ep_rxintr(musb, ep->epnum);
} else if (ep->epnum) {
csr = musb_readw(epio, MUSB_TXCSR);
if (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-- 
1.9.1

--
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/2] usb: musb: fix bogus rx endpoint interrupt

[Bin Liu]

Hi,

This fixes a long standing musb bogus rx interrupt problem. I am not sure on
other platforms, but on AM335x with CPPI DMA enabled, occasionally any of the
following kernel messages shows up from musb driver. (The endpoint number is
random, of cause.)

musb_host_rx 1853: BOGUS RX2 ready, csr , count 0

musb_host_rx 1936: RX3 dma busy, csr 2020

In one of the test cases with FT4232H I observed the issue when sometimes
closing the uart port. It turns out the issue is that during handling urb
dequeue, the controller is still receiving data while the CPPI channel is
already aborted, then musb core generates endpoint rx interrupt, instead of
rx dma interrupt in normal rx transfer.

Based on the inline comments, the fix is to call the new platform ops to clear
the rx ep interrupt in musb_cleanup_urb().

I am not sure if this fix should be back ported to all stable trees, but I
only tested up to v4.1, so cc'd stable v4.1+.

Regards,
-Bin.
---

Bin Liu (2):
  usb: musb: core: add clear_ep_rxintr() to musb_platform_ops
  usb: musb: dsps: implement clear_ep_rxintr() callback

 drivers/usb/musb/musb_core.h |  7 +++
 drivers/usb/musb/musb_dsps.c | 12 
 drivers/usb/musb/musb_host.c | 10 --
 3 files changed, 23 insertions(+), 6 deletions(-)

-- 
1.9.1

--
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/2] usb: musb: dsps: implement clear_ep_rxintr() callback

[Bin Liu]

This is for avoiding rx ep bogus interrupt during CPPI channel teardown.

cc: sta...@vger.kernel.org # 4.1+
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_dsps.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0f17d2140db6..092e255e6f77 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -251,6 +251,17 @@ static void otg_timer(unsigned long _musb)
pm_runtime_put_autosuspend(dev);
 }
 
+void dsps_musb_clear_ep_rxintr(struct musb *musb, int epnum)
+{
+   u32 epintr;
+   struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
+   const struct dsps_musb_wrapper *wrp = glue->wrp;
+
+   /* musb->lock might already been held */
+   epintr = (1 << epnum) << wrp->rxep_shift;
+   musb_writel(musb->ctrl_base, wrp->epintr_status, epintr);
+}
+
 static irqreturn_t dsps_interrupt(int irq, void *hci)
 {
struct musb  *musb = hci;
@@ -606,6 +617,7 @@ static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 
len, u8 *dst)
 
.set_mode   = dsps_musb_set_mode,
.recover= dsps_musb_recover,
+   .clear_ep_rxintr = dsps_musb_clear_ep_rxintr,
 };
 
 static u64 musb_dmamask = DMA_BIT_MASK(32);
-- 
1.9.1

--
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 net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [mailto:ml...@pobox.com]
> Sent: Thursday, November 24, 2016 3:30 AM
[...]
> Worth repeating: other dongles we have tried, eg. those using the asix driver,
> do not cause us any troubles here.  Only the r8152 dongles do.

I couldn't tell you why you would see the problem. I have tested the
RTL8152 on raspberry pi platform with iperf more than 17 hours. And
I don't see any invalid rx descriptor. I don't think it really is the
issue about our hw.

Best Regards,
Hayes

--
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: [PATCHv12 1/3] lib/string: add sysfs_match_string helper

[Guenter Roeck]

On Tue, Nov 22, 2016 at 04:11:45PM +0200, Heikki Krogerus wrote:
> Make a simple helper for matching strings with sysfs
> attribute files. In most parts the same as match_string(),
> except sysfs_match_string() uses sysfs_streq() instead of
> strcmp() for matching. This is more convenient when used
> with sysfs attributes.
> 
> Signed-off-by: Heikki Krogerus 

Tested-by: Guenter Roeck 

> ---
>  include/linux/string.h | 10 ++
>  lib/string.c   | 26 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..c4011b2 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -135,6 +135,16 @@ static inline int strtobool(const char *s, bool *res)
>  }
>  
>  int match_string(const char * const *array, size_t n, const char *string);
> +int __sysfs_match_string(const char * const *array, size_t n, const char *s);
> +
> +/**
> + * sysfs_match_string - matches given string in an array
> + * @_a: array of strings
> + * @_s: string to match with
> + *
> + * Helper for __sysfs_match_string(). Calculates the size of @a 
> automatically.
> + */
> +#define sysfs_match_string(_a, _s) __sysfs_match_string(_a, ARRAY_SIZE(_a), 
> _s)
>  
>  #ifdef CONFIG_BINARY_PRINTF
>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..c7a20cb 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -656,6 +656,32 @@ int match_string(const char * const *array, size_t n, 
> const char *string)
>  }
>  EXPORT_SYMBOL(match_string);
>  
> +/**
> + * __sysfs_match_string - matches given string in an array
> + * @array: array of strings
> + * @n: number of strings in the array or -1 for NULL terminated arrays
> + * @str: string to match with
> + *
> + * Returns index of @str in the @array or -EINVAL, just like match_string().
> + * Uses sysfs_streq instead of strcmp for matching.
> + */
> +int __sysfs_match_string(const char * const *array, size_t n, const char 
> *str)
> +{
> + const char *item;
> + int index;
> +
> + for (index = 0; index < n; index++) {
> + item = array[index];
> + if (!item)
> + break;
> + if (!sysfs_streq(item, str))
> + return index;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(__sysfs_match_string);
> +
>  #ifndef __HAVE_ARCH_MEMSET
>  /**
>   * memset - Fill a region of memory with the given value
> -- 
> 2.10.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

Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Guenter Roeck]

On Tue, Nov 22, 2016 at 04:11:46PM +0200, Heikki Krogerus wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
> 
> Signed-off-by: Heikki Krogerus 

Tested-by: Guenter Roeck 

> ---
>  Documentation/ABI/testing/sysfs-class-typec |  222 ++
>  Documentation/usb/typec.txt |  103 +++
>  MAINTAINERS |9 +
>  drivers/usb/Kconfig |2 +
>  drivers/usb/Makefile|2 +
>  drivers/usb/typec/Kconfig   |7 +
>  drivers/usb/typec/Makefile  |1 +
>  drivers/usb/typec/typec.c   | 1013 
> +++
>  include/linux/usb/typec.h   |  252 +++
>  9 files changed, 1611 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.txt
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 include/linux/usb/typec.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index 000..4fac77c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,222 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What:/sys/class/typec//current_data_role
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + The current USB data role the port is operating in. This
> + attribute can be used for requesting data role swapping on the
> + port. Swapping is supported as synchronous operation, so
> + write(2) to the attribute will not return until the operation
> + has finished. The attribute is notified about role changes so
> + that poll(2) on the attribute wakes up. Change on the role will
> + also generate uevent KOBJ_CHANGE on the port.
> +
> + Valid values:
> + - host
> + - device
> +
> +What:/sys/class/typec//current_power_role
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + The current power role of the port. This attribute can be used
> + to request power role swap on the port when the port supports
> + USB Power Delivery. Swapping is supported as synchronous
> + operation, so write(2) to the attribute will not return until
> + the operation has finished. The attribute is notified about role
> + changes so that poll(2) on the attribute wakes up. Change on the
> + role will also generate uevent KOBJ_CHANGE.
> +
> + Valid values:
> + - source
> + - sink
> +
> +What:/sys/class/typec//vconn_source
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + Shows is the port VCONN Source. This attribute can be used to
> + request VCONN swap to change the VCONN Source during connection
> + when both the port and the partner support USB Power Delivery.
> + Swapping is supported as synchronous operation, so write(2) to
> + the attribute will not return until the operation has finished.
> + The attribute is notified about VCONN source changes so that
> + poll(2) on the attribute wakes up. Change on VCONN source also
> + generates uevent KOBJ_CHANGE.
> +
> + Valid values are:
> + - 0 when the port is not the VCONN Source
> + - 1 when the port is the VCONN Source
> +
> +What:/sys/class/typec//power_operation_mode
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + Shows the current power operational mode the port is in.
> +
> + Valid values:
> + - USB - Normal power levels defined in USB specifications
> + - BC1.2 - Power levels defined in Battery Charging Specification
> +   v1.2
> + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
> + specification.
> + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C
> + specification.
> +- USB Power 

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[Vincent Pelletier]

On Wed, 23 Nov 2016 22:48:35 +0900, OGAWA Hirofumi
 wrote:
> ping?

FWIW, I intend to run this patch on the hardware which caused the
issue (thanks Mathias for the CC !).

So far, in the very short attempt I made, I failed to even build the
kernel (some stack protection feature error when being probed in
gcc...). I should have time to try again this week-end, but please do
not wait for me.

Regards,
-- 
Vincent Pelletier
--
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 v13 03/10] usbip: exporting devices: new connect operation

[Shuah Khan]

On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> Implementation of new connect operation. This is linked as a part of 
> usbip commnad. With this patch, usbip command has following operations.
> 
>  bind
>  unbind
>  list (local/remote)
>  attach
>  detach
>  port
>  connect ... this patch

I am finding it hard to follow these changes without the server and client
side details. A few comments below:

I want know a clear use-case definition of why the current client pull
model to import devices doesn't work. I understand firewalls pose some
issues. However, it doesn't sound right that server initiates export
and contacts the client. What happens when client doesn't have access
to the information on the server.

Can client use the same import over vpn connection or some sort of
authorized mechanism to access to the server?

> 
> In device side node, this binds a device internally, sends an export 
> request and receives export response. The definition of the request and 
> response are defined in original code: tools/usb/usbip/usbip_network.h 
> but was not used. They are corresponding to NEW-3 in following diagram. 
> 
> EXISTING) - invites devices from application(vhci)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>  1) usbipd ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip bind
> <--- list bound devices ---   4) usbip list --remote
> <--- import a device --   5) usbip attach
>  = = =
>X disconnected 6) usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stb)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>   1) usbipa ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip connect --- export a device -->
>  = = =
>  4) usbip disconnect  --- un-export a device --->
> 
> Signed-off-by: Nobuo Iwata 
> ---
>  tools/usb/usbip/src/Makefile.am |   3 +-
>  tools/usb/usbip/src/usbip.c |   9 +-
>  tools/usb/usbip/src/usbip.h |   5 +-
>  tools/usb/usbip/src/usbip_connect.c | 228 
>  4 files changed, 242 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am
> index e81a4eb..0947476 100644
> --- a/tools/usb/usbip/src/Makefile.am
> +++ b/tools/usb/usbip/src/Makefile.am
> @@ -6,6 +6,7 @@ sbin_PROGRAMS := usbip usbipd
>  
>  usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
>usbip_attach.c usbip_detach.c usbip_list.c \
> -  usbip_bind.c usbip_unbind.c usbip_port.c
> +  usbip_bind.c usbip_unbind.c usbip_port.c \
> +  usbip_connect.c
>  
>  usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
> diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c
> index d7599d9..584d7d5 100644
> --- a/tools/usb/usbip/src/usbip.c
> +++ b/tools/usb/usbip/src/usbip.c
> @@ -2,7 +2,8 @@
>   * command structure borrowed from udev
>   * (git://git.kernel.org/pub/scm/linux/hotplug/udev.git)
>   *
> - * Copyright (C) 2011 matt mooney 
> + * Copyright (C) 2015 Nobuo Iwata
> + *   2011 matt mooney 
>   *   2005-2007 Takahiro Hirofuchi
>   *
>   * This program is free software: you can redistribute it and/or modify
> @@ -76,6 +77,12 @@ static const struct command cmds[] = {
>   .usage = usbip_detach_usage
>   },
>   {
> + .name  = "connect",
> + .fn= usbip_connect,
> + .help  = "Connect a USB device to a remote computer",
> + .usage = usbip_connect_usage
> + },
> + {
>   .name  = "list",
>   .fn= usbip_list,
>   .help  = "List exportable or local USB devices",
> diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h
> index c296910..f365353 100644
> --- a/tools/usb/usbip/src/usbip.h
> +++ b/tools/usb/usbip/src/usbip.h
> @@ -1,5 +1,6 @@
>  /*
> - * Copyright (C) 2011 matt mooney 
> + * Copyright (C) 2015 Nobuo Iwata

Please don't add new Copyright to an existing file. Also the above
doesn't look right without email address. Please treat this as a
global comment for the Copyright changes in this patch series.


> + *   2011 matt mooney 
>   *   2005-2007 Takahiro Hirofuchi
>   *
>   * This program is free software: you can redistribute it and/or modify
> @@ -30,12 +31,14 @@ int usbip_list(int argc, char *argv[]);
>  int usbip_bind(int argc, char 

Re: [PATCH v13 02/10] usbip: exporting devices: modifications to host side libraries

[Shuah Khan]

On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> usbip_get_device() method in usbip_host_driver_ops was not used. It is 
> modified as a function to find an exported device for new operations 
> 'connect' and 'disconnect'.
> 
> bind and unbind function are exported to reuse by new connect and 
> disconnect operation.

This doesn't look like a simple change to rename and reuse an unused
function. This patch does lot more and is changing the user interface.
Looks like instead of taking an integer value for device lookup, you
are changing it to char *.

Any reason why you have to change the user interface to go to char *busid?

I would like to see a good explanation why this user interface change is
necessary.

thanks,
-- Shuah

> 
> Here, connect and disconnect is NEW-3 and NEW-4 respactively in diagram 
> below.
> 
> EXISTING) - invites devices from application(vhci)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>  1) usbipd ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip bind
> <--- list bound devices ---   4) usbip list --remote
> <--- import a device --   5) usbip attach
>  = = =
>X disconnected 6) usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stb)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>   1) usbipa ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip connect --- export a device -->
>  = = =
>  4) usbip disconnect  --- un-export a device --->
> 
>  Bind and unbind are done in connect and disconnect internally.
> 
> Signed-off-by: Nobuo Iwata 
> ---
>  tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++
>  tools/usb/usbip/libsrc/usbip_host_common.h | 8 
>  tools/usb/usbip/src/usbip.h| 3 +++
>  tools/usb/usbip/src/usbip_bind.c   | 4 ++--
>  tools/usb/usbip/src/usbip_unbind.c | 4 ++--
>  5 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c 
> b/tools/usb/usbip/libsrc/usbip_host_common.c
> index 9d41522..6a98d6c 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
> @@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device 
> *edev, int sockfd)
>  }
>  
>  struct usbip_exported_device *usbip_generic_get_device(
> - struct usbip_host_driver *hdriver, int num)
> + struct usbip_host_driver *hdriver, char *busid)
>  {
>   struct list_head *i;
>   struct usbip_exported_device *edev;
> - int cnt = 0;
>  
>   list_for_each(i, >edev_list) {
>   edev = list_entry(i, struct usbip_exported_device, node);
> - if (num == cnt)
> + if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE))
>   return edev;
> - cnt++;
>   }
>  
>   return NULL;
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h 
> b/tools/usb/usbip/libsrc/usbip_host_common.h
> index a64b803..f9a9def 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.h
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
> @@ -38,7 +38,7 @@ struct usbip_host_driver_ops {
>   void (*close)(struct usbip_host_driver *hdriver);
>   int (*refresh_device_list)(struct usbip_host_driver *hdriver);
>   struct usbip_exported_device * (*get_device)(
> - struct usbip_host_driver *hdriver, int num);
> + struct usbip_host_driver *hdriver, char *busid);
>  
>   int (*read_device)(struct udev_device *sdev,
>  struct usbip_usb_device *dev);
> @@ -86,11 +86,11 @@ static inline int usbip_refresh_device_list(struct 
> usbip_host_driver *hdriver)
>  }
>  
>  static inline struct usbip_exported_device *
> -usbip_get_device(struct usbip_host_driver *hdriver, int num)
> +usbip_get_device(struct usbip_host_driver *hdriver, char *busid)
>  {
>   if (!hdriver->ops.get_device)
>   return NULL;
> - return hdriver->ops.get_device(hdriver, num);
> + return hdriver->ops.get_device(hdriver, busid);
>  }
>  
>  /* Helper functions for implementing driver backend */
> @@ -99,6 +99,6 @@ void usbip_generic_driver_close(struct usbip_host_driver 
> *hdriver);
>  int usbip_generic_refresh_device_list(struct usbip_host_driver *hdriver);
>  int usbip_export_device(struct usbip_exported_device *edev, int sockfd);
>  struct usbip_exported_device *usbip_generic_get_device(
> - struct usbip_host_driver *hdriver, int num);
> + struct usbip_host_driver 

Re: [PATCH] USB: EHCI: use module_platform_driver macro

[Alan Stern]

On Tue, 22 Nov 2016 csmanjuvi...@gmail.com wrote:

> From: Majunath Goudar 
> 
> Use the module_platform_driver macro to do module init/exit.
> This eliminates a lot of boilerplate.This also removes redundant
> code and overhead of a function call.

I really don't like this patch, or the corresponding patch for 
ohci-nxp.c.  By moving the contents of ehci_w90X900_init() into
ehci_w90x900_probe(), you cause it to run at the wrong time and 
possibly run more than once.

(Also, the title of this patch is wrong.  You are not changing the EHCI 
core files; you are changing the specific ehci-w90x900 driver.)

On the other hand, I do like the fact that the patch removes two 
useless functions in the probe and remove paths.  But that can be done 
separately.

Alan Stern

> Signed-off-by: Manjunath Goudar 
> Cc: Arnd Bergmann 
> Cc: Greg KH 
> Cc: Alan Stern 
> Cc: Wan ZongShun 
> Cc: linux-usb@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/usb/host/ehci-w90x900.c | 55 
> -
>  1 file changed, 16 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-w90x900.c b/drivers/usb/host/ehci-w90x900.c
> index e42a29e..4cb2651 100644
> --- a/drivers/usb/host/ehci-w90x900.c
> +++ b/drivers/usb/host/ehci-w90x900.c
> @@ -33,8 +33,7 @@ static const char hcd_name[] = "ehci-w90x900 ";
>  
>  static struct hc_driver __read_mostly ehci_w90x900_hc_driver;
>  
> -static int usb_w90x900_probe(const struct hc_driver *driver,
> -   struct platform_device *pdev)
> +static int ehci_w90x900_probe(struct platform_device *pdev)
>  {
>   struct usb_hcd *hcd;
>   struct ehci_hcd *ehci;
> @@ -42,7 +41,15 @@ static int usb_w90x900_probe(const struct hc_driver 
> *driver,
>   int retval = 0, irq;
>   unsigned long val;
>  
> - hcd = usb_create_hcd(driver, >dev, "w90x900 EHCI");
> + if (usb_disabled())
> + return -ENODEV;
> +
> + pr_info("%s: " DRIVER_DESC "\n", hcd_name);
> +
> + ehci_init_driver(_w90x900_hc_driver, NULL);
> +
> + hcd = usb_create_hcd(_w90x900_hc_driver, >dev,
> + "w90x900 EHCI");
>   if (!hcd) {
>   retval = -ENOMEM;
>   goto err1;
> @@ -63,9 +70,9 @@ static int usb_w90x900_probe(const struct hc_driver *driver,
>   HC_LENGTH(ehci, ehci_readl(ehci, >caps->hc_capbase));
>  
>   /* enable PHY 0,1,the regs only apply to w90p910
> - *  0xA4,0xA8 were offsets of PHY0 and PHY1 controller of
> - *  w90p910 IC relative to ehci->regs.
> - */
> +  *  0xA4,0xA8 were offsets of PHY0 and PHY1 controller of
> +  *  w90p910 IC relative to ehci->regs.
> +  */
>   val = __raw_readl(ehci->regs+PHY0_CTR);
>   val |= ENPHY;
>   __raw_writel(val, ehci->regs+PHY0_CTR);
> @@ -92,26 +99,12 @@ static int usb_w90x900_probe(const struct hc_driver 
> *driver,
>   return retval;
>  }
>  
> -static void usb_w90x900_remove(struct usb_hcd *hcd,
> - struct platform_device *pdev)
> -{
> - usb_remove_hcd(hcd);
> - usb_put_hcd(hcd);
> -}
> -
> -static int ehci_w90x900_probe(struct platform_device *pdev)
> -{
> - if (usb_disabled())
> - return -ENODEV;
> -
> - return usb_w90x900_probe(_w90x900_hc_driver, pdev);
> -}
> -
>  static int ehci_w90x900_remove(struct platform_device *pdev)
>  {
>   struct usb_hcd *hcd = platform_get_drvdata(pdev);
>  
> - usb_w90x900_remove(hcd, pdev);
> + usb_remove_hcd(hcd);
> + usb_put_hcd(hcd);
>  
>   return 0;
>  }
> @@ -124,23 +117,7 @@ static struct platform_driver ehci_hcd_w90x900_driver = {
>   },
>  };
>  
> -static int __init ehci_w90X900_init(void)
> -{
> - if (usb_disabled())
> - return -ENODEV;
> -
> - pr_info("%s: " DRIVER_DESC "\n", hcd_name);
> -
> - ehci_init_driver(_w90x900_hc_driver, NULL);
> - return platform_driver_register(_hcd_w90x900_driver);
> -}
> -module_init(ehci_w90X900_init);
> -
> -static void __exit ehci_w90X900_cleanup(void)
> -{
> - platform_driver_unregister(_hcd_w90x900_driver);
> -}
> -module_exit(ehci_w90X900_cleanup);
> +module_platform_driver(ehci_hcd_w90x900_driver);
>  
>  MODULE_DESCRIPTION(DRIVER_DESC);
>  MODULE_AUTHOR("Wan ZongShun ");
> 

--
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: ohci: da8xx: Balance ochi_disable with ohci_enable in resume.

[Alan Stern]

On Wed, 23 Nov 2016, Axel Haslam wrote:

> On resume from suspend a failure with -ESHUTDOWN is returned
> from ohci_bus_resume, and the usb is inoperable.
> 
> This happens because ohci_suspend disables the master interrupt
> and sets an hcd flag to say that the hw is no longer accessible.

This patch is okay, but the title and the two sentences above are 
completely beside the point.  The real point of this patch is that 
ohci_da8xx_suspend() calls ohci_suspend(), and therefore 
ohci_da8xx_resume() should call ohci_resume() rather than 
ohci_bus_resume().

Which is the right thing to do in any case, because the 
ohci_da8xx_resume() callback wants to resume the entire controller and 
not just the root hub.

After fixing the title and those two sentences, you can add:

Acked-by: Alan Stern 

Alan Stern

> 
> Calling ohci_resume reverts the steps taken on ohci_suspend
> and flags the HW as "accessible" again, resume completes
> successfully and usb is working after a suspend/resume sequence.
> 
> While we are here, remove setting device power_state,
> as this is no longer needed and scheduled for removal.
> 
> Signed-off-by: Axel Haslam 
> ---
>  drivers/usb/host/ohci-da8xx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index b3de8bc..a598bd8 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -340,8 +340,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
>   if (ret)
>   return ret;
>  
> - dev->dev.power.power_state = PMSG_ON;
> - usb_hcd_resume_root_hub(hcd);
> + ohci_resume(hcd, false);
>  
>   return 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 v13 01/10] usbip: exporting devices: modifications to network header

[Shuah Khan]

On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> Modification to export and un-export response in 
> tools/usb/usbip/src/usbip_network.h. It just changes return code type 
> from int to uint32_t as same as other responses.
> 
> Signed-off-by: Nobuo Iwata 

Looks fine to me.

Acked-by: Shuah Khan 

> ---
>  tools/usb/usbip/src/usbip_network.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/usb/usbip/src/usbip_network.h 
> b/tools/usb/usbip/src/usbip_network.h
> index c1e875c..e1ca86a 100644
> --- a/tools/usb/usbip/src/usbip_network.h
> +++ b/tools/usb/usbip/src/usbip_network.h
> @@ -93,7 +93,7 @@ struct op_export_request {
>  } __attribute__((packed));
>  
>  struct op_export_reply {
> - int returncode;
> + uint32_t returncode;
>  } __attribute__((packed));
>  
>  
> @@ -115,7 +115,7 @@ struct op_unexport_request {
>  } __attribute__((packed));
>  
>  struct op_unexport_reply {
> - int returncode;
> + uint32_t returncode;
>  } __attribute__((packed));
>  
>  #define PACK_OP_UNEXPORT_REQUEST(pack, request)  do {\
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America(Silicon Valley)
shuah...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Mark Lord]

On 16-11-23 10:12 AM, Hayes Wang wrote:

Mark Lord [ml...@pobox.com]
[...]

What does this code do:



static void r8153_set_rx_early_size(struct r8152 *tp)
{
   u32 mtu = tp->netdev->mtu;
   u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;

   ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
}


This only works for RTL8153. However, what you use is RTL8152.
It is like delay completion. It is used to reduce the loading of CPU
by letting a transfer contain more data to reduce the number of
transfers.


How is ocp_data used by the hardware?
Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?


The algorithm is from our hw engineers, and it should be

   (agg_buf_sz - packet size) / 8

You could refer to commit a59e6d815226 ("r8152: correct the rx early size").


Thanks.

Right now I am working quite hard trying to narrow things down exactly.
You are correct that the driver does appear to be careful about accesses
beyond the filled portion of a URB buffer -- for some reason I thought
the original driver had issues there, but looking again it does not seem to.

One idea that is now looking more likely:
Things could be suffering from speculative CPU accesses to RAM
(the system here has non-coherent d-cache/RAM).
This could incorrectly pre-load data from adjacent URB buffers
into the d-cache, creating coherency issues.  I am testing now
with cacheline-sized guard zones between the buffers to see if
that is the issue or not.

Worth repeating: other dongles we have tried, eg. those using the asix driver,
do not cause us any troubles here.  Only the r8152 dongles do.

The other drivers do not use hardware checksums, so even if they did
incur similar bad packets, whatever the reason, those bad packets
would be detected/rejected by the Linux network stack (software checksums).
So everything appears to behave fine with them, as it does with
the r8152 driver when hardware checksums are disabled.

Still trying to understand exactly how these errors are happening.
It takes a very long time to do a conclusive test of anything here,
and I only have the hardware for a day or two a week.
So my apologies if I am slow in getting back to you on stuff.

Cheers



--
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: Issue with Telit LE922 and cdc_mbim

[Bjørn Mork]

Bjørn Mork  writes:
> Daniele Palmas  writes:
>> 2016-11-23 15:49 GMT+01:00 Bjørn Mork :
>>
>>> I see that your testing also included Intel based modems. That's good.
>>> It would still be nice to have test results from a few more MBIM
>>> implementations, in particular the more "difficult" ones. But I don't
>>
>> I agree, especially Huawei ones for which the alternate setting
>> toggling was introduced.
>
> I'll see what I can do about that. Looks like I have to dig through my
> mailbox.  It seems I didn't add any Reported-by tags on the relevant
> commits.  Wonder why...  Well, there you have the reason why you always
> should.  Now I got to do the work instead of just loading it on you :)

OK, I found the details I should have included with that commit.  The
bug was observed on a Huawei E3372: It didn't respect our request
for 16bit headers until we added this altsetting toggle.

Ref https://bugs.freedesktop.org/show_bug.cgi?id=94340

And thanks to Andrew I have one of those modems, so I should be able to
test your alternative fix.  If I can only figure out where my modems
are...


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

Re: Issue with Telit LE922 and cdc_mbim

[Bjørn Mork]

Daniele Palmas  writes:
> 2016-11-23 15:49 GMT+01:00 Bjørn Mork :
>> Daniele Palmas  writes:
>>> 2016-11-21 10:49 GMT+01:00 Bjørn Mork :
 Daniele Palmas  writes:

> it turned out that resetting the interface in cdc_ncm_init after
> getting the NTB parameters removes the need for the sleep, making the
> modem to work fine.

 Sounds very good, although I must admit that it isn't perfectly clear to
 me what kind of reset we're talking about here.  But no worries, the
 patch will make that clear :)

>>>
>>> Sorry for the confusion, I meant resetting the MBIM function with
>>> RESET_FUNCTION request code.
>>
>> Yes, the patch did make that clear, as expected :)
>>
> I wonder if this is an acceptable solution and can be applied also for 
> MC7455.

 Quite possible.  I will definitely test it.  If we can avoid an
 arbitrary and pointless delay, then that's great.  But I guess this also
 requires testing with a wide range of other MBIM devices to find out if
 we can apply it unconditionally without breaking anything. Avoiding
 device-specific or vendor-specific code is important in a class driver,
 if possible.

>>>
>>> Ok, unfortunately I can test only with Telit modems, so I'm not sure
>>> if the change I did is harmless for all the other modems:
>>> RESET_FUNCTION should not cause issues,
>>
>> Agreed.  Unfortunately, we cannot depend on sane firmware behaviour ;)
>>
>> I did a semi-quick test on a Sierra Wireless EM7455 and can confirm that
>> your patch makes it work without the delay.  Very nice.
>>
>
> Cool!
>
>> Do you happen to know if the Windows MBIM driver use  RESET_FUNCTION
>> during initialisation?  That would make this feel much safer.
>>
>
> Yes, Windows driver seems to send RESET_FUNCTION two times: after
> getting NTB parameters and after setting NTB input size. But it seems
> that only the first one is mandatory.
>
> Windows USB traces taken with TotalPhase Datacenter are available at
>
> https://drive.google.com/drive/folders/0B1kPnH2g8ISIZWJiV05qeWN5dVE
>
> file LE922win10_2.tdc

This is comforting. It means that the risk introducing the
RESET_FUNCTION call is close to zero.  The only remaining issue is
whether we can safely remove the altsetting toggle.

>> I see that your testing also included Intel based modems. That's good.
>> It would still be nice to have test results from a few more MBIM
>> implementations, in particular the more "difficult" ones. But I don't
>
> I agree, especially Huawei ones for which the alternate setting
> toggling was introduced.

I'll see what I can do about that. Looks like I have to dig through my
mailbox.  It seems I didn't add any Reported-by tags on the relevant
commits.  Wonder why...  Well, there you have the reason why you always
should.  Now I got to do the work instead of just loading it on you :)

>> know how much help I can promise here. At the moment I don't even know
>> which box my modems are in.  Moved a month ago and am still living in a
>> box.  Or more like a hundred boxes ;)
>>
>> The easiest way to get this thoroughly tested is of course to push it
>> now, and try to respond quickly in case it breaks something. Don't know
>> what others think about that?
>>
>>> but I also removed altsetting
>>> toggling for all MBIM modems and maybe this is not acceptable.
>>
>> I wonder about that.  It was added specifically for modems which seemed
>> to need it.  Don't remember if we ever tried RESET_FUNCTION as an
>> alternative. Removing workaorunds which have proven to be necessary at
>> some point in time is a bit risky.
>>
>
> I totally agree, but the other way would be to add another quirk at
> the cdc_mbim driver level, isn't it?

Yes, so I'm definitely with you here - let's try without the quirk
first.  I'm ready to say "commit it", if noone else objects.

> If this is a preferred solution I can try to modify the patch
> according to this path.

No, the patch is fine as is.  Just one question: If keeping the
altsetting toggle proves necessary, will that break the Qualcomm
devices?  Yes, I could test that myself, but I'm lazy and I guess you
already have tested it?


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

Re: [PATCH 6/6] pinctrl: mt8173: set GPIO16 to usb iddig mode

[Matthias Brugger]

Hi Hongzhou,

On 12/05/16 04:55, Hongzhou Yang wrote:

On Wed, 2016-05-11 at 19:09 -0700, Hongzhou Yang wrote:

On Thu, 2016-05-12 at 09:41 +0800, chunfeng yun wrote:

Hi,

On Wed, 2016-05-11 at 11:32 -0700, Hongzhou Yang wrote:

On Wed, 2016-05-11 at 13:56 +0200, Linus Walleij wrote:

On Tue, May 10, 2016 at 10:23 AM, Chunfeng Yun
 wrote:


the default mode of GPIO16 pin is gpio, when set EINT16 to
IRQ_TYPE_LEVEL_HIGH, no interrupt is triggered, it can be
fixed when set its default mode as usb iddig.

Signed-off-by: Chunfeng Yun 




Chunfeng, GPIO16 can be used as EINT16 mode, but the pinmux should be 0.
If you want to set its default mode to iddig, you should set it in dts.


I set it in DTS, but it didn't work, because when usb driver requested
IRQ, pinmux was switched back to default mode set by
MTK_EINT_FUNCTION().



After confirmed, there are something wrong with data sheet and pinmux
table, and GPIO16 can only receive interrupt by mode 1. So

Acked-by: Hongzhou Yang 



Linus,

We find there are some other pins still have the same problem, so please
hold on it. Sorry for so much noise.



Did you made any progress on this? I didn't see any patch on the mailing 
list.


Regards,
Matthias
--
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: ohci: da8xx: Balance ochi_disable with ohci_enable in resume.

[Axel Haslam]

On resume from suspend a failure with -ESHUTDOWN is returned
from ohci_bus_resume, and the usb is inoperable.

This happens because ohci_suspend disables the master interrupt
and sets an hcd flag to say that the hw is no longer accessible.

Calling ohci_resume reverts the steps taken on ohci_suspend
and flags the HW as "accessible" again, resume completes
successfully and usb is working after a suspend/resume sequence.

While we are here, remove setting device power_state,
as this is no longer needed and scheduled for removal.

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index b3de8bc..a598bd8 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -340,8 +340,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
if (ret)
return ret;
 
-   dev->dev.power.power_state = PMSG_ON;
-   usb_hcd_resume_root_hub(hcd);
+   ohci_resume(hcd, false);
 
return 0;
 }
-- 
2.9.3

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

[PATCHv7 1/5] USB: ohci: da8xx: use ohci priv data instead of globals

[Axel Haslam]

Instead of global variables, use the extra_priv_size of
the ohci driver.

We cannot yet move the ocic mask because this is used on
the interrupt handler which is registered through platform
data and does not have an hcd pointer. This will be moved
on a later patch.

Tested-by: David Lechner 
Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 73 +--
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index bd6cf3c..cd75677 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -35,43 +35,50 @@ static int (*orig_ohci_hub_control)(struct usb_hcd  *hcd, 
u16 typeReq,
u16 wValue, u16 wIndex, char *buf, u16 wLength);
 static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
 
-static struct clk *usb11_clk;
-static struct phy *usb11_phy;
+struct da8xx_ohci_hcd {
+   struct clk *usb11_clk;
+   struct phy *usb11_phy;
+};
+
+#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
 
 /* Over-current indicator change bitmask */
 static volatile u16 ocic_mask;
 
-static int ohci_da8xx_enable(void)
+static int ohci_da8xx_enable(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret;
 
-   ret = clk_prepare_enable(usb11_clk);
+   ret = clk_prepare_enable(da8xx_ohci->usb11_clk);
if (ret)
return ret;
 
-   ret = phy_init(usb11_phy);
+   ret = phy_init(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_init;
 
-   ret = phy_power_on(usb11_phy);
+   ret = phy_power_on(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_power_on;
 
return 0;
 
 err_phy_power_on:
-   phy_exit(usb11_phy);
+   phy_exit(da8xx_ohci->usb11_phy);
 err_phy_init:
-   clk_disable_unprepare(usb11_clk);
+   clk_disable_unprepare(da8xx_ohci->usb11_clk);
 
return ret;
 }
 
-static void ohci_da8xx_disable(void)
+static void ohci_da8xx_disable(struct usb_hcd *hcd)
 {
-   phy_power_off(usb11_phy);
-   phy_exit(usb11_phy);
-   clk_disable_unprepare(usb11_clk);
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+   phy_power_off(da8xx_ohci->usb11_phy);
+   phy_exit(da8xx_ohci->usb11_phy);
+   clk_disable_unprepare(da8xx_ohci->usb11_clk);
 }
 
 /*
@@ -97,7 +104,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 
dev_dbg(dev, "starting USB controller\n");
 
-   result = ohci_da8xx_enable();
+   result = ohci_da8xx_enable(hcd);
if (result < 0)
return result;
 
@@ -109,7 +116,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 
result = ohci_setup(hcd);
if (result < 0) {
-   ohci_da8xx_disable();
+   ohci_da8xx_disable(hcd);
return result;
}
 
@@ -231,6 +238,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 static int ohci_da8xx_probe(struct platform_device *pdev)
 {
struct da8xx_ohci_root_hub *hub = dev_get_platdata(>dev);
+   struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd  *hcd;
struct resource *mem;
int error, irq;
@@ -238,25 +246,29 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
if (hub == NULL)
return -ENODEV;
 
-   usb11_clk = devm_clk_get(>dev, "usb11");
-   if (IS_ERR(usb11_clk)) {
-   if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
+   hcd = usb_create_hcd(_da8xx_hc_driver, >dev,
+   dev_name(>dev));
+   if (!hcd)
+   return -ENOMEM;
+
+   da8xx_ohci = to_da8xx_ohci(hcd);
+
+   da8xx_ohci->usb11_clk = devm_clk_get(>dev, "usb11");
+   if (IS_ERR(da8xx_ohci->usb11_clk)) {
+   error = PTR_ERR(da8xx_ohci->usb11_clk);
+   if (error != -EPROBE_DEFER)
dev_err(>dev, "Failed to get clock.\n");
-   return PTR_ERR(usb11_clk);
+   goto err;
}
 
-   usb11_phy = devm_phy_get(>dev, "usb-phy");
-   if (IS_ERR(usb11_phy)) {
-   if (PTR_ERR(usb11_phy) != -EPROBE_DEFER)
+   da8xx_ohci->usb11_phy = devm_phy_get(>dev, "usb-phy");
+   if (IS_ERR(da8xx_ohci->usb11_phy)) {
+   error = PTR_ERR(da8xx_ohci->usb11_phy);
+   if (error != -EPROBE_DEFER)
dev_err(>dev, "Failed to get phy.\n");
-   return PTR_ERR(usb11_phy);
+   goto err;
}
 
-   hcd = usb_create_hcd(_da8xx_hc_driver, >dev,
-   dev_name(>dev));
-   if (!hcd)
-   return -ENOMEM;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(>dev, mem);
if (IS_ERR(hcd->regs)) {
@@ 

[PATCHv7 0/5] SB: ohci-da8xx: Add device tree support

[Axel Haslam]

When booting using device tree, we can not make use of
platform callbacks to handle vbus and over current gpios.

This series allows the ohci-da8xx driver to use a regulator
instead of the platform callbacks to control vbus and adds
the device tree bindings to be able to probe using DT.

Once all users of the platform callbacks will be converted to
use a regulator, we will be able to remove platform data completely.

**DEPENDENCY**:
These patches depend on a regulator patch that is
merged into linux-next, but not yet on usb-next:

regulator: core: Add new API to poll for error conditions
https://lkml.org/lkml/2016/11/3/191


Changes from v6->v7
* remove print on over current event as the usb core already does it (David)
* fix ocic mask for regulator event (David)
* spelling fix and tested-by on first patch.
* skip error message on probe defer from regulator (David)

Changes from v5->v6
* Fix regulator over current flag check (David)
* Spelling fixes and code cleanups (David)
* add Ack for device tree binding

Changes from v4->v5
* Append the Device tree patches to v4.
* Submit only the first part of the series (no dependencies).
this can be applied and merged through the usb tree.

Changes from v3->v4
* separate the series into smaller series for driver and arch/arm code,
  to ease review and merging to different trees.

Changes form v2->v3
* drop patches that have been integrated to arch/arm
* drop regulator patches which will be integrated through regulator tree
* use of the accepted regulator API to get over current status
* better patch separation with the use of wrappers

Changes from v1->v2
* Rebased and added patch to make ohci a separate driver
* Use a regulator instead of handling Gpios (David Lechner)
* Add an over current mode to regulator framework
* Fixed regulator is able to register for and over current irq
* Added patch by Alexandre to remove build warnings
* Moved global variables into private hcd structure.
Axel Haslam (5):
  USB: ohci: da8xx: use ohci priv data instead of globals
  USB: ohci: da8xx: Add wrappers for platform callbacks
  USB: ohci: da8xx: Allow a regulator to handle VBUS
  USB: ohci: da8xx: Add devicetree bindings
  USB: ohci: da8xx: Allow probing from DT

 .../devicetree/bindings/usb/ohci-da8xx.txt |  23 ++
 drivers/usb/host/ohci-da8xx.c  | 290 +
 2 files changed, 263 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt

-- 
2.9.3

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

[PATCHv7 2/5] USB: ohci: da8xx: Add wrappers for platform callbacks

[Axel Haslam]

To migrate to a DT based boot, we will remove the use of platform
callbacks, in favor of using the regulator framework to handle
vbus and over current.

In preparation to use a regulator instead of callbacks, move the platform
data callbacks into separate functions. This provides well defined place
to for the regulator API to coexist with the platform callbacks before
all users are converted.

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 125 ++
 1 file changed, 102 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index cd75677..aab4e3a 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -81,6 +81,72 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
clk_disable_unprepare(da8xx_ohci->usb11_clk);
 }
 
+static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->set_power)
+   return hub->set_power(1, on);
+
+   return 0;
+}
+
+static int ohci_da8xx_get_power(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_power)
+   return hub->get_power(1);
+
+   return 1;
+}
+
+static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_oci)
+   return hub->get_oci(1);
+
+   return 0;
+}
+
+static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->set_power)
+   return 1;
+
+   return 0;
+}
+
+static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_oci)
+   return 1;
+
+   return 0;
+}
+
+static int ohci_da8xx_has_potpgt(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->potpgt)
+   return 1;
+
+   return 0;
+}
+
 /*
  * Handle the port over-current indicator change.
  */
@@ -94,6 +160,26 @@ static void ohci_da8xx_ocic_handler(struct 
da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
 }
 
+static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->ocic_notify)
+   return hub->ocic_notify(ohci_da8xx_ocic_handler);
+
+   return 0;
+}
+
+static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->ocic_notify)
+   hub->ocic_notify(NULL);
+}
+
 static int ohci_da8xx_reset(struct usb_hcd *hcd)
 {
struct device *dev  = hcd->self.controller;
@@ -127,16 +213,18 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 * the correct hub descriptor...
 */
rh_a = ohci_readl(ohci, >regs->roothub.a);
-   if (hub->set_power) {
+   if (ohci_da8xx_has_set_power(hcd)) {
rh_a &= ~RH_A_NPS;
rh_a |=  RH_A_PSM;
}
-   if (hub->get_oci) {
+   if (ohci_da8xx_has_oci(hcd)) {
rh_a &= ~RH_A_NOCP;
rh_a |=  RH_A_OCPM;
}
-   rh_a &= ~RH_A_POTPGT;
-   rh_a |= hub->potpgt << 24;
+   if (ohci_da8xx_has_potpgt(hcd)) {
+   rh_a &= ~RH_A_POTPGT;
+   rh_a |= hub->potpgt << 24;
+   }
ohci_writel(ohci, rh_a, >regs->roothub.a);
 
return result;
@@ -169,7 +257,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
  u16 wIndex, char *buf, u16 wLength)
 {
struct device *dev  = hcd->self.controller;
-   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int temp;
 
switch (typeReq) {
@@ -183,11 +270,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, 
u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
 
/* The port power status (PPS) bit defaults to 1 */
-   if (hub->get_power && hub->get_power(wIndex) == 0)
+   if (!ohci_da8xx_get_power(hcd))
temp &= 

[PATCHv7 3/5] USB: ohci: da8xx: Allow a regulator to handle VBUS

[Axel Haslam]

Using a regulator to handle VBUS will eliminate the need for
platform data and callbacks, and make the driver more generic
allowing different types of regulators to handle VBUS.

The regulator equivalents to the platform callbacks are:
set_power   ->  regulator_enable/regulator_disable
get_power   ->  regulator_is_enabled
get_oci ->  regulator_get_error_flags
ocic_notify ->  regulator event notification

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 96 +--
 1 file changed, 93 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index aab4e3a..07366ae 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd  *hcd, 
u16 typeReq,
 static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
 
 struct da8xx_ohci_hcd {
+   struct usb_hcd *hcd;
struct clk *usb11_clk;
struct phy *usb11_phy;
+   struct regulator *vbus_reg;
+   struct notifier_block nb;
+   unsigned int reg_enabled;
 };
 
 #define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
@@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
 
 static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+   int ret;
 
if (hub && hub->set_power)
return hub->set_power(1, on);
 
+   if (!da8xx_ohci->vbus_reg)
+   return 0;
+
+   if (on && !da8xx_ohci->reg_enabled) {
+   ret = regulator_enable(da8xx_ohci->vbus_reg);
+   if (ret) {
+   dev_err(dev, "Failed to enable regulator: %d\n", ret);
+   return ret;
+   }
+   da8xx_ohci->reg_enabled = 1;
+
+   } else if (!on && da8xx_ohci->reg_enabled) {
+   ret = regulator_disable(da8xx_ohci->vbus_reg);
+   if (ret) {
+   dev_err(dev, "Failed  to disable regulator: %d\n", ret);
+   return ret;
+   }
+   da8xx_ohci->reg_enabled = 0;
+   }
+
return 0;
 }
 
 static int ohci_da8xx_get_power(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->get_power)
return hub->get_power(1);
 
+   if (da8xx_ohci->vbus_reg)
+   return regulator_is_enabled(da8xx_ohci->vbus_reg);
+
return 1;
 }
 
 static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+   unsigned int flags;
+   int ret;
 
if (hub && hub->get_oci)
return hub->get_oci(1);
 
+   if (!da8xx_ohci->vbus_reg)
+   return 0;
+
+   ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, );
+   if (ret)
+   return ret;
+
+   if (flags & REGULATOR_ERROR_OVER_CURRENT)
+   return 1;
+
return 0;
 }
 
 static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->set_power)
return 1;
 
+   if (da8xx_ohci->vbus_reg)
+   return 1;
+
return 0;
 }
 
 static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->get_oci)
return 1;
 
+   if (da8xx_ohci->vbus_reg)
+   return 1;
+
return 0;
 }
 
@@ -160,15 +212,39 @@ static void ohci_da8xx_ocic_handler(struct 
da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
 }
 
+static int ohci_da8xx_regulator_event(struct notifier_block *nb,
+   unsigned long event, void *data)
+{
+   struct da8xx_ohci_hcd *da8xx_ohci =
+   container_of(nb, struct da8xx_ohci_hcd, nb);
+
+   if (event & REGULATOR_EVENT_OVER_CURRENT) {
+   ocic_mask |= 1 << 1;
+   ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
+   }
+
+   return 

[PATCHv7 5/5] USB: ohci: da8xx: Allow probing from DT

[Axel Haslam]

This adds the compatible string to the ohci driver
to be able to probe from DT

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 07366ae..1818206 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -394,6 +394,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 }
 
 /*-*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+   { .compatible = "ti,da830-ohci" },
+   { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+#endif
 
 static int ohci_da8xx_probe(struct platform_device *pdev)
 {
@@ -546,6 +553,7 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
 #endif
.driver = {
.name   = DRV_NAME,
+   .of_match_table = of_match_ptr(da8xx_ohci_ids),
},
 };
 
-- 
2.9.3

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

[PATCHv7 4/5] USB: ohci: da8xx: Add devicetree bindings

[Axel Haslam]

This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's

Cc: robh...@kernel.org
Cc: mark.rutl...@arm.com
Cc: devicet...@vger.kernel.org
Acked-by: Rob Herring 
Signed-off-by: Axel Haslam 
---
 .../devicetree/bindings/usb/ohci-da8xx.txt | 23 ++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt

diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt 
b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 000..2dc8f67
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,23 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg:Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys:   Phandle for the PHY device
+ - phy-names:  Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: phandle of regulator that controls vbus power / over-current
+
+Example:
+
+ohci: usb@0225000 {
+compatible = "ti,da830-ohci";
+reg = <0x225000 0x1000>;
+interrupts = <59>;
+phys = <_phy 1>;
+phy-names = "usb-phy";
+vbus-supply = <_usb_ohci>;
+};
-- 
2.9.3

--
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 1/2] usb: host: plat: Enable xhci plat runtime PM

[kbuild test robot]

Hi Baolin,

[auto build test ERROR on v4.9-rc6]
[also build test ERROR on next-20161123]
[cannot apply to balbi-usb/next usb/usb-testing]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/usb-host-plat-Enable-xhci-plat-runtime-PM/20161124-012323
config: x86_64-randconfig-x008-201647 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb/host/xhci-plat.c: In function 'xhci_plat_remove':
>> drivers/usb/host/xhci-plat.c:281:28: error: 'pdev' undeclared (first use in 
>> this function)
 pm_runtime_set_suspended(>dev);
   ^~~~
   drivers/usb/host/xhci-plat.c:281:28: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/pdev +281 drivers/usb/host/xhci-plat.c

   275  static int xhci_plat_remove(struct platform_device *dev)
   276  {
   277  struct usb_hcd  *hcd = platform_get_drvdata(dev);
   278  struct xhci_hcd *xhci = hcd_to_xhci(hcd);
   279  struct clk *clk = xhci->clk;
   280  
 > 281  pm_runtime_set_suspended(>dev);
   282  pm_runtime_put_noidle(>dev);
   283  pm_runtime_disable(>dev);
   284  

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


.config.gz
Description: application/gzip

[PATCH] usb: musb: Fix trying to free already-free IRQ 4

[Tony Lindgren]

When unloading omap2430, we can get the following splat:

WARNING: CPU: 1 PID: 295 at kernel/irq/manage.c:1478 __free_irq+0xa8/0x2c8
Trying to free already-free IRQ 4
...
[] (free_irq) from []
(musbhs_dma_controller_destroy+0x28/0xb0 [musb_hdrc])
[] (musbhs_dma_controller_destroy [musb_hdrc]) from
[] (musb_remove+0xf0/0x12c [musb_hdrc])
[] (musb_remove [musb_hdrc]) from []
(platform_drv_remove+0x24/0x3c)
...

This is because the irq number in use is 260 nowadays, and the dma
controller is using u8 instead of int.

Signed-off-by: Tony Lindgren 
---

Found one more issue.. This has been around for years so can probably
wait until after v4.10-rc1. Probably should be tagged stable to avoid
multiple people debugging this over and over.

---
 drivers/usb/musb/musbhsdma.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musbhsdma.h b/drivers/usb/musb/musbhsdma.h
--- a/drivers/usb/musb/musbhsdma.h
+++ b/drivers/usb/musb/musbhsdma.h
@@ -157,5 +157,5 @@ struct musb_dma_controller {
void __iomem*base;
u8  channel_count;
u8  used_channels;
-   u8  irq;
+   int irq;
 };
-- 
2.10.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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tony Lindgren]

* Tomi Valkeinen  [161123 08:40]:
> On 23/11/16 18:34, Tony Lindgren wrote:
> 
> > OK. And what changes to your current .config make the musb_bus_suspend()
> > issues show up?
> > 
> > If it happens with USB-B cable from host to musb with musb set to host
> > only mode I'm not suprised there are errors :)
> 
> I have no USB cables connected. I have attached my config. If
> CONFIG_USB_MUSB_HDRC is disabled, the musb error goes away.

Maybe compare against omap2plus_defconfig? If you have musb
set to host only mode it still tries to follow the ID pin
from phy and do things on it's own. And ID pin is floating
in B mode and with no cable connected.

AFAIK, the only sane configuration for musb is the OTG mode
because the hardware tries to do things on it's own. And the
host only mode only really works if phy has ID pin grounded
or else the phy has to do trickery to rewrite the state to
musb.

Regards,

Tony
--
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: Problem with USB driver using two devices

[Greg KH]

On Wed, Nov 23, 2016 at 05:17:35PM +0100, Wolfgang Wilhelm wrote:
> Thankyou very much for the really fast answer.
> 
> I don't get any error messages and I can communicate with
> the driver for the second device via ioctrl and write functions,
> i.e. write registers and read registers via the RBUF ioctrl function,
> only the read function for the second device does not work,
> i.e. no data is obtained from the mcs6_read function for the
> second device.

Hm, let me go look at the driver again, maybe something's odd with it.

> Thankyou for your hint using libusb, I will have a look on it.
> 
> Our USB device has three endpoints, two with 64 kb packet size
> for reading and writing registers (I know this is not standard
> for high speed) and one with 512 kb packet size for reading data.
> Do you think the problem could arise from this deviation
> from the USB standard?

No, it should be fine, the USB standard only care about endpoint sizes,
not logical "packet" sizes, right?

> Of course we would be happy if our driver could be merged into
> the Linux kernel.

If you can use libusb, I'd strongly recommend using that and not a
kernel driver at all, as we don't like adding kernel drivers where they
are not needed.

> Our device uses the same vendor/product id's as a
> "D-Link DSB-R100 USB FM radio" dsbr100, that has a built-in
> driver in some Linux distributions, so that dsbr100 must be
> blacklisted in /etc/modprobe.d/fbdev-blacklist.conf for
> using our driver.

Does your device work like the USB FM radio device?  Why is the same
vendor/device id being used here?  They are supposed to be unique for
different types of devices.

> Which security problems do you see in the code?

No checking that the values given to you by userspace are actually valid
and within "sane" ranges :)

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 0/4] musb fixes for v4.9-rc cycle

[Laurent Pinchart]

Hello,

On Wednesday 23 Nov 2016 18:40:09 Tomi Valkeinen wrote:
> On 23/11/16 18:34, Tony Lindgren wrote:
> > OK. And what changes to your current .config make the musb_bus_suspend()
> > issues show up?
> > 
> > If it happens with USB-B cable from host to musb with musb set to host
> > only mode I'm not suprised there are errors :)
> 
> I have no USB cables connected. I have attached my config. If
> CONFIG_USB_MUSB_HDRC is disabled, the musb error goes away.

For what it's worth, that's exactly the same problem I experienced. See 
https://www.spinics.net/lists/linux-usb/msg149172.html for an analysis.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tony Lindgren]

* Tomi Valkeinen  [161123 08:13]:
> On 23/11/16 17:57, Tony Lindgren wrote:
> > * Tomi Valkeinen  [161123 07:54]:
> >> On 23/11/16 17:49, Laurent Pinchart wrote:
> >>> Hi Tomi,
> >>>
> >>> On Wednesday 23 Nov 2016 12:14:17 Tomi Valkeinen wrote:
>  On 10/11/16 23:25, Laurent Pinchart wrote:
>  [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle 
>  while
>  active
> 
>  printed in a loop at boot time. I've traced musb->is_active being set
>  to 1 in musb_start() with
> >
> > Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite
> > understandably. It's not a fix though, just a workaround. Are you
> > interested in investigating this ?
> 
>  I'm seeing this too. Did you manage to boot your Panda? If I disable
>  CONFIG_USB_MUSB_HDRC, I don't see that error print, but the ethernet
>  doesn't work, and so my nfsroot doesn't mount.
> >>>
> >>> Do you use ethernet over USB gadget or over LAN9514 (smsx95xx) ? The 
> >>> LAN9514 
> >>> is connected to the EHCI USB interface so it shouldn't depend on MUSB.
> >>
> >> smsx95xx. I didn't look at it further, so possibly there's something
> >> else broken/changed that prevented the ethernet from working.
> > 
> > I have omap2plus_defconfig with initramfs work just fine for nfsroot
> > and musb peripheral mode on pandaboard es. Well I have not tried it
> > since last week with Linux next.
> 
> Ok, I found the issue. I had to enable CONFIG_NOP_USB_XCEIV. (the
> musb_bus_suspend issue is still there, though).

OK. And what changes to your current .config make the musb_bus_suspend()
issues show up?

If it happens with USB-B cable from host to musb with musb set to host
only mode I'm not suprised there are errors :)

Regards,

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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tomi Valkeinen]

On 23/11/16 18:34, Tony Lindgren wrote:

> OK. And what changes to your current .config make the musb_bus_suspend()
> issues show up?
> 
> If it happens with USB-B cable from host to musb with musb set to host
> only mode I'm not suprised there are errors :)

I have no USB cables connected. I have attached my config. If
CONFIG_USB_MUSB_HDRC is disabled, the musb error goes away.

 Tomi
#
# Automatically generated file; DO NOT EDIT.
# Linux/arm 4.9.0-rc6 Kernel Configuration
#
CONFIG_ARM=y
CONFIG_ARM_HAS_SG_CHAIN=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_ARM_DMA_USE_IOMMU=y
CONFIG_ARM_DMA_IOMMU_ALIGNMENT=8
CONFIG_MIGHT_HAVE_PCI=y
CONFIG_SYS_SUPPORTS_APM_EMULATION=y
CONFIG_HAVE_PROC_CPU=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_TRACE_IRQFLAGS_SUPPORT=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_ARCH_HAS_BANDGAP=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_VECTORS_BASE=0x
CONFIG_ARM_PATCH_PHYS_VIRT=y
CONFIG_GENERIC_BUG=y
CONFIG_PGTABLE_LEVELS=2
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
CONFIG_KERNEL_LZMA=y
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
# CONFIG_USELIB is not set
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_SHOW_LEVEL=y
CONFIG_HARDIRQS_SW_RESEND=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_HANDLE_DOMAIN_IRQ=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_ARCH_HAS_TICK_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
# CONFIG_BSD_PROCESS_ACCT_V3 is not set
# CONFIG_TASKSTATS is not set

#
# RCU Subsystem
#
CONFIG_PREEMPT_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
CONFIG_RCU_STALL_COMMON=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
CONFIG_BUILD_BIN2C=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=16
CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
CONFIG_NMI_LOG_BUF_SHIFT=13
CONFIG_GENERIC_SCHED_CLOCK=y
CONFIG_CGROUPS=y
CONFIG_PAGE_COUNTER=y
CONFIG_MEMCG=y
CONFIG_MEMCG_SWAP=y
CONFIG_MEMCG_SWAP_ENABLED=y
CONFIG_BLK_CGROUP=y
# CONFIG_DEBUG_BLK_CGROUP is not set
CONFIG_CGROUP_WRITEBACK=y
CONFIG_CGROUP_SCHED=y
CONFIG_FAIR_GROUP_SCHED=y
CONFIG_CFS_BANDWIDTH=y
CONFIG_RT_GROUP_SCHED=y
# CONFIG_CGROUP_PIDS is not set
CONFIG_CGROUP_FREEZER=y
CONFIG_CPUSETS=y
CONFIG_PROC_PID_CPUSET=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_CGROUP_PERF=y
# CONFIG_CGROUP_DEBUG is not set
# CONFIG_CHECKPOINT_RESTORE is not set
CONFIG_NAMESPACES=y
CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
# CONFIG_USER_NS is not set
CONFIG_PID_NS=y
CONFIG_NET_NS=y
# CONFIG_SCHED_AUTOGROUP is not set
# CONFIG_SYSFS_DEPRECATED is not set
# CONFIG_RELAY is not set
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE=""
CONFIG_RD_GZIP=y
CONFIG_RD_BZIP2=y
CONFIG_RD_LZMA=y
CONFIG_RD_XZ=y
CONFIG_RD_LZO=y
CONFIG_RD_LZ4=y
CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
CONFIG_SYSCTL=y
CONFIG_ANON_INODES=y
CONFIG_HAVE_UID16=y
CONFIG_BPF=y
CONFIG_EXPERT=y
CONFIG_UID16=y
CONFIG_MULTIUSER=y
# CONFIG_SGETMASK_SYSCALL is not set
CONFIG_SYSFS_SYSCALL=y
# CONFIG_SYSCTL_SYSCALL is not set
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_ALL=y
# CONFIG_KALLSYMS_ABSOLUTE_PERCPU is not set
CONFIG_KALLSYMS_BASE_RELATIVE=y
CONFIG_PRINTK=y
CONFIG_PRINTK_NMI=y
CONFIG_BUG=y
CONFIG_ELF_CORE=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_EPOLL=y
CONFIG_SIGNALFD=y
CONFIG_TIMERFD=y
CONFIG_EVENTFD=y
# CONFIG_BPF_SYSCALL is not set
CONFIG_SHMEM=y
CONFIG_AIO=y
CONFIG_ADVISE_SYSCALLS=y
# CONFIG_USERFAULTFD is not set
CONFIG_MEMBARRIER=y
# CONFIG_EMBEDDED is not set
CONFIG_HAVE_PERF_EVENTS=y
CONFIG_PERF_USE_VMALLOC=y

#
# Kernel Performance Events And Counters
#

Re: [PATCH v13 00/10] usbip: exporting devices

[Shuah Khan]

On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> Dear all,
> 
> This series of patches adds exporting device operation to USB/IP.
> 
> NOTE:
> This patch set modifies only userspace codes in tools/usb/usbip.
> 
> 1. Goal
> 
> 1-1) To give flexibility to direction of connection
> Using USB/IP in internet, there can be two cases.
> a) an application is inside firewall and devices are outside.
> b) devices are inside firewall and an application is inside.
> In case-a, import works because the connection is from inside.
> In case-b, import doesn't works because the connection is from outside. 
> Connection from device side is needed. This patch set adds the 
> direction of connection establishment.
> 

Can you elaborate on the use-case a bit more? What does it mean
to "Connection from device side is needed"?

I would like to see server side and client side operations clearly.
It would help me understand the use-case you are trying to add.

I do have some concerns about security on client side. User sits
on the client side and it should be a pull from client side as
opposed to push from server side.

It sounds like this patch series adds push from server side.

thanks,
-- Shuah

> NOTE:
> Directions of URBs requests and responses are not changed. Only 
> direction of connection establishment initiated with usbip command is 
> added to exsiting one.
> 
> 1-2) To improve usability of operations
> When two Linux machines are in a small distance, it's OK to bind (makes 
> importable) at device side machine and attach (import) at application 
> side.
> If application is as cloud service or in blade server, it's not 
> practical to attach from application side. It's useful to connect 
> (export) from device side. This patch set adds the new operation to 
> connect from device side.
> 
> 2. What's 'exporting' device
> 
> Exporting devices is not new. The request and response PDU have already 
> been defined in tools/usbip/usbip/src/usbip_network.h.
> #/* Export a USB device to a remote host. */
> #define OP_EXPORT   0x06
> #define OP_REQ_EXPORT   (OP_REQUEST | OP_EXPORT)
> #define OP_REP_EXPORT   (OP_REPLY   | OP_EXPORT)
> # struct op_export_request
> # struct op_export_reply
> #/* un-Export a USB device from a remote host. */
> #define OP_UNEXPORT 0x07
> #define OP_REQ_UNEXPORT (OP_REQUEST | OP_UNEXPORT)
> #define OP_REP_UNEXPORT (OP_REPLY   | OP_UNEXPORT)
> # struct op_unexport_request
> # struct op_unexport_reply 
> 
> But they have not been used yet. This series adds new operations: 
> 'connect' and 'disconnect' using these PDUs.
> 
> EXISTING) - invites devices from application(vhci)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>  1) usbipd ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip bind
> <--- list bound devices ---   4) usbip list --remote
> <--- import a device --   5) usbip attach
>  = = =
>X disconnected 6) usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stb)-side
>  +--+ +--+
>  device--+ STUB | | application/VHCI |
>  +--+ +--+
>   1) usbipa ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip connect --- export a device -->
>  = = =
>  4) usbip disconnect  --- un-export a device --->
> 
>  Bind and unbind are done in connect and disconnect internally.
> 
> 3. The use cases
> 
> EXISTING)
> 
> In existing way, computers in small distance, having same user account, 
> can be easily managed by a same user. Bind in local machine and attach 
> in remote machine by the user. The devices can be exporsed 
> automatically in the local machine, for example, at strat up. They can 
> be attached from remote.
> 
> When there are distributes linux nodes with USB devices in internet, 
> they are exposed by bind operation at start up, server behind firewall 
> can list and attach the devices.  
>Internet  
>  Exposed   +--+++++
>  +--+  |Linux |+   |Router, ||Service |
> +|device|--|Controller||---|proxy,  ||on  |
> |+--+  +--+|   |firewall||Linux   |
> +--++--+   ++++
><--- attach(import)
>   USB/IP + WS proxy   WS proxy + USB/IP
> 
> NEW)
> 
> Assuming that a server computer which runs application and VHCI is in a 
> server room and device side machines are small distributed nodes 
> outside of the server room, the operator of 

Re: [PATCHv12 3/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY

[Guenter Roeck]

On Tue, Nov 22, 2016 at 04:11:47PM +0200, Heikki Krogerus wrote:
> This adds driver for the USB Type-C PHY on Intel WhiskeyCove
> PMIC which is available on some of the Intel Broxton SoC
> based platforms.
> 
> Signed-off-by: Heikki Krogerus 

LGTM, though I don't really know anything about the chip.
Couple of questions below, otherwise

Reviewed-by: Guenter Roeck 

> ---
>  drivers/usb/typec/Kconfig   |  14 ++
>  drivers/usb/typec/Makefile  |   1 +
>  drivers/usb/typec/typec_wcove.c | 372 
> 
>  3 files changed, 387 insertions(+)
>  create mode 100644 drivers/usb/typec/typec_wcove.c
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 17792f9..2abbcb0 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -4,4 +4,18 @@ menu "USB Power Delivery and Type-C drivers"
>  config TYPEC
>   tristate
>  
> +config TYPEC_WCOVE
> + tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
> + depends on ACPI
> + depends on INTEL_SOC_PMIC
> + depends on INTEL_PMC_IPC
> + select TYPEC
> + help
> +   This driver adds support for USB Type-C detection on Intel Broxton
> +   platforms that have Intel Whiskey Cove PMIC. The driver can detect the
> +   role and cable orientation.
> +
> +   To compile this driver as module, choose M here: the module will be
> +   called typec_wcove
> +
>  endmenu
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 1012a8b..b9cb862 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_TYPEC)  += typec.o
> +obj-$(CONFIG_TYPEC_WCOVE)+= typec_wcove.o
> diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
> new file mode 100644
> index 000..953d59b
> --- /dev/null
> +++ b/drivers/usb/typec/typec_wcove.c
> @@ -0,0 +1,372 @@
> +/**
> + * typec_wcove.c - WhiskeyCove PMIC USB Type-C PHY driver
> + *
> + * Copyright (C) 2016 Intel Corporation
> + * Author: Heikki Krogerus 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register offsets */
> +#define WCOVE_CHGRIRQ0   0x4e09
> +#define WCOVE_PHYCTRL0x5e07
> +
> +#define USBC_CONTROL10x7001
> +#define USBC_CONTROL20x7002
> +#define USBC_CONTROL30x7003
> +#define USBC_CC1_CTRL0x7004
> +#define USBC_CC2_CTRL0x7005
> +#define USBC_STATUS1 0x7007
> +#define USBC_STATUS2 0x7008
> +#define USBC_STATUS3 0x7009
> +#define USBC_IRQ10x7015
> +#define USBC_IRQ20x7016
> +#define USBC_IRQMASK10x7017
> +#define USBC_IRQMASK20x7018
> +
> +/* Register bits */
> +
> +#define USBC_CONTROL1_MODE_DRP(r)(((r) & ~0x7) | 4)
> +
> +#define USBC_CONTROL2_UNATT_SNK  BIT(0)
> +#define USBC_CONTROL2_UNATT_SRC  BIT(1)
> +#define USBC_CONTROL2_DIS_ST BIT(2)
> +
> +#define USBC_CONTROL3_PD_DIS BIT(1)
> +
> +#define USBC_CC_CTRL_VCONN_ENBIT(1)
> +
> +#define USBC_STATUS1_DET_ONGOING BIT(6)
> +#define USBC_STATUS1_RSLT(r) ((r) & 0xf)
> +#define USBC_RSLT_NOTHING0
> +#define USBC_RSLT_SRC_DEFAULT1
> +#define USBC_RSLT_SRC_1_5A   2
> +#define USBC_RSLT_SRC_3_0A   3
> +#define USBC_RSLT_SNK4
> +#define USBC_RSLT_DEBUG_ACC  5
> +#define USBC_RSLT_AUDIO_ACC  6
> +#define USBC_RSLT_UNDEF  15
> +#define USBC_STATUS1_ORIENT(r)   (((r) >> 4) & 0x3)
> +#define USBC_ORIENT_NORMAL   1
> +#define USBC_ORIENT_REVERSE  2
> +
> +#define USBC_STATUS2_VBUS_REQBIT(5)
> +
> +#define USBC_IRQ1_ADCDONE1   BIT(2)
> +#define USBC_IRQ1_OVERTEMP   BIT(1)
> +#define USBC_IRQ1_SHORT  BIT(0)
> +
> +#define USBC_IRQ2_CC_CHANGE  BIT(7)
> +#define USBC_IRQ2_RX_PD  BIT(6)
> +#define USBC_IRQ2_RX_HR  BIT(5)
> +#define USBC_IRQ2_RX_CR  BIT(4)
> +#define USBC_IRQ2_TX_SUCCESS BIT(3)
> +#define USBC_IRQ2_TX_FAILBIT(2)
> +
> +#define USBC_IRQMASK1_ALL(USBC_IRQ1_ADCDONE1 | USBC_IRQ1_OVERTEMP | \
> +  USBC_IRQ1_SHORT)
> +
> +#define USBC_IRQMASK2_ALL(USBC_IRQ2_CC_CHANGE | USBC_IRQ2_RX_PD | \
> +  USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \
> +  USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL)
> +
> +struct wcove_typec {
> + 

Re: Problem with USB driver using two devices

[Greg KH]

On Wed, Nov 23, 2016 at 05:35:45PM +0100, Greg KH wrote:
> On Wed, Nov 23, 2016 at 05:17:35PM +0100, Wolfgang Wilhelm wrote:
> > Thankyou very much for the really fast answer.
> > 
> > I don't get any error messages and I can communicate with
> > the driver for the second device via ioctrl and write functions,
> > i.e. write registers and read registers via the RBUF ioctrl function,
> > only the read function for the second device does not work,
> > i.e. no data is obtained from the mcs6_read function for the
> > second device.
> 
> Hm, let me go look at the driver again, maybe something's odd with it.

I don't see anything odd with your read function, is it just timing out?

Have you tried doing read/writes from userspace with libusb and that
works correctly?

> > Which security problems do you see in the code?
> 
> No checking that the values given to you by userspace are actually valid
> and within "sane" ranges :)

Oh, also your ioctl types need to be better specified than "int" as that
doesn't make much sense with 64bit kernels and a 32bit userspace.  You
need to use types like __u32 in order to make sure that works correctly
in all situations.

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: [PATCHv12 2/3] usb: USB Type-C connector class

[Guenter Roeck]

On Tue, Nov 22, 2016 at 04:11:46PM +0200, Heikki Krogerus wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
> 
> Signed-off-by: Heikki Krogerus 

Just a couple of nitpicks, otherwise

Reviewed-by: Guenter Roeck 

Guenter

> ---
>  Documentation/ABI/testing/sysfs-class-typec |  222 ++
>  Documentation/usb/typec.txt |  103 +++
>  MAINTAINERS |9 +
>  drivers/usb/Kconfig |2 +
>  drivers/usb/Makefile|2 +
>  drivers/usb/typec/Kconfig   |7 +
>  drivers/usb/typec/Makefile  |1 +
>  drivers/usb/typec/typec.c   | 1013 
> +++
>  include/linux/usb/typec.h   |  252 +++
>  9 files changed, 1611 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.txt
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 include/linux/usb/typec.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index 000..4fac77c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,222 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What:/sys/class/typec//current_data_role
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + The current USB data role the port is operating in. This
> + attribute can be used for requesting data role swapping on the
> + port. Swapping is supported as synchronous operation, so
> + write(2) to the attribute will not return until the operation
> + has finished. The attribute is notified about role changes so
> + that poll(2) on the attribute wakes up. Change on the role will
> + also generate uevent KOBJ_CHANGE on the port.
> +
> + Valid values:
> + - host
> + - device
> +
> +What:/sys/class/typec//current_power_role
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + The current power role of the port. This attribute can be used
> + to request power role swap on the port when the port supports
> + USB Power Delivery. Swapping is supported as synchronous
> + operation, so write(2) to the attribute will not return until
> + the operation has finished. The attribute is notified about role
> + changes so that poll(2) on the attribute wakes up. Change on the
> + role will also generate uevent KOBJ_CHANGE.
> +
> + Valid values:
> + - source
> + - sink
> +
> +What:/sys/class/typec//vconn_source
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + Shows is the port VCONN Source. This attribute can be used to
> + request VCONN swap to change the VCONN Source during connection
> + when both the port and the partner support USB Power Delivery.
> + Swapping is supported as synchronous operation, so write(2) to
> + the attribute will not return until the operation has finished.
> + The attribute is notified about VCONN source changes so that
> + poll(2) on the attribute wakes up. Change on VCONN source also
> + generates uevent KOBJ_CHANGE.
> +
> + Valid values are:
> + - 0 when the port is not the VCONN Source
> + - 1 when the port is the VCONN Source
> +
> +What:/sys/class/typec//power_operation_mode
> +Date:December 2016
> +Contact: Heikki Krogerus 
> +Description:
> + Shows the current power operational mode the port is in.
> +
> + Valid values:
> + - USB - Normal power levels defined in USB specifications
> + - BC1.2 - Power levels defined in Battery Charging Specification
> +   v1.2

This does not match the code. Maybe add later if it is ever added, or add it
now to the code ?

> + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
> + specification.
> + - 

Re: [PATCHv12 1/3] lib/string: add sysfs_match_string helper

[Guenter Roeck]

On Tue, Nov 22, 2016 at 04:11:45PM +0200, Heikki Krogerus wrote:
> Make a simple helper for matching strings with sysfs
> attribute files. In most parts the same as match_string(),
> except sysfs_match_string() uses sysfs_streq() instead of
> strcmp() for matching. This is more convenient when used
> with sysfs attributes.
> 
> Signed-off-by: Heikki Krogerus 

Reviewed-by: Guenter Roeck 

> ---
>  include/linux/string.h | 10 ++
>  lib/string.c   | 26 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..c4011b2 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -135,6 +135,16 @@ static inline int strtobool(const char *s, bool *res)
>  }
>  
>  int match_string(const char * const *array, size_t n, const char *string);
> +int __sysfs_match_string(const char * const *array, size_t n, const char *s);
> +
> +/**
> + * sysfs_match_string - matches given string in an array
> + * @_a: array of strings
> + * @_s: string to match with
> + *
> + * Helper for __sysfs_match_string(). Calculates the size of @a 
> automatically.
> + */
> +#define sysfs_match_string(_a, _s) __sysfs_match_string(_a, ARRAY_SIZE(_a), 
> _s)
>  
>  #ifdef CONFIG_BINARY_PRINTF
>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..c7a20cb 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -656,6 +656,32 @@ int match_string(const char * const *array, size_t n, 
> const char *string)
>  }
>  EXPORT_SYMBOL(match_string);
>  
> +/**
> + * __sysfs_match_string - matches given string in an array
> + * @array: array of strings
> + * @n: number of strings in the array or -1 for NULL terminated arrays
> + * @str: string to match with
> + *
> + * Returns index of @str in the @array or -EINVAL, just like match_string().
> + * Uses sysfs_streq instead of strcmp for matching.
> + */
> +int __sysfs_match_string(const char * const *array, size_t n, const char 
> *str)
> +{
> + const char *item;
> + int index;
> +
> + for (index = 0; index < n; index++) {
> + item = array[index];
> + if (!item)
> + break;
> + if (!sysfs_streq(item, str))
> + return index;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(__sysfs_match_string);
> +
>  #ifndef __HAVE_ARCH_MEMSET
>  /**
>   * memset - Fill a region of memory with the given value
> -- 
> 2.10.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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tony Lindgren]

* Tomi Valkeinen  [161123 07:54]:
> On 23/11/16 17:49, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > On Wednesday 23 Nov 2016 12:14:17 Tomi Valkeinen wrote:
> >> On 10/11/16 23:25, Laurent Pinchart wrote:
> >> [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while
> >> active
> >>
> >> printed in a loop at boot time. I've traced musb->is_active being set
> >> to 1 in musb_start() with
> >>>
> >>> Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite
> >>> understandably. It's not a fix though, just a workaround. Are you
> >>> interested in investigating this ?
> >>
> >> I'm seeing this too. Did you manage to boot your Panda? If I disable
> >> CONFIG_USB_MUSB_HDRC, I don't see that error print, but the ethernet
> >> doesn't work, and so my nfsroot doesn't mount.
> > 
> > Do you use ethernet over USB gadget or over LAN9514 (smsx95xx) ? The 
> > LAN9514 
> > is connected to the EHCI USB interface so it shouldn't depend on MUSB.
> 
> smsx95xx. I didn't look at it further, so possibly there's something
> else broken/changed that prevented the ethernet from working.

I have omap2plus_defconfig with initramfs work just fine for nfsroot
and musb peripheral mode on pandaboard es. Well I have not tried it
since last week with Linux next.

Regards,

Tony



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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tomi Valkeinen]

On 23/11/16 17:49, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 23 Nov 2016 12:14:17 Tomi Valkeinen wrote:
>> On 10/11/16 23:25, Laurent Pinchart wrote:
>> [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while
>> active
>>
>> printed in a loop at boot time. I've traced musb->is_active being set
>> to 1 in musb_start() with
>>>
>>> Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite
>>> understandably. It's not a fix though, just a workaround. Are you
>>> interested in investigating this ?
>>
>> I'm seeing this too. Did you manage to boot your Panda? If I disable
>> CONFIG_USB_MUSB_HDRC, I don't see that error print, but the ethernet
>> doesn't work, and so my nfsroot doesn't mount.
> 
> Do you use ethernet over USB gadget or over LAN9514 (smsx95xx) ? The LAN9514 
> is connected to the EHCI USB interface so it shouldn't depend on MUSB.

smsx95xx. I didn't look at it further, so possibly there's something
else broken/changed that prevented the ethernet from working.

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Laurent Pinchart]

Hi Tomi,

On Wednesday 23 Nov 2016 12:14:17 Tomi Valkeinen wrote:
> On 10/11/16 23:25, Laurent Pinchart wrote:
>  [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while
>  active
>  
>  printed in a loop at boot time. I've traced musb->is_active being set
>  to 1 in musb_start() with
> > 
> > Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite
> > understandably. It's not a fix though, just a workaround. Are you
> > interested in investigating this ?
> 
> I'm seeing this too. Did you manage to boot your Panda? If I disable
> CONFIG_USB_MUSB_HDRC, I don't see that error print, but the ethernet
> doesn't work, and so my nfsroot doesn't mount.

Do you use ethernet over USB gadget or over LAN9514 (smsx95xx) ? The LAN9514 
is connected to the EHCI USB interface so it shouldn't depend on MUSB.

I've attached my .config to this e-mail.

-- 
Regards,

Laurent Pinchart


.config.gz
Description: application/gzip

Re: [PATCH v2 1/3] ARM: davinci: da8xx: Fix ohci device name

[Sekhar Nori]

On Thursday 03 November 2016 09:33 PM, Axel Haslam wrote:
> While the clk lookup table is making reference to "ohci"
> other subsystems (such as phy) are trying to match "ohci.0"
> 
> Since there is a single ohci instance, instead of changing
> the clk name, change the dev id to -1, and add the "-da8xx"
> postfix to match the driver name that will also be changed
> in a subsequent patch.
> 
> Signed-off-by: Axel Haslam 

Applied to v4.10/soc

Thanks,
Sekhar
--
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: Issue with Telit LE922 and cdc_mbim

[Daniele Palmas]

Re-adding the list, sorry.

2016-11-23 15:49 GMT+01:00 Bjørn Mork :
> Daniele Palmas  writes:
>> 2016-11-21 10:49 GMT+01:00 Bjørn Mork :
>>> Daniele Palmas  writes:
>>>
 it turned out that resetting the interface in cdc_ncm_init after
 getting the NTB parameters removes the need for the sleep, making the
 modem to work fine.
>>>
>>> Sounds very good, although I must admit that it isn't perfectly clear to
>>> me what kind of reset we're talking about here.  But no worries, the
>>> patch will make that clear :)
>>>
>>
>> Sorry for the confusion, I meant resetting the MBIM function with
>> RESET_FUNCTION request code.
>
> Yes, the patch did make that clear, as expected :)
>
 I wonder if this is an acceptable solution and can be applied also for 
 MC7455.
>>>
>>> Quite possible.  I will definitely test it.  If we can avoid an
>>> arbitrary and pointless delay, then that's great.  But I guess this also
>>> requires testing with a wide range of other MBIM devices to find out if
>>> we can apply it unconditionally without breaking anything. Avoiding
>>> device-specific or vendor-specific code is important in a class driver,
>>> if possible.
>>>
>>
>> Ok, unfortunately I can test only with Telit modems, so I'm not sure
>> if the change I did is harmless for all the other modems:
>> RESET_FUNCTION should not cause issues,
>
> Agreed.  Unfortunately, we cannot depend on sane firmware behaviour ;)
>
> I did a semi-quick test on a Sierra Wireless EM7455 and can confirm that
> your patch makes it work without the delay.  Very nice.
>

Cool!

> Do you happen to know if the Windows MBIM driver use  RESET_FUNCTION
> during initialisation?  That would make this feel much safer.
>

Yes, Windows driver seems to send RESET_FUNCTION two times: after
getting NTB parameters and after setting NTB input size. But it seems
that only the first one is mandatory.

Windows USB traces taken with TotalPhase Datacenter are available at

https://drive.google.com/drive/folders/0B1kPnH2g8ISIZWJiV05qeWN5dVE

file LE922win10_2.tdc

> I see that your testing also included Intel based modems. That's good.
> It would still be nice to have test results from a few more MBIM
> implementations, in particular the more "difficult" ones. But I don't

I agree, especially Huawei ones for which the alternate setting
toggling was introduced.

> know how much help I can promise here. At the moment I don't even know
> which box my modems are in.  Moved a month ago and am still living in a
> box.  Or more like a hundred boxes ;)
>
> The easiest way to get this thoroughly tested is of course to push it
> now, and try to respond quickly in case it breaks something. Don't know
> what others think about that?
>
>> but I also removed altsetting
>> toggling for all MBIM modems and maybe this is not acceptable.
>
> I wonder about that.  It was added specifically for modems which seemed
> to need it.  Don't remember if we ever tried RESET_FUNCTION as an
> alternative. Removing workaorunds which have proven to be necessary at
> some point in time is a bit risky.
>

I totally agree, but the other way would be to add another quirk at
the cdc_mbim driver level, isn't it?

If this is a preferred solution I can try to modify the patch
according to this path.

Thanks,
Daniele

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

RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Hayes Wang]

Mark Lord [ml...@pobox.com]
[...]
> What does this code do:

> >static void r8153_set_rx_early_size(struct r8152 *tp)
> >{
> >u32 mtu = tp->netdev->mtu;
> >u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
> >
> >ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
> >}

This only works for RTL8153. However, what you use is RTL8152.
It is like delay completion. It is used to reduce the loading of CPU
by letting a transfer contain more data to reduce the number of
transfers.

> How is ocp_data used by the hardware?
> Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?

The algorithm is from our hw engineers, and it should be 

   (agg_buf_sz - packet size) / 8

You could refer to commit a59e6d815226 ("r8152: correct the rx early size").--
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: [PATCHv12 0/3] USB Type-C Connector class

[Heikki Krogerus]

Hi Guenter,

On Tue, Nov 22, 2016 at 04:11:44PM +0200, Heikki Krogerus wrote:
> The USB Type-C class is meant to provide unified interface to the
> userspace to present the USB Type-C ports in a system.
> 
> Changes since v11:
> - The port drivers are responsible of removing the alternate
>   modes (just like the documentation already said).

Can you check these, and give your ACK again if they OK.


Thanks,

-- 
heikki
--
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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Alan Stern]

On Wed, 23 Nov 2016, [ISO-8859-1] Bj�rn Mork wrote:

> On November 23, 2016 1:54:57 AM CET, Wim Osterholt  
> wrote:
> >On Tue, Nov 22, 2016 at 07:08:30PM +0100, Bjørn Mork wrote:
> >> > On kernel 4.8.8  this crashes hard and produces over a serial link:
> >> 
> >> Huh?  That device shouldn't ever enter that code path AFAICS.
> >> Unless you wouldn't happen to add a dynamic entry for this
> >device,
> >
> >No idea of what you mean here.
> >
> >> would you?  What's the output of
> >> 
> >>  cat /sys/bus/usb/drivers/cdc_acm/new_id
> >
> >Just empty.
> 
> Shit. Back to not understanding how you could possibly enter the debugging 
> code at all.

You're in the lucky position of having an engaged and responsive bug
reporter who is willing to apply patches and run tests.  Just write a
simple diagnostic patch that will reveal exactly what pathway is being
followed.

Alan Stern

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

Re: Problem with USB driver using two devices

[Greg KH]

On Wed, Nov 23, 2016 at 02:14:23PM +0100, Wolfgang Wilhelm wrote:
> Dear Sir,

Hi!

 and putting stable@
in bcc: as this has nothing to do with stable kernel releases.>

> 
> we are a small company FAST ComTec GmbH 
> (www.fastcomtec.com) and produce multichannel analyzers 
> with Windows software. I am the software developer and 
> would like to get our software working also under Linux 
> with the help of WINE.
> 
> I was already successfull with our USB devices and most
> of our PCI cards, but now we have found that our USB
> driver does not work with the second device if more than
> one USB device is connected. The "read" function does 
> not work with the second device, all other functions work.
> (tested with Debian v 8, kernel 3.16.0)
> 
> The driver is based on a skeleton source provided by you. 
> The driver source can be downloaded here:
> 
> https://www.fastcomtec.com/ftp/usb1mcs6.zip
> 
> Please could you have a look on it if you can see an error?
> Or do you think it could be an error in the Linux kernel?

This isn't a limitation in the kernel at all, something must be odd with
the driver.  We can review it a bit better if you want us to.

At first glance, the driver looks fine, I don't see anything obvious
that is making it only work for one device and not others.

But, it's a bit hard to tell, what type of error messages do you get
when you plug two devices in?  What is your userspace program expecting
to have happen that is not working properly here?

But, let's step back a bit, why do you need a kernel driver at all for
this device?  Can you just use libusb and a userspace program to control
your device instead?  That would get you a solution that worked on all
operating systems, without a need for a kernel driver for any of them.

If you really do want this to be a kernel driver, we will be glad to
review it in further detail if you wish for us to merge it into the main
kernel source tree, so that users don't have to download anything, and
then your device would "just work" with all future Linux kernel
releases.

Also note, I do see some security problems with your driver code, but
they aren't things that would cause multiple devices to not work, but
could cause problems for users with this driver loaded as you could tie
up resources in bad ways, or potentially crash the kernel easily.

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: Issue with Telit LE922 and cdc_mbim

[Bjørn Mork]

Daniele Palmas  writes:
> 2016-11-21 10:49 GMT+01:00 Bjørn Mork :
>> Daniele Palmas  writes:
>>
>>> it turned out that resetting the interface in cdc_ncm_init after
>>> getting the NTB parameters removes the need for the sleep, making the
>>> modem to work fine.
>>
>> Sounds very good, although I must admit that it isn't perfectly clear to
>> me what kind of reset we're talking about here.  But no worries, the
>> patch will make that clear :)
>>
>
> Sorry for the confusion, I meant resetting the MBIM function with
> RESET_FUNCTION request code.

Yes, the patch did make that clear, as expected :)

>>> I wonder if this is an acceptable solution and can be applied also for 
>>> MC7455.
>>
>> Quite possible.  I will definitely test it.  If we can avoid an
>> arbitrary and pointless delay, then that's great.  But I guess this also
>> requires testing with a wide range of other MBIM devices to find out if
>> we can apply it unconditionally without breaking anything. Avoiding
>> device-specific or vendor-specific code is important in a class driver,
>> if possible.
>>
>
> Ok, unfortunately I can test only with Telit modems, so I'm not sure
> if the change I did is harmless for all the other modems:
> RESET_FUNCTION should not cause issues,

Agreed.  Unfortunately, we cannot depend on sane firmware behaviour ;)

I did a semi-quick test on a Sierra Wireless EM7455 and can confirm that
your patch makes it work without the delay.  Very nice.

Do you happen to know if the Windows MBIM driver use  RESET_FUNCTION
during initialisation?  That would make this feel much safer.

I see that your testing also included Intel based modems. That's good.
It would still be nice to have test results from a few more MBIM
implementations, in particular the more "difficult" ones. But I don't
know how much help I can promise here. At the moment I don't even know
which box my modems are in.  Moved a month ago and am still living in a
box.  Or more like a hundred boxes ;)

The easiest way to get this thoroughly tested is of course to push it
now, and try to respond quickly in case it breaks something. Don't know
what others think about that?

> but I also removed altsetting
> toggling for all MBIM modems and maybe this is not acceptable.

I wonder about that.  It was added specifically for modems which seemed
to need it.  Don't remember if we ever tried RESET_FUNCTION as an
alternative. Removing workaorunds which have proven to be necessary at
some point in time is a bit risky.


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

Re: [PATCH v13 00/10] usbip: exporting devices

[Shuah Khan]

On 11/23/2016 02:30 AM, Greg KH wrote:
> On Tue, Nov 22, 2016 at 03:48:09PM +0900, Nobuo Iwata wrote:
>> Dear all,
>>
>> This series of patches adds exporting device operation to USB/IP.
> 
> I would _love_ it if some of the people who are listed as MAINTAINERS of
> this code could actually review these patch series.  I don't think I've
> seen that happen yet, which isn't good...
> 
> {hint hint hint}
> 
> greg k-h
> 

I started reviewing the patch series. Didn't get a chance to go
through all of them yet.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America(Silicon Valley)
shuah...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first

[Mathias Nyman]

On 23.11.2016 15:32, Guenter Roeck wrote:

On 11/23/2016 04:24 AM, Mathias Nyman wrote:

the tt_info provided by a HS hub might be in use to by a child device
Make sure we free the devices in the correct order.

This is needed in special cases such as when xhci controller is
reset when resuming from hibernate, and all virt_devices are freed.

Also free the virt_devices starting from max slot_id as children
more commonly have higher slot_id than parent.

CC: 
Signed-off-by: Mathias Nyman 

---

Guenter Roeck, does this work for you?

A rework of how tt_info is stored and used might be needed,
but that will take some time and won't go to stable as easily.


I'll give it a try. One concern, though: xhci_free_virt_device() is called
from multiple places, and does not always remove all devices. Is it save
to assume that all other callers remove children first ?



This should be the only place that xhci does a massive xhci_free_virt_device(),

In the other places it's initiated per device by usb core which should handle
child-parent relationships, or then just error paths when failing to allocate
a device in the first place (no children yet)

-Mathias




--
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: mark PM functions as __maybe_unused

[Tony Lindgren]

* Arnd Bergmann  [161122 06:30]:
> Building without CONFIG_PM causes a harmless warning:
> 
> drivers/usb/musb/musb_core.c:2041:12: error: ‘musb_run_resume_work’ defined 
> but not used [-Werror=unused-function]
> 
> Removing the #ifdef around the PM code and instead marking the suspend/resume
> functions as __maybe_unused will do the right thing without warning.

OK works for me:

Acked-by: Tony Lindgren 
--
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 2/3 v3] xhci: Fix race related to abort operation

[OGAWA Hirofumi]

ping?

OGAWA Hirofumi  writes:

> Current abort operation has race.
>
> xhci_handle_command_timeout()
>   xhci_abort_cmd_ring()
> xhci_write_64(CMD_RING_ABORT)
> xhci_handshake(5s)
> do {
>   check CMD_RING_RUNNING
> udelay(1)
>  ...
>  COMP_CMD_ABORT event
>  COMP_CMD_STOP event
>  xhci_handle_stopped_cmd_ring()
>restart cmd_ring
>  CMD_RING_RUNNING become 1 again
> } while ()
>   return -ETIMEDOUT
> xhci_write_64(CMD_RING_ABORT)
> /* can abort random command */
>
> To do abort operation correctly, we have to wait both of COMP_CMD_STOP
> event and negation of CMD_RING_RUNNING.
>
> But like above, while timeout handler is waiting negation of
> CMD_RING_RUNNING, event handler can restart cmd_ring. So timeout
> handler never be notice negation of CMD_RING_RUNNING, and retry of
> CMD_RING_ABORT can abort random command (BTW, I guess retry of
> CMD_RING_ABORT was workaround of this race).
>
> To fix this race, this moves xhci_handle_stopped_cmd_ring() to
> xhci_abort_cmd_ring().  And timeout handler waits COMP_CMD_STOP event.
>
> At this point, timeout handler is owner of cmd_ring, and safely
> restart cmd_ring by using xhci_handle_stopped_cmd_ring().
>
> [FWIW, as bonus, this way would be easily extend to add CMD_RING_PAUSE
> operation]
>
> Signed-off-by: OGAWA Hirofumi 
> ---
>
>  drivers/usb/host/xhci-mem.c  |1 
>  drivers/usb/host/xhci-ring.c |  163 
> ++
>  drivers/usb/host/xhci.h  |1 
>  3 files changed, 88 insertions(+), 77 deletions(-)
>
> diff -puN drivers/usb/host/xhci-mem.c~xhci-fix-abort-race2 
> drivers/usb/host/xhci-mem.c
> --- xhci/drivers/usb/host/xhci-mem.c~xhci-fix-abort-race2 2016-11-16 
> 18:38:52.551146764 +0900
> +++ xhci-hirofumi/drivers/usb/host/xhci-mem.c 2016-11-17 01:04:11.402014445 
> +0900
> @@ -2344,6 +2344,7 @@ int xhci_mem_init(struct xhci_hcd *xhci,
>  
>   /* init command timeout work */
>   INIT_DELAYED_WORK(>cmd_timer, xhci_handle_command_timeout);
> + init_completion(>cmd_ring_stop_completion);
>  
>   page_size = readl(>op_regs->page_size);
>   xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> diff -puN drivers/usb/host/xhci-ring.c~xhci-fix-abort-race2 
> drivers/usb/host/xhci-ring.c
> --- xhci/drivers/usb/host/xhci-ring.c~xhci-fix-abort-race22016-11-16 
> 18:38:52.552146764 +0900
> +++ xhci-hirofumi/drivers/usb/host/xhci-ring.c2016-11-17 
> 01:04:06.393018485 +0900
> @@ -284,6 +284,60 @@ static bool xhci_mod_cmd_timer(struct xh
>   return mod_delayed_work(system_wq, >cmd_timer, delay);
>  }
>  
> +static struct xhci_command *xhci_next_queued_cmd(struct xhci_hcd *xhci)
> +{
> + return list_first_entry_or_null(>cmd_list, struct xhci_command,
> + cmd_list);
> +}
> +
> +/*
> + * Turn all commands on command ring with status set to "aborted" to no-op 
> trbs.
> + * If there are other commands waiting then restart the ring and kick the 
> timer.
> + * This must be called with command ring stopped and xhci->lock held.
> + */
> +static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci,
> +  struct xhci_command *cur_cmd)
> +{
> + struct xhci_command *i_cmd;
> + u32 cycle_state;
> +
> + /* Turn all aborted commands in list to no-ops, then restart */
> + list_for_each_entry(i_cmd, >cmd_list, cmd_list) {
> +
> + if (i_cmd->status != COMP_CMD_ABORT)
> + continue;
> +
> + i_cmd->status = COMP_CMD_STOP;
> +
> + xhci_dbg(xhci, "Turn aborted command %p to no-op\n",
> +  i_cmd->command_trb);
> + /* get cycle state from the original cmd trb */
> + cycle_state = le32_to_cpu(
> + i_cmd->command_trb->generic.field[3]) & TRB_CYCLE;
> + /* modify the command trb to no-op command */
> + i_cmd->command_trb->generic.field[0] = 0;
> + i_cmd->command_trb->generic.field[1] = 0;
> + i_cmd->command_trb->generic.field[2] = 0;
> + i_cmd->command_trb->generic.field[3] = cpu_to_le32(
> + TRB_TYPE(TRB_CMD_NOOP) | cycle_state);
> +
> + /*
> +  * caller waiting for completion is called when command
> +  *  completion event is received for these no-op commands
> +  */
> + }
> +
> + xhci->cmd_ring_state = CMD_RING_STATE_RUNNING;
> +
> + /* ring command ring doorbell to restart the command ring */
> + if ((xhci->cmd_ring->dequeue != xhci->cmd_ring->enqueue) &&
> + !(xhci->xhc_state 

Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable

[Mark Lord]

What does this code do:

>static void r8153_set_rx_early_size(struct r8152 *tp)
>{
>u32 mtu = tp->netdev->mtu;
>u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
>
>ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
>}

How is ocp_data used by the hardware?
Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?

Thanks
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first

[Guenter Roeck]

On 11/23/2016 04:24 AM, Mathias Nyman wrote:

the tt_info provided by a HS hub might be in use to by a child device
Make sure we free the devices in the correct order.

This is needed in special cases such as when xhci controller is
reset when resuming from hibernate, and all virt_devices are freed.

Also free the virt_devices starting from max slot_id as children
more commonly have higher slot_id than parent.

CC: 
Signed-off-by: Mathias Nyman 

---

Guenter Roeck, does this work for you?

A rework of how tt_info is stored and used might be needed,
but that will take some time and won't go to stable as easily.


I'll give it a try. One concern, though: xhci_free_virt_device() is called
from multiple places, and does not always remove all devices. Is it save
to assume that all other callers remove children first ?

Thanks,
Guenter


---
 drivers/usb/host/xhci-mem.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6afe323..b3a5cd8 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int 
slot_id)
xhci->devs[slot_id] = NULL;
 }

+/*
+ * Free a virt_device structure.
+ * If the virt_device added a tt_info (a hub) and has children pointing to
+ * that tt_info, then free the child first. Recursive.
+ * We can't rely on udev at this point to find child-parent relationships.
+ */
+void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
+{
+   struct xhci_virt_device *vdev;
+   struct list_head *tt_list_head;
+   struct xhci_tt_bw_info *tt_info, *next;
+   int i;
+
+   vdev = xhci->devs[slot_id];
+   if (!vdev)
+   return;
+
+   tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
+   list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
+   /* is this a hub device that added a tt_info to the tts list */
+   if (tt_info->slot_id == slot_id) {
+   /* are any devices using this tt_info? */
+   for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+   vdev = xhci->devs[i];
+   if (vdev && (vdev->tt_info == tt_info))
+   xhci_free_virt_devices_depth_first(
+   xhci, i);
+   }
+   }
+   }
+   /* we are now at a leaf device */
+   xhci_free_virt_device(xhci, slot_id);
+}
+
 int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
struct usb_device *udev, gfp_t flags)
 {
@@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
}
}

-   for (i = 1; i < MAX_HC_SLOTS; ++i)
-   xhci_free_virt_device(xhci, i);
+   for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--)
+   xhci_free_virt_devices_depth_first(xhci, i);

dma_pool_destroy(xhci->segment_pool);
xhci->segment_pool = NULL;



--
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: Issue with Telit LE922 and cdc_mbim

[Daniele Palmas]

Hi Bjørn,

2016-11-21 10:49 GMT+01:00 Bjørn Mork :
> Daniele Palmas  writes:
>
>> it turned out that resetting the interface in cdc_ncm_init after
>> getting the NTB parameters removes the need for the sleep, making the
>> modem to work fine.
>
> Sounds very good, although I must admit that it isn't perfectly clear to
> me what kind of reset we're talking about here.  But no worries, the
> patch will make that clear :)
>

Sorry for the confusion, I meant resetting the MBIM function with
RESET_FUNCTION request code.

>> I wonder if this is an acceptable solution and can be applied also for 
>> MC7455.
>
> Quite possible.  I will definitely test it.  If we can avoid an
> arbitrary and pointless delay, then that's great.  But I guess this also
> requires testing with a wide range of other MBIM devices to find out if
> we can apply it unconditionally without breaking anything. Avoiding
> device-specific or vendor-specific code is important in a class driver,
> if possible.
>

Ok, unfortunately I can test only with Telit modems, so I'm not sure
if the change I did is harmless for all the other modems:
RESET_FUNCTION should not cause issues, but I also removed altsetting
toggling for all MBIM modems and maybe this is not acceptable.

Thanks,
Daniele

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

[PATCH 1/1] NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying ncm bind common code

[Daniele Palmas]

Some latest QC based modems seem not to properly accept altsetting
toggling in cdc_ncm_bind_common, making them to fail. The workaround
was to introduce an empirically decided pause to avoid the failure.

This patch introduces a different approach: for MBIM devices, instead
of toggling interfaces, the MBIM class-specific request code
RESET_FUNCTION is used in order to reset the function to its initial
state, removing the need for the pause.

Signed-off-by: Daniele Palmas 
---
 drivers/net/usb/cdc_ncm.c| 39 +++
 include/uapi/linux/usb/cdc.h |  1 +
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 877c951..e8a7a76 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -498,6 +498,22 @@ static int cdc_ncm_init(struct usbnet *dev)
return err; /* GET_NTB_PARAMETERS is required */
}
 
+   /* Some modems (e.g. Telit LE922A6) need to reset the MBIM function
+* or they will fail to work properly.
+* For details on RESET_FUNCTION request see document
+* "USB Communication Class Subclass Specification for MBIM"
+* RESET_FUNCTION should be harmless for all the other MBIM modems
+*/
+   if (cdc_ncm_comm_intf_is_mbim(ctx->control->cur_altsetting)) {
+   err = usbnet_write_cmd(dev, USB_CDC_RESET_FUNCTION,
+  USB_TYPE_CLASS | USB_DIR_OUT
+  | USB_RECIP_INTERFACE,
+  0, iface_no, NULL, 0);
+   if (err < 0) {
+   dev_err(>intf->dev, "failed RESET_FUNCTION\n");
+   }
+   }
+
/* set CRC Mode */
if (cdc_ncm_flags(dev) & USB_CDC_NCM_NCAP_CRC_MODE) {
dev_dbg(>intf->dev, "Setting CRC mode off\n");
@@ -842,25 +858,24 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
/* Reset data interface. Some devices will not reset properly
 * unless they are configured first.  Toggle the altsetting to
 * force a reset
+* This is applied only to ncm devices, since it has been verified
+* to cause issues with some MBIM modems (e.g. Telit LE922A6).
+* MBIM devices reset is achieved using MBIM request RESET_FUNCTION
+* in cdc_ncm_init
 */
-   usb_set_interface(dev->udev, iface_no, data_altsetting);
-   temp = usb_set_interface(dev->udev, iface_no, 0);
-   if (temp) {
-   dev_dbg(>dev, "set interface failed\n");
-   goto error2;
+   if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting)) {
+   usb_set_interface(dev->udev, iface_no, data_altsetting);
+   temp = usb_set_interface(dev->udev, iface_no, 0);
+   if (temp) {
+   dev_dbg(>dev, "set interface failed\n");
+   goto error2;
+   }
}
 
/* initialize basic device settings */
if (cdc_ncm_init(dev))
goto error2;
 
-   /* Some firmwares need a pause here or they will silently fail
-* to set up the interface properly.  This value was decided
-* empirically on a Sierra Wireless MC7455 running 02.08.02.00
-* firmware.
-*/
-   usleep_range(1, 2);
-
/* configure data interface */
temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
if (temp) {
diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index e2bc417..30258fb 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -231,6 +231,7 @@ struct usb_cdc_mbim_extended_desc {
 
 #define USB_CDC_SEND_ENCAPSULATED_COMMAND  0x00
 #define USB_CDC_GET_ENCAPSULATED_RESPONSE  0x01
+#define USB_CDC_RESET_FUNCTION 0x05
 #define USB_CDC_REQ_SET_LINE_CODING0x20
 #define USB_CDC_REQ_GET_LINE_CODING0x21
 #define USB_CDC_REQ_SET_CONTROL_LINE_STATE 0x22
-- 
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/1] NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying ncm bind common code

[Daniele Palmas]

Some latest QC based modems seem not to properly accept altsetting
toggling in cdc_ncm_bind_common, making them to fail. The workaround
was to introduce an empirically decided pause to avoid the failure.

This patch introduces a different approach: for MBIM devices, instead
of toggling interfaces, the MBIM class-specific request code
RESET_FUNCTION is used in order to reset the function to its initial
state, removing the need for the pause.

Patch has been tested with a few Telit QC and Intel based MBIM modems.

Patch has also been tested with an Intel NCM based device, for
regression checking.

Daniele Palmas (1):
  NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying
ncm bind common code

 drivers/net/usb/cdc_ncm.c| 39 +++
 include/uapi/linux/usb/cdc.h |  1 +
 2 files changed, 28 insertions(+), 12 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

[RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first

[Mathias Nyman]

the tt_info provided by a HS hub might be in use to by a child device
Make sure we free the devices in the correct order.

This is needed in special cases such as when xhci controller is
reset when resuming from hibernate, and all virt_devices are freed.

Also free the virt_devices starting from max slot_id as children
more commonly have higher slot_id than parent.

CC: 
Signed-off-by: Mathias Nyman 

---

Guenter Roeck, does this work for you?

A rework of how tt_info is stored and used might be needed,
but that will take some time and won't go to stable as easily.
---
 drivers/usb/host/xhci-mem.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6afe323..b3a5cd8 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int 
slot_id)
xhci->devs[slot_id] = NULL;
 }
 
+/*
+ * Free a virt_device structure.
+ * If the virt_device added a tt_info (a hub) and has children pointing to
+ * that tt_info, then free the child first. Recursive.
+ * We can't rely on udev at this point to find child-parent relationships.
+ */
+void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
+{
+   struct xhci_virt_device *vdev;
+   struct list_head *tt_list_head;
+   struct xhci_tt_bw_info *tt_info, *next;
+   int i;
+
+   vdev = xhci->devs[slot_id];
+   if (!vdev)
+   return;
+
+   tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
+   list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
+   /* is this a hub device that added a tt_info to the tts list */
+   if (tt_info->slot_id == slot_id) {
+   /* are any devices using this tt_info? */
+   for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+   vdev = xhci->devs[i];
+   if (vdev && (vdev->tt_info == tt_info))
+   xhci_free_virt_devices_depth_first(
+   xhci, i);
+   }
+   }
+   }
+   /* we are now at a leaf device */
+   xhci_free_virt_device(xhci, slot_id);
+}
+
 int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
struct usb_device *udev, gfp_t flags)
 {
@@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
}
}
 
-   for (i = 1; i < MAX_HC_SLOTS; ++i)
-   xhci_free_virt_device(xhci, i);
+   for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--)
+   xhci_free_virt_devices_depth_first(xhci, i);
 
dma_pool_destroy(xhci->segment_pool);
xhci->segment_pool = NULL;
-- 
1.9.1

--
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/2] usb: dwc3: core: Support the dwc3 host suspend/resume

[Baolin Wang]

For some mobile devices with strict power management, we also want to suspend
the host when the slave is detached for power saving. Thus we add the host
suspend/resume functions to support this requirement.

Signed-off-by: Baolin Wang 
---
Changes since v2:
 - Remove pm_children_suspended() and other unused macros.

Changes since v1:
  - Add pm_runtime.h head file to avoid kbuild error.
---
 drivers/usb/dwc3/Kconfig |7 +++
 drivers/usb/dwc3/core.c  |   26 +-
 drivers/usb/dwc3/core.h  |   15 +++
 drivers/usb/dwc3/host.c  |   37 +
 4 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index a45b4f1..47bb2f3 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE
 
 endchoice
 
+config USB_DWC3_HOST_SUSPEND
+   bool "Choose if the DWC3 host (xhci) can be suspend/resume"
+   depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
+   help
+ We can suspend the host when the slave is detached for power saving,
+ and resume the host when one slave is attached.
+
 comment "Platform Glue Driver Support"
 
 config USB_DWC3_OMAP
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9a4a5e4..7ad4bc3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
pm_runtime_enable(dev);
+   pm_suspend_ignore_children(dev, true);
ret = pm_runtime_get_sync(dev);
if (ret < 0)
goto err1;
@@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev)
 static int dwc3_suspend_common(struct dwc3 *dwc)
 {
unsigned long   flags;
+   int ret;
 
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
+   spin_lock_irqsave(>lock, flags);
+   dwc3_gadget_suspend(dwc);
+   spin_unlock_irqrestore(>lock, flags);
+   break;
case USB_DR_MODE_OTG:
+   ret = dwc3_host_suspend(dwc);
+   if (ret)
+   return ret;
+
spin_lock_irqsave(>lock, flags);
dwc3_gadget_suspend(dwc);
spin_unlock_irqrestore(>lock, flags);
break;
case USB_DR_MODE_HOST:
+   ret = dwc3_host_suspend(dwc);
+   if (ret)
+   return ret;
default:
/* do nothing */
break;
@@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc)
 
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
+   spin_lock_irqsave(>lock, flags);
+   dwc3_gadget_resume(dwc);
+   spin_unlock_irqrestore(>lock, flags);
+   break;
case USB_DR_MODE_OTG:
+   ret = dwc3_host_resume(dwc);
+   if (ret)
+   return ret;
+
spin_lock_irqsave(>lock, flags);
dwc3_gadget_resume(dwc);
spin_unlock_irqrestore(>lock, flags);
-   /* FALLTHROUGH */
+   break;
case USB_DR_MODE_HOST:
+   ret = dwc3_host_resume(dwc);
+   if (ret)
+   return ret;
default:
/* do nothing */
break;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index b585a30..db41908 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
 { }
 #endif
 
+#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND)
+int dwc3_host_suspend(struct dwc3 *dwc);
+int dwc3_host_resume(struct dwc3 *dwc);
+#else
+static inline int dwc3_host_suspend(struct dwc3 *dwc)
+{
+   return 0;
+}
+
+static inline int dwc3_host_resume(struct dwc3 *dwc)
+{
+   return 0;
+}
+#endif
+
 #endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index ed82464..8e5309d6 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -16,6 +16,7 @@
  */
 
 #include 
+#include 
 
 #include "core.h"
 
@@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc)
  dev_name(>xhci->dev));
platform_device_unregister(dwc->xhci);
 }
+
+#ifdef CONFIG_USB_DWC3_HOST_SUSPEND
+int dwc3_host_suspend(struct dwc3 *dwc)
+{
+   struct device *xhci = >xhci->dev;
+   int ret;
+
+   /*
+* Note: if we get the -EBUSY, which means the xHCI children devices are
+* not in suspend state yet, the glue layer need to wait for a while and
+* try to suspend xHCI device again.
+*/
+   ret = pm_runtime_put_sync(xhci);
+   if 

[PATCH v3 1/2] usb: host: plat: Enable xhci plat runtime PM

[Baolin Wang]

Enable the xhci plat runtime PM for parent device to suspend/resume xhci.
Also call pm_runtime_get_noresume() in probe() function in case the parent
device doesn't call suspend/resume callback by runtime PM now.

Signed-off-by: Baolin Wang 
---
Changes since v2:
 - Add pm_runtime_get_noresume() in probe() function.
 - Add pm_runtime_set_suspended()/pm_runtime_put_noidle() in remove() function.

Changes since v1:
 - No updates.
---
 drivers/usb/host/xhci-plat.c |   41 -
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..f174502 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 
@@ -274,6 +278,10 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
 
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
+   pm_runtime_disable(>dev);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
@@ -311,14 +319,37 @@ static int xhci_plat_resume(struct device *dev)
 
return xhci_resume(xhci, 0);
 }
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int xhci_plat_runtime_suspend(struct device *dev)
+{
+   struct usb_hcd  *hcd = dev_get_drvdata(dev);
+   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+   return xhci_suspend(xhci, device_may_wakeup(dev));
+}
+
+static int xhci_plat_runtime_resume(struct device *dev)
+{
+   struct usb_hcd  *hcd = dev_get_drvdata(dev);
+   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+   return xhci_resume(xhci, 0);
+}
+
+static int xhci_plat_runtime_idle(struct device *dev)
+{
+   return 0;
+}
+#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
+
+   SET_RUNTIME_PM_OPS(xhci_plat_runtime_suspend, xhci_plat_runtime_resume,
+  xhci_plat_runtime_idle)
 };
-#define DEV_PM_OPS (_plat_pm_ops)
-#else
-#define DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
 
 static const struct acpi_device_id usb_xhci_acpi_match[] = {
/* XHCI-compliant USB Controller */
@@ -332,7 +363,7 @@ static int xhci_plat_resume(struct device *dev)
.remove = xhci_plat_remove,
.driver = {
.name = "xhci-hcd",
-   .pm = DEV_PM_OPS,
+   .pm = _plat_pm_ops,
.of_match_table = of_match_ptr(usb_xhci_of_match),
.acpi_match_table = ACPI_PTR(usb_xhci_acpi_match),
},
-- 
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

did you receive my previous email ?

[Friedrich Mayrhofer]

This is the second time i am sending you this mail. I, Friedrich Mayrhofer 
Donate $ 1,000,000.00 to You, Email  Me personally for more details.


Regards.
Friedrich Mayrhofer

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

Re: [PATCH 0/4] musb fixes for v4.9-rc cycle

[Tomi Valkeinen]

On 10/11/16 23:25, Laurent Pinchart wrote:

 [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while
 active

 printed in a loop at boot time. I've traced musb->is_active being set to
 1 in musb_start() with

> Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite 
> understandably. It's not a fix though, just a workaround. Are you interested 
> in investigating this ?

I'm seeing this too. Did you manage to boot your Panda? If I disable
CONFIG_USB_MUSB_HDRC, I don't see that error print, but the ethernet
doesn't work, and so my nfsroot doesn't mount.

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v13 00/10] usbip: exporting devices

[Greg KH]

On Tue, Nov 22, 2016 at 03:48:09PM +0900, Nobuo Iwata wrote:
> Dear all,
> 
> This series of patches adds exporting device operation to USB/IP.

I would _love_ it if some of the people who are listed as MAINTAINERS of
this code could actually review these patch series.  I don't think I've
seen that happen yet, which isn't good...

{hint hint hint}

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 v2 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx

[Greg KH]

On Mon, Nov 21, 2016 at 06:10:50PM +0100, Axel Haslam wrote:
> Hi Greg,
> 
> On Thu, Nov 3, 2016 at 5:03 PM, Axel Haslam  wrote:
> > The davinci ohci driver name (currently "ohci") is too generic.
> > To be consistent with other usb dirvers, append the "-da8xx" postfix
> > to the name.
> >
> 
> if there are no objections, would it be possible to pick up this patch?
> the corresponding phy patch was merged and the platform changes
> are ack'ed, and will we taken by the davinci maintainer once this patch
> gets in.

Now applied.

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: gadget: uvc: fix returnvar.cocci warnings

[Andrzej Pietrasiewicz]

Hi Laurent,

Thanks for a reminder.  Please see inline.

W dniu 22.11.2016 o 18:27, Laurent Pinchart pisze:

Hi Andrzej and Julia,

Could one of you please submit a patch to fix this ?

On Thursday 17 Sep 2015 13:18:04 Andrzej Pietrasiewicz wrote:

Hi Julia,

W dniu 17.09.2015 o 10:57, Julia Lawall pisze:

Coccinelle suggests the following patch.  But the code is curious.  Is the
function expected to always return a failure value?


As a matter of fact it seems it should not return anything at all,
because...



Thank you for catching this. The function is not expected to always
return a failure value. Fortunately it does not matter anyway because


...because

the return value of the drop_link() operation is silently ignored by


And the Documentation/filesystems/configfs/configfs.txt says here:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/configfs/configfs.txt#n397

"When unlink(2) is called on the symbolic link, the source item is
notified via the ->drop_link() method.  Like the ->drop_item() method,
this is a void function and cannot return failure."

The ->drop_item() is indeed a void function, the ->drop_link() is
actually not. This, together with the fact that the value of ->drop_link()
is silently ignored suggests, that it is the ->drop_link() return
type that should be corrected and changed to void.

@Joel: What is your opinion? Should return type be changed to void?
Is there any reason why it should still be declared int?

I'm sending a copy of this mail to target-devel and linux-nvme,
because other potentially affected users of configfs live there.

AP
--
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] fsl/usb: Add FSL USB Gadget entry in platform device id table

[Changming Huang]

Add FSL USB Gadget entry in platform device id table

Signed-off-by: Changming Huang 
Signed-off-by: Suresh Gupta 
---
 drivers/usb/gadget/udc/fsl_udc_core.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index aab5221..b24b1c9 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2671,6 +2671,8 @@ static int fsl_udc_otg_resume(struct device *dev)
}, {
.name = "imx-udc-mx51",
}, {
+   .name = "fsl-usb2-udc",
+   }, {
/* sentinel */
}
 };
-- 
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

Re: [PATCH] fsl/usb: Add FSL USB Gadget entry in platform device id table

[Felipe Balbi]

Hi,

Changming Huang  writes:
> Add FSL USB Gadget entry in platform device id table
>
> Signed-off-by: Changming Huang 
> Signed-off-by: Suresh Gupta 
> ---
>  drivers/usb/gadget/udc/fsl_udc_core.c |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
> b/drivers/usb/gadget/udc/fsl_udc_core.c
> index aab5221..b24b1c9 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -2671,6 +2671,8 @@ static int fsl_udc_otg_resume(struct device *dev)
>   }, {
>   .name = "imx-udc-mx51",
>   }, {
> + .name = "fsl-usb2-udc",

please use chipidea, instead. No more new users for this driver.

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