| Message ID | 1364935250-26422-1-git-send-email-thierry@gilfi.de |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 04/02/2013 02:40 PM, Thierry Reding wrote: > The cleanup path checks whether the transceiver was properly initialized > using IS_ERR(). However it can also happen that the cleanup path is run > before the transceiver was initialized (or the operating mode isn't set > to TEGRA_USB_OTG) and is therefore NULL. Add a separate label for error > unwinding and initialize the transceiver field to ERR_PTR(-ENODEV) when > the operating mode isn't TEGRA_USB_OTG to allow for consistent checking. Acked-by: Stephen Warren <swarren@nvidia.com> I also forwarded the patch to our USB expert off-list to make sure he's OK with it; I hope he comments on it in the near future. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- > owner@vger.kernel.org] On Behalf Of Thierry Reding > Sent: Wednesday, April 03, 2013 2:11 AM > To: Felipe Balbi > Cc: Stephen Warren; Alan Stern; linux-tegra@vger.kernel.org; linux- > usb@vger.kernel.org > Subject: [PATCH] usb: host: ehci-tegra: Fix oops in error cleanup > > The cleanup path checks whether the transceiver was properly initialized > using IS_ERR(). However it can also happen that the cleanup path is run > before the transceiver was initialized (or the operating mode isn't set > to TEGRA_USB_OTG) and is therefore NULL. Add a separate label for error > unwinding and initialize the transceiver field to ERR_PTR(-ENODEV) when > the operating mode isn't TEGRA_USB_OTG to allow for consistent checking. This change looks good to me. Acked-by: Venu Byravarasu <vbyravarasu@nvidia.com> > > Signed-off-by: Thierry Reding <thierry@gilfi.de> > --- > drivers/usb/host/ehci-tegra.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c > index 1d2488c..c8cb488 100644 > --- a/drivers/usb/host/ehci-tegra.c > +++ b/drivers/usb/host/ehci-tegra.c > @@ -755,7 +755,7 @@ static int tegra_ehci_probe(struct platform_device > *pdev) > err = usb_phy_set_suspend(hcd->phy, 0); > if (err) { > dev_err(&pdev->dev, "Failed to power on the phy\n"); > - goto fail; > + goto fail_phy; > } -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 02, 2013 at 10:40:50PM +0200, Thierry Reding wrote: > The cleanup path checks whether the transceiver was properly initialized > using IS_ERR(). However it can also happen that the cleanup path is run > before the transceiver was initialized (or the operating mode isn't set > to TEGRA_USB_OTG) and is therefore NULL. Add a separate label for error > unwinding and initialize the transceiver field to ERR_PTR(-ENODEV) when > the operating mode isn't TEGRA_USB_OTG to allow for consistent checking. > > Signed-off-by: Thierry Reding <thierry@gilfi.de> > Acked-by: Venu Byravarasu <vbyravarasu@nvidia.com> > Acked-by: Stephen Warren <swarren@nvidia.com> > --- > drivers/usb/host/ehci-tegra.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) This patch fails to apply to my tree: checking file drivers/usb/host/ehci-tegra.c Hunk #2 succeeded at 765 with fuzz 1. Hunk #3 succeeded at 774 with fuzz 2 (offset 1 line). Hunk #4 FAILED at 796. 1 out of 4 hunks FAILED Can you please fix it up and resend it, keeping the needed acks on it? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 1d2488c..c8cb488 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -755,7 +755,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) err = usb_phy_set_suspend(hcd->phy, 0); if (err) { dev_err(&pdev->dev, "Failed to power on the phy\n"); - goto fail; + goto fail_phy; } tegra->host_resumed = 1; @@ -765,7 +765,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) if (!irq) { dev_err(&pdev->dev, "Failed to get IRQ\n"); err = -ENODEV; - goto fail; + goto fail_phy; } if (pdata->operating_mode == TEGRA_USB_OTG) { @@ -773,6 +773,8 @@ static int tegra_ehci_probe(struct platform_device *pdev) devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2); if (!IS_ERR(tegra->transceiver)) otg_set_host(tegra->transceiver->otg, &hcd->self); + } else { + tegra->transceiver = ERR_PTR(-ENODEV); } err = usb_add_hcd(hcd, irq, IRQF_SHARED); @@ -794,6 +796,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) fail: if (!IS_ERR(tegra->transceiver)) otg_set_host(tegra->transceiver->otg, NULL); +fail_phy: usb_phy_shutdown(hcd->phy); fail_io: clk_disable_unprepare(tegra->clk);
The cleanup path checks whether the transceiver was properly initialized using IS_ERR(). However it can also happen that the cleanup path is run before the transceiver was initialized (or the operating mode isn't set to TEGRA_USB_OTG) and is therefore NULL. Add a separate label for error unwinding and initialize the transceiver field to ERR_PTR(-ENODEV) when the operating mode isn't TEGRA_USB_OTG to allow for consistent checking. Signed-off-by: Thierry Reding <thierry@gilfi.de> --- drivers/usb/host/ehci-tegra.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)