Re: Need help investigating not working NKRO over USB in Linux

2017-08-22 Thread Greg KH 
On Sun, Aug 20, 2017 at 09:43:01PM +0300, IFo Hancroft wrote:
> Hello Everyone,
> 
> Greg Kroah-Hartman directed me to here.
> 
> Here is what I am trying to do, what problems am I facing and what I've
> tried so far:
> 
> My keyboard is USB and it is supposed to have NKRO (the ability to
> register as many keys as pressed). (I know the protocol doesn't really
> allow it and it's using some hack).

What hack is that, multiple keyboards showing up, or somehow sending
multiple HID reports with different keys showing up there?

> However, the NKRO doesn't work in Linux (By not working, I mean it only
> registers 6 keys and modifiers.), so I am trying to investigate what is
> happening.

How have you tested this?

> For example is it just sending one stream of
> data (the 6KRO one (referred to as boot mode)) or two and one gets
> ignored. In theory, since I know the NKRO works in Windows and
> supposedly in Mac OS X, I think it is sending two but one of them gets
> ignored somehow.
> 
> I've first ran lsbusb and got this:
> 
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 003 Device 011: ID 2516:003c 
> Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 003: ID 046d:082d Logitech, Inc. HD Pro Webcam C920
> Bus 001 Device 002: ID 046d:c07d Logitech, Inc.
> Bus 001 Device 005: ID 0909:001c Audio-Technica Corp.
> Bus 001 Device 004: ID 0951:16a4 Kingston Technology
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> 
> I know that the second line refers to the keyboard:
> 
> Bus 003 Device 011: ID 2516:003c
> 
> I've run: sudo bash -c "echo -n 3-2:1.0
> >/sys/bus/usb/drivers/usbhid/unbind"; sudo bash -c "echo -n 3-2:1.2
> >/sys/bus/usb/drivers/usbhid/unbind"; sudo lsusb -v -d 2516:003c;
> (I've run it as sudo so I get rid of the 'Couldn't open device, some
> information will be missing' and hopefully get the full info. I've
> unbind the device to get the Debug descriptor, however since at places,
> I still get
> can't get debug descriptor: Resource temporarily unavailable I thought
> I'd unbind both device events that show up in dmesg after plugging the
> device')
> The output for the device in dmesg is the following:
> 
> [387473.151166] usb 3-2: new full-speed USB device number 11 using xhci_hcd
> [387473.320074] usb 3-2: New USB device found, idVendor=2516, idProduct=003c
> [387473.320079] usb 3-2: New USB device strings: Mfr=1, Product=2,
> SerialNumber=0
> [387473.320103] usb 3-2: Product: MASTERKEYS PRO S
> [387473.320105] usb 3-2: Manufacturer: Cooler Master Technology Inc.
> [387473.322051] input: Cooler Master Technology Inc. MASTERKEYS PRO S as
> /devices/pci:00/:00:1c.0/:03:00.0/:04:02.0/:07:00.0/usb3/3-2/3-2:1.0/0003:2516:003C.0026/input/input44
> [387473.373897] hid-generic 0003:2516:003C.0026: input,hidraw2: USB HID
> v1.11 Keyboard [Cooler Master Technology Inc. MASTERKEYS PRO S] on
> usb-:07:00.0-2/input0
> [387473.374961] hid-generic 0003:2516:003C.0027: hiddev0,hidraw3: USB
> HID v1.11 Device [Cooler Master Technology Inc. MASTERKEYS PRO S] on
> usb-:07:00.0-2/input1
> [387473.376754] input: Cooler Master Technology Inc. MASTERKEYS PRO S as
> /devices/pci:00/:00:1c.0/:03:00.0/:04:02.0/:07:00.0/usb3/3-2/3-2:1.2/0003:2516:003C.0028/input/input45
> [387473.428595] hid-generic 0003:2516:003C.0028: input,hidraw4: USB HID
> v1.11 Keyboard [Cooler Master Technology Inc. MASTERKEYS PRO S] on
> usb-:07:00.0-2/input2
> 
> As you can see, there are two input: lines having different parts in the
> path in the form of 3-2:1.0 and 3-2:1.2 (to be honest no idea why and
> what that means)

So you have a number of different HID keyboards showing up in your
system from the one device, perhaps that is how it gets away with this
hack.

You might want to ask on the linux-input mailing list, the people there
might know how NKRO works better than we do, we just know the
lower-level USB stuff :)

good luck!

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

Re: [PATCH v2 2/4] usb: xhci-mtk: add generic compatible string

2017-08-22 Thread Chunfeng Yun 
Hi, Mathias

On Wed, 2017-08-16 at 14:08 +0800, Chunfeng Yun wrote:
> The xhci-mtk driver is a generic driver for MediaTek xHCI IP, add
> a generic compatible to avoid confusion when support new SoCs but
> use a compatible with specific SoC's name "mt8173".
> 
> Signed-off-by: Chunfeng Yun 
> ---
>  drivers/usb/host/xhci-mtk.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 67d5dc7..8fb6065 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -795,6 +795,7 @@ static int __maybe_unused xhci_mtk_resume(struct device 
> *dev)
>  #ifdef CONFIG_OF
>  static const struct of_device_id mtk_xhci_of_match[] = {
>   { .compatible = "mediatek,mt8173-xhci"},
> + { .compatible = "mediatek,mtk-xhci"},
>   { },
>  };
>  MODULE_DEVICE_TABLE(of, mtk_xhci_of_match);

Do you have any comment about this patch?
Thanks.



--
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: high irqs-off latency caused by USB serial driver

2017-08-22 Thread David Mosberger 
On Tue, Aug 22, 2017 at 3:40 PM, Greg KH  wrote:
> On Tue, Aug 22, 2017 at 02:44:20PM -0600, David Mosberger wrote:
>> Greg,
>>
>> On Tue, Aug 22, 2017 at 2:25 PM, Greg KH  wrote:
>>
>> > USB has always been a big problem with this, the IRQ patch is very long,
>> > and messy and complex.
>>
>> Yeah.
>>
>> > There was an option a while ago to turn USB irqs
>> > into threaded irqs, do those work on your platform?  If so, that might
>> > help you out here.
>>
>> Do you mean this:
>>
>>https://lkml.org/lkml/2008/10/20/465
>>
>> or is there something else/newer?
>
> I think there was something newer than that almost-a-decade-old thread,
> but I don't remember.  Look at what the RT kernel patch does, it might
> be in there if it wasn't merged into the tree already.

OK, thanks for the pointer.

