[PATCH v5 1/8] dt-bindings: usb: usbmisc-imx: add fsl,imx8ulp-usbmisc compatible

[PATCH v5 1/8] dt-bindings: usb: usbmisc-imx: add fsl,imx8ulp-usbmisc compatible

[Xu Yang]

Add "fsl,imx8ulp-usbmisc" compatible.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v3:
 - new patch due to missed this little one
Changes in v4:
 - no changes
Changes in v5:
 - add Acked-by tag
---
 Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
index 2d3589d284b2..0a6e7ac1b37e 100644
--- a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
+++ b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
@@ -33,6 +33,7 @@ properties:
               - fsl,imx7ulp-usbmisc
               - fsl,imx8mm-usbmisc
               - fsl,imx8mn-usbmisc
+              - fsl,imx8ulp-usbmisc
           - const: fsl,imx7d-usbmisc
           - const: fsl,imx6q-usbmisc
       - items:
-- 
2.34.1

[PATCH v5 2/8] arm64: dts: imx8ulp: add usb nodes

[Xu Yang]

Add USB nodes on i.MX8ULP platform which has 2 USB controllers.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - no changes
Changes in v3:
 - no changes
Changes in v4:
 - no changes
Changes in v5:
 - no changes
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index c4a0082f30d3..ead1f57e08ef 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -28,6 +28,8 @@ aliases {
 		serial1 = &lpuart5;
 		serial2 = &lpuart6;
 		serial3 = &lpuart7;
+		usbphy0 = &usbphy1;
+		usbphy1 = &usbphy2;
 	};
 
 	cpus {
@@ -472,6 +474,68 @@ usdhc2: mmc@298f0000 {
 				status = "disabled";
 			};
 
+			usbotg1: usb@29900000 {
+				compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
+				reg = <0x29900000 0x200>;
+				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc4 IMX8ULP_CLK_USB0>;
+				power-domains = <&scmi_devpd IMX8ULP_PD_USB0>;
+				phys = <&usbphy1>;
+				fsl,usbmisc = <&usbmisc1 0>;
+				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x8>;
+				rx-burst-size-dword = <0x8>;
+				status = "disabled";
+			};
+
+			usbmisc1: usbmisc@29900200 {
+				compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
+						"fsl,imx6q-usbmisc";
+				#index-cells = <1>;
+				reg = <0x29900200 0x200>;
+				status = "disabled";
+			};
+
+			usbphy1: usb-phy@29910000 {
+				compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
+				reg = <0x29910000 0x10000>;
+				interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc4 IMX8ULP_CLK_USB0_PHY>;
+				#phy-cells = <0>;
+				status = "disabled";
+			};
+
+			usbotg2: usb@29920000 {
+				compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
+				reg = <0x29920000 0x200>;
+				interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc4 IMX8ULP_CLK_USB1>;
+				power-domains = <&scmi_devpd IMX8ULP_PD_USDHC2_USB1>;
+				phys = <&usbphy2>;
+				fsl,usbmisc = <&usbmisc2 0>;
+				ahb-burst-config = <0x0>;
+				tx-burst-size-dword = <0x8>;
+				rx-burst-size-dword = <0x8>;
+				status = "disabled";
+			};
+
+			usbmisc2: usbmisc@29920200 {
+				compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
+						"fsl,imx6q-usbmisc";
+				#index-cells = <1>;
+				reg = <0x29920200 0x200>;
+				status = "disabled";
+			};
+
+			usbphy2: usb-phy@29930000 {
+				compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
+				reg = <0x29930000 0x10000>;
+				interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc4 IMX8ULP_CLK_USB1_PHY>;
+				#phy-cells = <0>;
+				status = "disabled";
+			};
+
 			fec: ethernet@29950000 {
 				compatible = "fsl,imx8ulp-fec", "fsl,imx6ul-fec", "fsl,imx6q-fec";
 				reg = <0x29950000 0x10000>;
-- 
2.34.1

Re: [PATCH v5 2/8] arm64: dts: imx8ulp: add usb nodes

[Rob Herring]

On Wed, Jan 31, 2024 at 07:43:18PM +0800, Xu Yang wrote:
> Add USB nodes on i.MX8ULP platform which has 2 USB controllers.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - no changes
> Changes in v3:
>  - no changes
> Changes in v4:
>  - no changes
> Changes in v5:
>  - no changes
> ---
>  arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index c4a0082f30d3..ead1f57e08ef 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -28,6 +28,8 @@ aliases {
>  		serial1 = &lpuart5;
>  		serial2 = &lpuart6;
>  		serial3 = &lpuart7;
> +		usbphy0 = &usbphy1;
> +		usbphy1 = &usbphy2;

Drop these. We have no use for aliases to phys.

>  	};
>  
>  	cpus {
> @@ -472,6 +474,68 @@ usdhc2: mmc@298f0000 {
>  				status = "disabled";
>  			};
>  
> +			usbotg1: usb@29900000 {
> +				compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
> +				reg = <0x29900000 0x200>;
> +				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc4 IMX8ULP_CLK_USB0>;
> +				power-domains = <&scmi_devpd IMX8ULP_PD_USB0>;
> +				phys = <&usbphy1>;
> +				fsl,usbmisc = <&usbmisc1 0>;
> +				ahb-burst-config = <0x0>;
> +				tx-burst-size-dword = <0x8>;
> +				rx-burst-size-dword = <0x8>;
> +				status = "disabled";
> +			};
> +
> +			usbmisc1: usbmisc@29900200 {
> +				compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
> +						"fsl,imx6q-usbmisc";
> +				#index-cells = <1>;

Deprecated. Why are you using this?

Rob

RE: [EXT] Re: [PATCH v5 2/8] arm64: dts: imx8ulp: add usb nodes

[Xu Yang]

Hi Rob,

> 
> On Wed, Jan 31, 2024 at 07:43:18PM +0800, Xu Yang wrote:
> > Add USB nodes on i.MX8ULP platform which has 2 USB controllers.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> >  - no changes
> > Changes in v3:
> >  - no changes
> > Changes in v4:
> >  - no changes
> > Changes in v5:
> >  - no changes
> > ---
> >  arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 64 ++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> > index c4a0082f30d3..ead1f57e08ef 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> > @@ -28,6 +28,8 @@ aliases {
> >               serial1 = &lpuart5;
> >               serial2 = &lpuart6;
> >               serial3 = &lpuart7;
> > +             usbphy0 = &usbphy1;
> > +             usbphy1 = &usbphy2;
> 
> Drop these. We have no use for aliases to phys.

Checked again. Yes, these two alias can be removed.

> 
> >       };
> >
> >       cpus {
> > @@ -472,6 +474,68 @@ usdhc2: mmc@298f0000 {
> >                               status = "disabled";
> >                       };
> >
> > +                     usbotg1: usb@29900000 {
> > +                             compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
> > +                             reg = <0x29900000 0x200>;
> > +                             interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > +                             clocks = <&pcc4 IMX8ULP_CLK_USB0>;
> > +                             power-domains = <&scmi_devpd IMX8ULP_PD_USB0>;
> > +                             phys = <&usbphy1>;
> > +                             fsl,usbmisc = <&usbmisc1 0>;
> > +                             ahb-burst-config = <0x0>;
> > +                             tx-burst-size-dword = <0x8>;
> > +                             rx-burst-size-dword = <0x8>;
> > +                             status = "disabled";
> > +                     };
> > +
> > +                     usbmisc1: usbmisc@29900200 {
> > +                             compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
> > +                                             "fsl,imx6q-usbmisc";
> > +                             #index-cells = <1>;
> 
> Deprecated. Why are you using this?

Do you mean "#index-cells"? 
We are using it all the time.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/tree/drivers/usb/chipidea/ci_hdrc_imx.c?h=linux-6.7.y#n137

Thanks,
Xu Yang

[PATCH v5 3/8] arm64: dts: imx8ulp-evk: enable usb nodes and add ptn5150 nodes

[Xu Yang]

Enable 2 USB nodes and add 2 PTN5150 nodes on i.MX8ULP evk board.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - fix format as suggusted by Fabio
 - add PTN5150 nodes
Changes in v3:
 - no changes
Changes in v4:
 - no changes
Changes in v5:
 - no changes
---
 arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
index 69dd8e31027c..bf418af31039 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
@@ -133,6 +133,64 @@ pcal6408: gpio@21 {
 		gpio-controller;
 		#gpio-cells = <2>;
 	};
+
+	ptn5150_1: typec@1d {
+		compatible = "nxp,ptn5150";
+		reg = <0x1d>;
+		int-gpios = <&gpiof 3 IRQ_TYPE_EDGE_FALLING>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec1>;
+		status = "disabled";
+	};
+
+	ptn5150_2: typec@3d {
+		compatible = "nxp,ptn5150";
+		reg = <0x3d>;
+		int-gpios = <&gpiof 5 IRQ_TYPE_EDGE_FALLING>;
+			pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec2>;
+		status = "disabled";
+	};
+};
+
+&usbotg1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usb1>;
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	over-current-active-low;
+	status = "okay";
+};
+
+&usbphy1 {
+	fsl,tx-d-cal = <110>;
+	status = "okay";
+};
+
+&usbmisc1 {
+	status = "okay";
+};
+
+&usbotg2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usb2>;
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	over-current-active-low;
+	status = "okay";
+};
+
+&usbphy2 {
+	fsl,tx-d-cal = <110>;
+	status = "okay";
+};
+
+&usbmisc2 {
+	status = "okay";
 };
 
 &usdhc0 {
@@ -224,6 +282,32 @@ MX8ULP_PAD_PTE13__LPI2C7_SDA	0x20
 		>;
 	};
 
+	pinctrl_typec1: typec1grp {
+		fsl,pins = <
+			MX8ULP_PAD_PTF3__PTF3           0x3
+		>;
+	};
+
+	pinctrl_typec2: typec2grp {
+		fsl,pins = <
+			MX8ULP_PAD_PTF5__PTF5           0x3
+		>;
+	};
+
+	pinctrl_usb1: usb1grp {
+		fsl,pins = <
+			MX8ULP_PAD_PTF2__USB0_ID	0x10003
+			MX8ULP_PAD_PTF4__USB0_OC	0x10003
+		>;
+	};
+
+	pinctrl_usb2: usb2grp {
+		fsl,pins = <
+			MX8ULP_PAD_PTD23__USB1_ID	0x10003
+			MX8ULP_PAD_PTF6__USB1_OC	0x10003
+		>;
+	};
+
 	pinctrl_usdhc0: usdhc0grp {
 		fsl,pins = <
 			MX8ULP_PAD_PTD1__SDHC0_CMD	0x3
-- 
2.34.1

[PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Xu Yang]

Change reg, interrupts, clock and clock-names as common properties and add
restrictions on them for different compatibles.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v4:
 - new patch since v3's discussion
 - split the reg, interrupts, clock and clock-names properties into
   common part and device-specific
Changes in v5:
 - keep common property unchanged
 - make if-then more readable
 - remove non imx part
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 3b56e0edb1c6..6ad3582051b8 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -412,6 +412,124 @@ allOf:
         samsung,picophy-pre-emp-curr-control: false
         samsung,picophy-dc-vol-level-adjust: false
 
+  - if:
+      properties:
+        compatible:
+          const: fsl,imx27-usb
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          minItems: 3
+          maxItems: 3
+          items:
+            anyOf:
+              - const: ipg
+              - const: ahb
+              - const: per
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx25-usb
+              - fsl,imx35-usb
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          minItems: 3
+          maxItems: 3
+          items:
+            anyOf:
+              - const: ipg
+              - const: ahb
+              - const: per
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ci-hdrc
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        interrupts:
+          minItems: 1
+          maxItems: 2
+        clocks:
+          minItems: 2
+          maxItems: 3
+        clock-names:
+          minItems: 2
+          maxItems: 3
+          items:
+            anyOf:
+              - const: core
+              - const: iface
+              - const: fs
+                description: optional
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - chipidea,usb2
+              - fsl,imx23-usb
+              - fsl,imx28-usb
+              - fsl,imx7d-usb
+              - fsl,vf610-usb
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 1
+        clock-names:
+          minItems: 1
+          maxItems: 1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            oneOf:
+              - pattern: '^fsl,imx5[0-3]+-usb$'
+              - pattern: '^fsl,imx6[a-z]+-usb$'
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 1
+        clock-names:
+          minItems: 1
+          maxItems: 1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - chipidea,usb2
+              - fsl,imx27-usb
+              - fsl,imx6ul-usb
+    then:
+      properties:
+        reg:
+          minItems: 1
+          maxItems: 1
+        interrupts:
+          minItems: 1
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Krzysztof Kozlowski]

On 31/01/2024 12:43, Xu Yang wrote:
> Change reg, interrupts, clock and clock-names as common properties and add
> restrictions on them for different compatibles.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v4:
>  - new patch since v3's discussion
>  - split the reg, interrupts, clock and clock-names properties into
>    common part and device-specific
> Changes in v5:
>  - keep common property unchanged
>  - make if-then more readable
>  - remove non imx part
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 3b56e0edb1c6..6ad3582051b8 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -412,6 +412,124 @@ allOf:
>          samsung,picophy-pre-emp-curr-control: false
>          samsung,picophy-dc-vol-level-adjust: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,imx27-usb
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +          maxItems: 3
> +        clock-names:
> +          minItems: 3
> +          maxItems: 3
> +          items:
> +            anyOf:
> +              - const: ipg
> +              - const: ahb
> +              - const: per

This would be just: enum: [ipg, ahb, per], but in both cases I question
why the order should be flexible? Nothing in commit msg explains it.

Plus I will repeat myself from your v4. I don't think this is helping,
because the file will soon grow to umnanageable chunk. I prefer to fix
it at beginning, before we reach snps-schema level of complexities.

Please define common schema, reference in this file and move IMX to own
file.

In any case, please provide short comment in the code why you have such
duplicated if:then:

Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Xu Yang]

Hi Krzysztof,

> 
> On 31/01/2024 12:43, Xu Yang wrote:
> > Change reg, interrupts, clock and clock-names as common properties and add
> > restrictions on them for different compatibles.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v4:
> >  - new patch since v3's discussion
> >  - split the reg, interrupts, clock and clock-names properties into
> >    common part and device-specific
> > Changes in v5:
> >  - keep common property unchanged
> >  - make if-then more readable
> >  - remove non imx part
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> hdrc-usb2.yaml
> > index 3b56e0edb1c6..6ad3582051b8 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > @@ -412,6 +412,124 @@ allOf:
> >          samsung,picophy-pre-emp-curr-control: false
> >          samsung,picophy-dc-vol-level-adjust: false
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: fsl,imx27-usb
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> > +          maxItems: 3
> > +        clock-names:
> > +          minItems: 3
> > +          maxItems: 3
> > +          items:
> > +            anyOf:
> > +              - const: ipg
> > +              - const: ahb
> > +              - const: per
> 
> This would be just: enum: [ipg, ahb, per], but in both cases I question
> why the order should be flexible? Nothing in commit msg explains it.

The driver will get the clock by clock-name, then the order should not
matter. However, these three clock-names should be present at the same
time. I should use enum then.

> 
> Plus I will repeat myself from your v4. I don't think this is helping,
> because the file will soon grow to umnanageable chunk. I prefer to fix
> it at beginning, before we reach snps-schema level of complexities.
> 
> Please define common schema, reference in this file and move IMX to own
> file.

I'm not that familiar with dt-bindings architecture. If I define a common
schema, then should I create imx, qcom, nvidia and other dt-binding files
too? 

> 
> In any case, please provide short comment in the code why you have such
> duplicated if:then:

Okay.

> 
> Best regards,
> Krzysztof

Re: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Krzysztof Kozlowski]

On 02/02/2024 10:10, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 31/01/2024 12:43, Xu Yang wrote:
>>> Change reg, interrupts, clock and clock-names as common properties and add
>>> restrictions on them for different compatibles.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>>
>>> ---
>>> Changes in v4:
>>>  - new patch since v3's discussion
>>>  - split the reg, interrupts, clock and clock-names properties into
>>>    common part and device-specific
>>> Changes in v5:
>>>  - keep common property unchanged
>>>  - make if-then more readable
>>>  - remove non imx part
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
>>>  1 file changed, 118 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index 3b56e0edb1c6..6ad3582051b8 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -412,6 +412,124 @@ allOf:
>>>          samsung,picophy-pre-emp-curr-control: false
>>>          samsung,picophy-dc-vol-level-adjust: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          const: fsl,imx27-usb
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +        clock-names:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +          items:
>>> +            anyOf:
>>> +              - const: ipg
>>> +              - const: ahb
>>> +              - const: per
>>
>> This would be just: enum: [ipg, ahb, per], but in both cases I question
>> why the order should be flexible? Nothing in commit msg explains it.
> 
> The driver will get the clock by clock-name, then the order should not
> matter. However, these three clock-names should be present at the same
> time. I should use enum then.
> 
>>
>> Plus I will repeat myself from your v4. I don't think this is helping,
>> because the file will soon grow to umnanageable chunk. I prefer to fix
>> it at beginning, before we reach snps-schema level of complexities.
>>
>> Please define common schema, reference in this file and move IMX to own
>> file.
> 
> I'm not that familiar with dt-bindings architecture. If I define a common
> schema, then should I create imx, qcom, nvidia and other dt-binding files
> too? 

No, the rest you can leave here. Someone, maybe me, will move them some
time. The point is to move at least IMX to its own file.

Best regards,
Krzysztof

Re: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Rob Herring]

