MIDI gadget allocating too much from coherent pool

[Felipe Tonello]

Hi,

I have the following setup:

DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB
MIDI GADGET -> HOST

When the throughput from the DSP is high, thus causing the throughput
on the USB to be high as well, I get a Kernel Panic saying to increase
the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M.
Obviously when I increase, it takes longer to crash but it still
crashes.

I am using linux-fsl 3.14.28.

The BT as follows:

[  193.619701] ERROR: 1024 KiB atomic DMA coherent pool is too small!
[  193.619701] Please increase it with coherent_pool= kernel parameter!
[  193.632261] Unable to handle kernel NULL pointer dereference at
virtual address 
[  193.640358] pgd = 80004000
[  193.643070] [] *pgd=
[  193.646670] Internal error: Oops: 817 [#1] PREEMPT SMP ARM
[  193.652162] Modules linked in:0d 0 :s0n2d:_5s2e.q2_2m1i
snd_seq_midi_event snd_seq_dummy snd_seq snd_soc_cs4272
snd_soc_fsl_ssi imx_pcm_dma imx_pcm_fiq snd_soc_imx_cs4272
snd_soc_core snd_compress snd_pcm_dmaengine snd_pcm snd_te
 [  193.689447] CPU: 2 PID: 16 Comm: ksoftirqd/2 Not tainted
3.14.28-1.0.0_ga+g91cf351 #1
 [  193.697285] task: ea0a5100 ti: ea0d8000 task.ti: ea0d8000
 [  193.702708] PC is at _ep_queue.isra.24+0x178/0x480
 [  193.707508] LR is at 0xc43cee80
 [  193.710657] pc : [<8039ea38>]lr : []psr: 600f0093
 [  193.710657] sp : ea0d9de0  ip : 6e0ea000  fp : ea0d9e1c
 [  193.722139] r10: 5000  r9 : ea03c010  r8 : c43d05b4
 [  193.727369] r7 :   r6 : 0004  r5 : ea03c6f4  r4 : c43d0580
 [  193.733899] r3 : c43d05bc  r2 :   r1 : 0001  r0 : fff4
 [  193.740433] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
 [  193.747834] Control: 10c5387d  Table: 7aca804a  DAC: 0015
 [  193.753585] Process ksoftirqd/2 (pid: 16, stack limit = 0xea0d8238)
 [  193.759857] Stack: (0xea0d9de0 to 0xea0da000)
 [  193.764225] 9de0: 0001 eaa72480 a00f0013 c43d05bc ea0d9e24
0003 ea03c6f4 a00f0013
 [  193.772413] 9e00: c43d0580 ea51a200 ea51a328 ea51a328 ea0d9e3c
ea0d9e20 8039fc30 8039e8cc
 [  193.780598] 9e20:   c4467000 c43d0580 ea0d9e7c
ea0d9e40 7f078754 8039fbe8
 [  193.788784] 9e40: ea03c6f4 ea51a2ec 80009990 6f00 ea0d9e84
ea51a330 ea51a334 
 [  193.796968] 9e60: 80718f40  ea0d8000 806d82bc ea0d9e8c
ea0d9e80 7f0787a0 7f07832c
 [  193.805153] 9e80: ea0d9ebc ea0d9e90 80031fe4 7f078798 80031f60
  806dc080
 [  193.826207] 9ec0:  ea0d9f0c 04208040 806dc0c0 d672
80507358 000a 80718f40
 [  193.834392] 9ee0: ea0d8000 806d8458 806dc080 0002 80053c8c
ea0d8000 ea0870c0 806e9cc8
 [  193.842576] 9f00: 0001    ea0d9f34
ea0d9f20 80032490 800321f0
 [  193.868396] 9f40: ea087140 ea0870c0 800512e0  ea0d9fac
ea0d9f60 8004a458 800512ec
 [  193.876582] 9f60: 30b80088 0001 0002 ea0870c0 
00030003 ea0d9f78 ea0d9f78
 [  193.884766] 9f80:   ea0d9f88 ea0d9f88 ea087140
8004a384  
 [  193.892952] 9fa0:  ea0d9fb0 8000ea58 8004a390 
  
 [  193.901137] 9fc0:     
  
 [  193.909322] 9fe0:     0013
 2001a1a0 61391094
 [  193.917501] Backtrace:
 [  193.919982] [<8039e8c0>] (_ep_queue.isra.24) from [<8039fc30>]
(ep_queue+0x54/0x88)
 [  193.927643]  r10:ea51a328 r9:ea51a328 r8:ea51a200 r7:c43d0580
r6:a00f0013 r5:ea03c6f4
 [  193.935554]  r4:0003
 [  193.938120] [<8039fbdc>] (ep_queue) from [<7f078754>]
(f_midi_transmit+0x434/0x46c [g_midi])
 [  193.946562]  r7:c43d0580 r6:c4467000 r5: r4:
 [  193.952299] [<7f078320>] (f_midi_transmit [g_midi]) from
[<7f0787a0>] (f_midi_in_tasklet+0x14/0x18 [g_midi])
 [  193.962129]  r10:806d82bc r9:ea0d8000 r8: r7:80718f40
r6: r5:ea51a334
 [  193.970039]  r4:ea51a330
 [  193.972609] [<7f07878c>] (f_midi_in_tasklet [g_midi]) from
[<80031fe4>] (tasklet_hi_action+0x84/0x114)
 [  193.981928] [<80031f60>] (tasklet_hi_action) from [<8003232c>]
(__do_softirq+0x148/0x260)
 [  193.990107]  r10:4000 r9:806dc080 r8:0100 r7:ea0d8000
r6:806dc080 r5:
 [  193.998018]  r4: r3:80031f60
 [  194.001635] [<800321e4>] (__do_softirq) from [<80032490>]
(run_ksoftirqd+0x4c/0x64)
 [  194.009294]  r10: r9: r8: r7:0001
r6:806e9cc8 r5:ea0870c0
 [  194.017203]  r4:ea0d8000
 [  194.019773] [<80032444>] (run_ksoftirqd) from [<80051468>]
(smpboot_thread_fn+0x188/0x278)
 [  194.028059] [<800512e0>] (smpboot_thread_fn) from [<8004a458>]
(kthread+0xd4/0xec)
 [  194.035632]  r8: r7:800512e0 r6:ea0870c0 r5:ea087140
r4: r3:ea0a5100
 [  194.043468] [<8004a384>] (kthread) from [<8000ea58>]
(ret_from_fork+0x14/0x3c)
 [  194.050695]  r7: r6: r5:8004a384 r4:ea087140
 [  194.056426] Code: e3a01001 e594203c e50b2030 e593200c (e5821000)
 [  194.062529] ---[ end 

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Peter Chen]

On Wed, Jul 01, 2015 at 12:06:19PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> This reverts commit 73dea4a912b2bfe955305de4891018f9e71e399d.
> 
> Since commit 73dea4a912b2("usb: chipidea: usbmisc_imx: delete clock
> information") it is not possible to boot a kernel on a mx27pdk:

Hi Fabio,

I am back for this patch, but the dts is wrong for mx25/mx27, it should
include three clocks for controller, I can change the driver, but
how to cover the old incorrect dts?

Peter

> 
> usbcore: registered new interface driver usb-storage
> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> pgd = c0004000
> [f4424600] *pgd=1452(bad)
> Internal error: : 8 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> Hardware name: Freescale i.MX27 (Device Tree Support)
> task: c7832b60 ti: c783e000 task.ti: c783e000
> PC is at usbmisc_imx27_init+0x4c/0xbc
> LR is at usbmisc_imx27_init+0x40/0xbc
> pc : []lr : []psr: 6093
> sp : c783fe08  ip :   fp : 
> r10: c0576434  r9 : 009c  r8 : c7a773a0
> r7 : 0100  r6 : 6013  r5 : c7a776f0  r4 : c7a773f0
> r3 : f4424600  r2 :   r1 : 0001  r0 : 0001
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 0017
> Process swapper (pid: 1, stack limit = 0xc783e190)
> Stack: (0xc783fe08 to 0xc784)
> 
> This commit also breaks the booting of imx6q-udoo board [1], so revert
> it to avoid these regressions.
> 
> [1] http://www.spinics.net/lists/linux-usb/msg125597.html
> 
> Signed-off-by: Fabio Estevam 
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 140945c..8f4cd92 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -11,6 +11,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -83,6 +84,7 @@ struct usbmisc_ops {
>  struct imx_usbmisc {
>   void __iomem *base;
>   spinlock_t lock;
> + struct clk *clk;
>   const struct usbmisc_ops *ops;
>  };
>  
> @@ -426,6 +428,7 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>  {
>   struct resource *res;
>   struct imx_usbmisc *data;
> + int ret;
>   struct of_device_id *tmp_dev;
>  
>   data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> @@ -439,6 +442,20 @@ static int usbmisc_imx_probe(struct platform_device 
> *pdev)
>   if (IS_ERR(data->base))
>   return PTR_ERR(data->base);
>  
> + data->clk = devm_clk_get(>dev, NULL);
> + if (IS_ERR(data->clk)) {
> + dev_err(>dev,
> + "failed to get clock, err=%ld\n", PTR_ERR(data->clk));
> + return PTR_ERR(data->clk);
> + }
> +
> + ret = clk_prepare_enable(data->clk);
> + if (ret) {
> + dev_err(>dev,
> + "clk_prepare_enable failed, err=%d\n", ret);
> + return ret;
> + }
> +
>   tmp_dev = (struct of_device_id *)
>   of_match_device(usbmisc_imx_dt_ids, >dev);
>   data->ops = (const struct usbmisc_ops *)tmp_dev->data;
> @@ -449,6 +466,8 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>  
>  static int usbmisc_imx_remove(struct platform_device *pdev)
>  {
> + struct imx_usbmisc *usbmisc = dev_get_drvdata(>dev);
> + clk_disable_unprepare(usbmisc->clk);
>   return 0;
>  }
>  
> -- 
> 1.9.1
> 

-- 

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 v1] media: uvcvideo: handle urb completion in a work queue

[Kaukab, Yousaf]

> -Original Message-
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Wednesday, September 9, 2015 6:30 PM
> To: Laurent Pinchart
> Cc: Hans de Goede; Kaukab, Yousaf; linux-me...@vger.kernel.org;
> mche...@osg.samsung.com; linux-usb@vger.kernel.org
> Subject: Re: [PATCH v1] media: uvcvideo: handle urb completion in a work
> queue
> 
> On Wed, 9 Sep 2015, Laurent Pinchart wrote:
> 
> > > > Instead of fixing the issue in the uvcvideo driver, would it then
> > > > make more sense to fix it in the remaining hcd drivers ?
> > >
> > > Unfortunately that's not so easy.  It involves some subtle changes
> > > related to the way isochronous endpoints are handled.  I wouldn't
> > > know what to change in any of the HCDs, except the ones that I maintain.
> >
> > I'm not saying it would be easy, but I'm wondering whether it makes
> > change to move individual USB device drivers to work queues when the
> > long term goal is to use tasklets for URB completion anyway.
> 
> I'm not sure that this is a long-term goal for every HCD.  For instance, there
> probably isn't much incentive to convert a driver if its host controllers can 
> only
> run at low speed or full speed.
> 

I can convert this patch to use tasklets instead and only schedule the
tasklet if urb->complete is called in interrupt context. This way, hcd
using tasklet or not, uvc driver behavior will be almost same. Only
difference is that local interrupts will still be enabled when tasklet 
scheduled from uvc driver is executing the completion function.

This patch can be dropped once all hcd's start using tasklets for the
completion callback (if that ever happens).

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

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
> On 09/09/15 11:45, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
> >> On 09/09/15 11:13, Peter Chen wrote:
> >>> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
>  On 09/09/15 05:21, Peter Chen wrote:
> > On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> >>
> >>
> >> On 08/09/15 11:31, Peter Chen wrote:
> >>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>  On 07/09/15 04:23, Peter Chen wrote:
> > On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >> + * This is used by the USB Host stack to register the Host 
> >> controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> +   unsigned long irqflags, struct otg_hcd_ops 
> >> *ops)
> >> +{
> >> +  struct usb_otg *otgd;
> >> +  struct device *hcd_dev = hcd->self.controller;
> >> +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >> +
> >
> > One big problem here is: there are two designs for current (IP) 
> > driver
> > code, one creates dedicated hcd device as roothub's parent, like 
> > dwc3.
> > Another one doesn't do this, roothub's parent is core device (or 
> > otg device
> > in your patch), like chipidea and dwc2.
> >
> > Then, otg_dev will be glue layer device for chipidea after that.
> 
>  OK. Let's add a way for the otg controller driver to provide the 
>  host and gadget
>  information to the otg core for such devices like chipidea and dwc2.
> 
> >>>
> >>> Roger, not only chipidea and dwc2, I think the musb uses the same
> >>> hierarchy. If the host, device, and otg share the same register
> >>> region, host part can't be a platform driver since we don't want
> >>> to remap the same register region again.
> >>>
> >>> So, in the design, we may need to consider both situations, one
> >>> is otg/host/device has its own register region, and host is a
> >>> separate platform device (A), the other is three parts share the
> >>> same register region, there is only one platform driver (B).
> >>>
> >>> A:
> >>>
> >>>   IP core device 
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget   host platform device 
> >>>   |
> >>>   roothub
> >>>
> >>> B:
> >>>
> >>>   IP core device
> >>>   |
> >>>   |
> >>> |-|-|
> >>> gadget roothub
> >>>   
> >>>
>  This API must be called before the hcd/gadget-driver is added so 
>  that the otg
>  core knows it's linked to an OTG controller.
> 
>  Any better idea?
> 
> >>>
> >>> A flag stands for this hcd controller is the same with otg controller
> >>> can be used, this flag can be stored at struct usb_otg_config.
> >>
> >> What if there is another architecture like so?
> >>
> >> C:
> >>[Parent]
> >>   |
> >>   |
> >>|--|--|
> >>[OTG core]  [gadget][host]
> >>
> >> We need a more flexible mechanism to link the gadget and
> >> host device to the otg core for non DT case.
> >>
> >> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> >>
> >> e.g.
> >> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> >>
> >> If otg is NULL it will try DT otg-controller property or parent to
> >> get the otg controller.
> >
> > How usb_otg_register_hcd get struct usb_otg, from where?
> 
>  This only works when the parent driver creating the hcd has registered 
>  the
>  otg controller too.
> 
> >>>
> >>> Sorry? So we need to find another way to solve this issue, right?
> >>
> >> For existing cases this is sufficient.
> >> The otg device is either the one supplied during usb_otg_register_hcd
> >> (cases B and C) or it is the parent device (case A).
> > 
> > How we differentiate case A and case B at usb_otg_register_hcd?
> > Would you show me the sample code?
> 
> Case A:

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

Hi Peter,

On Thu, Sep 10, 2015 at 4:21 AM, Peter Chen  wrote:

> Hi Fabio,
>
> I am back for this patch, but the dts is wrong for mx25/mx27, it should
> include three clocks for controller, I can change the driver, but
> how to cover the old incorrect dts?

For the mx27pdk the old dts does not allow the kernel to boot a 4.2
kernel. You could send the fix and Cc stable.

Regards,

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

xhci_hcd: unstable communication with Opella-XD JTAG probe

[Alexey Brodkin]

Hello,

I'm seeing a problem when attaching USB device (in my case Ashling
Opella-XD JTAG probe) in Dell e7440 laptop if USB 3.0 is enable in BIOS.

If I disable USB 3.0 in BIOS the same device works perfectly fine.
What's also interesting on my previous laptop (it was HP Elitebook
something) I saw the same device working fine even if plugged in USB 3.0
port (essentially USB 3.0 was enabled in BIOS as well).

And essentially that same device works on the same Laptop in Windows 7.

I run very simple utility that just reads some basic information
from my JTAG probe like serial number, firmware version etc. And I see it
succeeds every second run.

>8---
$ ./opxddiag --list
Opella-XD Diagnostic Utility (OPXDDIAG).
v1.0.4, 17-Aug-2012, (c)Ashling Microsystems Ltd 2012.

Detecting connected Opella-XD(s) ... 1 found

Opella-XD serial number: 880475
   Manufactured: 16-Jul-2008, R1-M0-E0
   Firmware: v1.1.1, 20-Dec-2012
>8---

And for that successful run I see the following in system log:
>8---
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Waiting for status stage event
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Waiting for status stage event
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x1 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x82 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x3 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x84 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Waiting for status stage event
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Waiting for status stage event
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x1 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x82 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x3 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Endpoint 0x84 ep reset callback 
called
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Cancel URB 88037ae293c0, dev 
2, ep 0x82, starting at offset
0x3cd8fd500
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: // Ding dong!
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Stopped on Transfer TRB
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Removing canceled TD starting at 
0x3cd8fd500 (dma).
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Finding endpoint context
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Cycle state = 0x1
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: New dequeue segment = 
8803cd8932c0 (virtual)
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: New dequeue pointer = 
0x3cd8fd550 (DMA)
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Set TR Deq Ptr cmd, new deq seg 
= 8803cd8932c0 (0x3cd8fd000 dma), new
deq ptr = 8803cd8fd550 (0x3cd8fd550 dma), new cycle = 1
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: // Ding dong!
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Successful Set TR Deq Ptr cmd, 
deq = @3cd8fd550
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: ep 0x82 - asked for 65664 bytes, 
16311 bytes untransferred
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: WARN Event TRB for slot 8 ep 4 
with no TDs queued?
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Event TRB with TRB type ID 32
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x0 = 0xcd8fd590
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x4 = 0x3
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x8 = 0xd80
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0xc = 0x8058001
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: ep 0x82 - asked for 65664 bytes, 
16363 bytes untransferred
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: WARN Event TRB for slot 8 ep 4 
with no TDs queued?
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Event TRB with TRB type ID 32
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x0 = 0xcd8fd5e0
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x4 = 0x3
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x8 = 0xd80
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0xc = 0x8058001
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: ep 0x82 - asked for 65664 bytes, 
16347 bytes untransferred
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: WARN Event TRB for slot 8 ep 4 
with no TDs queued?
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Event TRB with TRB type ID 32
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x0 = 0xcd8fd630
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x4 = 0x3
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0x8 = 0xd80
Sep 10 14:11:22 kernel: xhci_hcd :00:14.0: Offset 0xc = 0x8058001
>8---

Now on the next run test utility is executed significantly longer 

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Li Jun]

