[PATCH] USB: fix coding style issue

[pierre Kuo]

try to fix some codeing style issue, such as "space prohibited" and "not
initialise statics".

Signed-off-by: pierre Kuo 
---
 drivers/usb/host/ehci-hcd.c   |  6 ++---
 drivers/usb/host/ehci-hub.c   | 22 -
 drivers/usb/host/ehci-mem.c   | 55 ++-
 drivers/usb/host/ehci-pci.c   |  4 ++--
 drivers/usb/host/ehci-sysfs.c |  2 +-
 5 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 6e834b83..cbcac78 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -68,7 +68,7 @@
 #define DRIVER_AUTHOR "David Brownell"
 #define DRIVER_DESC "USB 2.0 'Enhanced' Host Controller (EHCI) Driver"
 
-static const char  hcd_name [] = "ehci_hcd";
+static const char  hcd_name[] = "ehci_hcd";
 
 
 #undef EHCI_URB_TRACE
@@ -88,12 +88,12 @@
 #defineEHCI_TUNE_FLS   1   /* (medium) 512-frame schedule 
*/
 
 /* Initial IRQ latency:  faster than hw default */
-static int log2_irq_thresh = 0;// 0 to 6
+static int log2_irq_thresh;// 0 to 6
 module_param (log2_irq_thresh, int, S_IRUGO);
 MODULE_PARM_DESC (log2_irq_thresh, "log2 IRQ latency, 1-64 microframes");
 
 /* initial park setting:  slower than hw default */
-static unsigned park = 0;
+static unsigned int park;
 module_param (park, uint, S_IRUGO);
 MODULE_PARM_DESC (park, "park setting; 1-3 back-to-back async packets");
 
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index df169c8..e880861 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -268,7 +268,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
fs_idle_delay = false;
port = HCS_N_PORTS(ehci->hcs_params);
while (port--) {
-   u32 __iomem *reg = >regs->port_status [port];
+   u32 __iomem *reg = >regs->port_status[port];
u32 t1 = ehci_readl(ehci, reg) & ~PORT_RWC_BITS;
u32 t2 = t1 & ~PORT_WAKE_BITS;
 
@@ -474,14 +474,14 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
/* manually resume the ports we suspended during bus_suspend() */
i = HCS_N_PORTS (ehci->hcs_params);
while (i--) {
-   temp = ehci_readl(ehci, >regs->port_status [i]);
+   temp = ehci_readl(ehci, >regs->port_status[i]);
temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS);
if (test_bit(i, >bus_suspended) &&
(temp & PORT_SUSPEND)) {
temp |= PORT_RESUME;
set_bit(i, _needed);
}
-   ehci_writel(ehci, temp, >regs->port_status [i]);
+   ehci_writel(ehci, temp, >regs->port_status[i]);
}
 
/*
@@ -498,10 +498,10 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
 
i = HCS_N_PORTS (ehci->hcs_params);
while (i--) {
-   temp = ehci_readl(ehci, >regs->port_status [i]);
+   temp = ehci_readl(ehci, >regs->port_status[i]);
if (test_bit(i, _needed)) {
temp &= ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME);
-   ehci_writel(ehci, temp, >regs->port_status [i]);
+   ehci_writel(ehci, temp, >regs->port_status[i]);
}
}
 
@@ -628,7 +628,7 @@ static int check_reset_complete (
u32 ppcd = ~0;
 
/* init status to no-changes */
-   buf [0] = 0;
+   buf[0] = 0;
ports = HCS_N_PORTS (ehci->hcs_params);
if (ports > 7) {
buf [1] = 0;
@@ -679,9 +679,9 @@ static int check_reset_complete (
|| (ehci->reset_done[i] && time_after_eq(
jiffies, ehci->reset_done[i]))) {
if (i < 7)
-   buf [0] |= 1 << (i + 1);
+   buf[0] |= 1 << (i + 1);
else
-   buf [1] |= 1 << (i - 7);
+   buf[1] |= 1 << (i - 7);
status = STS_PCD;
}
}
@@ -1016,7 +1016,7 @@ int ehci_hub_control(
if (temp & PORT_PEC)
status |= USB_PORT_STAT_C_ENABLE << 16;
 
-   if ((temp & PORT_OCC) && !ignore_oc){
+   if ((temp & PORT_OCC) && !ignore_oc) {
status |= USB_PORT_STAT_C_OVERCURRENT << 16;
 
/*
@@ -1077,7 +1077,7 @@ int ehci_hub_control(
/* whoever resets must GetPortStatus to complete it!! */
} else {
status |= USB_PORT_STAT_C_RESET << 16;
-   ehci->reset_done [wIndex] = 0;
+   ehci->reset_done[wIndex] = 0;
 
/* force reset to complete */
   

[PATCH] usb: quirks: add delay init quirk for Corsair Strafe RGB keyboard

[Kai-Heng Feng]

Corsair Strafe RGB keyboard has trouble to initialize:

[ 1.679455] usb 3-6: new full-speed USB device number 4 using xhci_hcd
[ 6.871136] usb 3-6: unable to read config index 0 descriptor/all
[ 6.871138] usb 3-6: can't read configurations, error -110
[ 6.991019] usb 3-6: new full-speed USB device number 5 using xhci_hcd
[ 12.246642] usb 3-6: unable to read config index 0 descriptor/all
[ 12.246644] usb 3-6: can't read configurations, error -110
[ 12.366555] usb 3-6: new full-speed USB device number 6 using xhci_hcd
[ 17.622145] usb 3-6: unable to read config index 0 descriptor/all
[ 17.622147] usb 3-6: can't read configurations, error -110
[ 17.742093] usb 3-6: new full-speed USB device number 7 using xhci_hcd
[ 22.997715] usb 3-6: unable to read config index 0 descriptor/all
[ 22.997716] usb 3-6: can't read configurations, error -110

Although it may work after several times unpluging/pluging:

[ 68.195240] usb 3-6: new full-speed USB device number 11 using xhci_hcd
[ 68.337459] usb 3-6: New USB device found, idVendor=1b1c, idProduct=1b20
[ 68.337463] usb 3-6: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 68.337466] usb 3-6: Product: Corsair STRAFE RGB Gaming Keyboard
[ 68.337468] usb 3-6: Manufacturer: Corsair
[ 68.337470] usb 3-6: SerialNumber: 0F013021AEB8046755A93ED3F5001941

Tried three quirks: USB_QUIRK_DELAY_INIT, USB_QUIRK_NO_LPM and
USB_QUIRK_DEVICE_QUALIFIER, user confirmed that USB_QUIRK_DELAY_INIT alone
can workaround this issue. Hence add the quirk for Corsair Strafe RGB.

BugLink: https://bugs.launchpad.net/bugs/1678477
Signed-off-by: Kai-Heng Feng 
---
 drivers/usb/core/quirks.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 574da2b4529c..1ea5060dae69 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -217,6 +217,9 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x1a0a, 0x0200), .driver_info =
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
 
+   /* Corsair Strafe RGB */
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+
/* Acer C120 LED Projector */
{ USB_DEVICE(0x1de1, 0xc102), .driver_info = USB_QUIRK_NO_LPM },
 
-- 
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 v3 5/5] usb: xhci: Handle USB transaction error on address command

[Lu Baolu]

Hi,

On 08/15/2017 07:30 PM, Mathias Nyman wrote:
> On 11.08.2017 05:41, Lu Baolu wrote:
>> Xhci driver handles USB transaction errors on transfer events,
>> but transaction errors are possible on address device command
>> completion events as well.
>>
>> The xHCI specification (section 4.6.5) says: A USB Transaction
>> Error Completion Code for an Address Device Command may be due
>> to a Stall response from a device. Software should issue a Disable
>> Slot Command for the Device Slot then an Enable Slot Command to
>> recover from this error.
>>
>> This patch handles USB transaction errors on address command
>> completion events. The related discussion threads can be found
>> through below links.
>>
>> http://marc.info/?l=linux-usb=149362010728921=2
>> http://marc.info/?l=linux-usb=149252752825755=2
>>
>> Suggested-by: Mathias Nyman 
>> Signed-off-by: Lu Baolu 
>> ---
>>   drivers/usb/host/xhci.c | 5 +
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index d6b728d..95780f8 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, 
>> struct usb_device *udev,
>>   break;
>>   case COMP_USB_TRANSACTION_ERROR:
>>   dev_warn(>dev, "Device not responding to setup %s.\n", act);
>> +
>> +ret = xhci_disable_slot(xhci, udev->slot_id);
>> +if (!ret)
>> +xhci_alloc_dev(hcd, udev);
>
> Might be a xhci->mutex locking issue here,
> both xhci_setup_device() and xhci_alloc_dev() take xhci->mutex
>

I will apply xhci->mutex in this patch for code consistency, but I think
we can drop xhci->mutex (in a separated patch) anyway.

xhci->mutex was introduced to protect two race sources of xhci->slot_id
and xhci->addr_dev by below commit:

commit a00918d0521df1c7a2ec9143142a3ea998c8526d
Author: Chris Bainbridge 
Date:   Tue May 19 16:30:51 2015 +0300

usb: host: xhci: add mutex for non-thread-safe data
   