> What are your requirements that this code path is causing you problems?
> Odds are your USB host controller is pretty horrid, any chance to use a
> different chip for it?

We just have some other soft-realtime stuff going on that doesn't like overly
long periods with interrupts disabled.

  --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: high irqs-off latency caused by USB serial driver

2017-08-22 Thread Greg KH 
On Tue, Aug 22, 2017 at 02:44:20PM -0600, David Mosberger wrote:
> Greg,
> 
> On Tue, Aug 22, 2017 at 2:25 PM, Greg KH  wrote:
> 
> > USB has always been a big problem with this, the IRQ patch is very long,
> > and messy and complex.
> 
> Yeah.
> 
> > There was an option a while ago to turn USB irqs
> > into threaded irqs, do those work on your platform?  If so, that might
> > help you out here.
> 
> Do you mean this:
> 
>https://lkml.org/lkml/2008/10/20/465
> 
> or is there something else/newer?

I think there was something newer than that almost-a-decade-old thread,
but I don't remember.  Look at what the RT kernel patch does, it might
be in there if it wasn't merged into the tree already.

> > The usb-serial path should be really "short" overall, compared with the
> > ohci and tty core logic involved.  What USB-serial driver is this that
> > you are using here?
> 
> It's the FTDI driver (via generic).  I did a quick
> back-of-the-envelope calculation
> and it looks like by changing usb/serial/generic.c to move most of the 
> interrupt
> work to softinterrupt would save about 200 usec out of the 746 usec.  Surely
> that'd be an improvement, but moving the entire OHCI path to softirq would be
> a much bigger win.

By moving just part of the work to a "softirq", yes, that might help,
but then you have increased latency to deal with.

What are your requirements that this code path is causing you problems?
Odds are your USB host controller is pretty horrid, any chance to use a
different chip for it?

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: high irqs-off latency caused by USB serial driver

2017-08-22 Thread David Mosberger 
Greg,

On Tue, Aug 22, 2017 at 2:25 PM, Greg KH  wrote:

> USB has always been a big problem with this, the IRQ patch is very long,
> and messy and complex.

Yeah.

> There was an option a while ago to turn USB irqs
> into threaded irqs, do those work on your platform?  If so, that might
> help you out here.

Do you mean this:

   https://lkml.org/lkml/2008/10/20/465

or is there something else/newer?

> The usb-serial path should be really "short" overall, compared with the
> ohci and tty core logic involved.  What USB-serial driver is this that
> you are using here?

It's the FTDI driver (via generic).  I did a quick
back-of-the-envelope calculation
and it looks like by changing usb/serial/generic.c to move most of the interrupt
work to softinterrupt would save about 200 usec out of the 746 usec.  Surely
that'd be an improvement, but moving the entire OHCI path to softirq would be
a much bigger win.

  --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: high irqs-off latency caused by USB serial driver

2017-08-22 Thread Greg KH 
On Tue, Aug 22, 2017 at 12:42:33PM -0600, David Mosberger wrote:
> Has anyone here looked into reducing the amount of time the USB serial
> driver disables interrupts?  On an ARM system, I'm seeing about 746 us
> of latency for handling a USB interrupt, which seems rather excessive.
> I attached a trace captured with the irqsoff tracer.

USB has always been a big problem with this, the IRQ patch is very long,
and messy and complex.  There was an option a while ago to turn USB irqs
into threaded irqs, do those work on your platform?  If so, that might
help you out here.

The usb-serial path should be really "short" overall, compared with the
ohci and tty core logic involved.  What USB-serial driver is this that
you are using here?

> 
> Note: the 500MHz ARM cycle counter was used for timestamping
> the entry so the reported times have to be doubled to get actual time
> in micro-seconds.
> 
> >From what I can see:
> 
>  o OHCI takes interrupt and frees up an URB that is done
>  o USB serial driver submits a new URB (for transmit, I think)
>  o another URB is freed (receive, I think), then a new receive URB is
> submited
> 
> I'm hoping there is something silly going on and things are done at the
> hardirq level when they should be done as softirqs, but I just started
> looking into this.
> 
> Anyone have any helpful thoughts/pointers?

Don't ever use USB on a device that you care about IRQ timing :)

Seriously, it's a mess, all because of the hardware involved...

good luck!

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: [GIT PULL] USB changes for v4.14

