Re: [PATCH v4 4/9] usb: ehci-s5p: Change to use phy provided by the generic phy framework
From: Alan Stern <stern@rowland.harvard.edu>
Date: 2013-12-05 18:52:49
Also in:
linux-samsung-soc, lkml
On Thu, 5 Dec 2013, Kamil Debski wrote:
Change the phy provider used from the old usb phy specific to a new one using the generic phy framework. Signed-off-by: Kamil Debski <redacted> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
quoted hunk ↗ jump to hunk
--- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c
quoted hunk ↗ jump to hunk
@@ -42,10 +42,10 @@ static const char hcd_name[] = "ehci-exynos"; static struct hc_driver __read_mostly exynos_ehci_hc_driver; +#define PHY_NUMBER 3 struct exynos_ehci_hcd { struct clk *clk; - struct usb_phy *phy; - struct usb_otg *otg;
Are you sure you want to remove that line?
+ struct phy *phy[PHY_NUMBER]; }; #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
quoted hunk ↗ jump to hunk
@@ -102,13 +132,24 @@ static int exynos_ehci_probe(struct platform_device *pdev) "samsung,exynos5440-ehci")) 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; - } else { - exynos_ehci->phy = phy; + for_each_available_child_of_node(pdev->dev.of_node, child) { + err = of_property_read_u32(child, "reg", &phy_number); + if (err) { + dev_err(&pdev->dev, "Failed to parse device tree\n"); + return err; + } + if (phy_number >= PHY_NUMBER) { + dev_err(&pdev->dev, "Failed to parse device tree - number out of range\n"); + return -EINVAL;
Do you need to call of_node_put(child) before each of these return statements?
+ }
+ phy = devm_of_phy_get(&pdev->dev, child, 0);
+ of_node_put(child);
+ if (IS_ERR(phy)) {
+ dev_err(&pdev->dev, "Failed to get phy number %d",
+ phy_number);
+ return PTR_ERR(phy);
+ }
+ exynos_ehci->phy[phy_number] = phy;
exynos_ehci->otg = phy->otg;Did you intend to remove this line? Above, you removed the exynos_ehci->otg field. I can't see how this patch would ever compile without an error.
quoted hunk ↗ jump to hunk
}@@ -149,11 +190,11 @@ skip_phy: goto fail_io; } - if (exynos_ehci->otg) - exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self); - - if (exynos_ehci->phy) - usb_phy_init(exynos_ehci->phy); + err = exynos_phys_on(exynos_ehci->phy); + if (err) { + dev_err(&pdev->dev, "Failed to enabled phys\n"); + goto fail_phys_on;
Why add a new statement label? Just goto fail_io.
quoted hunk ↗ jump to hunk
+ } ehci = hcd_to_ehci(hcd); ehci->caps = hcd->regs;@@ -172,8 +213,8 @@ skip_phy: return 0; fail_add_hcd: - if (exynos_ehci->phy) - usb_phy_shutdown(exynos_ehci->phy); + exynos_phys_off(exynos_ehci->phy); +fail_phys_on: fail_io: clk_disable_unprepare(exynos_ehci->clk); fail_clk:
Alan Stern