Regression in commit 638139eb95d2 ("usb: hub: allow to process more usb
hub events in parallel")
   
The regression resulted in intermittent failure to initialise a 10-port
hub (with three internal VL812 4-port hub controllers) on boot, with a
failure rate of around 8%, due to multiple race conditions when
accessing addr_dev and slot_id in struct xhci_hcd.
   
This regression also exposed a problem with xhci_setup_device, which
"should be protected by the usb_address0_mutex" but no longer is due to
   
commit 6fecd4f2a58c ("USB: separate usb_address0 mutexes for each bus")
   
With separate buses (and locks) it is no longer the case that a single
lock will protect xhci_setup_device from accesses by two parallel
threads processing events on the two buses.
   
Fix this by adding a mutex to protect addr_dev and slot_id in struct
xhci_hcd, and by making the assignment of slot_id atomic.

[--cut---]

We have already removed these two race sources after that by below
two commits:

c2d3d49 usb: xhci: move slot_id from xhci_hcd to xhci_command structure
87e44f2 usb: xhci: remove the use of xhci->addr_dev

So we don't need xhci->mutex any more. I will try to do this in a separated
patch with more tests. For now, I will add xhci->mutex for code consistency.

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

Re: [PATCH v2 0/5] usb: Replace the deprecated extcon API

[Chanwoo Choi]

Dear Kishon and Felipe,

I applied these patches on extcon-next branch for v4.14-rc1.

And I created the immutable branch ('ib-extcon-usb-phy-4.14')
and send the pull-request for these patches in order to
prevent the merge conflict.

Best Regards,
Chanwoo Choi


The following changes since commit 5771a8c08880cdca3bfb4a3fc6d309d6bba20877:

  Linux v4.13-rc1 (2017-07-15 15:22:10 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git 
ib-extcon-usb-phy-4.14

for you to fetch changes up to 808ae8f3c7fefef3aece08820c108b68cdb06e1e:

  extcon: Remove deprecated extcon_set/get_cable_state_() (2017-08-16 09:21:49 
+0900)


Chanwoo Choi (5):
  phy: qcom-usb-hs: Replace the extcon API
  phy: rockchip-inno-usb2: Replace the extcon API
  phy: phy-bcm-ns2-usbdrd: Replace the deprecated extcon API
  usb: gadget: udc: Replace the deprecated extcon API
  extcon: Remove deprecated extcon_set/get_cable_state_()

 drivers/phy/broadcom/phy-bcm-ns2-usbdrd.c |  8 
 drivers/phy/qualcomm/phy-qcom-usb-hs.c| 14 +++---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 10 +-
 drivers/usb/gadget/udc/snps_udc_plat.c|  6 +++---
 include/linux/extcon.h| 11 ---
 5 files changed, 15 insertions(+), 34 deletions(-)


On 2017년 08월 03일 17:20, Chanwoo Choi wrote:
> These patches replace the deprecated extcon API and remove them from extcon.
> 
> Patch4 (drivers/usb/gadget/udc/snps_udc_plat.c) neeeds the review
> from usb maintainer. After finishing the review of patch4,
> I'll create the immutable branch and send the pull request
> to both usb and phy maintainer.
> 
> Changes from v1:
> - Fix capital error for 'acked-by' tag on patch2
> - Add the acked-by tag of 'Kishon Vijay Abraham I' to patch3
> 
> Chanwoo Choi (5):
>   phy: qcom-usb-hs: Replace the extcon API
>   phy: rockchip-inno-usb2: Replace the extcon API
>   phy: phy-bcm-ns2-usbdrd: Replace the deprecated extcon API
>   usb: gadget: udc: Replace the deprecated extcon API
>   extcon: Remove deprecated extcon_set/get_cable_state_()
> 
>  drivers/phy/broadcom/phy-bcm-ns2-usbdrd.c |  8 
>  drivers/phy/qualcomm/phy-qcom-usb-hs.c| 14 +++---
>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 10 +-
>  drivers/usb/gadget/udc/snps_udc_plat.c|  6 +++---
>  include/linux/extcon.h| 11 ---
>  5 files changed, 15 insertions(+), 34 deletions(-)
> 

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

Re: [PATCH v2 4/5] usb: gadget: udc: Replace the deprecated extcon API

[Chanwoo Choi]

Hi,

On 2017년 08월 15일 18:48, Felipe Balbi wrote:
> 
> Hi,
> 
> Chanwoo Choi  writes:
>> This patch replaces the deprecated extcon API as following:
>> - extcon_get_cable_state_() -> extcon_get_state()
>>
>> Cc: Felipe Balbi 
>> Cc: Greg Kroah-Hartman 
>> Cc: Raviteja Garimella 
>> Signed-off-by: Chanwoo Choi 
> 
> Do you want to take these through your tree or mine? In case you want
> them in your tree:
> 
> Acked-by: Felipe Balbi 
> 

Thanks for review. These patches included the patch related to extcon (patch5).
So, After creating the immutable branch, I'll send the pull request to both
phy and usb maintainers.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe 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: Sometimes supports_usb_power_delivery reports incorrect value.

[Badhri Jagan Sridharan]

Submitted couple of patches for the missing pieces in TCPM.
Those patches along with "usb: typec: update partner power delivery
support with opmode"
seems to address the issue of reporting the right value for
supports_usb_power_delivery.

Thanks,
Badhri

On Tue, Aug 15, 2017 at 7:13 AM, Badhri Jagan Sridharan
 wrote:
> On Tue, Aug 15, 2017 at 6:36 AM, Heikki Krogerus
>  wrote:
>> Hi,
>>
>> On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
>>> Hi Heikki,
>>>
>>> While testing with different type-c phones available in the market,
>>> With some phones, I noticed that supports_usb_power_delivery
>>> reports "no" eventhough an explicit pd contract has been
>>> established. After spending sometime debugging, I noticed that
>>> the root cause of this is that the partner device(acting as source)
>>> takes too long to send the SRC_CAP message. This makes the
>>> underlying TCPM code to report usb_pd set to 0 while initially
>>> calling typec_register_partner. However,since  there is no
>>> provision in the type-c sysfs interface to update
>>> supports_usb_power_delivery once the contract is established,
>>> supports_usb_power_delivery is left to report "no" even if the partner
>>> source device is at present performing Type-C PD.
>>> Is it OK to add a api to enable updating supports_usb_power_delivery
>>> node in the typec sysfs code after typec_register_partner has been
>>> called ? Or do you have other suggestions ? Please advice.
>>
>> supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
>> is called with value TYPEC_PWR_MODE_PD, and it should be called, also
>
> Oops my bad !! I somehow did not notice the presence of your following
> commit:
>
> usb: typec: update partner power delivery support with opmode
>
> which has not been picked-up in our codebase yet.
>
>> in tcpm.c, always when USB PD contract has been established. I did not
>> check the latest tcpm.c code, but I assume it does that. If it
>
> TCPM does do this.
>
>> doesn't, it needs to be fixed of course.
>>
>> Are you sure you really have the contract established?
>
> Yes I did verify using the pd-analyzer. I will your change and see how it 
> goes.
> Thanks !
>
>>
>>
>> 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

Inconsistency in usb_add_gadget_udc_release() interface

[Alexey Khoroshilov]

Hello,

usb_add_gadget_udc_release() gets release() argument that allows to
release user resources.

As far as I can see, the release() is called on error paths 
of usb_add_gadget_udc_release() as a result of
put_device(>dev);
except for the only path going via err1.

As a result a caller of the usb_add_gadget_udc_release() have no chance
to know if the release() was invoked or not.

It may lead to memory leaks (drivers/usb/gadget/udc/snps_udc_core.c)
or to double free (drivers/usb/gadget/udc/fsl_udc_core.c).

Is my reading correct? If so, should we always call release() on error paths?

--
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org

--
To unsubscribe from this list: send the line "unsubscribe 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: uvc-gadget for UVC testing doesn't seem to work with vivid

[Rail Shafigulin]

On Tue, Aug 15, 2017 at 2:43 AM, Felipe Balbi
 wrote:
>
> Hi,
>
> Rail Shafigulin  writes:
>> Let me apologize for emailing directly to the list as I'm not one of
>> the developers and just starting out with USB and UVC. After searching
>
> the list is open to anybody and we welcome newcomers :-)
>
>> online for about a week I just couldn't find answers and I hope the
>> original authors of the uvc-gadget tool are on the list and can help
>> out.
>>
>> Needed to test a UVC in a custom built kernel (Xilinx petalinux), .
>> Followed these directions,
>> https://github.com/torvalds/linux/blob/ef954844c7ace62f773f4f23e28d2d915adc419f/Documentation/usb/gadget-testing.txt#L717-L730.
>>
>> Patches didn't work. Had to look around for correct ones. Found them
>> here, 
>> http://markmail.org/message/hb7evzvigbuxptz5#query:+page:1+mid:s73fdeffjgb2v2yw+state:results.
>>
>> Combined and applied the patches into a repo here,
>> https://github.com/cyboflash/uvc-gadget.git.
>>
>> When I ran a test command, given in the instructions above,
>> uvc-gadget -u /dev/video -v /dev/video
>>
>> got the following error:
>> V4L2_CORE: (jpeg decoder) error while decoding frame
>>
>> and a black screen.
>>
>> One thing to note is that I was not using luvcview, but guvcview.
>>
>> It looks like the error is coming from here,
>> https://sourceforge.net/p/guvcview/git-master/ci/master/tree/gview_v4l2core/jpeg_decoder.c#l1503
>>
>> My thoughts
>> 1. I don't think the error is coming from v4l2. I tested it on another
>> machine and it worked. But I'm not an expert so I can't say for sure.
>> 2. I don't think the error is coming from UVC. I think since I see a
>> black screen, it is working. Again, I'm not an expert, so I can't say
>> for sure.
>> 3. I think the error is due to uvc-gadget test application. It could
>> be that the applied patches are outdated, but I just didn't find
>> anything else online. But, I'm not an expert so definitely can't say
>> for sure.
>>
>> I would greatly appreciate any help with this as I'm just starting out
>> with UVC and USB.
>
> Which kernel are you using? Which UDC driver are you using?

Balbi,

The board is configured as a USB Camera Gadget. Here is the output of uname -a
Linux Xilinx-ZCU102-2016_3 4.6.0 #33 SMP Thu Aug 10 11:47:57 PDT 2017
aarch64 GNU/Linux

When you say UDC (USB Device Controller) driver, what exactly do you mean?

Here is what I do on my board:

modprobe g_webcam
modprobe vivid
uvc-gadget -u /dev/video0 -v /dev/video1

Where uvc-gadget is precompiled app from here,
https://github.com/cyboflash/uvc-gadget.git

On my host (Linux ubuntu 4.10.0-32-generic #36~16.04.1-Ubuntu SMP Wed
Aug 9 09:19:02 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux) I simply
execute the following in my terminal

guvcview

and then later choose the source for my input.

At this point I'm starting to see V4L2_CORE: (jpeg decoder) error
while decoding frame.

I've tested guvcview with my built-in camera and didn't see any issues.

Any help is appreciated.

>
> --
> balbi

-- 




*ESENCIA TECHNOLOGIES, INC.*3945 Freedom Circle, Suite #360,
Santa Clara CA 95054


Phone: +1 408 736 8284 Fax: +1 408 519 3475 
http://www.esenciatech.com | http://www.lnttechservices.com


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

Re: Possible null pointer dereference in adutux.ko

[Oliver Neukum]

Am Dienstag, den 15.08.2017, 16:38 +0300 schrieb Anton Volkov:
> On 15.08.2017 16:20, Oliver Neukum wrote:
> > 
> > Am Dienstag, den 15.08.2017, 15:59 +0300 schrieb Anton Volkov:
> > > 
> > > Hello.
> > > 
> > > While searching for races in the Linux kernel I've come across
> > > "drivers/usb/misc/adutux.ko" module. Here is a question 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:
> > > adu_release
> > > ->adu_release_internal  adu_disconnect
> > >   udev->dev>dev->udev = NULL
> > >   (adutux.c: line 298)  (adutux.c: line 771)
> > > usb_deregister_dev
> > > 
> > > Comments in the source code point at the possibility of adu_release()
> > > being called separately from adu_disconnect(). adu_release() and
> > > adu_disconnect() acquire different mutexes, so they are not protected
> > > from one another. If adu_disconnect() changes dev->udev before its value
> > > is read in adu_release_internal() there will be a NULL pointer
> > > dereference on a read attempt. Is this case feasible from your point of
> > > view?
> > > 
> > > Thank you for your time.
> > 
> > Hi,
> > 
> > your analysis seems correct to me. In fact it looks like
> > 
> > 66d4bc30d128e7c7ac4cf64aa78cb76e971cec5b
> > USB: adutux: remove custom debug macro
> > 
> > more or less broke disconnect on this driver
> > (the URBs can also finish after dev->udev = NULL)
> > 
> > Do you want to do a fix or do you want me to do it?
> > 
> > Regards
> > Oliver
> > 
> 
> Hello, Oliver.
> 
> I am not sure about the best way to solve this problem. If you have any 
> ideas about it then it would probably be better if you could handle the 
> fix. Or if you share the ideas I can prepare a patch.

Hi,

given the age of the drivers I would suggest to simply remove the debugging 
statements

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

Re: [PATCH v5 1/8] usb: gadget: f_ecm/f_eem/f_rndis: Setup quirk_avoids_skb_reserve

[Dmitry Osipenko]

Hi,

On 15.08.2017 12:49, Felipe Balbi wrote:
> Dmitry Osipenko  writes:
>> This quirk is required to make USB Ethernet gadget working with HW that
>> can't cope with unaligned DMA. For some reason only f_ncm handles that
>> quirk, let's handle it in the rest of the network models. All models have
>> been tested with a ChipIdea UDC driver on NVIDIA Tegra20 SoC that require
>> DMA to be aligned.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/usb/gadget/function/f_ecm.c   | 7 +++
>>  drivers/usb/gadget/function/f_eem.c   | 5 +
>>  drivers/usb/gadget/function/f_rndis.c | 4 
>>  3 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_ecm.c 
>> b/drivers/usb/gadget/function/f_ecm.c
>> index 4c488d15b6f6..1d198055fd74 100644
>> --- a/drivers/usb/gadget/function/f_ecm.c
>> +++ b/drivers/usb/gadget/function/f_ecm.c
>> @@ -584,6 +584,13 @@ static int ecm_set_alt(struct usb_function *f, unsigned 
>> intf, unsigned alt)
>>   */
>>  ecm->port.is_zlp_ok =
>>  gadget_is_zlp_supported(cdev->gadget);
>> +
>> +/* Setup DMA alignment workaround for UDC's that
>> + * need it.
>> + */
>> +ecm->port.no_skb_reserve =
>> +gadget_avoids_skb_reserve(cdev->gadget);
> 
> looks like the quirk should be moved to u_ether.c instead.
> 

Indeed, thank you very much for the suggestion. I'll prepare a new revision of
the patch.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe 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: Sometimes supports_usb_power_delivery reports incorrect value.

[Badhri Jagan Sridharan]

On Tue, Aug 15, 2017 at 6:36 AM, Heikki Krogerus
 wrote:
> Hi,
>
> On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
>> Hi Heikki,
>>
>> While testing with different type-c phones available in the market,
>> With some phones, I noticed that supports_usb_power_delivery
>> reports "no" eventhough an explicit pd contract has been
>> established. After spending sometime debugging, I noticed that
>> the root cause of this is that the partner device(acting as source)
>> takes too long to send the SRC_CAP message. This makes the
>> underlying TCPM code to report usb_pd set to 0 while initially
>> calling typec_register_partner. However,since  there is no
>> provision in the type-c sysfs interface to update
>> supports_usb_power_delivery once the contract is established,
>> supports_usb_power_delivery is left to report "no" even if the partner
>> source device is at present performing Type-C PD.
>> Is it OK to add a api to enable updating supports_usb_power_delivery
>> node in the typec sysfs code after typec_register_partner has been
>> called ? Or do you have other suggestions ? Please advice.
>
> supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
> is called with value TYPEC_PWR_MODE_PD, and it should be called, also

Oops my bad !! I somehow did not notice the presence of your following
commit:

usb: typec: update partner power delivery support with opmode

which has not been picked-up in our codebase yet.

> in tcpm.c, always when USB PD contract has been established. I did not
> check the latest tcpm.c code, but I assume it does that. If it

TCPM does do this.

> doesn't, it needs to be fixed of course.
>
> Are you sure you really have the contract established?

Yes I did verify using the pd-analyzer. I will your change and see how it goes.
Thanks !

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

Re: This is probably crazy

[Alan Stern]

On Tue, 15 Aug 2017, Steven Timms wrote:

> Hi Alan, thank you for looking in to this. I am not a programmer but I am
> technically capable. Although I have not done debugging before I am happy
> to provide the data. Is it as simple as sending audio data to the DAC and
> then running a program from the terminal? If so it should be simple for me
> if you don't mind giving me some steps to follow.

Here are some things you can easily do right away:

Mount a debugfs filesystem:

mount -t debugfs none /sys/kernel/debug

Then while sending data to the DAC (something that would cause an
unpatched kernel to fail), make a copy of /sys/kernel/debug/usb/devices
and post that copy.

For a second test, enable debugging for ehci-hcd:

echo 'module ehci_hcd =p' >/sys/kernel/debug/dynamic_debug/control

Then clear the system log buffer:

dmesg -C

The run the test program briefly (a few seconds of data to the DAC will 
be enough).  When that's done, make a copy of the output from "dmesg" 
and post that.

> Thanks, I am gobsmacked that emailing Linus actually worked, I've never
> done this before :) you guys are awesome

