All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Elson Serrao <quic_eserrao@quicinc.com>,
	"balbi@kernel.org" <balbi@kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"quic_wcheng@quicinc.com" <quic_wcheng@quicinc.com>,
	"quic_jackp@quicinc.com" <quic_jackp@quicinc.com>,
	"quic_mrana@quicinc.com" <quic_mrana@quicinc.com>
Subject: Re: [PATCH 2/5] usb: gadget: Add function wakeup support
Date: Fri, 12 Aug 2022 02:19:01 +0000	[thread overview]
Message-ID: <e3bcfd4c-efdb-c7b0-4e94-1afcd3b8eb73@synopsys.com> (raw)
In-Reply-To: <98966b47-0bc5-6ec0-ec80-5eff1d71d9fd@synopsys.com>

On 8/11/2022, Thinh Nguyen wrote:
> On 8/11/2022, Elson Serrao wrote:
>>
>>
>> On 8/9/2022 6:08 PM, Thinh Nguyen wrote:
> 
> <snip>
> 
> 
>>> To summarize the points:
>>>
>>> 1) The host only arms function remote wakeup if the device is capable of
>>> remote wakeup (check USB_CONFIG_ATT_WAKEUP in bmAttributes and hardware
>>> capability)
>>>
>>> 2) If the device is in suspend, the device can do remote wakeup (through
>>> LFPS handshake) if the function is armed for remote wakeup (through
>>> SET_FEATURE(FUNC_SUSPEND)).
>>>
>>> 3) If the link transitions to U0 after the device triggering a remote
>>> wakeup, the device will also send device notification function wake for
>>> all the interfaces armed with remote wakeup.
>>>
>>> 4) If the device is not in suspend, the device can send device
>>> notification function wake if it's in U0.
>>>
>>>
>>> Now, remote wakeup and function wake device notification are 2 separate
>>> operations. We have the usb_gadget_ops->wakeup() for remote wakeup. I
>>> suggested to maybe add usb_gadget_ops->send_wakeup_notification(gadget,
>>> intf_id) for the device notification. What you did was combining both
>>> operations in usb_gadget_ops->func_wakeup(). That may only work for
>>> point 4) (assuming you fix the U0 check), but not point 3).
>>
>> Thank you for your feedback and summary. I will rename func_wakeup to
>> send_wakeup_notification to better align with the approach. The reason I
>> have combined remote_wakeup and function wake notification in
>> usb_gadget_ops->func_wakeup() is because since the implementation is at
>> function/composite level it has no knowledge on the link state. So I
>> have delegated that task to controller driver to handle the notification
>> accordingly. That is do a LFPS handshake first if the device is
>> suspended and then send notification (explained below). But we can
>> definitely separate this by adding an additional flag in the composite
>> layer to set the link state based on the gadget suspend callback called
>> when U3 SUSPEND interrupt is received. Let me know if you feel
>> separating the two is a better approach.
>>
> 
> The reason I think we need to separate it is because of point 3. As I
> note earlier, the spec states that "If remote wake event occurs in
> multiple functions, each function shall send a Function Wake Notification."
> 
> But if there's no remote wake event, and the host brought the device up
> instead, then the function suspend state is retained.
> 
> If we separate these 2 operations, the caller can check whether the
> operation went through properly. For example, if the wakeup() is
> initiated properly, but the function wake device notification didn't go
> through. We would only need to resend the device notification rather
> than initiate remote wakeup again.

If we don't have to send device notification for other interfaces, we
can combine the operations here as you did.

BR,
Thinh

  reply	other threads:[~2022-08-12  2:19 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-02 19:18 [PATCH 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
2022-08-02 19:18 ` [PATCH 1/5] usb: dwc3: Add remote wakeup handling Elson Roy Serrao
2022-08-03  0:01   ` Thinh Nguyen
2022-08-02 19:18 ` [PATCH 2/5] usb: gadget: Add function wakeup support Elson Roy Serrao
2022-08-02 23:51   ` Thinh Nguyen
2022-08-04 21:42     ` Elson Serrao
2022-08-05  1:26       ` Thinh Nguyen
2022-08-09 19:42         ` Elson Serrao
2022-08-10  1:08           ` Thinh Nguyen
2022-08-11 20:31             ` Elson Serrao
2022-08-12  2:00               ` Thinh Nguyen
2022-08-12  2:19                 ` Thinh Nguyen [this message]
2022-08-13  0:46                   ` Thinh Nguyen
2022-08-16 19:57                     ` Elson Serrao
2022-08-16 23:51                       ` Thinh Nguyen
2022-08-18 18:17                         ` Elson Serrao
2022-08-18 19:41                           ` Elson Serrao
2022-08-23  1:01                           ` Thinh Nguyen
2022-08-23 22:06                             ` Elson Serrao
2022-08-26  1:30                               ` Thinh Nguyen
2022-09-13 20:13                                 ` Elson Serrao
2022-09-15  2:06                                   ` Thinh Nguyen
2022-08-11 21:03             ` Elson Serrao
2022-08-12  2:07               ` Thinh Nguyen
2022-08-02 19:18 ` [PATCH 3/5] usb: dwc3: Add function suspend and " Elson Roy Serrao
2022-08-02 23:44   ` Thinh Nguyen
2022-08-02 19:18 ` [PATCH 4/5] usb: gadget: f_ecm: Add suspend/resume and remote " Elson Roy Serrao
2022-08-02 19:18 ` [PATCH 5/5] usb: gadget: f_ecm: Add function suspend and " Elson Roy Serrao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e3bcfd4c-efdb-c7b0-4e94-1afcd3b8eb73@synopsys.com \
    --to=thinh.nguyen@synopsys.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=quic_eserrao@quicinc.com \
    --cc=quic_jackp@quicinc.com \
    --cc=quic_mrana@quicinc.com \
    --cc=quic_wcheng@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.