diff mbox

usb: phy: msm: fix error handling in probe.

Message ID 1452593376-7163-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Srinivas Kandagatla Jan. 12, 2016, 10:09 a.m. UTC
This driver registers for extcon events as part of its probe, but
never unregisters them in case of error in the probe path.

There were multiple issues noticed due to this missing error handling.
One of them is random crashes if the regulators are not ready yet by the
time probe is invoked.

Ivan's previous attempt [1] to fix this issue, did not really address
all the failure cases like regualtor failures.

[1] https://lkml.org/lkml/2015/9/7/62

Without this patch the kernel would carsh with log:
...
Unable to handle kernel paging request at virtual address 17d78410
pgd = ffffffc001a5c000
[17d78410] *pgd=00000000b6806003, *pud=00000000b6806003, *pmd=0000000000000000
Internal error: Oops: 96000005 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 6 Comm: kworker/u8:0 Not tainted 4.4.0+ #48
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
Workqueue: deferwq deferred_probe_work_func
task: ffffffc03686e900 ti: ffffffc0368b0000 task.ti: ffffffc0368b0000
PC is at raw_notifier_chain_register+0x1c/0x44
LR is at extcon_register_notifier+0x88/0xc8
pc : [<ffffffc0000da43c>] lr : [<ffffffc000606298>] pstate: 80000085
sp : ffffffc0368b3a70
x29: ffffffc0368b3a70 x28: ffffffc03680c310
x27: ffffffc035518000 x26: ffffffc035518000
x25: ffffffc03bfa20e0 x24: ffffffc035580a18
x23: 0000000000000000 x22: ffffffc035518458
x21: ffffffc0355e9a60 x20: ffffffc035518000
x19: 0000000000000000 x18: 0000000000000028
x17: 0000000000000003 x16: ffffffc0018153c8
x15: 0000000000000001 x14: ffffffc03686f0f8
x13: ffffffc03686f0f8 x12: 0000000000000003
x11: 0000000000000001 x10: 0000000000000001
x9 : ffffffc03686f0f8 x8 : 0000e3872014c1a1
x7 : 0000000000000028 x6 : 0000000000000000
x5 : 0000000000000001 x4 : 0000000000000000
x3 : 00000000354fb170 x2 : 0000000017d78400
x1 : ffffffc0355e9a60 x0 : ffffffc0354fb268

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/usb/phy/phy-msm-usb.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

Comments

Felipe Balbi Jan. 13, 2016, 7:41 a.m. UTC | #1
Hi,

Srinivas Kandagatla <srinivas.kandagatla@linaro.org> writes:
> This driver registers for extcon events as part of its probe, but
> never unregisters them in case of error in the probe path.
>
> There were multiple issues noticed due to this missing error handling.
> One of them is random crashes if the regulators are not ready yet by the
> time probe is invoked.
>
> Ivan's previous attempt [1] to fix this issue, did not really address
> all the failure cases like regualtor failures.
>
> [1] https://lkml.org/lkml/2015/9/7/62
>
> Without this patch the kernel would carsh with log:
> ...
> Unable to handle kernel paging request at virtual address 17d78410
> pgd = ffffffc001a5c000
> [17d78410] *pgd=00000000b6806003, *pud=00000000b6806003, *pmd=0000000000000000
> Internal error: Oops: 96000005 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 6 Comm: kworker/u8:0 Not tainted 4.4.0+ #48
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> Workqueue: deferwq deferred_probe_work_func
> task: ffffffc03686e900 ti: ffffffc0368b0000 task.ti: ffffffc0368b0000
> PC is at raw_notifier_chain_register+0x1c/0x44
> LR is at extcon_register_notifier+0x88/0xc8
> pc : [<ffffffc0000da43c>] lr : [<ffffffc000606298>] pstate: 80000085
> sp : ffffffc0368b3a70
> x29: ffffffc0368b3a70 x28: ffffffc03680c310
> x27: ffffffc035518000 x26: ffffffc035518000
> x25: ffffffc03bfa20e0 x24: ffffffc035580a18
> x23: 0000000000000000 x22: ffffffc035518458
> x21: ffffffc0355e9a60 x20: ffffffc035518000
> x19: 0000000000000000 x18: 0000000000000028
> x17: 0000000000000003 x16: ffffffc0018153c8
> x15: 0000000000000001 x14: ffffffc03686f0f8
> x13: ffffffc03686f0f8 x12: 0000000000000003
> x11: 0000000000000001 x10: 0000000000000001
> x9 : ffffffc03686f0f8 x8 : 0000e3872014c1a1
> x7 : 0000000000000028 x6 : 0000000000000000
> x5 : 0000000000000001 x4 : 0000000000000000
> x3 : 00000000354fb170 x2 : 0000000017d78400
> x1 : ffffffc0355e9a60 x0 : ffffffc0354fb268
>