Linus tends to be pretty busy.  It's generally better to email the 
developers responsible for the particular devices or subsystems where 
the problems occur.

Alan Stern

> On 15 Aug. 2017 6:49 am, "Alan Stern"  wrote:
> 
> On Mon, 14 Aug 2017, Linus Torvalds wrote:
> 
> > Greg, Alan,
> >
> >  You guys are presumably aware of this, but I got this email about the
> > EHCI iso strem scheduler problem with some audio DAC's.
> 
> Actually no, I wasn't aware of it.
> 
> > This kernel code all goes back to 2013 or earlier, so I dunno, but
> > there's a patch in there:
> >
> > https://github.com/comps/ehci-mdac/blob/master/patches/
> Ubuntu-3.11.0-17.30.patch
> >
> > that looks like it obviously fixes some problem for somebody.
> >
> > The reason I'm forwarding this is that the patch actually looks better
> > than the current code, so maybe it really is valid, even if the fact
> > that this is all old code makes me wonder.
> >
> > Comments?
> 
> The difference in appearance notwithstanding, the patch is really the
> same as the existing code except for the order of the search (the patch
> searches forward and the existing code searches backward).
> 
> Whether the results are superior depends very much on the particular
> load running on the system.  Different DACs will have different
> requirements.  If Steven is willing to do some debugging, I can try to
> figure out what is going wrong on his system.
> 
> The difficulty, of course, is that the existing code _fixes_ a number
> of systems.  See commit 811c926c538f ("USB: EHCI: fix HUB TT scheduling
> issue with iso transfer").  sitd scheduling is ridiculously
> complicated, and the way we do it will not always work.
> 
> Alan Stern
> 
> >  Linus
> >
> > On Mon, Aug 14, 2017 at 4:34 AM, Steven Timms 
> wrote:
> > > Hi Linus,
> > >
> > > I am a big fan and I hope you don't mind an email question sometimes.
> Since
> > > 2014 there has been a change to the ehci-hcd driver, namely 'split
> > > transaction schedualing'. This has broken many USB audio DACs. I am not
> a
> > > programmer, just a patient 'user', I was hoping something might change
> over
> > > the years but still some DACs don't work properly.
> > > There was a github page dedicated to the issue:
> > > https://github.com/comps/ehci-mdac
> > > But hasn't ended up in the linux kernel. Perhaps the DACs are the
> problem
> > > and not the kernel driver. I don't know. Anyway I just wanted to bring
> it to
> > > your attention.
> > >
> > >
> > > Thank you so much,
> > >
> > > Steven
> 

--
To unsubscribe from this list: send the line "unsubscribe 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: dummy: fix infinite loop because of missing loop decrement

[Alan Stern]

On Tue, 15 Aug 2017, Colin King wrote:

> From: Colin Ian King 
> 
> The while loop never terminates because the loop counter i is never
> decremented. Fix this by decrementing i.
> 
> Detected by CoverityScan, CID#751073 ("Infinite Loop")
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c 
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 3c3760315910..a030d7923d7d 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2776,7 +2776,7 @@ static int __init init(void)
>   if (retval < 0) {
>   i--;
>   while (i >= 0)
> - platform_device_del(the_udc_pdev[i]);
> + platform_device_del(the_udc_pdev[i--]);
>   goto err_add_udc;
>   }
>   }

Acked-by: Alan Stern 

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

Re: Sometimes supports_usb_power_delivery reports incorrect value.

[Heikki Krogerus]

Hi,

On Mon, Aug 14, 2017 at 11:57:15AM -0700, Badhri Jagan Sridharan wrote:
> Hi Heikki,
> 
> While testing with different type-c phones available in the market,
> With some phones, I noticed that supports_usb_power_delivery
> reports "no" eventhough an explicit pd contract has been
> established. After spending sometime debugging, I noticed that
> the root cause of this is that the partner device(acting as source)
> takes too long to send the SRC_CAP message. This makes the
> underlying TCPM code to report usb_pd set to 0 while initially
> calling typec_register_partner. However,since  there is no
> provision in the type-c sysfs interface to update
> supports_usb_power_delivery once the contract is established,
> supports_usb_power_delivery is left to report "no" even if the partner
> source device is at present performing Type-C PD.
> Is it OK to add a api to enable updating supports_usb_power_delivery
> node in the typec sysfs code after typec_register_partner has been
> called ? Or do you have other suggestions ? Please advice.

supports_usb_power_delivery will be updated if typec_set_pwr_opmode()
is called with value TYPEC_PWR_MODE_PD, and it should be called, also
in tcpm.c, always when USB PD contract has been established. I did not
check the latest tcpm.c code, but I assume it does that. If it
doesn't, it needs to be fixed of course.

Are you sure you really have the contract established?


Thanks,

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

Re: Possible null pointer dereference in adutux.ko

[Anton Volkov]

On 15.08.2017 16:20, Oliver Neukum wrote:

