[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Hans de Goede]

Hi All,

This version of my ohci and ehci-platform clks, phy and dt support patch-set,
really fixes the 2 small bugs Alan found.

Regards,

Hans

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Hans de Goede]

Hi All,

This version of my ohci and ehci-platform clks, phy and dt support patch-set,
really fixes the 2 small bugs Alan found.

Regards,

Hans

[PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Hans de Goede]

Add support for ohci-platform instantiation from devicetree, including
optionally getting clks and a phy from devicetree, and enabling / disabling
those on power_on / off.

This should allow using ohci-platform from devicetree in various cases.
Specifically after this commit it can be used for the ohci controller found
on Allwinner sunxi SoCs.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 Documentation/devicetree/bindings/usb/usb-ohci.txt |  22 +++
 drivers/usb/host/ohci-platform.c                   | 164 ++++++++++++++++++---
 2 files changed, 162 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-ohci.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
new file mode 100644
index 0000000..f9d6c73
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -0,0 +1,22 @@
+USB OHCI controllers
+
+Required properties:
+- compatible : "usb-ohci"
+- reg : ohci controller register range (address and length)
+- interrupts : ohci controller interrupt
+
+Optional properties:
+- clocks : a list of phandle + clock specifier pairs
+- phys : phandle + phy specifier pair
+- phy-names : "usb"
+
+Example:
+
+	ohci0: ohci at 0x01c14400 {
+		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
+		reg = <0x01c14400 0x100>;
+		interrupts = <64>;
+		clocks = <&usb_clk 6>, <&ahb_gates 2>;
+		phys = <&usbphy 1>;
+		phy-names = "usb";
+	};
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index f351ff5..b2d0e1e 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -3,6 +3,7 @@
  *
  * Copyright 2007 Michael Buesch <m@bues.ch>
  * Copyright 2011-2012 Hauke Mehrtens <hauke@hauke-m.de>
+ * Copyright 2014 Hans de Goede <hdegoede@redhat.com>
  *
  * Derived from the OCHI-SSB driver
  * Derived from the OHCI-PCI driver
@@ -14,11 +15,14 @@
  * Licensed under the GNU/GPL. See COPYING for details.
  */
 
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
 #include <linux/hrtimer.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb/ohci_pdriver.h>
 #include <linux/usb.h>
@@ -27,6 +31,13 @@
 #include "ohci.h"
 
 #define DRIVER_DESC "OHCI generic platform driver"
+#define OHCI_MAX_CLKS 3
+#define hcd_to_ohci_priv(h) ((struct ohci_platform_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_platform_priv {
+	struct clk *clks[OHCI_MAX_CLKS];
+	struct phy *phy;
+};
 
 static const char hcd_name[] = "ohci-platform";
 
@@ -48,11 +59,67 @@ static int ohci_platform_reset(struct usb_hcd *hcd)
 	return ohci_setup(hcd);
 }
 
+static int ohci_platform_power_on(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk, ret;
+
+	for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+		ret = clk_prepare_enable(priv->clks[clk]);
+		if (ret)
+			goto err_disable_clks;
+	}
+
+	if (priv->phy) {
+		ret = phy_init(priv->phy);
+		if (ret)
+			goto err_disable_clks;
+
+		ret = phy_power_on(priv->phy);
+		if (ret)
+			goto err_exit_phy;
+	}
+
+	return 0;
+
+err_exit_phy:
+	phy_exit(priv->phy);
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(priv->clks[clk]);
+
+	return ret;
+}
+
+static void ohci_platform_power_off(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk;
+
+	if (priv->phy) {
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
+	for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--)
+		if (priv->clks[clk])
+			clk_disable_unprepare(priv->clks[clk]);
+}
+
 static struct hc_driver __read_mostly ohci_platform_hc_driver;
 
 static const struct ohci_driver_overrides platform_overrides __initconst = {
-	.product_desc =	"Generic Platform OHCI controller",
-	.reset =	ohci_platform_reset,
+	.product_desc =		"Generic Platform OHCI controller",
+	.reset =		ohci_platform_reset,
+	.extra_priv_size =	sizeof(struct ohci_platform_priv),
+};
+
+static struct usb_ohci_pdata ohci_platform_defaults = {
+	.power_on =		ohci_platform_power_on,
+	.power_suspend =	ohci_platform_power_off,
+	.power_off =		ohci_platform_power_off,
 };
 
 static int ohci_platform_probe(struct platform_device *dev)
@@ -60,17 +127,23 @@ static int ohci_platform_probe(struct platform_device *dev)
 	struct usb_hcd *hcd;
 	struct resource *res_mem;
 	struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
-	int irq;
-	int err = -ENOMEM;
-
-	if (!pdata) {
-		WARN_ON(1);
-		return -ENODEV;
-	}
+	struct ohci_platform_priv *priv;
+	int err, irq, clk = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
 
+	/*
+	 * Use reasonable defaults so platforms don't have to provide these
+	 * with DT probing on ARM.
+	 */
+	if (!pdata)
+		pdata = &ohci_platform_defaults;
+
+	err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+	if (err)
+		return err;
+
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0) {
 		dev_err(&dev->dev, "no irq provided");
@@ -83,17 +156,40 @@ static int ohci_platform_probe(struct platform_device *dev)
 		return -ENXIO;
 	}
 
+	hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
+			dev_name(&dev->dev));
+	if (!hcd)
+		return -ENOMEM;
+
+	platform_set_drvdata(dev, hcd);
+	dev->dev.platform_data = pdata;
+	priv = hcd_to_ohci_priv(hcd);
+
+	if (pdata == &ohci_platform_defaults && dev->dev.of_node) {
+		priv->phy = devm_phy_get(&dev->dev, "usb");
+		if (IS_ERR(priv->phy)) {
+			err = PTR_ERR(priv->phy);
+			if (err == -EPROBE_DEFER)
+				goto err_put_hcd;
+			priv->phy = NULL;
+		}
+
+		for (clk = 0; clk < OHCI_MAX_CLKS; clk++) {
+			priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+			if (IS_ERR(priv->clks[clk])) {
+				err = PTR_ERR(priv->clks[clk]);
+				if (err == -EPROBE_DEFER)
+					goto err_put_clks;
+				priv->clks[clk] = NULL;
+				break;
+			}
+		}
+	}
+
 	if (pdata->power_on) {
 		err = pdata->power_on(dev);
 		if (err < 0)
-			return err;
-	}
-
-	hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
-			dev_name(&dev->dev));
-	if (!hcd) {
-		err = -ENOMEM;
-		goto err_power;
+			goto err_put_clks;
 	}
 
 	hcd->rsrc_start = res_mem->start;
@@ -102,21 +198,25 @@ static int ohci_platform_probe(struct platform_device *dev)
 	hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);
-		goto err_put_hcd;
+		goto err_power;
 	}
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err_put_hcd;
-
-	platform_set_drvdata(dev, hcd);
+		goto err_power;
 
 	return err;
 
-err_put_hcd:
-	usb_put_hcd(hcd);
 err_power:
 	if (pdata->power_off)
 		pdata->power_off(dev);
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(priv->clks[clk]);
+err_put_hcd:
+	if (pdata == &ohci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
+	usb_put_hcd(hcd);
 
 	return err;
 }
@@ -125,13 +225,22 @@ static int ohci_platform_remove(struct platform_device *dev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(dev);
 	struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk;
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(hcd);
 
 	if (pdata->power_off)
 		pdata->power_off(dev);
 
+	for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++)
+		clk_put(priv->clks[clk]);
+
+	usb_put_hcd(hcd);
+
+	if (pdata == &ohci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
 	return 0;
 }
 