On Fri, Feb 02, 2024 at 12:04:51PM +0100, Krzysztof Kozlowski wrote:
> On 02/02/2024 10:10, Xu Yang wrote:
> > Hi Krzysztof,
> > 
> >>
> >> On 31/01/2024 12:43, Xu Yang wrote:
> >>> Change reg, interrupts, clock and clock-names as common properties and add
> >>> restrictions on them for different compatibles.
> >>>
> >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >>>
> >>> ---
> >>> Changes in v4:
> >>>  - new patch since v3's discussion
> >>>  - split the reg, interrupts, clock and clock-names properties into
> >>>    common part and device-specific
> >>> Changes in v5:
> >>>  - keep common property unchanged
> >>>  - make if-then more readable
> >>>  - remove non imx part
> >>> ---
> >>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
> >>>  1 file changed, 118 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> >> hdrc-usb2.yaml
> >>> index 3b56e0edb1c6..6ad3582051b8 100644
> >>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> @@ -412,6 +412,124 @@ allOf:
> >>>          samsung,picophy-pre-emp-curr-control: false
> >>>          samsung,picophy-dc-vol-level-adjust: false
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          const: fsl,imx27-usb
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          minItems: 3
> >>> +          maxItems: 3
> >>> +        clock-names:
> >>> +          minItems: 3
> >>> +          maxItems: 3
> >>> +          items:
> >>> +            anyOf:
> >>> +              - const: ipg
> >>> +              - const: ahb
> >>> +              - const: per
> >>
> >> This would be just: enum: [ipg, ahb, per], but in both cases I question
> >> why the order should be flexible? Nothing in commit msg explains it.
> > 
> > The driver will get the clock by clock-name, then the order should not
> > matter. However, these three clock-names should be present at the same
> > time. I should use enum then.
> > 
> >>
> >> Plus I will repeat myself from your v4. I don't think this is helping,
> >> because the file will soon grow to umnanageable chunk. I prefer to fix
> >> it at beginning, before we reach snps-schema level of complexities.
> >>
> >> Please define common schema, reference in this file and move IMX to own
> >> file.
> > 
> > I'm not that familiar with dt-bindings architecture. If I define a common
> > schema, then should I create imx, qcom, nvidia and other dt-binding files
> > too? 
> 
> No, the rest you can leave here. Someone, maybe me, will move them some
> time. The point is to move at least IMX to its own file.