Am Dienstag, den 15.08.2017, 15:59 +0300 schrieb Anton Volkov:

Hello.

While searching for races in the Linux kernel I've come across
"drivers/usb/misc/adutux.ko" module. Here is a question 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:
adu_release
->adu_release_internal  adu_disconnect
  udev->dev>dev->udev = NULL
  (adutux.c: line 298)  (adutux.c: line 771)
usb_deregister_dev

Comments in the source code point at the possibility of adu_release()
being called separately from adu_disconnect(). adu_release() and
adu_disconnect() acquire different mutexes, so they are not protected
from one another. If adu_disconnect() changes dev->udev before its value
is read in adu_release_internal() there will be a NULL pointer
dereference on a read attempt. Is this case feasible from your point of
view?

Thank you for your time.


Hi,

your analysis seems correct to me. In fact it looks like

66d4bc30d128e7c7ac4cf64aa78cb76e971cec5b
USB: adutux: remove custom debug macro

more or less broke disconnect on this driver
(the URBs can also finish after dev->udev = NULL)

Do you want to do a fix or do you want me to do it?

Regards
Oliver



Hello, Oliver.

I am not sure about the best way to solve this problem. If you have any 
ideas about it then it would probably be better if you could handle the 
fix. Or if you share the ideas I can prepare a patch.


Regards,
Anton

-- 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: [PATCH] arm64: dts: hi6220: improve g-tx-fifo-size setting for usb device

[Wei Xu]

Hi Shawn, John,

On 2017/8/7 22:18, John Stultz wrote:
> On Sun, Aug 6, 2017 at 10:01 PM, Shawn Guo  wrote:
>> From: Shawn Guo 
>>
>> The current usb device g-tx-fifo-size setting in DT causes two problems
>> for kernel driver.
>>
>> 1. On hi6220, there are 15 tx_fifo dedicated for all EPs except EP0,
>>while DT only provides tx_fifo settings for 6 EPs.  It results in the
>>following annoying complaints from kernel.
>>
>> [4.451623] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[7]=0
>> [4.461303] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[8]=0
>> [4.470969] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[9]=0
>> [4.480632] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[10]=0
>> [4.490385] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[11]=0
>> [4.500140] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[12]=0
>> [4.509892] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[13]=0
>> [4.519646] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[14]=0
>> [4.529399] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[15]=0
>> [4.539244] dwc2 f72c.usb: EPs: 16, dedicated fifos, 1920 entries in 
>> SPRAM
>>
>>Besides of that, the total 1920 fifo entries isn't fully utilized.
>>Endpoint Info Control block consumes 128 entries, g-rx-fifo-size
>>is 512, and g-np-tx-fifo-size is 128.  So the fifi entries available
>>for tx_fifo is: 1920 - 128 - 512 - 128 = 1152.  Considering that
>>the minimal valid tx_fifo size for each EP is 16, it should be
>>reasonable to allocate 1152 entries as: 128 x 8 + 16 x 7 = 1136 (only
>>16 entries unused).  With this new setting, we can get more EPs to
>>use while removing the above warning messages in the meantime.
>>
>> 2. Another consequence of above invalid g_tx_fifo_size parameter is that
>>kernel driver will use values read from hardware register as the
>>fall-back.  The value is 2048 for each EP fifo.  That's obviously
>>invalid either, because even fifo entries for one EP exceeds the
>>total entries 1920.  That's why we see the following fat warning from
>>function dwc2_hsotg_init_fifo().  The new g-tx-fifo-size settings
>>help to remove the warning as well.
> 
> 
> Nice! This has been bothering me for awhile, and your fix seems to
> resolve it well! I can now remove one of the hack patches I've been
> carrying.
> 
> Tested-by: John Stultz 
> (if its useful: Acked-by: John Stultz )
> 
> Wei: Can we be sure to get this queued for 4.14?

Thanks!
Applied to the hisilicon dt tree.

Best Regards,
Wei

> 
> thanks
> -john
> 
> .
> 

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

Re: Possible null pointer dereference in adutux.ko

[Oliver Neukum]

Am Dienstag, den 15.08.2017, 15:59 +0300 schrieb Anton Volkov:
> Hello.
> 
> While searching for races in the Linux kernel I've come across 
> "drivers/usb/misc/adutux.ko" module. Here is a question 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:
> adu_release
> ->adu_release_internal  adu_disconnect
>  udev->dev>dev->udev = NULL
>  (adutux.c: line 298)  (adutux.c: line 771)
>usb_deregister_dev
> 
> Comments in the source code point at the possibility of adu_release() 
> being called separately from adu_disconnect(). adu_release() and 
> adu_disconnect() acquire different mutexes, so they are not protected 
> from one another. If adu_disconnect() changes dev->udev before its value 
> is read in adu_release_internal() there will be a NULL pointer 
> dereference on a read attempt. Is this case feasible from your point of 
> view?
> 
> Thank you for your time.

Hi,

your analysis seems correct to me. In fact it looks like

66d4bc30d128e7c7ac4cf64aa78cb76e971cec5b
USB: adutux: remove custom debug macro

more or less broke disconnect on this driver
(the URBs can also finish after dev->udev = NULL)

Do you want to do a fix or do you want me to do it?

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

Possible null pointer dereference in adutux.ko

[Anton Volkov]

Hello.

While searching for races in the Linux kernel I've come across 
"drivers/usb/misc/adutux.ko" module. Here is a question 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:
adu_release
->adu_release_internal  adu_disconnect
udev->dev>dev->udev = NULL
(adutux.c: line 298)  (adutux.c: line 771)
  usb_deregister_dev

Comments in the source code point at the possibility of adu_release() 
being called separately from adu_disconnect(). adu_release() and 
adu_disconnect() acquire different mutexes, so they are not protected 
from one another. If adu_disconnect() changes dev->udev before its value 
is read in adu_release_internal() there will be a NULL pointer 
dereference on a read attempt. Is this case feasible from your point of 
view?


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: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

On 2017-08-15 14:02, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:

thanks for reviewing.


np :-)


On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:
udc_stop needs to be called before gadget driver unbind. Otherwise 
it
might happen that udc drivers still call into the gadget driver 
(e.g.
to reset gadget after OTG event). If this happens it is likely to 
get
panics from gadget driver dereferencing NULL ptr, as gadget's 
drvdata

is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.


I mentioned this just as example, it can happen whenever a UDC driver
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) 
after

gadget
drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by
unbind()
and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the
gadget
driver is getting called after unbind.


We have a known problem in the design of the gadget API that causes 
this

races but we couldn't come up with a solution yet :-)

Inverting these two calls is not the correct way to go about this :-)


Now I see, thanks for explanation below.


Signed-off-by: Danilo Krummrich 
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as
exsample)

CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in
udc_stop
and check for it before calling into the gadget driver. To fix the
issue
above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can 
provide

further patches. This patch could also just serve as a base for
discussion
if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a
quick
workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c
b/drivers/usb/gadget/udc/core.c
index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct
usb_udc *udc)

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+   /* udc_stop needs to be called before gadget driver unbind to
prevent
+* udc driver calls into gadget driver after unbind which could
cause
+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests
to
gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget 
driver

anymore? If not, I did not got your point, sorry for that. Can you
please
help me out? Would the changed order raise another issue I'm not aware
of?


right, ->udc_stop() is supposed to completely teardown the USB
controller, including disabling interrupts and all. The only thing it
_can_ do from ->udc_stop() would be giving back any pending requests
that were left (which would cause req->complete() to be called with an
error status). But even that is unlikely in the case you mention since
->unbind() was already called.


Ok, got it. That's why req->context = cdev, to overcome being unbound
already. Thanks for clarification.


If I understood you correctly, without this patch udc driver can not
call
the gadget driver back as well, because this would result in a NULL 
ptr

dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still 
happen,
regardless of the order of udc_stop and unbind. But with this patch 
the

needed locking could easily done within the udc driver only. Without,
the
lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc 
driver
trying to call into the gadget driver could do this after gadget 
driver

already unbound.


right

Is someone working on this issue, already?
If not, I would like to offer introducing the needed locking to overcome
this issue.
If you are about to refactor the whole 

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Felipe Balbi]

Hi,

Danilo Krummrich  writes:
> thanks for reviewing.

np :-)

> On 2017-08-15 12:03, Felipe Balbi wrote:
>> Hi,
>> 
>> Danilo Krummrich  writes:
>>> udc_stop needs to be called before gadget driver unbind. Otherwise it
>>> might happen that udc drivers still call into the gadget driver (e.g.
>>> to reset gadget after OTG event). If this happens it is likely to get
>>> panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
>>> is set to NULL on unbind.
>> 
>> seems like the problem here is with the OTG layer, not UDC core.
>> 
> I mentioned this just as example, it can happen whenever a UDC driver 
> calls
> the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) after 
> gadget
> drivers unbind() was called already (e.g. by gadget configfs).
> If this happens gadget drivers drvdata was already set to NULL by 
> unbind()
> and reset() could result into a NULL ptr exception.
> Therefore my assumption was that it needs to be prevented that the 
> gadget
> driver is getting called after unbind.

We have a known problem in the design of the gadget API that causes this
races but we couldn't come up with a solution yet :-)

Inverting these two calls is not the correct way to go about this :-)

>>> Signed-off-by: Danilo Krummrich 
>>> ---
>>> Actually there could still be a race:
>>> (CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as 
>>> exsample)
>>> 
>>> CPU0CPU1
>>> 
>>> usb_gadget_disconnect(udc->gadget);
>>> udc->driver->disconnect(udc->gadget);
>>> if (dwc->gadget_driver && 
>>> dwc->gadget_driver->disconnect)
>>> usb_gadget_udc_stop(udc);
>>> udc->driver->unbind(udc->gadget);
>>> 
>>> dwc->gadget_driver->disconnect(>gadget);
>>> 
>>> UDC drivers typically set their gadget driver pointer to NULL in 
>>> udc_stop
>>> and check for it before calling into the gadget driver. To fix the 
>>> issue
>>> above every udc driver could apply a lock around this.
>>> 
>>> If you see the need for having this or another solutions I can provide
>>> further patches. This patch could also just serve as a base for 
>>> discussion
>>> if someone knows a smarter solution.
>>> 
>>> I saw this problem causing a panic on hikey960 board and provided a 
>>> quick
>>> workaround for the same problem here:
>>> https://android-review.googlesource.com/#/c/kernel/common/+/457476/
>>> (panic log in the commit message of the linked patch)
>>> ---
>>>  drivers/usb/gadget/udc/core.c | 6 +-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/usb/gadget/udc/core.c 
>>> b/drivers/usb/gadget/udc/core.c
>>> index efce68e9a8e0..8155468afc0d 100644
>>> --- a/drivers/usb/gadget/udc/core.c
>>> +++ b/drivers/usb/gadget/udc/core.c
>>> @@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct 
>>> usb_udc *udc)
>>> 
>>> usb_gadget_disconnect(udc->gadget);
>>> udc->driver->disconnect(udc->gadget);
>>> -   udc->driver->unbind(udc->gadget);
>>> +   /* udc_stop needs to be called before gadget driver unbind to 
>>> prevent
>>> +* udc driver calls into gadget driver after unbind which could 
>>> cause
>>> +* a nullptr exception.
>>> +*/
>>> usb_gadget_udc_stop(udc);
>>> +   udc->driver->unbind(udc->gadget);
>> 
>> This patch is incorrect, it will prevent us from giving back requests 
>> to
>> gadget driver properly. ->unbind() has to happen before ->udc_stop().
>
> Do you mean after udc_stop the udc driver can not call the gadget driver
> anymore? If not, I did not got your point, sorry for that. Can you 
> please
> help me out? Would the changed order raise another issue I'm not aware 
> of?

