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

From: Tim Bird
Date: Tue Sep 08 2015 - 15:08:09 EST




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@xxxxxxxxxxxxxx>
> Signed-off-by: Ivan T. Ivanov <ivan.ivanov@xxxxxxxxxx>
> ---
>
> 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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/