Re: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Takashi Iwai
On Mon, 25 Sep 2017 14:39:51 +0200,
Andrey Konovalov wrote:
> 
> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2).
> 
> It seems that there's no check of the endpoint type.
> 
> usb 1-1: BOGUS urb xfer, pipe 1 != type 3
> [ cut here ]
> WARNING: CPU: 0 PID: 1846 at drivers/usb/core/urb.c:449

How is this bug triggered?  As it's syzkaller with QEMU, it looks
hitting an inconsistent state the driver didn't expect (it sets the
fixed endpoint), then USB-core detects the inconsistency and spews the
kernel warning with stack trace.  If so, it's no serious problem as it
appears.

Suppose my guess is right, I'm not sure what's the best way to fix
this.  Certainly we can add more sanity check in the caller side.
OTOH, I find the reaction of USB core too aggressive, it's not
necessary to be dev_WARN() but a normal dev_err().
Or I might be looking at a wrong place?

Adding USB guys to Cc for hearing their comments.


thanks,

Takashi


> usb_submit_urb+0xf8a/0x11d0
> Modules linked in:
> CPU: 0 PID: 1846 Comm: kworker/0:2 Not tainted
> 4.14.0-rc2-42613-g1488251d1a98 #238
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> task: 880064296300 task.stack: 8800643b
> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
> RSP: 0018:8800643b6140 EFLAGS: 00010286
> RAX: 0029 RBX: 880063842400 RCX: 
> RDX: 0029 RSI: 85a58800 RDI: ed000c876c1a
> RBP: 8800643b6240 R08: 11000c876ac0 R09: 
> R10:  R11:  R12: 11000c876c2f
> R13: 0003 R14: 0001 R15: 88006325c768
> FS:  () GS:88006c80() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7fa2a51c CR3: 6acc8000 CR4: 06f0
> Call Trace:
>  bcd2000_init_device sound/usb/bcd2000/bcd2000.c:289
>  bcd2000_init_midi sound/usb/bcd2000/bcd2000.c:345
>  bcd2000_probe+0xe64/0x19e0 sound/usb/bcd2000/bcd2000.c:406
>  usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
>  really_probe drivers/base/dd.c:413
>  driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
>  __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
>  bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
>  __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
>  device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
>  bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
>  device_add+0xd0b/0x1660 drivers/base/core.c:1835
>  usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
>  generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
>  usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
>  really_probe drivers/base/dd.c:413
>  driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
>  __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
>  bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
>  __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
>  device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
>  bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
>  device_add+0xd0b/0x1660 drivers/base/core.c:1835
>  usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
>  hub_port_connect drivers/usb/core/hub.c:4903
>  hub_port_connect_change drivers/usb/core/hub.c:5009
>  port_event drivers/usb/core/hub.c:5115
>  hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
>  process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
>  worker_thread+0x221/0x1850 kernel/workqueue.c:2253
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 6e 91 8b ff 45 89
> e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 c0 5a c8 85 e8 10 50 dd fd <0f>
> ff e9 9b f7 ff ff e8 ba cf 26 fe e9 80 f7 ff ff e8 a0 a5 f4
> ---[ end trace bad127706d5fe2d6 ]---
> 
--
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 05/18] net: use ARRAY_SIZE

2017-10-03 Thread Andy Shevchenko
On Tue, Oct 3, 2017 at 4:22 AM, Jérémy Lefaure
 wrote:
> On Mon, 2 Oct 2017 16:07:36 +0300
> Andy Shevchenko  wrote:
>
>> > +   {&gainctrl_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 
>> > 26, 192,
>> > +32},
>>
>> For all such cases I would rather put on one line disregard checkpatch
>> warning for better readability.
> I agree that it would be better. I didn't know that it was possible to
> not follow this rule for anything else than a string.

IMO, it increases readability quite enough to overrule checkpatch recomendation.

-- 
With Best Regards,
Andy Shevchenko
--
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: Support for buggy MIDI-keyboard VID:PID 1c75:0204

2017-10-03 Thread Felipe Balbi

Hi,

(we don't top-post here :-)

Владимир Мартьянов  writes:
> Oh, I didn't know about quirks.c. Your code solved the problem, thank you!

alright, I'll send it as a proper patch and add yourself as "Reported-by".

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

2017-10-03 Thread Jerome Brunet
On Mon, 2017-10-02 at 14:44 +0200, Greg KH wrote:
> On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
> > On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> > > Hello Greg, Hello Mathias,
> > > 
> > > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH 
> > > wrote:
> > > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > > > Hello Mathias, Hello Greg,
> > > > > 
> > > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > > >  wrote:
> > > > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > > > see [0] and [1].
> > > > > > The quick-summary of this is:
> > > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > > > >   correct
> > > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c
> > > > > > and
> > > > > >   ohci-platform.c) do not have a limitation on the number of PHYs -
> > > > > > they
> > > > > >   support one PHY per actual host port
> > > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has
> > > > > > two
> > > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs
> > > > > > also
> > > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > > > >   otherwise USB devices cannot be enumerated (even if just one PHY
> > > > > > is
> > > > > >   disabled and if the device is plugged into another, enabled port)
> > > > > > 
> > > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I
> > > > > > tried
> > > > > > to work-around the problem that I could not pass multiple PHYs to
> > > > > > the
> > > > > > dwc3 controller.
> > > > > > This was rejected by Rob Herring (which was definitely the thing to
> > > > > > do
> > > > > > in
> > > > > > my opinion), see [2]
> > > > > > 
> > > > > > This series adds a new "platform-roothub". This can be configured
> > > > > > through
> > > > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > > > controller. Additionally there has to be a child-node for each port
> > > > > > on
> > > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > > > property. This allows modeling the root-hub in devicetree similar to
> > > > > > the
> > > > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > > > device.txt)
> > > > > > This avoids and backwards-compatibility problems (which was a
> > > > > > concern
> > > > > > regardless of the solution, see [3]) since the binding for the root-
> > > > > > hub
> > > > > > was previously not specified (and we're not using the "phys"
> > > > > > property of
> > > > > > the controller, which might have served different purposes before,
> > > > > > depending on the drivers).
> > > > > > 
> > > > > > Additionally this integrates the new platform-roothub into xhci-
> > > > > > plat.c
> > > > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > > > 
> > > > > > 
> > > > > > Changes since RFCv3 at [6]:
> > > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > > > >   by Rob Herring (and slightly adjusted the commit message to
> > > > > > account
> > > > > >   for that)
> > > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > > > >   concept and implementation works fine on Mediatek SoCs - many
> > > > > > thanks
> > > > > >   again!) to patch #2
> > > > > > - added Rob Herring's ACK to patches 1 and 3
> > > > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > > > 
> > > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > > > patches are still identical to v4 after rebasing).
> > > > > 
> > > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > > > Mediatek (= non-Amlogic) SoC.
> > > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > > > are just waiting on this series.
> > > > > 
> > > > > what is still missing to get this series into v4.15?
> > > > 
> > > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > > > us catch up on patch review please...
> > > 
> > > OK, I understand that.
> > > please let me know once you've caught up with the review backlog - as
> > > I said I would like to get this into 4.15 if nothing else comes up
> > > during the code-review
> > 
> > This series works well on the libretech-cc (le potato)
> > For the series:
> > 
> > Tested-by: Jerome Brunet 
> 
> Hey, I have one of those boards now, was just about to try to get it to
> work.  Is this series necessary for it to run properly on 4.14-rc?

To run w/o usb, no. It should already be decent on 4.14-rc.
To run with usb, you need this series and:

* the usb3 phy driver sent but (I believe) not yet merged [0]
* A few DT changes enable the whole thing.


[PATCH] usb: quirks: add quirk for WORLDE MINI MIDI keyboard

2017-10-03 Thread Felipe Balbi
This keyboard doesn't implement Get String descriptors properly even
though string indexes are valid. What happens is that when requesting
for the String descriptor, the device disconnects and
reconnects. Without this quirk, this loop will continue forever.

Cc: Alan Stern 
Reported-by: Владимир Мартьянов 
Signed-off-by: Felipe Balbi 
---
 drivers/usb/core/quirks.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 82806e311202..a6aaf2f193a4 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -221,6 +221,10 @@ static const struct usb_device_id usb_quirk_list[] = {
/* Corsair Strafe RGB */
{ USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
 
+   /* MIDI keyboard WORLDE MINI */
+   { USB_DEVICE(0x1c75, 0x0204), .driver_info =
+   USB_QUIRK_CONFIG_INTF_STRINGS },
+
/* Acer C120 LED Projector */
{ USB_DEVICE(0x1de1, 0xc102), .driver_info = USB_QUIRK_NO_LPM },
 
-- 
2.14.2.642.g20fed7cad4

--
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: quirks: add quirk for WORLDE MINI MIDI keyboard

2017-10-03 Thread David Laight
From: Felipe Balbi
> Sent: 03 October 2017 09:17
> This keyboard doesn't implement Get String descriptors properly even
> though string indexes are valid. What happens is that when requesting
> for the String descriptor, the device disconnects and
> reconnects. Without this quirk, this loop will continue forever.

If this is a common/known problem, could the driver detect the
disconnect sequence and avoid reading the string descriptors
on the subsequent reconnect?

Then the quirk list wouldn't need to be updated for new id.

David



Re: [PATCH] usb: quirks: add quirk for WORLDE MINI MIDI keyboard

2017-10-03 Thread Greg Kroah-Hartman
On Tue, Oct 03, 2017 at 08:19:52AM +, David Laight wrote:
> From: Felipe Balbi
> > Sent: 03 October 2017 09:17
> > This keyboard doesn't implement Get String descriptors properly even
> > though string indexes are valid. What happens is that when requesting
> > for the String descriptor, the device disconnects and
> > reconnects. Without this quirk, this loop will continue forever.
> 
> If this is a common/known problem, could the driver detect the
> disconnect sequence and avoid reading the string descriptors
> on the subsequent reconnect?
> 
> Then the quirk list wouldn't need to be updated for new id.

Patches are always welcome, but this might be a bit complex...

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: Random xHCI HC died on device disconnect

2017-10-03 Thread Mathias Nyman

On 02.10.2017 22:44, Kristian Evensen wrote:

Hello,

I am currently working on a custom Armada 385-based board running
kernel 4.9.52. I have a USB 2.0 LTE modem connected to USB 3.0, and
the modem sometimes crashes due to various internal (on the modem)
reasons. The board supports restarting the USB-ports using GPIOs, and
at random points when I restart the USB-port I get the following error
in dmesg:

[12446.639458] qmi_wwan 4-1:1.4: nonzero urb status received: -71
[12446.639462] qmi_wwan 4-1:1.4: wdm_int_callback - 0 bytes
[12446.639466] qmi_wwan 4-1:1.4: wdm_int_callback - usb_submit_urb
failed with result -19
[12446.658760] option1 ttyUSB0: GSM modem (1-port) converter now
disconnected from ttyUSB0
[12446.666837] option 4-1:1.0: device disconnected
[12446.671576] option1 ttyUSB1: GSM modem (1-port) converter now
disconnected from ttyUSB1
[12446.679686] option 4-1:1.1: device disconnected
[12451.763245] xhci-hcd f10f8000.usb3: xHCI host not responding to
stop endpoint command.
[12451.771187] xhci-hcd f10f8000.usb3: Assuming host is dying, halting host.
[12451.778027] xhci-hcd f10f8000.usb3: HC died; cleaning up
[12451.783589] option1 ttyUSB2: GSM modem (1-port) converter now
disconnected from ttyUSB2
[12451.791654] option 4-1:1.2: device disconnected
[12451.796416] option1 ttyUSB3: GSM modem (1-port) converter now
disconnected from ttyUSB3
[12451.804530] option 4-1:1.3: device disconnected
[12451.809206] qmi_wwan 4-1:1.4 wwan0: unregister 'qmi_wwan'
usb-f10f8000.usb3-1, WWAN/QMI device

After this, the port is dead until I reboot the device and it is
sufficient to do a soft reboot in order to bring the port back. Most
of the time, restarting the port works fine and the time it takes for
the error to occur seems random.

In order to rule out (or at least reduce the probability of) that
there is something wrong with the board I am testing, I found two
other Armada-boards with upstream-support and compiled kernel 4.14-rc3
for them. The modem can be rebooted using AT-commands, so I created a
small script which contained a loop where I rebooted the modem as soon
as possible. Doing this, I was able to replicate the error I saw on my
custom board-

Does anyone have any tips on where to start looking, how to solve this
issue or how to work around it? For me, if there is a way to restart
the host controller, then that would be sufficient as I only have one
USB device connected. When searching online, I find lots of references
to similar issues, but they all (or at least the ones where a solution
has been found) seem to related to suspend/resume.


For temporary workarounds:
if xhci is a module then unload and reload in using modprobe

Or then unbind/rebind the driver and device.
on a Intel PCI based xhci it would be something like:
echo :00:14.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
echo :00:14.0 > /sys/bus/pci/drivers/xhci_hcd/bind

On your Armada xhci might be found under the platform bus instead of PCI,
and ID won't be :00:14.0, but you get the idea.

If you want to try messing around with the driver itself you could try to
just disable the xhci stop command timer.
The timer is there for a reason, but it's also possible that your command queue 
is
just clogged because its trying to address the faulty device.
You could play around and remove the timer and see what happens:

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index df0a82b..b0e7083 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1518,7 +1518,6 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct 
urb *urb, int status)
ep->ep_state |= EP_STOP_CMD_PENDING;
ep->stop_cmd_timer.expires = jiffies +
XHCI_STOP_EP_CMD_TIMEOUT * HZ;
-   add_timer(&ep->stop_cmd_timer);
xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
 ep_index, 0);