That will only work as long as i.MX doesn't define any extra properties 
which it already has. We have to have at a minium common, imx, and 
everything else schema docs.

Rob

Re: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Krzysztof Kozlowski]

On 02/02/2024 10:10, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 31/01/2024 12:43, Xu Yang wrote:
>>> Change reg, interrupts, clock and clock-names as common properties and add
>>> restrictions on them for different compatibles.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>>
>>> ---
>>> Changes in v4:
>>>  - new patch since v3's discussion
>>>  - split the reg, interrupts, clock and clock-names properties into
>>>    common part and device-specific
>>> Changes in v5:
>>>  - keep common property unchanged
>>>  - make if-then more readable
>>>  - remove non imx part
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
>>>  1 file changed, 118 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index 3b56e0edb1c6..6ad3582051b8 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -412,6 +412,124 @@ allOf:
>>>          samsung,picophy-pre-emp-curr-control: false
>>>          samsung,picophy-dc-vol-level-adjust: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          const: fsl,imx27-usb
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +        clock-names:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +          items:
>>> +            anyOf:
>>> +              - const: ipg
>>> +              - const: ahb
>>> +              - const: per
>>
>> This would be just: enum: [ipg, ahb, per], but in both cases I question
>> why the order should be flexible? Nothing in commit msg explains it.
> 
> The driver will get the clock by clock-name, then the order should not
> matter. However, these three clock-names should be present at the same
> time. I should use enum then.

