Re: [PATCH v2 00/13] usb: dwc2: descriptor dma mode bug fixes

[Herrero, Gregory]

Hi Felipe,

Can you take this serie or would you like me to resend it?

BR,
Gregory

On Fri, Nov 06, 2015 at 12:02:42AM +0100, John Youn wrote:
> On 11/5/2015 12:41 AM, Gregory Herrero wrote:
> > Hi,
> > 
> > This patchset contains bug fixes for host descriptor dma mode.
> > 
> > Descriptor dma mode can't be used as the default mode since controller
> > does not support split transfers in this mode.
> > So we add a new configuration parameter which allows descriptor dma mode
> > to be enabled for full-speed devices only.
> > 
> > All patches are verified on dwc2 v3.0a with dedicated fifos and our
> > main test target was usb audio devices.
> > 
> > All patches are based on Felipe's testing/next branch.
> > 
> > Regards,
> > Gregory
> > 
> > History:
> > v2:
> >   - Use dma cache in "usb: dwc2: host: use kmem cache to allocate 
> > descriptors"
> > and replace usage of deprecated GFP_DMA32 flag.
> >   - Remove "usb: dwc2: host: free status_buf on hcd de-init".
> > 
> > v1:
> >   - Fix compilation error introduced by:
> > "usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled"
> >   - Fix warning introduced by:
> > "usb: dwc2: host: fix descriptor list address masking"
> > 
> > Gregory Herrero (11):
> >   usb: dwc2: host: ensure filling of isoc desc is correctly done
> >   usb: dwc2: host: set active bit in isochronous descriptors
> >   usb: dwc2: host: rework isochronous halt path
> >   usb: dwc2: host: fix use of qtd after free in desc dma mode
> >   usb: dwc2: host: spinlock release channel
> >   usb: dwc2: host: add function to compare frame index
> >   usb: dwc2: host: program descriptor for next frame
> >   usb: dwc2: host: always increment available host channel during
> > release
> >   usb: dwc2: host: process all completed urbs
> >   usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled
> >   usb: dwc2: host: use kmem cache to allocate descriptors
> > 
> > Mian Yousaf Kaukab (2):
> >   usb: dwc2: host: enable descriptor dma for fs devices
> >   usb: dwc2: host: fix descriptor list address masking
> > 
> >  drivers/usb/dwc2/core.c  |  37 +--
> >  drivers/usb/dwc2/core.h  |  26 +
> >  drivers/usb/dwc2/hcd.c   |  76 +-
> >  drivers/usb/dwc2/hcd.h   |  19 
> >  drivers/usb/dwc2/hcd_ddma.c  | 240 
> > +++
> >  drivers/usb/dwc2/hcd_intr.c  |  15 ++-
> >  drivers/usb/dwc2/hcd_queue.c |   2 +-
> >  drivers/usb/dwc2/hw.h|   4 -
> >  drivers/usb/dwc2/platform.c  |   4 +
> >  9 files changed, 367 insertions(+), 56 deletions(-)
> > 
> 
> For this series:
> 
> Acked-by: John Youn 
> 
> 
> I'm not able to test DDMA on host. But I checked that it didn't
> break anything else.
> 
> Regards,
> John
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] USB-serial fixes for v4.4-rc2

[Johan Hovold]

Hi Greg,

Here are a few fixes and new device ids for v4.4-rc2. All have been in
linux-next for a few days now.

Thanks,
Johan


The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec:

  Linux 4.4-rc1 (2015-11-15 17:00:27 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
tags/usb-serial-4.4-rc2

for you to fetch changes up to 59536da34513c594af2a6fd35ba65ea45b6960a1:

  USB: qcserial: Fix support for HP lt4112 LTE/HSPA+ Gobi 4G Modem (2015-11-16 
18:29:07 +0100)


USB-serial fixes for v4.4-rc2

Here are some new device ids, support for an odd qcserial Gobi interface
layout and a fix for the qcserial Huawei interface layout.

Signed-off-by: Johan Hovold 


Aleksander Morgado (1):
  USB: serial: option: add support for Novatel MiFi USB620L

Bjørn Mork (1):
  USB: qcserial: Fix support for HP lt4112 LTE/HSPA+ Gobi 4G Modem

David Woodhouse (1):
  USB: ti_usb_3410_5052: Add Honeywell HGI80 ID

Petr Štetiar (1):
  USB: qcserial: Add support for Quectel EC20 Mini PCIe module

 drivers/usb/serial/option.c   |  2 +
 drivers/usb/serial/qcserial.c | 94 +++
 drivers/usb/serial/ti_usb_3410_5052.c |  2 +
 drivers/usb/serial/ti_usb_3410_5052.h |  4 ++
 4 files changed, 82 insertions(+), 20 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] cdc_acm: Ignore Infineon Flash Loader utility

[Johan Hovold]

On Mon, Nov 16, 2015 at 01:34:14PM +0100, Jonas Jonsson wrote:
> Some modems, such as the Telit UE910, are using an Infineon Flash Loader
> utility. It has two interfaces, 2/2/0 (Abstract Modem) and 10/0/0 (CDC
> Data). The latter can be used as a serial interface to upgrade the
> firmware of the modem. However, that isn't possible when the cdc-acm
> driver takes control of the device.

Could you expand the commit message with some more information from the
thread were Daniele explained how the flash loader works here?
Specifically, that the device looks like a CDC-ACM device but really is
not until after the flash loader has timed out.

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

Re: [PATCH v2 2/2] USB: serial: Another Infineon flash loader USB ID

[Johan Hovold]

On Mon, Nov 16, 2015 at 01:34:15PM +0100, Jonas Jonsson wrote:
> This has been seen on a Telit UE910 modem.

Please expand this message as well and mention why this is not a CDC
device so we do not forget.

> Signed-off-by: Jonas Jonsson 
> Tested-by: Daniele Palmas 
> ---
>  drivers/usb/serial/usb-serial-simple.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/usb-serial-simple.c 
> b/drivers/usb/serial/usb-serial-simple.c
> index 3658662..93ab784 100644
> --- a/drivers/usb/serial/usb-serial-simple.c
> +++ b/drivers/usb/serial/usb-serial-simple.c
> @@ -53,7 +53,9 @@ DEVICE(funsoft, FUNSOFT_IDS);
>  
>  /* Infineon Flashloader driver */
>  #define FLASHLOADER_IDS()\
> - { USB_DEVICE(0x8087, 0x0716) }
> + { USB_DEVICE(0x8087, 0x0716) }, \
> + { USB_DEVICE_INTERFACE_CLASS(0x058b, 0x0041, 0x0a) }

Please use USB_CLASS_CDC_DATA here.

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

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

[Jiri Kosina]

On Thu, 19 Nov 2015, Ioan-Adrian Ratiu wrote:

> First part of lockdep report:
> http://imgur.com/clLsCWe
> 
> Second part:
> http://imgur.com/Wa2PzRl
> 
> Here are some printk's of mine while reproducing + debugging the issue:
> http://imgur.com/SETOHT7

So the real problem is that Intuos driver is calling hid_hw_request() 
(which tries to grab the lock in usbhid_submit_report()) while handling 
the CTRL IRQ (lock gets acquired there).

So the proper way to fix seems to be delaying the scheduling of the 
proximity read event in wacom_intuos_inout() to workqueue.

> I'll continue to research this more in depth, but progress is slow 
> because I don't have much time, I'm doing this in my spare time because 
> it's my girlfriend's tablet.

Oh, now I understand the level of severity of this bug! :-)

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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: Add the console support for usb-to-serial port

[Peter Hurley]

On 11/19/2015 01:48 AM, Baolin Wang wrote:
>>
>>> +{
>>> + struct gscons_info *info = gserial_cons.data;
>>> + int port_num = gserial_cons.index;
>>> + struct usb_request *req;
>>> + struct gs_port *port;
>>> + struct usb_ep *ep;
>>> +
>>> + if (port_num >= MAX_U_SERIAL_PORTS || port_num < 0) {
>>> + pr_err("%s: port num [%d] exceeds the range.\n",
>>> +__func__, port_num);
>>> + return -ENXIO;
>>> + }
>>> +
>>> + port = ports[port_num].port;
>>> + if (!port) {
>>> + pr_err("%s: serial line [%d] not allocated.\n",
>>> +__func__, port_num);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (!port->port_usb) {
>>> + pr_err("%s: no port usb.\n", __func__);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + ep = port->port_usb->in;
>>> + if (!ep) {
>>> + pr_err("%s: no usb endpoint.\n", __func__);
>>> + return -ENXIO;
>>> + }
>>
>> Looking at the caller, gserial_connect(), none of the error
>> conditions above look possible.
>>
> 
> I re-look the code and do some tests, I found the checking is
> necessary. Cause we get the port number from the console->index, if
> the cmdline is not set the ttyGS0 as the console, the console->index
> will be -1 that is a wrong value. Also the serial.c file will create 1
> usb-to-seial port as default (default n_ports = 1), so we need to
> check the port and the endpoint of the port. So I think here checking
> is necessary and I have tested it.

static void gs_console_connect(int port_num)
{
.
.
if (port_num != gserial_cons.index)
return;
.
.


@@ -1219,6 +1453,7 @@ int gserial_connect(struct gserial *gser, u8 port_num)
gser->disconnect(gser);
}
 
+   status = gs_console_connect(port_num);
spin_unlock_irqrestore(>port_lock, flags);
 
return status;







--
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: Add the console support for usb-to-serial port

[Baolin Wang]

On 19 November 2015 at 17:36, Peter Hurley  wrote:
> On 11/19/2015 01:48 AM, Baolin Wang wrote:
>>>
 +{
 + struct gscons_info *info = gserial_cons.data;
 + int port_num = gserial_cons.index;
 + struct usb_request *req;
 + struct gs_port *port;
 + struct usb_ep *ep;
 +
 + if (port_num >= MAX_U_SERIAL_PORTS || port_num < 0) {
 + pr_err("%s: port num [%d] exceeds the range.\n",
 +__func__, port_num);
 + return -ENXIO;
 + }
 +
 + port = ports[port_num].port;
 + if (!port) {
 + pr_err("%s: serial line [%d] not allocated.\n",
 +__func__, port_num);
 + return -ENODEV;
 + }
 +
 + if (!port->port_usb) {
 + pr_err("%s: no port usb.\n", __func__);
 + return -ENODEV;
 + }
 +
 + ep = port->port_usb->in;
 + if (!ep) {
 + pr_err("%s: no usb endpoint.\n", __func__);
 + return -ENXIO;
 + }
>>>
>>> Looking at the caller, gserial_connect(), none of the error
>>> conditions above look possible.
>>>
>>
>> I re-look the code and do some tests, I found the checking is
>> necessary. Cause we get the port number from the console->index, if
>> the cmdline is not set the ttyGS0 as the console, the console->index
>> will be -1 that is a wrong value. Also the serial.c file will create 1
>> usb-to-seial port as default (default n_ports = 1), so we need to
>> check the port and the endpoint of the port. So I think here checking
>> is necessary and I have tested it.
>
> static void gs_console_connect(int port_num)
> {
> .
> .
> if (port_num != gserial_cons.index)
> return;
> .

OK. Thanks.

> .
>
>
> @@ -1219,6 +1453,7 @@ int gserial_connect(struct gserial *gser, u8 port_num)
> gser->disconnect(gser);
> }
>
> +   status = gs_console_connect(port_num);
> spin_unlock_irqrestore(>port_lock, flags);
>
> return status;
>
>
>
>
>
>
>



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

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

[Jiri Kosina]

On Wed, 18 Nov 2015, Ioan-Adrian Ratiu wrote:

> > > The critical section protected by usbhid->lock in hid_ctrl() is too
> > > big and in rare cases causes a recursive deadlock because of its call
> > > to hid_input_report().
> > > 
> > > This deadlock reproduces on newer wacom tablets like 056a:033c because
> > > the wacom driver in its irq handler ends up calling hid_hw_request()
> > > from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
> > > is that it submits a report to reschedule a proximity read through a
> > > sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
> > > before calling hid_input_report(). When the irq kicks in on the same
> > > cpu, it also tries to grab the lock resulting in a recursive deadlock.
> > > 
> > > The proper fix is to shrink the critical section in hid_ctrl() to
> > > protect only the instructions which modify usbhid, thus move the lock
> > > after the hid_input_report() call and the deadlock dissapears.  
> > 
> > I think the proper fix actually is to spin_lock_irqsave() in hid_ctrl(), 
> > isn't it?
> 
> That was my first attempt, yes, but the deadlock still happens with interrupts
> disabled. 

That unfortunately however directly implies that your explanation above 
isn't actually correct description of the real problem.

So we'd better first understand the problem rather than papering it over 
with more or less random fixes.

First, have you tried to run your usecase on your system with lockdep 
enabled?

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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 1/4] usb: gadget: f_sourcesink: add queue depth

[Krzysztof Opasiak]

On 11/19/2015 08:02 AM, Peter Chen wrote:

Add queue depth for both iso and bulk transfer, with more queues, we
can do performance and stress test using sourcesink, and update g_zero
accordingly.

Signed-off-by: Peter Chen 


Reviewed-by: Krzysztof Opasiak 

Best regards,
--
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 v2 3/4] Doc: ABI: configfs-usb-gadget-sourcesink: add two entries for depth of queue

[Krzysztof Opasiak]

On 11/19/2015 08:02 AM, Peter Chen wrote:

Add both bulk and iso depth of queue entries.

Signed-off-by: Peter Chen 


Reviewed-by: Krzysztof Opasiak 


---
  Documentation/ABI/testing/configfs-usb-gadget-sourcesink | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-sourcesink 
b/Documentation/ABI/testing/configfs-usb-gadget-sourcesink
index bc7ff73..f56335a 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-sourcesink
+++ b/Documentation/ABI/testing/configfs-usb-gadget-sourcesink
@@ -10,3 +10,5 @@ Description:
isoc_mult   - 0..2 (hs/ss only)
isoc_maxburst   - 0..15 (ss only)
buflen  - buffer length
+   bulk_qlen   - depth of queue for bulk
+   iso_qlen- depth of queue for iso



The same as in previous patch.

Best regards,
--
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 v2 4/4] usb: gadget: f_sourcesink: quit if usb_ep_queue returns error

[Krzysztof Opasiak]

On 11/19/2015 08:02 AM, Peter Chen wrote:

Since now, we may have more than one request during the test, and
it is better we just quit once the error occurs instead of try
queueing further requests.

Signed-off-by: Peter Chen 
Suggested-by: Krzysztof Opasiak 


Reviewed-by: Krzysztof Opasiak 


---
  drivers/usb/gadget/function/f_sourcesink.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 9df4aa1..807af32 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -635,6 +635,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, 
bool is_in,
  is_iso ? "ISO-" : "", is_in ? "IN" : "OUT",
  ep->name, status);