@@ -178,6 +287,12 @@ static int ohci_platform_resume(struct device *dev)
 #define ohci_platform_resume	NULL
 #endif /* CONFIG_PM */
 
+static const struct of_device_id ohci_platform_ids[] = {
+	{ .compatible = "usb-ohci", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ohci_platform_ids);
+
 static const struct platform_device_id ohci_platform_table[] = {
 	{ "ohci-platform", 0 },
 	{ }
@@ -198,6 +313,7 @@ static struct platform_driver ohci_platform_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "ohci-platform",
 		.pm	= &ohci_platform_pm_ops,
+		.of_match_table = ohci_platform_ids,
 	}
 };
 
-- 
1.8.4.2

[PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Hans de Goede]

Add support for ohci-platform instantiation from devicetree, including
optionally getting clks and a phy from devicetree, and enabling / disabling
those on power_on / off.

This should allow using ohci-platform from devicetree in various cases.
Specifically after this commit it can be used for the ohci controller found
on Allwinner sunxi SoCs.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb-ohci.txt |  22 +++
 drivers/usb/host/ohci-platform.c                   | 164 ++++++++++++++++++---
 2 files changed, 162 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-ohci.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
new file mode 100644
index 0000000..f9d6c73
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -0,0 +1,22 @@
+USB OHCI controllers
+
+Required properties:
+- compatible : "usb-ohci"
+- reg : ohci controller register range (address and length)
+- interrupts : ohci controller interrupt
+
+Optional properties:
+- clocks : a list of phandle + clock specifier pairs
+- phys : phandle + phy specifier pair
+- phy-names : "usb"
+
+Example:
+
+	ohci0: ohci@0x01c14400 {
+		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
+		reg = <0x01c14400 0x100>;
+		interrupts = <64>;
+		clocks = <&usb_clk 6>, <&ahb_gates 2>;
+		phys = <&usbphy 1>;
+		phy-names = "usb";
+	};
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index f351ff5..b2d0e1e 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -3,6 +3,7 @@
  *
  * Copyright 2007 Michael Buesch <m@bues.ch>
  * Copyright 2011-2012 Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
+ * Copyright 2014 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  *
  * Derived from the OCHI-SSB driver
  * Derived from the OHCI-PCI driver
@@ -14,11 +15,14 @@
  * Licensed under the GNU/GPL. See COPYING for details.
  */
 
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
 #include <linux/hrtimer.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb/ohci_pdriver.h>
 #include <linux/usb.h>
@@ -27,6 +31,13 @@
 #include "ohci.h"
 
 #define DRIVER_DESC "OHCI generic platform driver"
+#define OHCI_MAX_CLKS 3
+#define hcd_to_ohci_priv(h) ((struct ohci_platform_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_platform_priv {
+	struct clk *clks[OHCI_MAX_CLKS];
+	struct phy *phy;
+};
 
 static const char hcd_name[] = "ohci-platform";
 
@@ -48,11 +59,67 @@ static int ohci_platform_reset(struct usb_hcd *hcd)
 	return ohci_setup(hcd);
 }
 
+static int ohci_platform_power_on(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk, ret;
+
+	for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+		ret = clk_prepare_enable(priv->clks[clk]);
+		if (ret)
+			goto err_disable_clks;
+	}
+
+	if (priv->phy) {
+		ret = phy_init(priv->phy);
+		if (ret)
+			goto err_disable_clks;
+
+		ret = phy_power_on(priv->phy);
+		if (ret)
+			goto err_exit_phy;
+	}
+
+	return 0;
+
+err_exit_phy:
+	phy_exit(priv->phy);
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(priv->clks[clk]);
+
+	return ret;
+}
+
+static void ohci_platform_power_off(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk;
+
+	if (priv->phy) {
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
+	for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--)
+		if (priv->clks[clk])
+			clk_disable_unprepare(priv->clks[clk]);
+}
+
 static struct hc_driver __read_mostly ohci_platform_hc_driver;
 
 static const struct ohci_driver_overrides platform_overrides __initconst = {
-	.product_desc =	"Generic Platform OHCI controller",
-	.reset =	ohci_platform_reset,
+	.product_desc =		"Generic Platform OHCI controller",
+	.reset =		ohci_platform_reset,
+	.extra_priv_size =	sizeof(struct ohci_platform_priv),
+};
+
+static struct usb_ohci_pdata ohci_platform_defaults = {
+	.power_on =		ohci_platform_power_on,
+	.power_suspend =	ohci_platform_power_off,
+	.power_off =		ohci_platform_power_off,
 };
 
 static int ohci_platform_probe(struct platform_device *dev)
@@ -60,17 +127,23 @@ static int ohci_platform_probe(struct platform_device *dev)
 	struct usb_hcd *hcd;
 	struct resource *res_mem;
 	struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
-	int irq;
-	int err = -ENOMEM;
-
-	if (!pdata) {
-		WARN_ON(1);
-		return -ENODEV;
-	}
+	struct ohci_platform_priv *priv;
+	int err, irq, clk = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
 
+	/*
+	 * Use reasonable defaults so platforms don't have to provide these
+	 * with DT probing on ARM.
+	 */
+	if (!pdata)
+		pdata = &ohci_platform_defaults;
+
+	err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
+	if (err)
+		return err;
+
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0) {
 		dev_err(&dev->dev, "no irq provided");
@@ -83,17 +156,40 @@ static int ohci_platform_probe(struct platform_device *dev)
 		return -ENXIO;
 	}
 
+	hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
+			dev_name(&dev->dev));
+	if (!hcd)
+		return -ENOMEM;
+
+	platform_set_drvdata(dev, hcd);
+	dev->dev.platform_data = pdata;
+	priv = hcd_to_ohci_priv(hcd);
+
+	if (pdata == &ohci_platform_defaults && dev->dev.of_node) {
+		priv->phy = devm_phy_get(&dev->dev, "usb");
+		if (IS_ERR(priv->phy)) {
+			err = PTR_ERR(priv->phy);
+			if (err == -EPROBE_DEFER)
+				goto err_put_hcd;
+			priv->phy = NULL;
+		}
+
+		for (clk = 0; clk < OHCI_MAX_CLKS; clk++) {
+			priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+			if (IS_ERR(priv->clks[clk])) {
+				err = PTR_ERR(priv->clks[clk]);
+				if (err == -EPROBE_DEFER)
+					goto err_put_clks;
+				priv->clks[clk] = NULL;
+				break;
+			}
+		}
+	}
+
 	if (pdata->power_on) {
 		err = pdata->power_on(dev);
 		if (err < 0)
-			return err;
-	}
-
-	hcd = usb_create_hcd(&ohci_platform_hc_driver, &dev->dev,
-			dev_name(&dev->dev));
-	if (!hcd) {
-		err = -ENOMEM;
-		goto err_power;
+			goto err_put_clks;
 	}
 
 	hcd->rsrc_start = res_mem->start;
@@ -102,21 +198,25 @@ static int ohci_platform_probe(struct platform_device *dev)
 	hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);
-		goto err_put_hcd;
+		goto err_power;
 	}
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err_put_hcd;
-
-	platform_set_drvdata(dev, hcd);
+		goto err_power;
 
 	return err;
 
-err_put_hcd:
-	usb_put_hcd(hcd);
 err_power:
 	if (pdata->power_off)
 		pdata->power_off(dev);
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(priv->clks[clk]);
+err_put_hcd:
+	if (pdata == &ohci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
+	usb_put_hcd(hcd);
 
 	return err;
 }