xhci_ring_cmd_db(xhci);

-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] extcon: Split out extcon header file for consumer and provider device

2017-10-03 Thread Charles Keepax
On Fri, Sep 29, 2017 at 09:01:45AM +0900, Chanwoo Choi wrote:
> The extcon has two type of extcon devices as following.
> - 'extcon provider deivce' adds new extcon device and detect the
>state/properties of external connector. Also, it notifies the
>state/properties to the extcon consumer device.
> - 'extcon consumer device' gets the change state/properties
>from extcon provider device.
> Prior to that, include/linux/extcon.h contains all exported API for
> both provider and consumer device driver. To clarify the meaning of
> header file and to remove the wrong use-case on consumer device,
> this patch separates into extcon.h and extcon-provider.h.
> 
> [Description for include/linux/{extcon.h|extcon-provider.h}]
> - extcon.h includes the extcon API and data structure for extcon consumer
>   device driver. This header file contains the following APIs:
>   : Register/unregister the notifier to catch the change of extcon device
>   : Get the extcon device instance
>   : Get the extcon device name
>   : Get the state of each external connector
>   : Get the property value of each external connector
>   : Get the property capability of each external connector
> 
> - extcon-provider.h includes the extcon API and data structure for extcon
>   provider device driver. This header file contains the following APIs:
>   : Include 'include/linux/extcon.h'
>   : Allocate the memory for extcon device instance
>   : Register/unregister extcon device
>   : Set the state of each external connector
>   : Set the property value of each external connector
>   : Set the property capability of each external connector
> 
> Cc: Felipe Balbi 
> Cc: Kishon Vijay Abraham I 
> Cc: Greg Kroah-Hartman 
> Cc: Sebastian Reichel 
> Cc: Lee Jones 
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon-arizona.c   |   2 +-

Acked-by: Charles Keepax 

Thanks,
Charles
--
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: Random xHCI HC died on device disconnect

2017-10-03 Thread Kristian Evensen
Hi Mathias,

Thank you very much for your reply.

On Tue, Oct 3, 2017 at 10:28 AM, Mathias Nyman
 wrote:
> For temporary workarounds:
> if xhci is a module then unload and reload in using modprobe
>
> Or then unbind/rebind the driver and device.
> on a Intel PCI based xhci it would be something like:
> echo :00:14.0 > /sys/bus/pci/drivers/xhci_hcd/unbind
> echo :00:14.0 > /sys/bus/pci/drivers/xhci_hcd/bind

xhci is unfortunately not a module on my system, but I will try to
build it is a module today. I did try bind/unbind yesterday, but
unfortunately without any success. unbind worked fine, but bind only
caused an error message. I can't remember the exact error message (I
unfortunately forgot to write it down), but the code was -22 and I
think the message was something that communication with hub failed.

> If you want to try messing around with the driver itself you could try to
> just disable the xhci stop command timer.
> The timer is there for a reason, but it's also possible that your command
> queue is
> just clogged because its trying to address the faulty device.
> You could play around and remove the timer and see what happens:

Messing with the driver is no problem :) Thanks for this pointer, I am
testing stopping the timer now and will let you know how it goes.

I performed some additional tests yesterday and this morning, and it
seems you are right in that the error is triggered by messages being
stuck in the queue when the device is disconnected. For my new set of
tests, I made a small script which only restarts the modem using GPIO
at some interval. I.e., there is no data sent to the modem. I left the
script running over night, and saw no errors. I then started my
default tool, which configures the modem and performs the restart, and
the error appeared within a couple of hours.

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


Re: usb/serial/visor: slab-out-of-bounds in palm_os_3_probe

2017-10-03 Thread Johan Hovold
On Fri, Sep 29, 2017 at 10:37:55AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 28, 2017 at 07:57:46PM +0200, Andrey Konovalov wrote:
> > Hi!
> > 
> > I've got the following report while fuzzing the kernel with syzkaller.
> > 
> > On commit dc972a67cc54585bd83ad811c4e9b6ab3dcd427e (4.14-rc2+).
> > 
> > There's no check on the connection_info->num_ports value when
> > iterating over ports.
> > 
> > usb 1-1: Handspring Visor / Palm OS: port 162, is for unknown use
> > usb 1-1: Handspring Visor / Palm OS: port 81, is for unknown use
> > ==
> > BUG: KASAN: slab-out-of-bounds in palm_os_3_probe+0x4e4/0x570
> > Read of size 1 at addr 8800686daa26 by task kworker/0:1/24

Thanks for the report, Andrey.

> Ah, nice catch, this bug is _old_, sorry about that.
> 
> The patch below should resolve this.  It looks bigger than it really is,
> as I'm just moving the error checking higher up in the function, and
> loosing an indentation for when there is invalid data.
> 
> Can you let me know if this solves the issue?

And thanks for fixing this up, Greg. Will you send a proper patch that I
can apply?

A couple of comments below.

> diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
> index 9f3317a940ef..18c31bae0e10 100644
> --- a/drivers/usb/serial/visor.c
> +++ b/drivers/usb/serial/visor.c
> @@ -338,47 +338,48 @@ static int palm_os_3_probe(struct usb_serial *serial,
>   goto exit;
>   }
>  
> - if (retval == sizeof(*connection_info)) {
> - connection_info = (struct visor_connection_info *)
> - transfer_buffer;
> -
> - num_ports = le16_to_cpu(connection_info->num_ports);
> - for (i = 0; i < num_ports; ++i) {
> - switch (
> -connection_info->connections[i].port_function_id) {
> - case VISOR_FUNCTION_GENERIC:
> - string = "Generic";
> - break;
> - case VISOR_FUNCTION_DEBUGGER:
> - string = "Debugger";
> - break;
> - case VISOR_FUNCTION_HOTSYNC:
> - string = "HotSync";
> - break;
> - case VISOR_FUNCTION_CONSOLE:
> - string = "Console";
> - break;
> - case VISOR_FUNCTION_REMOTE_FILE_SYS:
> - string = "Remote File System";
> - break;
> - default:
> - string = "unknown";
> - break;
> - }
> - dev_info(dev, "%s: port %d, is for %s use\n",
> - serial->type->description,
> - connection_info->connections[i].port, string);
> - }
> + if (retval != sizeof(*connection_info)) {
> + retval = -EINVAL;

Since we're refusing to bind here -ENODEV may be more appropriate. And
I think a dev_err() (or dev_warn()) is warranted too.

> + goto exit;

So far we have not been bailing probe when the returned connection-info size
wasn't the expected one (we'd only skip the dev_info() above). I'm sure
this is fine, but the tightened check could potentially cause issues if
there are devices returning additional fields.

>   }
> - /*
> - * Handle devices that report invalid stuff here.
> - */
> +
> + connection_info = (struct visor_connection_info *)transfer_buffer;
> +
> + num_ports = le16_to_cpu(connection_info->num_ports);
> +
> + /* Handle devices that report invalid stuff here. */
>   if (num_ports == 0 || num_ports > 2) {
>   dev_warn(dev, "%s: No valid connect info available\n",
>   serial->type->description);
>   num_ports = 2;
>   }
>  
> + for (i = 0; i < num_ports; ++i) {
> + switch (
> +connection_info->connections[i].port_function_id) {

Perhaps you can merge the above two lines now while at it.

> + case VISOR_FUNCTION_GENERIC:
> + string = "Generic";
> + break;
> + case VISOR_FUNCTION_DEBUGGER:
> + string = "Debugger";
> + break;
> + case VISOR_FUNCTION_HOTSYNC:
> + string = "HotSync";
> + break;
> + case VISOR_FUNCTION_CONSOLE:
> + string = "Console";
> + break;
> + case VISOR_FUNCTION_REMOTE_FILE_SYS:
> + string = "Remote File System";
> + break;
> + default:
> + string = "unknown";
> + break;
> + 

Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

2017-10-03 Thread Minas Harutyunyan
On 9/30/2017 9:13 PM, John Youn wrote:
> On 09/20/2017 12:57 PM, John Stultz wrote:
>> So here are a few dwc2 fixes that I've been using with HiKey.
>> I'm not totally sure these are all ideal, but they avoid edge case
>> issues that we have been running into with switching between
>> gadget mode and host mode.
>>
>> I'd guess the first two are potentially -stable material, and
>> the last might be worth sending to -stable too, as its a relatively
>> simple fix, but to my understanding the UDC state tracking has
>> always been broken so its not really a regression. But still.
>>
>> I'd love to get some feedback on the patches and consideration
>> to be merged upstream.
>>
>> thanks
>> -john
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Amit Pundir 
>> Cc: YongQin Liu 
>> Cc: John Youn 
>> Cc: Minas Harutyunyan 
>> Cc: Douglas Anderson 
>> Cc: Chen Yu 
>> Cc: Felipe Balbi 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-usb@vger.kernel.org
>>
>> John Stultz (3):
>>   usb: dwc2: Improve gadget state disconnection handling
>>   usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
>>   usb: dwc2: Fix UDC state tracking
>>
>>  drivers/usb/dwc2/gadget.c | 7 +++
>>  drivers/usb/dwc2/hcd.c| 8 ++--
>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>
>
> Hi John,
>
> I think we have something that fixes these issues.
>
> Minas,
>
> Could you take a look at this? I was not able to find the patches we
> talked about. If possible, please post them so that John can try them
> out.
>
> Thanks,
> John
>
Hi John Stultz,

Could you please apply patch from Vardan Mikayelyan "usb: dwc2: Fix 
dwc2_hsotg_core_init_disconnected()" submitted at 02/25/2017 
(https://marc.info/?l=linux-usb&m=148801589931039&w=2) instead of your 
"usb: dwc2: Improve gadget state disconnection handling" and test again 
failing scenario.
Other 2 patches from series "[PATCH 0/3] dwc2 fixes for edge cases on 
hikey" are Ok.

Thanks,
Minas

--
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: [RESEND x2][PATCH 2/3] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

2017-10-03 Thread Minas Harutyunyan
On 9/20/2017 11:57 PM, John Stultz wrote:
> We've found that while in host mode, using Android, if one runs
> the command:
>   stop adbd
>
> The existing usb devices being utilized in host mode are disconnected.
> This is most visible with usb networking devices.
>
> This seems to be due to adbd closing the file:
>   /dev/usb-ffs/adb/ep0
> Which calls ffs_ep0_release() and the following backtrace:
>
> [] dwc2_hsotg_ep_disable+0x148/0x150
> [] dwc2_hsotg_udc_stop+0x60/0x110
> [] usb_gadget_remove_driver+0x58/0x78
> [] usb_gadget_unregister_driver+0x74/0xe8
> [] unregister_gadget+0x28/0x58
> [] unregister_gadget_item+0x2c/0x40
> [] ffs_data_clear+0xe8/0xf8
> [] ffs_data_reset+0x20/0x58
> [] ffs_data_closed+0x98/0xe8
> [] ffs_ep0_release+0x20/0x30
>
> Then when dwc2_hsotg_ep_disable() is called, we call
> kill_all_requests() which causes a bunch of the following
> messages:
>
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
> init: Service 'adbd' (pid 1915) killed by signal 9
> init: Sending signal 9 to service 'adbd' (pid 1915) process group...
> init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
> init: processing action (init.svc.adbd=stopped) from 
> (/init.usb.configfs.rc:15)
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but 
> reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but 
> reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but 
> reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but 
> reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but 
> reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
> dwc2 f72c.usb: dwc2_update_urb_state_abn(): trimming xfer length
>
> And the usb devices connected are basically hung at this point.
>
> It seems like if we're in host mode, we probably shouldn't run
> the dwc2_hostg_ep_disable logic, so this patch returns an error
> in that case.
>
> With this patch (along with the two previous patches mailed out
> earlier:
>   
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_8_3_1008&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=YbED3GZFyun_ID3-6Off3kdvd9xTUepWt4lzd2__ZSs&s=65BnVw7lagEfnyuD2WHnFlGTUnjvPHWyFiwL_F1vwfE&e=
>   
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2017_8_3_1010&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=YbED3GZFyun_ID3-6Off3kdvd9xTUepWt4lzd2__ZSs&s=MdrOhV0i6kRrV2mHK5zHIwE1eF21MsTkHIjvsV2k7uw&e=
> ), we avoid the mismatched interrupts and connected usb devices
> continue to function.
>
> I'm not sure if some other solution would be better here, but this seems
> to work, so I wanted to send it out for input on what the right approach
> should be.
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Amit Pundir 
> Cc: YongQin Liu 
> Cc: John Youn 
> Cc: Minas Harutyunyan 
> Cc: Douglas Anderson 
> Cc: Chen Yu 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Reported-by: YongQin Liu 
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d8e09c..7fd0e38 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4004,6 +4004,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
>   return -EINVAL;
>   }
>
> + if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> + dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
> + return -EINVAL;
> + }
> +
>   epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
>
>   spin_lock_irqsave(&hsotg->lock, flags);
>

Tested by: Minas Harutyunyan 

--
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: [RESEND x2][PATCH 3/3] usb: dwc2: Fix UDC state tracking

2017-10-03 Thread Minas Harutyunyan
On 9/20/2017 11:57 PM, John Stultz wrote:
> It has been noticed that the dwc2 udc state reporting doesn't
> seem to work (at least on HiKey boards). Where after the initial
> setup, the sysfs /sys/class/udc/f72c.usb/state file would
> report "configured" no matter the state of the OTG port.
>
> This patch adds a call so that we report to the UDC layer when
> the gadget device is disconnected.
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Amit Pundir 
> Cc: YongQin Liu 
> Cc: John Youn 
> Cc: Minas Harutyunyan 
> Cc: Douglas Anderson 
> Cc: Chen Yu 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Reported-by: Amit Pundir 
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc2/gadget.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 7fd0e38..603c216 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3202,6 +3202,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
>
>   call_gadget(hsotg, disconnect);
>   hsotg->lx_state = DWC2_L3;
> +
> + usb_gadget_set_state(&hsotg->gadget, USB_STATE_NOTATTACHED);
>  }
>
>  /**
>

Tested by: Minas Harutyunyan 

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


am33xx, musb, cppi41, packet loss?

2017-10-03 Thread Frédéric Dalleau

Hi,

On Am33xx I am using a bluetooth adapter over musb host using cppi41.

I'm doing some Bluetooth Audio using a USB adapter. It using RFCOMM and 
SCO (the former uses bulk transfer the latter uses isochronous 
transfers), i am missing a packet in a bulk transfer after the 
isochronous transfer stopped.


This seems to happens after isochronous packet was received, i can 
receive still data from the bluetooth adapter, but the first send does 
not seem to happen, even though I am seeing the tx complete interrupt. I 
mean, I am expecting a response from the adapter that I do not see. So I 
guess that the adapter did not receive the message. I also tried 
different adapters to rule out.


I have a very simple code that can reproduce the issue on 1/3 rate.

I noticed that If I don't establish the audio connection, there is no 
packet loss.


The kernel on this device is 4.9 kernel with 
https://lkml.org/lkml/2017/8/14/1167


I am open to any suggestion and can provide details if needed.

Thanks
Frédéric
--
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: renesas_usbhs: add support for R-Car D3

2017-10-03 Thread Yoshihiro Shimoda
This patch set is based on the latest Felipe's usb.git / testing/next branch
(commit id = af846d7ab55c846af6d6f150e9af96295101e068)

Yoshihiro Shimoda (2):
  usb: renesas_usbhs: unify Gen2/3 pipe_config setting
  usb: renesas_usbhs: add support for R-Car D3

 .../devicetree/bindings/usb/renesas_usbhs.txt  |  1 +
 drivers/usb/renesas_usbhs/common.c | 21 ++
 drivers/usb/renesas_usbhs/rcar3.c  | 48 ++
 drivers/usb/renesas_usbhs/rcar3.h  |  1 +
 include/linux/usb/renesas_usbhs.h  |  5 ++-
 5 files changed, 65 insertions(+), 11 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: renesas_usbhs: add support for R-Car D3

2017-10-03 Thread Yoshihiro Shimoda
This patch adds support for R-Car D3. This SoC needs to release
the PLL reset by the UGCTRL register. So, since this is not the same
as other R-Car Gen3 SoCs, this patch adds a new type as
"USBHS_TYPE_RCAR_GEN3_WITH_PLL".

Signed-off-by: Yoshihiro Shimoda 
---
 .../devicetree/bindings/usb/renesas_usbhs.txt  |  1 +
 drivers/usb/renesas_usbhs/common.c | 10 -
 drivers/usb/renesas_usbhs/rcar3.c  | 48 ++
 drivers/usb/renesas_usbhs/rcar3.h  |  1 +
 include/linux/usb/renesas_usbhs.h  |  5 ++-
 5 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 9e18e00..e79f6e4 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -10,6 +10,7 @@ Required properties:
- "renesas,usbhs-r8a7794" for r8a7794 (R-Car E2) compatible device
- "renesas,usbhs-r8a7795" for r8a7795 (R-Car H3) compatible device
- "renesas,usbhs-r8a7796" for r8a7796 (R-Car M3-W) compatible device
+   - "renesas,usbhs-r8a77995" for r8a77995 (R-Car D3) compatible device
- "renesas,rcar-gen2-usbhs" for R-Car Gen2 compatible device
- "renesas,rcar-gen3-usbhs" for R-Car Gen3 compatible device
 
diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index 2a860e4..3e92a78 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -486,6 +486,10 @@ static int usbhsc_drvcllbck_notify_hotplug(struct 
platform_device *pdev)
.data = (void *)USBHS_TYPE_RCAR_GEN3,
},
{
+   .compatible = "renesas,usbhs-r8a77995",
+   .data = (void *)USBHS_TYPE_RCAR_GEN3_WITH_PLL,
+   },
+   {
.compatible = "renesas,rcar-gen2-usbhs",
.data = (void *)USBHS_TYPE_RCAR_GEN2,
},
@@ -519,7 +523,8 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
dparam->enable_gpio = gpio;
 
if (dparam->type == USBHS_TYPE_RCAR_GEN2 ||
-   dparam->type == USBHS_TYPE_RCAR_GEN3) {
+   dparam->type == USBHS_TYPE_RCAR_GEN3 ||
+   dparam->type == USBHS_TYPE_RCAR_GEN3_WITH_PLL) {
dparam->has_usb_dmac = 1;
dparam->pipe_configs = usbhsc_new_pipe;
dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
@@ -584,6 +589,9 @@ static int usbhs_probe(struct platform_device *pdev)
case USBHS_TYPE_RCAR_GEN3:
priv->pfunc = usbhs_rcar3_ops;
break;
+   case USBHS_TYPE_RCAR_GEN3_WITH_PLL:
+   priv->pfunc = usbhs_rcar3_with_pll_ops;
+   break;
default:
if (!info->platform_callback.get_id) {
dev_err(&pdev->dev, "no platform callbacks");
diff --git a/drivers/usb/renesas_usbhs/rcar3.c 
b/drivers/usb/renesas_usbhs/rcar3.c
index 02b67ab..f436e9d 100644
--- a/drivers/usb/renesas_usbhs/rcar3.c
+++ b/drivers/usb/renesas_usbhs/rcar3.c
@@ -15,24 +15,39 @@
 #include "rcar3.h"
 
 #define LPSTS  0x102
+#define UGCTRL 0x180   /* 32-bit register */
 #define UGCTRL20x184   /* 32-bit register */