free_ep_req(ep, req);
+   return status;
}
}




Best regards,
--
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 v2 2/4] Documentation: usb: gadget-testing: add description for depth of queue

[Krzysztof Opasiak]

On 11/19/2015 08:02 AM, Peter Chen wrote:

Add both bulk and iso depth of queue for sourcesink.

Signed-off-by: Peter Chen 


Reviewed-by: Krzysztof Opasiak 


---
  Documentation/usb/gadget-testing.txt | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index b24d3ef..84b3d10 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -579,6 +579,8 @@ The SOURCESINK function provides these attributes in its 
function directory:
isoc_mult   - 0..2 (hs/ss only)
isoc_maxburst   - 0..15 (ss only)
bulk_buflen - buffer length
+   bulk_qlen   - depth of queue for bulk
+   iso_qlen- depth of queue for iso



I'd prefer "queue length for bulk/iso requests" but I'm not native 
speaker or even English expert. For me, your form is also understandable 
so you may leave this as is, change or wait for another opinion, as you 
wish;)


Best regards,
--
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

[PATCH v3 2/2] usb: dwc2: host: Clear interrupts before handling them

[Douglas Anderson]

In general it is wise to clear interrupts before processing them.  If
you don't do that, you can get:
 1. Interrupt happens
 2. You look at system state and process interrupt
 3. A new interrupt happens
 4. You clear interrupt without processing it.

This patch was actually a first attempt to fix missing device insertions
as described in (usb: dwc2: host: Fix missing device insertions) and it
did solve some of the signal bouncing problems but not all of
them (which is why I submitted the other patch).  Specifically, this
patch itself would sometimes change:
 1. hardware sees connect
 2. hardware sees disconnect
 3. hardware sees connect
 4. dwc2_port_intr() - clears connect interrupt
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

...to:
 1. hardware sees connect
 2. hardware sees disconnect
 3. dwc2_port_intr() - clears connect interrupt
 4. hardware sees connect
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

...but with different timing then sometimes we'd still miss cable
insertions.

In any case, though this patch doesn't fix any (known) problems, it
still seems wise as a general policy to clear interrupt before handling
them.

Signed-off-by: Douglas Anderson 
Acked-by: John Youn 
Tested-by: John Youn 
---
Changes in v3:
- Don't (uselessly) clear the PRTINT anymore (Felipe Balbi).

Changes in v2: None

 drivers/usb/dwc2/core_intr.c | 49 +---
 drivers/usb/dwc2/hcd_intr.c  | 16 +++
 2 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 61601d16e233..d85c5c9f96c1 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -86,9 +86,6 @@ static void dwc2_handle_usb_port_intr(struct dwc2_hsotg 
*hsotg)
hprt0 &= ~HPRT0_ENA;
dwc2_writel(hprt0, hsotg->regs + HPRT0);
}
-
-   /* Clear interrupt */
-   dwc2_writel(GINTSTS_PRTINT, hsotg->regs + GINTSTS);
 }
 
 /**
@@ -98,11 +95,11 @@ static void dwc2_handle_usb_port_intr(struct dwc2_hsotg 
*hsotg)
  */
 static void dwc2_handle_mode_mismatch_intr(struct dwc2_hsotg *hsotg)
 {
-   dev_warn(hsotg->dev, "Mode Mismatch Interrupt: currently in %s mode\n",
-dwc2_is_host_mode(hsotg) ? "Host" : "Device");
-
/* Clear interrupt */
dwc2_writel(GINTSTS_MODEMIS, hsotg->regs + GINTSTS);
+
+   dev_warn(hsotg->dev, "Mode Mismatch Interrupt: currently in %s mode\n",
+dwc2_is_host_mode(hsotg) ? "Host" : "Device");
 }
 
 /**
@@ -276,9 +273,13 @@ static void dwc2_handle_otg_intr(struct dwc2_hsotg *hsotg)
  */
 static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
 {
-   u32 gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+   u32 gintmsk;
+
+   /* Clear interrupt */
+   dwc2_writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
 
/* Need to disable SOF interrupt immediately */
+   gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
gintmsk &= ~GINTSTS_SOF;
dwc2_writel(gintmsk, hsotg->regs + GINTMSK);
 
@@ -295,9 +296,6 @@ static void dwc2_handle_conn_id_status_change_intr(struct 
dwc2_hsotg *hsotg)
queue_work(hsotg->wq_otg, >wf_otg);
spin_lock(>lock);
}
-
-   /* Clear interrupt */
-   dwc2_writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
 }
 
 /**
@@ -315,12 +313,12 @@ static void dwc2_handle_session_req_intr(struct 
dwc2_hsotg *hsotg)
 {
int ret;
 
-   dev_dbg(hsotg->dev, "Session request interrupt - lx_state=%d\n",
-   hsotg->lx_state);
-
/* Clear interrupt */
dwc2_writel(GINTSTS_SESSREQINT, hsotg->regs + GINTSTS);
 
+   dev_dbg(hsotg->dev, "Session request interrupt - lx_state=%d\n",
+   hsotg->lx_state);
+
if (dwc2_is_device_mode(hsotg)) {
if (hsotg->lx_state == DWC2_L2) {
ret = dwc2_exit_hibernation(hsotg, true);
@@ -347,6 +345,10 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg 
*hsotg)
 static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
 {
int ret;
+
+   /* Clear interrupt */
+   dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
+
dev_dbg(hsotg->dev, "++Resume or Remote Wakeup Detected Interrupt++\n");
dev_dbg(hsotg->dev, "%s lxstate = %d\n", __func__, hsotg->lx_state);
 
@@ -368,10 +370,9 @@ static void dwc2_handle_wakeup_detected_intr(struct 
dwc2_hsotg *hsotg)
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;
} else {
-   if (hsotg->core_params->hibernation) {
-   dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
+   if (hsotg->core_params->hibernation)
return;
-

[PATCH v3 1/2] usb: dwc2: host: Fix missing device insertions

[Douglas Anderson]

If you've got your interrupt signals bouncing a bit as you insert your
USB device, you might end up in a state when the device is connected but
the driver doesn't know it.

Specifically, the observed order is:
 1. hardware sees connect
 2. hardware sees disconnect
 3. hardware sees connect
 4. dwc2_port_intr() - clears connect interrupt
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

Now you'll be stuck with the cable plugged in and no further interrupts
coming in but the driver will think we're disconnected.

We'll fix this by checking for the missing connect interrupt and
re-connecting after the disconnect is posted.  We don't skip the
disconnect because if there is a transitory disconnect we really want to
de-enumerate and re-enumerate.

Notes:
1. As part of this change we add a "force" parameter to
   dwc2_hcd_disconnect() so that when we're unloading the module we
   avoid the new behavior.  The need for this was pointed out by John
   Youn.
2. The bit of code needed at the end of dwc2_hcd_disconnect() is
   exactly the same bit of code from dwc2_port_intr().  To avoid
   duplication, we refactor that code out into a new function
   dwc2_hcd_connect().

Signed-off-by: Douglas Anderson 
Acked-by: John Youn 
Tested-by: John Youn 
---
Changes in v3:
- Add notes to device insertions commit message (Felipe Balbi)

Changes in v2:
- Don't reconnect when called from _dwc2_hcd_stop() (John Youn)

 drivers/usb/dwc2/core.h  |  6 --
 drivers/usb/dwc2/core_intr.c |  4 ++--
 drivers/usb/dwc2/hcd.c   | 40 ++--
 drivers/usb/dwc2/hcd_intr.c  |  6 +-
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a66d3cb62b65..daa1c342cdac 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1154,12 +1154,14 @@ static inline int dwc2_hsotg_set_test_mode(struct 
dwc2_hsotg *hsotg,
 
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
 extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
 #else
 static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
 { return 0; }
-static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_connect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) {}
 static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
 static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
 static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 27daa42788b1..61601d16e233 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -239,7 +239,7 @@ static void dwc2_handle_otg_intr(struct dwc2_hsotg *hsotg)
dev_dbg(hsotg->dev, "a_suspend->a_peripheral (%d)\n",
hsotg->op_state);
spin_unlock(>lock);
-   dwc2_hcd_disconnect(hsotg);
+   dwc2_hcd_disconnect(hsotg, false);
spin_lock(>lock);
hsotg->op_state = OTG_STATE_A_PERIPHERAL;
} else {
@@ -401,7 +401,7 @@ static void dwc2_handle_disconnect_intr(struct dwc2_hsotg 
*hsotg)
dwc2_op_state_str(hsotg));
 
if (hsotg->op_state == OTG_STATE_A_HOST)
-   dwc2_hcd_disconnect(hsotg);
+   dwc2_hcd_disconnect(hsotg, false);
 
dwc2_writel(GINTSTS_DISCONNINT, hsotg->regs + GINTSTS);
 }
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index e79baf73c234..e208eac7ff57 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -268,15 +268,33 @@ static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg 
*hsotg)
 }
 
 /**
+ * dwc2_hcd_connect() - Handles connect of the HCD
+ *
+ * @hsotg: Pointer to struct dwc2_hsotg
+ *
+ * Must be called with interrupt disabled and spinlock held
+ */
+void dwc2_hcd_connect(struct dwc2_hsotg *hsotg)
+{
+   if (hsotg->lx_state != DWC2_L0)
+   usb_hcd_resume_root_hub(hsotg->priv);
+
+   hsotg->flags.b.port_connect_status_change = 1;
+   hsotg->flags.b.port_connect_status = 1;
+}
+
+/**
  * dwc2_hcd_disconnect() - Handles disconnect of the HCD
  *
  * @hsotg: Pointer to struct dwc2_hsotg
+ * @force: If true, we won't try to reconnect even if we see device connected.
  *
  * Must be called with interrupt disabled and spinlock held
  */
-void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg)
+void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force)
 {
u32 

Re: [PATCH] usb: gadget: pxa27x: fix suspend callback

[Felipe Balbi]

Hi,

Robert Jarzmik  writes:
> Felipe Balbi  writes:
>
>> pxa27x disconnects pullups on suspend but doesn't
>> notify the gadget driver about it, so gadget driver
>> can't disable the endpoints it was using.
>>
>> This causes problems on resume because gadget core
>> will think endpoints are still enabled and just
>> ignore the following usb_ep_enable().
>>
>> Fix this problem by calling
>> gadget_driver->disconnect().
> Thanks for doing this for me.
>> @@ -2535,6 +2535,7 @@ static int pxa_udc_suspend(struct platform_device 
>> *_dev, pm_message_t state)
>>  udc_disable(udc);
>>  udc->pullup_resume = udc->pullup_on;
>>  dplus_pullup(udc, 0);
>> +udc->driver->disconnect(>gadget);
> If no driver is bound, this will segfault, right ?
> Shouldn't an "if (udc->driver)" protect this line ?

good catch. v2 coming shortly.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: gadget: pxa27x: fix suspend callback

[Felipe Balbi]

pxa27x disconnects pullups on suspend but doesn't
notify the gadget driver about it, so gadget driver
can't disable the endpoints it was using.

This causes problems on resume because gadget core
will think endpoints are still enabled and just
ignore the following usb_ep_enable().

Fix this problem by calling
gadget_driver->disconnect().

Cc:  # v3.10+
Signed-off-by: Felipe Balbi 
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index 670ac0b12f00..001a3b74a993 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2536,6 +2536,9 @@ static int pxa_udc_suspend(struct platform_device *_dev, 
pm_message_t state)
udc->pullup_resume = udc->pullup_on;
dplus_pullup(udc, 0);
 
+   if (udc->driver)
+   udc->driver->disconnect(>gadget);
+
return 0;
 }
 
-- 
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 RESEND 1/2] usb: dwc2: Make PHY optional

[Felipe Balbi]

Hi John,

John Youn  writes:
> Fixes commit 09a75e85
> "usb: dwc2: refactor common low-level hw code to platform.c"

these two lines should be placed ...

>
> The above commit consolidated the low-level phy access into a common
> location. This change introduced a check from the gadget requiring
> that a PHY is specified. This requirement never existed on the host
> side and broke some platforms when it was moved into platform.c.
>
> The gadget doesn't require the PHY either so remove the check.
>

... here with the following format:

Fixes: 09a75e857790 ("usb: dwc2: refactor common low-level
hw code to platform.c")

Just is just FYI, as I have already applied another version ;-)

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: dwc3: add generic OF glue layer

[Subbaraya Sundeep Bhatta]

Hi Felipe,

> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Thursday, November 19, 2015 8:28 PM
> To: Linux USB Mailing List
> Cc: Subbaraya Sundeep Bhatta; Ivan T . Ivanov
> Subject: Re: [PATCH] usb: dwc3: add generic OF glue layer
> 
> 
> Hi,
> 
> Felipe Balbi  writes:
> > For simple platforms which merely enable some clocks and populate its
> > children, we can use this generic glue layer to avoid boilerplate code
> > duplication.
> >
> > For now this supports Qcom and Xilinx, but if we find a way to add
> > generic handling of regulators and optional PHYs, we can absorb exynos
> > as well.
> >
> > Signed-off-by: Felipe Balbi 
> > ---
> >
> > Can you guys check if this works for your respective platforms ? If it
> > does we can some code duplication going forward.
> 
> gentle ping on this one.

It is working on Xilinx platform.

Thanks,
Sundeep

> 
> --
> 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 v2 2/2] usb: phy: mxs: add "fsl,imx6ul-usbphy" compatible string

[Felipe Balbi]

Hi,

Peter Chen  writes:
> On Wed, Sep 16, 2015 at 03:52:33PM +0800, Li Jun wrote:
>> From: Peter Chen 
>> 
>> Add "fsl,imx6ul-usbphy" compatible string for iMX6ul usb phy
>> 
>> Signed-off-by: Peter Chen 
>> Signed-off-by: Li Jun 
>> ---
>>  drivers/usb/phy/phy-mxs-usb.c | 5 +
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
>> index 4d863eb..630f643 100644
>> --- a/drivers/usb/phy/phy-mxs-usb.c
>> +++ b/drivers/usb/phy/phy-mxs-usb.c
>> @@ -143,12 +143,17 @@ static const struct mxs_phy_data imx6sx_phy_data = {
>>  .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,
>>  };
>>  
>> +static const struct mxs_phy_data imx6ul_phy_data = {
>> +.flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,
>> +};
>> +
>>  static const struct of_device_id mxs_phy_dt_ids[] = {
>>  { .compatible = "fsl,imx6sx-usbphy", .data = _phy_data, },
>>  { .compatible = "fsl,imx6sl-usbphy", .data = _phy_data, },
>>  { .compatible = "fsl,imx6q-usbphy", .data = _phy_data, },
>>  { .compatible = "fsl,imx23-usbphy", .data = _phy_data, },
>>  { .compatible = "fsl,vf610-usbphy", .data = _phy_data, },
>> +{ .compatible = "fsl,imx6ul-usbphy", .data = _phy_data, },
>>  { /* sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
>> -- 
>> 1.9.1
>> 
>
> ping...


https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/fixes=4c55c3cdabdcbe6f458ba5192e1df20cc2909488

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 10/13] usb: dwc2: host: enable descriptor dma for fs devices

[Felipe Balbi]

Hi,

Gregory Herrero  writes:
> From: Mian Yousaf Kaukab 
>
> As descriptor dma mode does not support split transfers, it can't be
> enabled for high speed devices. Add a core parameter to enable it for
> full speed devices.
>
> Ensure frame list and descriptor list are correctly freed during
> disconnect.
>
> Signed-off-by: Mian Yousaf Kaukab 
> Signed-off-by: Gregory Herrero 

this one doesn't apply:

Applying: usb: dwc2: host: enable descriptor dma for fs devices
error: drivers/usb/dwc2/core.h: does not match index
error: drivers/usb/dwc2/hcd.c: does not match index
error: drivers/usb/dwc2/hcd_intr.c: does not match index
error: drivers/usb/dwc2/platform.c: does not match index
Patch failed at 0001 usb: dwc2: host: enable descriptor dma for fs devices
The copy of the patch that failed is found in: 
workspace/linux/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Care to rebase on my testing/next ? Patches 1-9 are already applied.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: dwc3: remove dwc3-qcom in favor of dwc3-of-simple

[Felipe Balbi]

Now that we have a generic dwc3-of-simple.c, we can
use that instead of maintaining dwc3-qcom.c which is
extremely similar.

Cc: Ivan T. Ivanov 
Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/Kconfig |   8 ---
 drivers/usb/dwc3/Makefile|   1 -
 drivers/usb/dwc3/dwc3-qcom.c | 130 ---
 3 files changed, 139 deletions(-)
 delete mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 070e704829e5..a64ce1c94d6d 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -105,12 +105,4 @@ config USB_DWC3_ST
  inside (i.e. STiH407).
  Say 'Y' or 'M' if you have one such device.
 
-config USB_DWC3_QCOM
-   tristate "Qualcomm Platforms"
-   depends on ARCH_QCOM || COMPILE_TEST
-   default USB_DWC3
-   help
- Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
- say 'Y' or 'M' if you have one such device.
-
 endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 6491f9b474d4..22420e17d68b 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -38,5 +38,4 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o
 obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)   += dwc3-of-simple.o
-obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
 obj-$(CONFIG_USB_DWC3_ST)  += dwc3-st.o
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
deleted file mode 100644
index 088026048f49..
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ /dev/null
@@ -1,130 +0,0 @@
-/* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-struct dwc3_qcom {
-   struct device   *dev;
-
-   struct clk  *core_clk;
-   struct clk  *iface_clk;
-   struct clk  *sleep_clk;
-};
-
-static int dwc3_qcom_probe(struct platform_device *pdev)
-{
-   struct device_node *node = pdev->dev.of_node;
-   struct dwc3_qcom *qdwc;
-   int ret;
-
-   qdwc = devm_kzalloc(>dev, sizeof(*qdwc), GFP_KERNEL);
-   if (!qdwc)
-   return -ENOMEM;
-
-   platform_set_drvdata(pdev, qdwc);
-
-   qdwc->dev = >dev;
-
-   qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
-   if (IS_ERR(qdwc->core_clk)) {
-   dev_err(qdwc->dev, "failed to get core clock\n");
-   return PTR_ERR(qdwc->core_clk);
-   }
-
-   qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
-   if (IS_ERR(qdwc->iface_clk)) {
-   dev_info(qdwc->dev, "failed to get optional iface clock\n");
-   qdwc->iface_clk = NULL;
-   }
-
-   qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
-   if (IS_ERR(qdwc->sleep_clk)) {
-   dev_info(qdwc->dev, "failed to get optional sleep clock\n");
-   qdwc->sleep_clk = NULL;
-   }
-
-   ret = clk_prepare_enable(qdwc->core_clk);
-   if (ret) {
-   dev_err(qdwc->dev, "failed to enable core clock\n");
-   goto err_core;
-   }
-
-   ret = clk_prepare_enable(qdwc->iface_clk);
-   if (ret) {
-   dev_err(qdwc->dev, "failed to enable optional iface clock\n");
-   goto err_iface;
-   }
-
-   ret = clk_prepare_enable(qdwc->sleep_clk);
-   if (ret) {
-   dev_err(qdwc->dev, "failed to enable optional sleep clock\n");
-   goto err_sleep;
-   }
-
-   ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
-   if (ret) {
-   dev_err(qdwc->dev, "failed to register core - %d\n", ret);
-   goto err_clks;
-   }
-
-   return 0;
-
-err_clks:
-   clk_disable_unprepare(qdwc->sleep_clk);
-err_sleep:
-   clk_disable_unprepare(qdwc->iface_clk);
-err_iface:
-   clk_disable_unprepare(qdwc->core_clk);
-err_core:
-   return ret;
-}
-
-static int dwc3_qcom_remove(struct platform_device *pdev)
-{
-   struct dwc3_qcom *qdwc = platform_get_drvdata(pdev);
-
-   of_platform_depopulate(>dev);
-
-   clk_disable_unprepare(qdwc->sleep_clk);
-   clk_disable_unprepare(qdwc->iface_clk);
-   clk_disable_unprepare(qdwc->core_clk);
-
-   return 0;
-}
-
-static const struct of_device_id 

Re: [PATCH] usb: gadget: pxa27x: fix suspend callback

[Robert Jarzmik]

Felipe Balbi  writes:

> pxa27x disconnects pullups on suspend but doesn't
> notify the gadget driver about it, so gadget driver
> can't disable the endpoints it was using.
>
> This causes problems on resume because gadget core
> will think endpoints are still enabled and just
> ignore the following usb_ep_enable().
>
> Fix this problem by calling
> gadget_driver->disconnect().
Thanks for doing this for me.
> @@ -2535,6 +2535,7 @@ static int pxa_udc_suspend(struct platform_device 
> *_dev, pm_message_t state)
>   udc_disable(udc);
>   udc->pullup_resume = udc->pullup_on;
>   dplus_pullup(udc, 0);
> + udc->driver->disconnect(>gadget);
If no driver is bound, this will segfault, right ?
Shouldn't an "if (udc->driver)" protect this line ?

Cheers.

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

RE: [PATCH] usb: dwc3: add generic OF glue layer

[Subbaraya Sundeep Bhatta]

Hi Felipe,

> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Thursday, November 19, 2015 8:28 PM
> To: Linux USB Mailing List
> Cc: Subbaraya Sundeep Bhatta; Ivan T . Ivanov
> Subject: Re: [PATCH] usb: dwc3: add generic OF glue layer
>
>
> Hi,
>
> Felipe Balbi  writes:
> > For simple platforms which merely enable some clocks and populate its
> > children, we can use this generic glue layer to avoid boilerplate code
> > duplication.
> >
> > For now this supports Qcom and Xilinx, but if we find a way to add
> > generic handling of regulators and optional PHYs, we can absorb exynos
> > as well.
> >
> > Signed-off-by: Felipe Balbi 
> > ---
> >
> > Can you guys check if this works for your respective platforms ? If it
> > does we can some code duplication going forward.
>
> gentle ping on this one.

Tested and working fine on Xilinx platform.

Thanks,
Sundeep

>
> --
> balbi


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

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

RE: [PATCH] usb: dwc3: add generic OF glue layer

[Felipe Balbi]

Hi,

Subbaraya Sundeep Bhatta  writes:
> Hi Felipe,
>
>> -Original Message-
>> From: Felipe Balbi [mailto:ba...@ti.com]
>> Sent: Thursday, November 19, 2015 8:28 PM
>> To: Linux USB Mailing List
>> Cc: Subbaraya Sundeep Bhatta; Ivan T . Ivanov
>> Subject: Re: [PATCH] usb: dwc3: add generic OF glue layer
>>
>>
>> Hi,
>>
>> Felipe Balbi  writes:
>> > For simple platforms which merely enable some clocks and populate its
>> > children, we can use this generic glue layer to avoid boilerplate code
>> > duplication.
>> >
>> > For now this supports Qcom and Xilinx, but if we find a way to add
>> > generic handling of regulators and optional PHYs, we can absorb exynos
>> > as well.
>> >
>> > Signed-off-by: Felipe Balbi 
>> > ---
>> >
>> > Can you guys check if this works for your respective platforms ? If it
>> > does we can some code duplication going forward.
>>
>> gentle ping on this one.
>
> Tested and working fine on Xilinx platform.

thank you, I'll take your DTS with this generic glue, and also delete
qcom glue.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 2/4] Documentation: usb: gadget-testing: add description for depth of queue

[Felipe Balbi]

Hi,

Peter Chen  writes:
> Add both bulk and iso depth of queue for sourcesink.
>
> Signed-off-by: Peter Chen 
> ---
>  Documentation/usb/gadget-testing.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/usb/gadget-testing.txt 
> b/Documentation/usb/gadget-testing.txt
> index b24d3ef..84b3d10 100644
> --- a/Documentation/usb/gadget-testing.txt
> +++ b/Documentation/usb/gadget-testing.txt
> @@ -579,6 +579,8 @@ The SOURCESINK function provides these attributes in its 
> function directory:
>   isoc_mult   - 0..2 (hs/ss only)
>   isoc_maxburst   - 0..15 (ss only)
>   bulk_buflen - buffer length
> + bulk_qlen   - depth of queue for bulk
> + iso_qlen- depth of queue for iso

doesn't apply for me:

Applying: Documentation: usb: gadget-testing: add description for depth of queue
error: patch failed: Documentation/usb/gadget-testing.txt:579
error: Documentation/usb/gadget-testing.txt: patch does not apply
Patch failed at 0001 Documentation: usb: gadget-testing: add description for 
depth of queue
The copy of the patch that failed is found in: 
workspace/linux/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Care to rebase on my testing/next ?

Note that patch 1 is already applied. When rebasing, please collect
Krzysztof's Reviewed-by ;-)

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] usb: dwc2: host: Clear interrupts before handling them

[Felipe Balbi]

Hi,

Doug Anderson  writes:
 isn't this a regression ? You're first clearing the interrupts and only
 then reading to check what's pending, however, what's pending has just
 been cleared. Seems like this should be:

 hprt0 = dwc2_readl(HPRT0);
 dwc2_writeal(PRTINT, GINTSTS);
>>>
>>> Actually, we could probably remove the setting of GINTSTS_PRTINT
>>> completely.  The docs I have say that the GINTSTS_PRTINT is read only
>>> and that:
>>>
 The core sets this bit to indicate a change in port status of one of the
 DWC_otg core ports in Host mode. The application must read the
 Host Port Control and Status (HPRT) register to determine the exact
 event that caused this interrupt. The application must clear the
 appropriate status bit in the Host Port Control and Status register to
 clear this bit.
>>>
>>> ...so writing PRTINT is probably useless, but John can confirm.
>>>
>>
>> Yup, it seems it can be removed.
>
> How do you guys want this handled?  Should I send up a new version of
> this patch?  ...or should I send a followon patch that does this
> removal?

I'll leave the final decision to John, but my opinion is that a new
version of the patch would be preferrable.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 2/2] usb: dwc2: host: Clear interrupts before handling them

[Antti Seppälä]

On 19 November 2015 at 21:45, Douglas Anderson  wrote:
> In general it is wise to clear interrupts before processing them.  If
> you don't do that, you can get:
>  1. Interrupt happens
>  2. You look at system state and process interrupt
>  3. A new interrupt happens
>  4. You clear interrupt without processing it.
>
> This patch was actually a first attempt to fix missing device insertions
> as described in (usb: dwc2: host: Fix missing device insertions) and it
> did solve some of the signal bouncing problems but not all of
> them (which is why I submitted the other patch).  Specifically, this
> patch itself would sometimes change:
>  1. hardware sees connect
>  2. hardware sees disconnect
>  3. hardware sees connect
>  4. dwc2_port_intr() - clears connect interrupt
>  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>
> ...to:
>  1. hardware sees connect
>  2. hardware sees disconnect
>  3. dwc2_port_intr() - clears connect interrupt
>  4. hardware sees connect
>  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>
> ...but with different timing then sometimes we'd still miss cable
> insertions.
>
> In any case, though this patch doesn't fix any (known) problems, it
> still seems wise as a general policy to clear interrupt before handling
> them.
>
> Signed-off-by: Douglas Anderson 
> Acked-by: John Youn 
> Tested-by: John Youn 
> ---
> Changes in v3:
> - Don't (uselessly) clear the PRTINT anymore (Felipe Balbi).
>
> Changes in v2: None
>

Hi.

It seems that towards the end of hcd_intr.c you seem to be switching a
few calls of dwc2_writel() to plain writel(). It looks like something
that is easily overlooked but please always use dwc2_writel to
preserve correct register endianness on big-endian platforms.

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

[PATCH 1/1] usb: phy: omap-otg: do not write to unallocated memory

[Heinrich Schuchardt]

The current coding writes to memory before allocating it.

Signed-off-by: Heinrich Schuchardt 
---
 drivers/usb/phy/phy-omap-otg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
index 1270906..8bbf121 100644
--- a/drivers/usb/phy/phy-omap-otg.c
+++ b/drivers/usb/phy/phy-omap-otg.c
@@ -105,12 +105,13 @@ static int omap_otg_probe(struct platform_device *pdev)
extcon = extcon_get_extcon_dev(config->extcon);
if (!extcon)
return -EPROBE_DEFER;
-   otg_dev->extcon = extcon;
 
otg_dev = devm_kzalloc(>dev, sizeof(*otg_dev), GFP_KERNEL);
if (!otg_dev)
return -ENOMEM;
 
+   otg_dev->extcon = extcon;
+
otg_dev->base = devm_ioremap_resource(>dev, >resource[0]);
if (IS_ERR(otg_dev->base))
return PTR_ERR(otg_dev->base);
-- 
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] HID: usbhid: add Logitech G710+ keyboard quirk NOGET

[Greg KH]

On Tue, Nov 17, 2015 at 12:40:12AM -0600, Jimmy Berry wrote:
> Without quirk keyboard repeats '6' until volume control is used since it
> indicates the key is pressed without ever releasing.
> 
> Signed-off-by: Jimmy Berry 
> ---
>  drivers/hid/hid-ids.h   | 1 +
>  drivers/hid/usbhid/hid-quirks.c | 1 +
>  2 files changed, 2 insertions(+)

Please use scripts/get_maintainer.pl to find the right people to cc: and
the correct mailing list for your patch (hint, it's not me...)

thanks,

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

[PATCH] usb-storage: Fix scsi-sd failure "Invalid field in cdb" for USB adapter JMicron

[Dmitry Katsubo]

From: Dmitry Katsubo 

The patch extends the family of SATA-to-USB JMicron adapters that need
FUA to be disabled and applies the same policy for uas driver.
See details in http://unix.stackexchange.com/questions/237204/

Signed-off-by: Dmitry Katsubo 
Tested-by: Dmitry Katsubo 
---
The change is trivial, however it spans also to JMicron adapter with
bcdDevice 1.15, which I haven't tested. Nevertheless it is very likely
that it is buggy as well. Patch was applied and tested on Debian Jessie
4.2.5-1~bpo8+1. There is a checkpatch warning, but it is caused by original
source code formatting.

---
 drivers/usb/storage/uas.c  | 4 
 drivers/usb/storage/unusual_devs.h | 2 +-
 drivers/usb/storage/unusual_uas.h  | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 48ca9c2..ce37e30 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -796,6 +796,10 @@ static int uas_slave_configure(struct scsi_device *sdev)
if (devinfo->flags & US_FL_NO_REPORT_OPCODES)
sdev->no_report_opcodes = 1;
 
+   /* A few buggy USB-ATA bridges don't understand FUA */
+   if (devinfo->flags & US_FL_BROKEN_FUA)
+   sdev->broken_fua = 1;
+
scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
return 0;
 }
diff --git a/drivers/usb/storage/unusual_devs.h 
b/drivers/usb/storage/unusual_devs.h
index 6b24791..7ffe420 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1987,7 +1987,7 @@ UNUSUAL_DEV(  0x14cd, 0x6600, 0x0201, 0x0201,
US_FL_IGNORE_RESIDUE ),
 
 /* Reported by Michael Büsch  */
-UNUSUAL_DEV(  0x152d, 0x0567, 0x0114, 0x0114,
+UNUSUAL_DEV(  0x152d, 0x0567, 0x0114, 0x0116,
"JMicron",
"USB to ATA/ATAPI Bridge",
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index c85ea53..ccc113e 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -132,7 +132,7 @@ UNUSUAL_DEV(0x152d, 0x0567, 0x, 0x,
"JMicron",
"JMS567",
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
-   US_FL_NO_REPORT_OPCODES),
+   US_FL_BROKEN_FUA | US_FL_NO_REPORT_OPCODES),
 
 /* Reported-by: Hans de Goede  */
 UNUSUAL_DEV(0x2109, 0x0711, 0x, 0x,
-- 
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 v3 2/2] usb: dwc2: host: Clear interrupts before handling them

[Doug Anderson]

Antti,

On Thu, Nov 19, 2015 at 1:09 PM, Antti Seppälä  wrote:
> On 19 November 2015 at 21:45, Douglas Anderson  wrote:
>> In general it is wise to clear interrupts before processing them.  If
>> you don't do that, you can get:
>>  1. Interrupt happens
>>  2. You look at system state and process interrupt
>>  3. A new interrupt happens
>>  4. You clear interrupt without processing it.
>>
>> This patch was actually a first attempt to fix missing device insertions
>> as described in (usb: dwc2: host: Fix missing device insertions) and it
>> did solve some of the signal bouncing problems but not all of
>> them (which is why I submitted the other patch).  Specifically, this
>> patch itself would sometimes change:
>>  1. hardware sees connect
>>  2. hardware sees disconnect
>>  3. hardware sees connect
>>  4. dwc2_port_intr() - clears connect interrupt
>>  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>>
>> ...to:
>>  1. hardware sees connect
>>  2. hardware sees disconnect
>>  3. dwc2_port_intr() - clears connect interrupt
>>  4. hardware sees connect
>>  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()
>>
>> ...but with different timing then sometimes we'd still miss cable
>> insertions.
>>
>> In any case, though this patch doesn't fix any (known) problems, it
>> still seems wise as a general policy to clear interrupt before handling
>> them.
>>
>> Signed-off-by: Douglas Anderson 
>> Acked-by: John Youn 
>> Tested-by: John Youn 
>> ---
>> Changes in v3:
>> - Don't (uselessly) clear the PRTINT anymore (Felipe Balbi).
>>
>> Changes in v2: None
>>
>
> Hi.
>
> It seems that towards the end of hcd_intr.c you seem to be switching a
> few calls of dwc2_writel() to plain writel(). It looks like something
> that is easily overlooked but please always use dwc2_writel to
> preserve correct register endianness on big-endian platforms.

Oops!  That was a very dumb oversight, thanks for catching.  Sorry
about that.  Fix coming.

-Doug
--
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: pxa27x: fix suspend callback

[Robert Jarzmik]

Felipe Balbi  writes:

> pxa27x disconnects pullups on suspend but doesn't
> notify the gadget driver about it, so gadget driver
> can't disable the endpoints it was using.
>
> This causes problems on resume because gadget core
> will think endpoints are still enabled and just
> ignore the following usb_ep_enable().
>
> Fix this problem by calling
> gadget_driver->disconnect().
>
> Cc:  # v3.10+
> Signed-off-by: Felipe Balbi 
Tested-by: Robert Jarzmik 

Cheers.

--
Robert
--
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: dwc2: add support of hi6220

[Felipe Balbi]

Hi,

Zhangfei Gao  writes:
> Support hisilicon,hi6220-usb for HiKey board
>
> Signed-off-by: Zhangfei Gao 

doesn't apply:

Applying: usb: dwc2: add support of hi6220
error: drivers/usb/dwc2/platform.c: does not match index
Patch failed at 0001 usb: dwc2: add support of hi6220
The copy of the patch that failed is found in: 
workspace/linux/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Care to rebase on my testing/next and also collect John's and Rob's ack ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 6/8] usb: dwc2: host: Assume all devices are on one single_tt hub

[Felipe Balbi]

Hi,

Doug Anderson  writes:
>> Douglas Anderson  writes:
>>> Until we have logic to determine which devices share the same TT let's
>>> add logic to assume that all devices on a given dwc2 controller are on
>>> one single_tt hub.  This is better than the previous code that assumed
>>> that all devices were on one multi_tt hub, since single_tt hubs
>>> appear (in my experience) to be much more common and any schedule that
>>> would work on a single_tt hub will also work on a multi_tt hub.  This
>>> will prevent more than 8 total low/full speed devices to be on the bus
>>> at one time, but that's a reasonable restriction until we've made things
>>> smarter.
>>>
>>> Signed-off-by: Douglas Anderson 
>>> ---
>>> Changes in v3:
>>> - Assuming single_tt is new for v3; not terribly well tested (yet).
>>>
>>> Changes in v2: None
>>>
>>>  drivers/usb/dwc2/core.h  |  1 +
>>>  drivers/usb/dwc2/hcd_queue.c | 40 +++-
>>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 567ee2c9e69f..09aa2b5ae29e 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -782,6 +782,7 @@ struct dwc2_hsotg {
>>>   u16 periodic_usecs;
>>>   unsigned long periodic_bitmap[DIV_ROUND_UP(TOTAL_PERIODIC_USEC,
>>>  BITS_PER_LONG)];
>>> + bool has_split[8];
>>
>> why don't you use a u8 instead then just set each bit for each
>> "has_split" you need to take care of. This array is kinda ugly.
>
> Let's actually drop this patch completely.  Julius and I sat down and
> he talked me through things, and with my current understanding the
> current microframe scheduler in dwc2 is broken enough that small
> little band-aids like this will do little more than just push the
> problems around.
>
> I'm a good portion of the way through a better microframe scheduler.
> I have no doubt that it won't be perfect, but hopefully it will at
> least be based in reality...
>
> My latest thinking on the patches in this series:
>
> 1. usb: dwc2: rockchip: Make the max_transfer_size automatic
>
> I'll probably separate this out into its own patch so I can stop
> sending it as part of this series.  ...or if someone wanted to land it
> then I won't bother.
>
>
> 2. usb: dwc2: host: Get aligned DMA in a more supported way
>
> Still can land any time and has good benefits.  I believe that I can't
> separate this because it will cause conflicts with scheduler patches.
>
>
> 3. usb: dwc2: host: Add scheduler tracing
>
> Would be nice to land.
>
>
> 4. usb: dwc2: host: Rewrite the microframe scheduler
> 5. usb: dwc2: host: Keep track of and use our scheduled microframe
> 6. usb: dwc2: host: Assume all devices are on one single_tt hub
>
> Please drop patches 4-6 right now.
>
>
> 7. usb: dwc2: host: Add a delay before releasing periodic bandwidth
> 8. usb: dwc2: host: Giveback URB in tasklet context

if you can, it's best to send a new series with the changes. This makes
mine and John's life a lot easier :-)

> I'll probably move these back up in the series (like in v2) and put
> microframe rewrite atop them.  With my current understanding the
> scheduling is so broken today that the concerns Alan brought up can
> wait until we have a proper scheduler to be addressed.  In the
> meantime getting the huge boost in interrupt speed will help with
> dwc2's correctness (and performance) because it means we're much less
> likely to miss SOF interrupts.
>
> If anyone has any review time, giving a review to v2 of these patches
> would be helpful.  Otherwise I'll double check that v2 still looks
> good with my current understanding and eventually post them again.

That would have to be someone experienced with that IP. I don't even
have docs :-)

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v4 1/2] usb: dwc2: host: Fix missing device insertions

