ARM: dts: mx25: USB block requires only one clock

This converts the Chipidea usbmisc driver to
use the standard usb-phy property.

drivers/usb/chipidea/ci_hdrc_imx.c +0 -0

index c00f772..9a74100 100644

Hi,

Quoted 3 lines · Denis Carikli

This converts the Chipidea usbmisc driver to
use the standard usb-phy property.

[...]

Quoted 8 lines

drivers/usb/chipidea/ci_hdrc_imx.c +0 -0

index c00f772..9a74100 100644
@@ -130,7 +130,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 		return ret;
 	}
 

drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 136

Quoted 6 lines

drivers/usb/chipidea/ci_hdrc_imx.c +1 -1

-	data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+	data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
 	if (IS_ERR(data->phy)) {
 		ret = PTR_ERR(data->phy);
 		goto err_clk;

How about accepting the old property too and print a warning, that it is deprecated, rather than breaking all existing DT blobs?

It also adapt the dts that uses it.

Signed-off-by Denis Carikli <[email protected]>

---
 arch/arm/boot/dts/imx23.dtsi       |    2 +-
 arch/arm/boot/dts/imx27.dtsi       |    4 ++--
 arch/arm/boot/dts/imx28.dtsi       |    4 ++--
 arch/arm/boot/dts/imx51.dtsi       |    2 +-
 arch/arm/boot/dts/imx53.dtsi       |    4 ++--
 arch/arm/boot/dts/imx6qdl.dtsi     |    4 ++--
 arch/arm/boot/dts/imx6sl.dtsi      |    4 ++--
 drivers/usb/chipidea/ci_hdrc_imx.c |    2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

arch/arm/boot/dts/imx23.dtsi +1 -1

index bbcfb5a..e3c9924 100644
@@ -522,7 +522,7 @@
 			compatible = "fsl,imx23-usb", "fsl,imx27-usb";
 			reg = <0x80080000 0x40000>;
 			interrupts = <11>;
-			fsl,usbphy = <&usbphy0>;
+			usb-phy = <&usbphy0>;
 			clocks = <&clks 40>;
 			status = "disabled";
 		};

arch/arm/boot/dts/imx27.dtsi +2 -2

index 83a8247..94bcf1a 100644
@@ -466,7 +466,7 @@
 				interrupts = <56>;
 				clocks = <&clks 15>;
 				fsl,usbmisc = <&usbmisc 0>;
-				fsl,usbphy = <&usbphy0>;
+				usb-phy = <&usbphy0>;
 				status = "disabled";
 			};
 

@@ -485,7 +485,7 @@
 				interrupts = <55>;
 				clocks = <&clks 15>;
 				fsl,usbmisc = <&usbmisc 2>;
-				fsl,usbphy = <&usbphy2>;
+				usb-phy = <&usbphy2>;
 				status = "disabled";
 			};
 

arch/arm/boot/dts/imx28.dtsi +2 -2

index 90a5795..52ad72f 100644
@@ -1143,7 +1143,7 @@
 			reg = <0x80080000 0x10000>;
 			interrupts = <93>;
 			clocks = <&clks 60>;
-			fsl,usbphy = <&usbphy0>;
+			usb-phy = <&usbphy0>;
 			status = "disabled";
 		};
 

@@ -1152,7 +1152,7 @@
 			reg = <0x80090000 0x10000>;
 			interrupts = <92>;
 			clocks = <&clks 61>;
-			fsl,usbphy = <&usbphy1>;
+			usb-phy = <&usbphy1>;
 			status = "disabled";
 		};
 

arch/arm/boot/dts/imx51.dtsi +1 -1

index cb3204a..b290947 100644
@@ -249,7 +249,7 @@
 				interrupts = <18>;
 				clocks = <&clks IMX5_CLK_USBOH3_GATE>;
 				fsl,usbmisc = <&usbmisc 0>;
-				fsl,usbphy = <&usbphy0>;
+				usb-phy = <&usbphy0>;
 				status = "disabled";
 			};
 

arch/arm/boot/dts/imx53.dtsi +2 -2

index f0962e5..b10a3de 100644
@@ -276,7 +276,7 @@
 				interrupts = <18>;
 				clocks = <&clks IMX5_CLK_USBOH3_GATE>;
 				fsl,usbmisc = <&usbmisc 0>;
-				fsl,usbphy = <&usbphy0>;
+				usb-phy = <&usbphy0>;
 				status = "disabled";
 			};
 

@@ -286,7 +286,7 @@
 				interrupts = <14>;
 				clocks = <&clks IMX5_CLK_USBOH3_GATE>;
 				fsl,usbmisc = <&usbmisc 1>;