Forgot to answer to this.

Which driver? U-boot? OpenBSD? The one in my custom Linux kernel fork
(open-source and published on github)? Did you check all of them?

The order should be strict (or "fixed" as in non-flexible), unless there
is some reason against. If one driver does not care about order, is not
the reason.

This comment applies to all bindings so to all your future
contributions. I really keep repeating it...


Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Xu Yang]

> 
> On 02/02/2024 10:10, Xu Yang wrote:
> > Hi Krzysztof,
> >
> >>
> >> On 31/01/2024 12:43, Xu Yang wrote:
> >>> Change reg, interrupts, clock and clock-names as common properties and add
> >>> restrictions on them for different compatibles.
> >>>
> >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >>>
> >>> ---
> >>> Changes in v4:
> >>>  - new patch since v3's discussion
> >>>  - split the reg, interrupts, clock and clock-names properties into
> >>>    common part and device-specific
> >>> Changes in v5:
> >>>  - keep common property unchanged
> >>>  - make if-then more readable
> >>>  - remove non imx part
> >>> ---
> >>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
> >>>  1 file changed, 118 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> >> hdrc-usb2.yaml
> >>> index 3b56e0edb1c6..6ad3582051b8 100644
> >>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> >>> @@ -412,6 +412,124 @@ allOf:
> >>>          samsung,picophy-pre-emp-curr-control: false
> >>>          samsung,picophy-dc-vol-level-adjust: false
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          const: fsl,imx27-usb
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          minItems: 3
> >>> +          maxItems: 3
> >>> +        clock-names:
> >>> +          minItems: 3
> >>> +          maxItems: 3
> >>> +          items:
> >>> +            anyOf:
> >>> +              - const: ipg
> >>> +              - const: ahb
> >>> +              - const: per
> >>
> >> This would be just: enum: [ipg, ahb, per], but in both cases I question
> >> why the order should be flexible? Nothing in commit msg explains it.
> >
> > The driver will get the clock by clock-name, then the order should not
> > matter. However, these three clock-names should be present at the same
> > time. I should use enum then.
> 
> Forgot to answer to this.
> 
> Which driver? U-boot? OpenBSD? The one in my custom Linux kernel fork
> (open-source and published on github)? Did you check all of them?