[Douglas Anderson]

If you've got your interrupt signals bouncing a bit as you insert your
USB device, you might end up in a state when the device is connected but
the driver doesn't know it.

Specifically, the observed order is:
 1. hardware sees connect
 2. hardware sees disconnect
 3. hardware sees connect
 4. dwc2_port_intr() - clears connect interrupt
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

Now you'll be stuck with the cable plugged in and no further interrupts
coming in but the driver will think we're disconnected.

We'll fix this by checking for the missing connect interrupt and
re-connecting after the disconnect is posted.  We don't skip the
disconnect because if there is a transitory disconnect we really want to
de-enumerate and re-enumerate.

Notes:
1. As part of this change we add a "force" parameter to
   dwc2_hcd_disconnect() so that when we're unloading the module we
   avoid the new behavior.  The need for this was pointed out by John
   Youn.
2. The bit of code needed at the end of dwc2_hcd_disconnect() is
   exactly the same bit of code from dwc2_port_intr().  To avoid
   duplication, we refactor that code out into a new function
   dwc2_hcd_connect().

Signed-off-by: Douglas Anderson 
Acked-by: John Youn 
Tested-by: John Youn 
---
Changes in v4: None
Changes in v3:
- Add notes to device insertions commit message (Felipe Balbi)

Changes in v2:
- Don't reconnect when called from _dwc2_hcd_stop() (John Youn)

 drivers/usb/dwc2/core.h  |  6 --
 drivers/usb/dwc2/core_intr.c |  4 ++--
 drivers/usb/dwc2/hcd.c   | 40 ++--
 drivers/usb/dwc2/hcd_intr.c  |  6 +-
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a66d3cb62b65..daa1c342cdac 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1154,12 +1154,14 @@ static inline int dwc2_hsotg_set_test_mode(struct 
dwc2_hsotg *hsotg,
 
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
-extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
+extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
 extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
 #else
 static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
 { return 0; }
-static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_connect(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) {}
 static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
 static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
 static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 27daa42788b1..61601d16e233 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -239,7 +239,7 @@ static void dwc2_handle_otg_intr(struct dwc2_hsotg *hsotg)
dev_dbg(hsotg->dev, "a_suspend->a_peripheral (%d)\n",
hsotg->op_state);
spin_unlock(>lock);
-   dwc2_hcd_disconnect(hsotg);
+   dwc2_hcd_disconnect(hsotg, false);
spin_lock(>lock);
hsotg->op_state = OTG_STATE_A_PERIPHERAL;
} else {
@@ -401,7 +401,7 @@ static void dwc2_handle_disconnect_intr(struct dwc2_hsotg 
*hsotg)
dwc2_op_state_str(hsotg));
 