On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote:
... ...

>  +return -EINVAL;
> >>>
> >>> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> >>> usb_otg_register_hcd() fails?
> >>
> >> You should not call usb_otg_register_hcd() but usb_add_hcd().
> >> If that fails then you fail as ususal.
> > 
> > My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(),
> > then usb_otg_add_hcd() will be called in *all* error case, is this your
> > expectation?
> > if (usb_otg_register_hcd(hcd, irqnum, irqflags, _hcd_intf))
> > return usb_otg_add_hcd(hcd, irqnum, irqflags);
> > 
> 
> Yes, my intention was that if otg fails then it is a non otg HCD so register 
> normally.
> Let me correct my previous statement. If you are absolutely sure
> that the HCD is for otg/dual-role usage then you should call 
> usb_otg_register_hcd().
> 

I think this is not just about a statement, in your usb_otg_register_hcd()
implementation, there are several places will return error, I think only
the first two are for a non-otg HCD case, the following error cases seems
mean this is for otg usage, but it fails in middle of registration, if that
is the case, is it reasonable to call usb_otg_add_hcd()?

>  + * @fsm_running: state machine running/stopped indicator
>  + */
>   struct usb_otg {
>   u8  default_a;
>   
>   struct phy  *phy;
>   /* old usb_phy interface */
>   struct usb_phy  *usb_phy;
>  +
> >>>
> >>> add a blank line?
> >>>
> > 
> > You missed this.
> 
> Sorry. Did you suggest to remove that blank line
> or add a new one before usb_phy?
> 

Remove it.

> > 
>   struct usb_bus  *host;
>   struct usb_gadget   *gadget;
>   
>   enum usb_otg_state  state;
>   
>  +struct device *dev;
>  +struct usb_otg_caps *caps;
>  +struct otg_fsm fsm;
>  +struct otg_fsm_ops fsm_ops;
>  +
>  +/* internal use only */
>  +struct otg_hcd primary_hcd;
>  +struct otg_hcd shared_hcd;
>  +struct otg_gadget_ops *gadget_ops;
>  +struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>  +struct list_head list;
>  +struct work_struct work;
>  -- 
>  2.1.4
> > 
> 
> --
> cheers,
> -roger
--
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: Chipidea ULPI driver

[Felipe Balbi]

(break your lines at 80-characters)

On Thu, Sep 10, 2015 at 02:57:49PM +, Punnaiah Choudary Kalluri wrote:
> 
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Thursday, September 10, 2015 8:14 PM
> > To: Subbaraya Sundeep Bhatta
> > Cc: Peter Chen; ba...@ti.com; linux-usb@vger.kernel.org; linux-
> > ker...@vger.kernel.org; Greg Kroah-Hartman
> > (gre...@linuxfoundation.org); kis...@ti.com; Punnaiah Choudary Kalluri
> > Subject: Re: Chipidea ULPI driver
> > 
> > (break your lines at 80-characters)
> > 
> > On Thu, Sep 10, 2015 at 12:44:58PM +, Subbaraya Sundeep Bhatta wrote:
> > > Hi Peter,
> > >
> > > We are using NOP transceiver driver for USB3320 ULPI PHY with ChipIdea
> > > controller.
> > >
> > > Recently we found that one of the boards (zedboard) requires PHY
> > > register access to set VBUS.
> > >
> > > Note that our local driver we had before migrating to ChipIdea driver
> > > calls otg_ulpi_create with flags  ULPI_OTG_DRVVBUS |
> > > ULPI_OTG_DRVVBUS_EXT so that VBUS is enabled at initialization.
> > >
> > > Can you please let me know how to do this with ChipIdea case? I see
> > > the following solutions:
> > >
> > > 1. Write ULPI driver for USB3320 similar to tusb1210.
> > 
> > this
> 
> How about extending the phy-ulpi driver to use it as platform driver?

this is already doable. See drivers/usb/phy/phy-am335x.c, see how it
uses nop as a library and just adds its own stuff on top.

-- 
balbi


signature.asc
Description: Digital signature

RE: Chipidea ULPI driver

[Punnaiah Choudary Kalluri]

> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Thursday, September 10, 2015 8:14 PM
> To: Subbaraya Sundeep Bhatta
> Cc: Peter Chen; ba...@ti.com; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Greg Kroah-Hartman
> (gre...@linuxfoundation.org); kis...@ti.com; Punnaiah Choudary Kalluri
> Subject: Re: Chipidea ULPI driver
> 
> (break your lines at 80-characters)
> 
> On Thu, Sep 10, 2015 at 12:44:58PM +, Subbaraya Sundeep Bhatta wrote:
> > Hi Peter,
> >
> > We are using NOP transceiver driver for USB3320 ULPI PHY with ChipIdea
> > controller.
> >
> > Recently we found that one of the boards (zedboard) requires PHY
> > register access to set VBUS.
> >
> > Note that our local driver we had before migrating to ChipIdea driver
> > calls otg_ulpi_create with flags  ULPI_OTG_DRVVBUS |
> > ULPI_OTG_DRVVBUS_EXT so that VBUS is enabled at initialization.
> >
> > Can you please let me know how to do this with ChipIdea case? I see
> > the following solutions:
> >
> > 1. Write ULPI driver for USB3320 similar to tusb1210.
> 
> this

How about extending the phy-ulpi driver to use it as platform driver?
So that boards that are using the ulpi compatible phy and driving vbus from the 
phy
can use this driver.

Regards,
Punnaiah
> 
> > 2. Write ci_hdrc_zynq.c which does PHY access required for Zynq.
> 
> not this
> 
> --
> 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 v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 10/09/15 08:35, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
>> On 09/09/15 11:45, Peter Chen wrote:
>>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
 On 09/09/15 11:13, Peter Chen wrote:
> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
>> On 09/09/15 05:21, Peter Chen wrote:
>>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:


 On 08/09/15 11:31, Peter Chen wrote:
> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
>> On 07/09/15 04:23, Peter Chen wrote:
>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
 + * This is used by the USB Host stack to register the Host 
 controller
 + * to the OTG core. Host controller must not be started by the
 + * caller as it is left upto the OTG state machine to do so.
 + *
 + * Returns: 0 on success, error value otherwise.
 + */
 +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 +   unsigned long irqflags, struct otg_hcd_ops 
 *ops)
 +{
 +  struct usb_otg *otgd;
 +  struct device *hcd_dev = hcd->self.controller;
 +  struct device *otg_dev = usb_otg_get_device(hcd_dev);
 +
>>>
>>> One big problem here is: there are two designs for current (IP) 
>>> driver
>>> code, one creates dedicated hcd device as roothub's parent, like 
>>> dwc3.
>>> Another one doesn't do this, roothub's parent is core device (or 
>>> otg device
>>> in your patch), like chipidea and dwc2.
>>>
>>> Then, otg_dev will be glue layer device for chipidea after that.
>>
>> OK. Let's add a way for the otg controller driver to provide the 
>> host and gadget
>> information to the otg core for such devices like chipidea and dwc2.
>>
>
> Roger, not only chipidea and dwc2, I think the musb uses the same
> hierarchy. If the host, device, and otg share the same register
> region, host part can't be a platform driver since we don't want
> to remap the same register region again.
>
> So, in the design, we may need to consider both situations, one
> is otg/host/device has its own register region, and host is a
> separate platform device (A), the other is three parts share the
> same register region, there is only one platform driver (B).
>
> A:
>
>   IP core device 
>   |
>   |
> |-|-|
> gadget   host platform device 
>   |
>   roothub
>
> B:
>
>   IP core device
>   |
>   |
> |-|-|
> gadget roothub
>   
>
>> This API must be called before the hcd/gadget-driver is added so 
>> that the otg
>> core knows it's linked to an OTG controller.
>>
>> Any better idea?
>>
>
> A flag stands for this hcd controller is the same with otg controller
> can be used, this flag can be stored at struct usb_otg_config.

 What if there is another architecture like so?

 C:
[Parent]
   |
   |
|--|--|
[OTG core]  [gadget][host]

 We need a more flexible mechanism to link the gadget and
 host device to the otg core for non DT case.

 How about adding struct usb_otg parameter to usb_otg_register_hcd()?

 e.g.
 int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)

 If otg is NULL it will try DT otg-controller property or parent to
 get the otg controller.
