[PATCH 1/4] doc: dt-binding: ci-hdrc-usb2: split vendor specific properties

[PATCH 1/4] doc: dt-binding: ci-hdrc-usb2: split vendor specific properties

[Peter Chen]

Each vendor may have its specific properties, they are not belonged
to common optional properties, split them from common's.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index d543e8e..f15a317 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -26,10 +26,6 @@ Optional properties:
 - vbus-supply: reference to the VBUS regulator
 - maximum-speed: limit the maximum connection speed to "full-speed".
 - tpl-support: TPL (Targeted Peripheral List) feature for targeted hosts
-- fsl,usbmisc: (FSL only) phandler of non-core register device, with one
-  argument that indicate usb controller index
-- disable-over-current: (FSL only) disable over current detect
-- external-vbus-divider: (FSL only) enables off-chip resistor divider for Vbus
 - itc-setting: interrupt threshold control register control, the setting
   should be aligned with ITC bits at register USBCMD.
 - ahb-burst-config: it is vendor dependent, the required value should be
@@ -53,6 +49,12 @@ Optional properties:
 - phy-clkgate-delay-us: the delay time (us) between putting the PHY into
   low power mode and gating the PHY clock.
 
+i.mx specific properties
+- fsl,usbmisc: phandler of non-core register device, with one
+  argument that indicate usb controller index
+- disable-over-current: disable over current detect
+- external-vbus-divider: enables off-chip resistor divider for Vbus
+
 Example:
 
 	usb@f7ed0000 {
-- 
1.9.1

--
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 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Peter Chen]

Some SoCs needs three clock to let controller work, but others only
need one, add one property to differentiate this.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index f15a317..4900092 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -54,6 +54,9 @@ i.mx specific properties
   argument that indicate usb controller index
 - disable-over-current: disable over current detect
 - external-vbus-divider: enables off-chip resistor divider for Vbus
+- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
+  needs three clcoks for controller, others only need one. Without this
+  property, the driver will consider this controller only need one clock.
 
 Example:
 
-- 
1.9.1

--
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 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Peter Chen]

For imx27, it needs three clocks to let the controller work,
the old code is wrong, and will cause below data abort when
accass usbmisc registers.

usbcore: registered new interface driver usb-storage
Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
pgd = c0004000
[f4424600] *pgd=10000452(bad)
Internal error: : 8 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
Hardware name: Freescale i.MX27 (Device Tree Support)
task: c7832b60 ti: c783e000 task.ti: c783e000
PC is at usbmisc_imx27_init+0x4c/0xbc
LR is at usbmisc_imx27_init+0x40/0xbc
pc : [<c03cb5c0>]    lr : [<c03cb5b4>]    psr: 60000093
sp : c783fe08  ip : 00000000  fp : 00000000
r10: c0576434  r9 : 0000009c  r8 : c7a773a0
r7 : 01000000  r6 : 60000013  r5 : c7a776f0  r4 : c7a773f0
r3 : f4424600  r2 : 00000000  r1 : 00000001  r0 : 00000001
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: a0004000  DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc783e190)
Stack: (0xc783fe08 to 0xc7840000)

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Reported-by: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 arch/arm/boot/dts/imx27.dtsi | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index feb9d34..93e3e14 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -486,7 +486,11 @@
 				compatible = "fsl,imx27-usb";
 				reg = <0x10024000 0x200>;
 				interrupts = <56>;
-				clocks = <&clks IMX27_CLK_USB_IPG_GATE>;
+				need-three-clocks;
+				clocks = <&clks IMX27_CLK_USB_IPG_GATE>,
+					<&clks IMX27_CLK_USB_AHB_GATE>,
+					<&clks IMX27_CLK_USB_DIV>;
+				clock-names = "ipg", "ahb", "per";
 				fsl,usbmisc = <&usbmisc 0>;
 				status = "disabled";
 			};