right, ->udc_stop() is supposed to completely teardown the USB
controller, including disabling interrupts and all. The only thing it
_can_ do from ->udc_stop() would be giving back any pending requests
that were left (which would cause req->complete() to be called with an
error status). But even that is unlikely in the case you mention since
->unbind() was already called.

> If I understood you correctly, without this patch udc driver can not 
> call
> the gadget driver back as well, because this would result in a NULL ptr
> dereference, as unbind() sets drvdata to NULL.
>
> In any case the race described in my original message can still happen,
> regardless of the order of udc_stop and unbind. But with this patch the
> needed locking could easily done within the udc driver only. Without, 
> the
> lock needs to be acquired before udc->driver->unbind(udc->gadget) and
> released after usb_gadget_udc_stop(). Otherwise an ISR of the udc driver
> trying to call into the gadget driver could do this after gadget driver
> already unbound.

right

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: udc: udc_stop before gadget unbind

[Danilo Krummrich]

Hi,

thanks for reviewing.

On 2017-08-15 12:03, Felipe Balbi wrote:

Hi,

Danilo Krummrich  writes:

udc_stop needs to be called before gadget driver unbind. Otherwise it
might happen that udc drivers still call into the gadget driver (e.g.
to reset gadget after OTG event). If this happens it is likely to get
panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
is set to NULL on unbind.


seems like the problem here is with the OTG layer, not UDC core.

I mentioned this just as example, it can happen whenever a UDC driver 
calls
the gadget driver (e.g. by calling usb_gadget_udc_reset() in ISR) after 
gadget

drivers unbind() was called already (e.g. by gadget configfs).
If this happens gadget drivers drvdata was already set to NULL by 
unbind()

and reset() could result into a NULL ptr exception.
Therefore my assumption was that it needs to be prevented that the 
gadget

driver is getting called after unbind.

Signed-off-by: Danilo Krummrich 
---
Actually there could still be a race:
(CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as 
exsample)


CPU0CPU1

usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
if (dwc->gadget_driver && 
dwc->gadget_driver->disconnect)
usb_gadget_udc_stop(udc);
udc->driver->unbind(udc->gadget);

dwc->gadget_driver->disconnect(>gadget);

UDC drivers typically set their gadget driver pointer to NULL in 
udc_stop
and check for it before calling into the gadget driver. To fix the 
issue

above every udc driver could apply a lock around this.

If you see the need for having this or another solutions I can provide
further patches. This patch could also just serve as a base for 
discussion

if someone knows a smarter solution.

I saw this problem causing a panic on hikey960 board and provided a 
quick

workaround for the same problem here:
https://android-review.googlesource.com/#/c/kernel/common/+/457476/
(panic log in the commit message of the linked patch)
---
 drivers/usb/gadget/udc/core.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c 
b/drivers/usb/gadget/udc/core.c

index efce68e9a8e0..8155468afc0d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct 
usb_udc *udc)


usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
-   udc->driver->unbind(udc->gadget);
+	/* udc_stop needs to be called before gadget driver unbind to 
prevent
+	 * udc driver calls into gadget driver after unbind which could 
cause

+* a nullptr exception.
+*/
usb_gadget_udc_stop(udc);
+   udc->driver->unbind(udc->gadget);


This patch is incorrect, it will prevent us from giving back requests 
to

gadget driver properly. ->unbind() has to happen before ->udc_stop().


Do you mean after udc_stop the udc driver can not call the gadget driver
anymore? If not, I did not got your point, sorry for that. Can you 
please
help me out? Would the changed order raise another issue I'm not aware 
of?


If I understood you correctly, without this patch udc driver can not 
call

the gadget driver back as well, because this would result in a NULL ptr
dereference, as unbind() sets drvdata to NULL.

In any case the race described in my original message can still happen,
regardless of the order of udc_stop and unbind. But with this patch the
needed locking could easily done within the udc driver only. Without, 
the

lock needs to be acquired before udc->driver->unbind(udc->gadget) and
released after usb_gadget_udc_stop(). Otherwise an ISR of the udc driver
trying to call into the gadget driver could do this after gadget driver
already unbound.

Regards,
Danilo
--
To unsubscribe from this list: send the line "unsubscribe 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/3] usb: chipidea: Hook into mux framework to toggle usb switch

[Peter Rosin]

On 2017-08-12 00:26, Stephen Boyd wrote:
> Quoting Peter Rosin (2017-08-08 05:46:30)
>> On 2017-08-08 03:51, Stephen Boyd wrote:
>>
>>>  It looked like we paired the start/stop ops with
>>> each other so that the mux is properly managed across these ops.
>>
>> Yes, it *looks* ok...
>>
>>>  My
>>> testing hasn't shown a problem, but maybe there's some corner case
>>> you're thinking of? I'll double check the code.
>>
>> ...but since I do not know the usb code, I can't tell. What I worry about
>> is the usb core calling udc_id_switch_for_host or udc_id_switch_for_device
>> more than once without any call to the other in between. Maybe that is a
>> guarantee that the usb core makes? Or maybe it isn't? If e.g. there is a
>> third mode (or if one is added in the future), then the calls to
>> mux_control_select and mux_control_deselect would not be paired correctly.
>> Ok, sure, a third mode probably doesn't exist and will probably not be
>> added, but but but...
>>
>> Also, what happens if udc_id_switch_for_device fails? Is it certain that
>> it will be called again before udc_id_switch_for_host is called, or is
>> the failure simply logged? If the latter, you might have a call to
>> mux_control_deselect without a preceding (and successful) call to
>> mux_control_select. That's fatal.
> 
> The only thing that could fail right now is the mux selection, so we
> wouldn't get into some sort of situation where that's locked in and
> unchangeable. We do rollback the role if it fails to switch, so we also
> wouldn't go into a half-way state of being in one role but not actually
> switching all the way over to it.

What do you roll back to if the initial setting of the role fails?
Hopefully that causes a probe failure?

Cheers,
Peter

>> I have similar worries for host_start/host_stop, but for that case
>> host_stop is not allowed to fail, and it seems like a safe bet that
>> host_stop will only be called if host_start succeeds. So, I'm not as
>> worried there.
>>
>> In other words, the question is if the usb core is designed to allow
>> this kind of "raw" resource administration in udc_id_switch_for_host and
>> udc_id_switch_for_device, or if you need to keep a local record of the
>> state so that you do not do double resource acquisition or attempt to
>> free resources you don't have?
>>
>> I think I would feel better if the muxing for the device mode could
>> be done in a start/stop pair of function just like the host mode is
>> doing. Again, I don't know the usb code and don't know if such hooks
>> exist or not?
>>
> 
> The host_start/host_stop functions are assigned to the same struct
> ci_role_driver ops that udc_idc_switch_for_{device,host} are for the
> gadget role. Really, these things are called from the same place by the
> chipidea driver so not much is different between the two files I modify
> to make the mux calls. Furthermore, we don't want to do this if we have
> HNP or "true" OTG support so I've put it behind the ci_otg_is_fsm_mode()
> check to make sure we don't do any muxing stuff based on fsm state
> changes. It doesn't really make any sense here anyway because this
> device I have doesn't support OTG, just role switching.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] usb: xhci: Handle USB transaction error on address command

[Mathias Nyman]

On 11.08.2017 05:41, Lu Baolu wrote:

Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.

The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.

This patch handles USB transaction errors on address command
completion events. The related discussion threads can be found
through below links.

http://marc.info/?l=linux-usb=149362010728921=2
http://marc.info/?l=linux-usb=149252752825755=2

Suggested-by: Mathias Nyman 
Signed-off-by: Lu Baolu 
---
  drivers/usb/host/xhci.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d6b728d..95780f8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct 
usb_device *udev,
break;
case COMP_USB_TRANSACTION_ERROR:
dev_warn(>dev, "Device not responding to setup %s.\n", 
act);
+
+   ret = xhci_disable_slot(xhci, udev->slot_id);
+   if (!ret)
+   xhci_alloc_dev(hcd, udev);


Might be a xhci->mutex locking issue here,
both xhci_setup_device() and xhci_alloc_dev() take xhci->mutex

-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 v4 0/3] Introduce USB charger support in USB phy

[Baolin Wang]

Hi Felipe,

On 15 August 2017 at 17:53, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>>> Currently the Linux kernel does not provide any standard integration of this
>>> feature that integrates the USB subsystem with the system power regulation
>>> provided by PMICs meaning that either vendors must add this in their kernels
>>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>>> as they should. Thus provide a standard USB charger support in USB phy core
>>> for doing this in kernel.
>>>
>>> Now introduce one user with wm831x_power to support and test the usb 
>>> charger.
>>> In future we will also cnvert below power drivers:
>>> drivers/power/supply/axp288_charger.c
>>> drivers/power/supply/bq24190_charger.c
>>> drivers/power/supply/charger-manager.c
>>> drivers/power/supply/qcom_smbb.c
>>>
>>> Changes since v3:
>>>  - Bail out errors when failed to find usb phy for wm831x_power driver.
>>> Changes since v2:
>>>  - Add DT binding documentation for wm831x_power driver.
>>>  - Change 'usb-phy' as one optional property for wm831x_power driver.
>>> Changes since v1:
>>>  - Fix building errors.
>>
>> Do you have any comments about usb charger support in usb phy core? Thanks.
>
> No more comments from me