-				fsl,usbphy = <&usbphy1>;
+				usb-phy = <&usbphy1>;
 				status = "disabled";
 			};
 

(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index cfc85be..d2f0087 100644

arch/arm/boot/dts/imx6qdl.dtsi +2 -2

@@ -789,7 +789,7 @@
 				reg = <0x02184000 0x200>;
 				interrupts = <0 43 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks 162>;
-				fsl,usbphy = <&usbphy1>;
+				usb-phy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
 				status = "disabled";
 			};

@@ -799,7 +799,7 @@
 				reg = <0x02184200 0x200>;
 				interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks 162>;
-				fsl,usbphy = <&usbphy2>;
+				usb-phy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
 				status = "disabled";
 			};

(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6sl.dtsi index 6477447..33f501a 100644

arch/arm/boot/dts/imx6sl.dtsi +2 -2

@@ -651,7 +651,7 @@
 				reg = <0x02184000 0x200>;
 				interrupts = <0 43 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_USBOH3>;
-				fsl,usbphy = <&usbphy1>;
+				usb-phy = <&usbphy1>;
 				fsl,usbmisc = <&usbmisc 0>;
 				status = "disabled";
 			};

@@ -661,7 +661,7 @@
 				reg = <0x02184200 0x200>;
 				interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_USBOH3>;
-				fsl,usbphy = <&usbphy2>;
+				usb-phy = <&usbphy2>;
 				fsl,usbmisc = <&usbmisc 1>;
 				status = "disabled";
 			};

drivers/usb/chipidea/ci_hdrc_imx.c +1 -1

index c00f772..9a74100 100644
@@ -130,7 +130,7 @@ static int ci_hdrc_imx_probe(struct platform_device
 *pdev)
 		return ret;
 	}
 
-	data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy",
 0);