@@ -495,7 +499,11 @@
 				compatible = "fsl,imx27-usb";
 				reg = <0x10024200 0x200>;
 				interrupts = <54>;
-				clocks = <&clks IMX27_CLK_USB_IPG_GATE>;
+				need-three-clocks;
+				clocks = <&clks IMX27_CLK_USB_IPG_GATE>,
+					<&clks IMX27_CLK_USB_AHB_GATE>,
+					<&clks IMX27_CLK_USB_DIV>;
+				clock-names = "ipg", "ahb", "per";
 				fsl,usbmisc = <&usbmisc 1>;
 				dr_mode = "host";
 				status = "disabled";
@@ -505,7 +513,11 @@
 				compatible = "fsl,imx27-usb";
 				reg = <0x10024400 0x200>;
 				interrupts = <55>;
-				clocks = <&clks IMX27_CLK_USB_IPG_GATE>;
+				need-three-clocks;
+				clocks = <&clks IMX27_CLK_USB_IPG_GATE>,
+					<&clks IMX27_CLK_USB_AHB_GATE>,
+					<&clks IMX27_CLK_USB_DIV>;
+				clock-names = "ipg", "ahb", "per";
 				fsl,usbmisc = <&usbmisc 2>;
 				dr_mode = "host";
 				status = "disabled";
@@ -515,7 +527,6 @@
 				#index-cells = <1>;
 				compatible = "fsl,imx27-usbmisc";
 				reg = <0x10024600 0x200>;
-				clocks = <&clks IMX27_CLK_USB_AHB_GATE>;
 			};
 
 			sahara2: sahara@10025000 {
-- 
1.9.1

--
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 4/4] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Peter Chen]

Some i.mx platforms need three clocks to let controller work, but
others only need one, refine clock operation to adapt for all
platforms.

Cc: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 138 ++++++++++++++++++++++++++++++++-----
 1 file changed, 120 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index c038bca..e88a212 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -78,6 +78,12 @@ struct ci_hdrc_imx_data {
 	struct imx_usbmisc_data *usbmisc_data;
 	bool supports_runtime_pm;
 	bool in_lpm;
+	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
+	bool need_three_clks;
+	struct clk *clk_ipg;
+	struct clk *clk_ahb;
+	struct clk *clk_per;
+	/* --------------------------------- */
 };
 
 /* Common functions shared by usbmisc drivers */
@@ -129,6 +135,105 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 }
 
 /* End of common functions shared by usbmisc drivers*/
