linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Hovold <johan+linaro@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Krishna Kurapati <quic_kriskura@quicinc.com>,
	Stephen Boyd <swboyd@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	"Matthias Kaehlcke" <mka@chromium.org>,
	Pavankumar Kondeti <quic_pkondeti@quicinc.com>,
	quic_ppratap@quicinc.com, quic_vpulyala@quicinc.com,
	linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Johan Hovold <johan+linaro@kernel.org>
Subject: [PATCH v2 6/9] usb: dwc3: qcom: fix peripheral and OTG suspend
Date: Thu,  4 Aug 2022 17:09:58 +0200	[thread overview]
Message-ID: <20220804151001.23612-7-johan+linaro@kernel.org> (raw)
In-Reply-To: <20220804151001.23612-1-johan+linaro@kernel.org>

A recent commit implementing wakeup support in host mode instead broke
suspend for peripheral and OTG mode.

The hack that was added in the suspend path to determine the speed of
any device connected to the USB2 bus not only accesses internal driver
data for a child device, but also dereferences a NULL pointer or
accesses freed data when the controller is not acting as host.

There's no quick fix to the layering violation, but since reverting
would leave us with broken suspend in host mode with wakeup triggering
immediately, let's keep the hack for now.

Fix the immediate issues by only checking the host bus speed and
enabling wakeup interrupts when acting as host.

Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---

Changes in v2
 - disable wakeup completely instead of falling back to the
   "disconnected" host configuration when not acting as host

 drivers/usb/dwc3/dwc3-qcom.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 05b4666fde14..6ae0b7fc4e2c 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -309,8 +309,13 @@ static bool dwc3_qcom_is_host(struct dwc3_qcom *qcom)
 static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom)
 {
 	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
-	struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
 	struct usb_device *udev;
+	struct usb_hcd *hcd;
+
+	/*
+	 * FIXME: Fix this layering violation.
+	 */
+	hcd = platform_get_drvdata(dwc->xhci);
 
 	/*
 	 * It is possible to query the speed of all children of
@@ -416,7 +421,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
 	if (ret)
 		dev_warn(qcom->dev, "failed to disable interconnect: %d\n", ret);
 
-	if (wakeup) {
+	/*
+	 * The role is stable during suspend as role switching is done from a
+	 * freezable workqueue.
+	 */
+	if (dwc3_qcom_is_host(qcom) && wakeup) {
 		qcom->usb2_speed = dwc3_qcom_read_usb2_speed(qcom);
 		dwc3_qcom_enable_interrupts(qcom);
 	}
@@ -434,7 +443,7 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
 	if (!qcom->is_suspended)
 		return 0;
 
-	if (wakeup)
+	if (dwc3_qcom_is_host(qcom) && wakeup)
 		dwc3_qcom_disable_interrupts(qcom);
 
 	for (i = 0; i < qcom->num_clocks; i++) {
-- 
2.35.1


  parent reply	other threads:[~2022-08-04 15:10 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-04 15:09 [PATCH v2 0/9] usb: dwc3: qcom: fix wakeup implementation Johan Hovold
2022-08-04 15:09 ` [PATCH v2 1/9] usb: dwc3: fix PHY disable sequence Johan Hovold
2022-08-06 13:48   ` Manivannan Sadhasivam
2022-08-04 15:09 ` [PATCH v2 2/9] Revert "usb: dwc3: qcom: Keep power domain on to retain controller status" Johan Hovold
2022-08-06 13:52   ` Manivannan Sadhasivam
2022-08-04 15:09 ` [PATCH v2 3/9] usb: dwc3: qcom: fix gadget-only builds Johan Hovold
2022-08-04 18:18   ` Randy Dunlap
2022-08-06 14:15   ` Manivannan Sadhasivam
2022-08-06 16:04     ` Johan Hovold
2022-08-06 16:42       ` Manivannan Sadhasivam
2022-08-06 16:51         ` Johan Hovold
2022-08-08 13:05   ` Greg Kroah-Hartman
2022-08-08 13:34     ` Johan Hovold
2022-08-18 17:44       ` Greg Kroah-Hartman
2022-08-04 15:09 ` [PATCH v2 4/9] usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup Johan Hovold
2022-08-04 15:53   ` Matthias Kaehlcke
2022-08-06 14:33   ` Manivannan Sadhasivam
2022-08-06 16:08     ` Johan Hovold
2022-08-06 16:44       ` Manivannan Sadhasivam
2022-08-04 15:09 ` [PATCH v2 5/9] usb: dwc3: qcom: fix runtime PM wakeup Johan Hovold
2022-08-04 20:00   ` Matthias Kaehlcke
2022-08-06 14:35   ` Manivannan Sadhasivam
2022-08-04 15:09 ` Johan Hovold [this message]
2022-08-04 21:38   ` [PATCH v2 6/9] usb: dwc3: qcom: fix peripheral and OTG suspend kernel test robot
2022-08-05  6:58     ` Johan Hovold
2022-08-05  7:10       ` Johan Hovold
2022-08-04 15:09 ` [PATCH v2 7/9] dt-bindings: usb: qcom,dwc3: add wakeup-source property Johan Hovold
2022-08-06 15:08   ` Manivannan Sadhasivam
2022-08-06 16:41     ` Johan Hovold
2022-08-06 16:52       ` Manivannan Sadhasivam
2022-08-06 17:09         ` Johan Hovold
2022-08-08  8:05           ` Manivannan Sadhasivam
2022-08-04 15:10 ` [PATCH v2 8/9] usb: dwc3: qcom: fix wakeup implementation Johan Hovold
2022-08-04 16:59   ` Matthias Kaehlcke
2022-08-05 16:58     ` Matthias Kaehlcke
2022-08-06 16:22       ` Johan Hovold
2022-08-08 17:22         ` Matthias Kaehlcke
2022-08-06 14:57   ` Manivannan Sadhasivam
2022-08-06 16:33     ` Johan Hovold
2022-08-04 15:10 ` [PATCH v2 9/9] usb: dwc3: qcom: clean up suspend callbacks Johan Hovold
2022-08-04 17:08   ` Matthias Kaehlcke
2022-08-04 15:15 ` [PATCH v2 0/9] usb: dwc3: qcom: fix wakeup implementation Johan Hovold

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=20220804151001.23612-7-johan+linaro@kernel.org \
    --to=johan+linaro@kernel.org \
    --cc=agross@kernel.org \
    --cc=balbi@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=mka@chromium.org \
    --cc=quic_kriskura@quicinc.com \
    --cc=quic_pkondeti@quicinc.com \
    --cc=quic_ppratap@quicinc.com \
    --cc=quic_vpulyala@quicinc.com \
    --cc=robh+dt@kernel.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).