you need to "blame" a commit with:

Fixes: First-12-characters-of-commit-hash ("commit subject line")

and if the failing commit sits in a released kernel, you also need to
add:

Cc: <stable@vger.kernel.org> # vX.Y
diff mbox

Patch

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 0d19a6d..970a30e 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1599,6 +1599,8 @@  static int msm_otg_read_dt(struct platform_device *pdev, struct msm_otg *motg)
 						&motg->id.nb);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "register ID notifier failed\n");
+			extcon_unregister_notifier(motg->vbus.extcon,
+						   EXTCON_USB, &motg->vbus.nb);
 			return ret;
 		}
 
@@ -1660,15 +1662,6 @@  static int msm_otg_probe(struct platform_device *pdev)
 	if (!motg)
 		return -ENOMEM;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		if (!np)
-			return -ENXIO;
-		ret = msm_otg_read_dt(pdev, motg);
-		if (ret)
-			return ret;
-	}
-
 	motg->phy.otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg),
 				     GFP_KERNEL);
 	if (!motg->phy.otg)
@@ -1710,6 +1703,15 @@  static int msm_otg_probe(struct platform_device *pdev)
 	if (!motg->regs)
 		return -ENOMEM;
 
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		if (!np)
+			return -ENXIO;
+		ret = msm_otg_read_dt(pdev, motg);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * NOTE: The PHYs can be multiplexed between the chipidea controller
 	 * and the dwc3 controller, using a single bit. It is important that
@@ -1717,8 +1719,10 @@  static int msm_otg_probe(struct platform_device *pdev)
 	 */
 	if (motg->phy_number) {
 		phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4);
-		if (!phy_select)
-			return -ENOMEM;
+		if (!phy_select) {
+			ret = -ENOMEM;
+			goto unregister_extcon;
+		}
 		/* Enable second PHY with the OTG port */
 		writel(0x1, phy_select);
 	}
@@ -1728,7 +1732,8 @@  static int msm_otg_probe(struct platform_device *pdev)
 	motg->irq = platform_get_irq(pdev, 0);
 	if (motg->irq < 0) {
 		dev_err(&pdev->dev, "platform_get_irq failed\n");
-		return motg->irq;
+		ret = motg->irq;
+		goto unregister_extcon;
 	}
 
 	regs[0].supply = "vddcx";
@@ -1737,7 +1742,7 @@  static int msm_otg_probe(struct platform_device *pdev)
 
 	ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(regs), regs);
 	if (ret)
-		return ret;
+		goto unregister_extcon;
 
 	motg->vddcx = regs[0].consumer;
 	motg->v3p3  = regs[1].consumer;
@@ -1834,6 +1839,12 @@  disable_clks:
 	clk_disable_unprepare(motg->clk);
 	if (!IS_ERR(motg->core_clk))
 		clk_disable_unprepare(motg->core_clk);
+unregister_extcon:
+	extcon_unregister_notifier(motg->id.extcon,
+				   EXTCON_USB_HOST, &motg->id.nb);
+	extcon_unregister_notifier(motg->vbus.extcon,
+				   EXTCON_USB, &motg->vbus.nb);
+
 	return ret;
 }