From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Bird Subject: Re: [PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers Date: Tue, 8 Sep 2015 12:07:59 -0700 Message-ID: <55EF320F.3010809@sonymobile.com> References: <1441613229-13867-1-git-send-email-ivan.ivanov@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1441613229-13867-1-git-send-email-ivan.ivanov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Ivan T. Ivanov" , Felipe Balbi Cc: Greg Kroah-Hartman , "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-arm-msm@vger.kernel.org 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 > Signed-off-by: Ivan T. Ivanov > --- > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187AbbIHTIH (ORCPT ); Tue, 8 Sep 2015 15:08:07 -0400 Received: from seldrel01.sonyericsson.com ([37.139.156.2]:18359 "EHLO seldrel01.sonyericsson.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751136AbbIHTIF (ORCPT ); Tue, 8 Sep 2015 15:08:05 -0400 Subject: Re: [PATCH v2] usb: phy: msm: Unregister VBUS and ID events notifiers To: "Ivan T. Ivanov" , Felipe Balbi References: <1441613229-13867-1-git-send-email-ivan.ivanov@linaro.org> CC: Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-msm@vger.kernel.org" From: Tim Bird Message-ID: <55EF320F.3010809@sonymobile.com> Date: Tue, 8 Sep 2015 12:07:59 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <1441613229-13867-1-git-send-email-ivan.ivanov@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Ivan T. Ivanov > --- > > 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