+static int imx_get_clks(struct device *dev)
+{
+	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (data->need_three_clks) {
+		data->clk_ipg = devm_clk_get(dev, "ipg");
+		if (IS_ERR(data->clk_ipg)) {
+			ret = PTR_ERR(data->clk_ipg);
+			dev_err(dev,
+				"Failed to get ipg clock, err=%d\n", ret);
+			return ret;
+		}
+
+		data->clk_ahb = devm_clk_get(dev, "ahb");
+		if (IS_ERR(data->clk_ahb)) {
+			ret = PTR_ERR(data->clk_ahb);
+			dev_err(dev,
+				"Failed to get ahb clock, err=%d\n", ret);
+			return ret;
+		}
+
+		data->clk_per = devm_clk_get(dev, "per");
+		if (IS_ERR(data->clk_per)) {
+			ret = PTR_ERR(data->clk_per);
+			dev_err(dev,
+				"Failed to get per clock, err=%d\n", ret);
+			return ret;
+		}
+	} else {
+		data->clk = devm_clk_get(dev, NULL);
+		if (IS_ERR(data->clk)) {
+			ret = PTR_ERR(data->clk);
+			dev_err(dev,
+				"Failed to get clock, err=%d\n", ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static int imx_prepare_enable_clks(struct device *dev)
+{
+	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (data->need_three_clks) {
+		ret = clk_prepare_enable(data->clk_ipg);
+		if (ret) {
+			dev_err(dev,
+				"Failed to prepare/enable ipg clk, err=%d\n",
+				ret);
+			return ret;
+		}
+
+		ret = clk_prepare_enable(data->clk_ahb);
+		if (ret) {
+			dev_err(dev,
+				"Failed to prepare/enable ahb clk, err=%d\n",
+				ret);
+			clk_disable_unprepare(data->clk_ipg);
+			return ret;
+		}
+
+		ret = clk_prepare_enable(data->clk_per);
+		if (ret) {
+			dev_err(dev,
+				"Failed to prepare/enable per clk, err=%d\n",
+				ret);
+			clk_disable_unprepare(data->clk_ahb);
+			clk_disable_unprepare(data->clk_ipg);
+			return ret;
+		}
+	} else {
+		ret = clk_prepare_enable(data->clk);
+		if (ret) {
+			dev_err(dev,
+				"Failed to prepare/enable clk, err=%d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static void imx_disable_unprepare_clks(struct device *dev)
+{
+	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+
+	if (data->need_three_clks) {
+		clk_disable_unprepare(data->clk_per);
+		clk_disable_unprepare(data->clk_ipg);
+		clk_disable_unprepare(data->clk_ahb);
+	} else {
+		clk_disable_unprepare(data->clk);
+	}
+}
 
 static int ci_hdrc_imx_probe(struct platform_device *pdev)
 {
@@ -142,28 +247,27 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id =
 			of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
 	const struct ci_hdrc_imx_platform_flag *imx_platform_flag = of_id->data;
+	struct device_node *np = pdev->dev.of_node;
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, data);
 	data->usbmisc_data = usbmisc_get_init_data(&pdev->dev);
 	if (IS_ERR(data->usbmisc_data))
 		return PTR_ERR(data->usbmisc_data);
 
-	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);
-	}
+	if (of_find_property(np, "need-three-clocks", NULL))
+		data->need_three_clks = true;
 
-	ret = clk_prepare_enable(data->clk);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"Failed to prepare or enable clock, err=%d\n", ret);
+	ret = imx_get_clks(&pdev->dev);
+	if (ret)
+		return ret;
+
+	ret = imx_prepare_enable_clks(&pdev->dev);
+	if (ret)
 		return ret;
-	}
 
 	data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
 	if (IS_ERR(data->phy)) {
@@ -206,8 +310,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 		goto disable_device;
 	}
 
-	platform_set_drvdata(pdev, data);
-
 	if (data->supports_runtime_pm) {
 		pm_runtime_set_active(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
@@ -220,7 +322,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 disable_device:
 	ci_hdrc_remove_device(data->ci_pdev);
 err_clk:
-	clk_disable_unprepare(data->clk);
+	imx_disable_unprepare_clks(&pdev->dev);
 	return ret;
 }
 
@@ -234,7 +336,7 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
 		pm_runtime_put_noidle(&pdev->dev);
 	}
 	ci_hdrc_remove_device(data->ci_pdev);
-	clk_disable_unprepare(data->clk);
+	imx_disable_unprepare_clks(&pdev->dev);
 
 	return 0;
 }
@@ -246,7 +348,7 @@ static int imx_controller_suspend(struct device *dev)
 
 	dev_dbg(dev, "at %s\n", __func__);
 
-	clk_disable_unprepare(data->clk);
+	imx_disable_unprepare_clks(dev);
 	data->in_lpm = true;
 
 	return 0;
@@ -264,7 +366,7 @@ static int imx_controller_resume(struct device *dev)
 		return 0;
 	}
 
-	ret = clk_prepare_enable(data->clk);
+	ret = imx_prepare_enable_clks(dev);
 	if (ret)
 		return ret;
 
@@ -279,7 +381,7 @@ static int imx_controller_resume(struct device *dev)
 	return 0;
 
 clk_disable:
-	clk_disable_unprepare(data->clk);
+	imx_disable_unprepare_clks(dev);
 	return ret;
 }
 
-- 
1.9.1

--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Peter Chen]