>>>
>>> How usb_otg_register_hcd get struct usb_otg, from where?
>>
>> This only works when the parent driver creating the hcd has registered 
>> the
>> otg controller too.
>>
>
> Sorry? So we need to find another way to solve this issue, right?

 For existing cases this is sufficient.
 The otg device is either the one supplied during usb_otg_register_hcd
 (cases B and C) or it is the parent device (case A).
>>>
>>> How we differentiate case A and case B at usb_otg_register_hcd?
>>> Would you 

Re: Chipidea ULPI driver

[Felipe Balbi]

(break your lines at 80-characters)

On Thu, Sep 10, 2015 at 12:44:58PM +, Subbaraya Sundeep Bhatta wrote:
> Hi Peter,
> 
> We are using NOP transceiver driver for USB3320 ULPI PHY with ChipIdea
> controller.
> 
> Recently we found that one of the boards (zedboard) requires PHY
> register access to set VBUS.
> 
> Note that our local driver we had before migrating to ChipIdea driver
> calls otg_ulpi_create with flags  ULPI_OTG_DRVVBUS |
> ULPI_OTG_DRVVBUS_EXT so that VBUS is enabled at initialization.
> 
> Can you please let me know how to do this with ChipIdea case? I see
> the following solutions:
> 
> 1. Write ULPI driver for USB3320 similar to tusb1210.

this

> 2. Write ci_hdrc_zynq.c which does PHY access required for Zynq.

not this

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Roger Quadros]

On 10/09/15 12:28, Li Jun wrote:
> On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote:
> ... ...
> 
>> +return -EINVAL;
>
> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> usb_otg_register_hcd() fails?

 You should not call usb_otg_register_hcd() but usb_add_hcd().
 If that fails then you fail as ususal.
>>>
>>> My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(),
>>> then usb_otg_add_hcd() will be called in *all* error case, is this your
>>> expectation?
>>> if (usb_otg_register_hcd(hcd, irqnum, irqflags, _hcd_intf))
>>> return usb_otg_add_hcd(hcd, irqnum, irqflags);
>>>
>>
>> Yes, my intention was that if otg fails then it is a non otg HCD so register 
>> normally.
>> Let me correct my previous statement. If you are absolutely sure
>> that the HCD is for otg/dual-role usage then you should call 
>> usb_otg_register_hcd().
>>
> 
> I think this is not just about a statement, in your usb_otg_register_hcd()
> implementation, there are several places will return error, I think only
> the first two are for a non-otg HCD case, the following error cases seems
> mean this is for otg usage, but it fails in middle of registration, if that
> is the case, is it reasonable to call usb_otg_add_hcd()?

OK. We need to check the return value then and differentiate if it is non-otg
or otg with failure.
If it is non-otg then only we must call usb_otg_add_hcd().

I will fix usb_add_hcd().

