Re: [PATCH 1/2] usb: chipidea: add flag CI_HDRC_DP_ALWAYS_PULLUP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 18, 2016 at 02:36:06PM +0530, maitysanchayan@xxxxxxxxx wrote:
> Hello Peter,
> 
> I tested this on a Colibri Vybrid VF61 module with the patches applied
> on top of for-next branch.
> 
> root@colibri-vf:~# uname -a
> Linux colibri-vf 4.6.0-rc1-00044-gc76529e-dirty #120 Mon Apr 18 13:46:34 IST 2016 armv7l GNU/Linux
> 
> Host and client mode work only on boot up. Trying to change the mode by disconnecting/reconnecting
> the cable or plugging in a USB device does not work. Also when the USB client mode is working on
> boot up, disconnecting the cable results in the below stack trace.
> 
> root@colibri-vf:~# ping 192.168.11.234                                                                                    
> PING 192.168.11.234 (192.168.11.234): 56 data bytes
> 64 bytes from 192.168.11.234: seq=0 ttl=64 time=5.399 ms
> ^C
> --- 192.168.11.234 ping statistics ---
> 1 packets transmitted, 1 packets received, 0% packet loss
> round-trip min/avg/max = 5.399/5.399/5.399 ms
> --------------------- On Cable disconnection-------------------------
> root@colibri-vf:~# [   23.923607] irq 35: nobody cared (try booting with the "irqpoll" option)
> [   23.930354] CPU: 0 PID: 0 Comm: swapper Not tainted 4.6.0-rc1-00044-gc76529e-dirty #120
> [   23.938359] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> [   23.944807] Backtrace: 
> [   23.947327] [<8010b56c>] (dump_backtrace) from [<8010b764>] (show_stack+0x18/0x1c)
> [   23.954900]  r7:00000023 r6:00000000 r5:00000000 r4:8f4ad240
> [   23.960644] [<8010b74c>] (show_stack) from [<803a1c00>] (dump_stack+0x24/0x28)
> [   23.967893] [<803a1bdc>] (dump_stack) from [<80148150>] (__report_bad_irq+0x30/0xb8)
> [   23.975655] [<80148120>] (__report_bad_irq) from [<801484a8>] (note_interrupt+0x260/0x2b0)
> [   23.983918]  r5:00000000 r4:8f4ad240
> [   23.987538] [<80148248>] (note_interrupt) from [<801460cc>] (handle_irq_event_percpu+0xe0/0x154)
> [   23.996328]  r10:00000000 r9:80b35683 r8:8f4ad240 r7:00000023 r6:00000000 r5:00000000
> [   24.004242]  r4:00000000 r3:00000000
> [   24.007860] [<80145fec>] (handle_irq_event_percpu) from [<80146170>] (handle_irq_event+0x30/0x44)
> [   24.016732]  r10:80b0210c r9:90003100 r8:8f406000 r7:80b01f10 r6:00000000 r5:80b14ed0
> [   24.024646]  r4:8f4ad240
> [   24.027206] [<80146140>] (handle_irq_event) from [<80148d2c>] (handle_fasteoi_irq+0xa8/0x170)
> [   24.035737]  r5:80b14ed0 r4:8f4ad240
> [   24.039363] [<80148c84>] (handle_fasteoi_irq) from [<801457a0>] (generic_handle_irq+0x2c/0x3c)
> [   24.047973]  r7:80b01f10 r6:00000000 r5:00000023 r4:80b14d8c
> [   24.053706] [<80145774>] (generic_handle_irq) from [<80145a30>] (__handle_domain_irq+0x5c/0xb0)
> [   24.062420] [<801459d4>] (__handle_domain_irq) from [<801013d0>] (gic_handle_irq+0x50/0x84)
> [   24.070771]  r9:90003100 r8:80b01e00 r7:90002100 r6:9000210c r5:80b023b0 r4:80b14ec0
> [   24.078611] [<80101380>] (gic_handle_irq) from [<8010c214>] (__irq_svc+0x54/0x70)
> [   24.086101] Exception stack(0x80b01e00 to 0x80b01e48)
> [   24.091173] 1e00: 80b360c0 00200000 80b36080 00000000 00000002 00000010 00000000 00000000
> [   24.099363] 1e20: 8f406000 90003100 80b0210c 80b01eac 00200000 80b01e50 8011cf18 8011caec
> [   24.107545] 1e40: 600c0113 ffffffff
> [   24.111039]  r9:90003100 r8:8f406000 r7:80b01e34 r6:ffffffff r5:600c0113 r4:8011caec
> [   24.118892] [<8011ca4c>] (__do_softirq) from [<8011cf18>] (irq_exit+0xb8/0xf4)
> [   24.126114]  r10:80b0210c r9:90003100 r8:8f406000 r7:00000000 r6:00000000 r5:00000010
> [   24.134028]  r4:80b14d8c
> [   24.136587] [<8011ce60>] (irq_exit) from [<80145a34>] (__handle_domain_irq+0x60/0xb0)
> [   24.144429] [<801459d4>] (__handle_domain_irq) from [<801013d0>] (gic_handle_irq+0x50/0x84)
> [   24.152782]  r9:90003100 r8:80b01f10 r7:90002100 r6:9000210c r5:80b023b0 r4:80b14ec0
> [   24.160621] [<80101380>] (gic_handle_irq) from [<8010c214>] (__irq_svc+0x54/0x70)
> [   24.168110] Exception stack(0x80b01f10 to 0x80b01f58)
> [   24.173175] 1f00:                                     00000001 00000000 00000000 80115ac0
> [   24.181365] 1f20: 00000000 80b0210c 00000000 80b29c78 80b02114 80b00000 80b0210c 80b01f6c
> [   24.189554] 1f40: 80b01f70 80b01f60 80108200 80108204 600c0013 ffffffff
> [   24.196173]  r9:80b00000 r8:80b02114 r7:80b01f44 r6:ffffffff r5:600c0013 r4:80108204
> [   24.204026] [<801081c4>] (arch_cpu_idle) from [<8013dea8>] (default_idle_call+0x28/0x34)
> [   24.212134] [<8013de80>] (default_idle_call) from [<8013e018>] (cpu_startup_entry+0x164/0x1c0)
> [   24.220776] [<8013deb4>] (cpu_startup_entry) from [<806fa2a4>] (rest_init+0x78/0x7c)
> [   24.228518]  r7:ffffffff
> [   24.231097] [<806fa22c>] (rest_init) from [<80a00ce0>] (start_kernel+0x370/0x37c)
> [   24.238599] [<80a00970>] (start_kernel) from [<80008078>] (0x80008078)
> [   24.245127] handlers:
> [   24.247415] [<804f2dcc>] ci_irq
> [   24.250578] Disabling IRQ #35
> 
> Below is a diff of the changes to the kernel after applying these two patches
> 
> diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> index a8a8e43..e85b534 100644
> --- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> @@ -50,6 +50,14 @@
>                 clock-frequency = <16000000>;
>         };
>  
> +       extcon_usbc_det: usbc_det {
> +               compatible = "linux,extcon-usb-gpio";
> +               debounce = <25>;
> +               id-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_usbc_det>;
> +       };
> +
>         panel: panel {
>                 compatible = "edt,et057090dhu";
>                 backlight = <&bl>;
> @@ -162,6 +170,10 @@
>         status = "okay";
>  };
>  
> +&usbdev0 {
> +       extcon = <&extcon_usbc_det>, <&extcon_usbc_det>;
> +};
> +