On Wed, Sep 16, 2015 at 12:23:26AM -0300, Fabio Estevam wrote:
> On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > Some SoCs needs three clock to let controller work, but others only
> > need one, add one property to differentiate this.
> >
> > Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index f15a317..4900092 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -54,6 +54,9 @@ i.mx specific properties
> >    argument that indicate usb controller index
> >  - disable-over-current: disable over current detect
> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
> > +  needs three clcoks for controller, others only need one. Without this
> > +  property, the driver will consider this controller only need one clock.
> 
> Looking at mx31, mx35, mx25 reference manuals we see that they don't
> really need three usb clocks.
> 
> Also, if we look at the old drivers/usb/host/ehci-mxc.c we only
> require 'ipg' and 'ahb' clocks.
> 

but drivers/usb/gadget/udc/fsl_mxc_udc.c has three clocks

> So it would be better to just require these two clocks and if they are
> not found on dts, then we fall back to requesting clk_get(NULL),
> without the need of an extra property.

I considered before, but if it has clk_ipg, but without clk_ahb, do we
consider it is an error or not ? Using extra property can make things
cleaner.

-- 

Best Regards,
Peter Chen
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Peter Chen]

On Wed, Sep 16, 2015 at 12:25:17AM -0300, Fabio Estevam wrote:
> On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > For imx27, it needs three clocks to let the controller work,
> > the old code is wrong, and will cause below data abort when
> > accass usbmisc registers.
> >
> > usbcore: registered new interface driver usb-storage
> > Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> > pgd = c0004000
> > [f4424600] *pgd=10000452(bad)
> > Internal error: : 8 [#1] PREEMPT ARM
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> > Hardware name: Freescale i.MX27 (Device Tree Support)
> > task: c7832b60 ti: c783e000 task.ti: c783e000
> > PC is at usbmisc_imx27_init+0x4c/0xbc
> > LR is at usbmisc_imx27_init+0x40/0xbc
> > pc : [<c03cb5c0>]    lr : [<c03cb5b4>]    psr: 60000093
> > sp : c783fe08  ip : 00000000  fp : 00000000
> > r10: c0576434  r9 : 0000009c  r8 : c7a773a0
> > r7 : 01000000  r6 : 60000013  r5 : c7a776f0  r4 : c7a773f0
> > r3 : f4424600  r2 : 00000000  r1 : 00000001  r0 : 00000001
> > Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> > Control: 0005317f  Table: a0004000  DAC: 00000017
> > Process swapper (pid: 1, stack limit = 0xc783e190)
> > Stack: (0xc783fe08 to 0xc7840000)
> >
> > Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Reported-by: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx27.dtsi | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
> > index feb9d34..93e3e14 100644
> > --- a/arch/arm/boot/dts/imx27.dtsi
> > +++ b/arch/arm/boot/dts/imx27.dtsi
> 
> mx25.dtsi and mx35.dtsi also need to be changed.
> 

If you can help to test, I will update.

-- 

Best Regards,
Peter Chen
--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Some SoCs needs three clock to let controller work, but others only
> need one, add one property to differentiate this.
>
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index f15a317..4900092 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -54,6 +54,9 @@ i.mx specific properties
>    argument that indicate usb controller index
>  - disable-over-current: disable over current detect
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
> +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
> +  needs three clcoks for controller, others only need one. Without this
> +  property, the driver will consider this controller only need one clock.

Looking at mx31, mx35, mx25 reference manuals we see that they don't
really need three usb clocks.

Also, if we look at the old drivers/usb/host/ehci-mxc.c we only
require 'ipg' and 'ahb' clocks.

So it would be better to just require these two clocks and if they are
not found on dts, then we fall back to requesting clk_get(NULL),
without the need of an extra property.
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen@freescale.com> wrote:
> For imx27, it needs three clocks to let the controller work,
> the old code is wrong, and will cause below data abort when
> accass usbmisc registers.
>
> usbcore: registered new interface driver usb-storage
> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> pgd = c0004000
> [f4424600] *pgd=10000452(bad)
> Internal error: : 8 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> Hardware name: Freescale i.MX27 (Device Tree Support)
> task: c7832b60 ti: c783e000 task.ti: c783e000
> PC is at usbmisc_imx27_init+0x4c/0xbc
> LR is at usbmisc_imx27_init+0x40/0xbc
> pc : [<c03cb5c0>]    lr : [<c03cb5b4>]    psr: 60000093
> sp : c783fe08  ip : 00000000  fp : 00000000
> r10: c0576434  r9 : 0000009c  r8 : c7a773a0
> r7 : 01000000  r6 : 60000013  r5 : c7a776f0  r4 : c7a773f0
> r3 : f4424600  r2 : 00000000  r1 : 00000001  r0 : 00000001
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 00000017
> Process swapper (pid: 1, stack limit = 0xc783e190)
> Stack: (0xc783fe08 to 0xc7840000)
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> Reported-by: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: <stable@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx27.dtsi | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
> index feb9d34..93e3e14 100644
> --- a/arch/arm/boot/dts/imx27.dtsi
> +++ b/arch/arm/boot/dts/imx27.dtsi

mx25.dtsi and mx35.dtsi also need to be changed.

> @@ -486,7 +486,11 @@
>                                 compatible = "fsl,imx27-usb";
>                                 reg = <0x10024000 0x200>;
>                                 interrupts = <56>;
> -                               clocks = <&clks IMX27_CLK_USB_IPG_GATE>;
> +                               need-three-clocks;
> +                               clocks = <&clks IMX27_CLK_USB_IPG_GATE>,
> +                                       <&clks IMX27_CLK_USB_AHB_GATE>,
> +                                       <&clks IMX27_CLK_USB_DIV>;
> +                               clock-names = "ipg", "ahb", "per";

'ipg' and 'ahb' are enough.

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Fabio Estevam]

