Re: [PATCH] Add support for usbfs zerocopy.

[Alan Stern]

On Tue, 22 Dec 2015, Oliver Neukum wrote:

> On Tue, 2015-12-22 at 01:12 +0100, Steinar H. Gunderson wrote:
> > On Tue, Dec 15, 2015 at 03:32:45PM +, David Laight wrote:
> > > That still isn't entirely correct.
> > > 
> > > Someone with more knowledge than either of us has needs to sort out
> > > how to handle this properly.
> > 
> > This discussion sort of stalled. Short of actually becoming a kernel VM
> > expert (not very likely), is there anything I can do to help move this
> > forward?
> 
> It may be profitable to look at how other drivers do this.
> graphics should have the same issues. Maybe sound, too.

Steinar, you can look at those if you want to.  But I really don't 
think anything more is needed other than the try_module_get and 
module_put calls.  At least, the Linux Device Drivers book doesn't 
mention anything else.

Alan Stern

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

Re: [PATCH v3] usb: dwc3: gadget: handle request->zero

[John Youn]

On 12/22/2015 9:31 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn  writes:
>> On 12/3/2015 7:18 AM, Felipe Balbi wrote:
>>> So far, dwc3 has always missed request->zero
>>> handling for every endpoint. Let's implement
>>> that so we can handle cases where transfer must
>>> be finished with a ZLP.
>>>
>>> Note that dwc3 is a little special. Even though
>>> we're dealing with a ZLP, we still need a buffer
>>> of wMaxPacketSize bytes; to hide that detail from
>>> every gadget driver, we have a preallocated buffer
>>> of 1024 bytes (biggest bulk size) to use (and
>>> share) among all endpoints.
>>>
>>> Reported-by: Ravi B 
>>> Signed-off-by: Felipe Balbi 
>>> ---
>>>
>>> since v1:
>>> - remove unnecessary 'free_on_complete' flag
>>>
>>> since v2:
>>> - remove unnecessary 'out' label
>>>
>>>  drivers/usb/dwc3/core.h   |  3 +++
>>>  drivers/usb/dwc3/gadget.c | 50 
>>> +--
>>>  2 files changed, 51 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 36f1cb74588c..29130682e547 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -37,6 +37,7 @@
>>>  #define DWC3_MSG_MAX   500
>>>  
>>>  /* Global constants */
>>> +#define DWC3_ZLP_BUF_SIZE  1024/* size of a superspeed bulk */
>>>  #define DWC3_EP0_BOUNCE_SIZE   512
>>>  #define DWC3_ENDPOINTS_NUM 32
>>>  #define DWC3_XHCI_RESOURCES_NUM2
>>> @@ -647,6 +648,7 @@ struct dwc3_scratchpad_array {
>>>   * @ctrl_req: usb control request which is used for ep0
>>>   * @ep0_trb: trb which is used for the ctrl_req
>>>   * @ep0_bounce: bounce buffer for ep0
>>> + * @zlp_buf: used when request->zero is set
>>>   * @setup_buf: used while precessing STD USB requests
>>>   * @ctrl_req_addr: dma address of ctrl_req
>>>   * @ep0_trb: dma address of ep0_trb
>>> @@ -734,6 +736,7 @@ struct dwc3 {
>>> struct usb_ctrlrequest  *ctrl_req;
>>> struct dwc3_trb *ep0_trb;
>>> void*ep0_bounce;
>>> +   void*zlp_buf;
>>> void*scratchbuf;
>>> u8  *setup_buf;
>>> dma_addr_t  ctrl_req_addr;
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index e341f034296f..e916c11ded59 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1158,6 +1158,32 @@ out:
>>> return ret;
>>>  }
>>>  
>>> +static void __dwc3_gadget_ep_zlp_complete(struct usb_ep *ep,
>>> +   struct usb_request *request)
>>> +{
>>> +   dwc3_gadget_ep_free_request(ep, request);
>>> +}
>>> +
>>> +static int __dwc3_gadget_ep_queue_zlp(struct dwc3 *dwc, struct dwc3_ep 
>>> *dep)
>>> +{
>>> +   struct dwc3_request *req;
>>> +   struct usb_request  *request;
>>> +   struct usb_ep   *ep = >endpoint;
>>> +
>>> +   dwc3_trace(trace_dwc3_gadget, "queueing ZLP\n");
>>> +   request = dwc3_gadget_ep_alloc_request(ep, GFP_ATOMIC);
>>> +   if (!request)
>>> +   return -ENOMEM;
>>> +
>>> +   request->length = 0;
>>> +   request->buf = dwc->zlp_buf;
>>> +   request->complete = __dwc3_gadget_ep_zlp_complete;
>>> +
>>> +   req = to_dwc3_request(request);
>>> +
>>> +   return __dwc3_gadget_ep_queue(dep, req);
>>> +}
>>> +
>>>  static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request 
>>> *request,
>>> gfp_t gfp_flags)
>>>  {
>>> @@ -1171,6 +1197,16 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, 
>>> struct usb_request *request,
>>>  
>>> spin_lock_irqsave(>lock, flags);
>>> ret = __dwc3_gadget_ep_queue(dep, req);
>>> +
>>> +   /*
>>> +* Okay, here's the thing, if gadget driver has requested for a ZLP by
>>> +* setting request->zero, instead of doing magic, we will just queue an
>>> +* extra usb_request ourselves so that it gets handled the same way as
>>> +* any other request.
>>> +*/
>>> +   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 0))
>>> +   ret = __dwc3_gadget_ep_queue_zlp(dwc, dep);
>>
>> Hi Felipe,
>>
>> This causes regression with at least mass storage + Windows host.
>>
>> When the gadget queues a ZLP, we end up sending two ZLPs which leads
>> to violating the MSC protocol.
> 
> heh, no idea why mass storage would set Zero flag in this case :-p
> 
>> The following fixes it:
>>
>> -   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 
>> 0))
>> +   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 
>> 0) &&
>> +   (request->length != 0))
> 
> Can you send this as a proper patch ?

Sure I can do that. I thought you might want fix it in place since
it's in your testing/next.

> And also patch g_mass_storage to
> _not_ set Zero flag in this case ?
> 

The mass storage driver has always done that and I think it is ok. It
sets this flag for the mass storage data IN phase and 

Re: [PATCH] usb: gadget: rndis: fix itnull.cocci warnings

[Felipe Balbi]

Hi,

Julia Lawall  writes:
> On Tue, 22 Dec 2015, Felipe Balbi wrote:
>
>> Julia Lawall  writes:
>>
>> > The index variable of list_for_each_entry_safe is never NULL.
>> >
>> > Generated by: scripts/coccinelle/iterators/itnull.cocci
>> >
>> > CC: Geliang Tang 
>> > Signed-off-by: Fengguang Wu 
>> > Signed-off-by: Julia Lawall 
>>
>> doesn't apply. Does this depend on anything ?
>
> It may be derived from a patch posted to a mailing list.  I don't have the
> original reference any more.

okay, it could be the big series converting gadgets to
list_for_each_entry_safe(). Thanks

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: option.c: Fix Cinterion AHxx enumeration.

[Johan Hovold]

[ +CC: linux-usb ]

On Tue, Dec 22, 2015 at 10:51:32AM +, John Ernberg wrote:
> From: John Ernberg 
> 
> In certain kernel configurations where the cdc_ether and option drivers
> are compiled as modules there can occur a race condition in enumeration.
> This causes the option driver to enumerate the ethernet(wwan) interface
> as usb-serial interfaces.
> 
> In dmesg it may look like this:
> [ 18.380585] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> [ 18.592290] usbcore: registered new interface driver usbserial
> [ 18.606636] usbcore: registered new interface driver usbserial_generic
> [ 18.614815] usbserial: USB Serial support registered for generic
> [ 18.652111] usbcore: registered new interface driver option
> [ 18.659745] usbserial: USB Serial support registered for GSM modem (1-port)
> [ 18.667600] option 1-1:1.0: GSM modem (1-port) converter detected
> [ 18.676906] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB0
> [ 18.686725] cdc_ether 1-1:1.4 wwan0: register 'cdc_ether' at 
> usb-ci_hdrc.1-1, Mobile Broadband Network Device
> [ 18.705587] option 1-1:1.1: GSM modem (1-port) converter detected
> [ 18.713468] usbcore: registered new interface driver cdc_ether
> [ 18.719930] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB1
> [ 18.729770] option 1-1:1.2: GSM modem (1-port) converter detected
> [ 18.737421] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB2
> [ 18.745232] option 1-1:1.3: GSM modem (1-port) converter detected
> [ 18.752838] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB3
> [ 43.422579] option1 ttyUSB3: option_instat_callback: error -71
> [ 43.622575] option1 ttyUSB3: option_instat_callback: error -71
> [ 43.822579] option1 ttyUSB3: option_instat_callback: error -71
> [ 44.022575] option1 ttyUSB3: option_instat_callback: error -71
> [ 44.222569] option1 ttyUSB3: option_instat_callback: error -71
> [ 44.398490] usb 1-1: USB disconnect, device number 2
> [ 44.405414] option1 ttyUSB0: GSM modem (1-port) converter now disconnected 
> from ttyUSB0
> [ 44.417200] option 1-1:1.0: device disconnected
> [ 44.424903] option1 ttyUSB1: GSM modem (1-port) converter now disconnected 
> from ttyUSB1
> [ 44.434249] option 1-1:1.1: device disconnected
> [ 44.438824] option1 ttyUSB3: option_instat_callback: error -71
> [ 44.448436] option1 ttyUSB2: GSM modem (1-port) converter now disconnected 
> from ttyUSB2
> [ 44.457736] option 1-1:1.2: device disconnected
> [ 44.465204] option1 ttyUSB3: GSM modem (1-port) converter now disconnected 
> from ttyUSB3
> [ 44.474480] option 1-1:1.3: device disconnected
> [ 44.479754] cdc_ether 1-1:1.4 wwan0: unregister 'cdc_ether' usb-ci_hdrc.1-1, 
> Mobile Broadband Network Device
> [ 48.960593] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> [ 49.116118] option 1-1:1.0: GSM modem (1-port) converter detected
> [ 49.123853] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB0
> [ 49.132029] option 1-1:1.1: GSM modem (1-port) converter detected
> [ 49.138778] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB1
> [ 49.147432] option 1-1:1.2: GSM modem (1-port) converter detected
> [ 49.154924] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB2
> [ 49.162940] option 1-1:1.3: GSM modem (1-port) converter detected
> [ 49.169724] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB3
> [ 49.178440] option 1-1:1.4: GSM modem (1-port) converter detected
> [ 49.185979] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB4
> [ 49.193985] option 1-1:1.5: GSM modem (1-port) converter detected
> [ 49.201458] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB5
> ---
>  drivers/usb/serial/option.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index f228060..4e483f2 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1679,7 +1679,7 @@ static const struct usb_device_id option_ids[] = {
>   { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_EU3_P) },
>   { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PH8),
>   .driver_info = (kernel_ulong_t)_intf4_blacklist },
> - { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX) },
> + { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, 
> CINTERION_PRODUCT_AHXX, 0xff) },
>   { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PLXX),
>   .driver_info = (kernel_ulong_t)_intf4_blacklist },
>   { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_HC28_MDM) }, 

Thanks for the patch.

Could you provide the output of lsusb -v for this device?

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

[PATCH] usb: dwc3: gadget: don't send extra ZLP

[John Youn]

If the request->length is zero, a ZLP should already be sent due to that
and another ZLP is not needed to terminate the transfer.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/gadget.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 64b2a83..af023a8 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1203,7 +1203,8 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct 
usb_request *request,
 * extra usb_request ourselves so that it gets handled the same way as
 * any other request.
 */
-   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 0))
+   if (ret == 0 && request->zero && request->length &&
+   (request->length % ep->maxpacket == 0))
ret = __dwc3_gadget_ep_queue_zlp(dwc, dep);
 
spin_unlock_irqrestore(>lock, flags);
-- 
2.6.3

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

Re: [PATCH v3 9/9] phy: omap-usb2: use *syscon* framework API to power on/off the PHY

[Rob Herring]

On Sun, Dec 20, 2015 at 6:07 AM, Kishon Vijay Abraham I  wrote:
> Hi Rob,
>
> On Sunday 20 December 2015 09:09 AM, Rob Herring wrote:
>> On Tue, Dec 15, 2015 at 02:46:08PM +0530, Kishon Vijay Abraham I wrote:
>>> Deprecate using phy-omap-control driver to power on/off the PHY,
>>> and use *syscon* framework to do the same. This handles
>>> powering on/off the PHY for the USB2 PHYs used in various TI SoCs.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I 
>>> ---
>>>  Documentation/devicetree/bindings/phy/ti-phy.txt |8 +-
>>>  drivers/phy/phy-omap-usb2.c  |   94 
>>> ++
>>>  include/linux/phy/omap_usb.h |   23 ++
>>>  3 files changed, 107 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt 
>>> b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> index 49e5b0c..a3b3945 100644
>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
>>> @@ -31,6 +31,8 @@ OMAP USB2 PHY
>>>
>>>  Required properties:
>>>   - compatible: Should be "ti,omap-usb2"
>>> +   Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
>>> +   in DRA7x
>>
>> The 2nd instance is different somehow?
>
> yeah, the bit fields are slightly different.