@@ -125,13 +225,22 @@ static int ohci_platform_remove(struct platform_device *dev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(dev);
 	struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
+	int clk;
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(hcd);
 
 	if (pdata->power_off)
 		pdata->power_off(dev);
 
+	for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++)
+		clk_put(priv->clks[clk]);
+
+	usb_put_hcd(hcd);
+
+	if (pdata == &ohci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
 	return 0;
 }
 
@@ -178,6 +287,12 @@ static int ohci_platform_resume(struct device *dev)
 #define ohci_platform_resume	NULL
 #endif /* CONFIG_PM */
 
+static const struct of_device_id ohci_platform_ids[] = {
+	{ .compatible = "usb-ohci", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ohci_platform_ids);
+
 static const struct platform_device_id ohci_platform_table[] = {
 	{ "ohci-platform", 0 },
 	{ }
@@ -198,6 +313,7 @@ static struct platform_driver ohci_platform_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "ohci-platform",
 		.pm	= &ohci_platform_pm_ops,
+		.of_match_table = ohci_platform_ids,
 	}
 };
 
-- 
1.8.4.2

[PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Sergei Shtylyov]

Hello.

On 01/15/2014 10:24 PM, Hans de Goede wrote:

> Add support for ohci-platform instantiation from devicetree, including
> optionally getting clks and a phy from devicetree, and enabling / disabling
> those on power_on / off.

> This should allow using ohci-platform from devicetree in various cases.
> Specifically after this commit it can be used for the ohci controller found
> on Allwinner sunxi SoCs.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
[...]

   Have only found time to fully read the patches just now...

> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> new file mode 100644
> index 0000000..f9d6c73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> @@ -0,0 +1,22 @@
> +USB OHCI controllers
> +
> +Required properties:
> +- compatible : "usb-ohci"
> +- reg : ohci controller register range (address and length)
> +- interrupts : ohci controller interrupt
> +
> +Optional properties:
> +- clocks : a list of phandle + clock specifier pairs
> +- phys : phandle + phy specifier pair
> +- phy-names : "usb"
> +
> +Example:
> +
> +	ohci0: ohci at 0x01c14400 {

    Two minor nits: there should be no "0x" in the address part of the node 
name. And according to ePAPR [1], "the name of a node should be somewhat 
generic, reflecting the function of the device and not its precise programming 
model. If appropriate, the name should be one of the following choices:
[...]
- usb".

    Same comments for "usb-ehci" binding.

> +		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
> +		reg = <0x01c14400 0x100>;
> +		interrupts = <64>;
> +		clocks = <&usb_clk 6>, <&ahb_gates 2>;
> +		phys = <&usbphy 1>;
> +		phy-names = "usb";
> +	};

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei

Re: [PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Sergei Shtylyov]

Hello.

On 01/15/2014 10:24 PM, Hans de Goede wrote:

> Add support for ohci-platform instantiation from devicetree, including
> optionally getting clks and a phy from devicetree, and enabling / disabling
> those on power_on / off.

> This should allow using ohci-platform from devicetree in various cases.
> Specifically after this commit it can be used for the ohci controller found
> on Allwinner sunxi SoCs.

> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
[...]

   Have only found time to fully read the patches just now...

> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> new file mode 100644
> index 0000000..f9d6c73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> @@ -0,0 +1,22 @@
> +USB OHCI controllers
> +
> +Required properties:
> +- compatible : "usb-ohci"
> +- reg : ohci controller register range (address and length)
> +- interrupts : ohci controller interrupt
> +
> +Optional properties:
> +- clocks : a list of phandle + clock specifier pairs
> +- phys : phandle + phy specifier pair
> +- phy-names : "usb"
> +
> +Example:
> +
> +	ohci0: ohci@0x01c14400 {

    Two minor nits: there should be no "0x" in the address part of the node 
name. And according to ePAPR [1], "the name of a node should be somewhat 
generic, reflecting the function of the device and not its precise programming 
model. If appropriate, the name should be one of the following choices:
[...]
- usb".

    Same comments for "usb-ehci" binding.

> +		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
> +		reg = <0x01c14400 0x100>;
> +		interrupts = <64>;
> +		clocks = <&usb_clk 6>, <&ahb_gates 2>;
> +		phys = <&usbphy 1>;
> +		phy-names = "usb";
> +	};

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei

[PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Hans de Goede]

Hi,