> 
>> + * @fsm_running: state machine running/stopped indicator
>> + */
>>  struct usb_otg {
>>  u8  default_a;
>>  
>>  struct phy  *phy;
>>  /* old usb_phy interface */
>>  struct usb_phy  *usb_phy;
>> +
>
> add a blank line?
>
>>>
>>> You missed this.
>>
>> Sorry. Did you suggest to remove that blank line
>> or add a new one before usb_phy?
>>
> 
> Remove it.

OK.

> 
>>>
>>  struct usb_bus  *host;
>>  struct usb_gadget   *gadget;
>>  
>>  enum usb_otg_state  state;
>>  
>> +struct device *dev;
>> +struct usb_otg_caps *caps;
>> +struct otg_fsm fsm;
>> +struct otg_fsm_ops fsm_ops;
>> +
>> +/* internal use only */
>> +struct otg_hcd primary_hcd;
>> +struct otg_hcd shared_hcd;
>> +struct otg_gadget_ops *gadget_ops;
>> +struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +struct list_head list;
>> +struct work_struct work;
>> -- 
>> 2.1.4
>>>
--
cheers,
-roger
--
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

[RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Laura Abbott]

We received several reports of systems rebooting and powering on
after an attempted shutdown. Testing showed that setting
XHCI_SPURIOUS_WAKEUP quirk in addition to the XHCI_SPURIOUS_REBOOT
quirk allowed the system to shutdown as expected for Lynxpoint
xHCI controllers. Set the qurik.

Signed-off-by: Laura Abbott 
---
Bugzilla for reference:

https://bugzilla.redhat.com/show_bug.cgi?id=1257131
https://bugzilla.redhat.com/show_bug.cgi?id=1189107

There was some discussion if this is actually needed across all
chipsets or if it's just some revision. Not sure how to narrow
that down.
---
 drivers/usb/host/xhci-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 5590eac..e6ed595 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -147,6 +147,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
xhci->quirks |= XHCI_SPURIOUS_REBOOT;
+   xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI ||
-- 
2.4.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: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode

[Hans de Goede]

Hi,

On 04-09-15 08:43, Olliver Schinagl wrote:

Hey Hans,

On 07-08-15 10:45, Olliver Schinagl wrote:



If you change the dr_mode to host then you _must_ also remove any id_det and 
vbus_det
gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
host vs otg mode by checking for the presence of these.

Yes, this fixes it and makes it work. Thanks.


I've been going back to this and am wondering if this is something I can look 
into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore 
the pins and treat them as unset?


AFAIK you cannot unset something in dts. The only solution I
can comeup with is to add a dr_mode argument to the phy like
we already have for the otg controller itself.

This is something which we likely need to do anyways to add
support for peripheral only mode, which we seem to need for
some "hdmi sticks".

Patches doing this are welcome from my pov.

Regards,

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

Re: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode

[Hans de Goede]

Hi,

On 10-09-15 20:30, Maxime Ripard wrote:

On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:

Hi,

On 04-09-15 08:43, Olliver Schinagl wrote:

Hey Hans,

On 07-08-15 10:45, Olliver Schinagl wrote:



If you change the dr_mode to host then you _must_ also remove any id_det and 
vbus_det
gpio settings from the usb_phy node in the dts, as the sun4i phy code detects
host vs otg mode by checking for the presence of these.

Yes, this fixes it and makes it work. Thanks.


I've been going back to this and am wondering if this is something I can look 
into to fix properly? E.g. if the dts sets dr_mode = host, can we simply ignore 
the pins and treat them as unset?


AFAIK you cannot unset something in dts. The only solution I
can comeup with is to add a dr_mode argument to the phy like
we already have for the otg controller itself.

This is something which we likely need to do anyways to add
support for peripheral only mode, which we seem to need for
some "hdmi sticks".


I haven't really followed the rest of the discussion, so sorry if you
already talked about that, but why can't you just set the dr_mode to
peripheral in such a case?


This is about the usbphy code not the musb-controller code, which are
2 different dts nodes, atm only the musb-controller node has a
dr_mode property, and the phy code decides between host-only
and otg mode based on whether an id pin is assigned or not.

My proposal is to get rid of the id-pin hack to determine the mode
and add a dr_mode property to the usbphy dts node.

Regards,

Hans
--
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: amd5536udc: fix NULL pointer dereference

[David Cohen]

Hi Sudip,

On Fri, Sep 04, 2015 at 05:12:23PM +0530, Sudip Mukherjee wrote:
> We were checking if dev->regs is NULL but it was done after
> dereferencing it. Lets reset the controller and iounmap dev->regs only
> if it is not NULL.
> free_irq() does not need dev->regs, so unmaping it before freeing the
> irq should not matter.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/usb/gadget/udc/amd5536udc.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
> b/drivers/usb/gadget/udc/amd5536udc.c
> index fdacddb..26066d3 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -3135,11 +3135,12 @@ static void udc_pci_remove(struct pci_dev *pdev)
>   }
>  
>   /* reset controller */
> - writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
> + if (dev->regs) {
> + writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), >regs->cfg);
> + iounmap(dev->regs);

I'm not familiar with the driver, but you're iounmap'ing before freeing
irq. Looks fishy to me.

Br, David

> + }
>   if (dev->irq_registered)
>   free_irq(pdev->irq, dev);
> - if (dev->regs)
> - iounmap(dev->regs);
>   if (dev->mem_region)
>   release_mem_region(pci_resource_start(pdev, 0),
>   pci_resource_len(pdev, 0));
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode

[Maxime Ripard]

On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 04-09-15 08:43, Olliver Schinagl wrote:
> >Hey Hans,
> >
> >On 07-08-15 10:45, Olliver Schinagl wrote:
> >>
> >>>If you change the dr_mode to host then you _must_ also remove any id_det 
> >>>and vbus_det
> >>>gpio settings from the usb_phy node in the dts, as the sun4i phy code 
> >>>detects
> >>>host vs otg mode by checking for the presence of these.
> >>Yes, this fixes it and makes it work. Thanks.
> >>
> >I've been going back to this and am wondering if this is something I can 
> >look into to fix properly? E.g. if the dts sets dr_mode = host, can we 
> >simply ignore the pins and treat them as unset?
> 
> AFAIK you cannot unset something in dts. The only solution I
> can comeup with is to add a dr_mode argument to the phy like
> we already have for the otg controller itself.
> 
> This is something which we likely need to do anyways to add
> support for peripheral only mode, which we seem to need for
> some "hdmi sticks".

I haven't really followed the rest of the discussion, so sorry if you
already talked about that, but why can't you just set the dr_mode to
peripheral in such a case?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

[Roland Weber]

Hi Alan,

> The only reason I can think of why it might hang is if some clock got 
> turned off.  But I don't know of any clock which would have that 
> effect, or which would get turned off before we reach this point.
> 
> I also don't understand why the ehci_readl() would freeze when the 
> preceding ehci_writel() succeeded.  Can you try putting a copy of the 
> ehci_readl() line just before the ehci_writel(), to see if it will work 
> there?

Done:
printk(KERN_INFO "ehci_halt: about to readl prematurely\n");
temp = ehci_readl(ehci, >regs->command);
printk(KERN_INFO "ehci_halt: premature readl returned %x\n", temp);

/* disable any irqs left enabled by previous code */
ehci_writel(ehci, 0, >regs->intr_enable);
printk(KERN_INFO "ehci_halt: after first ehci_writel\n");

It works there. Result:
premature readl returned 1

thanks and cheers,
  Roland
--
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 0/1] USB DWC2 parity fix in isochronous mode

[Scott Branden]

This patch contains a fix for a real world interop problem found
when using the Synopsis DWC2 USB controller with isochronous audio as
detailed in the commit message.

Changes from v2:
 - created s2c_hsotg_chage_ep_iso_parity function to call function in 3 places 
in code
 - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS

Changes from v1:
 - Address code review comments as per previous responses:
 - renamed parity_set to has_correct_parity and reorder some logic


Roman Bacik (1):
  usb: dwc2: gadget: parity fix in isochronous mode

 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/gadget.c | 58 ++-
 drivers/usb/dwc2/hw.h |  1 +
 3 files changed, 54 insertions(+), 6 deletions(-)

-- 
2.5.0

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

[PATCH v3 1/1] usb: dwc2: gadget: parity fix in isochronous mode

[Scott Branden]

From: Roman Bacik 

USB OTG driver in isochronous mode has to set the parity of the receiving
microframe. The parity is set to even by default. This causes problems for
an audio gadget, if the host starts transmitting on odd microframes.

This fix uses Incomplete Periodic Transfer interrupt to toggle between
even and odd parity until the Transfer Complete interrupt is received.

Signed-off-by: Roman Bacik 
Reviewed-by: Abhinav Ratna 
Reviewed-by: Srinath Mannam 
Signed-off-by: Scott Branden 
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/gadget.c | 58 ++-
 drivers/usb/dwc2/hw.h |  1 +
 3 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0ed87620..a5634fd 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
unsigned intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
+   unsigned inthas_correct_parity:1;
 
charname[10];
 };
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4d47b7c..efaab76 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1916,6 +1916,19 @@ static void s3c_hsotg_complete_in(struct dwc2_hsotg 
*hsotg,
s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
 }
 
+static void s3c_hsotg_change_ep_iso_parity(struct dwc2_hsotg *hsotg,
+   u32 epctl_reg)
+{
+   u32 ctrl;
+
+   ctrl = readl(hsotg->regs + epctl_reg);
+   if (ctrl & DXEPCTL_EOFRNUM)
+   ctrl |= DXEPCTL_SETEVENFR;
+   else
+   ctrl |= DXEPCTL_SETODDFR;
+   writel(ctrl, hsotg->regs + epctl_reg);
+}
+
 /**
  * s3c_hsotg_epint - handle an in/out endpoint interrupt
  * @hsotg: The driver state
@@ -1954,12 +1967,9 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
ints &= ~DXEPINT_XFERCOMPL;
 
if (ints & DXEPINT_XFERCOMPL) {
+   hs_ep->has_correct_parity = 1;
if (hs_ep->isochronous && hs_ep->interval == 1) {
-   if (ctrl & DXEPCTL_EOFRNUM)
-   ctrl |= DXEPCTL_SETEVENFR;
-   else
-   ctrl |= DXEPCTL_SETODDFR;
-   writel(ctrl, hsotg->regs + epctl_reg);
+   s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg);
}
 
dev_dbg(hsotg->dev,
@@ -2316,7 +2326,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
GINTSTS_RESETDET | GINTSTS_ENUMDONE |
GINTSTS_OTGINT | GINTSTS_USBSUSP |
-   GINTSTS_WKUPINT,
+   GINTSTS_WKUPINT |
+   GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT,
hsotg->regs + GINTMSK);
 
if (using_dma(hsotg))
@@ -2581,6 +2592,40 @@ irq_retry:
s3c_hsotg_dump(hsotg);
}
 
+   if (gintsts & GINTSTS_INCOMPL_SOIN) {
+   u32 idx, epctl_reg;
+   struct s3c_hsotg_ep *hs_ep;
+
+   dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n", __func__);
+   for (idx = 1; idx < hsotg->num_of_eps; idx++) {
+   hs_ep = hsotg->eps_in[idx];
+
+   if (!hs_ep->isochronous || hs_ep->has_correct_parity)
+   continue;
+
+   epctl_reg = DIEPCTL(idx);
+   s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg);
+   }
+   writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
+   }
+
+   if (gintsts & GINTSTS_INCOMPL_SOOUT) {
+   u32 idx, epctl_reg;
+   struct s3c_hsotg_ep *hs_ep;
+
+   dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n", __func__);
+   for (idx = 1; idx < hsotg->num_of_eps; idx++) {
+   hs_ep = hsotg->eps_out[idx];
+
+   if (!hs_ep->isochronous || hs_ep->has_correct_parity)
+   continue;
+
+   epctl_reg = DOEPCTL(idx);
+   s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg);
+   }
+   writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS);
+   }
+
/*
 * if we've had fifo events, we should try and go around the
 * loop again to see if there's any point in returning yet.
@@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
hs_ep->periodic = 0;
hs_ep->halted = 0;
hs_ep->interval = desc->bInterval;
+   hs_ep->has_correct_parity = 0;
 
if (hs_ep->interval > 1 && hs_ep->mc > 1)

Re: [PATCH] xhci: create one unified function to calculate TRB TD remainder.

[chunfeng yun]

Hi,

It works ok when I add MTK's quirk into xhci_td_remainder(), and test it
by usb camera, udisk, usb ethernet adapter, and hub.


On Tue, 2015-09-08 at 14:09 +0300, Mathias Nyman wrote:
> xhci versions 1.0 and later report the untransferred data remaining in a
> TD a bit differently than older hosts.
> 
> We used to have separate functions for these, and needed to check host
>  version before calling the right function.
> 
> Now Mediatek host has an additional quirk on how it uses the TD Size
> field for remaining data. To prevent yet another function for calculating
> remainder we instead want to make one quirk friendly unified function.
> 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-ring.c | 106 
> ++-
>  drivers/usb/host/xhci.h  |   2 +
>  2 files changed, 46 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a47a1e8..57f40a1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3020,21 +3020,6 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t 
> mem_flags,
>  }
>  
>  /*
> - * The TD size is the number of bytes remaining in the TD (including this 
> TRB),
> - * right shifted by 10.
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> - */
> -static u32 xhci_td_remainder(unsigned int remainder)
> -{
> - u32 max = (1 << (21 - 17 + 1)) - 1;
> -
> - if ((remainder >> 10) >= max)
> - return max << 17;
> - else
> - return (remainder >> 10) << 17;
> -}
> -
> -/*
>   * For xHCI 1.0 host controllers, TD size is the number of max packet sized
>   * packets remaining in the TD (*not* including this TRB).
>   *
> @@ -3046,30 +3031,36 @@ static u32 xhci_td_remainder(unsigned int remainder)
>   *
>   * TD size = total_packet_count - packets_transferred
>   *
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> + * For xHCI 0.96 and older, TD size field should be the remaining bytes
> + * including this TRB, right shifted by 10
> + *
> + * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
> + * This is taken care of in the TRB_TD_SIZE() macro
> + *
>   * The last TRB in a TD must have the TD size set to zero.
>   */
> -static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
> - unsigned int total_packet_count, struct urb *urb,
> - unsigned int num_trbs_left)
> +static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
> +   int trb_buff_len, unsigned int td_total_len,
> +   struct urb *urb, unsigned int num_trbs_left)
>  {
> - int packets_transferred;
> + u32 maxp, total_packet_count;
> +
> + if (xhci->hci_version < 0x100)
> + return ((td_total_len - transferred) >> 10);
> +
> + maxp = GET_MAX_PACKET(usb_endpoint_maxp(>ep->desc));
> + total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
>  
>   /* One TRB with a zero-length data packet. */
> - if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> + if (num_trbs_left == 0 || (transferred == 0 && trb_buff_len == 0) ||
> + trb_buff_len == td_total_len)
>   return 0;
>  
> - /* All the TRB queueing functions don't count the current TRB in
> -  * running_total.
> -  */
> - packets_transferred = (running_total + trb_buff_len) /
> - GET_MAX_PACKET(usb_endpoint_maxp(>ep->desc));
> -
> - if ((total_packet_count - packets_transferred) > 31)
> - return 31 << 17;
> - return (total_packet_count - packets_transferred) << 17;
> + /* Queueing functions don't count the current TRB into transferred */
> + return (total_packet_count - ((transferred + trb_buff_len) / maxp));
>  }
>  
> +
>  static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>   struct urb *urb, int slot_id, unsigned int ep_index)
>  {
> @@ -3191,17 +3182,12 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, 
> gfp_t mem_flags,
>   }
>  
>   /* Set the TRB length, TD size, and interrupter fields. */
> - if (xhci->hci_version < 0x100) {
> - remainder = xhci_td_remainder(
> - urb->transfer_buffer_length -
> - running_total);
> - } else {
> - remainder = xhci_v1_0_td_remainder(running_total,
> - trb_buff_len, total_packet_count, urb,
> - num_trbs - 1);
> - }
> + remainder = xhci_td_remainder(xhci, running_total, trb_buff_len,
> +urb->transfer_buffer_length,
> +urb, num_trbs - 1);
> +
>   length_field = TRB_LEN(trb_buff_len) |
> -  

