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 <k.debski@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -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)
> @@ -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.
> }
>
> @@ -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.
> + }
>
> 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
--
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