On Tue, Sep 15, 2015 at 11:24 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:

> but drivers/usb/gadget/udc/fsl_mxc_udc.c has three clocks

Ok, looking at the clock driver I see that they used to associate
three clocks with mxc-ehci:

    clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
    clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.0");
    clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.0");

So your approach looks good.

I only think that mx25 and mx35 dtsi should be also updated to have
the 'three-clock' property.

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

Re: [PATCH 4/4] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms.
>
> Cc: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Please Cc stable on this patch as it fixes a regression.
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 11:31 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:

>> mx25.dtsi and mx35.dtsi also need to be changed.
>
> If you can help to test, I will update.

Sure, I can test it on mx25pdk tomorrow.
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen@freescale.com> wrote:
> For imx27, it needs three clocks to let the controller work,
> the old code is wrong, and will cause below data abort when
> accass usbmisc registers.

s/acass/accessing

>
> usbcore: registered new interface driver usb-storage
> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> pgd = c0004000
> [f4424600] *pgd=10000452(bad)
> Internal error: : 8 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> Hardware name: Freescale i.MX27 (Device Tree Support)
> task: c7832b60 ti: c783e000 task.ti: c783e000
> PC is at usbmisc_imx27_init+0x4c/0xbc
> LR is at usbmisc_imx27_init+0x40/0xbc
> pc : [<c03cb5c0>]    lr : [<c03cb5b4>]    psr: 60000093
> sp : c783fe08  ip : 00000000  fp : 00000000
> r10: c0576434  r9 : 0000009c  r8 : c7a773a0
> r7 : 01000000  r6 : 60000013  r5 : c7a776f0  r4 : c7a773f0
> r3 : f4424600  r2 : 00000000  r1 : 00000001  r0 : 00000001
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 00000017
> Process swapper (pid: 1, stack limit = 0xc783e190)
> Stack: (0xc783fe08 to 0xc7840000)
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> Reported-by: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: <stable@vger.kernel.org>

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