Re: [PATCH v7 1/5] dt-bindings: Add usb3.0 phy binding for MT65xx SoCs

[chunfeng yun]

Hi,
On Tue, 2015-09-08 at 19:16 -0500, Rob Herring wrote:
> On 09/08/2015 01:17 AM, Chunfeng Yun wrote:
> > add a DT binding documentation of usb3.0 phy for MT65xx
> > SoCs from Mediatek.
> > 
> > Signed-off-by: Chunfeng Yun 
> 
> One comment, otherwise:
> 
> Acked-by: Rob Herring 
> 
> > ---
> >  .../devicetree/bindings/phy/phy-mt65xx-usb.txt | 69 
> > ++
> >  1 file changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt 
> > b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> > new file mode 100644
> > index 000..5812d20
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
> > @@ -0,0 +1,69 @@
> > +mt65xx USB3.0 PHY binding
> > +--
> > +
> > +This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC.
> > +
> > +Required properties (controller (parent) node):
> > + - compatible  : should be "mediatek,mt8173-u3phy"
> > + - reg : offset and length of register for phy, exclude port's
> > + register.
> > + - clocks  : a list of phandle + clock-specifier pairs, one for each
> > + entry in clock-names
> > + - clock-names : must contain
> > + "u3phya_ref": for reference clock of usb3.0 analog phy.
> > +
> > +Required nodes : a sub-node is required for each port the controller
> > + provides. Address range information including the usual
> > + 'reg' property is used inside these nodes to describe
> > + the controller's topology. These nodes are translated
> > + by the driver's .xlate() function.
> 
> The last sentence is Linux specific. Please remove.
> 
Ok, I will remove it later. Thanks
> > +
> > +Required properties (port (child) node):
> > +- reg  : address and length of the register set for the port.
> > +- #phy-cells   : should be 1 (See second example)
> > + cell after port phandle is phy type from:
> > +   - PHY_TYPE_USB2
> > +   - PHY_TYPE_USB3
> > +
> > +Example:
> > +
> > +u3phy: usb-phy@1129 {
> > +   compatible = "mediatek,mt8173-u3phy";
> > +   reg = <0 0x1129 0 0x800>;
> > +   clocks = < CLK_APMIXED_REF2USB_TX>;
> > +   clock-names = "u3phya_ref";
> > +   #address-cells = <2>;
> > +   #size-cells = <2>;
> > +   ranges;
> > +   status = "okay";
> > +
> > +   phy_port0: port@11290800 {
> > +   reg = <0 0x11290800 0 0x800>;
> > +   #phy-cells = <1>;
> > +   status = "okay";
> > +   };
> > +
> > +   phy_port1: port@11291000 {
> > +   reg = <0 0x11291000 0 0x800>;
> > +   #phy-cells = <1>;
> > +   status = "okay";
> > +   };
> > +};
> > +
> > +Specifying phy control of devices
> > +-
> > +
> > +Device nodes should specify the configuration required in their "phys"
> > +property, containing a phandle to the phy port node and a device type;
> > +phy-names for each port are optional.
> > +
> > +Example:
> > +
> > +#include 
> > +
> > +usb30: usb@1127 {
> > +   ...
> > +   phys = <_port0 PHY_TYPE_USB3>;
> > +   phy-names = "usb3-0";
> > +   ...
> > +};
> > 
> 


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

Re: [PATCH v4 07/13] usb: otg: add OTG core

[Peter Chen]

On Thu, Sep 10, 2015 at 05:17:36PM +0300, Roger Quadros wrote:
> On 10/09/15 08:35, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
> >> On 09/09/15 11:45, Peter Chen wrote:
> >>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
>  On 09/09/15 11:13, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
> >> On 09/09/15 05:21, Peter Chen wrote:
> >>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> 
> 
>  On 08/09/15 11:31, Peter Chen wrote:
> > On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >> On 07/09/15 04:23, Peter Chen wrote:
> >>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
>  + * This is used by the USB Host stack to register the Host 
>  controller
>  + * to the OTG core. Host controller must not be started by the
>  + * caller as it is left upto the OTG state machine to do so.
>  + *
>  + * Returns: 0 on success, error value otherwise.
>  + */
>  +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int 
>  irqnum,
>  + unsigned long irqflags, struct 
>  otg_hcd_ops *ops)
>  +{
>  +struct usb_otg *otgd;
>  +struct device *hcd_dev = hcd->self.controller;
>  +struct device *otg_dev = usb_otg_get_device(hcd_dev);
>  +
> >>>
> >>> One big problem here is: there are two designs for current (IP) 
> >>> driver
> >>> code, one creates dedicated hcd device as roothub's parent, like 
> >>> dwc3.
> >>> Another one doesn't do this, roothub's parent is core device (or 
> >>> otg device
> >>> in your patch), like chipidea and dwc2.
> >>>
> >>> Then, otg_dev will be glue layer device for chipidea after that.
> >>
> >> OK. Let's add a way for the otg controller driver to provide the 
> >> host and gadget
> >> information to the otg core for such devices like chipidea and 
> >> dwc2.
> >>
> >
> > Roger, not only chipidea and dwc2, I think the musb uses the same
> > hierarchy. If the host, device, and otg share the same register
> > region, host part can't be a platform driver since we don't want
> > to remap the same register region again.
> >
> > So, in the design, we may need to consider both situations, one
> > is otg/host/device has its own register region, and host is a
> > separate platform device (A), the other is three parts share the
> > same register region, there is only one platform driver (B).
> >
> > A:
> >
> > IP core device 
> > |
> > |
> >   |-|-|
> >   gadget   host platform device 
> > |
> > roothub
> >
> > B:
> >
> > IP core device
> > |
> > |
> >   |-|-|
> >   gadget roothub
> > 
> >
> >> This API must be called before the hcd/gadget-driver is added so 
> >> that the otg
> >> core knows it's linked to an OTG controller.
> >>
> >> Any better idea?
> >>
> >
> > A flag stands for this hcd controller is the same with otg 
> > controller
> > can be used, this flag can be stored at struct usb_otg_config.
> 
>  What if there is another architecture like so?
> 
>  C:
>   [Parent]
>  |
>  |
>   |--|--|
>   [OTG core]  [gadget][host]
> 
>  We need a more flexible mechanism to link the gadget and
>  host device to the otg core for non DT case.
> 
>  How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> 
>  e.g.
>  int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, 
>  ..)
> 
>  If otg is NULL it will try DT otg-controller property or parent to
>  get the otg controller.
> >>>
> >>> How usb_otg_register_hcd get struct usb_otg, from where?
> >>
> >> This only works when the parent driver creating the hcd has registered 
> >> the
> >> otg controller too.
> >>
> >
> > 

Re: [PATCH v7 2/5] dt-bindings: Add a binding for Mediatek xHCI host controller

[chunfeng yun]

