diff mbox

[v12,00/13] Add tested id switch and vbus connect detect support for Chipidea

Message ID 201307111957.19537.marex@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Vasut July 11, 2013, 5:57 p.m. UTC
Hi Peter,

> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
> 
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> 
> Changes for v12:
> - Rebased greg's usb-next tree (3.10.0-rc7+)
> - Split more small patches for single function and fix.

I tested the patchset. Here are the results:

- VBUS switching

I'm no longer getting any ID interrupts at all when I apply the patch below. The 
board stays in HOST mode all the time. If I configure it as peripheral, it works 
as peripheral. Note with [1], I was able to switch from Peripheral->Host , not 
the other way around.


[1] 
http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/v3.10/topic/usb-
peterchen

Best regards,
Marek Vasut

Comments

Peter Chen July 12, 2013, 3:12 a.m. UTC | #1
On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > This patchset adds tested otg id switch function and
> > vbus connect and disconnect detection for chipidea driver.
> > And fix kinds of bugs found at chipidea drivers after enabling
> > id and vbus detection.
> > 
> > This patch is fully tested at imx6 sabresd platform.
> > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > 
> > Changes for v12:
> > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > - Split more small patches for single function and fix.
> 
> I tested the patchset. Here are the results:
> 
> - VBUS switching
> 
> I'm no longer getting any ID interrupts at all when I apply the patch below. The 
> board stays in HOST mode all the time. If I configure it as peripheral, it works 
> as peripheral. Note with [1], I was able to switch from Peripheral->Host , not 
> the other way around.

Thanks for your testing. But first, can you have me check
if your ID wakeup is enabled?