I mean below driver in linux kernel tree.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/tree/drivers/usb/chipidea/ci_hdrc_imx.c?h=linux-6.1.y#n191

Thanks,
Xu Yang

Re: [EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Krzysztof Kozlowski]

On 04/02/2024 07:56, Xu Yang wrote:
> 
>>
>> On 02/02/2024 10:10, Xu Yang wrote:
>>> Hi Krzysztof,
>>>
>>>>
>>>> On 31/01/2024 12:43, Xu Yang wrote:
>>>>> Change reg, interrupts, clock and clock-names as common properties and add
>>>>> restrictions on them for different compatibles.
>>>>>
>>>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>>>>
>>>>> ---
>>>>> Changes in v4:
>>>>>  - new patch since v3's discussion
>>>>>  - split the reg, interrupts, clock and clock-names properties into
>>>>>    common part and device-specific
>>>>> Changes in v5:
>>>>>  - keep common property unchanged
>>>>>  - make if-then more readable
>>>>>  - remove non imx part
>>>>> ---
>>>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
>>>>>  1 file changed, 118 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>>>> hdrc-usb2.yaml
>>>>> index 3b56e0edb1c6..6ad3582051b8 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>>>> @@ -412,6 +412,124 @@ allOf:
>>>>>          samsung,picophy-pre-emp-curr-control: false
>>>>>          samsung,picophy-dc-vol-level-adjust: false
>>>>>
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          const: fsl,imx27-usb
>>>>> +    then:
>>>>> +      properties:
>>>>> +        clocks:
>>>>> +          minItems: 3
>>>>> +          maxItems: 3
>>>>> +        clock-names:
>>>>> +          minItems: 3
>>>>> +          maxItems: 3
>>>>> +          items:
>>>>> +            anyOf:
>>>>> +              - const: ipg
>>>>> +              - const: ahb
>>>>> +              - const: per
>>>>
>>>> This would be just: enum: [ipg, ahb, per], but in both cases I question
>>>> why the order should be flexible? Nothing in commit msg explains it.
>>>
>>> The driver will get the clock by clock-name, then the order should not
>>> matter. However, these three clock-names should be present at the same
>>> time. I should use enum then.
>>
>> Forgot to answer to this.
>>
>> Which driver? U-boot? OpenBSD? The one in my custom Linux kernel fork
>> (open-source and published on github)? Did you check all of them?
> 
> I mean below driver in linux kernel tree.
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/tree/drivers/usb/chipidea/ci_hdrc_imx.c?h=linux-6.1.y#n191