Okay,

Acked-by: Rob Herring 

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

Re: [PATCH v3] usb: dwc3: gadget: handle request->zero

[Alan Stern]

On Tue, 22 Dec 2015, John Youn wrote:

> >> The following fixes it:
> >>
> >> -   if (ret == 0 && request->zero && (request->length % ep->maxpacket 
> >> == 0))
> >> +   if (ret == 0 && request->zero && (request->length % ep->maxpacket 
> >> == 0) &&
> >> +   (request->length != 0))
> > 
> > Can you send this as a proper patch ?
> 
> Sure I can do that. I thought you might want fix it in place since
> it's in your testing/next.
> 
> > And also patch g_mass_storage to
> > _not_ set Zero flag in this case ?
> > 
> 
> The mass storage driver has always done that and I think it is ok. It
> sets this flag for the mass storage data IN phase and the data might
> be various lengths including zero-length.
> 
> It should be up to the controller to insert the ZLP as needed to
> terminate the transfer. And if the length is already short or zero,
> then there is no need to do so.
> 
> What do you think?

I agree with John.  In every case where we might want to insert a ZLP,
the class or gadget driver merely sets the flag for the transfer.  
It's up to the controller driver to check whether the conditions for
inserting the ZLP apply, and this includes checking whether the
transfer already has length 0.

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

[PATCH 2/2] USB: EHCI: add a delay when unlinking an active QH

[Alan Stern]

Michael Reutman reports that an AMD/ATI EHCI host controller on one of
his computers does not stop transferring data when an active bulk QH
is unlinked from the async schedule.  Apparently that host controller
fails to implement the IAA mechanism correctly when an active QH is
unlinked.  This leads to data corruption, because the controller
continues to update the QH in memory when the driver doesn't expect
it.  As a result the next URB submitted for that QH can hang, because
the link pointers for the TD queue have been messed up.  This
misbehavior is observed quite regularly.

To be fair, the EHCI spec (section 4.8.2) says that active QHs should
not be unlinked.  It goes on to recommend a procedure that involves
waiting for the QH to go inactive before unlinking it.  In the real
world this is impractical, not least because the QH may _never_ go
inactive.  (What were they thinking?)  Sometimes we have no choice but
to unlink an active QH.

In an attempt to avoid the problems that can ensue, this patch changes
how the driver decides when the unlink is complete.  In addition to
waiting through two IAA cycles, in cases where the QH was not known to
be inactive beforehand we now wait until a 2-ms period has elapsed
with the host controller making no change to the QH data structure
(the hw_current and hw_token fields in particular).  The intuition
here is that after such a long period, the endpoint must be NAKing and
hopefully the QH has been dropped from the host controller's internal
cache.  There's no way to know if this reasoning is really valid --
the spec is no help in this regard -- but at least this approach fixes
Michael's problem.

