[PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers

[PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers

[Ivan T. Ivanov]

Right now even if driver failed to probe extcon framework will
still deliver its VBUS and ID events, which will lead to random
exception codes.

Fix this by removing VBUS and ID events notifiers when probe fail.

Fixes: 591fc116f330 ("usb: phy: msm: Use extcon framework for VBUS and ID detection")

Reported-by: Tim Bird <tim.bird@sonymobile.com>
Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
---

Patch reworked to use new extcon API.

 drivers/usb/phy/phy-msm-usb.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c58c3c0dbe35..0fd21c147c7e 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1598,6 +1598,8 @@ static int msm_otg_read_dt(struct platform_device *pdev, struct msm_otg *motg)
 		ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
 						&motg->id.nb);
 		if (ret < 0) {
+			extcon_unregister_notifier(motg->vbus.extcon,
+						   EXTCON_USB, &motg->vbus.nb);
 			dev_err(&pdev->dev, "register ID notifier failed\n");
 			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)
@@ -1731,6 +1724,15 @@ static int msm_otg_probe(struct platform_device *pdev)
 		return motg->irq;
 	}

+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		if (!np)
+			return -ENXIO;
+		ret = msm_otg_read_dt(pdev, motg);
+		if (ret)
+			return ret;
+	}
+
 	regs[0].supply = "vddcx";
 	regs[1].supply = "v3p3";
 	regs[2].supply = "v1p8";
@@ -1834,6 +1836,11 @@ disable_clks:
 	clk_disable_unprepare(motg->clk);
 	if (!IS_ERR(motg->core_clk))
 		clk_disable_unprepare(motg->core_clk);
+
+	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;
 }

--
1.9.1

Re: [PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers

[Tim Bird]

On 09/07/2015 01:07 AM, Ivan T. Ivanov wrote:
> Right now even if driver failed to probe extcon framework will
> still deliver its VBUS and ID events, which will lead to random
> exception codes.
> 
> Fix this by removing VBUS and ID events notifiers when probe fail.
> 
> Fixes: 591fc116f330 ("usb: phy: msm: Use extcon framework for VBUS and ID detection")
> 
> Reported-by: Tim Bird <tim.bird-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> Signed-off-by: Ivan T. Ivanov <ivan.ivanov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> 
> Patch reworked to use new extcon API.
> 
>  drivers/usb/phy/phy-msm-usb.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index c58c3c0dbe35..0fd21c147c7e 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -1598,6 +1598,8 @@ static int msm_otg_read_dt(struct platform_device *pdev, struct msm_otg *motg)
>  		ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
>  						&motg->id.nb);
>  		if (ret < 0) {
> +			extcon_unregister_notifier(motg->vbus.extcon,
> +						   EXTCON_USB, &motg->vbus.nb);
>  			dev_err(&pdev->dev, "register ID notifier failed\n");
>  			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)
> @@ -1731,6 +1724,15 @@ static int msm_otg_probe(struct platform_device *pdev)
>  		return motg->irq;
>  	}
> 
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata) {
> +		if (!np)
> +			return -ENXIO;
> +		ret = msm_otg_read_dt(pdev, motg);
> +		if (ret)
> +			return ret;
> +	}
> +

The following code depends on msm_otg_read_dt(), but the
movement of the call puts the phy_number test before msm_otg_read_dt().

        /*
         * NOTE: The PHYs can be multiplexed between the chipidea controller
         * and the dwc3 controller, using a single bit. It is important that
         * the dwc3 driver does not set this bit in an incompatible way.
         */
        if (motg->phy_number) {
                phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4);
                if (!phy_select)
                        return -ENOMEM;
                /* Enable second PHY with the OTG port */
                writel(0x1, phy_select);
        }

Can you please move the motg->phy_number code after the msm_otg_read_dt() call?
(right here, in the patch, would be good :-)

>  	regs[0].supply = "vddcx";
>  	regs[1].supply = "v3p3";
>  	regs[2].supply = "v1p8";
> @@ -1834,6 +1836,11 @@ disable_clks:
>  	clk_disable_unprepare(motg->clk);
>  	if (!IS_ERR(motg->core_clk))
>  		clk_disable_unprepare(motg->core_clk);
> +
> +	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;
>  }

Other than that - it looks good.
 -- Tim

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html