On 01/21/2014 06:59 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 01/15/2014 10:24 PM, Hans de Goede wrote:
>
>> Add support for ohci-platform instantiation from devicetree, including
>> optionally getting clks and a phy from devicetree, and enabling / disabling
>> those on power_on / off.
>
>> This should allow using ohci-platform from devicetree in various cases.
>> Specifically after this commit it can be used for the ohci controller found
>> on Allwinner sunxi SoCs.
>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> [...]
>
>    Have only found time to fully read the patches just now...
>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> new file mode 100644
>> index 0000000..f9d6c73
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> @@ -0,0 +1,22 @@
>> +USB OHCI controllers
>> +
>> +Required properties:
>> +- compatible : "usb-ohci"
>> +- reg : ohci controller register range (address and length)
>> +- interrupts : ohci controller interrupt
>> +
>> +Optional properties:
>> +- clocks : a list of phandle + clock specifier pairs
>> +- phys : phandle + phy specifier pair
>> +- phy-names : "usb"
>> +
>> +Example:
>> +
>> +    ohci0: ohci at 0x01c14400 {
>
>     Two minor nits: there should be no "0x" in the address part of the node name. And according to ePAPR [1], "the name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model. If appropriate, the name should be one of the following choices:
> [...]
> - usb".
>
>     Same comments for "usb-ehci" binding.

You're right on both accounts, I'll do a v8 tomorrow, including a re-spin of the
big-endian patches.

Regards,

Hans

Re: [PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation

[Hans de Goede]

Hi,

On 01/21/2014 06:59 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 01/15/2014 10:24 PM, Hans de Goede wrote:
>
>> Add support for ohci-platform instantiation from devicetree, including
>> optionally getting clks and a phy from devicetree, and enabling / disabling
>> those on power_on / off.
>
>> This should allow using ohci-platform from devicetree in various cases.
>> Specifically after this commit it can be used for the ohci controller found
>> on Allwinner sunxi SoCs.
>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
> [...]
>
>    Have only found time to fully read the patches just now...
>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> new file mode 100644
>> index 0000000..f9d6c73
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> @@ -0,0 +1,22 @@
>> +USB OHCI controllers
>> +
>> +Required properties:
>> +- compatible : "usb-ohci"
>> +- reg : ohci controller register range (address and length)
>> +- interrupts : ohci controller interrupt
>> +
>> +Optional properties:
>> +- clocks : a list of phandle + clock specifier pairs
>> +- phys : phandle + phy specifier pair
>> +- phy-names : "usb"
>> +
>> +Example:
>> +
>> +    ohci0: ohci@0x01c14400 {
>
>     Two minor nits: there should be no "0x" in the address part of the node name. And according to ePAPR [1], "the name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model. If appropriate, the name should be one of the following choices:
> [...]
> - usb".
>
>     Same comments for "usb-ehci" binding.

You're right on both accounts, I'll do a v8 tomorrow, including a re-spin of the
big-endian patches.

Regards,

Hans

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH v7 2/2] ehci-platform: Add support for clks and phy passed through devicetree

[Hans de Goede]

Currently ehci-platform is only used in combination with devicetree when used
with some Via socs. By extending it to (optionally) get clks and a phy from
devicetree, and enabling / disabling those on power_on / off, it can be used
more generically. Specifically after this commit it can be used for the
ehci controller on Allwinner sunxi SoCs.

Since ehci-platform is intended to handle any generic enough non pci ehci
device, add a "usb-ehci" compatibility string.

There already is a usb-ehci device-tree bindings document, update this
with clks and phy bindings info.

Although actually quite generic so far the via,vt8500 compatibilty string
had its own bindings document. Somehow we even ended up with 2 of them. Since
these provide no extra information over the generic usb-ehci documentation,
this patch removes them.

The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
2 drivers claiming the same compatibility string getting build on ppc.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 Documentation/devicetree/bindings/usb/usb-ehci.txt |  25 +++-
 .../devicetree/bindings/usb/via,vt8500-ehci.txt    |  15 ---
 .../devicetree/bindings/usb/vt8500-ehci.txt        |  12 --
 drivers/usb/host/Kconfig                           |   1 +
 drivers/usb/host/ehci-platform.c                   | 149 +++++++++++++++++----
 5 files changed, 142 insertions(+), 60 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
 delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index fa18612..fad10f3 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -7,13 +7,14 @@ Required properties:
     (debug-port or other) can be also specified here, but only after
     definition of standard EHCI registers.
   - interrupts : one EHCI interrupt should be described here.
-If device registers are implemented in big endian mode, the device
-node should have "big-endian-regs" property.
-If controller implementation operates with big endian descriptors,
-"big-endian-desc" property should be specified.
-If both big endian registers and descriptors are used by the controller
-implementation, "big-endian" property can be specified instead of having
-both "big-endian-regs" and "big-endian-desc".
+
+Optional properties:
+ - big-endian-regs : boolean, set this for hcds with big-endian registers
+ - big-endian-desc : boolean, set this for hcds with big-endian descriptors
+ - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
+ - clocks : a list of phandle + clock specifier pairs
+ - phys : phandle + phy specifier pair
+ - phy-names : "usb"
 
 Example (Sequoia 440EPx):
     ehci at e0000300 {
@@ -23,3 +24,13 @@ Example (Sequoia 440EPx):
 	   reg = <0 e0000300 90 0 e0000390 70>;
 	   big-endian;
    };
+
+Example (Allwinner sun4i A10 SoC):
+   ehci0: ehci at 0x01c14000 {
+	   compatible = "allwinner,sun4i-a10-ehci", "usb-ehci";
+	   reg = <0x01c14000 0x100>;
+	   interrupts = <39>;
+	   clocks = <&ahb_gates 1>;
+	   phys = <&usbphy 1>;
+	   phy-names = "usb";
+   };
diff --git a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
deleted file mode 100644
index 17b3ad1..0000000
--- a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-VIA/Wondermedia VT8500 EHCI Controller
------------------------------------------------------
-
-Required properties:
-- compatible : "via,vt8500-ehci"
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : ehci controller interrupt
-
-Example:
-
-	ehci at d8007900 {
-		compatible = "via,vt8500-ehci";
-		reg = <0xd8007900 0x200>;
-		interrupts = <43>;
-	};
diff --git a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
deleted file mode 100644
index 5fb8fd6..0000000
--- a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-VIA VT8500 and Wondermedia WM8xxx SoC USB controllers.
-
-Required properties:
- - compatible: Should be "via,vt8500-ehci" or "wm,prizm-ehci".
- - reg: Address range of the ehci registers. size should be 0x200
- - interrupts: Should contain the ehci interrupt.
-
-usb: ehci at D8007100 {
-	compatible = "wm,prizm-ehci", "usb-ehci";
-	reg = <0xD8007100 0x200>;
-	interrupts = <1>;
-};
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index a9707da..e28cbe0 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -255,6 +255,7 @@ config USB_EHCI_ATH79
 
 config USB_EHCI_HCD_PLATFORM
 	tristate "Generic EHCI driver for a platform device"
+	depends on !PPC_OF
 	default n
 	---help---
 	  Adds an EHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 7f30b71..d8aebc0 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -3,6 +3,7 @@
  *
  * Copyright 2007 Steven Brown <sbrown@cortland.com>
  * Copyright 2010-2012 Hauke Mehrtens <hauke@hauke-m.de>
+ * Copyright 2014 Hans de Goede <hdegoede@redhat.com>
  *
  * Derived from the ohci-ssb driver
  * Copyright 2007 Michael Buesch <m@bues.ch>
@@ -18,6 +19,7 @@
  *
  * Licensed under the GNU/GPL. See COPYING for details.
  */
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
@@ -25,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
@@ -33,6 +36,13 @@
 #include "ehci.h"
 
 #define DRIVER_DESC "EHCI generic platform driver"
+#define EHCI_MAX_CLKS 3
+#define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv)
+
+struct ehci_platform_priv {
+	struct clk *clks[EHCI_MAX_CLKS];
+	struct phy *phy;
+};
 
 static const char hcd_name[] = "ehci-platform";
 
@@ -64,38 +74,90 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
 	return 0;
 }
 
+static int ehci_platform_power_on(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk, ret;
+
+	for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+		ret = clk_prepare_enable(priv->clks[clk]);
+		if (ret)
+			goto err_disable_clks;
+	}
+
+	if (priv->phy) {
+		ret = phy_init(priv->phy);
+		if (ret)
+			goto err_disable_clks;
+
+		ret = phy_power_on(priv->phy);
+		if (ret)
+			goto err_exit_phy;
+	}
+
+	return 0;
+
+err_exit_phy:
+	phy_exit(priv->phy);
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(priv->clks[clk]);
+
+	return ret;
+}
+
+static void ehci_platform_power_off(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk;
+
+	if (priv->phy) {
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
+	for (clk = EHCI_MAX_CLKS - 1; clk >= 0; clk--)
+		if (priv->clks[clk])
+			clk_disable_unprepare(priv->clks[clk]);
+}
+
 static struct hc_driver __read_mostly ehci_platform_hc_driver;
 
 static const struct ehci_driver_overrides platform_overrides __initconst = {
-	.reset =	ehci_platform_reset,
+	.reset =		ehci_platform_reset,
+	.extra_priv_size =	sizeof(struct ehci_platform_priv),
 };
 
-static struct usb_ehci_pdata ehci_platform_defaults;
+static struct usb_ehci_pdata ehci_platform_defaults = {
+	.power_on =		ehci_platform_power_on,
+	.power_suspend =	ehci_platform_power_off,
+	.power_off =		ehci_platform_power_off,
+};
 
 static int ehci_platform_probe(struct platform_device *dev)
 {
 	struct usb_hcd *hcd;
 	struct resource *res_mem;
-	struct usb_ehci_pdata *pdata;
-	int irq;
-	int err;
+	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ehci_platform_priv *priv;
+	int err, irq, clk = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
 
 	/*
-	 * use reasonable defaults so platforms don't have to provide these.
-	 * with DT probing on ARM, none of these are set.
+	 * Use reasonable defaults so platforms don't have to provide these
+	 * with DT probing on ARM.
 	 */
-	if (!dev_get_platdata(&dev->dev))
-		dev->dev.platform_data = &ehci_platform_defaults;
+	if (!pdata)
+		pdata = &ehci_platform_defaults;
 
 	err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
 	if (err)
 		return err;
 
-	pdata = dev_get_platdata(&dev->dev);
-
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0) {
 		dev_err(&dev->dev, "no irq provided");
@@ -107,17 +169,40 @@ static int ehci_platform_probe(struct platform_device *dev)
 		return -ENXIO;
 	}
 
+	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
+			     dev_name(&dev->dev));
+	if (!hcd)
+		return -ENOMEM;
+
+	platform_set_drvdata(dev, hcd);
+	dev->dev.platform_data = pdata;
+	priv = hcd_to_ehci_priv(hcd);
+
+	if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
+		priv->phy = devm_phy_get(&dev->dev, "usb");
+		if (IS_ERR(priv->phy)) {
+			err = PTR_ERR(priv->phy);
+			if (err == -EPROBE_DEFER)
+				goto err_put_hcd;
+			priv->phy = NULL;
+		}
+
+		for (clk = 0; clk < EHCI_MAX_CLKS; clk++) {
+			priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+			if (IS_ERR(priv->clks[clk])) {
+				err = PTR_ERR(priv->clks[clk]);
+				if (err == -EPROBE_DEFER)
+					goto err_put_clks;
+				priv->clks[clk] = NULL;
+				break;
+			}
+		}
+	}
+
 	if (pdata->power_on) {
 		err = pdata->power_on(dev);
 		if (err < 0)
-			return err;
-	}
-
-	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
-			     dev_name(&dev->dev));
-	if (!hcd) {
-		err = -ENOMEM;
-		goto err_power;
+			goto err_put_clks;
 	}
 
 	hcd->rsrc_start = res_mem->start;