I can have a test at mx28evk.
Is it current upstream kernel can boot mx28evk run?
I have a RevC board, I would like to know if
any patches needed.
> 
> --- a/arch/arm/boot/dts/imx28-m28evk.dts
> +++ b/arch/arm/boot/dts/imx28-m28evk.dts
> @@ -240,6 +240,8 @@
>  
>         ahb@80080000 {
>                 usb0: usb@80080000 {
> +                       dr_mode = "otg";
> +                       phy_mode = "utmi";
>                         vbus-supply = <&reg_usb0_vbus>;
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&usbphy0_pins_a>;
> 
> ---------------------------
> 
> - MX23 UDC issue
> 
> I found a workaround. Now running 'dmesg' via telnet through USB CDC link no 
> longer hangs the USB driver, but works as expected. I applied this small patch 
> that enables the streaming mode. Works on MX23EVK. It's surprising this issue 
> doesn't manifest on MX28, maybe MX28 contains a new revision of the controller. 
> I remember there was some discussion about the streaming mode on MXS some time 
> ago.

It seems not reasonable, the same question for mx23evk,
it can run well with current kernel? I have no board on hand,
let me see if I can find one.
Marek Vasut July 12, 2013, 4:04 a.m. UTC | #2
Hi Peter,

> On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > This patchset adds tested otg id switch function and
> > > vbus connect and disconnect detection for chipidea driver.
> > > And fix kinds of bugs found at chipidea drivers after enabling
> > > id and vbus detection.
> > > 
> > > This patch is fully tested at imx6 sabresd platform.
> > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > 
> > > Changes for v12:
> > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > - Split more small patches for single function and fix.
> > 
> > I tested the patchset. Here are the results:
> > 
> > - VBUS switching
> > 
> > I'm no longer getting any ID interrupts at all when I apply the patch
> > below. The board stays in HOST mode all the time. If I configure it as
> > peripheral, it works as peripheral. Note with [1], I was able to switch
> > from Peripheral->Host , not the other way around.
> 
> Thanks for your testing. But first, can you have me check
> if your ID wakeup is enabled?

ID wakeup? How do I check?

> I can have a test at mx28evk.
> Is it current upstream kernel can boot mx28evk run?

Yes, I use next-20130711 and it works fine.

> I have a RevC board, I would like to know if
> any patches needed.

Apply just this patchset and you should be all set.

> > --- a/arch/arm/boot/dts/imx28-m28evk.dts
> > +++ b/arch/arm/boot/dts/imx28-m28evk.dts
> > @@ -240,6 +240,8 @@
> > 
> >         ahb@80080000 {
> >         
> >                 usb0: usb@80080000 {
> > 
> > +                       dr_mode = "otg";
> > +                       phy_mode = "utmi";
> > 
> >                         vbus-supply = <&reg_usb0_vbus>;
> >                         pinctrl-names = "default";
> >                         pinctrl-0 = <&usbphy0_pins_a>;
> > 
> > ---------------------------
> > 
> > - MX23 UDC issue
> > 
> > I found a workaround. Now running 'dmesg' via telnet through USB CDC link
> > no longer hangs the USB driver, but works as expected. I applied this
> > small patch that enables the streaming mode. Works on MX23EVK. It's
> > surprising this issue doesn't manifest on MX28, maybe MX28 contains a
> > new revision of the controller. I remember there was some discussion
> > about the streaming mode on MXS some time ago.
> 
> It seems not reasonable

I talked to Fabio and he said he met similar issue on MX6, where disabling the 
streaming mode fixed the problem. Could it possibly be so that the non-streaming 
mode is broken on mx23?

btw. I also tested this on STMP3780-based device, works the same way as MX23 
does, streaming mode has to be enabled.

> the same question for mx23evk,
> it can run well with current kernel? I have no board on hand,
> let me see if I can find one.

Yes, MX23EVK rev. B2 works just fine with next-20130711 . You need to add the DT 
USB sections for MX23EVK, but they can be copied from imx28.dtsi + imx28-evk.dts 
.

Best regards,
Marek Vasut
Peter Chen July 12, 2013, 8:26 a.m. UTC | #3
On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > Hi Peter,
> > > 
> > > > This patchset adds tested otg id switch function and
> > > > vbus connect and disconnect detection for chipidea driver.
> > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > id and vbus detection.
> > > > 
> > > > This patch is fully tested at imx6 sabresd platform.
> > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > 
> > > > Changes for v12:
> > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > - Split more small patches for single function and fix.
> > > 
> > > I tested the patchset. Here are the results:
> > > 
> > > - VBUS switching
> > > 
> > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > below. The board stays in HOST mode all the time. If I configure it as
> > > peripheral, it works as peripheral. Note with [1], I was able to switch
> > > from Peripheral->Host , not the other way around.
> > 
> > Thanks for your testing. But first, can you have me check
> > if your ID wakeup is enabled?
> 
> ID wakeup? How do I check?
> 

See otgsc at controller register, the ID wakeup enable is bit 24.
Marek Vasut July 12, 2013, 1:18 p.m. UTC | #4
Hi Peter,

> On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > This patchset adds tested otg id switch function and
> > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > id and vbus detection.
> > > > > 
> > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > 
> > > > > Changes for v12:
> > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > - Split more small patches for single function and fix.
> > > > 
> > > > I tested the patchset. Here are the results:
> > > > 
> > > > - VBUS switching
> > > > 
> > > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > > below. The board stays in HOST mode all the time. If I configure it
> > > > as peripheral, it works as peripheral. Note with [1], I was able to
> > > > switch from Peripheral->Host , not the other way around.
> > > 
> > > Thanks for your testing. But first, can you have me check
> > > if your ID wakeup is enabled?
> > 
> > ID wakeup? How do I check?
> 
> See otgsc at controller register, the ID wakeup enable is bit 24.

Yes, ID interrupt (IDIE) is set.

I noticed this backtrace in the kernel bootlog, but this only happens if the 
dr_mode="otg" , it comes from the host-mode irq handler :

[    2.757563] irq 238: nobody cared (try booting with the "irqpoll" option)
[    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
next-20130711-00013-g011c4b3-dirty #703
[    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>] 
(show_stack+0x10/0x14)
[    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>] 
(__report_bad_irq.isra.6+0x20/0xe0)
[    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from 
[<80065c98>] (note_interrupt+0x16c/0x230)
[    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>] 
(handle_irq_event_percpu+0x10c/0x1a4)
[    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from 
[<800640e8>] (handle_irq_event+0x50/0x78)
[    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>] 
(handle_level_irq+0x88/0x10c)
[    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from [<800638d0>] 
(generic_handle_irq+0x28/0x3c)
[    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from [<8000f84c>] 
(handle_IRQ+0x30/0x84)
[    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>] 
(__do_softirq+0x90/0x1d8)
[    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>] 
(irq_exit+0x98/0xd4)
[    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>] 
(handle_IRQ+0x34/0x84)
[    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>] 
(vprintk_emit+0x1bc/0x524)
[    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>] 
(printk+0x30/0x40)
[    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>] 
(mousedev_init+0x4c/0x60)
[    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>] 
(do_one_initcall+0x94/0x14c)
[    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>] 
(kernel_init_freeable+0x16c/0x22c)
[    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from [<804d8cbc>] 
(kernel_init+0x8/0x150)
[    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>] 
(ret_from_fork+0x14/0x24)
[    2.949932] handlers:
[    2.952227] [<8033fc58>] ci_irq
[    2.955388] Disabling IRQ #238

btw. do you have any kind of other CI13xxx documentation than what's in the CPU 
datasheets ?

Best regards,
Marek Vasut
Chen Peter-B29397 July 13, 2013, 12:36 a.m. UTC | #5
> 
> Yes, ID interrupt (IDIE) is set.
> 
> I noticed this backtrace in the kernel bootlog, but this only happens if
> the
> dr_mode="otg" , it comes from the host-mode irq handler :
> 
> [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> option)
> [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> next-20130711-00013-g011c4b3-dirty #703
> [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>]
> (show_stack+0x10/0x14)
> [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> (__report_bad_irq.isra.6+0x20/0xe0)
> [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> [<80065c98>] (note_interrupt+0x16c/0x230)
> [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>]
> (handle_irq_event_percpu+0x10c/0x1a4)
> [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from
> [<800640e8>] (handle_irq_event+0x50/0x78)
> [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>]
> (handle_level_irq+0x88/0x10c)
> [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> [<800638d0>]
> (generic_handle_irq+0x28/0x3c)
> [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> [<8000f84c>]
> (handle_IRQ+0x30/0x84)
> [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> (__irq_svc+0x40/0x6c)
> [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> (__do_softirq+0x90/0x1d8)
> [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>]
> (irq_exit+0x98/0xd4)
> [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> (handle_IRQ+0x34/0x84)
> [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> (__irq_svc+0x40/0x6c)
> [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> (vprintk_emit+0x1bc/0x524)
> [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>]
> (printk+0x30/0x40)
> [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> (mousedev_init+0x4c/0x60)
> [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>]
> (do_one_initcall+0x94/0x14c)
> [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>]
> (kernel_init_freeable+0x16c/0x22c)
> [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> [<804d8cbc>]
> (kernel_init+0x8/0x150)
> [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> (ret_from_fork+0x14/0x24)
> [    2.949932] handlers:
> [    2.952227] [<8033fc58>] ci_irq
> [    2.955388] Disabling IRQ #238
> 

See this " Disabling IRQ #238", that is the reason you can't get
ID interrupt.

I have no other idea before I try, you can try below things:

1. please switch VDD 5V SOURCE SELECT to USB 5V
2. try not plug in mouse during the boots up

> btw. do you have any kind of other CI13xxx documentation than what's in
> the CPU
> datasheets ?
> 

Mx28 RM does not include many controller detail, please try to 
read it from mx6/mx5 RM.

Best regards,
Peter
diff mbox

Patch

--- a/arch/arm/boot/dts/imx28-m28evk.dts
+++ b/arch/arm/boot/dts/imx28-m28evk.dts
@@ -240,6 +240,8 @@ 
 
        ahb@80080000 {
                usb0: usb@80080000 {
+                       dr_mode = "otg";
+                       phy_mode = "utmi";
                        vbus-supply = <&reg_usb0_vbus>;
                        pinctrl-names = "default";
                        pinctrl-0 = <&usbphy0_pins_a>;

---------------------------

- MX23 UDC issue

I found a workaround. Now running 'dmesg' via telnet through USB CDC link no 
longer hangs the USB driver, but works as expected. I applied this small patch 
that enables the streaming mode. Works on MX23EVK. It's surprising this issue 
doesn't manifest on MX28, maybe MX28 contains a new revision of the controller. 
I remember there was some discussion about the streaming mode on MXS some time 
ago.

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index d06355e..bba8e25 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -92,8 +92,7 @@  static int ci_hdrc_imx_probe(struct platform_device *pdev)
                .name           = "ci_hdrc_imx",
                .capoffset      = DEF_CAPOFFSET,
                .flags          = CI_HDRC_REQUIRE_TRANSCEIVER |
-                                 CI_HDRC_PULLUP_ON_VBUS |
-                                 CI_HDRC_DISABLE_STREAMING,
+                                 CI_HDRC_PULLUP_ON_VBUS,
        };
        struct resource *res;
        int ret;