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@xxxxxxxxxxxxxx> > >--- > > 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 > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html