2017-08-22 Thread Greg Kroah-Hartman 
On Mon, Aug 21, 2017 at 02:41:37PM +0300, Felipe Balbi wrote:
> 
> Hi Greg,
> 
> Here's the USB gadget pull request. Not much going on this time around.
> 
> Changes have been in linux-next for a while with no bug reports. I have
> also tested what I could on GLK.
> 
> Let me know if you want anything to be changed.
> 
> The following changes since commit aae4e7a8bc44722fe70d58920a36916b1043195e:
> 
>   Linux 4.13-rc4 (2017-08-06 18:44:49 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> tags/usb-for-v4.14

Pulled and pushed out, 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

high irqs-off latency caused by USB serial driver

2017-08-22 Thread David Mosberger 
Has anyone here looked into reducing the amount of time the USB serial
driver disables interrupts?  On an ARM system, I'm seeing about 746 us
of latency for handling a USB interrupt, which seems rather excessive.
I attached a trace captured with the irqsoff tracer.

Note: the 500MHz ARM cycle counter was used for timestamping
the entry so the reported times have to be doubled to get actual time
in micro-seconds.

>From what I can see:

 o OHCI takes interrupt and frees up an URB that is done
 o USB serial driver submits a new URB (for transmit, I think)
 o another URB is freed (receive, I think), then a new receive URB is
submited

I'm hoping there is something silly going on and things are done at the
hardirq level when they should be done as softirqs, but I just started
looking into this.

Anyone have any helpful thoughts/pointers?

Thanks!

  --david

# tracer: irqsoff
#
# irqsoff latency trace v1.1.5 on 4.9.44+
# 
# latency: 373 us, #188/188, CPU#0 | (M:server VP:0, KP:0, SP:0 HP:0)
#-
#| task: kworker/0:0-1437 (uid:0 nice:0 policy:0 rt_prio:0)
#-
#  => started at: __irq_usr
#  => ended at:   __schedule
#
#
#  _--=> CPU#
# / _-=> irqs-off
#| / _=> need-resched
#|| / _---=> hardirq/softirq
#||| / _--=> preempt-depth
# / delay
#  cmd pid   | time  |   caller
# \   /  |  \|   /
   <...>-18960d... 1650: __irq_usr
   <...>-18960d... 5443: aic5_handle <-__irq_usr
   <...>-18960d... 7472: irq_get_domain_generic_chip <-aic5_handle
   <...>-18960d... 10194: __handle_domain_irq <-aic5_handle
   <...>-18960d... 11685: irq_enter <-__handle_domain_irq
   <...>-18960d.h. 13521: irq_find_mapping <-__handle_domain_irq
   <...>-18960d.h. 15403: generic_handle_irq <-__handle_domain_irq
   <...>-18960d.h. 16673: irq_to_desc <-generic_handle_irq
   <...>-18960d.h. 18662: handle_fasteoi_irq <-generic_handle_irq
   <...>-18960d.h. 20194: irq_may_run <-handle_fasteoi_irq
   <...>-18960d.h. 22426: handle_irq_event <-handle_fasteoi_irq
   <...>-18960d.h. 23743: handle_irq_event_percpu <-handle_irq_event
   <...>-18960d.h. 25072: __handle_irq_event_percpu
<-handle_irq_event_percpu
   <...>-18960d.h. 26492: usb_hcd_irq <-__handle_irq_event_percpu
   <...>-18960d.h. 28284: ohci_irq <-usb_hcd_irq
   <...>-18960d.h. 32109: arm_heavy_mb <-ohci_irq
   <...>-18960d.h. 33430: l2c210_sync <-arm_heavy_mb
   <...>-18960d.h. 37489: add_to_done_list.part.0 <-ohci_irq
   <...>-18960d.h. 39777: add_to_done_list.part.0 <-ohci_irq
   <...>-18960d.h. 41538: ohci_work <-ohci_irq
   <...>-18960d.h. 43671: td_done <-ohci_work
   <...>-18960d.h. 47023: finish_urb <-ohci_work
   <...>-18960d.h. 48342: urb_free_priv <-finish_urb
   <...>-18960d.h. 49684: td_free <-urb_free_priv
   <...>-18960d.h. 51204: dma_pool_free <-td_free
   <...>-18960d.h. 54677: kfree <-urb_free_priv
   <...>-18960d.h. 56661: ___cache_free <-kfree
   <...>-18960d.h. 59101: usb_hcd_unlink_urb_from_ep <-finish_urb
   <...>-18960d.h. 61064: usb_hcd_giveback_urb <-finish_urb
   <...>-18960d.h. 62745: __usb_hcd_giveback_urb <-usb_hcd_giveback_urb
   <...>-18960d.h. 64082: unmap_urb_for_dma <-__usb_hcd_giveback_urb
   <...>-18960d.h. 65456: usb_hcd_unmap_urb_for_dma <-unmap_urb_for_dma
   <...>-18960d.h. 66757: usb_hcd_unmap_urb_setup_for_dma
<-usb_hcd_unmap_urb_for_dma
   <...>-18960d.h. 69135: arm_dma_unmap_page <-usb_hcd_unmap_urb_for_dma
   <...>-18960d.h. 70590: __dma_page_dev_to_cpu <-arm_dma_unmap_page
   <...>-18960d.h. 72810: usb_anchor_suspend_wakeups
<-__usb_hcd_giveback_urb
   <...>-18960d.h. 74157: usb_unanchor_urb <-__usb_hcd_giveback_urb
   <...>-18960d.h. 76333: usb_serial_generic_write_bulk_callback
<-__usb_hcd_giveback_urb
   <...>-18960d.h. 79605: usb_serial_generic_write_start
<-usb_serial_generic_write_bulk_callback
   <...>-18960d.h. 84314: ftdi_prepare_write_buffer
<-usb_serial_generic_write_start
   <...>-18960d.h. 91988: usb_submit_urb <-usb_serial_generic_write_start
   <...>-18960d.h. 96555: usb_hcd_submit_urb <-usb_submit_urb
   <...>-18960d.h. 98117: usb_get_urb <-usb_hcd_submit_urb
   <...>-18960d.h. 99710: usb_hcd_map_urb_for_dma <-usb_hcd_submit_urb
   <...>-18960d.h. 102131: arm_dma_map_page <-usb_hcd_map_urb_for_dma
   <...>-18960d.h. 103581: __dma_page_cpu_to_dev <-arm_dma_map_page
   <...>-18960d.h. 105096: dma_cache_maint_page <-__dma_page_cpu_to_dev
   <...>-18960d.h. 107224: l2c210_clean_range <-__dma_page_cpu_to_dev
   <...>-18960d.h. 110437: ohci_urb_enqueue <-usb_hcd_submit_urb
   <...>-18960d.h. 114226: __kmalloc <-ohci_urb_enqueue
   <...>-18960d.h. 115568: kmalloc_slab <-__kmalloc

Re: [PATCH] udc: Memory leak on error path and use after free

2017-08-22 Thread Alan Stern 
On Tue, 22 Aug 2017, Anton Vasilyev wrote:

> Sorry for delayed reply.
> 
> On 16.08.2017 19:35, Alan Stern wrote:
> > On Wed, 16 Aug 2017, Anton Vasilyev wrote:
> > 
> >> On 16.08.2017 18:29, Alan Stern wrote:
> >>> On Wed, 16 Aug 2017, Anton Vasilyev wrote:
> >>>
>  gadget_release() is responsible for cleanup dev memory.
>  But if net2280_probe() fails after dev allocation, then
>  gadget_release() become unregistered and dev memory leaks.
> >>>
> >>> This isn't needed if usb_add_gadget_udc_release() is fixed, right?
> >>>
> >>
> >> No, this situation could appear before call
> >> usb_add_gadget_udc_release().
> >>
>  Also net2280_remove() calls usb_del_gadget_udc() which
>  perform schedule_delayed_work() with gadget_release(), so
>  it is possible that dev will be deallocated exactly after
>  this call and leads to use after free.
> >>>
> >>> Where is there a possible use after free?
> >>>
> >>
> >> net2280_remove() continue work with struct net2280 *dev after call
> >> usb_del_gadget_udc(>gadget), but this net2280 *dev could be
> >> deallocated by gadget_release()
> >>
>  The patch moves deallocation from gadget_release() to
>  net2280_remove().
> >>>
> >>> Alan Stern
> > 
> > Okay, now I understand what you were saying.  Yes, I agree, the
> > existing code isn't right.
> > 
> > But a better solution would be to move the usb_del_gadget_udc() call
> > from the beginning of net2280_remove() to the end.  And make the call
> > conditional, depending on whether usb_add_gadget_udc_release() has
> > already been called successfully.
> 
> If allow gadget_release() to deallocate net2280 *dev then it will be 
> called on fail of usb_add_gadget_udc_release() and it will be unsafe to 
> perform clean-up.
> My point is that gadget shouldn't deallocate its parent memory at all.

I disagree.  It's okay for the parent memory to be deallocated along
with the gadget, provided the driver can guarantee that the parent
memory won't be used any more after the gadget is released.

One way to guarantee this is to make usb_add_gadget_udc_release() do an 
extra device_get().  Then the release won't occur until after
usb_del_gadget_udc() has been called _and_ the driver has called 
device_put().

> > The point is that the device core does not allow drivers to deallocate
> > memory containing a struct device before the ->release callback has
> > been invoked.  Your patch might do that, if the release was delayed for
> > some reason.
> 
> I don't see possibility for parent device to be removed before its child 
> was released. Please point if I'm wrong.

The device core has lots of ways of keeping references to a device,
even after the device has been unregistered.  I don't know if any of
them apply in this case, but even if they don't, it's possible that
such a mechanism will be added in the future.

> Alternative way to move allocation under devm interface.

Yes, that would work too.

Don't forget, this problem affects all UDC drivers that call 
usb_add_gadget_udc_release(), not just net2280.  A proper fix will most 
likely have to change all of them.

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: Web camera USB bandwidth allocation

2017-08-22 Thread Alan Stern 
On Tue, 22 Aug 2017, Martin Oprešnik wrote:

> Hello,
> 
> we are working on a project, where we need multiple cameras connected to 
> embedded computer. For computer we have chosen odroid XU4 and for 
> cameras Logitech C920. We need at least 5 cameras running with 720p. The 
> problem we have is allocated USB bandwidth (cameras are using 
> isochronous transfer). Currently we can run 3 cameras on 720p. Image is 
> transferred in h264 compressed format. Writing on disk is done directly 
> trough memory map using V4L2 api and files on disk are really small and 
> using usbmon/wireshark shows ~1104kB/s (there should be a lot of USB 
> bandwidth left).

What happens when you try to use the fourth camera?

> I tried same setups on my laptop and I don't get any better results, so 
> I suspect there is some space for improvement in software and it's not 
> odroid's fault. I tried changing different parameters in uvcvideo driver 
> and I think only wMaxPacketSize is affecting bandwidth allocation. And 
> changing that is reflected in non working camera.
> Now I don't know enough about USB and linux kernel to work around this 
> problem and I would be very happy if you could give me some help or 
> directions.
> If I should provide some additional information, I'll be happy to add it.
> 
> I am looking forward to hearing from you.

Please try the following.  First, mount a debugfs filesystem:

mount -t debugfs none /sys/kernel/debug

Then while running with 3 cameras, make a copy of 
/sys/kernel/debug/usb/devices and post the copy.

Also, what version of the kernel are you using?

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: gadget: f_midi: Use snd_card_free_when_closed with refcount

2017-08-22 Thread Jerry Zhang 
Yes you're right. I'm not sure that would be the best approach though
since you'd have to iterate through every function instance, and you'd
end up leaking the memory if the device was open. I'm working on
modifying the original patch to make use of the opts refcnt as well.

On Mon, Aug 21, 2017 at 11:37 PM, Manu Gautam  wrote:
> Hi,
>
>
> On 8/18/2017 11:30 AM, Manu Gautam wrote:
>> Hi,
>>
>>
>> On 8/15/2017 2:44 AM, Jerry Zhang wrote:
>
>>> @@ -1197,14 +1200,21 @@ static void f_midi_free(struct usb_function *f)
>>>
>>>  midi = func_to_midi(f);
>>>  opts = container_of(f->fi, struct f_midi_opts, func_inst);
>> opts could be freed as well if f_midi_free_inst already happened. Say 
>> another user
>> deleted midi instance  before pcm_file was released.
>
> This would be a regression (use-after-free) with the patch.
> Do you plan to fix this as I see Felipe has already queued this for 4.14.
>
> One simple solution could be to fail midi free_instance if pcm device
> is in-use/open.
>
>
>>> -kfree(midi->id);
>>>  mutex_lock(>lock);
>>> -kfifo_free(>in_req_fifo);
>>> -kfree(midi);
>>> ---opts->refcnt;
>>> +if (!--midi->free_ref) {
>>> +kfree(midi->id);
>>> +kfifo_free(>in_req_fifo);
>>> +kfree(midi);
>>> +--opts->refcnt;
>>> +}
>>>  mutex_unlock(>lock);
>>>  }
>>>
>>> +static void f_midi_rmidi_free(struct snd_rawmidi *rmidi)
>>> +{
>>> +f_midi_free(rmidi->private_data);
>>> +}
>>> +
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Possible regression between 4.9 and 4.13