Thanks for your feedback. I've send out V5 patchset which just changes
phy phandle name from 'usb-phy' to 'phys' for patch 3 suggested by
Rob. Hope you can apply this version patchset into your branch if
there are no other comments.

-- 
Baolin.wang
Best 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

[PATCH v5 2/3] usb: phy: Add USB charger support

[Baolin Wang]

This patch introduces the usb charger support based on usb phy that
makes an enhancement to a power driver. The basic conception of the
usb charger is that, when one usb charger is added or removed by
reporting from the extcon device state change, the usb charger will
report to power user to set the current limitation.

Power user can register a notifiee on the usb phy by issuing
usb_register_notifier() to get notified by charger status changes
or charger current changes.

we can notify what current to be drawn to power user according to
different charger type, and now we have 2 methods to get charger type.
One is get charger type from extcon subsystem, which also means the
charger state changes. Another is we can get the charger type from
USB controller detecting or PMIC detecting, and the charger state
changes should be told by issuing usb_phy_set_charger_state().

Signed-off-by: Baolin Wang 
---
 drivers/usb/phy/phy.c   |  272 +++
 include/linux/usb/phy.h |   49 +
 2 files changed, 321 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 032f5af..2dc48bb 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -18,6 +18,18 @@
 
 #include 
 
+/* Default current range by charger type. */
+#define DEFAULT_SDP_CUR_MIN2
+#define DEFAULT_SDP_CUR_MAX500
+#define DEFAULT_SDP_CUR_MIN_SS 150
+#define DEFAULT_SDP_CUR_MAX_SS 900
+#define DEFAULT_DCP_CUR_MIN500
+#define DEFAULT_DCP_CUR_MAX5000
+#define DEFAULT_CDP_CUR_MIN1500
+#define DEFAULT_CDP_CUR_MAX5000
+#define DEFAULT_ACA_CUR_MIN1500
+#define DEFAULT_ACA_CUR_MAX5000
+
 static LIST_HEAD(phy_list);
 static LIST_HEAD(phy_bind_list);
 static DEFINE_SPINLOCK(phy_lock);
@@ -77,6 +89,221 @@ static struct usb_phy *__of_usb_find_phy(struct device_node 
*node)
return ERR_PTR(-EPROBE_DEFER);
 }
 
+static void usb_phy_set_default_current(struct usb_phy *usb_phy)
+{
+   usb_phy->chg_cur.sdp_min = DEFAULT_SDP_CUR_MIN;
+   usb_phy->chg_cur.sdp_max = DEFAULT_SDP_CUR_MAX;
+   usb_phy->chg_cur.dcp_min = DEFAULT_DCP_CUR_MIN;
+   usb_phy->chg_cur.dcp_max = DEFAULT_DCP_CUR_MAX;
+   usb_phy->chg_cur.cdp_min = DEFAULT_CDP_CUR_MIN;
+   usb_phy->chg_cur.cdp_max = DEFAULT_CDP_CUR_MAX;
+   usb_phy->chg_cur.aca_min = DEFAULT_ACA_CUR_MIN;
+   usb_phy->chg_cur.aca_max = DEFAULT_ACA_CUR_MAX;
+}
+
+/**
+ * usb_phy_notify_charger_work - notify the USB charger state
+ * @work - the charger work to notify the USB charger state
+ *
+ * This work can be issued when USB charger state has been changed or
+ * USB charger current has been changed, then we can notify the current
+ * what can be drawn to power user and the charger state to userspace.
+ *
+ * If we get the charger type from extcon subsystem, we can notify the
+ * charger state to power user automatically by usb_phy_get_charger_type()
+ * issuing from extcon subsystem.
+ *
+ * If we get the charger type from ->charger_detect() instead of extcon
+ * subsystem, the usb phy driver should issue usb_phy_set_charger_state()
+ * to set charger state when the charger state has been changed.
+ */
+static void usb_phy_notify_charger_work(struct work_struct *work)
+{
+   struct usb_phy *usb_phy = container_of(work, struct usb_phy, chg_work);
+   char uchger_state[50] = { 0 };
+   char *envp[] = { uchger_state, NULL };
+   unsigned int min, max;
+
+   switch (usb_phy->chg_state) {
+   case USB_CHARGER_PRESENT:
+   usb_phy_get_charger_current(usb_phy, , );
+
+   atomic_notifier_call_chain(_phy->notifier, max, usb_phy);
+   snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+"USB_CHARGER_STATE=%s", "USB_CHARGER_PRESENT");
+   break;
+   case USB_CHARGER_ABSENT:
+   usb_phy_set_default_current(usb_phy);
+
+   atomic_notifier_call_chain(_phy->notifier, 0, usb_phy);
+   snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+"USB_CHARGER_STATE=%s", "USB_CHARGER_ABSENT");
+   break;
+   default:
+   dev_warn(usb_phy->dev, "Unknown USB charger state: %d\n",
+usb_phy->chg_state);
+   return;
+   }
+
+   kobject_uevent_env(_phy->dev->kobj, KOBJ_CHANGE, envp);
+}
+
+static void __usb_phy_get_charger_type(struct usb_phy *usb_phy)
+{
+   if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_SDP) > 0) {
+   usb_phy->chg_type = SDP_TYPE;
+   usb_phy->chg_state = USB_CHARGER_PRESENT;
+   } else if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_CDP) > 0) {
+   usb_phy->chg_type = CDP_TYPE;
+   usb_phy->chg_state = USB_CHARGER_PRESENT;
+   } else if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_DCP) > 0) {
+   usb_phy->chg_type = DCP_TYPE;
+   usb_phy->chg_state = 

[PATCH v5 0/3] Introduce USB charger support in USB phy

[Baolin Wang]

Currently the Linux kernel does not provide any standard integration of this
feature that integrates the USB subsystem with the system power regulation
provided by PMICs meaning that either vendors must add this in their kernels
or USB gadget devices based on Linux (such as mobile phones) may not behave
as they should. Thus provide a standard USB charger support in USB phy core
for doing this in kernel.

Now introduce one user with wm831x_power to support and test the usb charger.
In future we will also cnvert below power drivers:
drivers/power/supply/axp288_charger.c
drivers/power/supply/bq24190_charger.c
drivers/power/supply/charger-manager.c
drivers/power/supply/qcom_smbb.c

Changes since v4:
 - Change the phandle name from 'usb-phy' to 'phys' for wm831x_power driver.
Changes since v3:
 - Bail out errors when failed to find usb phy for wm831x_power driver.
Changes since v2:
 - Add DT binding documentation for wm831x_power driver.
 - Change 'usb-phy' as one optional property for wm831x_power driver.
Changes since v1:
 - Fix building errors.

Baolin Wang (3):
  include: uapi: usb: Introduce USB charger type and state definition
  usb: phy: Add USB charger support
  power: wm831x_power: Support USB charger current limit management

 Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
 drivers/power/supply/wm831x_power.c  |   72 ++
 drivers/usb/phy/phy.c|  272 ++
 include/linux/usb/phy.h  |   49 
 include/uapi/linux/usb/charger.h |   31 +++
 5 files changed, 425 insertions(+)
 create mode 100644 include/uapi/linux/usb/charger.h

-- 
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 v5 1/3] include: uapi: usb: Introduce USB charger type and state definition

[Baolin Wang]

Introducing USB charger type and state definition can help
to support USB charging which will be added in USB phy core.

Signed-off-by: Baolin Wang 
---
 include/uapi/linux/usb/charger.h |   31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 include/uapi/linux/usb/charger.h

diff --git a/include/uapi/linux/usb/charger.h b/include/uapi/linux/usb/charger.h
new file mode 100644
index 000..5f72af3
--- /dev/null
+++ b/include/uapi/linux/usb/charger.h
@@ -0,0 +1,31 @@
+/*
+ * This file defines the USB charger type and state that are needed for
+ * USB device APIs.
+ */
+
+#ifndef _UAPI__LINUX_USB_CHARGER_H
+#define _UAPI__LINUX_USB_CHARGER_H
+
+/*
+ * USB charger type:
+ * SDP (Standard Downstream Port)
+ * DCP (Dedicated Charging Port)
+ * CDP (Charging Downstream Port)
+ * ACA (Accessory Charger Adapters)
+ */
+enum usb_charger_type {
+   UNKNOWN_TYPE,
+   SDP_TYPE,
+   DCP_TYPE,
+   CDP_TYPE,
+   ACA_TYPE,
+};
+
+/* USB charger state */
+enum usb_charger_state {
+   USB_CHARGER_DEFAULT,
+   USB_CHARGER_PRESENT,
+   USB_CHARGER_ABSENT,
+};
+
+#endif /* _UAPI__LINUX_USB_CHARGER_H */
-- 
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 v5 3/3] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown 
Signed-off-by: Baolin Wang 
Acked-by: Lee Jones 
Acked-by: Charles Keepax 
Acked-by: Sebastian Reichel 
---
 Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
 drivers/power/supply/wm831x_power.c  |   72 ++
 2 files changed, 73 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt 
b/Documentation/devicetree/bindings/mfd/wm831x.txt
index 9f8b743..5057094 100644
--- a/Documentation/devicetree/bindings/mfd/wm831x.txt
+++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
@@ -31,6 +31,7 @@ Required properties:
 ../interrupt-controller/interrupts.txt
 
 Optional sub-nodes:
+  - phys : Contains a phandle to the USB PHY.
   - regulators : Contains sub-nodes for each of the regulators supplied by
 the device. The regulators are bound using their names listed below:
 
diff --git a/drivers/power/supply/wm831x_power.c 
b/drivers/power/supply/wm831x_power.c
index 7082301..927050d 100644
--- a/drivers/power/supply/wm831x_power.c
+++ b/drivers/power/supply/wm831x_power.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+   struct usb_phy *usb_phy;
+   struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+   0,
+   2,
+   100,
+   500,
+   900,
+   1500,
+   1800,
+   550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+  unsigned long limit, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   unsigned int i, best;
+
+   /* Find the highest supported limit */
+   best = 0;
+   for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+   if (limit >= wm831x_usb_limits[i] &&
+   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+   best = i;
+   }
+
+   dev_dbg(wm831x_power->wm831x->dev,
+   "Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+   wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+   WM831X_USB_ILIM_MASK, best);
+
+   return 0;
+}
+
 /*
  * Battery properties
  */
@@ -607,6 +647,33 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
 
+   power->usb_phy = devm_usb_get_phy_by_phandle(>dev, "phys", 0);
+   ret = PTR_ERR_OR_ZERO(power->usb_phy);
+
+   switch (ret) {
+   case 0:
+   power->usb_notify.notifier_call = wm831x_usb_limit_change;
+   ret = usb_register_notifier(power->usb_phy, >usb_notify);
+   if (ret) {
+   dev_err(>dev, "Failed to register notifier: %d\n",
+   ret);
+   goto err_bat_irq;
+   }
+   break;
+   case -EINVAL:
+   case -ENODEV:
+   /* ignore missing usb-phy, it's optional */
+   power->usb_phy = NULL;
+   ret = 0;
+   break;
+   default:
+   dev_err(>dev, "Failed to find USB phy: %d\n", ret);
+   /* fall-through */
+   case -EPROBE_DEFER:
+   goto err_bat_irq;
+   break;
+   }
+
return ret;
 
 err_bat_irq:
@@ -637,6 +704,11 @@ static int wm831x_power_remove(struct platform_device 
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
 
+   if (wm831x_power->usb_phy) {
+   usb_unregister_notifier(wm831x_power->usb_phy,
+   _power->usb_notify);
+   }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x, 
 platform_get_irq_byname(pdev,
-- 
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

Re: [balbi-usb:next 21/42] drivers/usb/gadget/function/f_midi.c:836:24: error: 'f_midi_rmidi_free' undeclared

[Felipe Balbi]

Hi,

kbuild test robot  writes:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> head:   af982758d0a9e5189e657de2bbab2f65f16c48cc
> commit: b8bd28e32e5fc3ad2d8c80ad3978fabb523a1763 [21/42] usb: gadget: f_midi: 
> Use snd_card_free_when_closed with refcount
> config: i386-randconfig-x075-201733 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> git checkout b8bd28e32e5fc3ad2d8c80ad3978fabb523a1763
> # save the attached .config to linux build tree
> make ARCH=i386 
>
> All errors (new ones prefixed by >>):
>
>drivers/usb/gadget/function/f_midi.c: In function 'f_midi_register_card':
>>> drivers/usb/gadget/function/f_midi.c:836:24: error: 'f_midi_rmidi_free' 
>>> undeclared (first use in this function)
>  rmidi->private_free = f_midi_rmidi_free;
>^
>drivers/usb/gadget/function/f_midi.c:836:24: note: each undeclared 
> identifier is reported only once for each function it appears in
>At top level:
>drivers/usb/gadget/function/f_midi.c:1249:13: warning: 'f_midi_rmidi_free' 
> defined but not used [-Wunused-function]
> static void f_midi_rmidi_free(struct snd_rawmidi *rmidi)
> ^
>
> vim +/f_midi_rmidi_free +836 drivers/usb/gadget/function/f_midi.c

I'll fix it up locally, thanks

@@ -109,6 +109,7 @@ static inline struct f_midi *func_to_midi(struct 
usb_function *f)
 }
 
 static void f_midi_transmit(struct f_midi *midi);
+static void f_midi_rmidi_free(struct snd_rawmidi *rmidi);
 
 DECLARE_UAC_AC_HEADER_DESCRIPTOR(1);
 DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1);

-- 
balbi


signature.asc
Description: PGP signature

[balbi-usb:next 21/42] drivers/usb/gadget/function/f_midi.c:836:24: error: 'f_midi_rmidi_free' undeclared

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
head:   af982758d0a9e5189e657de2bbab2f65f16c48cc
commit: b8bd28e32e5fc3ad2d8c80ad3978fabb523a1763 [21/42] usb: gadget: f_midi: 
Use snd_card_free_when_closed with refcount
config: i386-randconfig-x075-201733 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout b8bd28e32e5fc3ad2d8c80ad3978fabb523a1763
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/usb/gadget/function/f_midi.c: In function 'f_midi_register_card':
>> drivers/usb/gadget/function/f_midi.c:836:24: error: 'f_midi_rmidi_free' 
>> undeclared (first use in this function)
 rmidi->private_free = f_midi_rmidi_free;
   ^
   drivers/usb/gadget/function/f_midi.c:836:24: note: each undeclared 
identifier is reported only once for each function it appears in
   At top level:
   drivers/usb/gadget/function/f_midi.c:1249:13: warning: 'f_midi_rmidi_free' 
defined but not used [-Wunused-function]
static void f_midi_rmidi_free(struct snd_rawmidi *rmidi)
^

vim +/f_midi_rmidi_free +836 drivers/usb/gadget/function/f_midi.c

   792  
   793  /* register as a sound "card" */
   794  static int f_midi_register_card(struct f_midi *midi)
   795  {
   796  struct snd_card *card;
   797  struct snd_rawmidi *rmidi;
   798  int err;
   799  static struct snd_device_ops ops = {
   800  .dev_free = f_midi_snd_free,
   801  };
   802  
   803  err = snd_card_new(>gadget->dev, midi->index, midi->id,
   804 THIS_MODULE, 0, );
   805  if (err < 0) {
   806  ERROR(midi, "snd_card_new() failed\n");
   807  goto fail;
   808  }
   809  midi->card = card;
   810  
   811  err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, midi, );
   812  if (err < 0) {
   813  ERROR(midi, "snd_device_new() failed: error %d\n", err);
   814  goto fail;
   815  }
   816  
   817  strcpy(card->driver, f_midi_longname);
   818  strcpy(card->longname, f_midi_longname);
   819  strcpy(card->shortname, f_midi_shortname);
   820  
   821  /* Set up rawmidi */
   822  snd_component_add(card, "MIDI");
   823  err = snd_rawmidi_new(card, card->longname, 0,
   824midi->out_ports, midi->in_ports, );
   825  if (err < 0) {
   826  ERROR(midi, "snd_rawmidi_new() failed: error %d\n", 
err);
   827  goto fail;
   828  }
   829  midi->rmidi = rmidi;
   830  midi->in_last_port = 0;
   831  strcpy(rmidi->name, card->shortname);
   832  rmidi->info_flags = SNDRV_RAWMIDI_INFO_OUTPUT |
   833  SNDRV_RAWMIDI_INFO_INPUT |
   834  SNDRV_RAWMIDI_INFO_DUPLEX;
   835  rmidi->private_data = midi;
 > 836  rmidi->private_free = f_midi_rmidi_free;
   837  midi->free_ref++;
   838  
   839  /*
   840   * Yes, rawmidi OUTPUT = USB IN, and rawmidi INPUT = USB OUT.
   841   * It's an upside-down world being a gadget.
   842   */
   843  snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT, 
_in_ops);
   844  snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_INPUT, 
_out_ops);
   845  
   846  /* register it - we're ready to go */
   847  err = snd_card_register(card);
   848  if (err < 0) {
   849  ERROR(midi, "snd_card_register() failed\n");
   850  goto fail;
   851  }
   852  
   853  VDBG(midi, "%s() finished ok\n", __func__);
   854  return 0;
   855  
   856  fail:
   857  f_midi_unregister_card(midi);
   858  return err;
   859  }
   860  

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


.config.gz
Description: application/gzip

Re: [PATCH 2/2] usb: musb: musb_cppi41: Fix cppi41_set_dma_mode() for DA8xx

[Sergei Shtylyov]

On 8/13/2017 3:04 PM, Alexandre Bailon wrote:


The way to configure the DMA mode on DA8xx is different from DSPS.
Add a new function to configure DMA mode on DA8xx and use a callback
to call the right function based on the platform.

Signed-off-by: Alexandre Bailon 
---
  drivers/usb/musb/musb_cppi41.c | 40 +---
  1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index dbff0e0a4ff5..7284ec7ecff7 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -26,6 +26,7 @@
  
  #define MUSB_DMA_NUM_CHANNELS 15


   Perhaps this needs parametrizing too? DA8xx only has 4 channels IIRC...



+#define DA8XX_USB_MODE_REG 0x10


   Drop this _REG suffix please.


  #define DA8XX_USB_AUTOREQ_REG 0x14
  #define DA8XX_USB_TEARDOWN_REG0x1c
  

[...]

@@ -413,14 +444,15 @@ static bool cppi41_configure_channel(struct dma_channel 
*channel,
} else {
musb_writel(musb->ctrl_base,
RNDIS_REG(cppi41_channel->port_num), 0);
-   cppi41_set_dma_mode(cppi41_channel,
+   controller->set_dma_mode(cppi41_channel,
EP_MODE_DMA_TRANSPARENT);
cppi41_set_autoreq_mode(cppi41_channel,
EP_MODE_AUTOREQ_NONE);
}
} else {
/* fallback mode */
-   cppi41_set_dma_mode(cppi41_channel, EP_MODE_DMA_TRANSPARENT);
+   controller->set_dma_mode(cppi41_channel,
+   EP_MODE_DMA_TRANSPARENT);


   Inconsistent indentation -- you used 2 extra tabs above and 3 here.

[...]

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

Re: [PATCH 1/2] usb: musb: musb_cppi41: Fix the address of teardown and autoreq registers

[Sergei Shtylyov]

Hello!

On 8/13/2017 3:04 PM, Alexandre Bailon wrote:


The DA8xx and DSPS platforms don't use the same address for few registers.
On Da8xx, this is causing some issues (e.g. teardown that doesn't work).
Configure the address of the register during the init and use them instead
of constants.

Reported-by: nsek...@ti.com
Signed-off-by: Alexandre Bailon 
---
  drivers/usb/musb/musb_cppi41.c | 23 +++
  1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index ba255280a624..dbff0e0a4ff5 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -26,6 +26,9 @@
  
  #define MUSB_DMA_NUM_CHANNELS 15
  
+#define DA8XX_USB_AUTOREQ_REG	0x14

+#define DA8XX_USB_TEARDOWN_REG 0x1c


   Why these _REG suffixes suddenly?

[...]

   Other than that looks sane. Need my ACK?

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe 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: udc: udc_stop before gadget unbind

[Felipe Balbi]

Hi,

Danilo Krummrich  writes:
> udc_stop needs to be called before gadget driver unbind. Otherwise it
> might happen that udc drivers still call into the gadget driver (e.g.
> to reset gadget after OTG event). If this happens it is likely to get
> panics from gadget driver dereferencing NULL ptr, as gadget's drvdata
> is set to NULL on unbind.

seems like the problem here is with the OTG layer, not UDC core.

> Signed-off-by: Danilo Krummrich 
> ---
> Actually there could still be a race:
> (CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as exsample)
>
> CPU0  CPU1
>   
> usb_gadget_disconnect(udc->gadget);
> udc->driver->disconnect(udc->gadget);
>   if (dwc->gadget_driver && 
> dwc->gadget_driver->disconnect)
> usb_gadget_udc_stop(udc);
> udc->driver->unbind(udc->gadget);
>   
> dwc->gadget_driver->disconnect(>gadget);
>
> UDC drivers typically set their gadget driver pointer to NULL in udc_stop
> and check for it before calling into the gadget driver. To fix the issue
> above every udc driver could apply a lock around this.
>
> If you see the need for having this or another solutions I can provide
> further patches. This patch could also just serve as a base for discussion
> if someone knows a smarter solution.
>
> I saw this problem causing a panic on hikey960 board and provided a quick
> workaround for the same problem here:
> https://android-review.googlesource.com/#/c/kernel/common/+/457476/
> (panic log in the commit message of the linked patch)
> ---
>  drivers/usb/gadget/udc/core.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index efce68e9a8e0..8155468afc0d 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct usb_udc 
> *udc)
>  
>   usb_gadget_disconnect(udc->gadget);
>   udc->driver->disconnect(udc->gadget);
> - udc->driver->unbind(udc->gadget);
> + /* udc_stop needs to be called before gadget driver unbind to prevent
> +  * udc driver calls into gadget driver after unbind which could cause
> +  * a nullptr exception.
> +  */
>   usb_gadget_udc_stop(udc);
> + udc->driver->unbind(udc->gadget);

This patch is incorrect, it will prevent us from giving back requests to
gadget driver properly. ->unbind() has to happen before ->udc_stop().

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v4 0/3] Introduce USB charger support in USB phy

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>> Currently the Linux kernel does not provide any standard integration of this
>> feature that integrates the USB subsystem with the system power regulation
>> provided by PMICs meaning that either vendors must add this in their kernels
>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>> as they should. Thus provide a standard USB charger support in USB phy core
>> for doing this in kernel.
>>
>> Now introduce one user with wm831x_power to support and test the usb charger.
>> In future we will also cnvert below power drivers:
>> drivers/power/supply/axp288_charger.c
>> drivers/power/supply/bq24190_charger.c
>> drivers/power/supply/charger-manager.c
>> drivers/power/supply/qcom_smbb.c
>>
>> Changes since v3:
>>  - Bail out errors when failed to find usb phy for wm831x_power driver.
>> Changes since v2:
>>  - Add DT binding documentation for wm831x_power driver.
>>  - Change 'usb-phy' as one optional property for wm831x_power driver.
>> Changes since v1:
>>  - Fix building errors.
>
> Do you have any comments about usb charger support in usb phy core? Thanks.

