[PATCH 1/2] usb: chipidea: usbmisc: skip clocks on i.MX6

[PATCH 1/2] usb: chipidea: usbmisc: skip clocks on i.MX6

[Philipp Zabel]

On i.MX6Q, the USBMISC registers are clocked by the IPG clock,
so there is no need to enable USBOH3 for the USBMISC driver.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 926c997..f7b878d 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -338,23 +338,27 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(data->base))
 		return PTR_ERR(data->base);
 
-	data->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(data->clk)) {
-		dev_err(&pdev->dev,
-			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
-		return PTR_ERR(data->clk);
-	}
-
-	ret = clk_prepare_enable(data->clk);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"clk_prepare_enable failed, err=%d\n", ret);
-		return ret;
-	}
-
 	tmp_dev = (struct of_device_id *)
 		of_match_device(usbmisc_imx_dt_ids, &pdev->dev);
 	data->ops = (const struct usbmisc_ops *)tmp_dev->data;
+
+	/* on i.MX6Q, the USBMISC register space is clocked by the IPG clock */
+	if (data->ops != &imx6q_usbmisc_ops) {
+		data->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(data->clk)) {
+			dev_err(&pdev->dev, "failed to get clock, err=%ld\n",
+				PTR_ERR(data->clk));
+			return PTR_ERR(data->clk);
+		}
+
+		ret = clk_prepare_enable(data->clk);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"clk_prepare_enable failed, err=%d\n", ret);
+			return ret;
+		}
+	}
+
 	platform_set_drvdata(pdev, data);
 
 	return 0;
-- 
2.1.3

[PATCH 2/2] ARM: dts: imx6qdl: remove USBOH3 clock from usbmisc, it is clocked by IPG

[Philipp Zabel]

On i.MX6Q, the USBMISC registers are clocked by the IPG clock,
so there is no need to enable USBOH3 for the USBMISC driver.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 28e6878..ff3edb0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -896,7 +896,6 @@
 				#index-cells = <1>;
 				compatible = "fsl,imx6q-usbmisc";
 				reg = <0x02184800 0x200>;
-				clocks = <&clks IMX6QDL_CLK_USBOH3>;
 			};
 
 			fec: ethernet at 02188000 {
-- 
2.1.3

[PATCH 1/2] usb: chipidea: usbmisc: skip clocks on i.MX6

[Peter Chen]

On Tue, Dec 09, 2014 at 05:13:35PM +0100, Philipp Zabel wrote:
> On i.MX6Q, the USBMISC registers are clocked by the IPG clock,
> so there is no need to enable USBOH3 for the USBMISC driver.
> 

Access the registers at usbmisc needs the same clock with chipidea core,
both are usboh3.

For i.mx6 series, there is only one usb clock gate at ccm which is
usboh3, it is used to access register. For transfer data, it needs
usb phy's (using mxs phy) clock is enabled.

> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 926c997..f7b878d 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -338,23 +338,27 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>  	if (IS_ERR(data->base))
>  		return PTR_ERR(data->base);
>  
> -	data->clk = devm_clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(data->clk)) {
> -		dev_err(&pdev->dev,
> -			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
> -		return PTR_ERR(data->clk);
> -	}
> -
> -	ret = clk_prepare_enable(data->clk);
> -	if (ret) {
> -		dev_err(&pdev->dev,
> -			"clk_prepare_enable failed, err=%d\n", ret);
> -		return ret;
> -	}
> -
>  	tmp_dev = (struct of_device_id *)
>  		of_match_device(usbmisc_imx_dt_ids, &pdev->dev);
>  	data->ops = (const struct usbmisc_ops *)tmp_dev->data;
> +
> +	/* on i.MX6Q, the USBMISC register space is clocked by the IPG clock */
> +	if (data->ops != &imx6q_usbmisc_ops) {
> +		data->clk = devm_clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(data->clk)) {
> +			dev_err(&pdev->dev, "failed to get clock, err=%ld\n",
> +				PTR_ERR(data->clk));
> +			return PTR_ERR(data->clk);
> +		}
> +
> +		ret = clk_prepare_enable(data->clk);
> +		if (ret) {
> +			dev_err(&pdev->dev,
> +				"clk_prepare_enable failed, err=%d\n", ret);
> +			return ret;
> +		}
> +	}
> +
>  	platform_set_drvdata(pdev, data);
>  
>  	return 0;
> -- 
> 2.1.3
> 

-- 

Best Regards,
Peter Chen

[PATCH 1/2] usb: chipidea: usbmisc: skip clocks on i.MX6

[Philipp Zabel]

Hi Peter,

Am Mittwoch, den 10.12.2014, 13:09 +0800 schrieb Peter Chen:
> On Tue, Dec 09, 2014 at 05:13:35PM +0100, Philipp Zabel wrote:
> > On i.MX6Q, the USBMISC registers are clocked by the IPG clock,
> > so there is no need to enable USBOH3 for the USBMISC driver.
> 
> Access the registers at usbmisc needs the same clock with chipidea core,
> both are usboh3.

Alright, I'll drop this patch.

It was my understanding that the usbmisc register space is clocked by
ipg_clk_s, which is not gated by usboh3_clk_enable:
Having a look at the "i.MX 6Dual/6Quad Applications Processor Reference
Manual, Rev. 1, 04/2013", Figure 65-1 "USB block diagram" shows the USB
Control Register (usbmisc) directly connected via "IP Bus1" separately
from the host cores (these are connected behind a PL301).
In chapter 18.4 "System Clocks", the table entry for the block instance
"USB" shows that the usboh3_clk_enable bit gates only the ipg_ahb_clk,
ipg_clk_s_pl301, test_clk_240m, test_clk_480m, and test_clk_60m inputs
to the USB block. The ipg_clk_s (from ipg_clk_root) and ipg_clk_32khz
inputs are not marked as gated. Still, it seems that indeed the usbmisc
region is inaccessible after gating usboh3 by unbinding the usb drivers.

regards
Philipp

[PATCH 1/2] usb: chipidea: usbmisc: skip clocks on i.MX6

[Uwe Kleine-König]

Hello Philipp,

On Tue, Dec 09, 2014 at 05:13:35PM +0100, Philipp Zabel wrote:
> +	/* on i.MX6Q, the USBMISC register space is clocked by the IPG clock */
> +	if (data->ops != &imx6q_usbmisc_ops) {
> +		data->clk = devm_clk_get(&pdev->dev, NULL);
I wonder if it was better/simpler to handle devm_clk_get returning
-ENOENT and assume this means there is no need for a clock in this case.

(I think devm_clk_get prints an error message in this case. That's ugly
when the error is handled just fine.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |