diff mbox

[v2,5/7] usb: chipidea: usbmisc: add support for ahb, ipg and per clock

Message ID 1352981028-14312-6-git-send-email-m.grzeschik@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Grzeschik Nov. 15, 2012, 12:03 p.m. UTC
From: Marc Kleine-Budde <mkl@pengutronix.de>

This patch adds support for a second and third clock to the usbmisc driver. On
modern freescale ARM cores like the imx51, imx53 and imx6q three clocks ("ahb",
"ipg" and "per") must be enabled in order to access the USB core.

ahb - AMBA High-Performance Bus clock domain
ipg - IP-Bus Gate clock domain
per - Peripheral clock domain

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
Changes since v1:
* changed patch description

 drivers/usb/chipidea/usbmisc_imx.c |   60 +++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 15 deletions(-)

Comments

Alexander Shishkin Nov. 16, 2012, 1:17 p.m. UTC | #1
Michael Grzeschik <m.grzeschik@pengutronix.de> writes:

> From: Marc Kleine-Budde <mkl@pengutronix.de>
>
> This patch adds support for a second and third clock to the usbmisc driver. On
> modern freescale ARM cores like the imx51, imx53 and imx6q three clocks ("ahb",
> "ipg" and "per") must be enabled in order to access the USB core.
>
> ahb - AMBA High-Performance Bus clock domain
> ipg - IP-Bus Gate clock domain
> per - Peripheral clock domain

Looks like usbmisc_imx and ci13xxx_imx are growing more and more common
code.
Btw, shouldn't there be a way to specify platform's clock tree
configuration via devicetree or whatnot so that each platform driver
didn't have to request platform specific clocks?

Regards,
--
Alex
Sascha Hauer Nov. 20, 2012, 3:09 p.m. UTC | #2
On Fri, Nov 16, 2012 at 03:17:47PM +0200, Alexander Shishkin wrote:
> Michael Grzeschik <m.grzeschik@pengutronix.de> writes:
> 
> > From: Marc Kleine-Budde <mkl@pengutronix.de>
> >
> > This patch adds support for a second and third clock to the usbmisc driver. On
> > modern freescale ARM cores like the imx51, imx53 and imx6q three clocks ("ahb",
> > "ipg" and "per") must be enabled in order to access the USB core.
> >
> > ahb - AMBA High-Performance Bus clock domain
> > ipg - IP-Bus Gate clock domain
> > per - Peripheral clock domain
> 
> Looks like usbmisc_imx and ci13xxx_imx are growing more and more common
> code.
> Btw, shouldn't there be a way to specify platform's clock tree
> configuration via devicetree or whatnot so that each platform driver
> didn't have to request platform specific clocks?

In theory that is the case. The clocks should be modeled after the input
clocks of the device (ci13xxx). Every SoC should have them, but on some
SoCs some of them may not be software controllable, so they have to
provide dummy clocks for these.
So when everyting is done correctly in the driver, the calls to clk_get
could be in the core instead of ci13xxx_imx. That said, we do not have
good insights into the SoC, so we do not know exactly which clock inputs
the ci13xx has and where they are connected in the SoCs clock module.
There are some guesses in there.

Sascha
diff mbox

Patch

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 4b1ed7c..913e414 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -24,7 +24,9 @@ 
 struct imx_usbmisc {
 	void __iomem *base;
 	spinlock_t lock;
-	struct clk *clk;
+	struct clk *clk_ahb;
+	struct clk *clk_ipg;
+	struct clk *clk_per;
 	struct usbmisc_usb_device usbdev[USB_DEV_MAX];
 	const struct usbmisc_ops *ops;
 };
@@ -104,38 +106,66 @@  static int __devinit usbmisc_imx_probe(struct platform_device *pdev)
 	if (!data->base)
 		return -EADDRNOTAVAIL;
 
-	data->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(data->clk)) {
+	data->clk_ahb = devm_clk_get(&pdev->dev, "ahb");
+	if (IS_ERR(data->clk_ahb)) {
 		dev_err(&pdev->dev,
-			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
-		return PTR_ERR(data->clk);
+			"failed to get ahb clock, err=%ld\n", PTR_ERR(data->clk_ahb));
+		return PTR_ERR(data->clk_ahb);
 	}
 
-	ret = clk_prepare_enable(data->clk);
-	if (ret) {
+	data->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
+	if (IS_ERR(data->clk_ipg)) {
 		dev_err(&pdev->dev,
-			"clk_prepare_enable failed, err=%d\n", ret);
-		return ret;
+			"failed to get ipg clock, err=%ld\n", PTR_ERR(data->clk_ipg));
+		return PTR_ERR(data->clk_ipg);
 	}
 
+	data->clk_per = devm_clk_get(&pdev->dev, "per");
+	if (IS_ERR(data->clk_per)) {
+		dev_err(&pdev->dev,
+			"failed to get per clock, err=%ld\n", PTR_ERR(data->clk_per));
+		return PTR_ERR(data->clk_per);
+	}
+
+	ret = clk_prepare_enable(data->clk_ahb);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(data->clk_ipg);
+	if (ret)
+		goto err_ipg_failed;
+
+	ret = clk_prepare_enable(data->clk_per);
+	if (ret)
+		goto err_per_failed;
+
 	tmp_dev = (struct of_device_id *)
 		of_match_device(usbmisc_imx_dt_ids, &pdev->dev);
 	data->ops = (const struct usbmisc_ops *)tmp_dev->data;
 	usbmisc = data;
 	ret = usbmisc_set_ops(data->ops);
-	if (ret) {
-		usbmisc = NULL;
-		clk_disable_unprepare(data->clk);
-		return ret;
-	}
+	if (ret)
+		goto err_set_ops_failed;
 
 	return 0;
+
+ err_set_ops_failed:
+	usbmisc = NULL;
+	clk_disable_unprepare(data->clk_per);
+ err_per_failed:
+	clk_disable_unprepare(data->clk_ipg);
+ err_ipg_failed:
+	clk_disable_unprepare(data->clk_ahb);
+
+	return ret;
 }
 
 static int __devexit usbmisc_imx_remove(struct platform_device *pdev)
 {
 	usbmisc_unset_ops(usbmisc->ops);
-	clk_disable_unprepare(usbmisc->clk);
+	clk_disable_unprepare(usbmisc->clk_per);
+	clk_disable_unprepare(usbmisc->clk_ipg);
+	clk_disable_unprepare(usbmisc->clk_ahb);
 	usbmisc = NULL;
 	return 0;
 }