if (hsotg->op_state == OTG_STATE_A_HOST)
-   dwc2_hcd_disconnect(hsotg);
+   dwc2_hcd_disconnect(hsotg, false);
 
dwc2_writel(GINTSTS_DISCONNINT, hsotg->regs + GINTSTS);
 }
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index e79baf73c234..e208eac7ff57 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -268,15 +268,33 @@ static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg 
*hsotg)
 }
 
 /**
+ * dwc2_hcd_connect() - Handles connect of the HCD
+ *
+ * @hsotg: Pointer to struct dwc2_hsotg
+ *
+ * Must be called with interrupt disabled and spinlock held
+ */
+void dwc2_hcd_connect(struct dwc2_hsotg *hsotg)
+{
+   if (hsotg->lx_state != DWC2_L0)
+   usb_hcd_resume_root_hub(hsotg->priv);
+
+   hsotg->flags.b.port_connect_status_change = 1;
+   hsotg->flags.b.port_connect_status = 1;
+}
+
+/**
  * dwc2_hcd_disconnect() - Handles disconnect of the HCD
  *
  * @hsotg: Pointer to struct dwc2_hsotg
+ * @force: If true, we won't try to reconnect even if we see device connected.
  *
  * Must be called with interrupt disabled and spinlock held
  */
-void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg)
+void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool 

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

[Jiri Kosina]

On Thu, 19 Nov 2015, Ioan-Adrian Ratiu wrote:

> But please understand further my reasoning for submitting this patch. 
> Consider if this is a bug in the wacom driver or in the usbhid core? IMO 
> this is a usbhid bug: the critical region in hid_ctrl() is too big, 
> there is no reason for the call to hid_input_report() to be protected by 
> usbhid->lock.

Hmm, it's actually true that we might not need usbhid->lock during 
hid_input_report() at the end of the day, as we shouldn't be doing any 
URB-related operations there, neither iofl are being manipulated.

If you have already done the full analysis that shows that usbhid->lock is 
indeed not needed, this absolutely needs to go into changelog as proper 
justification.

Could you please reformulate the changelog in this respect and resubmit?

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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 v4 2/2] usb: dwc2: host: Clear interrupts before handling them

[Douglas Anderson]

In general it is wise to clear interrupts before processing them.  If
you don't do that, you can get:
 1. Interrupt happens
 2. You look at system state and process interrupt
 3. A new interrupt happens
 4. You clear interrupt without processing it.

This patch was actually a first attempt to fix missing device insertions
as described in (usb: dwc2: host: Fix missing device insertions) and it
did solve some of the signal bouncing problems but not all of
them (which is why I submitted the other patch).  Specifically, this
patch itself would sometimes change:
 1. hardware sees connect
 2. hardware sees disconnect
 3. hardware sees connect
 4. dwc2_port_intr() - clears connect interrupt
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

...to:
 1. hardware sees connect
 2. hardware sees disconnect
 3. dwc2_port_intr() - clears connect interrupt
 4. hardware sees connect
 5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

...but with different timing then sometimes we'd still miss cable
insertions.

In any case, though this patch doesn't fix any (known) problems, it
still seems wise as a general policy to clear interrupt before handling
them.

Note that for dwc2_handle_usb_port_intr(), instead of moving the clear
of PRTINT to the beginning of the function we remove it completely.  The
only way to clear PRTINT is to clear the sources that set it in the
first place.

Signed-off-by: Douglas Anderson 
Acked-by: John Youn 
Tested-by: John Youn 
---
Changes in v4:
- Don't replace dwc2_writel with writel (Antti Seppälä).
- Update description to explain why we remove PRTINT clear.

Changes in v3:
- Don't (uselessly) clear the PRTINT anymore (Felipe Balbi).

Changes in v2: None

 drivers/usb/dwc2/core_intr.c | 49 +---
 drivers/usb/dwc2/hcd_intr.c  | 17 ---
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 61601d16e233..d85c5c9f96c1 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -86,9 +86,6 @@ static void dwc2_handle_usb_port_intr(struct dwc2_hsotg 
*hsotg)
hprt0 &= ~HPRT0_ENA;
dwc2_writel(hprt0, hsotg->regs + HPRT0);
}
-
-   /* Clear interrupt */
-   dwc2_writel(GINTSTS_PRTINT, hsotg->regs + GINTSTS);
 }
 
 /**
@@ -98,11 +95,11 @@ static void dwc2_handle_usb_port_intr(struct dwc2_hsotg 
*hsotg)
  */
 static void dwc2_handle_mode_mismatch_intr(struct dwc2_hsotg *hsotg)
 {
-   dev_warn(hsotg->dev, "Mode Mismatch Interrupt: currently in %s mode\n",
-dwc2_is_host_mode(hsotg) ? "Host" : "Device");
-
/* Clear interrupt */
dwc2_writel(GINTSTS_MODEMIS, hsotg->regs + GINTSTS);
+
+   dev_warn(hsotg->dev, "Mode Mismatch Interrupt: currently in %s mode\n",
+dwc2_is_host_mode(hsotg) ? "Host" : "Device");
 }
 
 /**
@@ -276,9 +273,13 @@ static void dwc2_handle_otg_intr(struct dwc2_hsotg *hsotg)
  */
 static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
 {
-   u32 gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+   u32 gintmsk;
+
+   /* Clear interrupt */
+   dwc2_writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
 
/* Need to disable SOF interrupt immediately */
+   gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
gintmsk &= ~GINTSTS_SOF;
dwc2_writel(gintmsk, hsotg->regs + GINTMSK);
 
@@ -295,9 +296,6 @@ static void dwc2_handle_conn_id_status_change_intr(struct 
dwc2_hsotg *hsotg)
queue_work(hsotg->wq_otg, >wf_otg);
spin_lock(>lock);
}
-
-   /* Clear interrupt */
-   dwc2_writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
 }
 
 /**
@@ -315,12 +313,12 @@ static void dwc2_handle_session_req_intr(struct 
dwc2_hsotg *hsotg)
 {
int ret;
 
-   dev_dbg(hsotg->dev, "Session request interrupt - lx_state=%d\n",
-   hsotg->lx_state);
-
/* Clear interrupt */
dwc2_writel(GINTSTS_SESSREQINT, hsotg->regs + GINTSTS);
 
