Re: [PATCH v3 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode

[Sumit Semwal]

Hi Peter,

On 8 June 2015 at 14:02, Peter Chen <peter.chen@xxxxxxxxxxxxx> wrote:
> On Mon, Jun 08, 2015 at 12:37:00PM +0530, Sumit Semwal wrote:
>> H Peter, Felipe,
>>
>> On 14 January 2015 at 06:56, Peter Chen <peter.chen@xxxxxxxxxxxxx> wrote:
>> > On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote:
>> >> On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote:
>> >> > Hi Felipe,
>> >> >
>> >> > On 1 December 2014 at 11:09, Kiran Raparthy <kiran.kumar@xxxxxxxxxx> wrote:
>> >> > > Hi Felipe,
>> >> > >
>> >> > > On 25 November 2014 at 20:15, Felipe Balbi <balbi@xxxxxx> wrote:
>> >> > >> On Tue, Nov 25, 2014 at 07:06:18AM +0000, Peter Chen wrote:
>> >> > >>>
>> >> > >>> >
>> >> > >>> > usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
>> >> > >>> >
>> >> > >>> > Some systems require a mechanism to prevent system to enter into suspend
>> >> > >>> > state when USB is connected and enumerated in peripheral mode.
>> >> > >>> >
>> >> > >>> > This patch provides an interface to hold a wakeupsource to prevent suspend.
>> >> > >>> > PHY drivers can use this interface when USB is connected and enumerated in
>> >> > >>> > peripheral mode.
>> >> > >>> >
>> >> > >>> > A timed wakeupsource is temporarily held on USB disconnect events, to allow
>> >> > >>> > the rest of the system to react to the USB disconnection (dropping host
>> >> > >>> > sessions, updating charger status, etc.) prior to re-allowing suspend.
>> >> > >>> >
>> >> > >>>
<snip>
>>
>> Kiran is unable to continue on this patch anymore, so despite my very
>> limited usb experience, I would like to take this over and with your
>> help, take it to closure.
>>
>> From what I understand, it might be possible to do this either at the
>> usb controller driver (where it detects if it has enumerated in host
>> mode or device mode) or in the extcon driver (where it detects VBUS
>> etc..) -  which would be a cleaner / more logical way to do it?
>>
>> Thanks for all your help!
>
> I added my suggestions below:
>
> My opinion is: if we need this feature, we should depend on gadget's
> state, eg 'configured', not vbus level. The system suspend should be
> allowed if the gadget state is 'suspended' even vbus is still there.
>
> And when the gadget goes to suspend or reset or disconnect state, we can
> go to disable this wakeup source.
>
> At drivers/usb/gadget/composite.c, we can know the gadget's state.
>
Thanks very much for your suggestion, and apologies in the delay in response.

I tried to study your suggestion and the code a little bit more, and
while I liked your idea quite a bit, I also realised that for the case
of letting the USB charger use cases, in gadget framework, there
doesn't seem to be a way to get the USB_EVENT_CHARGER, since charger
would likely not be a gadget.

In this case, how'd we be able to enable / disable wakeup_source for
charger events?

If indeed we can't handle charger events in the way suggested by you,
may I propose the following:

- implement this in USB-phy as proposed in Kiran's original patch,
- keep it disabled by default, and add a module param to enable/disable it
- for no vbus case, use another event (USB_EVENT_ENUMERATED /
USB_EVENT_NONE? ) to identify connect / disconnect?
     (OR, maybe don't do anything for such cases, since most likely
the chipidea kind of implementations would handle it themselves?)

Of course these are based on my limited reading of the USB code, so
I'd be grateful if you'd let me know of any obvious gaps in my
understanding!

> --
>
> Best Regards,
> Peter Chen


Best regards,
Sumit.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/