Re: [PATCH 4/4] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms.

It would be better to mention that this is fixing a regression on mx27.

>
> Cc: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Tested-by: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Rob Herring]

On 09/15/2015 08:49 PM, Peter Chen wrote:
> Some SoCs needs three clock to let controller work, but others only
> need one, add one property to differentiate this.

A given licensed IP block is going to have the same number of clock
inputs from SOC to SOC. So different numbers of clocks is a bit suspect.
I guess there can be variations in bus clocks or other outside logic.

> 
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index f15a317..4900092 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -54,6 +54,9 @@ i.mx specific properties
>    argument that indicate usb controller index
>  - disable-over-current: disable over current detect
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
> +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
> +  needs three clcoks for controller, others only need one. Without this
> +  property, the driver will consider this controller only need one clock.

That's pretty ugly and unnecessary. Either use the compatible string to
determine if you have 3 clocks or just always try to retrieve the 3
clocks in the driver and fall back to 1.

Rob

--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Peter Chen]

On Wed, Sep 16, 2015 at 08:54:25AM -0500, Rob Herring wrote:
> On 09/15/2015 08:49 PM, Peter Chen wrote:
> > Some SoCs needs three clock to let controller work, but others only
> > need one, add one property to differentiate this.
> 
> A given licensed IP block is going to have the same number of clock
> inputs from SOC to SOC.

Yes, I agree with you.

> So different numbers of clocks is a bit suspect.
> I guess there can be variations in bus clocks or other outside logic.

There are legacy platforms, no one knows exactly what the
other clocks are used for, and the former i.mx usb driver
(see drivers/usb/gadget/udc/fsl_mxc_udc.c) uses like this way.
So, I just follow it.

> 
> > 
> > Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index f15a317..4900092 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -54,6 +54,9 @@ i.mx specific properties
> >    argument that indicate usb controller index
> >  - disable-over-current: disable over current detect
> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
> > +  needs three clcoks for controller, others only need one. Without this
> > +  property, the driver will consider this controller only need one clock.
> 
> That's pretty ugly and unnecessary. Either use the compatible string to
> determine

Since there are too many SoCs to use it, I just didn't want to add
judgement for platforms, and thought using feature property is simpler.
If you doesn't agree, I will use other ways.

> if you have 3 clocks or just always try to retrieve the 3
> clocks in the driver and fall back to 1.
> 

It is not easy to use this way, one-clock platforms have no dev_id for
clock, but three-clock platforms have.

-- 

Best Regards,
Peter Chen
--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Rob Herring]

On Thu, Sep 17, 2015 at 9:35 PM, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> On Wed, Sep 16, 2015 at 08:54:25AM -0500, Rob Herring wrote:
>> On 09/15/2015 08:49 PM, Peter Chen wrote:
>> > Some SoCs needs three clock to let controller work, but others only
>> > need one, add one property to differentiate this.

[...]

>> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>> > index f15a317..4900092 100644
>> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>> > @@ -54,6 +54,9 @@ i.mx specific properties
>> >    argument that indicate usb controller index
>> >  - disable-over-current: disable over current detect
>> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
>> > +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
>> > +  needs three clcoks for controller, others only need one. Without this
>> > +  property, the driver will consider this controller only need one clock.
>>
>> That's pretty ugly and unnecessary. Either use the compatible string to
>> determine
>
> Since there are too many SoCs to use it, I just didn't want to add
> judgement for platforms, and thought using feature property is simpler.
> If you doesn't agree, I will use other ways.
>
>> if you have 3 clocks or just always try to retrieve the 3
>> clocks in the driver and fall back to 1.
>>
>
> It is not easy to use this way, one-clock platforms have no dev_id for
> clock, but three-clock platforms have.

You just need to try to retrieve the 3rd clock by name. If that
succeeds, retrieve clocks 1 and 2. If you can't retrieve the 3rd
clock, the just get the 1st clock with no name.

Rob
--
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