The test for whether the QH is already known to be inactive involves
the reason for unlinking the QH originally.  If it was unlinked
because it had halted, or it stopped in response to a short read, or
it overlaid a dummy TD (a silicon bug), then it certainly is inactive.
If it was unlinked because the TD queue was empty and no TDs have been
added to the queue in the meantime, then it must be inactive.  Or if
the hardware status indicates that the QH is currently halted (even if
that wasn't the reason for unlinking it), then it is inactive.
Otherwise, if none of those checks apply, we go through the 2-ms
delay.

Signed-off-by: Alan Stern 
Reported-by: Michael Reutman 
Tested-by: Michael Reutman 

---


[as1793]


 drivers/usb/host/ehci-hcd.c   |6 ++-
 drivers/usb/host/ehci-q.c |   73 +++---
 drivers/usb/host/ehci-timer.c |4 +-
 drivers/usb/host/ehci.h   |3 +
 4 files changed, 73 insertions(+), 13 deletions(-)

Index: usb-4.3/drivers/usb/host/ehci.h
===
--- usb-4.3.orig/drivers/usb/host/ehci.h
+++ usb-4.3/drivers/usb/host/ehci.h
@@ -110,6 +110,7 @@ enum ehci_hrtimer_event {
EHCI_HRTIMER_POLL_DEAD, /* Wait for dead controller to stop */
EHCI_HRTIMER_UNLINK_INTR,   /* Wait for interrupt QH unlink */
EHCI_HRTIMER_FREE_ITDS, /* Wait for unused iTDs and siTDs */
+   EHCI_HRTIMER_ACTIVE_UNLINK, /* Wait while unlinking an active QH */
EHCI_HRTIMER_START_UNLINK_INTR, /* Unlink empty interrupt QHs */
EHCI_HRTIMER_ASYNC_UNLINKS, /* Unlink empty async QHs */
EHCI_HRTIMER_IAA_WATCHDOG,  /* Handle lost IAA interrupts */
@@ -156,6 +157,8 @@ struct ehci_hcd {   /* one per controlle
struct list_headasync_idle;
unsignedasync_unlink_cycle;
unsignedasync_count;/* async activity count */
+   __hc32  old_current;/* Test for QH becoming */
+   __hc32  old_token;  /*  inactive during unlink */
 
/* periodic schedule support */
 #defineDEFAULT_I_TDPS  1024/* some HCs can do less 
*/
Index: usb-4.3/drivers/usb/host/ehci-hcd.c
===
--- usb-4.3.orig/drivers/usb/host/ehci-hcd.c
+++ usb-4.3/drivers/usb/host/ehci-hcd.c
@@ -306,6 +306,7 @@ static void ehci_quiesce (struct ehci_hc
 
 /*-*/
 
+static void end_iaa_cycle(struct ehci_hcd *ehci);
 static void end_unlink_async(struct ehci_hcd *ehci);
 static void unlink_empty_async(struct ehci_hcd *ehci);
 static void unlink_empty_async_suspended(struct ehci_hcd *ehci);
@@ -565,6 +566,9 @@ static int ehci_init(struct usb_hcd *hcd
/* Accept arbitrarily long scatter-gather lists */
if (!(hcd->driver->flags & HCD_LOCAL_MEM))
hcd->self.sg_tablesize = ~0;
+
+   /* Prepare for unlinking active QHs */
+   ehci->old_current = ~0;
return 0;
 }
 
@@ -758,7 +762,7 @@ static irqreturn_t ehci_irq (struct usb_
ehci_dbg(ehci, 

[PATCH 1/2] USB: EHCI: store reason for unlinking a QH

[Alan Stern]

This patch replaces the "exception" bitflag in the ehci_qh structure
with a more explicit "unlink_reason" bitmask.  This is for use in the
following patch, where we will need to have a good idea of the
reason for unlinking a QH, not just "something exceptional happened".

Signed-off-by: Alan Stern 
Tested-by: Michael Reutman 

---


[as1792]


 drivers/usb/host/ehci-hcd.c   |   11 +++
 drivers/usb/host/ehci-q.c |   13 +
 drivers/usb/host/ehci-sched.c |2 +-
 drivers/usb/host/ehci-timer.c |1 +
 drivers/usb/host/ehci.h   |   10 --
 5 files changed, 26 insertions(+), 11 deletions(-)

Index: usb-4.3/drivers/usb/host/ehci.h
===
--- usb-4.3.orig/drivers/usb/host/ehci.h
+++ usb-4.3/drivers/usb/host/ehci.h
@@ -432,13 +432,19 @@ struct ehci_qh {
u8  xacterrs;   /* XactErr retry counter */
 #defineQH_XACTERR_MAX  32  /* XactErr retry limit 
*/
 
+   u8  unlink_reason;
+#define QH_UNLINK_HALTED   0x01/* Halt flag is set */
+#define QH_UNLINK_SHORT_READ   0x02/* Recover from a short read */
+#define QH_UNLINK_DUMMY_OVERLAY0x04/* QH overlayed the 
dummy TD */
+#define QH_UNLINK_SHUTDOWN 0x08/* The HC isn't running */
+#define QH_UNLINK_QUEUE_EMPTY  0x10/* Reached end of the queue */
+#define QH_UNLINK_REQUESTED0x20/* Disable, reset, or dequeue */
+
u8  gap_uf; /* uframes split/csplit gap */
 
unsignedis_out:1;   /* bulk or intr OUT */
unsignedclearing_tt:1;  /* Clear-TT-Buf in progress */
unsigneddequeue_during_giveback:1;
-   unsignedexception:1;/* got a fault, or an unlink
-  was requested */
unsignedshould_be_inactive:1;
 };
 
Index: usb-4.3/drivers/usb/host/ehci-hcd.c
===
--- usb-4.3.orig/drivers/usb/host/ehci-hcd.c
+++ usb-4.3/drivers/usb/host/ehci-hcd.c
@@ -911,7 +911,7 @@ static int ehci_urb_dequeue(struct usb_h
 */
} else {
qh = (struct ehci_qh *) urb->hcpriv;
-   qh->exception = 1;
+   qh->unlink_reason |= QH_UNLINK_REQUESTED;
switch (qh->qh_state) {
case QH_STATE_LINKED:
if (usb_pipetype(urb->pipe) == PIPE_INTERRUPT)
@@ -972,10 +972,13 @@ rescan:
goto done;
}
 
-   qh->exception = 1;
+   qh->unlink_reason |= QH_UNLINK_REQUESTED;
switch (qh->qh_state) {
case QH_STATE_LINKED:
-   WARN_ON(!list_empty(>qtd_list));
+   if (list_empty(>qtd_list))
+   qh->unlink_reason |= QH_UNLINK_QUEUE_EMPTY;
+   else
+   WARN_ON(1);
if (usb_endpoint_type(>desc) != USB_ENDPOINT_XFER_INT)
start_unlink_async(ehci, qh);
else
@@ -1042,7 +1045,7 @@ ehci_endpoint_reset(struct usb_hcd *hcd,
 * re-linking will call qh_refresh().
 */
usb_settoggle(qh->ps.udev, epnum, is_out, 0);
-   qh->exception = 1;
+   qh->unlink_reason |= QH_UNLINK_REQUESTED;
if (eptype == USB_ENDPOINT_XFER_BULK)
start_unlink_async(ehci, qh);
else
Index: usb-4.3/drivers/usb/host/ehci-q.c
===
--- usb-4.3.orig/drivers/usb/host/ehci-q.c
+++ usb-4.3/drivers/usb/host/ehci-q.c
@@ -394,6 +394,7 @@ qh_completions (struct ehci_hcd *ehci, s
goto retry_xacterr;
}
stopped = 1;
+   qh->unlink_reason |= QH_UNLINK_HALTED;
 
/* magic dummy for some short reads; qh won't advance.
 * that silicon quirk can kick in with this dummy too.
@@ -408,6 +409,7 @@ qh_completions (struct ehci_hcd *ehci, s
&& !(qtd->hw_alt_next
& EHCI_LIST_END(ehci))) {
stopped = 1;
+   qh->unlink_reason |= QH_UNLINK_SHORT_READ;
}
 
/* stop scanning when we reach qtds the hc is using */
@@ -420,8 +422,10 @@ qh_completions (struct ehci_hcd *ehci, s
stopped = 1;
 
/* cancel everything if we halt, suspend, etc */
-   if (ehci->rh_state < 

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Felipe Balbi]

Hi,

Ramneek Mehresh  writes:
>> -Original Message-
>> From: Felipe Balbi [mailto:ba...@ti.com]
>> Sent: Saturday, October 10, 2015 3:04 AM
>> To: Mehresh Ramneek-B31383 ; linux-
>> ker...@vger.kernel.org
>> Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; linux-
>> u...@vger.kernel.org; Mehresh Ramneek-B31383
>> 
>> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
>> 
>> Felipe Balbi  writes:
>> 
>> > Hi,
>> >
>> > Ramneek Mehresh  writes:
>> >> Add support for otg for all freescale socs having internal usb phy.
>> >>
>> >> Ramneek Mehresh (7):
>> >>   usb:fsl:otg: Make fsl otg driver as tristate
>> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>> >>   usb:fsl:otg: Add support to add/remove usb host driver
>> >>   usb:fsl:otg: Signal host drv when host is otg
>> >>   usb:fsl:otg: Modify otg_event to start host drv
>> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>> >>   usb:fsl:otg: Add host-gadget drv sync delay
>> >
>> > Unless Alan's okay with the host side changes, I can't accept any of
>> > these. However, I must say some of the flags you add here already
>> > exist in some way, shape or form. For example, look at is_b_host flag.
>>
>
> Could you please be more specific...which flag you think that I should
>remove/I'm re-defining. The flags I'm defining are:
>
> have_hcd : defined in fsl specific structure for fsl specific use-case
>
> had_hcd: defined in fsl specific structure for fsl specific use-case
>
> is_otg : defined in include/linux/usb.h
>
> Are you suggesting using otg_port or is_b_host instead of is_otg?
>
> As I understand, is_b_host is specifically to check if an otg B device
> is in host mode...correct?  I just need a flag to check if a
> controller is capable of otg operations? That's why defined "is_otg"
> flag. Please suggest.

no, I don't know why I made that comment. You could use otg_port, but
that wouldn't look very clean. Can you resend with Alan's ack, then I'll
move this series into testing/next.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3] usb: dwc3: gadget: handle request->zero

[Felipe Balbi]

Hi,

John Youn  writes:
> On 12/22/2015 9:31 AM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> John Youn  writes:
>>> On 12/3/2015 7:18 AM, Felipe Balbi wrote:
 So far, dwc3 has always missed request->zero
 handling for every endpoint. Let's implement
 that so we can handle cases where transfer must
 be finished with a ZLP.

 Note that dwc3 is a little special. Even though
 we're dealing with a ZLP, we still need a buffer
 of wMaxPacketSize bytes; to hide that detail from
 every gadget driver, we have a preallocated buffer
 of 1024 bytes (biggest bulk size) to use (and
 share) among all endpoints.

 Reported-by: Ravi B 
 Signed-off-by: Felipe Balbi 
 ---

 since v1:
- remove unnecessary 'free_on_complete' flag

 since v2:
- remove unnecessary 'out' label

  drivers/usb/dwc3/core.h   |  3 +++
  drivers/usb/dwc3/gadget.c | 50 
 +--
  2 files changed, 51 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
 index 36f1cb74588c..29130682e547 100644
 --- a/drivers/usb/dwc3/core.h
 +++ b/drivers/usb/dwc3/core.h
 @@ -37,6 +37,7 @@
  #define DWC3_MSG_MAX  500
  
  /* Global constants */
 +#define DWC3_ZLP_BUF_SIZE 1024/* size of a superspeed bulk */
  #define DWC3_EP0_BOUNCE_SIZE  512
  #define DWC3_ENDPOINTS_NUM32
  #define DWC3_XHCI_RESOURCES_NUM   2
 @@ -647,6 +648,7 @@ struct dwc3_scratchpad_array {
   * @ctrl_req: usb control request which is used for ep0
   * @ep0_trb: trb which is used for the ctrl_req
   * @ep0_bounce: bounce buffer for ep0
 + * @zlp_buf: used when request->zero is set
   * @setup_buf: used while precessing STD USB requests
   * @ctrl_req_addr: dma address of ctrl_req
   * @ep0_trb: dma address of ep0_trb
 @@ -734,6 +736,7 @@ struct dwc3 {
struct usb_ctrlrequest  *ctrl_req;
struct dwc3_trb *ep0_trb;
void*ep0_bounce;
 +  void*zlp_buf;
void*scratchbuf;
u8  *setup_buf;
dma_addr_t  ctrl_req_addr;
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index e341f034296f..e916c11ded59 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -1158,6 +1158,32 @@ out:
return ret;
  }
  
 +static void __dwc3_gadget_ep_zlp_complete(struct usb_ep *ep,
 +  struct usb_request *request)
 +{
 +  dwc3_gadget_ep_free_request(ep, request);
 +}
 +
 +static int __dwc3_gadget_ep_queue_zlp(struct dwc3 *dwc, struct dwc3_ep 
 *dep)
 +{
 +  struct dwc3_request *req;
 +  struct usb_request  *request;
 +  struct usb_ep   *ep = >endpoint;
 +
 +  dwc3_trace(trace_dwc3_gadget, "queueing ZLP\n");
 +  request = dwc3_gadget_ep_alloc_request(ep, GFP_ATOMIC);
 +  if (!request)
 +  return -ENOMEM;
 +
 +  request->length = 0;
 +  request->buf = dwc->zlp_buf;
 +  request->complete = __dwc3_gadget_ep_zlp_complete;
 +
 +  req = to_dwc3_request(request);
 +
 +  return __dwc3_gadget_ep_queue(dep, req);
 +}
 +
  static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request 
 *request,
gfp_t gfp_flags)
  {
 @@ -1171,6 +1197,16 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, 
 struct usb_request *request,
  
spin_lock_irqsave(>lock, flags);
ret = __dwc3_gadget_ep_queue(dep, req);
 +
 +  /*
 +   * Okay, here's the thing, if gadget driver has requested for a ZLP by
 +   * setting request->zero, instead of doing magic, we will just queue an
 +   * extra usb_request ourselves so that it gets handled the same way as
 +   * any other request.
 +   */
 +  if (ret == 0 && request->zero && (request->length % ep->maxpacket == 0))
 +  ret = __dwc3_gadget_ep_queue_zlp(dwc, dep);
>>>
>>> Hi Felipe,
>>>
>>> This causes regression with at least mass storage + Windows host.
>>>
>>> When the gadget queues a ZLP, we end up sending two ZLPs which leads
>>> to violating the MSC protocol.
>> 
>> heh, no idea why mass storage would set Zero flag in this case :-p
>> 
>>> The following fixes it:
>>>
>>> -   if (ret == 0 && request->zero && (request->length % ep->maxpacket 
>>> == 0))
>>> +   if (ret == 0 && request->zero && (request->length % ep->maxpacket 
>>> == 0) &&
>>> +   (request->length != 0))
>> 
>> Can you send this as a proper patch ?
>
> Sure I can do that. I thought you might want fix it in place since
> it's in your testing/next.

it's already in next :-(

>> And also patch 

Re: [PATCH v5] extcon: add Maxim MAX3355 driver

[Chanwoo Choi]

On 2015년 12월 22일 20:15, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/22/2015 4:13 AM, Chanwoo Choi wrote:
> 
>> This patch depend on GPIOLIB configuration as following:
>> I modified it with following diff and applied it.
>>
>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
>> index ba4db7d..3d89e60 100644
>> --- a/drivers/extcon/Kconfig
>> +++ b/drivers/extcon/Kconfig
>> @@ -54,6 +54,7 @@ config EXTCON_MAX14577
>>
>> config EXTCON_MAX3355
>>tristate "Maxim MAX3355 USB OTG EXTCON Support"
>> +   depends on GPIOLIB || COMPILE_TEST
>
>  If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST?
>  And no, it shouldn't depend on gpiolib. It has empty stubs for the 
> case of CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO 
> headers, I'll look into it.

 Yes. When GPIOLIB is disabled, the build issue don't happen.
>>>
>>> What? It surely does happen!
>>
>> hmm
>> Sure. you need to check the include/linux/gpio/consumer.h.
>>
>> Because of build error happen, you miss to include the 
>> "linux/gpio/consumer.h"
>> header file in extcon-max3355.c. Please test it for enough time.
> 
>Yes, with this file #include'd, it build fine now.
> 
 because include/linux/gpio/consumer.h implement the dummy function
 for all gpio functions if CONFIG_GPIOLIB is disabled.
>>>
>>> Linus W. advised to #include this header explicitly -- I'll try and 
>>> post.
>>
>> Don't necessary. I already updated it including the 
>> "include/linux/gpio/consumer.h".
> 
>I saw that, yes.
> 
 For correct operation of max3355, you should add the dependency
 to the extcon-max3355.c driver. This driver use the GPIO library
 certainly.
>>>
>>> I disagree. The driver will just cease to load in this case. I don't 
>>> see why we need such dependency. Only compilation time dependencies should 
>>> be
>>> specified, I think.
>>
>> This driver have to depend on GPIOLIB.
>> Why are you disagreeing the COMPILE_TEST dependency? It is just compile test
>> without anything.
> 
>I agree now. I still disagree about the gpiolib dependency though.

If gpiolib is disabled, extcon-max3355.c might not operate it correctly.
Just this driver could be built without operation because gpiolib function
will not do the any behavior.

I think that it is not too much problem. I should send the pull request within 
this week.
If you want to need more discussion of extcon-max3355.c,
I will not include it on pull request for v4.5 because there is issue.



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

libusbg-neXt (libusbgx) v0.1.0 finally released!

[Krzysztof Opasiak]

Hello,

This is just a note to let you know that v0.1.0 of libusbg-neXt[1] has 
been just released.


In the beginning the library was developed as a github fork[2] of 
libusbg[3] with many patches sent to Matt and pull requests to mianline 
repository. Unfortunately libusbg project seems to be (at least for me) 
dead as last commit has been merged at 20th Sep 2014. That's why project 
has been forked as libusbg-neXt or shorter libusbgx.


As I see this project more like a continuation of libusbg rather than a 
real fork, all commits which has been merged to mainline or my github 
fork of libusbg has been also included in libusbgx.


Some key features of libusbgx-v0.1.0 in comparison to libusbg mainline 
state:

- Add dedicated structure for udc
- Add test suit and 77 test cases
- Introduce internal API to make adding support for new functions easier
- Rework API to be more usable and eliminate most of static buffers
- Add support for MIDI function
- Add support for Mass Storage function
- Add support for Loopback function

I encourage you to check out libusbgx and contribute!

Footnotes:
1 - https://github.com/libusbgx/libusbgx
2 - https://github.com/kopasiak/libusbg
3 - https://github.com/libusbg/libusbg

Cheers,
--
Krzysztof Opasiak
Samsung R Institute Poland
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: [PATCH v3] usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller

[Rob Herring]

On Mon, Dec 21, 2015 at 06:40:04PM +0900, Yoshihiro Shimoda wrote:
> R-Car H3 has USB3.0 peripheral controllers. This controller's has the
> following features:
>  - Supports super, high and full speed
>  - Contains 30 pipes for bulk or interrupt transfer
>  - Contains dedicated DMA controller
> 
> This driver doesn't support the dedicated DMAC for now.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  This patch is based on the latest Felipe's usb.git / testing/next branch.
>  (commit id = e9284de9fae69f1d5e57a4817bfc36dc5f3adf71)
> 
>  Changes from v2:
>   - Fix compatible string order to "-".
> 
>  Changes from v1:
>   - fix build error in i386 environment if COMPILE_TEST=y
>   - merge the header file code into the .c file
>   - revise the device tree document about "clocks"
>   - remove prototype declarations
>   - use udelay(1) instead of ndelay(1) in usb3_wait()
>   - remove empty function "usb3_init_phy()"
>   - use module_platform_driver()
>   - remove bit fields member in some structures
> 
> 
>  .../devicetree/bindings/usb/renesas_usb3.txt   |   23 +

For the binding:

Acked-by: Rob Herring 

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

[PATCH v2 3/3 RESEND] USB: serial: cp210x: New register access functions for large registers

[Konstantin Shkolnyy]

cp210x_get_config and cp210x_set_config are cumbersome to use. This change
switches large register access to use new block functions. The old
functions are removed because now they become unused.

Signed-off-by: Konstantin Shkolnyy 
---
 drivers/usb/serial/cp210x.c | 137 
 1 file changed, 24 insertions(+), 113 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 0c2273d..ce80d5f 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -323,105 +323,6 @@ struct cp210x_comm_status {
 #define PURGE_ALL  0x000f
 
 /*
- * cp210x_get_config
- * Reads from the CP210x configuration registers
- * 'size' is specified in bytes.
- * 'data' is a pointer to a pre-allocated array of integers large
- * enough to hold 'size' bytes (with 4 bytes to each integer)
- */
-static int cp210x_get_config(struct usb_serial_port *port, u8 request,
-   unsigned int *data, int size)
-{
-   struct usb_serial *serial = port->serial;
-   struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
-   __le32 *buf;
-   int result, i, length;
-
-   /* Number of integers required to contain the array */
-   length = (((size - 1) | 3) + 1) / 4;
-
-   buf = kcalloc(length, sizeof(__le32), GFP_KERNEL);
-   if (!buf)
-   return -ENOMEM;
-
-   /* Issue the request, attempting to read 'size' bytes */
-   result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
-   request, REQTYPE_INTERFACE_TO_HOST, 0x,
-   port_priv->bInterfaceNumber, buf, size,
-   USB_CTRL_GET_TIMEOUT);
-
-   /* Convert data into an array of integers */
-   for (i = 0; i < length; i++)
-   data[i] = le32_to_cpu(buf[i]);
-
-   kfree(buf);
-
-   if (result != size) {
-   dev_dbg(>dev, "%s - Unable to send config request, 
request=0x%x size=%d result=%d\n",
-   __func__, request, size, result);
-   if (result > 0)
-   result = -EPROTO;
-
-   return result;
-   }
-
-   return 0;
-}
-
-/*
- * cp210x_set_config
- * Writes to the CP210x configuration registers
- * Values less than 16 bits wide are sent directly
- * 'size' is specified in bytes.
- */
-static int cp210x_set_config(struct usb_serial_port *port, u8 request,
-   unsigned int *data, int size)
-{
-   struct usb_serial *serial = port->serial;
-   struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
-   __le32 *buf;
-   int result, i, length;
-
-   /* Number of integers required to contain the array */
-   length = (((size - 1) | 3) + 1) / 4;
-
-   buf = kmalloc(length * sizeof(__le32), GFP_KERNEL);
-   if (!buf)
-   return -ENOMEM;
-
-   /* Array of integers into bytes */
-   for (i = 0; i < length; i++)
-   buf[i] = cpu_to_le32(data[i]);
-
-   if (size > 2) {
-   result = usb_control_msg(serial->dev,
-   usb_sndctrlpipe(serial->dev, 0),
-   request, REQTYPE_HOST_TO_INTERFACE, 0x,
-   port_priv->bInterfaceNumber, buf, size,
-   USB_CTRL_SET_TIMEOUT);
-   } else {
-   result = usb_control_msg(serial->dev,
-   usb_sndctrlpipe(serial->dev, 0),
-   request, REQTYPE_HOST_TO_INTERFACE, data[0],
-   port_priv->bInterfaceNumber, NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
-   }
-
-   kfree(buf);
-
-   if ((size > 2 && result != size) || result < 0) {
-   dev_dbg(>dev, "%s - Unable to send request, request=0x%x 
size=%d result=%d\n",
-   __func__, request, size, result);
-   if (result > 0)
-   result = -EPROTO;
-
-   return result;
-   }
-
-   return 0;
-}
-
-/*
  * Reads a variable-sized block of CP210X_ registers, identified by req.
  * Returns data into buf in native USB byte order.
  */
@@ -786,7 +687,8 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
unsigned int *cflagp, unsigned int *baudp)
 {
struct device *dev = >dev;
-   unsigned int cflag, modem_ctl[4];
+   unsigned int cflag;
+   u8 modem_ctl[16];
u32 baud;
u16 bits;
 
@@ -884,8 +786,9 @@ static void cp210x_get_termios_port(struct usb_serial_port 
*port,
break;
}
 
-   cp210x_get_config(port, CP210X_GET_FLOW, modem_ctl, 16);
-   if (modem_ctl[0] & 0x0008) {
+   cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
+   sizeof(modem_ctl));
+   if (modem_ctl[0] & 8) {

[PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register access functions.

[Konstantin Shkolnyy]

cp210x_get_config and cp210x_set_config are cumbersome to use. This change
introduces new register access functions for 16-bit values, instead of
the above functions.

Signed-off-by: Konstantin Shkolnyy 
---
 drivers/usb/serial/cp210x.c | 155 +++-
 1 file changed, 111 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fd67958..fd7c4f4 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -422,14 +422,88 @@ static int cp210x_set_config(struct usb_serial_port 
*port, u8 request,
 }
 
 /*
- * cp210x_set_config_single
- * Convenience function for calling cp210x_set_config on single data values
- * without requiring an integer pointer
+ * Reads a variable-sized block of CP210X_ registers, identified by req.
+ * Returns data into buf in native USB byte order.
  */
-static inline int cp210x_set_config_single(struct usb_serial_port *port,
-   u8 request, unsigned int data)
+static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req,
+   void *buf, int bufsize)
 {
-   return cp210x_set_config(port, request, , 2);
+   struct usb_serial *serial = port->serial;
+   struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+   void *dmabuf;
+   int result;
+
+   dmabuf = kmalloc(bufsize, GFP_KERNEL);
+   if (!dmabuf) {
+   /*
+* FIXME Some callers don't bother to check for error,
+* at least give them consistent junk until they are fixed
+*/
+   memset(buf, 0, bufsize);
+   return -ENOMEM;
+   }
+
+   result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+   req, REQTYPE_INTERFACE_TO_HOST, 0,
+   port_priv->bInterfaceNumber, dmabuf, bufsize,
+   USB_CTRL_SET_TIMEOUT);
+   if (result == bufsize) {
+   memcpy(buf, dmabuf, bufsize);
+   result = 0;
+   } else {
+   dev_err(>dev, "failed get req 0x%x size %d status: %d\n",
+   req, bufsize, result);
+   if (result >= 0)
+   result = -EPROTO;
+
+   /*
+* FIXME Some callers don't bother to check for error,
+* at least give them consistent junk until they are fixed
+*/
+   memset(buf, 0, bufsize);
+   }
+
+   kfree(dmabuf);
+
+   return result;
+}
+
+/*
+ * Reads any 16-bit CP210X_ register identified by req.
+ */
+static int cp210x_read_u16_reg(struct usb_serial_port *port, u8 req, u16 *val)
+{
+   __le16 le16_val;
+   int err;
+
+   err = cp210x_read_reg_block(port, req, _val, sizeof(le16_val));
+   if (err)
+   return err;
+
+   *val = le16_to_cpu(le16_val);
+   return 0;
+}
+
+/*
+ * Writes any 16-bit CP210X_ register (req) whose value is passed
+ * entirely in the wValue field of the USB request.
+ */
+static int cp210x_write_u16_reg(struct usb_serial_port *port, u8 req, u16 val)
+{
+   struct usb_serial *serial = port->serial;
+   struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+   int result;
+
+   result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+   req, REQTYPE_HOST_TO_INTERFACE, val,
+   port_priv->bInterfaceNumber, NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   if (result < 0) {
+   dev_err(>dev, "failed set request 0x%x status: %d\n",
+   req, result);
+   }
+
+   return result;
 }
 
 /*
@@ -441,48 +515,47 @@ static inline int cp210x_set_config_single(struct 
usb_serial_port *port,
 static int cp210x_detect_swapped_line_ctl(struct usb_serial_port *port)
 {
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
-   unsigned int line_ctl_save;
-   unsigned int line_ctl_test;
+   u16 line_ctl_save;
+   u16 line_ctl_test;
int err;
 
-   err = cp210x_get_config(port, CP210X_GET_LINE_CTL, _ctl_save, 2);
+   err = cp210x_read_u16_reg(port, CP210X_GET_LINE_CTL, _ctl_save);
if (err)
return err;
 
-   line_ctl_test = 0x800;
-   err = cp210x_set_config(port, CP210X_SET_LINE_CTL, _ctl_test, 2);
+   err = cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, 0x800);
if (err)
return err;
 
-   err = cp210x_get_config(port, CP210X_GET_LINE_CTL, _ctl_test, 2);
+   err = cp210x_read_u16_reg(port, CP210X_GET_LINE_CTL, _ctl_test);
if (err)
return err;
 
/* has_swapped_line_ctl is 0 here because port_priv was kzalloced */
if (line_ctl_test == 8) {
port_priv->has_swapped_line_ctl = true;
-   line_ctl_save = 

[PATCH v2 2/3 RESEND] USB: serial: cp210x: New 8-bit and 32-bit register access functions.

[Konstantin Shkolnyy]

cp210x_get_config and cp210x_set_config are cumbersome to use. This change
introduces new register access functions for 8 and 32-bit values, instead
of the above functions.

Signed-off-by: Konstantin Shkolnyy 
---
 drivers/usb/serial/cp210x.c | 92 ++---
 1 file changed, 86 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fd7c4f4..0c2273d 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -469,6 +469,29 @@ static int cp210x_read_reg_block(struct usb_serial_port 
*port, u8 req,
 }
 
 /*
+ * Reads any 32-bit CP210X_ register identified by req.
+ */
+static int cp210x_read_u32_reg(struct usb_serial_port *port, u8 req, u32 *val)
+{
+   __le32 le32_val;
+   int err;
+
+   err = cp210x_read_reg_block(port, req, _val, sizeof(le32_val));
+   if (err) {
+   /*
+* FIXME Some callers don't bother to check for error,
+* at least give them consistent junk until they are fixed
+*/
+   *val = 0;
+   return err;
+   }
+
+   *val = le32_to_cpu(le32_val);
+
+   return 0;
+}
+
+/*
  * Reads any 16-bit CP210X_ register identified by req.
  */
 static int cp210x_read_u16_reg(struct usb_serial_port *port, u8 req, u16 *val)
@@ -481,10 +504,19 @@ static int cp210x_read_u16_reg(struct usb_serial_port 
*port, u8 req, u16 *val)
return err;
 
*val = le16_to_cpu(le16_val);
+
return 0;
 }
 
 /*
+ * Reads any 8-bit CP210X_ register identified by req.
+ */
+static int cp210x_read_u8_reg(struct usb_serial_port *port, u8 req, u8 *val)
+{
+   return cp210x_read_reg_block(port, req, val, sizeof(*val));
+}
+
+/*
  * Writes any 16-bit CP210X_ register (req) whose value is passed
  * entirely in the wValue field of the USB request.
  */
@@ -507,6 +539,55 @@ static int cp210x_write_u16_reg(struct usb_serial_port 
*port, u8 req, u16 val)
 }
 
 /*
+ * Writes a variable-sized block of CP210X_ registers, identified by req.
+ * Data in buf must be in native USB byte order.
+ */
+static int cp210x_write_reg_block(struct usb_serial_port *port, u8 req,
+   void *buf, int bufsize)
+{
+   struct usb_serial *serial = port->serial;
+   struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+   void *dmabuf;
+   int result;
+
+   dmabuf = kmalloc(bufsize, GFP_KERNEL);
+   if (!dmabuf)
+   return -ENOMEM;
+
+   memcpy(dmabuf, buf, bufsize);
+
+   result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+   req, REQTYPE_HOST_TO_INTERFACE, 0,
+   port_priv->bInterfaceNumber, dmabuf, bufsize,
+   USB_CTRL_SET_TIMEOUT);
+
+   kfree(dmabuf);
+
+   if (result == bufsize) {
+   result = 0;
+   } else {
+   dev_err(>dev, "failed set req 0x%x size %d status: %d\n",
+   req, bufsize, result);
+   if (result >= 0)
+   result = -EPROTO;
+   }
+
+   return result;
+}
+
+/*
+ * Writes any 32-bit CP210X_ register identified by req.
+ */
+static int cp210x_write_u32_reg(struct usb_serial_port *port, u8 req, u32 val)
+{
+   __le32 le32_val;
+
+   le32_val = cpu_to_le32(val);
+
+   return cp210x_write_reg_block(port, req, _val, sizeof(le32_val));
+}
+
+/*
  * Detect CP2108 GET_LINE_CTL bug and activate workaround.
  * Write a known good value 0x800, read it back.
  * If it comes back swapped the bug is detected.
@@ -706,10 +787,10 @@ static void cp210x_get_termios_port(struct 
usb_serial_port *port,
 {
struct device *dev = >dev;
unsigned int cflag, modem_ctl[4];
-   unsigned int baud;
+   u32 baud;
u16 bits;
 
-   cp210x_get_config(port, CP210X_GET_BAUDRATE, , 4);
+   cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, );
 
dev_dbg(dev, "%s - baud rate = %d\n", __func__, baud);
*baudp = baud;
@@ -856,8 +937,7 @@ static void cp210x_change_speed(struct tty_struct *tty,
baud = cp210x_quantise_baudrate(baud);
 
dev_dbg(>dev, "%s - setting baud rate to %u\n", __func__, baud);
-   if (cp210x_set_config(port, CP210X_SET_BAUDRATE, ,
-   sizeof(baud))) {
+   if (cp210x_write_u32_reg(port, CP210X_SET_BAUDRATE, baud)) {
dev_warn(>dev, "failed to set baud rate to %u\n", baud);
if (old_termios)
baud = old_termios->c_ospeed;
@@ -1028,10 +1108,10 @@ static void cp210x_dtr_rts(struct usb_serial_port *p, 
int on)
 static int cp210x_tiocmget(struct tty_struct *tty)
 {
struct usb_serial_port *port = tty->driver_data;
-   unsigned int control;
+   u8 control;
int result;
 
-   cp210x_get_config(port, CP210X_GET_MDMSTS, , 1);
+   

Re: [patch] usb: gadget: f_midi: missing unlock on error path

[SF Markus Elfring]

> We added a new error path to this function and we forgot to drop the lock.
> 
> Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index fb1fe96d..92b9ec8 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1213,8 +1213,10 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>   midi->in_last_port = 0;
>  
>   status = kfifo_alloc(>in_req_fifo, midi->qlen, GFP_KERNEL);
> - if (status)
> + if (status) {
> + mutex_unlock(>lock);
>   goto setup_fail;
> + }
>  
>   ++opts->refcnt;
>   mutex_unlock(>lock);

How do you think about to move this unlock call and the one that belongs to
the handling of a kstrdup() failure below the shown jump target?

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

Re: [PATCH 2/3] usb: phy-generic: register a struct phy device

[Uwe Kleine-König]

On Tue, Dec 22, 2015 at 12:06:14PM -0600, Felipe Balbi wrote:
> Uwe Kleine-König  writes:
> 
> > This is needed to let the omap3 otg controller make use of a generic
> > usb-nop-xceiv phy.
> 
> missing SoB

If this is your only concern I will happily resend the series with an
SoB and some dt-binding documentation for patch 3/3.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: phy-generic: register a struct phy device

[Felipe Balbi]

Hi,

Uwe Kleine-König  writes:
> On Tue, Dec 22, 2015 at 12:06:14PM -0600, Felipe Balbi wrote:
>> Uwe Kleine-König  writes:
>> 
>> > This is needed to let the omap3 otg controller make use of a generic
>> > usb-nop-xceiv phy.
>> 
>> missing SoB
>
> If this is your only concern I will happily resend the series with an
> SoB and some dt-binding documentation for patch 3/3.

mostly, yeah. I'm still considering your use of container_of() to fetch
the parent's pdev. Why do you need to go through all those hoops ?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH 3/3] usb: r8a66597: add locking to r8a66597_check_detect_child

[Heiner Kallweit]

Use mutex usb_bus_idr_lock to protect idr_find.

Signed-off-by: Heiner Kallweit 
---
 drivers/usb/host/r8a66597-hcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 1ef8873..bfa7fa3 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2099,11 +2099,13 @@ static void r8a66597_check_detect_child(struct r8a66597 
*r8a66597,
 
memset(now_map, 0, sizeof(now_map));
 
+   mutex_lock(_bus_idr_lock);
bus = idr_find(_bus_idr, hcd->self.busnum);
if (bus && bus->root_hub) {
collect_usb_address_map(bus->root_hub, now_map);
update_usb_address_map(r8a66597, bus->root_hub, now_map);
}
+   mutex_unlock(_bus_idr_lock);
 }
 
 static int r8a66597_hub_status_data(struct usb_hcd *hcd, char *buf)
-- 
2.6.4


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

[PATCH 1/3] usb: core: switch bus numbering to using idr

[Heiner Kallweit]

USB bus numbering is based on directly dealing with bitmaps and
defines a separate list of busses.
This can be simplified and unified by using existing idr functionality.

Signed-off-by: Heiner Kallweit 
---
 drivers/usb/core/devices.c  | 10 ++
 drivers/usb/core/hcd.c  | 21 ++---
 drivers/usb/core/usb.c  |  1 +
 drivers/usb/host/r8a66597-hcd.c |  9 ++---
 drivers/usb/mon/mon_main.c  |  5 ++---
 include/linux/usb.h |  1 -
 include/linux/usb/hcd.h |  3 ++-
 7 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index 2a3bbdf..f4c5962 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -110,13 +110,6 @@ static const char format_endpt[] =
 /* E:  Ad=xx(s) Atr=xx() MxPS= Ivl=D?s */
   "E:  Ad=%02x(%c) Atr=%02x(%-4s) MxPS=%4d Ivl=%d%cs\n";
 
-
-/*
- * Need access to the driver and USB bus lists.
- * extern struct list_head usb_bus_list;
- * However, these will come from functions that return ptrs to each of them.
- */
-
 /*
  * Wait for an connect/disconnect event to happen. We initialize
  * the event counter with an odd number, and each event will increment
@@ -616,6 +609,7 @@ static ssize_t usb_device_read(struct file *file, char 
__user *buf,
struct usb_bus *bus;
ssize_t ret, total_written = 0;
loff_t skip_bytes = *ppos;
+   int id;
 
if (*ppos < 0)
return -EINVAL;
@@ -626,7 +620,7 @@ static ssize_t usb_device_read(struct file *file, char 
__user *buf,
 
mutex_lock(_bus_list_lock);
/* print devices for all busses */
-   list_for_each_entry(bus, _bus_list, bus_list) {
+   idr_for_each_entry(_bus_idr, bus, id) {
/* recurse through all children of the root hub */
if (!bus_to_hcd(bus)->rh_registered)
continue;
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 1c102d6..08f5ea5 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -90,12 +90,11 @@ unsigned long usb_hcds_loaded;
 EXPORT_SYMBOL_GPL(usb_hcds_loaded);
 
 /* host controllers we manage */
-LIST_HEAD (usb_bus_list);
-EXPORT_SYMBOL_GPL (usb_bus_list);
+DEFINE_IDR (usb_bus_idr);
+EXPORT_SYMBOL_GPL (usb_bus_idr);
 
 /* used when allocating bus numbers */
 #define USB_MAXBUS 64
-static DECLARE_BITMAP(busmap, USB_MAXBUS);
 
 /* used when updating list of hcds */
 DEFINE_MUTEX(usb_bus_list_lock);   /* exported only for usbfs */
@@ -967,8 +966,6 @@ static void usb_bus_init (struct usb_bus *bus)
bus->bandwidth_int_reqs  = 0;
bus->bandwidth_isoc_reqs = 0;
mutex_init(>usb_address0_mutex);
-
-   INIT_LIST_HEAD (>bus_list);
 }
 
 /*-*/
@@ -989,16 +986,12 @@ static int usb_register_bus(struct usb_bus *bus)
int busnum;
 
mutex_lock(_bus_list_lock);
-   busnum = find_next_zero_bit(busmap, USB_MAXBUS, 1);
-   if (busnum >= USB_MAXBUS) {
-   printk (KERN_ERR "%s: too many buses\n", usbcore_name);
+   busnum = idr_alloc(_bus_idr, bus, 1, USB_MAXBUS, GFP_KERNEL);
+   if (busnum < 0) {
+   pr_err("%s: failed to get bus number\n", usbcore_name);
goto error_find_busnum;
}
-   set_bit(busnum, busmap);
bus->busnum = busnum;
-
-   /* Add it to the local list of buses */
-   list_add (>bus_list, _bus_list);
mutex_unlock(_bus_list_lock);
 
usb_notify_add_bus(bus);
@@ -1030,12 +1023,10 @@ static void usb_deregister_bus (struct usb_bus *bus)
 * itself up
 */
mutex_lock(_bus_list_lock);
-   list_del (>bus_list);
+   idr_remove(_bus_idr, bus->busnum);
mutex_unlock(_bus_list_lock);
 
usb_notify_remove_bus(bus);
-
-   clear_bit(bus->busnum, busmap);
 }
 
 /**
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index f8bbd0b..17f8230 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1114,6 +1114,7 @@ static void __exit usb_exit(void)
bus_unregister(_bus_type);
usb_acpi_unregister();
usb_debugfs_cleanup();
+   idr_destroy(_bus_idr);
 }
 
 subsys_initcall(usb_init);
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 4cbd063..1ef8873 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2099,13 +2099,8 @@ static void r8a66597_check_detect_child(struct r8a66597 
*r8a66597,
 
memset(now_map, 0, sizeof(now_map));
 
-   list_for_each_entry(bus, _bus_list, bus_list) {
-   if (!bus->root_hub)
-   continue;
-
-   if (bus->busnum != hcd->self.busnum)
-   continue;
-
+   bus = idr_find(_bus_idr, hcd->self.busnum);
+   if (bus && bus->root_hub) {

[PATCH 2/3] usb: core: rename mutex usb_bus_list_lock to usb_bus_idr_lock

[Heiner Kallweit]

Now that usb_bus_list has been removed and switched to idr
rename the related mutex accordingly.

Signed-off-by: Heiner Kallweit 
---
 drivers/usb/core/devices.c |  6 +++---
 drivers/usb/core/hcd.c | 30 +++---
 drivers/usb/core/hub.c |  4 ++--
 drivers/usb/mon/mon_main.c |  4 ++--
 include/linux/usb/hcd.h|  2 +-
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index f4c5962..1f37a18 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -618,7 +618,7 @@ static ssize_t usb_device_read(struct file *file, char 
__user *buf,
if (!access_ok(VERIFY_WRITE, buf, nbytes))
return -EFAULT;
 
-   mutex_lock(_bus_list_lock);
+   mutex_lock(_bus_idr_lock);
/* print devices for all busses */
idr_for_each_entry(_bus_idr, bus, id) {
/* recurse through all children of the root hub */
@@ -629,12 +629,12 @@ static ssize_t usb_device_read(struct file *file, char 
__user *buf,
  bus->root_hub, bus, 0, 0, 0);
usb_unlock_device(bus->root_hub);
if (ret < 0) {
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
return ret;
}
total_written += ret;
}
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
return total_written;
 }
 
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 08f5ea5..dc52405 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -97,8 +97,8 @@ EXPORT_SYMBOL_GPL (usb_bus_idr);
 #define USB_MAXBUS 64
 
 /* used when updating list of hcds */
-DEFINE_MUTEX(usb_bus_list_lock);   /* exported only for usbfs */
-EXPORT_SYMBOL_GPL (usb_bus_list_lock);
+DEFINE_MUTEX(usb_bus_idr_lock);/* exported only for usbfs */
+EXPORT_SYMBOL_GPL (usb_bus_idr_lock);
 
 /* used for controlling access to virtual root hubs */
 static DEFINE_SPINLOCK(hcd_root_hub_lock);
@@ -985,14 +985,14 @@ static int usb_register_bus(struct usb_bus *bus)
int result = -E2BIG;
int busnum;
 
-   mutex_lock(_bus_list_lock);
+   mutex_lock(_bus_idr_lock);
busnum = idr_alloc(_bus_idr, bus, 1, USB_MAXBUS, GFP_KERNEL);
if (busnum < 0) {
pr_err("%s: failed to get bus number\n", usbcore_name);
goto error_find_busnum;
}
bus->busnum = busnum;
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
 
usb_notify_add_bus(bus);
 
@@ -1001,7 +1001,7 @@ static int usb_register_bus(struct usb_bus *bus)
return 0;
 
 error_find_busnum:
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
return result;
 }
 
@@ -1022,9 +1022,9 @@ static void usb_deregister_bus (struct usb_bus *bus)
 * controller code, as well as having it call this when cleaning
 * itself up
 */
-   mutex_lock(_bus_list_lock);
+   mutex_lock(_bus_idr_lock);
idr_remove(_bus_idr, bus->busnum);
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
 
usb_notify_remove_bus(bus);
 }
@@ -1054,12 +1054,12 @@ static int register_root_hub(struct usb_hcd *hcd)
set_bit (devnum, usb_dev->bus->devmap.devicemap);
usb_set_device_state(usb_dev, USB_STATE_ADDRESS);
 
-   mutex_lock(_bus_list_lock);
+   mutex_lock(_bus_idr_lock);
 
usb_dev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
if (retval != sizeof usb_dev->descriptor) {
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
dev_dbg (parent_dev, "can't read %s device descriptor %d\n",
dev_name(_dev->dev), retval);
return (retval < 0) ? retval : -EMSGSIZE;
@@ -1070,7 +1070,7 @@ static int register_root_hub(struct usb_hcd *hcd)
if (!retval) {
usb_dev->lpm_capable = usb_device_supports_lpm(usb_dev);
} else if (usb_dev->speed == USB_SPEED_SUPER) {
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
dev_dbg(parent_dev, "can't read %s bos descriptor %d\n",
dev_name(_dev->dev), retval);
return retval;
@@ -1090,7 +1090,7 @@ static int register_root_hub(struct usb_hcd *hcd)
if (HCD_DEAD(hcd))
usb_hc_died (hcd);  /* This time clean up */
}
-   mutex_unlock(_bus_list_lock);
+   mutex_unlock(_bus_idr_lock);
 
return retval;
 }
@@ -2854,9 +2854,9 @@ error_create_attr_group:
 #ifdef CONFIG_PM

Re: [PATCH 2/3] usb: phy-generic: register a struct phy device

[Felipe Balbi]

Uwe Kleine-König  writes:

> This is needed to let the omap3 otg controller make use of a generic
> usb-nop-xceiv phy.

missing SoB

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 8/9] usb: gadget: rndis: use list_for_each_entry_safe

[Felipe Balbi]

Hi,

Geliang Tang  writes:
> Use list_for_each_entry_safe() instead of list_for_each_safe() to
> simplify the code.
>
> Signed-off-by: Geliang Tang 

there are other cleanups in this patch which shouldn't be here. Please
split and I'll apply for v4.6 merge window.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: rndis: fix itnull.cocci warnings

[Felipe Balbi]

Julia Lawall  writes:

> The index variable of list_for_each_entry_safe is never NULL.
>
> Generated by: scripts/coccinelle/iterators/itnull.cocci
>
> CC: Geliang Tang 
> Signed-off-by: Fengguang Wu 
> Signed-off-by: Julia Lawall 

doesn't apply. Does this depend on anything ?

-- 
balbi


signature.asc
Description: PGP signature

MX28 with hub cannot reset

[Fabio Estevam]

Hi,

On a mx28 custom board with a USB hub I can get a USB stick to get
detected after applying this change:
http://marc.info/?l=linux-usb=145079935324339=2

Things work fine if I boot without the USB stick connected. Then I can
insert/remove the USB device and it always gets detected.

However, if I boot with the USB stick connected I get:

[2.968513] hub 1-1:1.0: USB hub found
[2.973096] hub 1-1:1.0: 2 ports detected
[3.479731] usb 1-1: USB disconnect, device number 2
[3.525403] usb usb1-port1: cannot reset (err = -32)
[3.530828] usb usb1-port1: cannot reset (err = -32)
[3.536089] usb usb1-port1: cannot reset (err = -32)
[3.541600] usb usb1-port1: cannot reset (err = -32)
[3.546857] usb usb1-port1: cannot reset (err = -32)
[3.551982] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.560302] usb usb1-port1: cannot reset (err = -32)
[3.565566] usb usb1-port1: cannot reset (err = -32)
[3.571011] usb usb1-port1: cannot reset (err = -32)
[3.576266] usb usb1-port1: cannot reset (err = -32)
[3.581769] usb usb1-port1: cannot reset (err = -32)
[3.586780] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.594138] usb usb1-port1: cannot reset (err = -32)
[3.599615] usb usb1-port1: cannot reset (err = -32)
[3.604871] usb usb1-port1: cannot reset (err = -32)
[3.610244] usb usb1-port1: cannot reset (err = -32)
[3.615499] usb usb1-port1: cannot reset (err = -32)
[3.620649] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.627989] usb usb1-port1: cannot reset (err = -32)
[3.633370] usb usb1-port1: cannot reset (err = -32)
[3.638617] usb usb1-port1: cannot reset (err = -32)
[3.644171] usb usb1-port1: cannot reset (err = -32)
[3.649546] usb usb1-port1: cannot reset (err = -32)
[3.654557] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.661650] usb usb1-port1: unable to enumerate USB device

And the only way to get the USB port working again is to remove the
USB stick and reboot the board.

Commit d0ee68b59e6aa33221 ("usb: phy: mxs: Add VF610 USB PHY support")
shows the same error messages in the commit log, but trying the same
approach of adding the MXS_PHY_NEED_IP_FIX flag does not help on mx28.

Any suggestions?

Thanks,

Fabio Estevam
--
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] r8152: fix lockup when runtime PM is enabled

[Hayes Wang]

Oliver Neukum [mailto:oneu...@suse.com]
[...]
> It is clear to me that you cannot get away with using the same operation
> for resume() and reset_resume() in your driver. It is fundamentally
> impossible. Firmware cannot fix it.

I would think how to fix it.

> Sorry for the length of the explanation.

Thanks for your response. I have some questions. What are the flows when
the system resume follows a system suspend which follows a autosuspend?
Are they as following?

1. suspend() with PMSG_IS_AUTO for autosuspned.
2. suspend() for system suspend.
3. resume() for system resume.

And, should the device exist autosuspend before (2)? 

Best Regards,
Hayes

RE: [PATCH 0/7][v4] Add OTG support for FSL socs

[Jun Li]

Hi

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Felipe Balbi
> Sent: Wednesday, December 23, 2015 2:21 AM
> To: Ramneek Mehresh ; linux-
> ker...@vger.kernel.org
> Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; linux-
> u...@vger.kernel.org
> Subject: RE: [PATCH 0/7][v4] Add OTG support for FSL socs
> 
> 
> Hi,
> 
> Ramneek Mehresh  writes:
> >> -Original Message-
> >> From: Felipe Balbi [mailto:ba...@ti.com]
> >> Sent: Saturday, October 10, 2015 3:04 AM
> >> To: Mehresh Ramneek-B31383 ; linux-
> >> ker...@vger.kernel.org
> >> Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; linux-
> >> u...@vger.kernel.org; Mehresh Ramneek-B31383
> >> 
> >> Subject: Re: [PATCH 0/7][v4] Add OTG support for FSL socs
> >>
> >> Felipe Balbi  writes:
> >>
> >> > Hi,
> >> >
> >> > Ramneek Mehresh  writes:
> >> >> Add support for otg for all freescale socs having internal usb phy.
> >> >>
> >> >> Ramneek Mehresh (7):
> >> >>   usb:fsl:otg: Make fsl otg driver as tristate
> >> >>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
> >> >>   usb:fsl:otg: Add support to add/remove usb host driver
> >> >>   usb:fsl:otg: Signal host drv when host is otg
> >> >>   usb:fsl:otg: Modify otg_event to start host drv
> >> >>   usb:fsl:otg: Combine host/gadget start/resume for ID change
> >> >>   usb:fsl:otg: Add host-gadget drv sync delay
> >> >
> >> > Unless Alan's okay with the host side changes, I can't accept any
> >> > of these. However, I must say some of the flags you add here
> >> > already exist in some way, shape or form. For example, look at
> is_b_host flag.
> >>
> >
> > Could you please be more specific...which flag you think that I should
> >remove/I'm re-defining. The flags I'm defining are:
> >
> > have_hcd : defined in fsl specific structure for fsl specific use-case
> >
> > had_hcd: defined in fsl specific structure for fsl specific use-case
> >
> > is_otg : defined in include/linux/usb.h
> >
> > Are you suggesting using otg_port or is_b_host instead of is_otg?
> >
> > As I understand, is_b_host is specifically to check if an otg B device
> > is in host mode...correct?  I just need a flag to check if a
> > controller is capable of otg operations? That's why defined "is_otg"
> > flag. Please suggest.
> 
> no, I don't know why I made that comment. You could use otg_port, but that
> wouldn't look very clean. Can you resend with Alan's ack, then I'll move
> this series into testing/next.
> 
> --
> balbi

Can you directly put the change_hcd_work in its phy driver(phy-fsl-usb.c)?
Then add/remove hcd will not through ehci_fsl_drv_suspend/resume,
With this, you can make it work without a new flag "is_otg".

Li Jun 

--
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: dwc3: of-simple: fix build warning on !PM

[Felipe Balbi]

if we have a !PM kernel build, our runtime
suspend/resume callbacks will be left defined but
unused. Add a ifdef CONFIG_PM guard.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/dwc3-of-simple.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index 60c4c5a44307..9c9f74155066 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -122,6 +122,7 @@ static int dwc3_of_simple_remove(struct platform_device 
*pdev)
return 0;
 }
 
+#ifdef CONFIG_PM
 static int dwc3_of_simple_runtime_suspend(struct device *dev)
 {
struct dwc3_of_simple   *simple = dev_get_drvdata(dev);
@@ -150,6 +151,7 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
 
return 0;
 }
+#endif
 
 static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {
SET_RUNTIME_PM_OPS(dwc3_of_simple_runtime_suspend,
-- 
2.6.4

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

[PATCH v2 0/2] USB-FHCI: Use return type "int" for two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 22 Dec 2015 16:43:12 +0100

Another update suggestion was taken into account after a patch was applied
from static source code analysis.

Markus Elfring (2):
  Use a signed return type for fhci_create_ep()
  Use a signed return type for endpoint_zero_init()

 drivers/usb/host/fhci-hcd.c | 4 ++--
 drivers/usb/host/fhci-tds.c | 2 +-
 drivers/usb/host/fhci.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.6.3

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

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

[Alan Stern]

On Tue, 22 Dec 2015, Peter Chen wrote:

> > I don't really understand this.  However, you can always specify a USB
> > device by giving its port number on the parent hub, and the hub's port
> > number on _its_ parent hub, and so on back to the root hub and host
> > controller.  That works even if you're not using DT or OF or ACPI.
> >
> 
> Thanks, so the HUB's physical port number is the same with its logical port
> number which reported at its descriptor?

There's no distinction between logical and physical port numbers in
USB.  (However, there can be an issue with USB-3 root hubs, because
they may assign two different port numbers to the same physical port:
One is the port number on the LS/FS/HS bus and the other is the port
number on the SS bus.  This shouldn't cause any problems.)

>  If we assumed all HUBs follow it,
> then we can use port number to align the devices which we described at
> DT and detected by USB bus.

Yes.

> If the host controller has DT supported, and there are two ports connects
> two different onboard devices. When the device is found by the bus,
> we will know its portnum and parent (see usb_alloc_dev), and we know parent's
> device node, so we will know two children node by iterate its parent,
> and match its port number
> (property "reg" at below dts node) with udev->portnum we assigned at
> usb_alloc_dev.
> Then we can let the device know DT.
> 
> After USB device knows DT, it can handle DT properties at its driver.
> 
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -494,6 +494,8 @@ struct usb_device *usb_alloc_dev(struct usb_device 
> *parent,
> dev->portnum = port1;
> dev->bus = bus;
> dev->parent = parent;
> +   if (parent->of_node)
> +   dev->dev->of_node = find_node_by_bus(parent->of_node,
> dev->portnum);

That's right, except you should also handle the case where parent is 
NULL (a root hub).

> INIT_LIST_HEAD(>filelist);
> 
> 
>  {
> vbus-supply = <_usb_h1_vbus>;
> status = "okay";
> 
> devices-pre-operation = <_pre_operation>
> 
> #address-cells = <1>;
> #size-cells = <0>;
> usb: usb_mfd2415@01 {
> compatible = "usb multi-device";
> reg = <0x01>;
> clocks = < IMX6QDL_CLK_CKO>;
> reset-gpios = < 12 GPIO_ACTIVE_LOW>;
> reset-duration-us = <3000>;
> gpio-controller;
> #gpio-cells = <2>;
> };
> 
> usb: usb_mfd2415@02 {
> compatible = "usb multi-device";
> reg = <0x02>;
> clocks = < IMX6QDL_CLK_CKO2>;
> reset-gpios = < 13 GPIO_ACTIVE_LOW>;
> reset-duration-us = <3000>;
> gpio-controller;
> #gpio-cells = <2>;
> };
> };

I'll leave this for the DT experts to discuss.  However, it's worth 
pointing out that a similar scheme should work for ACPI.

Alan Stern

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

Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled

[Oliver Neukum]

On Tue, 2015-12-22 at 09:48 +, Hayes Wang wrote:
>  Peter Wu [mailto:pe...@lekensteyn.nl]
> > Sent: Tuesday, December 08, 2015 10:33 PM
> [...]
> > I found another problem with runtime PM. When a device is suspended via
> > autosuspend and a system suspend takes place, there is no network I/O
> > after resume. Triggering a renegotiation (ethtool -r eth1) brings back
> > network activity.
> 
> I think it is relative to the firmware. Could you try the driver from Realtek 
> website?

Hi,

at the risk of repeating myself I must say that there is a logic flaw
in the driver. If you look at this code:

static int rtl8152_resume(struct usb_interface *intf)
{
struct r8152 *tp = usb_get_intfdata(intf);

mutex_lock(>control);

if (!test_bit(SELECTIVE_SUSPEND, >flags)) {
tp->rtl_ops.init(tp);
netif_device_attach(tp->netdev);
}

if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, >flags);
napi_disable(>napi);
set_bit(WORK_ENABLE, >flags);
if (netif_carrier_ok(tp->netdev))
rtl_start_rx(tp);
napi_enable(>napi);
} else {
tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
  tp->mii.supports_gmii ?
  SPEED_1000 : SPEED_100,
  DUPLEX_FULL);
netif_carrier_off(tp->netdev);
set_bit(WORK_ENABLE, >flags);
}

You need to understand that its use of the flag SELECTIVE_SUSPEND
is invalid. SELECTIVE_SUSPEND is used at two places in the driver.

Once in rtl8152_start_xmit(), where it is working but misnamed.
At that time you need to know whether the device is suspended.
To the device it does not matter whether the suspension is selective
or for the whole bus. It cannot tell. The driver just needs to know
whether it should resume the device if a packet to be transmitted
through it is given to the driver. So far all is well.

But at the time rtl8152_resume() is called the flag has become
meaningless. It tells you whether the device was selectively suspended
(we call that autosuspended, but the concept is the same), but you
take it to mean that it was _only_ selectively suspended. It does not
tell you that.
That matters a lot because the behavior of the host regarding powering
the bus during S3 and S4 is not defined. As I mentioned the device
can't tell whether it is selectively suspended. But it does notice
if its power supply is cut. In that case the driver must reinitialize
the device.
The current code does that conditional on
!test_bit(SELECTIVE_SUSPEND ... )
That is wrong because you need to do this if power was lost. The test
only tells you whether the device was selectively suspend before
power was lost (if power was lost). That is not the same thing at all.

The way the USB subsystem is designed is that it tells you whether power
had been cut by calling reset_resume() if power was cut or resume() if
power was kept.
If reset_resume() is called you must always execute this code:

tp->rtl_ops.init(tp);

and

tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
  tp->mii.supports_gmii ?
  SPEED_1000 : SPEED_100,
  DUPLEX_FULL);

The conditions used in rtl8152_resume() are wrong.
That is the reason "ethtool -r eth1" is reported to restore the device.
It triggers equivalent operations.

It is clear to me that you cannot get away with using the same operation
for resume() and reset_resume() in your driver. It is fundamentally
impossible. Firmware cannot fix it.

Sorry for the length of the explanation.

HTH
Oliver


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

Re: [PATCH v5] extcon: add Maxim MAX3355 driver

[Sergei Shtylyov]

Hello.

On 12/22/2015 4:13 AM, Chanwoo Choi wrote:


This patch depend on GPIOLIB configuration as following:
I modified it with following diff and applied it.

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index ba4db7d..3d89e60 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -54,6 +54,7 @@ config EXTCON_MAX14577

config EXTCON_MAX3355
   tristate "Maxim MAX3355 USB OTG EXTCON Support"
+   depends on GPIOLIB || COMPILE_TEST


 If it won't compile w/o gpiolib, what's the use of COMIPLE_TEST?
 And no, it shouldn't depend on gpiolib. It has empty stubs for the case of 
CONFIG_GPIOLIB=n. Obviously something is wrong with the GPIO headers, I'll look 
into it.


Yes. When GPIOLIB is disabled, the build issue don't happen.


What? It surely does happen!


hmm
Sure. you need to check the include/linux/gpio/consumer.h.

Because of build error happen, you miss to include the "linux/gpio/consumer.h"
header file in extcon-max3355.c. Please test it for enough time.


   Yes, with this file #include'd, it build fine now.


because include/linux/gpio/consumer.h implement the dummy function
for all gpio functions if CONFIG_GPIOLIB is disabled.


Linus W. advised to #include this header explicitly -- I'll try and post.


Don't necessary. I already updated it including the 
"include/linux/gpio/consumer.h".


   I saw that, yes.


For correct operation of max3355, you should add the dependency
to the extcon-max3355.c driver. This driver use the GPIO library
certainly.


I disagree. The driver will just cease to load in this case. I don't see 
why we need such dependency. Only compilation time dependencies should be
specified, I think.


This driver have to depend on GPIOLIB.
Why are you disagreeing the COMPILE_TEST dependency? It is just compile test
without anything.


   I agree now. I still disagree about the gpiolib dependency though.

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 v2] r8152: fix lockup when runtime PM is enabled