@@ -126,21 +211,25 @@ static int ehci_platform_probe(struct platform_device *dev)
 	hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);
-		goto err_put_hcd;
+		goto err_power;
 	}
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err_put_hcd;
-
-	platform_set_drvdata(dev, hcd);
+		goto err_power;
 
 	return err;
 
-err_put_hcd:
-	usb_put_hcd(hcd);
 err_power:
 	if (pdata->power_off)
 		pdata->power_off(dev);
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(priv->clks[clk]);
+err_put_hcd:
+	if (pdata == &ehci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
+	usb_put_hcd(hcd);
 
 	return err;
 }
@@ -149,13 +238,19 @@ static int ehci_platform_remove(struct platform_device *dev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(dev);
 	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk;
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(hcd);
 
 	if (pdata->power_off)
 		pdata->power_off(dev);
 
+	for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
+		clk_put(priv->clks[clk]);
+
+	usb_put_hcd(hcd);
+
 	if (pdata == &ehci_platform_defaults)
 		dev->dev.platform_data = NULL;
 
@@ -206,8 +301,10 @@ static int ehci_platform_resume(struct device *dev)
 static const struct of_device_id vt8500_ehci_ids[] = {
 	{ .compatible = "via,vt8500-ehci", },
 	{ .compatible = "wm,prizm-ehci", },
+	{ .compatible = "usb-ehci", },
 	{}
 };
+MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
 
 static const struct platform_device_id ehci_platform_table[] = {
 	{ "ehci-platform", 0 },
-- 
1.8.4.2

[PATCH v7 2/2] ehci-platform: Add support for clks and phy passed through devicetree

[Hans de Goede]

Currently ehci-platform is only used in combination with devicetree when used
with some Via socs. By extending it to (optionally) get clks and a phy from
devicetree, and enabling / disabling those on power_on / off, it can be used
more generically. Specifically after this commit it can be used for the
ehci controller on Allwinner sunxi SoCs.

Since ehci-platform is intended to handle any generic enough non pci ehci
device, add a "usb-ehci" compatibility string.

There already is a usb-ehci device-tree bindings document, update this
with clks and phy bindings info.

Although actually quite generic so far the via,vt8500 compatibilty string
had its own bindings document. Somehow we even ended up with 2 of them. Since
these provide no extra information over the generic usb-ehci documentation,
this patch removes them.

The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
2 drivers claiming the same compatibility string getting build on ppc.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb-ehci.txt |  25 +++-
 .../devicetree/bindings/usb/via,vt8500-ehci.txt    |  15 ---
 .../devicetree/bindings/usb/vt8500-ehci.txt        |  12 --
 drivers/usb/host/Kconfig                           |   1 +
 drivers/usb/host/ehci-platform.c                   | 149 +++++++++++++++++----
 5 files changed, 142 insertions(+), 60 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
 delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index fa18612..fad10f3 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -7,13 +7,14 @@ Required properties:
     (debug-port or other) can be also specified here, but only after
     definition of standard EHCI registers.
   - interrupts : one EHCI interrupt should be described here.
-If device registers are implemented in big endian mode, the device
-node should have "big-endian-regs" property.
-If controller implementation operates with big endian descriptors,
-"big-endian-desc" property should be specified.
-If both big endian registers and descriptors are used by the controller
-implementation, "big-endian" property can be specified instead of having
-both "big-endian-regs" and "big-endian-desc".
+
+Optional properties:
+ - big-endian-regs : boolean, set this for hcds with big-endian registers
+ - big-endian-desc : boolean, set this for hcds with big-endian descriptors
+ - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
+ - clocks : a list of phandle + clock specifier pairs
+ - phys : phandle + phy specifier pair
+ - phy-names : "usb"
 
 Example (Sequoia 440EPx):
     ehci@e0000300 {
@@ -23,3 +24,13 @@ Example (Sequoia 440EPx):
 	   reg = <0 e0000300 90 0 e0000390 70>;
 	   big-endian;
    };
+
+Example (Allwinner sun4i A10 SoC):
+   ehci0: ehci@0x01c14000 {
+	   compatible = "allwinner,sun4i-a10-ehci", "usb-ehci";
+	   reg = <0x01c14000 0x100>;
+	   interrupts = <39>;
+	   clocks = <&ahb_gates 1>;
+	   phys = <&usbphy 1>;
+	   phy-names = "usb";
+   };
diff --git a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
deleted file mode 100644
index 17b3ad1..0000000
--- a/Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-VIA/Wondermedia VT8500 EHCI Controller
------------------------------------------------------
-
-Required properties:
-- compatible : "via,vt8500-ehci"
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : ehci controller interrupt
-
-Example:
-
-	ehci@d8007900 {
-		compatible = "via,vt8500-ehci";
-		reg = <0xd8007900 0x200>;
-		interrupts = <43>;
-	};
diff --git a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt b/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
deleted file mode 100644
index 5fb8fd6..0000000
--- a/Documentation/devicetree/bindings/usb/vt8500-ehci.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-VIA VT8500 and Wondermedia WM8xxx SoC USB controllers.
-
-Required properties:
- - compatible: Should be "via,vt8500-ehci" or "wm,prizm-ehci".
- - reg: Address range of the ehci registers. size should be 0x200
- - interrupts: Should contain the ehci interrupt.
-
-usb: ehci@D8007100 {
-	compatible = "wm,prizm-ehci", "usb-ehci";
-	reg = <0xD8007100 0x200>;
-	interrupts = <1>;
-};
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index a9707da..e28cbe0 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -255,6 +255,7 @@ config USB_EHCI_ATH79
 
 config USB_EHCI_HCD_PLATFORM
 	tristate "Generic EHCI driver for a platform device"
+	depends on !PPC_OF
 	default n
 	---help---
 	  Adds an EHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 7f30b71..d8aebc0 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -3,6 +3,7 @@
  *
  * Copyright 2007 Steven Brown <sbrown-aLUkz7fEuXBWk0Htik3J/w@public.gmane.org>
  * Copyright 2010-2012 Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
+ * Copyright 2014 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  *
  * Derived from the ohci-ssb driver
  * Copyright 2007 Michael Buesch <m@bues.ch>
@@ -18,6 +19,7 @@
  *
  * Licensed under the GNU/GPL. See COPYING for details.
  */
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
@@ -25,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
@@ -33,6 +36,13 @@
 #include "ehci.h"
 
 #define DRIVER_DESC "EHCI generic platform driver"
+#define EHCI_MAX_CLKS 3
+#define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv)
+
+struct ehci_platform_priv {
+	struct clk *clks[EHCI_MAX_CLKS];
+	struct phy *phy;
+};
 
 static const char hcd_name[] = "ehci-platform";
 
@@ -64,38 +74,90 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
 	return 0;
 }
 
+static int ehci_platform_power_on(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk, ret;
+
+	for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) {
+		ret = clk_prepare_enable(priv->clks[clk]);
+		if (ret)
+			goto err_disable_clks;
+	}
+
+	if (priv->phy) {
+		ret = phy_init(priv->phy);
+		if (ret)
+			goto err_disable_clks;
+
+		ret = phy_power_on(priv->phy);
+		if (ret)
+			goto err_exit_phy;
+	}
+
+	return 0;
+
+err_exit_phy:
+	phy_exit(priv->phy);
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(priv->clks[clk]);
+
+	return ret;
+}
+
+static void ehci_platform_power_off(struct platform_device *dev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk;
+
+	if (priv->phy) {
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
+	for (clk = EHCI_MAX_CLKS - 1; clk >= 0; clk--)
+		if (priv->clks[clk])
+			clk_disable_unprepare(priv->clks[clk]);
+}
+
 static struct hc_driver __read_mostly ehci_platform_hc_driver;
 
 static const struct ehci_driver_overrides platform_overrides __initconst = {
-	.reset =	ehci_platform_reset,
+	.reset =		ehci_platform_reset,
+	.extra_priv_size =	sizeof(struct ehci_platform_priv),
 };
 
-static struct usb_ehci_pdata ehci_platform_defaults;
+static struct usb_ehci_pdata ehci_platform_defaults = {
+	.power_on =		ehci_platform_power_on,
+	.power_suspend =	ehci_platform_power_off,
+	.power_off =		ehci_platform_power_off,
+};
 
 static int ehci_platform_probe(struct platform_device *dev)
 {
 	struct usb_hcd *hcd;
 	struct resource *res_mem;
-	struct usb_ehci_pdata *pdata;
-	int irq;
-	int err;
+	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ehci_platform_priv *priv;
+	int err, irq, clk = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
 
 	/*
-	 * use reasonable defaults so platforms don't have to provide these.
-	 * with DT probing on ARM, none of these are set.
+	 * Use reasonable defaults so platforms don't have to provide these
+	 * with DT probing on ARM.
 	 */
-	if (!dev_get_platdata(&dev->dev))
-		dev->dev.platform_data = &ehci_platform_defaults;
+	if (!pdata)
+		pdata = &ehci_platform_defaults;
 
 	err = dma_coerce_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
 	if (err)
 		return err;
 
-	pdata = dev_get_platdata(&dev->dev);
-
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0) {
 		dev_err(&dev->dev, "no irq provided");
@@ -107,17 +169,40 @@ static int ehci_platform_probe(struct platform_device *dev)
 		return -ENXIO;
 	}
 
+	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
+			     dev_name(&dev->dev));
+	if (!hcd)
+		return -ENOMEM;
+
+	platform_set_drvdata(dev, hcd);
+	dev->dev.platform_data = pdata;
+	priv = hcd_to_ehci_priv(hcd);
+
+	if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
+		priv->phy = devm_phy_get(&dev->dev, "usb");
+		if (IS_ERR(priv->phy)) {
+			err = PTR_ERR(priv->phy);
+			if (err == -EPROBE_DEFER)
+				goto err_put_hcd;
+			priv->phy = NULL;
+		}
+
+		for (clk = 0; clk < EHCI_MAX_CLKS; clk++) {
+			priv->clks[clk] = of_clk_get(dev->dev.of_node, clk);
+			if (IS_ERR(priv->clks[clk])) {
+				err = PTR_ERR(priv->clks[clk]);
+				if (err == -EPROBE_DEFER)
+					goto err_put_clks;
+				priv->clks[clk] = NULL;
+				break;
+			}
+		}
+	}
+
 	if (pdata->power_on) {
 		err = pdata->power_on(dev);
 		if (err < 0)
-			return err;
-	}
-
-	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
-			     dev_name(&dev->dev));
-	if (!hcd) {
-		err = -ENOMEM;
-		goto err_power;
+			goto err_put_clks;
 	}
 
 	hcd->rsrc_start = res_mem->start;
@@ -126,21 +211,25 @@ static int ehci_platform_probe(struct platform_device *dev)
 	hcd->regs = devm_ioremap_resource(&dev->dev, res_mem);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);
-		goto err_put_hcd;
+		goto err_power;
 	}
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err_put_hcd;
-
-	platform_set_drvdata(dev, hcd);
+		goto err_power;
 
 	return err;
 
-err_put_hcd:
-	usb_put_hcd(hcd);
 err_power:
 	if (pdata->power_off)
 		pdata->power_off(dev);
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(priv->clks[clk]);
+err_put_hcd:
+	if (pdata == &ehci_platform_defaults)
+		dev->dev.platform_data = NULL;
+
+	usb_put_hcd(hcd);
 
 	return err;
 }
@@ -149,13 +238,19 @@ static int ehci_platform_remove(struct platform_device *dev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(dev);
 	struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
+	struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+	int clk;
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(hcd);
 
 	if (pdata->power_off)
 		pdata->power_off(dev);
 
+	for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
+		clk_put(priv->clks[clk]);
+
+	usb_put_hcd(hcd);
+
 	if (pdata == &ehci_platform_defaults)
 		dev->dev.platform_data = NULL;
 
@@ -206,8 +301,10 @@ static int ehci_platform_resume(struct device *dev)
 static const struct of_device_id vt8500_ehci_ids[] = {
 	{ .compatible = "via,vt8500-ehci", },
 	{ .compatible = "wm,prizm-ehci", },
+	{ .compatible = "usb-ehci", },
 	{}
 };
+MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
 
 static const struct platform_device_id ehci_platform_table[] = {
 	{ "ehci-platform", 0 },
-- 
1.8.4.2

[PATCH v7 2/2] ehci-platform: Add support for clks and phy passed through devicetree

[Florian Fainelli]

2014/1/15 Hans de Goede <hdegoede@redhat.com>:
> Currently ehci-platform is only used in combination with devicetree when used
> with some Via socs. By extending it to (optionally) get clks and a phy from
> devicetree, and enabling / disabling those on power_on / off, it can be used
> more generically. Specifically after this commit it can be used for the
> ehci controller on Allwinner sunxi SoCs.
>
> Since ehci-platform is intended to handle any generic enough non pci ehci
> device, add a "usb-ehci" compatibility string.
>
> There already is a usb-ehci device-tree bindings document, update this
> with clks and phy bindings info.
>
> Although actually quite generic so far the via,vt8500 compatibilty string
> had its own bindings document. Somehow we even ended up with 2 of them. Since
> these provide no extra information over the generic usb-ehci documentation,
> this patch removes them.
>
> The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
> even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
> not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
> 2 drivers claiming the same compatibility string getting build on ppc.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---
>  Documentation/devicetree/bindings/usb/usb-ehci.txt |  25 +++-
>  .../devicetree/bindings/usb/via,vt8500-ehci.txt    |  15 ---
>  .../devicetree/bindings/usb/vt8500-ehci.txt        |  12 --
>  drivers/usb/host/Kconfig                           |   1 +
>  drivers/usb/host/ehci-platform.c                   | 149 +++++++++++++++++----
>  5 files changed, 142 insertions(+), 60 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
>  delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> index fa18612..fad10f3 100644
> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> @@ -7,13 +7,14 @@ Required properties:
>      (debug-port or other) can be also specified here, but only after
>      definition of standard EHCI registers.
>    - interrupts : one EHCI interrupt should be described here.
> -If device registers are implemented in big endian mode, the device
> -node should have "big-endian-regs" property.
> -If controller implementation operates with big endian descriptors,
> -"big-endian-desc" property should be specified.
> -If both big endian registers and descriptors are used by the controller
> -implementation, "big-endian" property can be specified instead of having
> -both "big-endian-regs" and "big-endian-desc".
> +
> +Optional properties:
> + - big-endian-regs : boolean, set this for hcds with big-endian registers
> + - big-endian-desc : boolean, set this for hcds with big-endian descriptors
> + - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc

These properties are specified here in the documentation binding, but
they are not handled by the ehci-platform driver, is that intentional?
Fairly quickly people will start adopting the ehci-platform driver on
DT-aware platforms, so it would be nice to support those 2 (3)
properties right away?
-- 
Florian

Re: [PATCH v7 2/2] ehci-platform: Add support for clks and phy passed through devicetree

[Florian Fainelli]

2014/1/15 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Currently ehci-platform is only used in combination with devicetree when used
> with some Via socs. By extending it to (optionally) get clks and a phy from
> devicetree, and enabling / disabling those on power_on / off, it can be used
> more generically. Specifically after this commit it can be used for the
> ehci controller on Allwinner sunxi SoCs.
>
> Since ehci-platform is intended to handle any generic enough non pci ehci
> device, add a "usb-ehci" compatibility string.
>
> There already is a usb-ehci device-tree bindings document, update this
> with clks and phy bindings info.
>
> Although actually quite generic so far the via,vt8500 compatibilty string
> had its own bindings document. Somehow we even ended up with 2 of them. Since
> these provide no extra information over the generic usb-ehci documentation,
> this patch removes them.
>
> The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
> even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
> not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
> 2 drivers claiming the same compatibility string getting build on ppc.
>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/usb-ehci.txt |  25 +++-
>  .../devicetree/bindings/usb/via,vt8500-ehci.txt    |  15 ---
>  .../devicetree/bindings/usb/vt8500-ehci.txt        |  12 --
>  drivers/usb/host/Kconfig                           |   1 +
>  drivers/usb/host/ehci-platform.c                   | 149 +++++++++++++++++----
>  5 files changed, 142 insertions(+), 60 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/usb/via,vt8500-ehci.txt
>  delete mode 100644 Documentation/devicetree/bindings/usb/vt8500-ehci.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> index fa18612..fad10f3 100644
> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> @@ -7,13 +7,14 @@ Required properties:
>      (debug-port or other) can be also specified here, but only after
>      definition of standard EHCI registers.
>    - interrupts : one EHCI interrupt should be described here.
> -If device registers are implemented in big endian mode, the device
> -node should have "big-endian-regs" property.
> -If controller implementation operates with big endian descriptors,
> -"big-endian-desc" property should be specified.
> -If both big endian registers and descriptors are used by the controller
> -implementation, "big-endian" property can be specified instead of having
> -both "big-endian-regs" and "big-endian-desc".
> +
> +Optional properties:
> + - big-endian-regs : boolean, set this for hcds with big-endian registers
> + - big-endian-desc : boolean, set this for hcds with big-endian descriptors
> + - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc

These properties are specified here in the documentation binding, but
they are not handled by the ehci-platform driver, is that intentional?
Fairly quickly people will start adopting the ehci-platform driver on
DT-aware platforms, so it would be nice to support those 2 (3)
properties right away?
-- 
Florian

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

On Wed, 15 Jan 2014, Hans de Goede wrote:

> Hi All,
> 
> This version of my ohci and ehci-platform clks, phy and dt support patch-set,
> really fixes the 2 small bugs Alan found.

All okay -- this time I can't find anything to complain about.  :-)

Alan Stern

Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

On Wed, 15 Jan 2014, Hans de Goede wrote:

> Hi All,
> 
> This version of my ohci and ehci-platform clks, phy and dt support patch-set,
> really fixes the 2 small bugs Alan found.

All okay -- this time I can't find anything to complain about.  :-)

Alan Stern

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Florian Fainelli]

Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a ?crit :
> On Wed, 15 Jan 2014, Hans de Goede wrote:
> > Hi All,
> > 
> > This version of my ohci and ehci-platform clks, phy and dt support
> > patch-set, really fixes the 2 small bugs Alan found.
> 
> All okay -- this time I can't find anything to complain about.  :-)

There is one minor issue; which is that the ehci binding claims the driver 
supports the following optional boolean properties:

- big-endian-regs : boolean, set this for hcds with big-endian registers
- big-endian-desc : boolean, set this for hcds with big-endian descriptors
- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc

while it does not (yet) so this is misleading. Can we at get that fixed before 
merging? Copy pasting the PPC ehci driver should do the job.
-- 
Florian

Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Florian Fainelli]

Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a écrit :
> On Wed, 15 Jan 2014, Hans de Goede wrote:
> > Hi All,
> > 
> > This version of my ohci and ehci-platform clks, phy and dt support
> > patch-set, really fixes the 2 small bugs Alan found.
> 
> All okay -- this time I can't find anything to complain about.  :-)