2017-08-22 Thread Mason 
Hello,

The driver for my system's PCIe host bridge landed recently
(in 4.13) but it was developed on 4.9

I tested the PCIe host bridge by plugging a 4-port USB3 adapter
into the PCIe slot (system at rest) and plugging an USB3 Flash
drive into the USB3 adapter (at run-time).

On 4.9, the setup works (almost perfectly, see below).
On 4.13, once I unplug the Flash drive, the controller port
remains unresponsive.


On 4.9, I said *almost* perfectly, because the pcieport driver
does report a few non-fatal errors when I unplug:

[  193.838504] usb 2-2: new SuperSpeed USB device number 2 using xhci_hcd
[  193.878081] usb-storage 2-2:1.0: USB Mass Storage device detected
[  193.884547] scsi host0: usb-storage 2-2:1.0
[  194.907936] scsi 0:0:0:0: Direct-Access Kingston DataTraveler 3.0  
PQ: 0 ANSI: 6
[  194.920296] sd 0:0:0:0: [sda] 15109516 512-byte logical blocks: (7.74 
GB/7.20 GiB)
[  194.928666] sd 0:0:0:0: [sda] Write Protect is off
[  194.933755] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, 
doesn't support DPO or FUA
[  194.946074]  sda: sda1
[  194.953608] sd 0:0:0:0: [sda] Attached SCSI removable disk