+	data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
 	if (IS_ERR(data->phy)) {

drivers/usb/chipidea/ci_hdrc_imx.c +0 -1

 		ret = PTR_ERR(data->phy);
 		goto err_clk;
--

1.7.9.5

Quoted 3 lines

It also adapt the dts that uses it.

Signed-off-by Denis Carikli <[email protected]>

According to Power_ePAPR_APPROVED_v1.1.pdf
"ethernet-phy" is the node name, it is the same with Sergei's suggestion.

But you have changed the property name at this patch, in fact, "fsl,usbphy" is ok for property name, "fsl,usb-phy" may be better, but not worth a patch to change it.

Peter

Quoted 5 lines

According to Power_ePAPR_APPROVED_v1.1.pdf "ethernet-phy" is the node name, it is the same with Sergei's suggestion.

Nobody's talking about "ehternet-phy" here.

Quoted 1 line

But you have changed the property name at this patch, in fact,

"fsl,usbphy"

Quoted 2 lines

is ok for property name, "fsl,usb-phy" may be better, but not worth a patch to change it.

No, it's not better and both are not OK because they're still
Freescale specific while the de-facto standard property "usb-phy" is already used by other USB drivers for the same purpose.

Quoted 4 lines

Quoted 2 lines

According to Power_ePAPR_APPROVED_v1.1.pdf "ethernet-phy" is the node name, it is the same with Sergei's suggestion.

Nobody's talking about "ehternet-phy" here.

Hmm, you take "ethernet-phy" as an example, and suggest changing to "usb-phy" at last email.

Quoted 8 lines

Quoted 1 line

But you have changed the property name at this patch, in fact,

"fsl,usbphy"

Quoted 2 lines

is ok for property name, "fsl,usb-phy" may be better, but not worth a patch to change it.

No, it's not better and both are not OK because they're still
Freescale specific while the de-facto standard property "usb-phy" is already used by other USB drivers for the same purpose.

Am I missing something, property name should be the same for all platforms?

Peter

This adds the i.MX25 and the i.MX35 support in the ChipIdea usbmisc driver.

The i.MX25 and i.MX35 usb controllers are similar enough to be able to use the same code.

Signed-off-by Denis Carikli <[email protected]>

---
Changelog v2->v3:
- Add a commit log

Changelog v1->v2:
- converted two remaining defines to BIT()
- Removed a variable declaration that was not used in usbmisc_imx25_init

---
 drivers/usb/chipidea/usbmisc_imx.c |   58
++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

drivers/usb/chipidea/usbmisc_imx.c +0 -0

index cd061ab..3523c09 100644

drivers/usb/chipidea/usbmisc_imx.c +37 -0

+#define MX25_OTG_SIC_SHIFT		29
+#define MX25_OTG_SIC_MASK		(0x3 << MX25_OTG_SIC_SHIFT)
+#define MX25_OTG_PM_BIT			BIT(24)
+#define MX25_OTG_PP_BIT			BIT(11)
+#define MX25_OTG_OCPOL_BIT		BIT(3)
+
+#define MX25_H1_SIC_SHIFT		21
+#define MX25_H1_SIC_MASK		(0x3 << MX25_H1_SIC_SHIFT)
+#define MX25_H1_PP_BIT			BIT(18)
+#define MX25_H1_PM_BIT			BIT(16)
+#define MX25_H1_IPPUE_UP_BIT		BIT(7)
+#define MX25_H1_IPPUE_DOWN_BIT		BIT(6)
+#define MX25_H1_TLL_BIT			BIT(5)
+#define MX25_H1_USBTE_BIT		BIT(4)
+#define MX25_H1_OCPOL_BIT		BIT(2)
+
 #define MX27_H1_PM_BIT			BIT(8)
 #define MX27_H2_PM_BIT			BIT(16)
 #define MX27_OTG_PM_BIT			BIT(24)

@@ -50,6 +70,39 @@ struct imx_usbmisc {
 
 static struct imx_usbmisc *usbmisc;
 
+static int usbmisc_imx25_init(struct imx_usbmisc_data *data) {
+	unsigned long flags;
+	u32 val = 0;
+
+	if (data->index > 1)
+		return -EINVAL;
+
+	spin_lock_irqsave(&usbmisc->lock, flags);
+	switch (data->index) {
+	case 0:
+		val = readl(usbmisc->base);
+		val &= ~(MX25_OTG_SIC_MASK | MX25_OTG_PP_BIT);
+		val |= (MXC_EHCI_INTERFACE_DIFF_UNI & MXC_EHCI_INTERFACE_MASK)
 << MX25_OTG_SIC_SHIFT;
+		val |= (MX25_OTG_PM_BIT | MX25_OTG_OCPOL_BIT);
+		writel(val, usbmisc->base);
+		break;
+	case 1:
+		val = readl(usbmisc->base);
+		val &= ~(MX25_H1_SIC_MASK | MX25_H1_PP_BIT |
 MX25_H1_IPPUE_UP_BIT);
+		val |= (MXC_EHCI_INTERFACE_SINGLE_UNI &
 MXC_EHCI_INTERFACE_MASK) << MX25_H1_SIC_SHIFT;
+		val |= (MX25_H1_PM_BIT | MX25_H1_OCPOL_BIT | MX25_H1_TLL_BIT

|

drivers/usb/chipidea/usbmisc_imx.c +16 -0

+			MX25_H1_USBTE_BIT | MX25_H1_IPPUE_DOWN_BIT);
+
+		writel(val, usbmisc->base);
+
+		break;
+	}
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+	return 0;
+}
+
 static int usbmisc_imx25_post(struct imx_usbmisc_data *data)  {
 	void __iomem *reg;

@@ -159,6 +212,7 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data
 *data)  }
 
 static const struct usbmisc_ops imx25_usbmisc_ops = {
+	.init = usbmisc_imx25_init,
 	.post = usbmisc_imx25_post,
 };
 

@@ -200,6 +254,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[]
 = {
 		.data = &imx25_usbmisc_ops,
 	},
 	{
+		.compatible = "fsl,imx35-usbmisc",
+		.data = &imx25_usbmisc_ops,
+	},
+	{
 		.compatible = "fsl,imx27-usbmisc",
 		.data = &imx27_usbmisc_ops,

},
--
1.7.9.5

From: Fabio Estevam <[email protected]>

This patch was adapted from the thread named
"USB Host support for mx25" on linux-usb at vger.kernel.org

Hi Denis,

As you add me in the From field, you also need to add:

Signed-off-by Fabio Estevam <[email protected]> above your

Signed-off-by line.

arch/arm/boot/dts/imx25.dtsi line 546

Quoted 4 lines

arch/arm/boot/dts/imx25.dtsi +4 -0

+       usbphy {
+               #address-cells = <1>;
+               #size-cells = <0>;
+               compatible = "simple-bus";

I made this comment earlier: why do we place usbphy0/1 under simple-bus?

ci_hdrc ci_hdrc.0: no usb2 phy configured platform ci_hdrc.0: Driver ci_hdrc requests probe deferral ci_hdrc ci_hdrc.1: no usb2 phy configured platform ci_hdrc.1: Driver ci_hdrc requests probe deferral

This is not documented in the the bindings.