There is one minor issue; which is that the ehci binding claims the driver 
supports the following optional boolean properties:

- big-endian-regs : boolean, set this for hcds with big-endian registers
- big-endian-desc : boolean, set this for hcds with big-endian descriptors
- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc

while it does not (yet) so this is misleading. Can we at get that fixed before 
merging? Copy pasting the PPC ehci driver should do the job.
-- 
Florian

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

On Thu, 16 Jan 2014, Florian Fainelli wrote:

> Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a ?crit :
> > On Wed, 15 Jan 2014, Hans de Goede wrote:
> > > Hi All,
> > > 
> > > This version of my ohci and ehci-platform clks, phy and dt support
> > > patch-set, really fixes the 2 small bugs Alan found.
> > 
> > All okay -- this time I can't find anything to complain about.  :-)
> 
> There is one minor issue; which is that the ehci binding claims the driver 
> supports the following optional boolean properties:
> 
> - big-endian-regs : boolean, set this for hcds with big-endian registers
> - big-endian-desc : boolean, set this for hcds with big-endian descriptors
> - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
> 
> while it does not (yet) so this is misleading. Can we at get that fixed before 
> merging? Copy pasting the PPC ehci driver should do the job.

I agree it needs to be fixed.  This can be done in a follow-up patch,
though.  It doesn't have to be in this one because nobody is using
ehci-platform with DT yet.

A simple copy from the PPC driver isn't quite enough, because the
platform data settings would override the DT values.  
ehci_platform_reset has to be changed so that it sets
ehci->big_endian_desc and _mmio if the pdata flags are set, but
otherwise leaves them alone.

Hans, would you like to write another patch to take care of this?

Alan Stern

Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1665 bytes --]

On Thu, 16 Jan 2014, Florian Fainelli wrote:

> Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a écrit :
> > On Wed, 15 Jan 2014, Hans de Goede wrote:
> > > Hi All,
> > > 
> > > This version of my ohci and ehci-platform clks, phy and dt support
> > > patch-set, really fixes the 2 small bugs Alan found.
> > 
> > All okay -- this time I can't find anything to complain about.  :-)
> 
> There is one minor issue; which is that the ehci binding claims the driver 
> supports the following optional boolean properties:
> 
> - big-endian-regs : boolean, set this for hcds with big-endian registers
> - big-endian-desc : boolean, set this for hcds with big-endian descriptors
> - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
> 
> while it does not (yet) so this is misleading. Can we at get that fixed before 
> merging? Copy pasting the PPC ehci driver should do the job.