What about others? How about driver in other upstream projects? My
comment should make you think...

Best regards,
Krzysztof

[PATCH v5 5/8] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Xu Yang]

The i.MX93 needs a wakup clock to work properly. This will add compatible
and restriction for i.MX93 platform.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - no changes
Changes in v3:
 - add clocks restriction
Changes in v4:
 - use 'contains' rather 'items'
Changes in v5:
 - rename clock name
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 6ad3582051b8..12466f7af00b 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -57,6 +57,7 @@ properties:
           - enum:
               - fsl,imx8mm-usb
               - fsl,imx8mn-usb
+              - fsl,imx93-usb
           - const: fsl,imx7d-usb
           - const: fsl,imx27-usb
       - items:
@@ -486,7 +487,6 @@ allOf:
               - chipidea,usb2
               - fsl,imx23-usb
               - fsl,imx28-usb
-              - fsl,imx7d-usb
               - fsl,vf610-usb
     then:
       properties:
@@ -497,6 +497,38 @@ allOf:
           minItems: 1
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: fsl,imx7d-usb
+            - const: fsl,imx27-usb
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 1
+        clock-names:
+          minItems: 1
+          maxItems: 1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx93-usb
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+        clock-names:
+          minItems: 2
+          maxItems: 2
+          contains:
+            const: usb_wakeup
+
   - if:
       properties:
         compatible:
@@ -504,6 +536,7 @@ allOf:
             oneOf:
               - pattern: '^fsl,imx5[0-3]+-usb$'
               - pattern: '^fsl,imx6[a-z]+-usb$'
+              - pattern: '^fsl,imx8[a-z]+-usb$'
     then:
       properties:
         clocks:
-- 
2.34.1

Re: [PATCH v5 5/8] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Krzysztof Kozlowski]

On 31/01/2024 12:43, Xu Yang wrote:
> The i.MX93 needs a wakup clock to work properly. This will add compatible
> and restriction for i.MX93 platform.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - no changes
> Changes in v3:
>  - add clocks restriction
> Changes in v4:
>  - use 'contains' rather 'items'
> Changes in v5:
>  - rename clock name
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 35 ++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 6ad3582051b8..12466f7af00b 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -57,6 +57,7 @@ properties:
>            - enum:
>                - fsl,imx8mm-usb
>                - fsl,imx8mn-usb
> +              - fsl,imx93-usb
>            - const: fsl,imx7d-usb
>            - const: fsl,imx27-usb
>        - items:
> @@ -486,7 +487,6 @@ allOf:
>                - chipidea,usb2
>                - fsl,imx23-usb
>                - fsl,imx28-usb
> -              - fsl,imx7d-usb
>                - fsl,vf610-usb
>      then:
>        properties:
> @@ -497,6 +497,38 @@ allOf:
>            minItems: 1
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          items:
> +            - const: fsl,imx7d-usb
> +            - const: fsl,imx27-usb

This does not look related to imx93.

> +    then:
> +      properties:
> +        clocks:
> +          minItems: 1
> +          maxItems: 1
> +        clock-names:
> +          minItems: 1
> +          maxItems: 1
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx93-usb
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +        clock-names:
> +          minItems: 2
> +          maxItems: 2
> +          contains:
> +            const: usb_wakeup

No, clocks must be fixed and specific. No contains allowed for new variants.

> +
>    - if:
>        properties:
>          compatible:
> @@ -504,6 +536,7 @@ allOf:
>              oneOf:
>                - pattern: '^fsl,imx5[0-3]+-usb$'
>                - pattern: '^fsl,imx6[a-z]+-usb$'
> +              - pattern: '^fsl,imx8[a-z]+-usb$'

Aren't you adding imx9?



Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v5 5/8] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Xu Yang]

Hi Krzysztof,

> 
> On 31/01/2024 12:43, Xu Yang wrote:
> > The i.MX93 needs a wakup clock to work properly. This will add compatible
> > and restriction for i.MX93 platform.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> >  - no changes
> > Changes in v3:
> >  - add clocks restriction
> > Changes in v4:
> >  - use 'contains' rather 'items'
> > Changes in v5:
> >  - rename clock name
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 35 ++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> hdrc-usb2.yaml
> > index 6ad3582051b8..12466f7af00b 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > @@ -57,6 +57,7 @@ properties:
> >            - enum:
> >                - fsl,imx8mm-usb
> >                - fsl,imx8mn-usb
> > +              - fsl,imx93-usb
> >            - const: fsl,imx7d-usb
> >            - const: fsl,imx27-usb
> >        - items:
> > @@ -486,7 +487,6 @@ allOf:
> >                - chipidea,usb2
> >                - fsl,imx23-usb
> >                - fsl,imx28-usb
> > -              - fsl,imx7d-usb
> >                - fsl,vf610-usb
> >      then:
> >        properties:
> > @@ -497,6 +497,38 @@ allOf:
> >            minItems: 1
> >            maxItems: 1
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          items:
> > +            - const: fsl,imx7d-usb
> > +            - const: fsl,imx27-usb
> 
> This does not look related to imx93.

