From: Sandeep Maheswaram <sanm@codeaurora.org>
To: Matthias Kaehlcke <mka@chromium.org>, Felipe Balbi <balbi@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Stephen Boyd <swboyd@chromium.org>,
Doug Anderson <dianders@chromium.org>,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org,
Manu Gautam <mgautam@codeaurora.org>
Subject: Re: [PATCH v7 2/5] usb: dwc3: core: Host wake up support from system suspend
Date: Wed, 26 May 2021 09:59:33 +0530 [thread overview]
Message-ID: <4e3951dc-f3e7-0815-7d73-d836240de3e9@codeaurora.org> (raw)
In-Reply-To: <YJxNBm0WiMqjJ2Cg@google.com>
Hi Felipe,
On 5/13/2021 3:17 AM, Matthias Kaehlcke wrote:
> On Mon, May 03, 2021 at 02:20:23PM +0300, Felipe Balbi wrote:
>> Hi,
>>
>> Sandeep Maheswaram <sanm@codeaurora.org> writes:
>>>> Sandeep Maheswaram <sanm@codeaurora.org> writes:
>>>>> Avoiding phy powerdown when wakeup capable devices are connected
>>>>> by checking phy_power_off flag.
>>>>> Phy should be on to wake up the device from suspend using wakeup capable
>>>>> devices such as keyboard and mouse.
>>>>>
>>>>> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
>>>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>>>>> ---
>>>>> drivers/usb/dwc3/core.c | 7 +++++--
>>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index b6e53d8..bb414c3 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -1738,7 +1738,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>>>> dwc3_core_exit(dwc);
>>>>> break;
>>>>> case DWC3_GCTL_PRTCAP_HOST:
>>>>> - if (!PMSG_IS_AUTO(msg)) {
>>>>> + if (!PMSG_IS_AUTO(msg) && dwc->phy_power_off) {
>>>> should be able to detect this generically, no? Shouldn't
>>>> device_may_wakeup() be valid here and give you the answer you want?
>>> I think device_may_wakeup() gives whether the controller is wake up
>>> capable or not.
>> Yes, but it's a bit more than that. Looking at devices.rst we read:
>>
>> If :c:func:`device_may_wakeup(dev)` returns ``true``, the device should be
>> prepared for generating hardware wakeup signals to trigger a system wakeup event
>> when the system is in the sleep state. For example, :c:func:`enable_irq_wake()`
>> might identify GPIO signals hooked up to a switch or other external hardware,
>> and :c:func:`pci_enable_wake()` does something similar for the PCI PME signal.
>>
>> So, if there is a condition where $this device has to, somehow, deal
>> with wakeup, it should be configured accordingly. This ->phy_power_off
>> flag is telling us the same thing.
>>
>>> But we want to keep phy powered on only when some wakeup capable devices
>>> (eg:keyboard ,mouse ) are connected to controller.
>> Understood, it could be that we're missing some method for propagating
>> that state (i.e. keyboard with PM support) up to the parent device, but
>> that's no excuse to bypass driver boundaries. Wouldn't you agree?
> I'm not sure if device_may_wakeup() is really the right tool for the
> job. This is the current implementation:
>
> static inline bool device_may_wakeup(struct device *dev)
> {
> return dev->power.can_wakeup && !!dev->power.wakeup;
> }
>
> IIUC power.can_wakeup specifies whether the device is wakeup
> capable, primarily in physical terms and indicating that the
> driver is ready to handle wakeups, and power.wakeup represents
> the policy which can be changed by userspace.
>
> Supposing the hub is generally wakeup capable that flag
> shouldn't be changed. Neither should be the policy based on
> what is connected to the bus.
Please suggest us how to proceed further with this patch.
next prev parent reply other threads:[~2021-05-26 4:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-28 5:11 [PATCH v7 0/5] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
2021-04-28 5:11 ` [PATCH v7 1/5] usb: dwc3: host: Set PHY mode during suspend Sandeep Maheswaram
2021-04-28 9:55 ` Felipe Balbi
[not found] ` <fd927828-a414-cd42-1e4a-b9e9b0744a3a@codeaurora.org>
2021-05-03 11:16 ` Felipe Balbi
2021-05-12 19:12 ` Matthias Kaehlcke
2021-05-13 13:46 ` Felipe Balbi
2021-05-13 14:37 ` Matthias Kaehlcke
2021-04-28 5:11 ` [PATCH v7 2/5] usb: dwc3: core: Host wake up support from system suspend Sandeep Maheswaram
2021-04-28 9:59 ` Felipe Balbi
2021-05-03 4:33 ` Sandeep Maheswaram
2021-05-03 11:20 ` Felipe Balbi
2021-05-12 21:47 ` Matthias Kaehlcke
2021-05-26 4:29 ` Sandeep Maheswaram [this message]
2021-04-28 5:11 ` [PATCH v7 3/5] usb: dwc3: qcom: Add helper functions to enable,disable wake irqs Sandeep Maheswaram
2021-04-28 5:11 ` [PATCH v7 4/5] usb: dwc3: qcom: Configure wakeup interrupts during suspend Sandeep Maheswaram
2021-04-28 10:02 ` Felipe Balbi
2021-04-28 5:11 ` [PATCH v7 5/5] usb: dwc3: qcom: Keep power domain on to support wakeup Sandeep Maheswaram
2021-04-28 10:04 ` Felipe Balbi
2021-05-13 0:23 ` Matthias Kaehlcke
2021-05-13 13:49 ` Felipe Balbi
2021-05-13 14:34 ` Matthias Kaehlcke
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=4e3951dc-f3e7-0815-7d73-d836240de3e9@codeaurora.org \
--to=sanm@codeaurora.org \
--cc=agross@kernel.org \
--cc=balbi@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=dianders@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mgautam@codeaurora.org \
--cc=mka@chromium.org \
--cc=swboyd@chromium.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).