I agree it needs to be fixed.  This can be done in a follow-up patch,
though.  It doesn't have to be in this one because nobody is using
ehci-platform with DT yet.

A simple copy from the PPC driver isn't quite enough, because the
platform data settings would override the DT values.  
ehci_platform_reset has to be changed so that it sets
ehci->big_endian_desc and _mmio if the pdata flags are set, but
otherwise leaves them alone.

Hans, would you like to write another patch to take care of this?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Hans de Goede]

Hi,

On 01/17/2014 06:12 PM, Alan Stern wrote:
> On Thu, 16 Jan 2014, Florian Fainelli wrote:
>
>> Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a ?crit :
>>> On Wed, 15 Jan 2014, Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> This version of my ohci and ehci-platform clks, phy and dt support
>>>> patch-set, really fixes the 2 small bugs Alan found.
>>>
>>> All okay -- this time I can't find anything to complain about.  :-)
>>
>> There is one minor issue; which is that the ehci binding claims the driver
>> supports the following optional boolean properties:
>>
>> - big-endian-regs : boolean, set this for hcds with big-endian registers
>> - big-endian-desc : boolean, set this for hcds with big-endian descriptors
>> - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
>>
>> while it does not (yet) so this is misleading. Can we at get that fixed before
>> merging? Copy pasting the PPC ehci driver should do the job.
>
> I agree it needs to be fixed.  This can be done in a follow-up patch,
> though.  It doesn't have to be in this one because nobody is using
> ehci-platform with DT yet.
>
> A simple copy from the PPC driver isn't quite enough, because the
> platform data settings would override the DT values.
> ehci_platform_reset has to be changed so that it sets
> ehci->big_endian_desc and _mmio if the pdata flags are set, but
> otherwise leaves them alone.
>
> Hans, would you like to write another patch to take care of this?

I already took a quick look at this, setting the ehci->big_endian* flags is easy,
the problem is that they won't work unless we also define
CONFIG_USB_EHCI_BIG_ENDIAN_DESC and CONFIG_USB_EHCI_BIG_ENDIAN_MMIO

I would like to avoid doing something like the ugliness we've with
USB_OHCI_HCD_PPC_OF_BE and USB_OHCI_HCD_PPC_OF_LE, which means just selecting these
2 config options whenever USB_EHCI_HCD_PLATFORM is enabled, but I'm not sure if that
is acceptable.

The same goes for the ohci code btw.

Regards,

Hans

Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Hans de Goede]

Hi,

On 01/17/2014 06:12 PM, Alan Stern wrote:
> On Thu, 16 Jan 2014, Florian Fainelli wrote:
>
>> Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a �crit :
>>> On Wed, 15 Jan 2014, Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> This version of my ohci and ehci-platform clks, phy and dt support
>>>> patch-set, really fixes the 2 small bugs Alan found.
>>>
>>> All okay -- this time I can't find anything to complain about.  :-)
>>
>> There is one minor issue; which is that the ehci binding claims the driver
>> supports the following optional boolean properties:
>>
>> - big-endian-regs : boolean, set this for hcds with big-endian registers
>> - big-endian-desc : boolean, set this for hcds with big-endian descriptors
>> - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
>>
>> while it does not (yet) so this is misleading. Can we at get that fixed before
>> merging? Copy pasting the PPC ehci driver should do the job.
>
> I agree it needs to be fixed.  This can be done in a follow-up patch,
> though.  It doesn't have to be in this one because nobody is using
> ehci-platform with DT yet.
>
> A simple copy from the PPC driver isn't quite enough, because the
> platform data settings would override the DT values.
> ehci_platform_reset has to be changed so that it sets
> ehci->big_endian_desc and _mmio if the pdata flags are set, but
> otherwise leaves them alone.
>
> Hans, would you like to write another patch to take care of this?

I already took a quick look at this, setting the ehci->big_endian* flags is easy,
the problem is that they won't work unless we also define
CONFIG_USB_EHCI_BIG_ENDIAN_DESC and CONFIG_USB_EHCI_BIG_ENDIAN_MMIO

I would like to avoid doing something like the ugliness we've with
USB_OHCI_HCD_PPC_OF_BE and USB_OHCI_HCD_PPC_OF_LE, which means just selecting these
2 config options whenever USB_EHCI_HCD_PLATFORM is enabled, but I'm not sure if that
is acceptable.

The same goes for the ohci code btw.

Regards,

Hans

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

On Sat, 18 Jan 2014, Hans de Goede wrote:

> > A simple copy from the PPC driver isn't quite enough, because the
> > platform data settings would override the DT values.
> > ehci_platform_reset has to be changed so that it sets
> > ehci->big_endian_desc and _mmio if the pdata flags are set, but
> > otherwise leaves them alone.
> >
> > Hans, would you like to write another patch to take care of this?
> 
> I already took a quick look at this, setting the ehci->big_endian* flags is easy,
> the problem is that they won't work unless we also define
> CONFIG_USB_EHCI_BIG_ENDIAN_DESC and CONFIG_USB_EHCI_BIG_ENDIAN_MMIO

That's right.  The platforms have to define those symbols if they use 
big-endian values (or even worse, mixed endian).

> I would like to avoid doing something like the ugliness we've with
> USB_OHCI_HCD_PPC_OF_BE and USB_OHCI_HCD_PPC_OF_LE, which means just selecting these
> 2 config options whenever USB_EHCI_HCD_PLATFORM is enabled, but I'm not sure if that
> is acceptable.

At the moment, I don't think it's so terrible.  That ohci_ppc ugliness
probably could have been avoided originally.  It looks like an attempt
to make sure you don't end up matching the hardware to a driver built
with the wrong endianness.  I imagine it would be less messy to detect 
that sort of problem at runtime.

> The same goes for the ohci code btw.

Certainly.

Alan Stern

Re: [PATCH v7 0/2] ohci and ehci-platform clks, phy and dt support

[Alan Stern]

On Sat, 18 Jan 2014, Hans de Goede wrote:

> > A simple copy from the PPC driver isn't quite enough, because the
> > platform data settings would override the DT values.
> > ehci_platform_reset has to be changed so that it sets
> > ehci->big_endian_desc and _mmio if the pdata flags are set, but
> > otherwise leaves them alone.
> >
> > Hans, would you like to write another patch to take care of this?
> 
> I already took a quick look at this, setting the ehci->big_endian* flags is easy,
> the problem is that they won't work unless we also define
> CONFIG_USB_EHCI_BIG_ENDIAN_DESC and CONFIG_USB_EHCI_BIG_ENDIAN_MMIO

That's right.  The platforms have to define those symbols if they use 
big-endian values (or even worse, mixed endian).

> I would like to avoid doing something like the ugliness we've with
> USB_OHCI_HCD_PPC_OF_BE and USB_OHCI_HCD_PPC_OF_LE, which means just selecting these
> 2 config options whenever USB_EHCI_HCD_PLATFORM is enabled, but I'm not sure if that
> is acceptable.

At the moment, I don't think it's so terrible.  That ohci_ppc ugliness
probably could have been avoided originally.  It looks like an attempt
to make sure you don't end up matching the hardware to a driver built
with the wrong endianness.  I imagine it would be less messy to detect 
that sort of problem at runtime.

> The same goes for the ohci code btw.

Certainly.

Alan Stern