Would you please show me your board design for vbus and id control?
And what's the default value for both signal?

In your dts, both signals use the same pin. In the most of the designs,
they are two different pins.
Below are content from the chipidea binding doc:

- extcon: phandles to external connector devices. First phandle should point to
  external connector, which provide "USB" cable events, the second should point
  to external connector device, which provide "USB-HOST" cable events. If one
  of the external connector devices is not required, empty <0> phandle should
  be specified.

>  &usbh1 {
>         vbus-supply = <&reg_usbh_vbus>;
>  };
> diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
> index b741709..fd8aa33 100644
> --- a/arch/arm/boot/dts/vf-colibri.dtsi
> +++ b/arch/arm/boot/dts/vf-colibri.dtsi
> @@ -174,6 +174,7 @@
>  
>  &usbdev0 {
>         disable-over-current;
> +       dr_mode = "otg";
>         status = "okay";
>  };
>  
> @@ -362,6 +363,12 @@
>                         >;
>                 };
>  
> +               pinctrl_usbc_det: gpio_usbc_det {
> +                       fsl,pins = <
> +                               VF610_PAD_PTC29__GPIO_102               0x22ed
> +                       >;
> +               };
> +
>                 pinctrl_usbh1_reg: gpio_usb_vbus {
>                         fsl,pins = <
>                                 VF610_PAD_PTD4__GPIO_83                 0x22ed
> diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> index 2c13ec6..2470761 100644
> --- a/arch/arm/boot/dts/vfxxx.dtsi
> +++ b/arch/arm/boot/dts/vfxxx.dtsi
> @@ -472,7 +472,7 @@
>                         };
>  
>                         usbdev0: usb@40034000 {
> -                               compatible = "fsl,vf610-usb", "fsl,imx27-usb";
> +                               compatible = "fsl,vf610-usb";
>                                 reg = <0x40034000 0x800>;
>                                 interrupts = <75 IRQ_TYPE_LEVEL_HIGH>;
>                                 clocks = <&clks VF610_CLK_USBC0>;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 9ce8c9f..9cf91c6 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -70,6 +70,10 @@ static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
>         .flags = CI_HDRC_SUPPORTS_RUNTIME_PM,
>  };
>  
> +static const struct ci_hdrc_imx_platform_flag vf610_usb_data = {
> +       .flags = CI_HDRC_DP_ALWAYS_PULLUP,
> +};
> +