[  208.930260] pcieport :00:00.0: AER: Uncorrected (Non-Fatal) error 
received: id=
[  208.938342] pcieport :00:00.0: PCIe Bus Error: severity=Uncorrected 
(Non-Fatal), type=Transaction Layer, id=(Requester ID)
[  208.950163] pcieport :00:00.0:   device [1105:0024] error 
status/mask=4000/
[  208.958577] pcieport :00:00.0:[14] Completion Timeout (First)
[  208.965432] pcieport :00:00.0: AER: Device recovery failed
[  209.663733] xhci_hcd :01:00.0: Cannot set link state.
[  209.669194] usb usb2-port2: cannot disable (err = -32)
[  209.674376] usb 2-2: USB disconnect, device number 2
[  209.680481] pcieport :00:00.0: AER: Uncorrected (Non-Fatal) error 
received: id=
[  209.688689] pcieport :00:00.0: PCIe Bus Error: severity=Uncorrected 
(Non-Fatal), type=Transaction Layer, id=(Requester ID)
[  209.700555] pcieport :00:00.0:   device [1105:0024] error 
status/mask=4000/
[  209.708978] pcieport :00:00.0:[14] Completion Timeout (First)
[  209.715845] pcieport :00:00.0: AER: Device recovery failed
[  209.721722] pcieport :00:00.0: AER: Uncorrected (Non-Fatal) error 
received: id=
[  209.729785] pcieport :00:00.0: PCIe Bus Error: severity=Uncorrected 
(Non-Fatal), type=Transaction Layer, id=(Requester ID)
[  209.741602] pcieport :00:00.0:   device [1105:0024] error 
status/mask=4000/
[  209.750027] pcieport :00:00.0:[14] Completion Timeout (First)
[  209.756866] pcieport :00:00.0: AER: Device recovery failed

After that, I can still plug the drive into the same port.

But on 4.13, I get

[   27.330378] usb 2-2: new SuperSpeed USB device number 2 using xhci_hcd
[   27.369383] usb-storage 2-2:1.0: USB Mass Storage device detected
[   27.375840] scsi host0: usb-storage 2-2:1.0
[   28.403035] scsi 0:0:0:0: Direct-Access Kingston DataTraveler 3.0  
PQ: 0 ANSI: 6
[   28.413326] sd 0:0:0:0: [sda] 15109516 512-byte logical blocks: (7.74 
GB/7.20 GiB)
[   28.423653] sd 0:0:0:0: [sda] Write Protect is off
[   28.429139] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, 
doesn't support DPO or FUA
[   28.441529]  sda: sda1
[   28.449431] sd 0:0:0:0: [sda] Attached SCSI removable disk

[   90.592134] xhci_hcd :01:00.0: xHCI host controller not responding, 
assume dead
[   90.599857] xhci_hcd :01:00.0: HC died; cleaning up
[   90.605336] usb 2-2: USB disconnect, device number 2
[   90.630414] udevd[955]: inotify_add_watch(6, /dev/sda, 10) failed: No such 
file or directory

Trying to replug into the same port = nothing happens
(Linux did say "assume dead")

Any idea what could have changed between 4.9 and 4.13 ?

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

Web camera USB bandwidth allocation

2017-08-22 Thread Martin Oprešnik 

Hello,

we are working on a project, where we need multiple cameras connected to 
embedded computer. For computer we have chosen odroid XU4 and for 
cameras Logitech C920. We need at least 5 cameras running with 720p. The 
problem we have is allocated USB bandwidth (cameras are using 
isochronous transfer). Currently we can run 3 cameras on 720p. Image is 
transferred in h264 compressed format. Writing on disk is done directly 
trough memory map using V4L2 api and files on disk are really small and 
using usbmon/wireshark shows ~1104kB/s (there should be a lot of USB 
bandwidth left).
I tried same setups on my laptop and I don't get any better results, so 
I suspect there is some space for improvement in software and it's not 
odroid's fault. I tried changing different parameters in uvcvideo driver 
and I think only wMaxPacketSize is affecting bandwidth allocation. And 
changing that is reflected in non working camera.
Now I don't know enough about USB and linux kernel to work around this 
problem and I would be very happy if you could give me some help or 
directions.

If I should provide some additional information, I'll be happy to add it.

I am looking forward to hearing from you.

Best regard, Martin

--
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] udc: Memory leak on error path and use after free

2017-08-22 Thread Anton Vasilyev 


Sorry for delayed reply.

On 16.08.2017 19:35, Alan Stern wrote:

On Wed, 16 Aug 2017, Anton Vasilyev wrote:


On 16.08.2017 18:29, Alan Stern wrote:

On Wed, 16 Aug 2017, Anton Vasilyev wrote:


gadget_release() is responsible for cleanup dev memory.
But if net2280_probe() fails after dev allocation, then
gadget_release() become unregistered and dev memory leaks.


This isn't needed if usb_add_gadget_udc_release() is fixed, right?



No, this situation could appear before call
usb_add_gadget_udc_release().


Also net2280_remove() calls usb_del_gadget_udc() which
perform schedule_delayed_work() with gadget_release(), so
it is possible that dev will be deallocated exactly after
this call and leads to use after free.


Where is there a possible use after free?



net2280_remove() continue work with struct net2280 *dev after call
usb_del_gadget_udc(>gadget), but this net2280 *dev could be
deallocated by gadget_release()


The patch moves deallocation from gadget_release() to
net2280_remove().


Alan Stern


Okay, now I understand what you were saying.  Yes, I agree, the
existing code isn't right.

But a better solution would be to move the usb_del_gadget_udc() call
from the beginning of net2280_remove() to the end.  And make the call
conditional, depending on whether usb_add_gadget_udc_release() has
already been called successfully.


If allow gadget_release() to deallocate net2280 *dev then it will be 
called on fail of usb_add_gadget_udc_release() and it will be unsafe to 
perform clean-up.

My point is that gadget shouldn't deallocate its parent memory at all.



The point is that the device core does not allow drivers to deallocate
memory containing a struct device before the ->release callback has
been invoked.  Your patch might do that, if the release was delayed for
some reason.


I don't see possibility for parent device to be removed before its child 
was released. Please point if I'm wrong.


Alternative way to move allocation under devm interface.



Alan Stern



--
Anton Vasilyev
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: vasil...@ispras.ru
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Possible double free in iowarrior.ko

2017-08-22 Thread Anton Volkov 

Hello.

While searching for races in the Linux kernel I've come across 
"drivers/usb/misc/iowarrior.ko" module. Here are questions that I came 
up with while analyzing results. Lines are given using the info from 
Linux v4.12.


Consider the following case:

Thread 1:Thread 2:
iowarrior_releaseiowarrior_disconnect
   mutex_lock(>mutex)
   dev->present = 0
   (iowarrior.c: line 889)
  mutex_lock(>mutex)  mutex_unlock(>mutex)
  dev->opened = 0
  (iowarrior.c: line 666)  if(dev->opened){
  if(dev->present){   //dev->opened == 0
//dev->present ==0
  } else { } else {
mutex_unlock(>mutex)iowarrior_delete(dev)
iowarrior_delete(dev)  }
  }