+#define UGSTS  0x188   /* 32-bit register */
 
 /* Low Power Status register (LPSTS) */
 #define LPSTS_SUSPM0x4000
 
+/* R-Car D3 only: USB General control register (UGCTRL) */
+#define UGCTRL_PLLRESET0x0001
+#define UGCTRL_CONNECT 0x0004
+
 /*
  * USB General control register 2 (UGCTRL2)
  * Remarks: bit[31:11] and bit[9:6] should be 0
  */
 #define UGCTRL2_RESERVED_3 0x0001  /* bit[3:0] should be B'0001 */
+#define UGCTRL2_USB0SEL_HSUSB  0x0020
 #define UGCTRL2_USB0SEL_OTG0x0030
 #define UGCTRL2_VBUSSEL0x0400
 
+/* R-Car D3 only: USB General status register (UGSTS) */
+#define UGSTS_LOCK 0x0100
+
 static void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data)
 {
iowrite32(data, priv->base + reg);
 }
 
+static u32 usbhs_read32(struct usbhs_priv *priv, u32 reg)
+{
+   return ioread32(priv->base + reg);
+}
+
 static int usbhs_rcar3_power_ctrl(struct platform_device *pdev,
void __iomem *base, int enable)
 {
@@ -52,6 +67,34 @@ static int usbhs_rcar3_power_ctrl(struct platform_device 
*pdev,
return 0;
 }
 
+/* R-Car D3 needs to release UGCTRL.PLLRESET */
+static int usbhs_rcar3_power_and_pll_ctrl(struct platform_device *pdev,
+ void __iomem *base, int enable)
+{
+   struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+   u32 val;
+   int timeout = 1000;
+
+   if (enable) {
+   usbhs_write32(priv, UGCTRL, 0); /* release PLLRESET */
+   usbhs_write32(priv, UG

[PATCH 1/2] usb: renesas_usbhs: unify Gen2/3 pipe_config setting

2017-10-03 Thread Yoshihiro Shimoda
This patch unifies the Gen2 and Gen3 pipe_config setting on
usbhs_parse_dt().

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/common.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index f0ce304..2a860e4 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -519,8 +519,11 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
dparam->enable_gpio = gpio;
 
if (dparam->type == USBHS_TYPE_RCAR_GEN2 ||
-   dparam->type == USBHS_TYPE_RCAR_GEN3)
+   dparam->type == USBHS_TYPE_RCAR_GEN3) {
dparam->has_usb_dmac = 1;
+   dparam->pipe_configs = usbhsc_new_pipe;
+   dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
+   }
 
return info;
 }
@@ -577,17 +580,9 @@ static int usbhs_probe(struct platform_device *pdev)
switch (priv->dparam.type) {
case USBHS_TYPE_RCAR_GEN2:
priv->pfunc = usbhs_rcar2_ops;
-   if (!priv->dparam.pipe_configs) {
-   priv->dparam.pipe_configs = usbhsc_new_pipe;
-   priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
-   }
break;
case USBHS_TYPE_RCAR_GEN3:
priv->pfunc = usbhs_rcar3_ops;
-   if (!priv->dparam.pipe_configs) {
-   priv->dparam.pipe_configs = usbhsc_new_pipe;
-   priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
-   }
break;
default:
if (!info->platform_callback.get_id) {
-- 
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: Mouse freezes on button depression

2017-10-03 Thread Christian Bullow
Apologies, I don't know if I replied to this in the end. Nothing on 
dmesg - the mouse just freezes.


Is it maybe not a driver issue but somewhere in the translation to X?

Sorry - I'm the diet coke of technical.


On 28/09/17 15:41, Oliver Neukum wrote:

Am Mittwoch, den 27.09.2017, 20:49 +1000 schrieb Christian Bullow:

Hi Oliver thank you for your reply.

Apologies for now replying from gmail. I'm being told my crjb.net
address is failing SPF and that I have to shoehorn this email into
plain-text for you. Here we go :)

I must correct/clarify that the browser forward and back buttons seem to
work in chrome and that in testing this evening, they have not frozen or
crashed anything. The output I provided before was simply the dmesg
record of the mouse being detected on boot or plugin, not any dmesg
records produced on the depression of the buttons of concern (which are
primarily the DPI+ and DPI- buttons).

*Relevant output from usbmon (cat /sys/kernel/debug/usb/usbmon/0u)*

I was able to capture my pressing the dpi+ button twice before the mouse
driver crashed.

a0b1fca52240 1143494193 C Ii:1:015:2 0:1 8 =  
a0b1fca52240 1143494212 S Ii:1:015:2 -115:1 8 <
a0b1fca52240 1144099217 C Ii:1:015:2 0:1 8 =  
a0b1fca52240 1144099233 S Ii:1:015:2 -115:1 8 <

*From lsusb*

Bus 001 Device 015: ID 1044:7a13 Chu Yuen Enterprise Co., Ltd

*From cat /sys/kernel/debug/usb/devices*

T:  Bus=01 Lev=02 Prnt=02 Port=01 Cnt=01 Dev#=  15 Spd=12   MxCh= 0
D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=1044 ProdID=7a13 Rev= 2.00
S:  Manufacturer=Texas Instruments
S:  Product=MSP430-USB Mouse
S:  SerialNumber=8C2F0C5132000C00
C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=15ms
E:  Ad=01(O) Atr=03(Int.) MxPS=   8 Ivl=15ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
E:  Ad=82(I) Atr=03(Int.) MxPS=   8 Ivl=1ms
E:  Ad=02(O) Atr=03(Int.) MxPS=   8 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
E:  Ad=83(I) Atr=03(Int.) MxPS=   8 Ivl=20ms
E:  Ad=03(O) Atr=03(Int.) MxPS=   8 Ivl=20ms
I:* If#= 3 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=84(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
E:  Ad=04(O) Atr=03(Int.) MxPS=  64 Ivl=1ms

Unplugging and re-plugging the mouse in, it moves to Bus 01, Device 16
and I captured the following:

*usbmon output from the DPI - button*

a0b20247ab40 1348549098 C Ii:1:016:2 0:1 8 =  
a0b20247ab40 1348549123 S Ii:1:016:2 -115:1 8 <*
*

That tells us practically nothing. If you press any button nothing
happens? Anything in dmesg?

Regards
Oliver



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


[GIT PULL] USB driver fixes for 4.14-rc4

2017-10-03 Thread Greg KH
The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:

  Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ 
tags/usb-4.14-rc4

for you to fetch changes up to 80c82ffebd2ec3f91a2daa24b561392de7fda895:

  Merge tag 'fixes-for-v4.14-rc3' of 
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus 
(2017-09-29 10:25:46 +0200)


USB fixes for 4.14-rc4

Here are a number of USB fixes for 4.14-rc4 to resolved reported issue.

There's a bunch of stuff in here based on the great work Andrey
Konovalov is doing in fuzzing the USB stack.  Lots of bug fixes when
dealing with corrupted USB descriptors that we've never seen in "normal"
operation, but is now ensuring the stack is much more hardened overall.

There's also the usual XHCI and gadget driver fixes as well, and a build
error fix, and a few other minor things, full details in the shortlog.

All of these have been in linux-next with no reported issues.

Signed-off-by: Greg Kroah-Hartman 


Adam Wallis (1):
  usb: host: xhci-plat: allow sysdev to inherit from ACPI

Alan Stern (9):
  usb-storage: fix bogus hardware error messages for ATA pass-thru devices
  usb-storage: unusual_devs entry to fix write-access regression for 
Seagate external drives
  USB: uas: fix bug in handling of alternate settings
  USB: gadgetfs: fix copy_to_user while holding spinlock
  USB: gadgetfs: Fix crash caused by inadequate synchronization
  USB: g_mass_storage: Fix deadlock when driver is unbound
  USB: dummy-hcd: fix connection failures (wrong speed)
  USB: dummy-hcd: fix infinite-loop resubmission bug
  USB: dummy-hcd: Fix erroneous synchronization change

Andrey Konovalov (2):
  uwb: ensure that endpoint is interrupt
  uwb: properly check kthread_run return value

Arnd Bergmann (1):
  usb: gadget: dummy: fix nonsensical comparisons

Baolin Wang (1):
  usb: dwc3: of-simple: Add compatible for Spreadtrum SC9860 platform

Bjørn Mork (1):
  USB: cdc-wdm: ignore -EPIPE from GetEncapsulatedResponse

Dan Carpenter (2):
  USB: devio: Prevent integer overflow in proc_do_submiturb()
  USB: devio: Don't corrupt user memory

Dmitry Fleytman (1):
  usb: Increase quirk delay for USB devices

Felipe Balbi (1):
  usb: dwc3: ep0: fix DMA starvation by assigning req->trb on ep0

Greg Kroah-Hartman (4):
  USB: fix out-of-bounds in usb_set_configuration
  Merge tag 'fixes-for-v4.14-rc2' of git://git.kernel.org/.../balbi/usb 
into usb-linus
  USB: core: harden cdc_parse_cdc_header
  Merge tag 'fixes-for-v4.14-rc3' of git://git.kernel.org/.../balbi/usb 
into usb-linus

Jim Dickerson (1):
  usb: pci-quirks.c: Corrected timeout values used in handshake

John Keeping (1):
  usb: gadget: ffs: handle I/O completion in-order

Kai-Heng Feng (1):
  Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

Lu Baolu (1):
  usb: xhci: Free the right ring in xhci_add_endpoint()

Mathias Nyman (4):
  xhci: fix finding correct bus_state structure for USB 3.1 hosts
  xhci: fix wrong endpoint ESIT value shown in tracing
  xhci: Fix sleeping with spin_lock_irq() held in ASmedia 1042A workaround
  xhci: set missing SuperSpeedPlus Link Protocol bit in roothub descriptor

Nicolas Ferre (1):
  usb: gadget: udc: atmel: set vbus irqflags explicitly

Randy Dunlap (1):
  usb: gadget: udc: fix snps_udc_plat.c build errors

Roger Quadros (1):
  usb: gadget: core: fix ->udc_set_speed() logic

Yoshihiro Shimoda (6):
  usb: gadget: function: printer: avoid spinlock recursion
  usb: gadget: udc: renesas_usb3: fix for no-data control transfer
  usb: gadget: udc: renesas_usb3: fix Pn_RAMMAP.Pn_MPKT value
  usb: gadget: udc: renesas_usb3: Fix return value of usb3_write_pipe()
  usb: renesas_usbhs: fix the BCLR setting condition for non-DCP pipe
  usb: renesas_usbhs: fix usbhsf_fifo_clear() for RX direction

 drivers/usb/class/cdc-wdm.c  |  4 +-
 drivers/usb/core/config.c| 16 +--
 drivers/usb/core/devio.c | 11 -
 drivers/usb/core/hub.c   |  2 +-
 drivers/usb/core/message.c   |  4 ++
 drivers/usb/dwc3/dwc3-of-simple.c|  1 +
 drivers/usb/dwc3/ep0.c   |  7 +++
 drivers/usb/gadget/function/f_fs.c   | 17 ++--
 drivers/usb/gadget/function/f_mass_storage.c | 27 +++-
 drivers/usb/gadget/function/f_mass_storage.h | 14 --
 drivers/usb/gadget/function/f_printer.c  |  7 ++-
 drivers/usb/gadget/function/u_fs.h   |  1 +
 drivers/usb/gadget/legacy/inode.c| 46 +---
 drivers/usb/gadget/legacy/mass_storage.c | 26 ++-
 driv

[bug report] usb: gadget: R8A66597 peripheral controller support.

2017-10-03 Thread Dan Carpenter
Hello Yoshihiro Shimoda,

The patch c41442474a26: "usb: gadget: R8A66597 peripheral controller
support." from Aug 19, 2009, leads to the following static checker
warning:

drivers/usb/gadget/udc/r8a66597-udc.c:1942 r8a66597_probe()
warn: integer overflow: (-1) * 2

drivers/usb/gadget/udc/r8a66597-udc.c
  1936  }
  1937  usb_ep_set_maxpacket_limit(&r8a66597->ep[0].ep, 64);
  1938  r8a66597->ep[0].pipenum = 0;
  1939  r8a66597->ep[0].fifoaddr = CFIFO;
  1940  r8a66597->ep[0].fifosel = CFIFOSEL;
  1941  r8a66597->ep[0].fifoctr = CFIFOCTR;
  1942  r8a66597->ep[0].pipectr = get_pipectr_addr(0);
  ^^^

#define get_pipectr_addr(pipenum) (M66592_PIPE1CTR + (pipenum - 1) * 2)

It's not normally valid to pass 0 to get_pipectr_addr() because it ends
up that we assign "r8a66597->ep[0].pipectr = M66592_PIPE1CTR - 2".

  1943  r8a66597->pipenum2ep[0] = &r8a66597->ep[0];
  1944  r8a66597->epaddr2ep[0] = &r8a66597->ep[0];
  1945  

Also:
drivers/usb/gadget/udc/m66592-udc.c:1653 m66592_probe()
warn: integer overflow: (-1) * 2

drivers/usb/gadget/udc/m66592-udc.c
  1647  usb_ep_set_maxpacket_limit(&m66592->ep[0].ep, 64);
  1648  m66592->ep[0].pipenum = 0;
  1649  m66592->ep[0].fifoaddr = M66592_CFIFO;
  1650  m66592->ep[0].fifosel = M66592_CFIFOSEL;
  1651  m66592->ep[0].fifoctr = M66592_CFIFOCTR;
  1652  m66592->ep[0].fifotrn = 0;
  1653  m66592->ep[0].pipectr = get_pipectr_addr(0);
  1654  m66592->pipenum2ep[0] = &m66592->ep[0];
  1655  m66592->epaddr2ep[0] = &m66592->ep[0];


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


[PATCHv2 1/2] drivers: phy: add calibrate method

2017-10-03 Thread Andrzej Pietrasiewicz
Some quirky UDCs (like dwc3 on exynos) need to have heir phys calibrated
e.g. for using super speed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/phy/phy-core.c  | 14 ++
 include/linux/phy/phy.h | 10 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index a268f4d..fdf343a 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -372,6 +372,20 @@ int phy_reset(struct phy *phy)
 }
 EXPORT_SYMBOL_GPL(phy_reset);
 
+int phy_calibrate(struct phy *phy)
+{
+   int ret;
+
+   if (!phy || !phy->ops->calibrate)
+   return 0;
+
+   mutex_lock(&phy->mutex);
+   ret = phy->ops->calibrate(phy);
+   mutex_unlock(&phy->mutex);
+
+   return ret;
+}
+
 /**
  * _of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @np: device_node for which to get the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index e694d40..87580c8 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -39,6 +39,7 @@ enum phy_mode {
  * @power_off: powering off the phy
  * @set_mode: set the mode of the phy
  * @reset: resetting the phy
+ * @calibrate: calibrate the phy
  * @owner: the module owner containing the ops
  */
 struct phy_ops {
@@ -48,6 +49,7 @@ struct phy_ops {
int (*power_off)(struct phy *phy);
int (*set_mode)(struct phy *phy, enum phy_mode mode);
int (*reset)(struct phy *phy);
+   int (*calibrate)(struct phy *phy);
struct module *owner;
 };
 
@@ -141,6 +143,7 @@ static inline void *phy_get_drvdata(struct phy *phy)
 int phy_power_off(struct phy *phy);
 int phy_set_mode(struct phy *phy, enum phy_mode mode);
 int phy_reset(struct phy *phy);
+int phy_calibrate(struct phy *phy);
 static inline int phy_get_bus_width(struct phy *phy)
 {
return phy->attrs.bus_width;
@@ -262,6 +265,13 @@ static inline int phy_reset(struct phy *phy)
return -ENOSYS;
 }
 
+static inline int phy_calibrate(struct phy *phy)
+{
+   if (!phy)
+   return 0;
+   return -ENOSYS;
+}
+
 static inline int phy_get_bus_width(struct phy *phy)
 {
return -ENOSYS;
-- 
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


[PATCHv2 2/2] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

2017-10-03 Thread Andrzej Pietrasiewicz
From: Vivek Gautam 

Adding phy calibration sequence for USB 3.0 DRD PHY present on
Exynos5420/5800 systems.
This calibration facilitates setting certain PHY parameters viz.
the Loss-of-Signal (LOS) Detector Threshold Level, as well as
Tx-Vboost-Level for Super-Speed operations.
Additionally we also set proper time to wait for RxDetect measurement,
for desired PHY reference clock, so as to solve issue with enumeration
of few USB 3.0 devices, like Samsung SUM-TSB16S 3.0 USB drive
on the controller.

We are using CR_port for this purpose to send required data
to override the LOS values.

On testing with USB 3.0 devices on USB 3.0 port present on
SMDK5420, and peach-pit boards should see following message:
usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd

and without this patch, should see below shown message:
usb 1-1: new high-speed USB device number 2 using xhci-hcd

[Also removed unnecessary extra lines in the register macro definitions]

Signed-off-by: Vivek Gautam 
[adapted to use phy_calibrate as entry point]
Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 +++
 drivers/usb/dwc3/core.c  |   7 +-
 2 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 22c68f5..9e83c15 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -90,7 +90,17 @@
 #define PHYCLKRST_COMMONONNBIT(0)
 
 #define EXYNOS5_DRD_PHYREG00x14
+#define PHYREG0_SSC_REF_CLK_SELBIT(21)
+#define PHYREG0_SSC_RANGE  BIT(20)
+#define PHYREG0_CR_WRITE   BIT(19)
+#define PHYREG0_CR_READBIT(18)
+#define PHYREG0_CR_DATA_IN(_x) ((_x) << 2)
+#define PHYREG0_CR_CAP_DATABIT(1)
+#define PHYREG0_CR_CAP_ADDRBIT(0)
+
 #define EXYNOS5_DRD_PHYREG10x18
+#define PHYREG1_CR_DATA_OUT(_x)((_x) << 1)
+#define PHYREG1_CR_ACK BIT(0)
 
 #define EXYNOS5_DRD_PHYPARAM0  0x1c
 
@@ -119,6 +129,25 @@
 #define EXYNOS5_DRD_PHYRESUME  0x34
 #define EXYNOS5_DRD_LINKPORT   0x44
 
+/* USB 3.0 DRD PHY SS Function Control Reg; accessed by CR_PORT */
+#define EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN (0x15)
+#define LOSLEVEL_OVRD_IN_LOS_BIAS_5420 (0x5 << 13)
+#define LOSLEVEL_OVRD_IN_LOS_BIAS_DEFAULT  (0x0 << 13)
+#define LOSLEVEL_OVRD_IN_EN(0x1 << 10)
+#define LOSLEVEL_OVRD_IN_LOS_LEVEL_DEFAULT (0x9 << 0)
+
+#define EXYNOS5_DRD_PHYSS_TX_VBOOSTLEVEL_OVRD_IN   (0x12)
+#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_5420 (0x5 << 13)
+#define TX_VBOOSTLEVEL_OVRD_IN_VBOOST_DEFAULT  (0x4 << 13)
+
+#define EXYNOS5_DRD_PHYSS_LANE0_TX_DEBUG   (0x1010)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_19M2_20M(0x4 << 4)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_24M (0x8 << 4)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_25M_26M (0x8 << 4)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_48M_50M_52M (0x20 << 4)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_62M5(0x20 << 4)
+#define LANE0_TX_DEBUG_RXDET_MEAS_TIME_96M_100M(0x40 << 4)
+
 #define KHZ1000
 #define MHZ(KHZ * KHZ)
 
@@ -527,6 +556,151 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
return 0;
 }
 
+static int crport_handshake(struct exynos5_usbdrd_phy *phy_drd,
+   u32 val, u32 cmd)
+{
+   u32 usec = 100;
+   unsigned int result;
+
+   writel(val | cmd, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
+
+   do {
+   result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
+   if (result & PHYREG1_CR_ACK)
+   break;
+
+   udelay(1);
+   } while (usec-- > 0);
+
+   if (!usec) {
+   dev_err(phy_drd->dev,
+   "CRPORT handshake timeout1 (0x%08x)\n", val);
+   return -ETIME;
+   }
+
+   usec = 100;
+
+   writel(val, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
+
+   do {
+   result = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYREG1);
+   if (!(result & PHYREG1_CR_ACK))
+   break;
+
+   udelay(1);
+   } while (usec-- > 0);
+
+   if (!usec) {
+   dev_err(phy_drd->dev,
+   "CRPORT handshake timeout2 (0x%08x)\n", val);
+   return -ETIME;
+   }
+
+   return 0;
+}
+
+static int crport_ctrl_write(struct exynos5_usbdrd_phy *phy_drd,
+   u32 addr, u32 data)
+{
+   int ret;
+
+   /* Write Address */
+   writel(PHYREG0_

[PATCHv2 0/2]

2017-10-03 Thread Andrzej Pietrasiewicz
Hi all,

This is the second version of patches in this thread.

The series fixes problems with enumerating of SuperSpeed devices
on an Odroid XU3. There was a patch series from Vivek Gautam in
circulation, but it got lost somehow. Please see:

https://lkml.org/lkml/2014/9/2/166
https://lkml.org/lkml/2015/2/2/257

I adapted his patch so that it does not use a hacky solution to force
additional initialization in order for calibration to happen.
With this patch enumeration happens correctly and a super speed device
is recognized as such.

@Kishon:

As far as I understand what you suggest is to move phy_init() and
phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
to the best of my knowledge, they are needed in device mode, too.

Changes since v1:

- added calibrate() callback to phy
- used calibrate() instead of reset() to trigger the calibration

Andrzej Pietrasiewicz (1):
  drivers: phy: add calibrate method

Vivek Gautam (1):
  phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

 drivers/phy/phy-core.c   |  14 +++
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 +++
 drivers/usb/dwc3/core.c  |   7 +-
 include/linux/phy/phy.h  |  10 ++
 4 files changed, 212 insertions(+), 2 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


Re: [PATCHv2 0/2]

2017-10-03 Thread Andrzej Pietrasiewicz

W dniu 03.10.2017 o 14:59, Andrzej Pietrasiewicz pisze:

Hi all,

This is the second version of patches in this thread.



I have lost the subject of the cover letter.
Should be "dwc3 on XU3". I did not resend the series in order
not to spam the recipients. Sorry for confusion.

Andrzej
--
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: 4879b7ae05 ("Merge tag 'dmaengine-4.12-rc1' of .."): WARNING: kernel stack regs at bd92bc2e in 01-cpu-hotplug:3811 has bad 'bp' value 000001be

2017-10-03 Thread Josh Poimboeuf
On Tue, Oct 03, 2017 at 11:45:38AM +0800, Fengguang Wu wrote:
> Hi Josh,
> 
> On Mon, Oct 02, 2017 at 04:31:09PM -0500, Josh Poimboeuf wrote:
> > On Mon, Oct 02, 2017 at 04:26:54PM -0500, Josh Poimboeuf wrote:
> > > Fengguang, assuming it's reliably recreatable, any chance you could
> > > recreate with the following patch?
> 
> Sure, I'll try your patch on v4.14-rc3 since it looks the most
> reproducible kernel. For the bisected 4879b7ae05, the warning only
> shows up once out of 909 boots according to the below stats. So I'm
> not sure whether it's the _first_ bad commit. To double confirm, I
> just queued 5000 more boot tests for each of its parent commits.

Fengguang, here's an improved version of the patch based on Linus'
suggestions.  If you can use it instead, that would be helpful because
it has a better chance of dumping useful data.  Thanks!


diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..191012762aa0 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -44,7 +44,8 @@ static void unwind_dump(struct unwind_state *state)
state->stack_info.type, state->stack_info.next_sp,
state->stack_mask, state->graph_idx);
 
-   for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, 
sizeof(long))) {
+   for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp;
+sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
if (get_stack_info(sp, state->task, &stack_info, &visit_mask))
break;
 
@@ -178,7 +179,7 @@ static struct pt_regs *decode_frame_pointer(unsigned long 
*bp)
 {
unsigned long regs = (unsigned long)bp;
 
-   if (!(regs & 0x1))
+   if ((regs & (sizeof(long)-1)) != 1)
return NULL;
 
return (struct pt_regs *)(regs & ~0x1);
@@ -221,6 +222,10 @@ static bool update_stack_state(struct unwind_state *state,
   &state->stack_mask))
return false;
 
+   /* Make sure the frame pointer is properly aligned: */
+   if ((unsigned long)frame & (sizeof(long)-1))
+   return false;
+
/* Make sure it only unwinds up and doesn't overlap the prev frame: */
if (state->orig_sp && state->stack_info.type == prev_type &&
frame < prev_frame_end)
--
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: serial: qcserial: add Dell DW5818, DW5819

2017-10-03 Thread Johan Hovold
On Fri, Sep 29, 2017 at 12:39:51PM +0800, Shrirang Bagul wrote:
> Dell Wireless 5819/5818 devices are re-branded Sierra Wireless MC74
> series which will by default boot with vid 0x413c and pid's 0x81cf,
> 0x81d0, 0x81d1,0x81d2.
> 
> Signed-off-by: Shrirang Bagul 

Now applied.

Don't you want to add these to qmi_wwan as well?

> ---
>  drivers/usb/serial/qcserial.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
> index ebc0beea69d6..eb9928963a53 100644
> --- a/drivers/usb/serial/qcserial.c
> +++ b/drivers/usb/serial/qcserial.c
> @@ -174,6 +174,10 @@ static const struct usb_device_id id_table[] = {
>   {DEVICE_SWI(0x413c, 0x81b3)},   /* Dell Wireless 5809e Gobi(TM) 4G LTE 
> Mobile Broadband Card (rev3) */
>   {DEVICE_SWI(0x413c, 0x81b5)},   /* Dell Wireless 5811e QDL */
>   {DEVICE_SWI(0x413c, 0x81b6)},   /* Dell Wireless 5811e QDL */
> + {DEVICE_SWI(0x413c, 0x81cf)},   /* Dell Wireless 5819 */
> + {DEVICE_SWI(0x413c, 0x81d0)},   /* Dell Wireless 5819 */
> + {DEVICE_SWI(0x413c, 0x81d1)},   /* Dell Wireless 5818 */
> + {DEVICE_SWI(0x413c, 0x81d2)},   /* Dell Wireless 5818 */
>  
>   /* Huawei devices */
>   {DEVICE_HWI(0x03f0, 0x581d)},   /* HP lt4112 LTE/HSPA+ Gobi 4G Modem 
> (Huawei me906e) */

Thanks,
Johan
--
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: 4879b7ae05 ("Merge tag 'dmaengine-4.12-rc1' of .."): WARNING: kernel stack regs at bd92bc2e in 01-cpu-hotplug:3811 has bad 'bp' value 000001be

2017-10-03 Thread Fengguang Wu

Hi Josh,

On Mon, Oct 02, 2017 at 04:31:09PM -0500, Josh Poimboeuf wrote:

On Mon, Oct 02, 2017 at 04:26:54PM -0500, Josh Poimboeuf wrote:

Fengguang, assuming it's reliably recreatable, any chance you could
recreate with the following patch?


Sorry, here's a version which actually compiles.

diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..00234fa5a33a 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -44,7 +44,8 @@ static void unwind_dump(struct unwind_state *state)
state->stack_info.type, state->stack_info.next_sp,
state->stack_mask, state->graph_idx);

-   for (sp = state->orig_sp; sp; sp = PTR_ALIGN(stack_info.next_sp, 
sizeof(long))) {
+   for (sp = PTR_ALIGN(state->orig_sp, sizeof(long)); sp;
+sp = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
if (get_stack_info(sp, state->task, &stack_info, &visit_mask))
break;


With the patch applied on v4.14-rc3, I get these WARNINGs in 300+
kernel boots:

[  185.430011] WARNING: kernel stack regs at bb68dcc2 in 01-cpu-hotplug:22473 
has bad 'bp' value 01bc
[   86.190009] WARNING: kernel stack regs at be3c9c9a in 01-cpu-hotplug:4941 
has bad 'bp' value 01be
[  144.450014] WARNING: kernel stack regs at be3d5c9a in 01-cpu-hotplug:14750 
has bad 'bp' value 01be
[  180.074990] WARNING: kernel stack regs at b0209c4e in 01-cpu-hotplug:22741 
has bad 'bp' value 01b0
[  138.140012] WARNING: kernel stack regs at be3c9c9a in 01-cpu-hotplug:12181 
has bad 'bp' value 01be
[  120.658716] WARNING: kernel stack regs at be3b9c4e in 01-cpu-hotplug:5548 
has bad 'bp' value 01be
[   88.390022] WARNING: kernel stack regs at be3adcc2 in 01-cpu-hotplug:3874 
has bad 'bp' value 01be
[  174.380015] WARNING: kernel stack regs at be3e1f68 in 01-cpu-hotplug:22049 
has bad 'bp' value   (null)
[   98.150011] WARNING: kernel stack regs at be38dcc2 in 01-cpu-hotplug:2549 
has bad 'bp' value 01be
[  114.450013] WARNING: kernel stack regs at be36bc9a in 01-cpu-hotplug:7747 
has bad 'bp' value 01be
[  151.020011] WARNING: kernel stack regs at b0203cc2 in 01-cpu-hotplug:14901 
has bad 'bp' value 01b8
[  106.770011] WARNING: kernel stack regs at be3c3c9a in 01-cpu-hotplug:5970 
has bad 'bp' value 01be
[  114.290012] WARNING: kernel stack regs at be3d7c9a in 01-cpu-hotplug:6287 
has bad 'bp' value 01be
[  118.818622] WARNING: kernel stack regs at be3cfc4e in 01-cpu-hotplug:5183 
has bad 'bp' value 01be
[  142.540011] WARNING: kernel stack regs at be3b9cc2 in 01-cpu-hotplug:9450 
has bad 'bp' value 01be
[   96.020013] WARNING: kernel stack regs at be3d9f6c in 01-cpu-hotplug:8433 
has bad 'bp' value   (null)
[  149.300010] WARNING: kernel stack regs at be3b1c9a in 01-cpu-hotplug:14589 
has bad 'bp' value 01be
[  140.580013] WARNING: kernel stack regs at be3bfc9a in 01-cpu-hotplug:15779 
has bad 'bp' value 01be
[   93.656698] WARNING: kernel stack regs at c0b9fc9a in 01-cpu-hotplug:6618 
has bad 'bp' value 01c0
[  112.863654] WARNING: kernel stack regs at c05d7c9a in procd:172 has bad 'bp' 
value 01c0
[   77.683057] WARNING: kernel stack regs at c0bcbc9a in 01-cpu-hotplug:5798 
has bad 'bp' value 01c0
[   46.071067] WARNING: kernel stack regs at c51e5d0a in rhashtable_thra:126 
has bad 'bp' value 03c6
[  149.471498] WARNING: kernel stack regs at c5d6bf68 in udevd:285 has bad 'bp' 
value   (null)
[  178.575899] WARNING: kernel stack regs at c21e5d02 in lock_torture_wr:46 has 
bad 'bp' value 03bc
[  129.175338] WARNING: kernel stack regs at cdde5d0a in swapper/0:0 has bad 
'bp' value 03c6
[   80.686106] WARNING: kernel stack regs at c7e29cca in ifup:339 has bad 'bp' 
value 01c8
[0.437556] WARNING: kernel stack regs at c6e15c4e in swapper/0:1 has bad 
'bp' value 01c6
[   12.354954] WARNING: kernel stack regs at c6ff7ca2 in rhashtable_thra:126 
has bad 'bp' value 03be
[  122.627926] WARNING: kernel stack regs at c6ff7d0a in swapper/1:0 has bad 
'bp' value 03c6

Here is one of them. The full dmesg is attached.

Please press Enter to activate this console.
[   89.505747] sock: process `trinity-main' is using obsolete setsockopt 
SO_BSDCOMPAT
procd: Instance odhcpd::instance1 s in a crash loop 6 crashes, 0 seconds since 
last crash
procd: Instance uhttpd::instance1 s in a crash loop 6 crashes, 0 seconds since 
last crash
procd: Instance dnsmasq::instance1 s in a crash loop 6 crashes, 0 seconds since 
last crash
[  114.450013] WARNING: kernel stack regs at be36bc9a in 01-cpu-hotplug:7747 
has bad 'bp' value 01be
[  114.450017] unwind stack type:0 next_sp:  (null) mask:0x2 graph_idx:0
[  114.450020] be36bc9c: 81be36bc (0x81be36bc)
[  114.450023] be36bca0: ebb56272 (0xebb56272)
[  114.450029] be36bca4: b5be36bc (drm_setup_crtcs+0x10ac/0x12e0)
[  114.450031] be36bca8: 4000bcb4 (0x4000bcb4)
[  114.450033] be36bcac

Re: [PATCH] rndis_host: support Novatel Verizon USB730L

2017-10-03 Thread Bjørn Mork
David Miller  writes:

> From: Aleksander Morgado 
> Date: Wed, 27 Sep 2017 23:31:03 +0200
>
>> I'm not sure if binding this logic to a specific vid:pid (1410:9030)
>> would be more appropriate here, or if it's ok to just bind
>> class/subclass/protocol (as in the activesync case).  Let me know
>> what you think.
>
> I don't have enough USB Networking knowledge to make a decision here.
>
> Some things seem confusing.  For example, if we should be matching
> USB_CLASS_MISC, subclass=4, protocol=1 for RNDIS over Ethernet, then
> we why aren't we also matching USB_CLASS_MISC, subclass=4, protocol=2
> for RNDIS over wireless and instead are matching "Remote RNDIS" in
> the form of USB_CLASS_WIRELSS, subclass=1, protocol=3?
>
> I really don't understand any of this magic :-)
>
> So someone more knowledgable needs to review this.

Let me just say that I am not qualified.  But since this needs a review,
I am going to offer my view anyway.  FWIW, I don't think *anyone*
understand this magic...  I certainly don't.

We can pretty much ignore the USB-IF and any specs, since that is what
the vendors appear to do.  They provide device specific drivers for
Windows, so all they care about is that their device "works" with their
driver.

But in Linux we prefer to create drivers for device classes whenever we
can, to avoid having to add every single device by ID.  So we try to
guess future patterns based on the devices we have observed, even when
there is no clear spec.  This is what Aleksander does here. He has a
device with a 'Cls=ef(misc ) Sub=04 Prot=01' function.  This device
works with the rndis_host driver. That is all we know.

We cannot prove that a class match is correct. But it does make sense to
try it.  At least we know that this works for one device.

Adding anything else, e.g. based on the table at
http://www.usb.org/developers/defined_class/#BaseClassEFh , is a bit
more risky.  We don't know if a driver will work with *any* such device
until we've actually seen one.

This is just my opinion, and probably full of bogus assumptions as
usual.  I was sort of hoping that some expert would speak up so I didn't
have to :-)

But FWIW:

Reviewed-by: Bjørn Mork 



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: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Alan Stern
On Tue, 3 Oct 2017, Takashi Iwai wrote:

> On Mon, 25 Sep 2017 14:39:51 +0200,
> Andrey Konovalov wrote:
> > 
> > Hi!
> > 
> > I've got the following report while fuzzing the kernel with syzkaller.
> > 
> > On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2).
> > 
> > It seems that there's no check of the endpoint type.
> > 
> > usb 1-1: BOGUS urb xfer, pipe 1 != type 3
> > [ cut here ]
> > WARNING: CPU: 0 PID: 1846 at drivers/usb/core/urb.c:449
> 
> How is this bug triggered?  As it's syzkaller with QEMU, it looks
> hitting an inconsistent state the driver didn't expect (it sets the
> fixed endpoint), then USB-core detects the inconsistency and spews the
> kernel warning with stack trace.  If so, it's no serious problem as it
> appears.
> 
> Suppose my guess is right, I'm not sure what's the best way to fix
> this.  Certainly we can add more sanity check in the caller side.
> OTOH, I find the reaction of USB core too aggressive, it's not
> necessary to be dev_WARN() but a normal dev_err().
> Or I might be looking at a wrong place?

It's a dev_WARN because it indicates a potentially serious error in the 
driver: The driver has submitted an interrupt URB to a bulk endpoint.  
That may not sound bad, but the same check gets triggered if a driver 
submits a bulk URB to an isochronous endpoint, or any other invalid 
combination.

Most likely the explanation here is that the driver doesn't bother to
check the endpoint type because it expects the endpoint will always be
interrupt.  But that is not a safe strategy.  USB devices and their
firmware should not be trusted unnecessarily.

The best fix is, like you said, to add a sanity check in the caller.

Alan Stern

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


RE: [PATCH] usb: quirks: add quirk for WORLDE MINI MIDI keyboard

2017-10-03 Thread Alan Stern
On Tue, 3 Oct 2017, David Laight wrote:

> From: Felipe Balbi
> > Sent: 03 October 2017 09:17
> > This keyboard doesn't implement Get String descriptors properly even
> > though string indexes are valid. What happens is that when requesting
> > for the String descriptor, the device disconnects and
> > reconnects. Without this quirk, this loop will continue forever.
> 
> If this is a common/known problem, could the driver detect the
> disconnect sequence and avoid reading the string descriptors
> on the subsequent reconnect?

We currently don't have a way for the kernel to add quirks dynamically, 
although it could be added.

We also don't have any way to know that a disconnect was triggered by 
an attempt to read a string descriptor.  Figuring that out would be 
rather difficult, IMO.

Alan Stern

> Then the quirk list wouldn't need to be updated for new id.
> 
>   David
> 
> 

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


Re: Random xHCI HC died on device disconnect

2017-10-03 Thread Kristian Evensen
Hi,

On Tue, Oct 3, 2017 at 11:16 AM, Kristian Evensen
 wrote:
> Messing with the driver is no problem :) Thanks for this pointer, I am
> testing stopping the timer now and will let you know how it goes.

Disabling the timer caused a different error to be triggered. Instead
of "HC died...", I now get the following message looping over and
over:

[16870.871935] qmi_wwan 4-1:1.4: Tx URB error: -19

Another difference from before is that the modem does not get
disconnected from host, i.e., it is still visible in lsusb. Trying to
for example deauthorize the device just hangs, which I guess is as
expected. Is there any way to clear the TX queue, but not halt the hc?
I am not familiar with the xhci-code, but it seems that
xhci_kill_endpoint_urbs does something similar to what I want. Perhaps
it is possible to change the watchdog to flush queue, but not kill
controller.

A soft reboot, as expected, solved the issue. I guess the soft reboot
indicates that this problem should be solvable without a reboot. I
found open Marvell's 4.4 kernel during the afternoon and see that they
power down/up phy during suspend/resumse. I will try to use that for
inspiration and see what happens.

-Kristian
--
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: Random xHCI HC died on device disconnect

2017-10-03 Thread Kristian Evensen
On Tue, Oct 3, 2017 at 4:51 PM, Kristian Evensen
 wrote:
> Disabling the timer caused a different error to be triggered. Instead
> of "HC died...", I now get the following message looping over and
> over:
>
> [16870.871935] qmi_wwan 4-1:1.4: Tx URB error: -19

I was not thinking clearly when I wrote the email. The reason this
message kept looping over and over was that I kept trying to
communicate with the modem. When I stopped my tool, the message
stopped. However, the host controller is still dead, so even if I try
to disconnect the device using GPIO, nothing happens. I thought the
log of when the error occurred was lost due to limited buffers, but I
was wrong and I have the following in dmesg when the error strikes:

[15986.400431] usb 4-1: USB disconnect, device number 13
[15986.400454] qmi_wwan 4-1:1.4: nonzero urb status received: -71
[15986.411366] qmi_wwan 4-1:1.4: wdm_int_callback - 0 bytes
[15986.416692] qmi_wwan 4-1:1.4: wdm_int_callback - usb_submit_urb
failed with result -19
[15986.424816] option1 ttyUSB0: GSM modem (1-port) converter now
disconnected from ttyUSB0
[15986.432886] option 4-1:1.0: device disconnected
[15986.437647] option1 ttyUSB1: GSM modem (1-port) converter now
disconnected from ttyUSB1
[15986.445765] option 4-1:1.1: device disconnected
[16001.110698] xhci-hcd f10f8000.usb3: Stopped the command ring
failed, maybe the host is dead
[16001.119077] xhci-hcd f10f8000.usb3: Abort command ring failed
[16001.124949] xhci-hcd f10f8000.usb3: HC died; cleaning up
[16100.854819] qmi_wwan 4-1:1.4: Tx URB error: -19
[16105.854944] qmi_wwan 4-1:1.4: Tx URB error: -19
[16110.855052] qmi_wwan 4-1:1.4: Tx URB error: -19
[16115.855159] qmi_wwan 4-1:1.4: Tx URB error: -19
[16120.855285] qmi_wwan 4-1:1.4: Tx URB error: -19
[16125.855396] qmi_wwan 4-1:1.4: Tx URB error: -19

I am bit surprised to see the HC-related messages. Maybe I missed a
place where the timer is stopped or something, time to investigate!

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


Re: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Takashi Iwai
On Tue, 03 Oct 2017 16:21:57 +0200,
Alan Stern wrote:
> 
> On Tue, 3 Oct 2017, Takashi Iwai wrote:
> 
> > On Mon, 25 Sep 2017 14:39:51 +0200,
> > Andrey Konovalov wrote:
> > > 
> > > Hi!
> > > 
> > > I've got the following report while fuzzing the kernel with syzkaller.
> > > 
> > > On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2).
> > > 
> > > It seems that there's no check of the endpoint type.
> > > 
> > > usb 1-1: BOGUS urb xfer, pipe 1 != type 3
> > > [ cut here ]
> > > WARNING: CPU: 0 PID: 1846 at drivers/usb/core/urb.c:449
> > 
> > How is this bug triggered?  As it's syzkaller with QEMU, it looks
> > hitting an inconsistent state the driver didn't expect (it sets the
> > fixed endpoint), then USB-core detects the inconsistency and spews the
> > kernel warning with stack trace.  If so, it's no serious problem as it
> > appears.
> > 
> > Suppose my guess is right, I'm not sure what's the best way to fix
> > this.  Certainly we can add more sanity check in the caller side.
> > OTOH, I find the reaction of USB core too aggressive, it's not
> > necessary to be dev_WARN() but a normal dev_err().
> > Or I might be looking at a wrong place?
> 
> It's a dev_WARN because it indicates a potentially serious error in the 
> driver: The driver has submitted an interrupt URB to a bulk endpoint.  
> That may not sound bad, but the same check gets triggered if a driver 
> submits a bulk URB to an isochronous endpoint, or any other invalid 
> combination.
> 
> Most likely the explanation here is that the driver doesn't bother to
> check the endpoint type because it expects the endpoint will always be
> interrupt.  But that is not a safe strategy.  USB devices and their
> firmware should not be trusted unnecessarily.
> 
> The best fix is, like you said, to add a sanity check in the caller.

OK, but then do we have some handy helper for the check?
As other bug reports by syzkaller suggest, there are a few other
drivers that do the same, submitting a urb with naive assumption of
the fixed EP for specific devices.  In the end we'll need to put the
very same checks there in multiple places.


thanks,

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


Re: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Alan Stern
On Tue, 3 Oct 2017, Takashi Iwai wrote:

> > It's a dev_WARN because it indicates a potentially serious error in the 
> > driver: The driver has submitted an interrupt URB to a bulk endpoint.  
> > That may not sound bad, but the same check gets triggered if a driver 
> > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > combination.
> > 
> > Most likely the explanation here is that the driver doesn't bother to
> > check the endpoint type because it expects the endpoint will always be
> > interrupt.  But that is not a safe strategy.  USB devices and their
> > firmware should not be trusted unnecessarily.
> > 
> > The best fix is, like you said, to add a sanity check in the caller.
> 
> OK, but then do we have some handy helper for the check?
> As other bug reports by syzkaller suggest, there are a few other
> drivers that do the same, submitting a urb with naive assumption of
> the fixed EP for specific devices.  In the end we'll need to put the
> very same checks there in multiple places.

Perhaps we could add a helper routine that would take a list of 
expected endpoint types and check that the actual endpoints match the 
types.  But of course, all the drivers you're talking about would have 
to add a call to this helper routine.

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: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Greg Kroah-Hartman
On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> On Tue, 3 Oct 2017, Takashi Iwai wrote:
> 
> > > It's a dev_WARN because it indicates a potentially serious error in the 
> > > driver: The driver has submitted an interrupt URB to a bulk endpoint.  
> > > That may not sound bad, but the same check gets triggered if a driver 
> > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > combination.
> > > 
> > > Most likely the explanation here is that the driver doesn't bother to
> > > check the endpoint type because it expects the endpoint will always be
> > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > firmware should not be trusted unnecessarily.
> > > 
> > > The best fix is, like you said, to add a sanity check in the caller.
> > 
> > OK, but then do we have some handy helper for the check?
> > As other bug reports by syzkaller suggest, there are a few other
> > drivers that do the same, submitting a urb with naive assumption of
> > the fixed EP for specific devices.  In the end we'll need to put the
> > very same checks there in multiple places.
> 
> Perhaps we could add a helper routine that would take a list of 
> expected endpoint types and check that the actual endpoints match the 
> types.  But of course, all the drivers you're talking about would have 
> to add a call to this helper routine.

We have almost this type of function, usb_find_common_endpoints(),
what's wrong with using that?  Johan has already swept the tree and
added a lot of these checks, odds are no one looked at the sound/
subdir...

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: dwc3: gadget: only unmap requests from DMA if mapped

2017-10-03 Thread Thinh Nguyen
Hi,

On 9/11/2017 12:42 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> Thinh Nguyen  writes:
>>> Felipe Balbi  writes:
 Thinh Nguyen  writes:

> Hi Felipe,
>
> On 9/7/2017 12:16 AM, Felipe Balbi wrote:
>  drivers/usb/dwc3/gadget.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 9e41605a..6b299c7 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -191,14 +191,16 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, 
> struct dwc3_request *req,
>  
>   req->started = false;
>   list_del(&req->list);
> - req->trb = NULL;
>   req->remaining = 0;
>  
>   if (req->request.status == -EINPROGRESS)
>   req->request.status = status;
>  
> - usb_gadget_unmap_request_by_dev(dwc->sysdev,
> - &req->request, req->direction);
> + if (req->trb)
 This check does not account for control data transfer. TRBs for ep0 are
 not set to its req->trb. ep0out request needs to be unmapped, otherwise
 device will receive bogus data.

 Our internal test showed that the device failed to interpret control
 data from host. I bisected to this patch.
>>
>> what was the testing? How can I reproduce it?
>
> This is a regression. The internal test found the issue when it does a
> 3-stage Control Write Transfer. You can reproduce this issue with just 1
> out data packet of size > 0. Read and check the data on control request
> completion.

 okay, is this enough to fix the problem for you?

 modified   drivers/usb/dwc3/ep0.c
 @@ -48,6 +48,9 @@ static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
dwc = dep->dwc;
trb = &dwc->ep0_trb[dep->trb_enqueue];

 +  if (!req->trb)
 +  req->trb = trb;
 +
if (chain)
dep->trb_enqueue++;
>>>
>>> sorry, no. this is totally wrong :-) Here's a better version:
>>>
>>> modified   drivers/usb/dwc3/ep0.c
>>> @@ -990,6 +990,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>>>  DWC3_TRBCTL_CONTROL_DATA,
>>>  true);
>>>
>>> +   req->trb = &dwc->ep0_trb[dep->trb_enqueue - 1]; > +
>>> /* Now prepare one extra TRB to align transfer size */
>>> dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
>>>  maxpacket - rem,
>>> @@ -1015,6 +1017,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 
>>> *dwc,
>>>  DWC3_TRBCTL_CONTROL_DATA,
>>>  true);
>>>
>>> +   req->trb = &dwc->ep0_trb[dep->trb_enqueue - 1]; > +
>>> /* Now prepare one extra TRB to align transfer size */
>>> dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
>>>  0, DWC3_TRBCTL_CONTROL_DATA,
>>> @@ -1029,6 +1033,9 @@ static void __dwc3_ep0_do_control_data(struct dwc3 
>>> *dwc,
>>> dwc3_ep0_prepare_one_trb(dep, req->request.dma,
>>> req->request.length, DWC3_TRBCTL_CONTROL_DATA,
>>> false);
>>> +
>>> +   req->trb = &dwc->ep0_trb[dep->trb_enqueue];
>>> +
>>> ret = dwc3_ep0_start_trans(dep);
>>> }
>>>
>>> (didn't even compile test)
>>>
>>>
>>
>> Yes this works.
> 
> Thank you, I'll send this as a formal patch after merge window closes.
> 

The fix for this patch is now in mainline. Please also apply it to 
stable kernel 4.13.x. Otherwise this regression will remain for the 
kernel 4.13.x.

Upstream:
commit 551684708356 ("usb: dwc3: ep0: fix DMA starvation by assigning 
req->trb on ep0")

Thanks,
Thinh
--
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] rndis_host: support Novatel Verizon USB730L

2017-10-03 Thread David Miller
From: Bjørn Mork 
Date: Tue, 03 Oct 2017 16:01:15 +0200

> We can pretty much ignore the USB-IF and any specs, since that is what
> the vendors appear to do.  They provide device specific drivers for
> Windows, so all they care about is that their device "works" with their
> driver.
> 
> But in Linux we prefer to create drivers for device classes whenever we
> can, to avoid having to add every single device by ID.  So we try to
> guess future patterns based on the devices we have observed, even when
> there is no clear spec.  This is what Aleksander does here. He has a
> device with a 'Cls=ef(misc ) Sub=04 Prot=01' function.  This device
> works with the rndis_host driver. That is all we know.
> 
> We cannot prove that a class match is correct. But it does make sense to
> try it.  At least we know that this works for one device.
> 
> Adding anything else, e.g. based on the table at
> http://www.usb.org/developers/defined_class/#BaseClassEFh , is a bit
> more risky.  We don't know if a driver will work with *any* such device
> until we've actually seen one.
> 
> This is just my opinion, and probably full of bogus assumptions as
> usual.  I was sort of hoping that some expert would speak up so I didn't
> have to :-)

Ok ;-)

> But FWIW:
> 
> Reviewed-by: Bjørn Mork 

So I'll apply this for now, thanks for your feedback.
--
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: [PATCHv2 0/2]

2017-10-03 Thread Kishon Vijay Abraham I
Hi,

On Tuesday 03 October 2017 06:29 PM, Andrzej Pietrasiewicz wrote:
> Hi all,
> 
> This is the second version of patches in this thread.
> 
> The series fixes problems with enumerating of SuperSpeed devices
> on an Odroid XU3. There was a patch series from Vivek Gautam in
> circulation, but it got lost somehow. Please see:
> 
> https://lkml.org/lkml/2014/9/2/166
> https://lkml.org/lkml/2015/2/2/257
> 
> I adapted his patch so that it does not use a hacky solution to force
> additional initialization in order for calibration to happen.
> With this patch enumeration happens correctly and a super speed device
> is recognized as such.
> 
> @Kishon:
> 
> As far as I understand what you suggest is to move phy_init() and
> phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
> to the best of my knowledge, they are needed in device mode, too.

What I meant is perform phy initializations for host mode in xhci and keep only
device mode phy initialization in dwc3.

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


Re: usb/sound/bcd2000: warning in bcd2000_init_device

2017-10-03 Thread Takashi Iwai
On Tue, 03 Oct 2017 19:42:21 +0200,
Greg Kroah-Hartman wrote:
> 
> On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > 
> > > > It's a dev_WARN because it indicates a potentially serious error in the 
> > > > driver: The driver has submitted an interrupt URB to a bulk endpoint.  
> > > > That may not sound bad, but the same check gets triggered if a driver 
> > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > combination.
> > > > 
> > > > Most likely the explanation here is that the driver doesn't bother to
> > > > check the endpoint type because it expects the endpoint will always be
> > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > firmware should not be trusted unnecessarily.
> > > > 
> > > > The best fix is, like you said, to add a sanity check in the caller.
> > > 
> > > OK, but then do we have some handy helper for the check?
> > > As other bug reports by syzkaller suggest, there are a few other
> > > drivers that do the same, submitting a urb with naive assumption of
> > > the fixed EP for specific devices.  In the end we'll need to put the
> > > very same checks there in multiple places.
> > 
> > Perhaps we could add a helper routine that would take a list of 
> > expected endpoint types and check that the actual endpoints match the 
> > types.  But of course, all the drivers you're talking about would have 
> > to add a call to this helper routine.
> 
> We have almost this type of function, usb_find_common_endpoints(),
> what's wrong with using that?  Johan has already swept the tree and
> added a lot of these checks, odds are no one looked at the sound/
> subdir...

Well, what I had in my mind is just a snippet from usb_submit_urb(),
something like:

bool usb_sanity_check_urb_pipe(struct urb *urb)
{
struct usb_host_endpoint *ep;
int xfertype;
static const int pipetypes[4] = {
PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
};

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
xfertype = usb_endpoint_type(&ep->desc);
return usb_pipetype(urb->pipe) != pipetypes[xfertype];
}

And calling this before usb_submit_urb() in each place that assigns
the fixed EP as device-specific quirks.
Does it make sense?


thanks,

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