| Message ID | 1359984275-24646-10-git-send-email-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 04-02-2013 17:24, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/usb/chipidea/ci13xxx_imx.c | 39 +++++++++++++++++------------------- > 1 file changed, 18 insertions(+), 21 deletions(-) > diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c > index b598bb8f..136869b 100644 > --- a/drivers/usb/chipidea/ci13xxx_imx.c > +++ b/drivers/usb/chipidea/ci13xxx_imx.c [...] > @@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) > return ret; > } > > + phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0); > + No need for emoty line here. Keep the style as it was please. > + if (PTR_ERR(phy) == -EPROBE_DEFER) { Is it valid to call PTR_ERR() on non-error pointer? Isn't it better to do this check under *else* clause below the next *if*. > + ret = -EPROBE_DEFER; > + goto err_clk; > + } > + This empty line is also not needed, I think. > + if (!IS_ERR(phy)) { > + ret = usb_phy_init(phy); > + if (ret) { > + dev_err(&pdev->dev, "unable to init phy: %d\n", ret); > + goto err_clk; > } > + > + data->phy = phy; > } > > /* we only support host now, so enable vbus here */ WBR, Sergei
On Tue, Feb 05, 2013 at 03:45:12PM +0400, Sergei Shtylyov wrote: > Hello. > > On 04-02-2013 17:24, Sascha Hauer wrote: > > >Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > >--- > > drivers/usb/chipidea/ci13xxx_imx.c | 39 +++++++++++++++++------------------- > > 1 file changed, 18 insertions(+), 21 deletions(-) > > >diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c > >index b598bb8f..136869b 100644 > >--- a/drivers/usb/chipidea/ci13xxx_imx.c > >+++ b/drivers/usb/chipidea/ci13xxx_imx.c > [...] > >@@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) > > return ret; > > } > > > >+ phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0); > >+ > > No need for emoty line here. Keep the style as it was please. > > >+ if (PTR_ERR(phy) == -EPROBE_DEFER) { > > Is it valid to call PTR_ERR() on non-error pointer? Why shouldn't it? > Isn't it > better to do this check under *else* clause below the next *if*. For better readability, yes. Sascha > > >+ ret = -EPROBE_DEFER; > >+ goto err_clk; > >+ } > >+ > > This empty line is also not needed, I think. > > >+ if (!IS_ERR(phy)) { > >+ ret = usb_phy_init(phy); > >+ if (ret) { > >+ dev_err(&pdev->dev, "unable to init phy: %d\n", ret); > >+ goto err_clk; > > } > >+ > >+ data->phy = phy; > > } > > > > /* we only support host now, so enable vbus here */ > > WBR, Sergei > >
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c index b598bb8f..136869b 100644 --- a/drivers/usb/chipidea/ci13xxx_imx.c +++ b/drivers/usb/chipidea/ci13xxx_imx.c @@ -30,7 +30,6 @@ ((struct usb_phy *)platform_get_drvdata(pdev)) struct ci13xxx_imx_data { - struct device_node *phy_np; struct usb_phy *phy; struct platform_device *ci_pdev; struct clk *clk; @@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) { struct ci13xxx_imx_data *data; struct ci13xxx_platform_data *pdata; - struct platform_device *plat_ci, *phy_pdev; - struct device_node *phy_np; + struct platform_device *plat_ci; struct resource *res; struct regulator *reg_vbus; struct pinctrl *pinctrl; int ret; + struct usb_phy *phy; if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL) && !usbmisc_ops) @@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) return ret; } - phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0); - if (phy_np) { - data->phy_np = phy_np; - phy_pdev = of_find_device_by_node(phy_np); - if (phy_pdev) { - struct usb_phy *phy; - phy = pdev_to_phy(phy_pdev); - if (phy && - try_module_get(phy_pdev->dev.driver->owner)) { - usb_phy_init(phy); - data->phy = phy; - } + phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0); + + if (PTR_ERR(phy) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto err_clk; + } + + if (!IS_ERR(phy)) { + ret = usb_phy_init(phy); + if (ret) { + dev_err(&pdev->dev, "unable to init phy: %d\n", ret); + goto err_clk; } + + data->phy = phy; } /* we only support host now, so enable vbus here */ @@ -170,7 +171,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to enable vbus regulator, err=%d\n", ret); - goto put_np; + goto err_clk; } data->reg_vbus = reg_vbus; } else { @@ -222,9 +223,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev) err: if (reg_vbus) regulator_disable(reg_vbus); -put_np: - if (phy_np) - of_node_put(phy_np); +err_clk: clk_disable_unprepare(data->clk); return ret; } @@ -244,8 +243,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev) module_put(data->phy->dev->driver->owner); } - of_node_put(data->phy_np); - clk_disable_unprepare(data->clk); platform_set_drvdata(pdev, NULL);
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/usb/chipidea/ci13xxx_imx.c | 39 +++++++++++++++++------------------- 1 file changed, 18 insertions(+), 21 deletions(-)