On Thu, 10 Apr 2014, Vivek Gautam wrote: > Add support to consume phy provided by Generic phy framework. > Keeping the support for older usb-phy intact right now, in order > to prevent any functionality break in absence of relevant > device tree side change for ohci-exynos. > Once we move to new phy in the device nodes for ohci, we can > remove the support for older phys. > > Signed-off-by: Vivek Gautam <gautam.vivek@xxxxxxxxxxx> > Cc: Jingoo Han <jg1.han@xxxxxxxxxxx> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > +static int exynos_ohci_phyg_on(struct phy *phy, bool on) > +{ > + int ret = 0; > + > + if (phy) { > + if (on) > + ret = phy_power_on(phy); > + else > + ret = phy_power_off(phy); > + } > + > + return ret; > +} This would be cleaner if you had separate routines for exynos_ohci_phyg_of and exynos_ohci_phyg_off. For example: static int exynos_ohci_phyg_on(struct phy *phy) { return phy ? phy_power_on(phy) : 0; } > @@ -88,16 +104,49 @@ static int exynos_ohci_probe(struct platform_device *pdev) > "samsung,exynos5440-ohci")) > goto skip_phy; > > - phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2); > - if (IS_ERR(phy)) { > - usb_put_hcd(hcd); > - dev_warn(&pdev->dev, "no platform data or transceiver defined\n"); > - return -EPROBE_DEFER; > + exynos_ohci->phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2); > + if (IS_ERR(exynos_ohci->phy)) { > + err = PTR_ERR(exynos_ohci->phy); > + if (err == -ENXIO || err == -ENODEV) { > + exynos_ohci->phy = NULL; > + } else if (err == -EPROBE_DEFER) { > + usb_put_hcd(hcd); > + return err; > + } else { > + dev_err(&pdev->dev, "no usb2 phy configured\n"); > + usb_put_hcd(hcd); > + return err; > + } Instead of all the calls to usb_put_hcd() here and below, just goto fail_clk. Or add a new fail_phy label at the same spot and goto it. Otherwise this looks basically okay. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html