This flag is used for the system which doesn't need to know connection and
disconnection event, is it your case?

Peter

>  static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
>         { .compatible = "fsl,imx23-usb", .data = &imx23_usb_data},
>         { .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
> @@ -79,6 +83,7 @@ static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
>         { .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
>         { .compatible = "fsl,imx6ul-usb", .data = &imx6ul_usb_data},
>         { .compatible = "fsl,imx7d-usb", .data = &imx7d_usb_data},
> +       { .compatible = "fsl,vf610-usb", .data = &vf610_usb_data},
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
> 
> Am I missing something for this patchset to work?
> 
> Regards,
> Sanchayan.
> 
> On 16-04-15 12:44:00, Peter Chen wrote:
> > For some platforms, the vbus status doesn't be known by system at
> > device mode. So, there is no way to trigger pulling dp up. We add
> > a platform flag CI_HDRC_DP_ALWAYS_PULLUP to cover this situation.
> > With this flag set, the dp will be pulled up during the driver probe.
> > 
> > Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
> > Cc: Svetoslav Neykov <svetoslav@xxxxxxxxxxx>
> > ---
> >  drivers/usb/chipidea/ci.h    | 2 ++
> >  drivers/usb/chipidea/core.c  | 6 ++++--
> >  drivers/usb/chipidea/otg.c   | 4 +++-
> >  include/linux/usb/chipidea.h | 1 +
> >  4 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index cd41455..f81dd3b 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -199,6 +199,7 @@ struct hw_bank {
> >   * @in_lpm: if the core in low power mode
> >   * @wakeup_int: if wakeup interrupt occur
> >   * @rev: The revision number for controller
> > + * @dp_always_pullup: keep dp always pullup at device mode
> >   */
> >  struct ci_hdrc {
> >  	struct device			*dev;
> > @@ -248,6 +249,7 @@ struct ci_hdrc {
> >  	bool				in_lpm;
> >  	bool				wakeup_int;
> >  	enum ci_revision		rev;
> > +	bool				dp_always_pullup;
> >  };
> >  
> >  static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 1ca7b04..6f22c6c 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -897,7 +897,7 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
> >  {
> >  	ci_hdrc_gadget_destroy(ci);
> >  	ci_hdrc_host_destroy(ci);
> > -	if (ci->is_otg)
> > +	if (!ci->dp_always_pullup && ci->roles[CI_ROLE_GADGET])
> >  		ci_hdrc_otg_destroy(ci);
> >  }
> >  
> > @@ -946,6 +946,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >  		CI_HDRC_IMX28_WRITE_FIX);
> >  	ci->supports_runtime_pm = !!(ci->platdata->flags &
> >  		CI_HDRC_SUPPORTS_RUNTIME_PM);
> > +	ci->dp_always_pullup = !!(ci->platdata->flags &
> > +		CI_HDRC_DP_ALWAYS_PULLUP);
> >  
> >  	ret = hw_device_init(ci, base);
> >  	if (ret < 0) {
> > @@ -1012,7 +1014,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >  		goto deinit_phy;
> >  	}
> >  
> > -	if (ci->is_otg && ci->roles[CI_ROLE_GADGET]) {
> > +	if (!ci->dp_always_pullup && ci->roles[CI_ROLE_GADGET]) {
> >  		ret = ci_hdrc_otg_init(ci);
> >  		if (ret) {
> >  			dev_err(dev, "init otg fails, ret = %d\n", ret);
> > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > index 03b6743..58be59e 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -95,8 +95,10 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
> >  
> >  void ci_handle_vbus_change(struct ci_hdrc *ci)
> >  {
> > -	if (!ci->is_otg)
> > +	if (ci->dp_always_pullup) {
> > +		usb_gadget_vbus_connect(&ci->gadget);
> >  		return;
> > +	}
> >  
> >  	if (hw_read_otgsc(ci, OTGSC_BSV))
> >  		usb_gadget_vbus_connect(&ci->gadget);
> > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> > index 8649930..d21fef9 100644
> > --- a/include/linux/usb/chipidea.h
> > +++ b/include/linux/usb/chipidea.h
> > @@ -55,6 +55,7 @@ struct ci_hdrc_platform_data {
> >  #define CI_HDRC_OVERRIDE_AHB_BURST	BIT(9)
> >  #define CI_HDRC_OVERRIDE_TX_BURST	BIT(10)
> >  #define CI_HDRC_OVERRIDE_RX_BURST	BIT(11)
> > +#define CI_HDRC_DP_ALWAYS_PULLUP	BIT(12)
> >  	enum usb_dr_mode	dr_mode;
> >  #define CI_HDRC_CONTROLLER_RESET_EVENT		0
> >  #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
> > -- 
> > 1.9.1
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux