linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Sandeep Maheswaram <sanm@codeaurora.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@kernel.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Pratham Pratap <prathampratap@codeaurora.org>
Subject: Re: [PATCH v8 2/6] usb: host: xhci: plat: Add suspend quirk for dwc3 controller
Date: Tue, 28 Sep 2021 16:08:54 -0700	[thread overview]
Message-ID: <YVOghtE9VkHGpItZ@google.com> (raw)
In-Reply-To: <1624882097-23265-3-git-send-email-sanm@codeaurora.org>

Hi,

On Mon, Jun 28, 2021 at 05:38:13PM +0530, Sandeep Maheswaram wrote:
> During suspend read the status of all port and make sure the PHYs
> are in the correct mode based on current speed.
> Phy interrupt masks are set based on this mode. Keep track of the mode
> of the HS PHY to be able to configure wakeup properly.
> 
> Also check during suspend if any wakeup capable devices are
> connected to the controller (directly or through hubs), if there
> are none set a flag to indicate that the PHY should be powered
> down during suspend.

...

> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c

> @@ -430,6 +431,39 @@ static int xhci_plat_remove(struct platform_device *dev)
>  
>  	return 0;
>  }
> +static void xhci_dwc3_suspend_quirk(struct usb_hcd *hcd)

nit: you need a blank line above this (in between functions).

> +{
> +	int i, num_ports;
> +	u32 reg;
> +	unsigned int ss_phy_mode = 0;
> +	struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
> +	struct xhci_hcd	*xhci_hcd = hcd_to_xhci(hcd);
> +
> +	dwc->hs_phy_mode = 0;
> +
> +	reg = readl(&xhci_hcd->cap_regs->hcs_params1);
> +	num_ports = HCS_MAX_PORTS(reg);
> +
> +	for (i = 0; i < num_ports; i++) {
> +		reg = readl(&xhci_hcd->op_regs->port_status_base + i * 0x04);
> +		if (reg & PORT_PE) {
> +			if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
> +				dwc->hs_phy_mode |= PHY_MODE_USB_HOST_HS;
> +			else if (DEV_LOWSPEED(reg))
> +				dwc->hs_phy_mode |= PHY_MODE_USB_HOST_LS;
> +
> +			if (DEV_SUPERSPEED(reg))
> +				ss_phy_mode |= PHY_MODE_USB_HOST_SS;
> +		}
> +	}
> +	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_mode);
> +	phy_set_mode(dwc->usb3_generic_phy, ss_phy_mode);
> +
> +	if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> +		dwc->phy_power_off = false;
> +	else
> +		dwc->phy_power_off = true;
> +}

This series breaks USB across S3 suspend/resume on Rockchip RK3399 Gru
platforms. Those platforms do not support USB wake (they power off many
of the relevant IP blocks in S3), and they *require* reconfiguring the
PHY on resume, but usb_wakeup_enabled_descendants() is returning
non-zero. If I revert patch 3, things work again.

Perhaps that's a Rockchip bug (should such platforms be disabling all PM
wakeup capabilities for their child hubs/devices?), but I'd much
appreciate resolving that before merging such a regression.

This also may be a sign that usb_wakeup_enabled_descendants() isn't
really the precise condition you should be using, if other platforms
aren't accurately reflecting feature support status in here.

Brian

P.S. In case it matters, I'm noticing this because earlier versions of
your patches (which have the same problem) are merged in our downstream
Chromium kernel tree.

  parent reply	other threads:[~2021-09-28 23:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-28 12:08 [PATCH v8 0/6] USB DWC3 host wake up support from system suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 1/6] usb: dwc3: core: Add HS phy mode variable and phy poweroff flag Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 2/6] usb: host: xhci: plat: Add suspend quirk for dwc3 controller Sandeep Maheswaram
2021-07-12  9:31   ` Felipe Balbi
2021-09-28 23:08   ` Brian Norris [this message]
2021-06-28 12:08 ` [PATCH v8 3/6] usb: dwc3: core: Host wake up support from system suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 4/6] usb: dwc3: qcom: Add helper functions to enable,disable wake irqs Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 5/6] usb: dwc3: qcom: Configure wakeup interrupts during suspend Sandeep Maheswaram
2021-06-28 12:08 ` [PATCH v8 6/6] usb: dwc3: qcom: Keep power domain on to support wakeup Sandeep Maheswaram
2021-06-28 21:23   ` Matthias Kaehlcke
2021-07-12  9:42     ` Felipe Balbi
2021-08-18  9:14       ` Sandeep Maheswaram
2021-08-18  9:56         ` Felipe Balbi
2021-09-15 14:05       ` Pavan Kondeti

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=YVOghtE9VkHGpItZ@google.com \
    --to=briannorris@chromium.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=mathias.nyman@intel.com \
    --cc=mka@chromium.org \
    --cc=prathampratap@codeaurora.org \
    --cc=sanm@codeaurora.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).