No more comments from me

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v5 1/8] usb: gadget: f_ecm/f_eem/f_rndis: Setup quirk_avoids_skb_reserve

[Felipe Balbi]

Hi,

Dmitry Osipenko  writes:
> This quirk is required to make USB Ethernet gadget working with HW that
> can't cope with unaligned DMA. For some reason only f_ncm handles that
> quirk, let's handle it in the rest of the network models. All models have
> been tested with a ChipIdea UDC driver on NVIDIA Tegra20 SoC that require
> DMA to be aligned.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/usb/gadget/function/f_ecm.c   | 7 +++
>  drivers/usb/gadget/function/f_eem.c   | 5 +
>  drivers/usb/gadget/function/f_rndis.c | 4 
>  3 files changed, 16 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_ecm.c 
> b/drivers/usb/gadget/function/f_ecm.c
> index 4c488d15b6f6..1d198055fd74 100644
> --- a/drivers/usb/gadget/function/f_ecm.c
> +++ b/drivers/usb/gadget/function/f_ecm.c
> @@ -584,6 +584,13 @@ static int ecm_set_alt(struct usb_function *f, unsigned 
> intf, unsigned alt)
>*/
>   ecm->port.is_zlp_ok =
>   gadget_is_zlp_supported(cdev->gadget);
> +
> + /* Setup DMA alignment workaround for UDC's that
> +  * need it.
> +  */
> + ecm->port.no_skb_reserve =
> + gadget_avoids_skb_reserve(cdev->gadget);

looks like the quirk should be moved to u_ether.c instead.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 4/5] usb: gadget: udc: Replace the deprecated extcon API

[Felipe Balbi]

Hi,

Chanwoo Choi  writes:
> This patch replaces the deprecated extcon API as following:
> - extcon_get_cable_state_() -> extcon_get_state()
>
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: Raviteja Garimella 
> Signed-off-by: Chanwoo Choi 

Do you want to take these through your tree or mine? In case you want
them in your tree:

Acked-by: Felipe Balbi 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 3/3] usb: gadget: udc: renesas_usb3: add support for R-Car M3-W

[Felipe Balbi]

Hi,

Rob Herring  writes:
> On Fri, Aug 04, 2017 at 11:16:58AM +0900, Yoshihiro Shimoda wrote:
>> This patch adds support for R-Car M3-W. This patch also adds R-Car
>> Gen3 generic version's compatible and changes ".compatible" in
>> the usb3_of_match from "renesas,r8a7796-usb3-peri" to
>> "renesas,rcar-gen3-usb3-peri".
>> 
>> Signed-off-by: Yoshihiro Shimoda 
>> ---
>>  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 16 +---
>>  drivers/usb/gadget/udc/renesas_usb3.c  |  2 +-
>>  2 files changed, 14 insertions(+), 4 deletions(-)
>
> Binding looks fine, but...
>
>> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
>> b/drivers/usb/gadget/udc/renesas_usb3.c
>> index aa2b185..b1e166c 100644
>> --- a/drivers/usb/gadget/udc/renesas_usb3.c
>> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
>> @@ -2503,7 +2503,7 @@ static void renesas_usb3_init_ram(struct renesas_usb3 
>> *usb3, struct device *dev,
>>  
>>  static const struct of_device_id usb3_of_match[] = {
>>  {
>> -.compatible = "renesas,r8a7795-usb3-peri",
>> +.compatible = "renesas,rcar-gen3-usb3-peri",
>
> You need to keep the existing string for compatibility with existing 
> dtbs.

I've fixed it up locally:

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index ff69f4645b7c..16ceb445bee8 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2512,6 +2512,10 @@ static const struct of_device_id usb3_of_match[] = {
.compatible = "renesas,r8a7795-usb3-peri",
.data = _usb3_priv_gen3,
},
+   {
+   .compatible = "renesas,rcar-gen3-usb3-peri",
+   .data = _usb3_priv_gen3,
+   },
{ },
 };
 MODULE_DEVICE_TABLE(of, usb3_of_match);


-- 
balbi


signature.asc
Description: PGP signature

Re: uvc-gadget for UVC testing doesn't seem to work with vivid

[Felipe Balbi]

Hi,

Rail Shafigulin  writes:
> Let me apologize for emailing directly to the list as I'm not one of
> the developers and just starting out with USB and UVC. After searching

the list is open to anybody and we welcome newcomers :-)

> online for about a week I just couldn't find answers and I hope the
> original authors of the uvc-gadget tool are on the list and can help
> out.
>
> Needed to test a UVC in a custom built kernel (Xilinx petalinux), .
> Followed these directions,
> https://github.com/torvalds/linux/blob/ef954844c7ace62f773f4f23e28d2d915adc419f/Documentation/usb/gadget-testing.txt#L717-L730.
>
> Patches didn't work. Had to look around for correct ones. Found them
> here, 
> http://markmail.org/message/hb7evzvigbuxptz5#query:+page:1+mid:s73fdeffjgb2v2yw+state:results.
>
> Combined and applied the patches into a repo here,
> https://github.com/cyboflash/uvc-gadget.git.
>
> When I ran a test command, given in the instructions above,
> uvc-gadget -u /dev/video -v /dev/video
>
> got the following error:
> V4L2_CORE: (jpeg decoder) error while decoding frame
>
> and a black screen.
>
> One thing to note is that I was not using luvcview, but guvcview.
>
> It looks like the error is coming from here,
> https://sourceforge.net/p/guvcview/git-master/ci/master/tree/gview_v4l2core/jpeg_decoder.c#l1503
>
> My thoughts
> 1. I don't think the error is coming from v4l2. I tested it on another
> machine and it worked. But I'm not an expert so I can't say for sure.
> 2. I don't think the error is coming from UVC. I think since I see a
> black screen, it is working. Again, I'm not an expert, so I can't say
> for sure.
> 3. I think the error is due to uvc-gadget test application. It could
> be that the applied patches are outdated, but I just didn't find
> anything else online. But, I'm not an expert so definitely can't say
> for sure.
>
> I would greatly appreciate any help with this as I'm just starting out
> with UVC and USB.

Which kernel are you using? Which UDC driver are you using?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH 0/3] constify snd_rawmidi_ops structures

[Julia Lawall]

These snd_rawmidi_ops structures are only passed as the third
argument of snd_rawmidi_set_ops.  This argument is const, so the
snd_rawmidi_ops structures can be const too.

Done with the help of Coccinelle.

---

 drivers/hid/hid-prodikeys.c  |2 +-
 drivers/usb/gadget/function/f_midi.c |4 ++--
 sound/firewire/motu/motu-midi.c  |4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe 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/3] usb: gadget: f_midi: constify snd_rawmidi_ops structures

[Julia Lawall]

These snd_rawmidi_ops structures are only passed as the third
argument of snd_rawmidi_set_ops.  This argument is const, so the
snd_rawmidi_ops structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/usb/gadget/function/f_midi.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index a5719f2..71ca86c0 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -755,13 +755,13 @@ static void f_midi_out_trigger(struct 
snd_rawmidi_substream *substream, int up)
clear_bit(substream->number, >out_triggered);
 }
 
-static struct snd_rawmidi_ops gmidi_in_ops = {
+static const struct snd_rawmidi_ops gmidi_in_ops = {
.open = f_midi_in_open,
.close = f_midi_in_close,
.trigger = f_midi_in_trigger,
 };
 
-static struct snd_rawmidi_ops gmidi_out_ops = {
+static const struct snd_rawmidi_ops gmidi_out_ops = {
.open = f_midi_out_open,
.close = f_midi_out_close,
.trigger = f_midi_out_trigger

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

[PATCH] usb: gadget: dummy: fix infinite loop because of missing loop decrement

[Colin King]

From: Colin Ian King 

The while loop never terminates because the loop counter i is never
decremented. Fix this by decrementing i.

Detected by CoverityScan, CID#751073 ("Infinite Loop")

Signed-off-by: Colin Ian King 
---
 drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c 
b/drivers/usb/gadget/udc/dummy_hcd.c
index 3c3760315910..a030d7923d7d 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2776,7 +2776,7 @@ static int __init init(void)
if (retval < 0) {
i--;
while (i >= 0)
-   platform_device_del(the_udc_pdev[i]);
+   platform_device_del(the_udc_pdev[i--]);
goto err_add_udc;
}
}
-- 
2.11.0

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