Because fsl,imx93-usb is compatible with fsl,imx7d-usb and only imx93 needs
wakeup clock, so I remove fsl,imx7d-usb from previous if-then restriction.
Otherwise, when do dt-validate for imx93, it will require 1 clocks and 1 clock-names
property. Then imx7d and imx8* socs will lost restrictions on clocks and clock-names.

Then I still need add restrictions for compatible = 'fsl,imx7d-usb ', 'fsl,imx27-usb '
and other compatibles like 'fsl,imx8mm-usb', 'fsl,imx7d-usb, 'fsl,imx27-usb''. So the
below changes are for this purpose.

> 
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 1
> > +          maxItems: 1
> > +        clock-names:
> > +          minItems: 1
> > +          maxItems: 1
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx93-usb
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 2
> > +        clock-names:
> > +          minItems: 2
> > +          maxItems: 2
> > +          contains:
> > +            const: usb_wakeup
> 
> No, clocks must be fixed and specific. No contains allowed for new variants.

Well, I will change this then.

> 
> > +
> >    - if:
> >        properties:
> >          compatible:
> > @@ -504,6 +536,7 @@ allOf:
> >              oneOf:
> >                - pattern: '^fsl,imx5[0-3]+-usb$'
> >                - pattern: '^fsl,imx6[a-z]+-usb$'
> > +              - pattern: '^fsl,imx8[a-z]+-usb$'
> 
> Aren't you adding imx9?

See above explanation.

Thanks,
Xu Yang

[PATCH v5 6/8] arm64: dts: imx93: add usb nodes

[Xu Yang]

There are 2 USB controllers on i.MX93. Add them.

Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa9352LA/CA
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - fix format as suggested by Alexander
 - change compatible from fsl,imx8mm-usb to fsl,imx93-usb
Changes in v3:
 - replace deprecated fsl,usbphy with phys as suggested by Alexander
 - reorder nodes
Changes in v4:
 - fix the alignment
Changes in v5:
 - rename usb_wakeup_clk to usb_wakeup
---
 arch/arm64/boot/dts/freescale/imx93.dtsi | 58 ++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 8f2e7c42ad6e..de9f9accb040 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -183,6 +183,20 @@ mqs2: mqs2 {
 		status = "disabled";
 	};
 