In this case double free of several pointers inside iowarrior_delete 
becomes possible and no calls to usb_kill_urb() and 
wake_up_interruptible() are present. Is this feasible from your point of 
view? If so, maybe it is a good idea to move mutex_unlock(>mutex) 
in iowarrior_disconnect() further down like in iowarrior_release() in 
both 'if' branches?


Thank you for your time

-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: avol...@ispras.ru
--
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: Possible bug in cypress_m8.ko

2017-08-22 Thread Oliver Neukum 
Am Dienstag, den 22.08.2017, 15:11 +0300 schrieb Anton Volkov:
> Hello.
> 
> Judging by the code of cypress_m8.c some functions are considered to be 
> capable of working concurrently with other functions, e.g. cypress_open.
> There are, however, entities that are protected by the locks at one 
> place and not protected in another. Lines are given using the info from 
> Linux kernel v4.12. Example:
> 
> cypress_send
>spin_lock_irqsave
>priv->write_urb_in_use = 1;
>spin_lock_irqrestore
>(cypress_m8.c: lines 761-763)
>...
>if (result) {
>   priv->write_urb_in_use = 0; //without lock protection
>   (cypress_m8.c: line 783)
>}
> 
> Is it a bug?

Yes, but not of the kind you describe.
The transition from "not in use" to "in use" must be guarded by
a lock, because it may be contended.
But if that transition is properly guarded, you already know
that there can be only user. He can theoretically give up
the resource without locking.

Yet there is a bug:

^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  702)   
spin_lock_irqsave(>lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  703)   if 
(priv->write_urb_in_use) {
441b62c1edb98 (Harvey Harrison2008-03-03 16:08:34 -0800  704)   
dbg("%s - can't write, urb in use", __func__);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  705)   
spin_unlock_irqrestore(>lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  706)   
return;
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  707)   }
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  708)   
spin_unlock_irqrestore(>lock, flags);

The flag is checked is checked under a lock. But then the lock is dropped.
And here:

^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  759)   
spin_lock_irqsave(>lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  760)   
priv->write_urb_in_use = 1;
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700  761)   
spin_unlock_irqrestore(>lock, flags);

The flag is set under lock, but unconditionally.
The code just makes no sense.


In addition, when you drop the flag without a lock you need to worry
about memory ordering.

HTH
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

Re: [PATCH 15/15] usb: make device_type const

2017-08-22 Thread Heikki Krogerus 
On Sat, Aug 19, 2017 at 01:52:26PM +0530, Bhumika Goyal wrote:
> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal 

Acked-by: Heikki Krogerus 

> ---
>  drivers/usb/common/ulpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 930e8f3..4aa5195 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -135,7 +135,7 @@ static void ulpi_dev_release(struct device *dev)
>   kfree(to_ulpi_dev(dev));
>  }
>  
> -static struct device_type ulpi_dev_type = {
> +static const struct device_type ulpi_dev_type = {
>   .name = "ulpi_device",
>   .groups = ulpi_dev_attr_groups,
>   .release = ulpi_dev_release,

Thanks,

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

[PATCH 0/4] constify videobuf_queue_ops structures

2017-08-22 Thread Arvind Yadav 
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by  work
with const videobuf_queue_ops. So mark the non-const structs as const.

Arvind Yadav (4):
  [PATCH 1/4] [media] saa7146: constify videobuf_queue_ops structures
  [PATCH 2/4] [media] pci: constify videobuf_queue_ops structures
  [PATCH 3/4] [media] platform: constify videobuf_queue_ops structures
  [PATCH 4/4] [media] usb: constify videobuf_queue_ops structures

 drivers/media/common/saa7146/saa7146_vbi.c| 2 +-
 drivers/media/common/saa7146/saa7146_video.c  | 2 +-
 drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
 drivers/media/pci/cx18/cx18-streams.c | 2 +-
 drivers/media/platform/davinci/vpfe_capture.c | 2 +-
 drivers/media/platform/fsl-viu.c  | 2 +-
 drivers/media/usb/cx231xx/cx231xx-417.c   | 2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c | 2 +-
 drivers/media/usb/tm6000/tm6000-video.c   | 2 +-
 drivers/media/usb/zr364xx/zr364xx.c   | 2 +-
 10 files changed, 10 insertions(+), 10 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 1/4] [media] saa7146: constify videobuf_queue_ops structures

2017-08-22 Thread Arvind Yadav 
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by  work
with const videobuf_queue_ops. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/common/saa7146/saa7146_vbi.c   | 2 +-
 drivers/media/common/saa7146/saa7146_video.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index 3553ac4..d79e4d7 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -308,7 +308,7 @@ static void buffer_release(struct videobuf_queue *q, struct 
videobuf_buffer *vb)
saa7146_dma_free(dev,q,buf);
 }
 