+   dev_dbg(hsotg->dev, "Session request interrupt - lx_state=%d\n",
+   hsotg->lx_state);
+
if (dwc2_is_device_mode(hsotg)) {
if (hsotg->lx_state == DWC2_L2) {
ret = dwc2_exit_hibernation(hsotg, true);
@@ -347,6 +345,10 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg 
*hsotg)
 static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
 {
int ret;
+
+   /* Clear interrupt */
+   dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
+
dev_dbg(hsotg->dev, "++Resume or Remote Wakeup Detected Interrupt++\n");
dev_dbg(hsotg->dev, "%s lxstate = %d\n", __func__, hsotg->lx_state);
 
@@ -368,10 +370,9 @@ static void 

Re: [GIT PULL] USB-serial fixes for v4.4-rc2

[Greg Kroah-Hartman]

On Thu, Nov 19, 2015 at 11:20:47AM +0100, Johan Hovold wrote:
> Hi Greg,
> 
> Here are a few fixes and new device ids for v4.4-rc2. All have been in
> linux-next for a few days now.
> 
> Thanks,
> Johan
> 
> 
> The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec:
> 
>   Linux 4.4-rc1 (2015-11-15 17:00:27 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
> tags/usb-serial-4.4-rc2

Pulled and pushed out, thanks.

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

Re: [PATCH RESEND] USB: serial: cp210x: Add tx_empty()

[Johan Hovold]

On Wed, Nov 11, 2015 at 03:47:21PM -0600, Konstantin Shkolnyy wrote:

Please make the commit message self-contained even if it means repeating
what callback you're implementing here.

> Without this function, when the port is closed the data in the chip's
> transmit FIFO are lost. If the actual byte count is reported the close
> can be delayed until all data are sent.

You could mention that the tx_empty callback is used to implement
generic wait-until-sent support.

> Signed-off-by: Konstantin Shkolnyy 
> ---
>  drivers/usb/serial/cp210x.c | 60 
> +
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index e91b654..756e432 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -38,6 +38,7 @@ static void cp210x_change_speed(struct tty_struct *, struct 
> usb_serial_port *,
>   struct ktermios *);
>  static void cp210x_set_termios(struct tty_struct *, struct usb_serial_port *,
>   struct ktermios*);
> +static bool cp210x_tx_empty(struct usb_serial_port *port);
>  static int cp210x_tiocmget(struct tty_struct *);
>  static int cp210x_tiocmset(struct tty_struct *, unsigned int, unsigned int);
>  static int cp210x_tiocmset_port(struct usb_serial_port *port,
> @@ -215,6 +216,7 @@ static struct usb_serial_driver cp210x_device = {
>   .close  = cp210x_close,
>   .break_ctl  = cp210x_break_ctl,
>   .set_termios= cp210x_set_termios,
> + .tx_empty   = cp210x_tx_empty,
>   .tiocmget   = cp210x_tiocmget,
>   .tiocmset   = cp210x_tiocmset,
>   .port_probe = cp210x_port_probe,
> @@ -301,6 +303,18 @@ static struct usb_serial_driver * const serial_drivers[] 
> = {
>  #define CONTROL_WRITE_DTR0x0100
>  #define CONTROL_WRITE_RTS0x0200
>  
> +/* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
> +#define CP210X_COMM_STATUS_SIZE 0x13
> +struct cp210x_comm_status {
> + __le32   ulErrors;
> + __le32   ulHoldReasons;
> + __le32   ulAmountInInQueue;
> + __le32   ulAmountInOutQueue;
> + u8   bEofReceived;
> + u8   bWaitForImmediate;
> + u8   bReserved;
> +};

Add a __packed attribute here so that you can use sizeof and drop the
size-define.

> +
>  /*
>   * CP210X_PURGE - 16 bits passed in wValue of USB request.
>   * SiLabs app note AN571 gives a strange description of the 4 bits:
> @@ -551,6 +565,52 @@ static void cp210x_close(struct usb_serial_port *port)
>  }
>  
>  /*
> + * Read how many bytes are waiting in the TX queue.
> + */
> +static int cp210x_get_tx_queue_byte_count(struct usb_serial_port *port,
> + u32 *count)

Indent continuation lines at least two tabs (also below).

> +{
> + struct usb_serial *serial = port->serial;
> + struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
> + struct cp210x_comm_status *sts;
> + int result;
> +
> + sts = kmalloc(CP210X_COMM_STATUS_SIZE, GFP_KERNEL);
> + if (!sts)
> + return -ENOMEM;
> +
> + result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> + CP210X_GET_COMM_STATUS, REQTYPE_INTERFACE_TO_HOST, 0x,

Just use 0 here.

> + port_priv->bInterfaceNumber, sts, CP210X_COMM_STATUS_SIZE,

And sizeof(*sts) here (and below).

> + USB_CTRL_GET_TIMEOUT);
> + if (result == CP210X_COMM_STATUS_SIZE) {
> + *count = le32_to_cpu(sts->ulAmountInOutQueue);
> + result = 0;
> + } else {
> + dev_dbg(>dev, "%s error: size=%d result=%d\n",
> + __func__, CP210X_COMM_STATUS_SIZE, result);

This should be a dev_err, skip the __func__ and spell out the error
(e.g. "failed to get comm status: %d\n"). No need to report the constant
size.

> + if (result >= 0)
> + result = -EPROTO;
> + }
> +
> + kfree(sts);
> +
> + return result;
> +}

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

Re: [PATCH] usb: gadget: Add the console support for usb-to-serial port

[Baolin Wang]

On 19 November 2015 at 18:28, Peter Hurley  wrote:
> On 11/18/2015 09:35 PM, Baolin Wang wrote:
>> On 18 November 2015 at 23:32, Peter Hurley  wrote:
>>> Hi Baolin,
>>>
>>> On 11/16/2015 02:05 AM, Baolin Wang wrote:
 It dose not work when we want to use the usb-to-serial port based
 on one usb gadget as a console. Thus this patch adds the console
 initialization to support this request.
>>>
>>> I have some high level concerns.
>>>
>>> 1. I would defer registering the console until the port has at least been
>>>allocated in gserial_alloc_line(), and unregister when the line is freed.
>>>That also reduces many of the conditions that you shouldn't need to
>>>check, like port number range and so on.
>>
>> The 'setup' callback of 'struct console' is just do some memory
>> allocation and member initialization, that no need to defer
>> registering the console in gserial_alloc_line(). But the
>> 'gs_console_connect()' which will use the port need to be called in
>> gserial_connect().
>
> My point here was why are you registering the console before the port
> table is even allocated and initialized?  The console can't possibly
> perform i/o that early because the port doesn't even exist.
> Which is why I suggested waiting until gserial_alloc_line() to
> register the console.
>
> A typical console setup() performs the cross-reference linking between
> the console data structure and the port table. The reason for that
> is the console needs to be cleaned up if the port is being torn down.
>
> For example, in gserial_disconnect() the port->port_usb is reset to NULL,
> and later in gserial_console_exit():
>
> if (port && port->port_usb) {
> 
> gs_request_free(req, ep);
> }
>
> which means your leaking the request allocation when the port has been
> disconnected.
>

Yeah, that's right. I'll defer the console registration until
gserial_connect() and unregistration in disconnect.

>
>>>Further, consider deferring the console registration until 
>>> gserial_connect();
>>>that would further simplify things. In this case, unregistration would
>>>happen at disconnect.
>>
>> That sounds reasonable. I would try.
>>
>>>
>>> 2. Why are you using a kworker for actual device i/o when all of the i/o
>>>is done with interrupts disabled anyway?
>>>Getting rid of the work would eliminate using the 8K intermediate buffer
>>>as well.
>>
>> If remove the kworker, there are some deadlocks to call the printk()
>> when in the process of transferring data with usb endpoint. So we need
>> a async kworker to complete the io or it can not work.
>
> The commit log should detail the major design choices, including why you
> used the kworker (because of re-entrancy issues with usb endpoint).
>

OK.

>
>
>>> 3. If for some reason i/o deferral is really necessary, consider using a 
>>> kthread
>>>kworker instead of the pooled kworker. The scheduler response will be 
>>> _way_
>>>better.
>>>
>>
>> OK, make sense.
>>
>>> 4. If for some reason i/o deferral is really necessary, use a circular 
>>> buffer
>>>for the intermediate buffer, preferably lockless since there is only
>>>one producer and one consumer.
>>>
>>
>> Yeah, the circular buffer is better but more tricky. I would try.
>>
>>> Some other review comments below; please ignore anything other reviewers
>>> have already noted.
>>>
>>> Regards,
>>> Peter Hurley
>>>
 Signed-off-by: Baolin Wang 
 ---
  drivers/usb/gadget/Kconfig |6 +
  drivers/usb/gadget/function/u_serial.c |  238 
 
  2 files changed, 244 insertions(+)

 diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
 index 33834aa..be5aab9 100644
 --- a/drivers/usb/gadget/Kconfig
 +++ b/drivers/usb/gadget/Kconfig
 @@ -127,6 +127,12 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
  a module parameter as well.
  If unsure, say 2.

 +config U_SERIAL_CONSOLE
 + bool "Serial gadget console support"
 + depends on USB_G_SERIAL
 + help
 +It supports the serial gadget can be used as a console.
 +
  source "drivers/usb/gadget/udc/Kconfig"

  #
 diff --git a/drivers/usb/gadget/function/u_serial.c 
 b/drivers/usb/gadget/function/u_serial.c
 index f7771d8..4ade527 100644
 --- a/drivers/usb/gadget/function/u_serial.c
 +++ b/drivers/usb/gadget/function/u_serial.c
 @@ -27,6 +27,7 @@
  #include 
  #include 
  #include 
 +#include 

  #include "u_serial.h"

 @@ -79,6 +80,16 @@
   */
  #define QUEUE_SIZE   16
  #define WRITE_BUF_SIZE   8192/* TX only */
 +#define GS_BUFFER_SIZE   (4096)
 +#define GS_CONSOLE_BUF_SIZE  (2 * GS_BUFFER_SIZE)
 +
 +struct 

Re: scsi-sd fails with error "Invalid field in cdb" for SATA-to-USB adapter JMicron

[Dmitry Katsubo]

On 2015-11-17 19:18, Alan Stern wrote:
> That line is completely inappropriate for uas; it applies only to 
> usb-storage.  Don't add it.

I got it. My first thought was like you have said (every module uses its
own structure), but I blindly tried to guess.

> Here you need to test devinfo->flags & US_FL_BROKEN_FUA.

Great, that worked!

scsi host6: uas
scsi 6:0:0:0: Direct-Access JMicron  Generic   0116 PQ: 0 ANSI: 6
sd 6:0:0:0: Attached scsi generic sg2 type 0
sd 6:0:0:0: [sdb] 312581808 512-byte logical blocks: (160 GB/149 GiB)
sd 6:0:0:0: [sdb] 4096-byte physical blocks
sd 6:0:0:0: [sdb] Write Protect is off
sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08
sd 6:0:0:0: [sdb] Disabling FUA
sd 6:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 6:0:0:0: [sdb] Attached SCSI disk

>> The show-stopper for me is to know, how the us->fflags is initialized
>> from list of "unusual devices". There is some magic in there, at least
>> this is not something on the surface. I can't get how the definition of
>> these unusual devices is shared between scsiglue.c and uas.c, basically,
>> where is the code that matches the USB vendor/product and sets
>> us->fflags. That code should be called before uas.c:
>> uas_slave_configure():792.
> 
> The code that sets devinfo->flags is in uas_probe().  The flags value 
> comes from uas_use_uas_driver() in uas_detect.h.  The table used by uas 
> is stored in unusual_uas.h, not unusual_devs.h, so you'll have to add a 
> completely new entry there.
> 
> Actually, you should modify both unusual_*.h files, because someone 
> might want to use that device with the usb-storage driver rather than 
> the uas driver.

Actually there is an entry in unusual_uas.h but there is a minor difference:

unusual_uas.h uses  UNUSUAL_DEV(0x152d, 0x0567, 0x, 0x, ...
unusual_devs.h uses UNUSUAL_DEV(0x152d, 0x0567, 0x0114, 0x0116, ...

so uas module captures the wider set of devices (effectively ignores
bcdDevice). Should it be left like that (then behaviour could be
different for usb-storage vs uas driver) or it makes sense to align
these two?

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

Re: [PATCH] usb: gadget: Add the console support for usb-to-serial port

[Peter Hurley]

On 11/18/2015 09:35 PM, Baolin Wang wrote:
> On 18 November 2015 at 23:32, Peter Hurley  wrote:
>> Hi Baolin,
>>
>> On 11/16/2015 02:05 AM, Baolin Wang wrote:
>>> It dose not work when we want to use the usb-to-serial port based
>>> on one usb gadget as a console. Thus this patch adds the console
>>> initialization to support this request.
>>
>> I have some high level concerns.
>>
>> 1. I would defer registering the console until the port has at least been
>>allocated in gserial_alloc_line(), and unregister when the line is freed.
>>That also reduces many of the conditions that you shouldn't need to
>>check, like port number range and so on.
> 
> The 'setup' callback of 'struct console' is just do some memory
> allocation and member initialization, that no need to defer
> registering the console in gserial_alloc_line(). But the
> 'gs_console_connect()' which will use the port need to be called in
> gserial_connect().

My point here was why are you registering the console before the port
table is even allocated and initialized?  The console can't possibly
perform i/o that early because the port doesn't even exist.
Which is why I suggested waiting until gserial_alloc_line() to
register the console.

A typical console setup() performs the cross-reference linking between
the console data structure and the port table. The reason for that
is the console needs to be cleaned up if the port is being torn down.

For example, in gserial_disconnect() the port->port_usb is reset to NULL,
and later in gserial_console_exit():

if (port && port->port_usb) {

gs_request_free(req, ep);
}

which means your leaking the request allocation when the port has been
disconnected.


>>Further, consider deferring the console registration until 
>> gserial_connect();
>>that would further simplify things. In this case, unregistration would
>>happen at disconnect.
> 
> That sounds reasonable. I would try.
> 
>>
>> 2. Why are you using a kworker for actual device i/o when all of the i/o
>>is done with interrupts disabled anyway?
>>Getting rid of the work would eliminate using the 8K intermediate buffer
>>as well.
> 
> If remove the kworker, there are some deadlocks to call the printk()
> when in the process of transferring data with usb endpoint. So we need
> a async kworker to complete the io or it can not work.

The commit log should detail the major design choices, including why you
used the kworker (because of re-entrancy issues with usb endpoint).



>> 3. If for some reason i/o deferral is really necessary, consider using a 
>> kthread
>>kworker instead of the pooled kworker. The scheduler response will be 
>> _way_
>>better.
>>
> 
> OK, make sense.
> 
>> 4. If for some reason i/o deferral is really necessary, use a circular buffer
>>for the intermediate buffer, preferably lockless since there is only
>>one producer and one consumer.
>>
> 
> Yeah, the circular buffer is better but more tricky. I would try.
> 
>> Some other review comments below; please ignore anything other reviewers
>> have already noted.
>>
>> Regards,
>> Peter Hurley
>>
>>> Signed-off-by: Baolin Wang 
>>> ---
>>>  drivers/usb/gadget/Kconfig |6 +
>>>  drivers/usb/gadget/function/u_serial.c |  238 
>>> 
>>>  2 files changed, 244 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>>> index 33834aa..be5aab9 100644
>>> --- a/drivers/usb/gadget/Kconfig
>>> +++ b/drivers/usb/gadget/Kconfig
>>> @@ -127,6 +127,12 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
>>>  a module parameter as well.
>>>  If unsure, say 2.
>>>
>>> +config U_SERIAL_CONSOLE
>>> + bool "Serial gadget console support"
>>> + depends on USB_G_SERIAL
>>> + help
>>> +It supports the serial gadget can be used as a console.
>>> +
>>>  source "drivers/usb/gadget/udc/Kconfig"
>>>
>>>  #
>>> diff --git a/drivers/usb/gadget/function/u_serial.c 
>>> b/drivers/usb/gadget/function/u_serial.c
>>> index f7771d8..4ade527 100644
>>> --- a/drivers/usb/gadget/function/u_serial.c
>>> +++ b/drivers/usb/gadget/function/u_serial.c
>>> @@ -27,6 +27,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>
>>>  #include "u_serial.h"
>>>
>>> @@ -79,6 +80,16 @@
>>>   */
>>>  #define QUEUE_SIZE   16
>>>  #define WRITE_BUF_SIZE   8192/* TX only */
>>> +#define GS_BUFFER_SIZE   (4096)
>>> +#define GS_CONSOLE_BUF_SIZE  (2 * GS_BUFFER_SIZE)
>>> +
>>> +struct gscons_info {
>>> + struct gs_port  *port;
>>> + struct tty_driver   *tty_driver;
>>> + struct work_struct  work;
>>> + int buf_tail;
>>> + charbuf[GS_CONSOLE_BUF_SIZE];
>>> +};
>>
>> Make struct gscons_info a static declaration instead of
>> allocating it.
> 
> But I think the 

Re: [PATCH] usb: dwc2: add support of hi6220

[John Youn]

On 11/19/2015 11:04 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> Zhangfei Gao  writes:
>> Support hisilicon,hi6220-usb for HiKey board
>>
>> Signed-off-by: Zhangfei Gao 
> 
> doesn't apply:
> 
> Applying: usb: dwc2: add support of hi6220
> error: drivers/usb/dwc2/platform.c: does not match index
> Patch failed at 0001 usb: dwc2: add support of hi6220
> The copy of the patch that failed is found in: 
> workspace/linux/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Care to rebase on my testing/next and also collect John's and Rob's ack ?
> 


That's weird. I just sync'd to your testing/next and it seems to
apply fine.

Same with the series from Gregory Herrero.

Any chance it's something to do with your local repo?

Regards,
John

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

Re: Udoo support for chipidea

[Alan Stern]

On Thu, 19 Nov 2015, Philipp Zabel wrote:

> On Wed, Oct 21, 2015 at 10:39:00AM +0800, Peter Chen wrote:
> > On Tue, Oct 20, 2015 at 02:09:38PM -0200, Fabio Estevam wrote:
> > > Hi Peter,
> > > 
> > > On Mon, Oct 19, 2015 at 12:50 AM, Peter Chen  
> > > wrote:
> > > 
> > > > Add linux-usb.
> > > >
> > > > Patryk, your problem is you may need to open 24M OSC for HUB 2514
> > > > manually, and you have used IMX6QDL_CLK_CKO for it in the design,
> > > > but this clock is not controller's clock, controller's clock has
> > > > already decided at SoC dts file (imx6qdl.dtsi), you don't need to
> > > > override it at board's dts file.
> > > >
> > > > You can try delete clock property at imx6qdl-udoo.dtsi, if it still
> > > > can't work, try to open IMX6QDL_CLK_CKO at one place to test.
> > > 
> > > What is the appropriate place to acquire and enable the USB hub clock?
> > > 
> > > This issue has appeared several times and it seems we don't have a
> > > solution for this yet.
> > > 
> > > Any suggestions?
> > 
> > Add Alan.
> > 
> > Hi Alan, we have several designs that the on-board HUB need to
> > be reset by gpio pin and its clock is also from the board or
> > the SoC. Any suggestions how to add these platform information
> > for HUB device?
> 
> How about putting it in the device tree?
> http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> clocks and reset-gpios properties could be added to the USB hub node.

Something like this is necessary.  Instead of making the hub driver
take care of the reset gpio and the clock, I suggest you make the host
controller's platform driver do these things.

This is because USB hubs are generic devices, not specific to any 
platform and (usually) hot-pluggable.  Associating platform-specific 
data with a hub is out of the ordinary, and it deserves to be handled 
by platform-specific code -- there is no such code in the hub driver.

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 6/8] usb: dwc2: host: Assume all devices are on one single_tt hub

[Doug Anderson]

Felipe,

On Thu, Nov 19, 2015 at 7:34 AM, Felipe Balbi  wrote:
>
> Hi,
>
> Douglas Anderson  writes:
>> Until we have logic to determine which devices share the same TT let's
>> add logic to assume that all devices on a given dwc2 controller are on
>> one single_tt hub.  This is better than the previous code that assumed
>> that all devices were on one multi_tt hub, since single_tt hubs
>> appear (in my experience) to be much more common and any schedule that
>> would work on a single_tt hub will also work on a multi_tt hub.  This
>> will prevent more than 8 total low/full speed devices to be on the bus
>> at one time, but that's a reasonable restriction until we've made things
>> smarter.
>>
>> Signed-off-by: Douglas Anderson 
>> ---
>> Changes in v3:
>> - Assuming single_tt is new for v3; not terribly well tested (yet).
>>
>> Changes in v2: None
>>
>>  drivers/usb/dwc2/core.h  |  1 +
>>  drivers/usb/dwc2/hcd_queue.c | 40 +++-
>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 567ee2c9e69f..09aa2b5ae29e 100644
>> --- a/drivers/usb/dwc2/core.h
>> +++ b/drivers/usb/dwc2/core.h
>> @@ -782,6 +782,7 @@ struct dwc2_hsotg {
>>   u16 periodic_usecs;
>>   unsigned long periodic_bitmap[DIV_ROUND_UP(TOTAL_PERIODIC_USEC,
>>  BITS_PER_LONG)];
>> + bool has_split[8];
>
> why don't you use a u8 instead then just set each bit for each
> "has_split" you need to take care of. This array is kinda ugly.

Let's actually drop this patch completely.  Julius and I sat down and
he talked me through things, and with my current understanding the
current microframe scheduler in dwc2 is broken enough that small
little band-aids like this will do little more than just push the
problems around.

I'm a good portion of the way through a better microframe scheduler.
I have no doubt that it won't be perfect, but hopefully it will at
least be based in reality...

My latest thinking on the patches in this series:

1. usb: dwc2: rockchip: Make the max_transfer_size automatic

I'll probably separate this out into its own patch so I can stop
sending it as part of this series.  ...or if someone wanted to land it
then I won't bother.


2. usb: dwc2: host: Get aligned DMA in a more supported way

Still can land any time and has good benefits.  I believe that I can't
separate this because it will cause conflicts with scheduler patches.


3. usb: dwc2: host: Add scheduler tracing

Would be nice to land.


4. usb: dwc2: host: Rewrite the microframe scheduler
5. usb: dwc2: host: Keep track of and use our scheduled microframe
6. usb: dwc2: host: Assume all devices are on one single_tt hub

Please drop patches 4-6 right now.


7. usb: dwc2: host: Add a delay before releasing periodic bandwidth
8. usb: dwc2: host: Giveback URB in tasklet context

I'll probably move these back up in the series (like in v2) and put
microframe rewrite atop them.  With my current understanding the
scheduling is so broken today that the concerns Alan brought up can
wait until we have a proper scheduler to be addressed.  In the
meantime getting the huge boost in interrupt speed will help with
dwc2's correctness (and performance) because it means we're much less
likely to miss SOF interrupts.

If anyone has any review time, giving a review to v2 of these patches
would be helpful.  Otherwise I'll double check that v2 still looks
good with my current understanding and eventually post them again.

-Doug
--
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: scsi-sd fails with error "Invalid field in cdb" for SATA-to-USB adapter JMicron

[Alan Stern]

On Thu, 19 Nov 2015, Dmitry Katsubo wrote:

> On 2015-11-17 19:18, Alan Stern wrote:
> > That line is completely inappropriate for uas; it applies only to 
> > usb-storage.  Don't add it.
> 
> I got it. My first thought was like you have said (every module uses its
> own structure), but I blindly tried to guess.
> 
> > Here you need to test devinfo->flags & US_FL_BROKEN_FUA.
> 
> Great, that worked!
> 
> scsi host6: uas
> scsi 6:0:0:0: Direct-Access JMicron  Generic   0116 PQ: 0 ANSI: 6
> sd 6:0:0:0: Attached scsi generic sg2 type 0
> sd 6:0:0:0: [sdb] 312581808 512-byte logical blocks: (160 GB/149 GiB)
> sd 6:0:0:0: [sdb] 4096-byte physical blocks
> sd 6:0:0:0: [sdb] Write Protect is off
> sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08
> sd 6:0:0:0: [sdb] Disabling FUA
> sd 6:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
> support DPO or FUA
> sd 6:0:0:0: [sdb] Attached SCSI disk

Good.

> Actually there is an entry in unusual_uas.h but there is a minor difference:
> 
> unusual_uas.h uses  UNUSUAL_DEV(0x152d, 0x0567, 0x, 0x, ...
> unusual_devs.h uses UNUSUAL_DEV(0x152d, 0x0567, 0x0114, 0x0116, ...
> 
> so uas module captures the wider set of devices (effectively ignores
> bcdDevice). Should it be left like that (then behaviour could be
> different for usb-storage vs uas driver) or it makes sense to align
> these two?

This is something we can never really answer, because it would require 
knowing what bugs the vendors will eliminate in future versions of 
their firmware.

For now I suggest making the minimum necessary changes, even if it 
means the two drivers are out of sync.

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] hid: usbhid: hid-core: fix recursive deadlock

[Ioan-Adrian Ratiu]

On Thu, 19 Nov 2015 10:10:19 +0100 (CET)
Jiri Kosina  wrote:

> On Thu, 19 Nov 2015, Ioan-Adrian Ratiu wrote:
> 
> > First part of lockdep report:
> > http://imgur.com/clLsCWe
> > 
> > Second part:
> > http://imgur.com/Wa2PzRl
> > 
> > Here are some printk's of mine while reproducing + debugging the issue:
> > http://imgur.com/SETOHT7  
> 
> So the real problem is that Intuos driver is calling hid_hw_request() 
> (which tries to grab the lock in usbhid_submit_report()) while handling 
> the CTRL IRQ (lock gets acquired there).
> 
> So the proper way to fix seems to be delaying the scheduling of the 
> proximity read event in wacom_intuos_inout() to workqueue.
> 
> > I'll continue to research this more in depth, but progress is slow 
> > because I don't have much time, I'm doing this in my spare time because 
> > it's my girlfriend's tablet.  
> 
> Oh, now I understand the level of severity of this bug! :-)
> 
> Thanks,
> 

Yes, exactly, you are beginning to understand! :)  When I've put my 2 variants
above to solve this deadlock, by "removing the call from wacom" at 1) I was
trying to say exactly this, removing it from the irq to a workqueue.

But please understand further my reasoning for submitting this patch. Consider
if this is a bug in the wacom driver or in the usbhid core? IMO
this is a usbhid bug: the critical region in hid_ctrl() is too big, there
is no reason for the call to hid_input_report() to be protected by
usbhid->lock.

The correct way to fix this deadlock is to fix the critical section in
usbhid, not remove the call from the wacom irq. If wacom wants to
reschedule in the irq, it should not deadlock on usbhid. "Fixing" the wacom call
would just work around the critical region bug inside usbhid.

I hope I've made myself clear this time; I really needed to explain this
patch better :( sorry.
--
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: Help needed for EHCI problem: removing an active bulk-in QH

[Michael Reutman]

On Wed, Nov 18, 2015 at 10:23 AM, Alan Stern  wrote:
> For the last patch.  And yes, do set usb_snoop_max.

Dmesg output with usb_snoop and usb_snoop_max is attached below.

> I hope so.  There is one thing I'm still undecided about: Should this
> workaround be applied only to AMD/ATI controllers or should it apply to
> everything?  It does have a small probability of slowing down transfers
> under certain circumstances, but on the other hand, it's quite possible
> that other controller types will have the same sort of weakness as the
> AMD ones.

We've tested this application on a handful of other platforms and have
really only had issues with this desktop and our tablet device which
is stuck on the 2.6 kernel. Looking at the output of that tablet when
it gets "stuck" (this was posted back on the original libusb post)
makes it also seem like it may have some other problem than the one we
have been debugging here. Not sure if we can provide any other input
here that would help in making that decision.

Michael Reutman
Software Engineer
Epiq Solutions
847-598-0218 x68
www.epiqsolutions.com
[  834.390818] usb 5-4: opened by process 2868: main
[  834.390842] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.390845] usb 5-4: usbfs: process 2868 (main) did not claim interface 0 
before use
[  834.390867] usb 5-4: urb submit 880328793a40
[  834.390869] usb 5-4: userurb 009b9480, ep2 bulk-in, length 16384
[  834.390902] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.390907] usb 5-4: urb submit 880328793080
[  834.390908] usb 5-4: userurb 009b94c0, ep2 bulk-in, length 16384
[  834.390911] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.390916] usb 5-4: urb submit 8803287926c0
[  834.390917] usb 5-4: userurb 009b9500, ep2 bulk-in, length 16384
[  834.390920] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.390924] usb 5-4: urb submit 880328793800
[  834.390925] usb 5-4: userurb 009b9540, ep2 bulk-in, length 16384
[  834.391349] usb 5-4: urb complete 880328793a40
[  834.391350] usb 5-4: userurb 009b9480, ep2 bulk-in, actual_length 
16384 status 0
[  834.391352] data: 01 51 0e 0b 00 00 00 00 87 c7  
  .Q
[  834.391366] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.391369] usb 5-4: proc_reapurbnonblock: REAP 009b9480
[  834.391718] usb 5-4: urb complete 880328793080
[  834.391720] usb 5-4: userurb 009b94c0, ep2 bulk-in, actual_length 
16384 status 0
[  834.391721] data: 4d 8d 71 24 00 00 00 00 8b 8d  
  M.q$..
[  834.392218] usb 5-4: urb complete 8803287926c0
[  834.392219] usb 5-4: userurb 009b9500, ep2 bulk-in, actual_length 
16384 status 0
[  834.392221] data: 35 9d 71 24 00 00 00 00 63 e0  
  5.q$c.
[  834.392718] usb 5-4: urb complete 880328793800
[  834.392720] usb 5-4: userurb 009b9540, ep2 bulk-in, actual_length 
16384 status 0
[  834.392721] data: 1d ad 71 24 00 00 00 00 3b 33  
  ..q$;3
[  834.393655] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.393658] usb 5-4: proc_reapurbnonblock: REAP 009b94c0
[  834.393999] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.394001] usb 5-4: proc_reapurbnonblock: REAP 009b9500
[  834.394023] usb 5-4: usbdev_do_ioctl: DISCARDURB 009b9540
[  834.394152] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.394153] usb 5-4: proc_reapurbnonblock: REAP 009b9540
[  834.394716] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.404826] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.404832] usb 5-4: urb submit 880328792a80
[  834.404834] usb 5-4: userurb 009b9540, ep2 bulk-in, length 16384
[  834.404856] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.404859] usb 5-4: urb submit 880328792540
[  834.404860] usb 5-4: userurb 009b9500, ep2 bulk-in, length 16384
[  834.404866] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.404869] usb 5-4: urb submit 8803287935c0
[  834.404870] usb 5-4: userurb 009b94c0, ep2 bulk-in, length 16384
[  834.404876] usb 5-4: usbdev_do_ioctl: SUBMITURB
[  834.404879] usb 5-4: urb submit 880328792780
[  834.404881] usb 5-4: userurb 009b9480, ep2 bulk-in, length 16384
[  834.405478] usb 5-4: urb complete 880328792a80
[  834.405480] usb 5-4: userurb 009b9540, ep2 bulk-in, actual_length 
16384 status 0
[  834.405481] data: 05 bd 71 24 00 00 00 00 13 86  
  ..q$..