On Tue, 2015-09-08 at 19:30 -0500, Rob Herring wrote:
> On 09/08/2015 01:18 AM, Chunfeng Yun wrote:
> > add a DT binding documentation of xHCI host controller for the
> > MT8173 SoC from Mediatek.
> > 
> > Signed-off-by: Chunfeng Yun 
> > ---
> >  .../devicetree/bindings/usb/mt8173-xhci.txt| 52 
> > ++
> >  1 file changed, 52 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt 
> > b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> > new file mode 100644
> > index 000..d851bf1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> > @@ -0,0 +1,52 @@
> > +MT8173 xHCI
> > +
> > +The device node for Mediatek SOC USB3.0 host controller
> > +
> > +Required properties:
> > + - compatible : should contain "mediatek,mt8173-xhci"
> > + - reg : specifies physical base address and size of the registers,
> > +   the first one for MAC, the second for IPPC
> > + - interrupts : interrupt used by the controller
> > + - power-domains : a phandle to USB power domain node to control USB's
> > +   mtcmos
> > + - vusb33-supply : regulator of USB avdd3.3v
> > +
> > + - clocks : a list of phandle + clock-specifier pairs, one for each
> > +   entry in clock-names
> > + - clock-names : must contain
> > +   "sys_ck": for clock of xHCI MAC
> > +   "wakeup_deb_p0": for USB wakeup debounce clock of port0
> > +   "wakeup_deb_p0": for USB wakeup debounce clock of port1
> > +
> > + - phys : a list of phandle + phy specifier pairs
> > + - usb3-lpm-capable : supports USB3.0 LPM
> > + - mediatek,syscon-wakeup : phandle to syscon used to access USB wakeup
> > +   control register
> 
> Wake-up should probably be done in some generic way. That said, I don't
> have a specific suggestion other than make it optional.
> 
> > + - mediatek,wakeup-src : 1: ip sleep wakeup mode; 2: line state wakeup
> > +   mode; Others means don't enable wakeup source of USB
> 
> This should be optional rather than any other value meaning disabled.
> 
Ok, "mediatek,wakeup-src" and "mediatek,syscon-wakeup" properties will
be put into optional.

Thanks a lot
> > +
> > +Optional properties:
> > + - vbus-supply : reference to the VBUS regulator;
> > +
> > +Example:
> > +usb30: usb@1127 {
> > +   compatible = "mediatek,mt8173-xhci";
> > +   reg = <0 0x1127 0 0x1000>,
> > + <0 0x11280700 0 0x0100>;
> > +   interrupts = ;
> > +   power-domains = < MT8173_POWER_DOMAIN_USB>;
> > +   clocks = < CLK_TOP_USB30_SEL>,
> > +< CLK_PERI_USB0>,
> > +< CLK_PERI_USB1>;
> > +   clock-names = "sys_ck",
> > + "wakeup_deb_p0",
> > + "wakeup_deb_p1";
> > +   phys = <_port0 PHY_TYPE_USB3>,
> > +  <_port1 PHY_TYPE_USB2>;
> > +   vusb33-supply = <_vusb_reg>;
> > +   vbus-supply = <_p1_vbus>;
> > +   usb3-lpm-capable;
> > +   mediatek,syscon-wakeup = <>;
> > +   mediatek,wakeup-src = <1>;
> > +   status = "okay";
> > +};
> > 
> 


--
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: Chipidea ULPI driver

[Peter Chen]

On Thu, Sep 10, 2015 at 02:57:49PM +, Punnaiah Choudary Kalluri wrote:
> 
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Thursday, September 10, 2015 8:14 PM
> > To: Subbaraya Sundeep Bhatta
> > Cc: Peter Chen; ba...@ti.com; linux-usb@vger.kernel.org; linux-
> > ker...@vger.kernel.org; Greg Kroah-Hartman
> > (gre...@linuxfoundation.org); kis...@ti.com; Punnaiah Choudary Kalluri
> > Subject: Re: Chipidea ULPI driver
> > 
> > (break your lines at 80-characters)
> > 
> > On Thu, Sep 10, 2015 at 12:44:58PM +, Subbaraya Sundeep Bhatta wrote:
> > > Hi Peter,
> > >
> > > We are using NOP transceiver driver for USB3320 ULPI PHY with ChipIdea
> > > controller.
> > >
> > > Recently we found that one of the boards (zedboard) requires PHY
> > > register access to set VBUS.
> > >
> > > Note that our local driver we had before migrating to ChipIdea driver
> > > calls otg_ulpi_create with flags  ULPI_OTG_DRVVBUS |
> > > ULPI_OTG_DRVVBUS_EXT so that VBUS is enabled at initialization.
> > >
> > > Can you please let me know how to do this with ChipIdea case? I see
> > > the following solutions:
> > >
> > > 1. Write ULPI driver for USB3320 similar to tusb1210.
> > 
> > this
> 
> How about extending the phy-ulpi driver to use it as platform driver?
> So that boards that are using the ulpi compatible phy and driving vbus from 
> the phy
> can use this driver.
> 

Yes, you can improve phy-ulpi driver, and it can not depend on
NOP transceiver driver.

-- 

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] xhci: create one unified function to calculate TRB TD remainder.

[chunfeng yun]

Hi,
On Tue, 2015-09-08 at 14:09 +0300, Mathias Nyman wrote:
> xhci versions 1.0 and later report the untransferred data remaining in a
> TD a bit differently than older hosts.
> 
> We used to have separate functions for these, and needed to check host
>  version before calling the right function.
> 
> Now Mediatek host has an additional quirk on how it uses the TD Size
> field for remaining data. To prevent yet another function for calculating
> remainder we instead want to make one quirk friendly unified function.
> 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-ring.c | 106 
> ++-
>  drivers/usb/host/xhci.h  |   2 +
>  2 files changed, 46 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a47a1e8..57f40a1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3020,21 +3020,6 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t 
> mem_flags,
>  }
>  
>  /*
> - * The TD size is the number of bytes remaining in the TD (including this 
> TRB),
> - * right shifted by 10.
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> - */
> -static u32 xhci_td_remainder(unsigned int remainder)
> -{
> - u32 max = (1 << (21 - 17 + 1)) - 1;
> -
> - if ((remainder >> 10) >= max)
> - return max << 17;
> - else
> - return (remainder >> 10) << 17;
> -}
> -
> -/*
>   * For xHCI 1.0 host controllers, TD size is the number of max packet sized
>   * packets remaining in the TD (*not* including this TRB).
>   *
> @@ -3046,30 +3031,36 @@ static u32 xhci_td_remainder(unsigned int remainder)
>   *
>   * TD size = total_packet_count - packets_transferred
>   *
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> + * For xHCI 0.96 and older, TD size field should be the remaining bytes
> + * including this TRB, right shifted by 10
> + *
> + * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
> + * This is taken care of in the TRB_TD_SIZE() macro
> + *
>   * The last TRB in a TD must have the TD size set to zero.
>   */
> -static u32 xhci_v1_0_td_remainder(int running_total, int trb_buff_len,
> - unsigned int total_packet_count, struct urb *urb,
> - unsigned int num_trbs_left)
> +static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
> +   int trb_buff_len, unsigned int td_total_len,
> +   struct urb *urb, unsigned int num_trbs_left)
>  {
> - int packets_transferred;
> + u32 maxp, total_packet_count;
> +
> + if (xhci->hci_version < 0x100)
> + return ((td_total_len - transferred) >> 10);
> +
> + maxp = GET_MAX_PACKET(usb_endpoint_maxp(>ep->desc));
> + total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
>  
>   /* One TRB with a zero-length data packet. */
> - if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> + if (num_trbs_left == 0 || (transferred == 0 && trb_buff_len == 0) ||
> + trb_buff_len == td_total_len)
>   return 0;
>  
I think when trb_buff_len == td_total_len, the TD only needs one trb, so
num_trbs_left will equal to 0; that means no need add this condition.

> - /* All the TRB queueing functions don't count the current TRB in
> -  * running_total.
> -  */
> - packets_transferred = (running_total + trb_buff_len) /
> - GET_MAX_PACKET(usb_endpoint_maxp(>ep->desc));
> -
> - if ((total_packet_count - packets_transferred) > 31)
> - return 31 << 17;
> - return (total_packet_count - packets_transferred) << 17;
> + /* Queueing functions don't count the current TRB into transferred */
> + return (total_packet_count - ((transferred + trb_buff_len) / maxp));
>  }
>  
> +
>  static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>   struct urb *urb, int slot_id, unsigned int ep_index)
>  {
> @@ -3191,17 +3182,12 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, 
> gfp_t mem_flags,
>   }
>  
>   /* Set the TRB length, TD size, and interrupter fields. */
> - if (xhci->hci_version < 0x100) {
> - remainder = xhci_td_remainder(
> - urb->transfer_buffer_length -
> - running_total);
> - } else {
> - remainder = xhci_v1_0_td_remainder(running_total,
> - trb_buff_len, total_packet_count, urb,
> - num_trbs - 1);
> - }
> + remainder = xhci_td_remainder(xhci, running_total, trb_buff_len,
> +urb->transfer_buffer_length,
> +urb, num_trbs - 1);
> +
>   length_field = TRB_LEN(trb_buff_len) 

RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode

[Roman Bacik]

> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-10-15 12:08 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> 
> On 9/10/2015 10:56 AM, Roman Bacik wrote:
> >> -Original Message-
> >> From: Roman Bacik
> >> Sent: September-09-15 7:59 PM
> >> To: 'John Youn'; Scott Branden; 'Greg Kroah-Hartman'; 'linux-
> >> u...@vger.kernel.org'
> >> Cc: 'linux-ker...@vger.kernel.org'; bcm-kernel-feedback-list
> >> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> >> isochronous mode
> >>
> >>> -Original Message-
> >>> From: Roman Bacik
> >>> Sent: September-09-15 7:36 PM
> >>> To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
> >>> u...@vger.kernel.org
> >>> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> >>> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> >>> isochronous mode
> >>>
>  -Original Message-
>  From: John Youn [mailto:john.y...@synopsys.com]
>  Sent: September-09-15 7:25 PM
>  To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
>  linux- u...@vger.kernel.org
>  Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
>  Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
>  isochronous mode
> 
>  On 9/9/2015 7:16 PM, Roman Bacik wrote:
> >> -Original Message-
> >> From: John Youn [mailto:john.y...@synopsys.com]
> >> Sent: September-09-15 7:11 PM
> >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> >> linux- u...@vger.kernel.org
> >> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> >> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> >> isochronous mode
> >>
> >> On 9/9/2015 11:16 AM, Roman Bacik wrote:
>  -Original Message-
>  From: John Youn [mailto:john.y...@synopsys.com]
>  Sent: September-03-15 11:53 PM
>  To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
>  u...@vger.kernel.org; Roman Bacik
>  Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
>  Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
>  isochronous mode
> 
>  On 8/31/2015 9:17 AM, Scott Branden wrote:
> > From: Roman Bacik 
> >
> > USB OTG driver in isochronous mode has to set the parity of
> > the receiving microframe. The parity is set to even by
> > default. This causes problems for an audio gadget, if the host
> > starts transmitting on odd
>  microframes.
> >
> > This fix uses Incomplete Periodic Transfer interrupt to toggle
> > between even and odd parity until the Transfer Complete
> > interrupt is
> >> received.
> >
> > Signed-off-by: Roman Bacik 
> > Reviewed-by: Abhinav Ratna 
> > Reviewed-by: Srinath Mannam
> >> 
> > Signed-off-by: Scott Branden 
> > ---
> >  drivers/usb/dwc2/core.h   |  1 +
> >  drivers/usb/dwc2/gadget.c | 51
>  ++-
> >  drivers/usb/dwc2/hw.h |  1 +
> >  3 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> > index 0ed87620..a5634fd 100644
> > --- a/drivers/usb/dwc2/core.h
> > +++ b/drivers/usb/dwc2/core.h
> > @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
> > unsigned intperiodic:1;
> > unsigned intisochronous:1;
> > unsigned intsend_zlp:1;
> > +   unsigned inthas_correct_parity:1;
> >
> > charname[10];
> >  };
> > diff --git a/drivers/usb/dwc2/gadget.c
> > b/drivers/usb/dwc2/gadget.c index 4d47b7c..fac3e2f 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> > @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct
> > dwc2_hsotg
>  *hsotg, unsigned int idx,
> > ints &= ~DXEPINT_XFERCOMPL;
> >
> > if (ints & DXEPINT_XFERCOMPL) {
> > +   hs_ep->has_correct_parity = 1;
> > if (hs_ep->isochronous && hs_ep->interval == 1) {
> > if (ctrl & DXEPCTL_EOFRNUM)
> > ctrl |= DXEPCTL_SETEVENFR; @@ -
> >> 2316,7 +2317,8 @@ void
>  s3c_hsotg_core_init_disconnected(struct
>  dwc2_hsotg *hsotg,
> > GINTSTS_CONIDSTSCHNG | 

Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode

[John Youn]

On 9/10/2015 10:56 AM, Roman Bacik wrote:
>> -Original Message-
>> From: Roman Bacik
>> Sent: September-09-15 7:59 PM
>> To: 'John Youn'; Scott Branden; 'Greg Kroah-Hartman'; 'linux-
>> u...@vger.kernel.org'
>> Cc: 'linux-ker...@vger.kernel.org'; bcm-kernel-feedback-list
>> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>>
>>> -Original Message-
>>> From: Roman Bacik
>>> Sent: September-09-15 7:36 PM
>>> To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
>>> u...@vger.kernel.org
>>> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
>>> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
>>> isochronous mode
>>>
 -Original Message-
 From: John Youn [mailto:john.y...@synopsys.com]
 Sent: September-09-15 7:25 PM
 To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
 linux- u...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
 Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
 isochronous mode

 On 9/9/2015 7:16 PM, Roman Bacik wrote:
>> -Original Message-
>> From: John Youn [mailto:john.y...@synopsys.com]
>> Sent: September-09-15 7:11 PM
>> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
>> linux- u...@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
>> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
>> isochronous mode
>>
>> On 9/9/2015 11:16 AM, Roman Bacik wrote:
 -Original Message-
 From: John Youn [mailto:john.y...@synopsys.com]
 Sent: September-03-15 11:53 PM
 To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
 u...@vger.kernel.org; Roman Bacik
 Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
 Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
 isochronous mode

 On 8/31/2015 9:17 AM, Scott Branden wrote:
> From: Roman Bacik 
>
> USB OTG driver in isochronous mode has to set the parity of
> the receiving microframe. The parity is set to even by
> default. This causes problems for an audio gadget, if the host
> starts transmitting on odd
 microframes.
>
> This fix uses Incomplete Periodic Transfer interrupt to toggle
> between even and odd parity until the Transfer Complete
> interrupt is
>> received.
>
> Signed-off-by: Roman Bacik 
> Reviewed-by: Abhinav Ratna 
> Reviewed-by: Srinath Mannam
>> 
> Signed-off-by: Scott Branden 
> ---
>  drivers/usb/dwc2/core.h   |  1 +
>  drivers/usb/dwc2/gadget.c | 51
 ++-
>  drivers/usb/dwc2/hw.h |  1 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 0ed87620..a5634fd 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
>   unsigned intperiodic:1;
>   unsigned intisochronous:1;
>   unsigned intsend_zlp:1;
> + unsigned inthas_correct_parity:1;
>
>   charname[10];
>  };
> diff --git a/drivers/usb/dwc2/gadget.c
> b/drivers/usb/dwc2/gadget.c index 4d47b7c..fac3e2f 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct
> dwc2_hsotg
 *hsotg, unsigned int idx,
>   ints &= ~DXEPINT_XFERCOMPL;
>
>   if (ints & DXEPINT_XFERCOMPL) {
> + hs_ep->has_correct_parity = 1;
>   if (hs_ep->isochronous && hs_ep->interval == 1) {
>   if (ctrl & DXEPCTL_EOFRNUM)
>   ctrl |= DXEPCTL_SETEVENFR; @@ -
>> 2316,7 +2317,8 @@ void
 s3c_hsotg_core_init_disconnected(struct
 dwc2_hsotg *hsotg,
>   GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
>   GINTSTS_RESETDET | GINTSTS_ENUMDONE |
>   GINTSTS_OTGINT | GINTSTS_USBSUSP |
> - GINTSTS_WKUPINT,
> + GINTSTS_WKUPINT |
> + GINTSTS_INCOMPL_SOIN |
 GINTSTS_INCOMPL_SOOUT,
>   hsotg->regs + GINTMSK);
>
>   if (using_dma(hsotg))
> @@ -2581,6 +2583,52 @@ irq_retry:
>   s3c_hsotg_dump(hsotg);
>   }
>
> + if (gintsts & 

Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

[Duc Dang]

On Tue, Sep 1, 2015 at 4:54 AM, Mathias Nyman
 wrote:
> On 31.08.2015 21:58, Duc Dang wrote:
>>
>> On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang  wrote:
>>>
>>> The xhci platform driver needs to work on systems that
>>> either only support 64-bit DMA or only support 32-bit DMA.
>>> Attempt to set a coherent dma mask for 64-bit DMA, and
>>> attempt again with 32-bit DMA if that fails.
>>>
>>> [dhdang: regenerate the patch over 4.2-rc5 and address new comments]
>>> Signed-off-by: Mark Langsdorf 
>>> Tested-by: Mark Salter 
>>> Signed-off-by: Duc Dang 
>>>
>>> ---
>>> Changes from v6:
>>>  -Add WARN_ON if dma_mask is NULL
>>>  -Use dma_coerce_mask_and_coherent to assign
>>>  dma_mask and coherent_dma_mask
>>>
>>> Changes from v5:
>>>  -Change comment
>>>  -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
>>>  to make sure dma_set_mask_and_coherent does not fail
>>> prematurely.
>>>
>>> Changes from v4:
>>>  -None
>>>
>>> Changes from v3:
>>>  -Re-generate the patch over 4.2-rc5
>>>  -No code change.
>>>
>>> Changes from v2:
>>>  -None
>>>
>>> Changes from v1:
>>>  -Consolidated to use dma_set_mask_and_coherent
>>>  -Got rid of the check against sizeof(dma_addr_t)
>>>
>>>   drivers/usb/host/xhci-plat.c | 21 +
>>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 890ad9d..e4c7f9d 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device
>>> *pdev)
>>>  if (irq < 0)
>>>  return -ENODEV;
>>>
>>> -   /* Initialize dma_mask and coherent_dma_mask to 32-bits */
>>> -   ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
>>> -   if (ret)
>>> -   return ret;
>>> -   if (!pdev->dev.dma_mask)
>>> -   pdev->dev.dma_mask = >dev.coherent_dma_mask;
>>> -   else
>>> -   dma_set_mask(>dev, DMA_BIT_MASK(32));
>>> +   /* Throw a waring if broken platform code didn't initialize
>>> dma_mask */
>>> +   WARN_ON(!pdev->dev.dma_mask);
>>> +   /*
>>> +* Try setting dma_mask and coherent_dma_mask to 64 bits,
>>> +* then try 32 bits
>>> +*/
>>> +   ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>>> +   if (ret) {
>>> +   ret = dma_coerce_mask_and_coherent(>dev,
>>> +  DMA_BIT_MASK(32));
>>> +   if (ret)
>>> +   return ret;
>>> +   }
>>>
>>>  hcd = usb_create_hcd(driver, >dev, dev_name(>dev));
>>>  if (!hcd)
>>> --
>>> 1.9.1
>>>
>>
>> Hi Greg, Arnd, Russell,
>>
>> Do you have any more comment about this patch set?
>>
>
> I'm not sure I fully understand why we need to try the 64 bit DMA mask in
> platform probe.
>
> As I understood it we just want to have some DMA mask set before calling
> usb_create_hcd()
> to make sure USB core gets the "uses_dma" flag set and dma_set_mask() won't
> fail because of
> missing dev->dma_mask.
>
> The correct DMA mask is set later in xhci_gen_setup()
>
> We also need to make sure the controller supports 64bit addressing
> capability before setting a 64 bit DMA mask.
> (bit 0 in HCCPARAMS)
>
> So for platform devices it goes look go this:
>
> xhci_plat_probe()
>   usb_create_hcd()
> usb_create_shared_hcd()
>   hcd->self.uses_dma = (dev->dma_mask != NULL);
>   usb_add_hcd()
> hcd->driver->reset()  (.reset = xhci_plat_setup)
>   xhci_plat_setup()
> xhci_gen_setup()
>   if (HCC_64BIT_ADDR(xhci->hcc_params) && !dma_set_mask(dev,
> DMA_BIT_MASK(64))) {
> xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
> dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
>   }
>
> or do we end up with dev->dma_mask = NULL on 64-bit DMA only after trying to
> set it to 32 in xhci_plat_probe(),
> and thus also failing the dma_set_mask() in xhci_gen_setup()?
>

Hi Mathias,

dma_set_coherent_mask(>dev, DMA_BIT_MASK(32)) in xhci_plat_probe
fails on our Arm64 X-Gene platform as we do not have DMA memory below 4GB. As a
consequence, xhci_plat_probe exits very early without creating any hcd.

The xhci_gen_setup code assumes that the dma_mask and
coherent_dma_mask were already
set to DMA_BIT_MASK(32) and only check to switch to DMA_BIT_MASK(64)
if the controller
supports 64-bit DMA. So I also see possible regression if a 32-bit
controller is used on 64-bit
system with my patch.

I will integrate Russell's suggestion and add changes to address the
case of 32-bit controller on 64-bit system
in my next version of this patch set.

> -Mathias
>
>
>
>
>
>
>
>
>
>


Regards,
-- 
Duc Dang.
--
To unsubscribe from this list: send 

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

[Alan Stern]

Mathias, Bjorn, and other PCI people:

I need help with a problem affecting certain Acer computers such as the
netbook model in $SUBJECT.  More information about the system in
question is available at

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1485057

In short, the problem is that the ehci-hcd driver causes the system to
hang at various times.  Roland has done a whole bunch of debugging and
narrowed one failure mode down to a particular line of source code in
the 4.2 kernel.

In drivers/usb/host/ehci-hcd.c, the ehci_halt() function looks like 
this:

spin_lock_irq(>lock);

/* disable any irqs left enabled by previous code */
ehci_writel(ehci, 0, >regs->intr_enable);

if (ehci_is_TDI(ehci) && !tdi_in_host_mode(ehci)) {
spin_unlock_irq(>lock);
return 0;
}

/*
 * This routine gets called during probe before ehci->command
 * has been initialized, so we can't rely on its value.
 */
ehci->command &= ~CMD_RUN;
temp = ehci_readl(ehci, >regs->command);
temp &= ~(CMD_RUN | CMD_IAAD);
ehci_writel(ehci, temp, >regs->command);

spin_unlock_irq(>lock);

[ehci_writel() and ehci_readl() are wrappers around the standard
readl()/writel() or readl_be()/writel_be() routines.  The choice of
endianness may be determined at build time (by Kconfig options) or at
run time (by hardware settings).  The routines are defined in ehci.h if
you want to see the details.]

By inserting printk() lines, Roland found that the first ehci_writel()
call works okay.  The condition in the "if" statement is false, so the
early return is not taken.  The ehci_readl() causes the system to
freeze, so the second ehci_writel() never gets executed.

Oddly, this same function gets called as part of the probe sequence, 
and it works fine then.  But it hangs later on when Roland tries to 
unbind ehci-hcd from the host controller device.  One important 
difference that may be relevant is that ehci-hcd gets probed before 
xhci-hcd, whereas the unbind occurs after xhci-hcd has been probed.

I say this may be relevant because Roland found that ehci-hcd was
unable to communicate over the USB bus if it tried to do so before
xhci-hcd was probed.  This happens in the 3.17 or earlier kernel -- and
that kernel doesn't suffer the freeze problem.  This suggests there is
some unwanted interaction or interference between the two drivers.

In fact, Roland traced the problem to a single line in commit
638139eb95d2.  That line made the USB hub work queue multi-threaded
rather than single-threaded, which accounts for the difference in
probing order but otherwise seems totally unrelated.

[Roland, what happens if you try unbinding xhci-hcd before ehci-hcd?  
Note that unbinding xhci-hcd will cause your wireless keyboard & mouse
to stop working, so you'll have to use a shell script or a network
login to run the test.]

Does anyone have any idea what could cause this simple readl() call to
freeze?


On Thu, 10 Sep 2015, Roland Weber wrote:

> Hi Alan,
> 
> > The only reason I can think of why it might hang is if some clock got 
> > turned off.  But I don't know of any clock which would have that 
> > effect, or which would get turned off before we reach this point.
> > 
> > I also don't understand why the ehci_readl() would freeze when the 
> > preceding ehci_writel() succeeded.  Can you try putting a copy of the 
> > ehci_readl() line just before the ehci_writel(), to see if it will work 
> > there?
> 
> Done:
> printk(KERN_INFO "ehci_halt: about to readl prematurely\n");
> temp = ehci_readl(ehci, >regs->command);
> printk(KERN_INFO "ehci_halt: premature readl returned %x\n", temp);
> 
> /* disable any irqs left enabled by previous code */
> ehci_writel(ehci, 0, >regs->intr_enable);
> printk(KERN_INFO "ehci_halt: after first ehci_writel\n");
> 
> It works there. Result:
> premature readl returned 1

That's what it should be.

Alan Stern

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

[PATCH -next] usb: phy: fix phy-qcom-8x16-usb build

[Randy Dunlap]

From: Randy Dunlap <rdun...@infradead.org>

Fix build errors that happen when USB_QCOM_8X16_PHY=y and EXTCON=m:

drivers/built-in.o: In function `phy_8x16_init':
phy-qcom-8x16-usb.c:(.text+0x86ef4): undefined reference to 
`extcon_get_cable_state'
drivers/built-in.o: In function `phy_8x16_probe':
phy-qcom-8x16-usb.c:(.text+0x870bf): undefined reference to 
`extcon_get_edev_by_phandle'
phy-qcom-8x16-usb.c:(.text+0x87133): undefined reference to 
`extcon_register_interest'
phy-qcom-8x16-usb.c:(.text+0x87151): undefined reference to 
`extcon_unregister_interest'
drivers/built-in.o: In function `phy_8x16_remove':
phy-qcom-8x16-usb.c:(.text+0x872ec): undefined reference to 
`extcon_unregister_interest'

Signed-off-by: Randy Dunlap <rdun...@infradead.org>
Cc: Felipe Balbi <ba...@ti.com>
---
 drivers/usb/phy/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20150910.orig/drivers/usb/phy/Kconfig
+++ linux-next-20150910/drivers/usb/phy/Kconfig
@@ -155,7 +155,7 @@ config USB_MSM_OTG
 config USB_QCOM_8X16_PHY
tristate "Qualcomm APQ8016/MSM8916 on-chip USB PHY controller support"
depends on ARCH_QCOM || COMPILE_TEST
-   depends on RESET_CONTROLLER
+   depends on RESET_CONTROLLER && EXTCON
select USB_PHY
select USB_ULPI_VIEWPORT
help
--
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/1] usb: dwc2: gadget: parity fix in isochronous mode

[Roman Bacik]

> -Original Message-
> From: Roman Bacik
> Sent: September-09-15 7:59 PM
> To: 'John Youn'; Scott Branden; 'Greg Kroah-Hartman'; 'linux-
> u...@vger.kernel.org'
> Cc: 'linux-ker...@vger.kernel.org'; bcm-kernel-feedback-list
> Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> 
> > -Original Message-
> > From: Roman Bacik
> > Sent: September-09-15 7:36 PM
> > To: 'John Youn'; Scott Branden; Greg Kroah-Hartman; linux-
> > u...@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> > Subject: RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> > isochronous mode
> >
> > > -Original Message-
> > > From: John Youn [mailto:john.y...@synopsys.com]
> > > Sent: September-09-15 7:25 PM
> > > To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> > > linux- u...@vger.kernel.org
> > > Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> > > Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> > > isochronous mode
> > >
> > > On 9/9/2015 7:16 PM, Roman Bacik wrote:
> > > >> -Original Message-
> > > >> From: John Youn [mailto:john.y...@synopsys.com]
> > > >> Sent: September-09-15 7:11 PM
> > > >> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman;
> > > >> linux- u...@vger.kernel.org
> > > >> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> > > >> Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> > > >> isochronous mode
> > > >>
> > > >> On 9/9/2015 11:16 AM, Roman Bacik wrote:
> > >  -Original Message-
> > >  From: John Youn [mailto:john.y...@synopsys.com]
> > >  Sent: September-03-15 11:53 PM
> > >  To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> > >  u...@vger.kernel.org; Roman Bacik
> > >  Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> > >  Subject: Re: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in
> > >  isochronous mode
> > > 
> > >  On 8/31/2015 9:17 AM, Scott Branden wrote:
> > > > From: Roman Bacik 
> > > >
> > > > USB OTG driver in isochronous mode has to set the parity of
> > > > the receiving microframe. The parity is set to even by
> > > > default. This causes problems for an audio gadget, if the host
> > > > starts transmitting on odd
> > >  microframes.
> > > >
> > > > This fix uses Incomplete Periodic Transfer interrupt to toggle
> > > > between even and odd parity until the Transfer Complete
> > > > interrupt is
> > > >> received.
> > > >
> > > > Signed-off-by: Roman Bacik 
> > > > Reviewed-by: Abhinav Ratna 
> > > > Reviewed-by: Srinath Mannam
> 
> > > > Signed-off-by: Scott Branden 
> > > > ---
> > > >  drivers/usb/dwc2/core.h   |  1 +
> > > >  drivers/usb/dwc2/gadget.c | 51
> > >  ++-
> > > >  drivers/usb/dwc2/hw.h |  1 +
> > > >  3 files changed, 52 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> > > > index 0ed87620..a5634fd 100644
> > > > --- a/drivers/usb/dwc2/core.h
> > > > +++ b/drivers/usb/dwc2/core.h
> > > > @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
> > > > unsigned intperiodic:1;
> > > > unsigned intisochronous:1;
> > > > unsigned intsend_zlp:1;
> > > > +   unsigned inthas_correct_parity:1;
> > > >
> > > > charname[10];
> > > >  };
> > > > diff --git a/drivers/usb/dwc2/gadget.c
> > > > b/drivers/usb/dwc2/gadget.c index 4d47b7c..fac3e2f 100644
> > > > --- a/drivers/usb/dwc2/gadget.c
> > > > +++ b/drivers/usb/dwc2/gadget.c
> > > > @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct
> > > > dwc2_hsotg
> > >  *hsotg, unsigned int idx,
> > > > ints &= ~DXEPINT_XFERCOMPL;
> > > >
> > > > if (ints & DXEPINT_XFERCOMPL) {
> > > > +   hs_ep->has_correct_parity = 1;
> > > > if (hs_ep->isochronous && hs_ep->interval == 1) {
> > > > if (ctrl & DXEPCTL_EOFRNUM)
> > > > ctrl |= DXEPCTL_SETEVENFR; @@ -
> 2316,7 +2317,8 @@ void
> > > s3c_hsotg_core_init_disconnected(struct
> > >  dwc2_hsotg *hsotg,
> > > > GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
> > > > GINTSTS_RESETDET | GINTSTS_ENUMDONE |
> > > > GINTSTS_OTGINT | GINTSTS_USBSUSP |
> > > > -   GINTSTS_WKUPINT,
> > > > +   GINTSTS_WKUPINT |
> > > > +   GINTSTS_INCOMPL_SOIN |
> > > GINTSTS_INCOMPL_SOOUT,
> > > > hsotg->regs + GINTMSK);
> > > >
> > > > if (using_dma(hsotg))
> > > > @@ -2581,6 +2583,52 @@ irq_retry:
> > > >