-static struct videobuf_queue_ops vbi_qops = {
+static const struct videobuf_queue_ops vbi_qops = {
.buf_setup= buffer_setup,
.buf_prepare  = buffer_prepare,
.buf_queue= buffer_queue,
diff --git a/drivers/media/common/saa7146/saa7146_video.c 
b/drivers/media/common/saa7146/saa7146_video.c
index b3b29d4..37b4654 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1187,7 +1187,7 @@ static void buffer_release(struct videobuf_queue *q, 
struct videobuf_buffer *vb)
release_all_pagetables(dev, buf);
 }
 
-static struct videobuf_queue_ops video_qops = {
+static const struct videobuf_queue_ops video_qops = {
.buf_setup= buffer_setup,
.buf_prepare  = buffer_prepare,
.buf_queue= buffer_queue,
-- 
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 4/4] [media] usb: constify videobuf_queue_ops structures

2017-08-22 Thread Arvind Yadav 
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by  work
with const videobuf_queue_ops. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/usb/cx231xx/cx231xx-417.c   | 2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c | 2 +-
 drivers/media/usb/tm6000/tm6000-video.c   | 2 +-
 drivers/media/usb/zr364xx/zr364xx.c   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c 
b/drivers/media/usb/cx231xx/cx231xx-417.c
index 509d971..2b16808 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1490,7 +1490,7 @@ static void bb_buf_release(struct videobuf_queue *q,
free_buffer(q, buf);
 }
 
-static struct videobuf_queue_ops cx231xx_qops = {
+static const struct videobuf_queue_ops cx231xx_qops = {
.buf_setup= bb_buf_setup,
.buf_prepare  = bb_buf_prepare,
.buf_queue= bb_buf_queue,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c 
b/drivers/media/usb/cx231xx/cx231xx-video.c
index f67f868..179b848 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -859,7 +859,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops cx231xx_video_qops = {
+static const struct videobuf_queue_ops cx231xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,
diff --git a/drivers/media/usb/tm6000/tm6000-video.c 
b/drivers/media/usb/tm6000/tm6000-video.c
index 7e960d0..35925f1 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -801,7 +801,7 @@ static void buffer_release(struct videobuf_queue *vq, 
struct videobuf_buffer *vb
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops tm6000_video_qops = {
+static const struct videobuf_queue_ops tm6000_video_qops = {
.buf_setup  = buffer_setup,
.buf_prepare= buffer_prepare,
.buf_queue  = buffer_queue,
diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index efdcd5b..24d5860 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -439,7 +439,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops zr364xx_video_qops = {
+static const struct videobuf_queue_ops zr364xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,
-- 
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/4] [media] pci: constify videobuf_queue_ops structures

2017-08-22 Thread Arvind Yadav 
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by  work
with const videobuf_queue_ops. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
 drivers/media/pci/cx18/cx18-streams.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c 
b/drivers/media/pci/bt8xx/bttv-driver.c
index ed319f1..c2f1ba0 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -1702,7 +1702,7 @@ static void buffer_release(struct videobuf_queue *q, 
struct videobuf_buffer *vb)
bttv_dma_free(q,fh->btv,buf);
 }
 
-static struct videobuf_queue_ops bttv_video_qops = {
+static const struct videobuf_queue_ops bttv_video_qops = {
.buf_setup= buffer_setup,
.buf_prepare  = buffer_prepare,
.buf_queue= buffer_queue,
diff --git a/drivers/media/pci/cx18/cx18-streams.c 
b/drivers/media/pci/cx18/cx18-streams.c
index 3c45e007..81d06c1 100644
--- a/drivers/media/pci/cx18/cx18-streams.c
+++ b/drivers/media/pci/cx18/cx18-streams.c
@@ -240,7 +240,7 @@ static void buffer_queue(struct videobuf_queue *q, struct 
videobuf_buffer *vb)
list_add_tail(>vb.queue, >vb_capture);
 }
 
-static struct videobuf_queue_ops cx18_videobuf_qops = {
+static const struct videobuf_queue_ops cx18_videobuf_qops = {
.buf_setup= buffer_setup,
.buf_prepare  = buffer_prepare,
.buf_queue= buffer_queue,
-- 
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 3/4] [media] platform: constify videobuf_queue_ops structures

2017-08-22 Thread Arvind Yadav 
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by  work
with const videobuf_queue_ops. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/platform/davinci/vpfe_capture.c | 2 +-
 drivers/media/platform/fsl-viu.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index b1bf4a7..6792da1 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1288,7 +1288,7 @@ static void vpfe_videobuf_release(struct videobuf_queue 
*vq,
vb->state = VIDEOBUF_NEEDS_INIT;
 }
 
-static struct videobuf_queue_ops vpfe_videobuf_qops = {
+static const struct videobuf_queue_ops vpfe_videobuf_qops = {
.buf_setup  = vpfe_videobuf_setup,
.buf_prepare= vpfe_videobuf_prepare,
.buf_queue  = vpfe_videobuf_queue,
diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index 97e164b..2aac8e8 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -549,7 +549,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops viu_video_qops = {
+static const struct videobuf_queue_ops viu_video_qops = {
.buf_setup  = buffer_setup,
.buf_prepare= buffer_prepare,
.buf_queue  = buffer_queue,
-- 
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

Possible bug in cypress_m8.ko

2017-08-22 Thread Anton Volkov 

Hello.

Judging by the code of cypress_m8.c some functions are considered to be 
capable of working concurrently with other functions, e.g. cypress_open.
There are, however, entities that are protected by the locks at one 
place and not protected in another. Lines are given using the info from 
Linux kernel v4.12. Example:


cypress_send
  spin_lock_irqsave
  priv->write_urb_in_use = 1;
  spin_lock_irqrestore
  (cypress_m8.c: lines 761-763)
  ...
  if (result) {
 priv->write_urb_in_use = 0; //without lock protection
 (cypress_m8.c: line 783)
  }

Is it a bug?

Thank you for your time.

-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: avol...@ispras.ru
--
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] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

2017-08-22 Thread Kai-Heng Feng 
This reverts commit dec08194ffeccfa1cf085906b53d301930eae18f.

Commit dec08194ffec ("xhci: Limit USB2 port wake support for AMD Promontory
hosts") makes all high speed USB ports on ASUS PRIME B350M-A cease to
function after enabling runtime PM.

All boards with this chipsets will be affected, so revert the commit.

Conflicts:
drivers/usb/host/xhci-pci.c
drivers/usb/host/xhci.h

Signed-off-by: Kai-Heng Feng 
---
 drivers/usb/host/xhci-hub.c |  3 ---
 drivers/usb/host/xhci-pci.c | 12 
 drivers/usb/host/xhci.h |  2 +-
 3 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 00721e8807ab..4bd8654b1233 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1473,9 +1473,6 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
t2 |= PORT_WKOC_E | PORT_WKCONN_E;
t2 &= ~PORT_WKDISC_E;
}
-   if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) &&
-   (hcd->speed < HCD_USB3))
-   t2 &= ~PORT_WAKE_BITS;
} else
t2 &= ~PORT_WAKE_BITS;
 
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 8071c8fdd15e..76f392954733 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -54,11 +54,6 @@
 #define PCI_DEVICE_ID_INTEL_APL_XHCI   0x5aa8
 #define PCI_DEVICE_ID_INTEL_DNV_XHCI   0x19d0
 
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_40x43b9
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_30x43ba
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_20x43bb
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_10x43bc
-
 #define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI   0x1142
 
 static const char hcd_name[] = "xhci_hcd";
@@ -142,13 +137,6 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_AMD)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 
-   if ((pdev->vendor == PCI_VENDOR_ID_AMD) &&
-   ((pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4) ||
-   (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_3) ||
-   (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_2) ||
-   (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_1)))
-   xhci->quirks |= XHCI_U2_DISABLE_WAKE;
-
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
xhci->quirks |= XHCI_LPM_SUPPORT;
xhci->quirks |= XHCI_INTEL_HOST;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e935291ed6..d6b471823a0b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1819,7 +1819,7 @@ struct xhci_hcd {
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED   (1 << 25)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
-#define XHCI_U2_DISABLE_WAKE   (1 << 27)
+/* Reserved. It was XHCI_U2_DISABLE_WAKE */
 #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL(1 << 28)
 
unsigned intnum_active_eps;
-- 
2.14.1

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

Re: [PATCH] usb: gadget: f_midi: Use snd_card_free_when_closed with refcount

2017-08-22 Thread Manu Gautam 
Hi,


On 8/18/2017 11:30 AM, Manu Gautam wrote:
> Hi,
>
>
> On 8/15/2017 2:44 AM, Jerry Zhang wrote:

>> @@ -1197,14 +1200,21 @@ static void f_midi_free(struct usb_function *f)
>>  
>>  midi = func_to_midi(f);
>>  opts = container_of(f->fi, struct f_midi_opts, func_inst);
> opts could be freed as well if f_midi_free_inst already happened. Say another 
> user
> deleted midi instance  before pcm_file was released.

This would be a regression (use-after-free) with the patch.
Do you plan to fix this as I see Felipe has already queued this for 4.14.

One simple solution could be to fail midi free_instance if pcm device
is in-use/open.


>> -kfree(midi->id);
>>  mutex_lock(>lock);
>> -kfifo_free(>in_req_fifo);
>> -kfree(midi);
>> ---opts->refcnt;
>> +if (!--midi->free_ref) {
>> +kfree(midi->id);
>> +kfifo_free(>in_req_fifo);
>> +kfree(midi);
>> +--opts->refcnt;
>> +}
>>  mutex_unlock(>lock);
>>  }
>>  
>> +static void f_midi_rmidi_free(struct snd_rawmidi *rmidi)
>> +{
>> +f_midi_free(rmidi->private_data);
>> +}
>> +

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
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 next 1/2] phy: phy-mtk-tphy: fix NULL point of chip bank

2017-08-22 Thread Chunfeng Yun 
Chip bank of version-1 is initialized as NULL, but it's used
by pcie_phy_instance_power_on/off(), so assign it a right
address.

Signed-off-by: Chunfeng Yun 
---
 drivers/phy/mediatek/phy-mtk-tphy.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c 
b/drivers/phy/mediatek/phy-mtk-tphy.c
index 1ca7fe3..a440de9 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -28,6 +28,7 @@
 /* banks shared by multiple phys */
 #define SSUSB_SIFSLV_V1_SPLLC  0x000   /* shared by u3 phys */
 #define SSUSB_SIFSLV_V1_U2FREQ 0x100   /* shared by u2 phys */
+#define SSUSB_SIFSLV_V1_CHIP   0x300   /* shared by u3 phys */
 /* u2 phy bank */
 #define SSUSB_SIFSLV_V1_U2PHY_COM  0x000
 /* u3/pcie/sata phy banks */
@@ -763,7 +764,7 @@ static void phy_v1_banks_init(struct mtk_tphy *tphy,
case PHY_TYPE_USB3:
case PHY_TYPE_PCIE:
u3_banks->spllc = tphy->sif_base + SSUSB_SIFSLV_V1_SPLLC;
-   u3_banks->chip = NULL;
+   u3_banks->chip = tphy->sif_base + SSUSB_SIFSLV_V1_CHIP;
u3_banks->phyd = instance->port_base + SSUSB_SIFSLV_V1_U3PHYD;
u3_banks->phya = instance->port_base + SSUSB_SIFSLV_V1_U3PHYA;
break;
-- 
1.7.9.5

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

[PATCH next 2/2] phy: phy-mtk-tphy: add set_mode callback

2017-08-22 Thread Chunfeng Yun 
This is used to force PHY with USB OTG function to enter a specific
mode, and override OTG VBUS and ID signals.

Signed-off-by: Chunfeng Yun 
---
 drivers/phy/mediatek/phy-mtk-tphy.c |   39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c 
b/drivers/phy/mediatek/phy-mtk-tphy.c
index a440de9..9549450 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -97,9 +97,11 @@
 
 #define U3P_U2PHYDTM1  0x06C
 #define P2C_RG_UART_EN BIT(16)
+#define P2C_FORCE_IDDIGBIT(9)
 #define P2C_RG_VBUSVALID   BIT(5)
 #define P2C_RG_SESSEND BIT(4)
 #define P2C_RG_AVALID  BIT(2)
+#define P2C_RG_IDDIG   BIT(1)
 
 #define U3P_U3_CHIP_GPIO_CTLD  0x0c
 #define P3C_REG_IP_SW_RST  BIT(31)
@@ -586,6 +588,31 @@ static void u2_phy_instance_exit(struct mtk_tphy *tphy,
}
 }
 
+static void u2_phy_instance_set_mode(struct mtk_tphy *tphy,
+struct mtk_phy_instance *instance,
+enum phy_mode mode)
+{
+   struct u2phy_banks *u2_banks = >u2_banks;
+   u32 tmp;
+
+   tmp = readl(u2_banks->com + U3P_U2PHYDTM1);
+   switch (mode) {
+   case PHY_MODE_USB_DEVICE:
+   tmp |= P2C_FORCE_IDDIG | P2C_RG_IDDIG;
+   break;
+   case PHY_MODE_USB_HOST:
+   tmp |= P2C_FORCE_IDDIG;
+   tmp &= ~P2C_RG_IDDIG;
+   break;
+   case PHY_MODE_USB_OTG:
+   tmp &= ~(P2C_FORCE_IDDIG | P2C_RG_IDDIG);
+   break;
+   default:
+   return;
+   }
+   writel(tmp, u2_banks->com + U3P_U2PHYDTM1);
+}
+
 static void pcie_phy_instance_init(struct mtk_tphy *tphy,
   struct mtk_phy_instance *instance)
 {
@@ -882,6 +909,17 @@ static int mtk_phy_exit(struct phy *phy)
return 0;
 }
 
+static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+   struct mtk_phy_instance *instance = phy_get_drvdata(phy);
+   struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
+
+   if (instance->type == PHY_TYPE_USB2)
+   u2_phy_instance_set_mode(tphy, instance, mode);
+
+   return 0;
+}
+
 static struct phy *mtk_phy_xlate(struct device *dev,
 struct of_phandle_args *args)
 {
@@ -932,6 +970,7 @@ static struct phy *mtk_phy_xlate(struct device *dev,
.exit   = mtk_phy_exit,
.power_on   = mtk_phy_power_on,
.power_off  = mtk_phy_power_off,
+   .set_mode   = mtk_phy_set_mode,
.owner  = THIS_MODULE,
 };
 
-- 
1.7.9.5

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