[Hayes Wang]

 Peter Wu [mailto:pe...@lekensteyn.nl]
> Sent: Tuesday, December 08, 2015 10:33 PM
[...]
> I found another problem with runtime PM. When a device is suspended via
> autosuspend and a system suspend takes place, there is no network I/O
> after resume. Triggering a renegotiation (ethtool -r eth1) brings back
> network activity.

I think it is relative to the firmware. Could you try the driver from Realtek 
website?

Best Regards,
Hayes

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

Re: USB 3 XHCI Host Controller in Ubuntu

[Andor J Kiss]

Hi Mathias,

  Ok, good to know.  How do I add to the quirk list?

  I contacted SP, they dn't understand how Linux works (told me to
update my drivers ).  Anyway, maybe I'll look into the USB
analyzer.  Will a virtual one work, or does one need a hardware device?

Regards,
Andor


On Tue, 2015-12-22 at 13:56 +0200, Mathias Nyman wrote:
> On 21.12.2015 19:39, Andor J Kiss wrote:
> > Hi Mathias,
> > 
> > Yes, native support for the graphics is in Kernel 4.3, not 4.2
> > (Intel South Lake HD520 chipset).  I gather there's been a bit of
> > shoehorning to get 4.2 to work with this board.  4.3 should be in
> > Ubuntu 16.04 (next upgrade - few months).
> > 
> > Update on USB 3 to USB 3.  Got the ports to work with an external
> > HDD
> > (wall powered).  So, it seems that the issue is a specific hardware
> > issue.  The USB thumbdrive is a 32GB Silicon Power USB 3.0 (
> > http://www.
> > silicon
> > -power.com/product/product_detail.php?currlang=utf8=10=79&
> > pro=
> > 269=sp#a_1) that was not working.
> > 
> > So...is it common to see product specific USB 3 issues?  Is this
> > due to
> > USB 3 protocols/specifications not being implemented properly by
> > manufacturers?
> > 
> > 
> > Anyway to troubleshoot this thumbdrive, or is that a piece by piece
> > fix
> > that is unfeasible?
> > 
> 
> Hi,
> 
> Some devices misbehave.
> If there is a known workaround then we can add a quirk for that
> device.
> There is a quirk list for that purpose.
> 
> Probably best to inform silicon-power that their usb thumbdrive is
> not working under Linux.
> I'll keep this case in mind, but unless we start to see similar
> behavior in other devices,
> or my todo list is empty, I'm not going to dig deeper into this
> myself.
> 
> Unless of course if you want to take this on.
> Record with a usb3 analyzer the link training signaling,
> both success case in Windows and failing case in Linux.
> 
> -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: USB 3 XHCI Host Controller in Ubuntu

[Mathias Nyman]

On 21.12.2015 19:39, Andor J Kiss wrote:

Hi Mathias,

Yes, native support for the graphics is in Kernel 4.3, not 4.2
(Intel South Lake HD520 chipset).  I gather there's been a bit of
shoehorning to get 4.2 to work with this board.  4.3 should be in
Ubuntu 16.04 (next upgrade - few months).

Update on USB 3 to USB 3.  Got the ports to work with an external HDD
(wall powered).  So, it seems that the issue is a specific hardware
issue.  The USB thumbdrive is a 32GB Silicon Power USB 3.0 (http://www.
silicon
-power.com/product/product_detail.php?currlang=utf8=10=79=
269=sp#a_1) that was not working.

So...is it common to see product specific USB 3 issues?  Is this due to
USB 3 protocols/specifications not being implemented properly by
manufacturers?


Anyway to troubleshoot this thumbdrive, or is that a piece by piece fix
that is unfeasible?



Hi,

Some devices misbehave.
If there is a known workaround then we can add a quirk for that device.
There is a quirk list for that purpose.

Probably best to inform silicon-power that their usb thumbdrive is not working 
under Linux.
I'll keep this case in mind, but unless we start to see similar behavior in 
other devices,
or my todo list is empty, I'm not going to dig deeper into this myself.

Unless of course if you want to take this on.
Record with a usb3 analyzer the link training signaling,
both success case in Windows and failing case in Linux.

-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

[PATCH v2 1/2] USB-FHCI: Use a signed return type for fhci_create_ep()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 22 Dec 2015 16:10:14 +0100

The return type "u32" was used by the fhci_create_ep() function even though
it will eventually return a negative error code.
Improve this implementation detail by using the type "int" instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/usb/host/fhci-tds.c | 2 +-
 drivers/usb/host/fhci.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/fhci-tds.c b/drivers/usb/host/fhci-tds.c
index f82ad5d..fc0b525 100644
--- a/drivers/usb/host/fhci-tds.c
+++ b/drivers/usb/host/fhci-tds.c
@@ -149,7 +149,7 @@ void fhci_ep0_free(struct fhci_usb *usb)
  * data_memThe data memory partition(BUS)
  * ring_lenTD ring length
  */
-u32 fhci_create_ep(struct fhci_usb *usb, enum fhci_mem_alloc data_mem,
+int fhci_create_ep(struct fhci_usb *usb, enum fhci_mem_alloc data_mem,
   u32 ring_len)
 {
struct endpoint *ep;
diff --git a/drivers/usb/host/fhci.h b/drivers/usb/host/fhci.h
index 154e6a0..afa3cfc 100644
--- a/drivers/usb/host/fhci.h
+++ b/drivers/usb/host/fhci.h
@@ -547,7 +547,7 @@ u32 fhci_host_transaction(struct fhci_usb *usb, struct 
packet *pkt,
 void fhci_host_transmit_actual_frame(struct fhci_usb *usb);
 void fhci_tx_conf_interrupt(struct fhci_usb *usb);
 void fhci_push_dummy_bd(struct endpoint *ep);
-u32 fhci_create_ep(struct fhci_usb *usb, enum fhci_mem_alloc data_mem,
+int fhci_create_ep(struct fhci_usb *usb, enum fhci_mem_alloc data_mem,
   u32 ring_len);
 void fhci_init_ep_registers(struct fhci_usb *usb,
struct endpoint *ep,
-- 
2.6.3

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

[PATCH v2 2/2] USB-FHCI: Use a signed return type for endpoint_zero_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 22 Dec 2015 16:24:46 +0100

The return type "u32" was used by the endpoint_zero_init() function
even though it can return a value which corresponds to a negative
error code from a call of the fhci_create_ep() function.
Improve this implementation detail by using the type "int" instead.

Signed-off-by: Markus Elfring 
---
 drivers/usb/host/fhci-hcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/fhci-hcd.c b/drivers/usb/host/fhci-hcd.c
index c6cebb9..b2889f0 100644
--- a/drivers/usb/host/fhci-hcd.c
+++ b/drivers/usb/host/fhci-hcd.c
@@ -64,11 +64,11 @@ u16 fhci_get_sof_timer_count(struct fhci_usb *usb)
 }
 
 /* initialize the endpoint zero */
-static u32 endpoint_zero_init(struct fhci_usb *usb,
+static int endpoint_zero_init(struct fhci_usb *usb,
  enum fhci_mem_alloc data_mem,
  u32 ring_len)
 {
-   u32 rc;
+   int rc;
 
rc = fhci_create_ep(usb, data_mem, ring_len);
if (rc)
-- 
2.6.3

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

[PATCH 1/2] usb: gadget: f_midi: refactor state machine

[Felipe F. Tonello]

This refactor includes the following:
 * Cleaner state machine code;
 * Reset state if MIDI message parsed is non-conformant;
 * Fixed bug when a conformant MIDI message was followed by a non-conformant
   causing the MIDI-USB message to use old temporary data (port->data[0..1]),
   thus packing a wrong MIDI-USB request.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 243 ---
 1 file changed, 141 insertions(+), 102 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index fb1fe96d..b70a830 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -50,6 +50,18 @@ static const char f_midi_longname[] = "MIDI Gadget";
  */
 #define MAX_PORTS 16
 
+/* MIDI message states */
+enum {
+   STATE_INITIAL = 0,
+   STATE_1PARAM,
+   STATE_2PARAM_1,
+   STATE_2PARAM_2,
+   STATE_SYSEX_0,
+   STATE_SYSEX_1,
+   STATE_SYSEX_2,
+   STATE_FINISHED,
+};
+
 /*
  * This is a gadget, and the IN/OUT naming is from the host's perspective.
  * USB -> OUT endpoint -> rawmidi
@@ -60,13 +72,6 @@ struct gmidi_in_port {
int active;
uint8_t cable;
uint8_t state;
-#define STATE_UNKNOWN  0
-#define STATE_1PARAM   1
-#define STATE_2PARAM_1 2
-#define STATE_2PARAM_2 3
-#define STATE_SYSEX_0  4
-#define STATE_SYSEX_1  5
-#define STATE_SYSEX_2  6
uint8_t data[2];
 };
 
@@ -400,118 +405,152 @@ static int f_midi_snd_free(struct snd_device *device)
return 0;
 }
 
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
-   uint8_t p1, uint8_t p2, uint8_t p3)
-{
-   unsigned length = req->length;
-   u8 *buf = (u8 *)req->buf + length;
-
-   buf[0] = p0;
-   buf[1] = p1;
-   buf[2] = p2;
-   buf[3] = p3;
-   req->length = length + 4;
-}
-
 /*
  * Converts MIDI commands to USB MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 struct gmidi_in_port *port, uint8_t b)
 {
-   uint8_t p0 = port->cable << 4;
-
-   if (b >= 0xf8) {
-   f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
-   } else if (b >= 0xf0) {
-   switch (b) {
-   case 0xf0:
-   port->data[0] = b;
-   port->state = STATE_SYSEX_1;
-   break;
-   case 0xf1:
-   case 0xf3:
-   port->data[0] = b;
-   port->state = STATE_1PARAM;
-   break;
-   case 0xf2:
+   uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+   uint8_t next_state = STATE_INITIAL;
+
+   switch (port->state) {
+   case STATE_INITIAL: {
+   if (b >= 0xf8) {
+   p[0] |= 0x0f;
+   p[1] = b;
+   next_state = STATE_FINISHED;
+   } else if (b >= 0xf0) {
+   switch (b) {
+   case 0xf0:
+   port->data[0] = b;
+   next_state = STATE_SYSEX_1;
+   break;
+   case 0xf1:
+   case 0xf3:
+   port->data[0] = b;
+   next_state = STATE_1PARAM;
+   break;
+   case 0xf2:
+   port->data[0] = b;
+   next_state = STATE_2PARAM_1;
+   break;
+   case 0xf4:
+   case 0xf5:
+   next_state = STATE_INITIAL;
+   break;
+   case 0xf6:
+   p[0] |= 0x05;
+   p[1] = 0xf6;
+   next_state = STATE_FINISHED;
+   break;
+   }
+   } else if (b >= 0x80) {
port->data[0] = b;
-   port->state = STATE_2PARAM_1;
-   break;
-   case 0xf4:
-   case 0xf5:
-   port->state = STATE_UNKNOWN;
-   break;
-   case 0xf6:
-   f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
-   port->state = STATE_UNKNOWN;
-   break;
-   case 0xf7:
+   if (b >= 0xc0 && b <= 0xdf)
+   next_state = STATE_1PARAM;
+   else
+   next_state = STATE_2PARAM_1;
+   }
+   break;
+   }
+   case STATE_1PARAM:
+   case STATE_2PARAM_1:
+   case STATE_2PARAM_2:
+   case STATE_SYSEX_0:
+   

Re: [patch] usb: gadget: f_midi: missing unlock on error path

[Felipe Ferreri Tonello]

Hi Dan,

On 21/12/15 13:20, Dan Carpenter wrote:
> We added a new error path to this function and we forgot to drop the
> lock.
> 
> Fixes: e1e3d7ec5da3 ('usb: gadget: f_midi: pre-allocate IN requests')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index fb1fe96d..92b9ec8 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1213,8 +1213,10 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>   midi->in_last_port = 0;
>  
>   status = kfifo_alloc(>in_req_fifo, midi->qlen, GFP_KERNEL);
> - if (status)
> + if (status) {
> + mutex_unlock(>lock);

This is fine. But looking into this further, I believe this mutex_unlock
should be under the setup_fail label, because even if the alloc() fails,
it will be need to unlock the mutex in any circumstance anyway.

>   goto setup_fail;
> + }
>  
>   ++opts->refcnt;
>   mutex_unlock(>lock);
> 

Felipe


0x92698E6A.asc
Description: application/pgp-keys

[PATCH 2/2] usb: gadget: f_midi: added spinlock on transmit function

[Felipe F. Tonello]

Since f_midi_transmit is called by both ALSA and USB frameworks, it can
potentially cause a race condition between both calls. This is bad because the
way f_midi_transmit is implemented can't handle concurrent calls. This is due
to the fact that the usb request fifo looks for the next element and only if
it has data to process it enqueues the request, otherwise re-uses it. If both
(ALSA and USB) frameworks calls this function at the same time, the
kfifo_seek() will return the same usb_request, which will cause a race
condition.

To solve this problem a syncronization mechanism is necessary. In this case it
is used a spinlock since f_midi_transmit is also called by usb_request->complete
callback in interrupt context.

On benchmarks realized by me, spinlocks were more efficient then scheduling
the f_midi_transmit tasklet in process context and using a mutex to
synchronize. Also it performs better then previous implementation that
allocated a usb_request for every new transmit made.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index b70a830..00a15e9 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -97,6 +98,7 @@ struct f_midi {
/* This fifo is used as a buffer ring for pre-allocated IN usb_requests 
*/
DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
unsigned int in_last_port;
+   spinlock_t transmit_lock;
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -574,12 +576,15 @@ static void f_midi_drop_out_substreams(struct f_midi 
*midi)
 static void f_midi_transmit(struct f_midi *midi)
 {
struct usb_ep *ep = midi->in_ep;
+   unsigned long flags;
bool active;
 
/* We only care about USB requests if IN endpoint is enabled */
if (!ep || !ep->enabled)
goto drop_out;
 
+   spin_lock_irqsave(>transmit_lock, flags);
+
do {
struct usb_request *req = NULL;
unsigned int len, i;
@@ -591,14 +596,17 @@ static void f_midi_transmit(struct f_midi *midi)
len = kfifo_peek(>in_req_fifo, );
if (len != 1) {
ERROR(midi, "%s: Couldn't get usb request\n", __func__);
+   spin_unlock_irqrestore(>transmit_lock, flags);
goto drop_out;
}
 
/* If buffer overrun, then we ignore this transmission.
 * IMPORTANT: This will cause the user-space rawmidi device to 
block until a) usb
 * requests have been completed or b) snd_rawmidi_write() times 
out. */
-   if (req->length > 0)
+   if (req->length > 0) {
+   spin_unlock_irqrestore(>transmit_lock, flags);
return;
+   }
 
for (i = midi->in_last_port; i < MAX_PORTS; i++) {
struct gmidi_in_port *port = midi->in_port[i];
@@ -650,6 +658,8 @@ static void f_midi_transmit(struct f_midi *midi)
}
} while (active);
 
+   spin_unlock_irqrestore(>transmit_lock, flags);
+
return;
 
 drop_out:
@@ -1255,6 +1265,8 @@ static struct usb_function *f_midi_alloc(struct 
usb_function_instance *fi)
if (status)
goto setup_fail;
 
+   spin_lock_init(>transmit_lock);
+
++opts->refcnt;
mutex_unlock(>lock);
 
-- 
2.5.0

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

[PATCH] chipidea: imx: Enable PM runtime for mx28

[Fabio Estevam]

On a custom mx28 board that has a USB hub we see the following
error:

hub 1-1:1.0: USB hub found
hub 1-1:1.0: 2 ports detected
usb 1-1: USB disconnect, device number 2
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: Cannot enable. Maybe the USB cable is bad?

Passing CI_HDRC_SUPPORTS_RUNTIME_PM flag fixes the problem.

Tested on a mx28evk and also on a mx28 custom board.

Signed-off-by: Fabio Estevam 
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index f14f4ab..0e7bcbb 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -34,6 +34,7 @@ static const struct ci_hdrc_imx_platform_flag imx27_usb_data 
= {
 
 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
.flags = CI_HDRC_IMX28_WRITE_FIX |
+   CI_HDRC_SUPPORTS_RUNTIME_PM |
CI_HDRC_TURN_VBUS_EARLY_ON |
CI_HDRC_DISABLE_STREAMING,
 };
-- 
1.9.1

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

Re: USB 3 XHCI Host Controller in Ubuntu

[Mathias Nyman]

On 22.12.2015 14:16, Andor J Kiss wrote:

Hi Mathias,

   Ok, good to know.  How do I add to the quirk list?


Once we got code to workaround the issue we can name a quirk for it.
Then in drivers/usb/core/quirks.c we set the quirk based on device/vendor ID of 
the device.

But we are not there yet.



   I contacted SP, they dn't understand how Linux works (told me to
update my drivers ).  Anyway, maybe I'll look into the USB
analyzer.  Will a virtual one work, or does one need a hardware device?


Hardware one needed in this case (for LFPS signalling and LTSSM states)

-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: [linux-usb-users] g_serial is not working in rx path (K3.14)

[Ramajayam S]

On Mon, Dec 14, 2015 at 11:17 PM, Ramajayam S
 wrote:
> On Mon, Dec 14, 2015 at 9:50 PM, Felipe Balbi  wrote:
>>
>> hi,
>>
>> Ramajayam S  writes:
>>> Hi
>>>We are trying to enable g_serial in Intel platform and facing
>>> issue with RX path.
>>>
>>> stable kernel version: K3.14
>>
>> you need to test with something more recent, like v4.3. If you are stuck
>> with Intel' v3.14 kernel, then you need to ask for support from Intel,
> ok
>> I'm afraid
>>
>>> configuration enabled: USB_G_SERIAL
>>>
>>> testing details:
>>>
>>> writing from device side(ttyS0) to Windows Host(com 11-g_serial): working 
>>> fine
>>>
>>> from ttyS0: echo test > /dev/ttyGS0  -- received in comm 11
>>>
>>> Typing any character from windows host pc(com 11 using teraterm) to
>>> device side: not working
>>
>> what do you mean not working ? What happens ? Got usbmon trace or some
>> usb sniffer ?
> We have profiled using USB SW analyzer, by pressing enter key from USB
> console(ttyGS0-com11 ) both the OUT/IN token sent from Host
> PC(windows).
> For IN token,
> Working case: device is responded with proper data(approx 20 bytes for
> command prompt display in ttyGSO-com11) and able to see raw data in
> function   gs_write.
> Non-Working Case: device is responded  with 1 byte of data and  able
> to see raw data in function   gs_write (no response in command prompt
> display --ttyGS0-com11)
If we had print in n_tty_read function(after job_control function),
console(ttyGS0-Com11) is working fine with double key press.
1st key press(enter key): user space is requesting to read 512 bytes
of data instead of 1 byte -- non working scenario
2nd key Press(enter key): user space is requesting to read 1 byte of
data --  working scenario
Ramajayam S
>>
>> --
>> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine

[Clemens Ladisch]

Felipe F. Tonello wrote:
> This refactor includes the following:
>  * Cleaner state machine code;

It does not correctly handle system real time messages inserted between
the status and data bytes of other messages.

>  * Reset state if MIDI message parsed is non-conformant;

Why?  In a byte stream like "C1 C3 44", where the data byte of the first
message was lost, the reset would also drop the second message.

>  * Fixed bug when a conformant MIDI message was followed by a non-conformant
>causing the MIDI-USB message to use old temporary data (port->data[0..1]),
>thus packing a wrong MIDI-USB request.

Running status is feature.


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

Re: [PATCH v3] usb: dwc3: gadget: handle request->zero

[Felipe Balbi]

Hi,

John Youn  writes:
> On 12/3/2015 7:18 AM, Felipe Balbi wrote:
>> So far, dwc3 has always missed request->zero
>> handling for every endpoint. Let's implement
>> that so we can handle cases where transfer must
>> be finished with a ZLP.
>> 
>> Note that dwc3 is a little special. Even though
>> we're dealing with a ZLP, we still need a buffer
>> of wMaxPacketSize bytes; to hide that detail from
>> every gadget driver, we have a preallocated buffer
>> of 1024 bytes (biggest bulk size) to use (and
>> share) among all endpoints.
>> 
>> Reported-by: Ravi B 
>> Signed-off-by: Felipe Balbi 
>> ---
>> 
>> since v1:
>>  - remove unnecessary 'free_on_complete' flag
>> 
>> since v2:
>>  - remove unnecessary 'out' label
>> 
>>  drivers/usb/dwc3/core.h   |  3 +++
>>  drivers/usb/dwc3/gadget.c | 50 
>> +--
>>  2 files changed, 51 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 36f1cb74588c..29130682e547 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -37,6 +37,7 @@
>>  #define DWC3_MSG_MAX500
>>  
>>  /* Global constants */
>> +#define DWC3_ZLP_BUF_SIZE   1024/* size of a superspeed bulk */
>>  #define DWC3_EP0_BOUNCE_SIZE512
>>  #define DWC3_ENDPOINTS_NUM  32
>>  #define DWC3_XHCI_RESOURCES_NUM 2
>> @@ -647,6 +648,7 @@ struct dwc3_scratchpad_array {
>>   * @ctrl_req: usb control request which is used for ep0
>>   * @ep0_trb: trb which is used for the ctrl_req
>>   * @ep0_bounce: bounce buffer for ep0
>> + * @zlp_buf: used when request->zero is set
>>   * @setup_buf: used while precessing STD USB requests
>>   * @ctrl_req_addr: dma address of ctrl_req
>>   * @ep0_trb: dma address of ep0_trb
>> @@ -734,6 +736,7 @@ struct dwc3 {
>>  struct usb_ctrlrequest  *ctrl_req;
>>  struct dwc3_trb *ep0_trb;
>>  void*ep0_bounce;
>> +void*zlp_buf;
>>  void*scratchbuf;
>>  u8  *setup_buf;
>>  dma_addr_t  ctrl_req_addr;
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index e341f034296f..e916c11ded59 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1158,6 +1158,32 @@ out:
>>  return ret;
>>  }
>>  
>> +static void __dwc3_gadget_ep_zlp_complete(struct usb_ep *ep,
>> +struct usb_request *request)
>> +{
>> +dwc3_gadget_ep_free_request(ep, request);
>> +}
>> +
>> +static int __dwc3_gadget_ep_queue_zlp(struct dwc3 *dwc, struct dwc3_ep *dep)
>> +{
>> +struct dwc3_request *req;
>> +struct usb_request  *request;
>> +struct usb_ep   *ep = >endpoint;
>> +
>> +dwc3_trace(trace_dwc3_gadget, "queueing ZLP\n");
>> +request = dwc3_gadget_ep_alloc_request(ep, GFP_ATOMIC);
>> +if (!request)
>> +return -ENOMEM;
>> +
>> +request->length = 0;
>> +request->buf = dwc->zlp_buf;
>> +request->complete = __dwc3_gadget_ep_zlp_complete;
>> +
>> +req = to_dwc3_request(request);
>> +
>> +return __dwc3_gadget_ep_queue(dep, req);
>> +}
>> +
>>  static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request 
>> *request,
>>  gfp_t gfp_flags)
>>  {
>> @@ -1171,6 +1197,16 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, 
>> struct usb_request *request,
>>  
>>  spin_lock_irqsave(>lock, flags);
>>  ret = __dwc3_gadget_ep_queue(dep, req);
>> +
>> +/*
>> + * Okay, here's the thing, if gadget driver has requested for a ZLP by
>> + * setting request->zero, instead of doing magic, we will just queue an
>> + * extra usb_request ourselves so that it gets handled the same way as
>> + * any other request.
>> + */
>> +if (ret == 0 && request->zero && (request->length % ep->maxpacket == 0))
>> +ret = __dwc3_gadget_ep_queue_zlp(dwc, dep);
>
> Hi Felipe,
>
> This causes regression with at least mass storage + Windows host.
>
> When the gadget queues a ZLP, we end up sending two ZLPs which leads
> to violating the MSC protocol.

heh, no idea why mass storage would set Zero flag in this case :-p

> The following fixes it:
>
> -   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 
> 0))
> +   if (ret == 0 && request->zero && (request->length % ep->maxpacket == 
> 0) &&
> +   (request->length != 0))

Can you send this as a proper patch ? And also patch g_mass_storage to
_not_ set Zero flag in this case ?

thanks

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine

[Felipe Balbi]

Hi,

"Felipe F. Tonello"  writes:
> This refactor includes the following:
>  * Cleaner state machine code;
>  * Reset state if MIDI message parsed is non-conformant;
>  * Fixed bug when a conformant MIDI message was followed by a non-conformant
>causing the MIDI-USB message to use old temporary data (port->data[0..1]),
>thus packing a wrong MIDI-USB request.

we don't do more than one logical thing per patch. Please split this up.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: core: devio.c: Removed unnecessary space

[Chase Metzger]

Removed an unnecessary space between a function name and arguments.

Signed-off-by: Chase Metzger 
---
 drivers/usb/core/devio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..0bcd45e 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1910,7 +1910,7 @@ static int proc_releaseinterface(struct usb_dev_state 
*ps, void __user *arg)
ret = releaseintf(ps, ifnum);
if (ret < 0)
return ret;
-   destroy_async_on_interface (ps, ifnum);
+   destroy_async_on_interface(ps, ifnum);
return 0;
 }
 
-- 
2.1.4

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

Re: [PATCH] usb: gadget: rndis: fix itnull.cocci warnings

[Julia Lawall]

On Tue, 22 Dec 2015, Felipe Balbi wrote:

> Julia Lawall  writes:
>
> > The index variable of list_for_each_entry_safe is never NULL.
> >
> > Generated by: scripts/coccinelle/iterators/itnull.cocci
> >
> > CC: Geliang Tang 
> > Signed-off-by: Fengguang Wu 
> > Signed-off-by: Julia Lawall 
>
> doesn't apply. Does this depend on anything ?

It may be derived from a patch posted to a mailing list.  I don't have the
original reference any more.

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