+	usbphynop1: usbphynop1 {
+		compatible = "usb-nop-xceiv";
+		#phy-cells = <0>;
+		clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
+		clock-names = "main_clk";
+	};
+
+	usbphynop2: usbphynop2 {
+		compatible = "usb-nop-xceiv";
+		#phy-cells = <0>;
+		clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
+		clock-names = "main_clk";
+	};
+
 	soc@0 {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -1167,6 +1181,50 @@ media_blk_ctrl: system-controller@4ac10000 {
 			status = "disabled";
 		};
 
+		usbotg1: usb@4c100000 {
+			compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
+			reg = <0x4c100000 0x200>;
+			interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
+				 <&clk IMX93_CLK_HSIO_32K_GATE>;
+			clock-names = "usb_ctrl_root_clk", "usb_wakeup";
+			assigned-clocks = <&clk IMX93_CLK_HSIO>;
+			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
+			assigned-clock-rates = <133000000>;
+			phys = <&usbphynop1>;
+			fsl,usbmisc = <&usbmisc1 0>;
+			status = "disabled";
+		};
+
+		usbmisc1: usbmisc@4c100200 {
+			compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
+				     "fsl,imx6q-usbmisc";
+			reg = <0x4c100200 0x200>;
+			#index-cells = <1>;
+		};
+
+		usbotg2: usb@4c200000 {
+			compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
+			reg = <0x4c200000 0x200>;
+			interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
+				 <&clk IMX93_CLK_HSIO_32K_GATE>;
+			clock-names = "usb_ctrl_root_clk", "usb_wakeup";
+			assigned-clocks = <&clk IMX93_CLK_HSIO>;
+			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
+			assigned-clock-rates = <133000000>;
+			phys = <&usbphynop2>;
+			fsl,usbmisc = <&usbmisc2 0>;
+			status = "disabled";
+		};
+
+		usbmisc2: usbmisc@4c200200 {
+			compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
+				     "fsl,imx6q-usbmisc";
+			reg = <0x4c200200 0x200>;
+			#index-cells = <1>;
+		};
+
 		ddr-pmu@4e300dc0 {
 			compatible = "fsl,imx93-ddr-pmu";
 			reg = <0x4e300dc0 0x200>;
-- 
2.34.1

[PATCH v5 7/8] arm64: dts: imx93-11x11-evk: enable usb and typec nodes

[Xu Yang]

There are 2 Type-C ports and 2 USB controllers on i.MX93. Enable them.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - remove status property in ptn5110 nodes
 - fix dt-schema warnings
Changes in v3:
 - no changes
Changes in v4:
 - no changes
Changes in v5:
 - no changes
---
 .../boot/dts/freescale/imx93-11x11-evk.dts    | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
index 9921ea13ab48..ec4cb50445fc 100644
--- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
 #include "imx93.dtsi"
 
 / {
@@ -104,6 +105,80 @@ &mu2 {
 	status = "okay";
 };
 
+&lpi2c3 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clock-frequency = <400000>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pinctrl_lpi2c3>;
+	pinctrl-1 = <&pinctrl_lpi2c3>;
+	status = "okay";
+
+	ptn5110: tcpc@50 {
+		compatible = "nxp,ptn5110";
+		reg = <0x50>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+
+		typec1_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			data-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_VAR(5000, 20000, 3000)>;
+			op-sink-microwatt = <15000000>;
+			self-powered;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					typec1_dr_sw: endpoint {
+						remote-endpoint = <&usb1_drd_sw>;
+					};
+				};
+			};
+		};
+	};
+
+	ptn5110_2: tcpc@51 {
+		compatible = "nxp,ptn5110";
+		reg = <0x51>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+
+		typec2_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			data-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_VAR(5000, 20000, 3000)>;
+			op-sink-microwatt = <15000000>;
+			self-powered;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					typec2_dr_sw: endpoint {
+						remote-endpoint = <&usb2_drd_sw>;
+					};
+				};
+			};
+		};
+	};
+};
+
 &eqos {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_eqos>;
@@ -156,6 +231,42 @@ &lpuart5 {
 	status = "okay";
 };
 
+&usbotg1 {
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	usb-role-switch;
+	disable-over-current;
+	samsung,picophy-pre-emp-curr-control = <3>;
+	samsung,picophy-dc-vol-level-adjust = <7>;
+	status = "okay";
+
+	port {
+		usb1_drd_sw: endpoint {
+			remote-endpoint = <&typec1_dr_sw>;
+		};
+	};
+};
+
+&usbotg2 {
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	usb-role-switch;
+	disable-over-current;
+	samsung,picophy-pre-emp-curr-control = <3>;
+	samsung,picophy-dc-vol-level-adjust = <7>;
+	status = "okay";
+
+	port {
+		usb2_drd_sw: endpoint {
+			remote-endpoint = <&typec2_dr_sw>;
+		};
+	};
+};
+
 &usdhc1 {
 	pinctrl-names = "default", "state_100mhz", "state_200mhz";
 	pinctrl-0 = <&pinctrl_usdhc1>;
@@ -222,6 +333,13 @@ MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL	0x57e
 		>;
 	};
 
+	pinctrl_lpi2c3: lpi2c3grp {
+		fsl,pins = <
+			MX93_PAD_GPIO_IO28__LPI2C3_SDA			0x40000b9e
+			MX93_PAD_GPIO_IO29__LPI2C3_SCL			0x40000b9e
+		>;
+	};
+
 	pinctrl_uart1: uart1grp {
 		fsl,pins = <
 			MX93_PAD_UART1_RXD__LPUART1_RX			0x31e
-- 
2.34.1

[PATCH v5 8/8] usb: chipidea: ci_hdrc_imx: align usb wakeup clock name with dt-bindings

[Xu Yang]

The dt-bindings is going to use "usb_wakeup" as wakup clock name. This will
align the change with dt-bindings.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - new patch
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index ae9a6a17ec6e..a17b6d619305 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -212,7 +212,7 @@ static int imx_get_clks(struct device *dev)
 		/* Get wakeup clock. Not all of the platforms need to
 		 * handle this clock. So make it optional.
 		 */
-		data->clk_wakeup = devm_clk_get_optional(dev, "usb_wakeup_clk");
+		data->clk_wakeup = devm_clk_get_optional(dev, "usb_wakeup");
 		if (IS_ERR(data->clk_wakeup))
 			ret = dev_err_probe(dev, PTR_ERR(data->clk_wakeup),
 					"Failed to get wakeup clk\n");
-- 
2.34.1

Re: [PATCH v5 8/8] usb: chipidea: ci_hdrc_imx: align usb wakeup clock name with dt-bindings

[Peter Chen]

On 24-01-31 19:43:24, Xu Yang wrote:
> The dt-bindings is going to use "usb_wakeup" as wakup clock name. This will
> align the change with dt-bindings.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v5:
>  - new patch
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index ae9a6a17ec6e..a17b6d619305 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -212,7 +212,7 @@ static int imx_get_clks(struct device *dev)
>  		/* Get wakeup clock. Not all of the platforms need to
>  		 * handle this clock. So make it optional.
>  		 */
> -		data->clk_wakeup = devm_clk_get_optional(dev, "usb_wakeup_clk");
> +		data->clk_wakeup = devm_clk_get_optional(dev, "usb_wakeup");
>  		if (IS_ERR(data->clk_wakeup))
>  			ret = dev_err_probe(dev, PTR_ERR(data->clk_wakeup),
>  					"Failed to get wakeup clk\n");
> -- 
> 2.34.1
> 

Acked-by: Peter Chen <peter.chen@kernel.org>

-- 

Thanks,
Peter Chen