[  834.405499] usb 5-4: usbdev_do_ioctl: REAPURBNDELAY
[  834.405501] usb 5-4: proc_reapurbnonblock: REAP 009b9540
[  834.405852] usb 5-4: urb complete 880328792540
[  834.405853] usb 5-4: userurb 009b9500, ep2 bulk-in, actual_length 
16384 status 0
[  834.405855] data: 

Re: [PATCH v2 2/2] usb: dwc2: host: Clear interrupts before handling them

[Doug Anderson]

Hi,

On Wed, Nov 18, 2015 at 5:43 PM, John Youn  wrote:
> On 11/16/2015 9:22 AM, Doug Anderson wrote:
>> Felipe,
>>
>> On Mon, Nov 16, 2015 at 8:28 AM, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> Douglas Anderson  writes:
 In general it is wise to clear interrupts before processing them.  If
 you don't do that, you can get:
  1. Interrupt happens
  2. You look at system state and process interrupt
  3. A new interrupt happens
  4. You clear interrupt without processing it.

 This patch was actually a first attempt to fix missing device insertions
 as described in (usb: dwc2: host: Fix missing device insertions) and it
 did solve some of the signal bouncing problems but not all of
 them (which is why I submitted the other patch).  Specifically, this
 patch itself would sometimes change:
  1. hardware sees connect
  2. hardware sees disconnect
  3. hardware sees connect
  4. dwc2_port_intr() - clears connect interrupt
  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

 ...to:
  1. hardware sees connect
  2. hardware sees disconnect
  3. dwc2_port_intr() - clears connect interrupt
  4. hardware sees connect
  5. dwc2_handle_common_intr() - calls dwc2_hcd_disconnect()

 ...but with different timing then sometimes we'd still miss cable
 insertions.

 In any case, though this patch doesn't fix any (known) problems, it
 still seems wise as a general policy to clear interrupt before handling
 them.

 Signed-off-by: Douglas Anderson 
 ---
 Changes in v2: None

  drivers/usb/dwc2/core_intr.c | 55 
 ++--
  drivers/usb/dwc2/hcd_intr.c  | 16 ++---
  2 files changed, 35 insertions(+), 36 deletions(-)

 diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
 index 61601d16e233..2a166b7eec41 100644
 --- a/drivers/usb/dwc2/core_intr.c
 +++ b/drivers/usb/dwc2/core_intr.c
 @@ -80,15 +80,16 @@ static const char *dwc2_op_state_str(struct dwc2_hsotg 
 *hsotg)
   */
  static void dwc2_handle_usb_port_intr(struct dwc2_hsotg *hsotg)
  {
 - u32 hprt0 = dwc2_readl(hsotg->regs + HPRT0);
 + u32 hprt0;

 + /* Clear interrupt */
 + dwc2_writel(GINTSTS_PRTINT, hsotg->regs + GINTSTS);
 +
 + hprt0 = dwc2_readl(hsotg->regs + HPRT0);
   if (hprt0 & HPRT0_ENACHG) {
   hprt0 &= ~HPRT0_ENA;
   dwc2_writel(hprt0, hsotg->regs + HPRT0);
   }
 -
 - /* Clear interrupt */
 - dwc2_writel(GINTSTS_PRTINT, hsotg->regs + GINTSTS);
>>>
>>> isn't this a regression ? You're first clearing the interrupts and only
>>> then reading to check what's pending, however, what's pending has just
>>> been cleared. Seems like this should be:
>>>
>>> hprt0 = dwc2_readl(HPRT0);
>>> dwc2_writeal(PRTINT, GINTSTS);
>>
>> Actually, we could probably remove the setting of GINTSTS_PRTINT
>> completely.  The docs I have say that the GINTSTS_PRTINT is read only
>> and that:
>>
>>> The core sets this bit to indicate a change in port status of one of the
>>> DWC_otg core ports in Host mode. The application must read the
>>> Host Port Control and Status (HPRT) register to determine the exact
>>> event that caused this interrupt. The application must clear the
>>> appropriate status bit in the Host Port Control and Status register to
>>> clear this bit.
>>
>> ...so writing PRTINT is probably useless, but John can confirm.
>>
>
> Yup, it seems it can be removed.

How do you guys want this handled?  Should I send up a new version of
this patch?  ...or should I send a followon patch that does this
removal?

-Doug
--
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 6/8] usb: dwc2: host: Assume all devices are on one single_tt hub

[John Youn]

On 11/19/2015 8:27 AM, Doug Anderson wrote:
> Felipe,
> 
> On Thu, Nov 19, 2015 at 7:34 AM, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Douglas Anderson  writes:
>>> Until we have logic to determine which devices share the same TT let's
>>> add logic to assume that all devices on a given dwc2 controller are on
>>> one single_tt hub.  This is better than the previous code that assumed
>>> that all devices were on one multi_tt hub, since single_tt hubs
>>> appear (in my experience) to be much more common and any schedule that
>>> would work on a single_tt hub will also work on a multi_tt hub.  This
>>> will prevent more than 8 total low/full speed devices to be on the bus
>>> at one time, but that's a reasonable restriction until we've made things
>>> smarter.
>>>
>>> Signed-off-by: Douglas Anderson 
>>> ---
>>> Changes in v3:
>>> - Assuming single_tt is new for v3; not terribly well tested (yet).
>>>
>>> Changes in v2: None
>>>
>>>  drivers/usb/dwc2/core.h  |  1 +
>>>  drivers/usb/dwc2/hcd_queue.c | 40 +++-
>>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 567ee2c9e69f..09aa2b5ae29e 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -782,6 +782,7 @@ struct dwc2_hsotg {
>>>   u16 periodic_usecs;
>>>   unsigned long periodic_bitmap[DIV_ROUND_UP(TOTAL_PERIODIC_USEC,
>>>  BITS_PER_LONG)];
>>> + bool has_split[8];
>>
>> why don't you use a u8 instead then just set each bit for each
>> "has_split" you need to take care of. This array is kinda ugly.
> 
> Let's actually drop this patch completely.  Julius and I sat down and
> he talked me through things, and with my current understanding the
> current microframe scheduler in dwc2 is broken enough that small
> little band-aids like this will do little more than just push the
> problems around.
> 
> I'm a good portion of the way through a better microframe scheduler.
> I have no doubt that it won't be perfect, but hopefully it will at
> least be based in reality...
> 
> My latest thinking on the patches in this series:
> 
> 1. usb: dwc2: rockchip: Make the max_transfer_size automatic
> 
> I'll probably separate this out into its own patch so I can stop
> sending it as part of this series.  ...or if someone wanted to land it
> then I won't bother.
> 
> 
> 2. usb: dwc2: host: Get aligned DMA in a more supported way
> 
> Still can land any time and has good benefits.  I believe that I can't
> separate this because it will cause conflicts with scheduler patches.
> 
> 
> 3. usb: dwc2: host: Add scheduler tracing
> 
> Would be nice to land.
> 
> 
> 4. usb: dwc2: host: Rewrite the microframe scheduler
> 5. usb: dwc2: host: Keep track of and use our scheduled microframe
> 6. usb: dwc2: host: Assume all devices are on one single_tt hub
> 
> Please drop patches 4-6 right now.
> 
> 
> 7. usb: dwc2: host: Add a delay before releasing periodic bandwidth
> 8. usb: dwc2: host: Giveback URB in tasklet context
> 
> I'll probably move these back up in the series (like in v2) and put
> microframe rewrite atop them.  With my current understanding the
> scheduling is so broken today that the concerns Alan brought up can
> wait until we have a proper scheduler to be addressed.  In the
> meantime getting the huge boost in interrupt speed will help with
> dwc2's correctness (and performance) because it means we're much less
> likely to miss SOF interrupts.
> 
> If anyone has any review time, giving a review to v2 of these patches
> would be helpful.  Otherwise I'll double check that v2 still looks
> good with my current understanding and eventually post them again.
> 
> -Doug
> 

Hi Doug,

Patches 1-3:
Acked-by: John Youn 

Patch 2:
Tested-by: John Youn 

Tested on core version 3.20 using internal TE for un-aligned
buffers.

I haven't had time to look into the scheduling patches yet. But I
agree with you that there are fundamental problems. I'll await
your rewrite.

Regards,
John

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

Re: [PATCH 1/1] usb: phy: omap-otg: do not write to unallocated memory

[Chanwoo Choi]

Hi,

The same patch was already reviewed and applied on usb.git repository[1]
[1] 
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/fixes=2c2025b41aeff57963f9ae2dd909fea704c625ab

Thanks,
Chanwoo Choi

On 2015년 11월 20일 08:43, Heinrich Schuchardt wrote:
> The current coding writes to memory before allocating it.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  drivers/usb/phy/phy-omap-otg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
> index 1270906..8bbf121 100644
> --- a/drivers/usb/phy/phy-omap-otg.c
> +++ b/drivers/usb/phy/phy-omap-otg.c
> @@ -105,12 +105,13 @@ static int omap_otg_probe(struct platform_device *pdev)
>   extcon = extcon_get_extcon_dev(config->extcon);
>   if (!extcon)
>   return -EPROBE_DEFER;
> - otg_dev->extcon = extcon;
>  
>   otg_dev = devm_kzalloc(>dev, sizeof(*otg_dev), GFP_KERNEL);
>   if (!otg_dev)
>   return -ENOMEM;
>  
> + otg_dev->extcon = extcon;
> +
>   otg_dev->base = devm_ioremap_resource(>dev, >resource[0]);
>   if (IS_ERR(otg_dev->base))
>   return PTR_ERR(otg_dev->base);
> 

--
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] HID: usbhid: add Logitech G710+ keyboard quirk NOGET

[Jimmy Berry]

On Thu, Nov 19, 2015 at 6:26 PM, Greg KH  wrote:
> On Tue, Nov 17, 2015 at 12:40:12AM -0600, Jimmy Berry wrote:
>> Without quirk keyboard repeats '6' until volume control is used since it
>> indicates the key is pressed without ever releasing.
>>
>> Signed-off-by: Jimmy Berry 
>> ---
>>  drivers/hid/hid-ids.h   | 1 +
>>  drivers/hid/usbhid/hid-quirks.c | 1 +
>>  2 files changed, 2 insertions(+)
>
> Please use scripts/get_maintainer.pl to find the right people to cc: and
> the correct mailing list for your patch (hint, it's not me...)

Thats exactly what I did. Perhaps the output needs to be tweaked. I
debated sending to linux-input, but the description seemed to fit too
well.

---
Jiri Kosina  (maintainer:HID CORE LAYER)
linux-in...@vger.kernel.org (open list:HID CORE LAYER)
linux-ker...@vger.kernel.org (open list)
linux-usb@vger.kernel.org (open list:USB HID/HIDBP DRIVERS (USB
KEYBOARDS, MICE, REM...)
---

Note the "USB HID" and "USB KEYBOARDS" sections. That happens to be
exactly what this patch is against. Either way, I'll assume it is
instead linux-input to which you wish me to send the patch.

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

Re: [PATCH v2 2/4] Documentation: usb: gadget-testing: add description for depth of queue

[Peter Chen]

On Thu, Nov 19, 2015 at 12:24:28PM -0600, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > Add both bulk and iso depth of queue for sourcesink.
> >
> > Signed-off-by: Peter Chen 
> > ---
> >  Documentation/usb/gadget-testing.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/usb/gadget-testing.txt 
> > b/Documentation/usb/gadget-testing.txt
> > index b24d3ef..84b3d10 100644
> > --- a/Documentation/usb/gadget-testing.txt
> > +++ b/Documentation/usb/gadget-testing.txt
> > @@ -579,6 +579,8 @@ The SOURCESINK function provides these attributes in 
> > its function directory:
> > isoc_mult   - 0..2 (hs/ss only)
> > isoc_maxburst   - 0..15 (ss only)
> > bulk_buflen - buffer length
> > +   bulk_qlen   - depth of queue for bulk
> > +   iso_qlen- depth of queue for iso
> 
> doesn't apply for me:
> 
> Applying: Documentation: usb: gadget-testing: add description for depth of 
> queue
> error: patch failed: Documentation/usb/gadget-testing.txt:579
> error: Documentation/usb/gadget-testing.txt: patch does not apply
> Patch failed at 0001 Documentation: usb: gadget-testing: add description for 
> depth of queue
> The copy of the patch that failed is found in: 
> workspace/linux/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Care to rebase on my testing/next ?
> 
> Note that patch 1 is already applied. When rebasing, please collect
> Krzysztof's Reviewed-by ;-)
> 

I find the first three has already been applied, thanks for doing that.
But the patch 4/4 has not applied, do you need me to re-send it after
adding Krzysztof's Reviewed-by?

-- 

Best Regards,
Peter Chen
--
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 6/8] usb: dwc2: host: Assume all devices are on one single_tt hub

[Felipe Balbi]

Hi,

Douglas Anderson  writes:
> Until we have logic to determine which devices share the same TT let's
> add logic to assume that all devices on a given dwc2 controller are on
> one single_tt hub.  This is better than the previous code that assumed
> that all devices were on one multi_tt hub, since single_tt hubs
> appear (in my experience) to be much more common and any schedule that
> would work on a single_tt hub will also work on a multi_tt hub.  This
> will prevent more than 8 total low/full speed devices to be on the bus
> at one time, but that's a reasonable restriction until we've made things
> smarter.
>
> Signed-off-by: Douglas Anderson 
> ---
> Changes in v3:
> - Assuming single_tt is new for v3; not terribly well tested (yet).
>
> Changes in v2: None
>
>  drivers/usb/dwc2/core.h  |  1 +
>  drivers/usb/dwc2/hcd_queue.c | 40 +++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 567ee2c9e69f..09aa2b5ae29e 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -782,6 +782,7 @@ struct dwc2_hsotg {
>   u16 periodic_usecs;
>   unsigned long periodic_bitmap[DIV_ROUND_UP(TOTAL_PERIODIC_USEC,
>  BITS_PER_LONG)];
> + bool has_split[8];

why don't you use a u8 instead then just set each bit for each
"has_split" you need to take care of. This array is kinda ugly.

> @@ -386,6 +409,13 @@ static int dwc2_find_multi_uframe(struct dwc2_hsotg 
> *hsotg, struct dwc2_qh *qh)
>   bitmap_set(hsotg->periodic_bitmap, start, qh->usecs);
>   qh->start_usecs = start;
>  
> + if (qh->do_split) {
> + for (i = start / EARLY_FRAME_USEC;
> +  i < DIV_ROUND_UP(start + utime - 1, EARLY_FRAME_USEC);
> +  i++)
> + hsotg->has_split[i] = true;

hsotg->has_split |= BIT(i);

> @@ -546,6 +577,13 @@ static void dwc2_deschedule_periodic(struct dwc2_hsotg 
> *hsotg,
>   }
>  
>   bitmap_clear(hsotg->periodic_bitmap, start, utime);
> +
> + if (qh->do_split) {
> + for (i = start / EARLY_FRAME_USEC;
> +  i < DIV_ROUND_UP(start + utime - 1, EARLY_FRAME_USEC);
> +  i++)
> + hsotg->has_split[i] = false;

hsotg->has_split &= ~BIT(i);

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: dwc3: add generic OF glue layer

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> For simple platforms which merely enable some clocks
> and populate its children, we can use this generic
> glue layer to avoid boilerplate code duplication.
>
> For now this supports Qcom and Xilinx, but if we
> find a way to add generic handling of regulators and
> optional PHYs, we can absorb exynos as well.
>
> Signed-off-by: Felipe Balbi 
> ---
>
> Can you guys check if this works for your respective platforms ? If it does we
> can some code duplication going forward.

gentle ping on this one.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: gadget: pxa27x: fix suspend callback

[Felipe Balbi]

pxa27x disconnects pullups on suspend but doesn't
notify the gadget driver about it, so gadget driver
can't disable the endpoints it was using.

This causes problems on resume because gadget core
will think endpoints are still enabled and just
ignore the following usb_ep_enable().

Fix this problem by calling
gadget_driver->disconnect().

Cc:  # v3.10+
Signed-off-by: Felipe Balbi 
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index 670ac0b12f00..a08ae19ca410 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2535,6 +2535,7 @@ static int pxa_udc_suspend(struct platform_device *_dev, 
pm_message_t state)
udc_disable(udc);
udc->pullup_resume = udc->pullup_on;
dplus_pullup(udc, 0);
+   udc->driver->disconnect(>gadget);
 
return 0;
 }
-- 
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