Re: [4.4-rc1 regression] pxa27x_udc and suspend/resume

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

 



Hi,

Robert Jarzmik <robert.jarzmik@xxxxxxx> writes:
> Robert Jarzmik <robert.jarzmik@xxxxxxx> writes:
>
>> Felipe Balbi <balbi@xxxxxx> writes:
>>
>>> Hi,
>>>
>>> without any sort of logs it a bit difficult :-) Care to send some output
>>> of the failure ? Are there any oopses or what exactly happens ?
>> Ah well, the UDC is the only way to "speak" to the board (no UART), so I don't
>> have any written feedback available. All I have are the logs displayed on the
>> phone's screen in the framebuffer screen.
>
> I can have logs if I kinda ... revert the patch, applying the diff in [1].
> This enables the suspend/resume to work again, and I can gather the logs when
> [1] is applied :
>
> [   63.649528] Suspending console(s) (use no_console_suspend to debug)
> [   63.688100] PM: suspend of devices complete after 37.275 msecs
> [   63.690351] PM: late suspend of devices complete after 2.191 msecs
> [   63.692595] PM: noirq suspend of devices complete after 2.196 msecs
> [   63.694387] PM: noirq resume of devices complete after 1.482 msecs
> [   63.696711] PM: early resume of devices complete after 1.533 msecs
> [   63.802930] PM: resume of devices complete after 106.122 msecs
> [   63.804976] Restarting tasks ... 
> [   63.821371] pxa27x-udc pxa27x-udc: USB reset
> [   63.822988] done.
> [   63.933666] pxa27x-udc pxa27x-udc: USB reset
> [   64.064026] g_ether gadget: full-speed config #1: CDC Subset/SAFE
> [   64.069692] pxa27x-udc pxa27x-udc: ep3:pxa_ep_enable:usb_ep ep1out-bulk already enabled, doing nothing

this could be a bug in either g_ether or pxa27x... Seems like something
is enabling endpoints which were already enabled. Not sure if this is
pxa27x not _really_ disabling endpoints or g_ether being stupid.

> [1] Stupid diff to renable the suspend/resume to work on 4.4-rc1
> rj@belgarion:~/mio_linux/kernel$ git diff
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 3d583a10b926..2b8dcf546d03 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -267,8 +267,8 @@ static inline int usb_ep_enable(struct usb_ep *ep)
>  {
>         int ret;
>  
> -       if (ep->enabled)
> -               return 0;
> +       /* if (ep->enabled) */
> +       /*      return 0; */
>  
>         ret = ep->ops->enable(ep, ep->desc);
>         if (ret)
> @@ -295,8 +295,8 @@ static inline int usb_ep_disable(struct usb_ep *ep)
>  {
>         int ret;
>  
> -       if (!ep->enabled)
> -               return 0;
> +       /* if (!ep->enabled) */
> +       /*      return 0; */

yeah, so something is not disabling endpoints when they should :-) So
this could be a bug with your suspend/resume callbacks. If you're going
to disconnect from the bus, you need to tell the gadget driver about it,
which means after disabling pullups, you should call
gadget_driver->disconnect().

Can you see if this *stupid* and *untested* diff helps :

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

I'm not 100% sure this is enough, as I'm not at all familiar with
pxa27x, but that driver looks hugely unmaintained. Which device are you
using for development/test ? Is it this Mio A701 ? I can't find any
sources of it :-s

-- 
balbi

Attachment: signature.asc
Description: PGP signature


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

  Powered by Linux