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

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

[Xu Yang]

Add "fsl,imx8ulp-usbmisc" compatible.

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

---
Changes in v3:
 - new patch due to missed this little one
---
 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 v3 2/6] 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
---
 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

[PATCH v3 3/6] 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
---
 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 v3 4/6] 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
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index b7e664f7395b..6e75099b6394 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:
@@ -412,6 +413,21 @@ allOf:
         samsung,picophy-pre-emp-curr-control: false
         samsung,picophy-dc-vol-level-adjust: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx93-usb
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: usb_ctrl_root_clk
+            - const: usb_wakeup_clk
+        clocks:
+          minItems: 2
+          maxItems: 2
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Alexander Stein]

Hi,

Am Freitag, 12. Januar 2024, 12:17:45 CET schrieb 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>

Looks good to me:
Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> 
> ---
> Changes in v2:
>  - no changes
> Changes in v3:
>  - add clocks restriction
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml index
> b7e664f7395b..6e75099b6394 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:
> @@ -412,6 +413,21 @@ allOf:
>          samsung,picophy-pre-emp-curr-control: false
>          samsung,picophy-dc-vol-level-adjust: false
> 
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx93-usb
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: usb_ctrl_root_clk
> +            - const: usb_wakeup_clk
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +
>  unevaluatedProperties: false
> 
>  examples:


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Krzysztof Kozlowski]

On 12/01/2024 12:17, 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
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index b7e664f7395b..6e75099b6394 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:
> @@ -412,6 +413,21 @@ allOf:
>          samsung,picophy-pre-emp-curr-control: false
>          samsung,picophy-dc-vol-level-adjust: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx93-usb
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: usb_ctrl_root_clk
> +            - const: usb_wakeup_clk
> +        clocks:
> +          minItems: 2
> +          maxItems: 2

Nothing improved regarding my comments. Why do you allow
any/unspecific/unconstrain interrupts and reg?

You said:
"However, reset, reg and interrupts property is not special for imx93."
but what does it even mean? How they can be special or not special?

My comments from previous version apply. If you do not want to work on
existing technical debt, BTW added by another NXP employee, then I will
NAK any new submissions.

Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Xu Yang]

Hi Krzysztof,

> 
> On 12/01/2024 12:17, 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
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> hdrc-usb2.yaml
> > index b7e664f7395b..6e75099b6394 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:
> > @@ -412,6 +413,21 @@ allOf:
> >          samsung,picophy-pre-emp-curr-control: false
> >          samsung,picophy-dc-vol-level-adjust: false
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx93-usb
> > +    then:
> > +      properties:
> > +        clock-names:
> > +          items:
> > +            - const: usb_ctrl_root_clk
> > +            - const: usb_wakeup_clk
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 2
> 
> Nothing improved regarding my comments. Why do you allow
> any/unspecific/unconstrain interrupts and reg?
> 
> You said:
> "However, reset, reg and interrupts property is not special for imx93."
> but what does it even mean? How they can be special or not special?
> 
> My comments from previous version apply. If you do not want to work on
> existing technical debt, BTW added by another NXP employee, then I will
> NAK any new submissions.

You want me to adjust below properties to be more common properties
and add device specific limitations, right?

---
  reg:
    minItems: 1
    maxItems: 2

  interrupts:
    minItems: 1
    maxItems: 2

  clocks:
    minItems: 1
    maxItems: 3

  clock-names:
    minItems: 1
    maxItems: 3
---

For most of the devices, property reg, interrupts, clocks and clock-names
has 1 item. So these properties can set maxItems to 1. For special devices,
I should list them standalone, is this your expectation?

Thanks,
Xu Yang

> 
> Best regards,
> Krzysztof

Re: [EXT] Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Krzysztof Kozlowski]

On 16/01/2024 08:49, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 12/01/2024 12:17, 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
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index b7e664f7395b..6e75099b6394 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:
>>> @@ -412,6 +413,21 @@ allOf:
>>>          samsung,picophy-pre-emp-curr-control: false
>>>          samsung,picophy-dc-vol-level-adjust: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,imx93-usb
>>> +    then:
>>> +      properties:
>>> +        clock-names:
>>> +          items:
>>> +            - const: usb_ctrl_root_clk
>>> +            - const: usb_wakeup_clk
>>> +        clocks:
>>> +          minItems: 2
>>> +          maxItems: 2
>>
>> Nothing improved regarding my comments. Why do you allow
>> any/unspecific/unconstrain interrupts and reg?
>>
>> You said:
>> "However, reset, reg and interrupts property is not special for imx93."
>> but what does it even mean? How they can be special or not special?
>>
>> My comments from previous version apply. If you do not want to work on
>> existing technical debt, BTW added by another NXP employee, then I will
>> NAK any new submissions.
> 
> You want me to adjust below properties to be more common properties
> and add device specific limitations, right?

Yes

> 
> ---
>   reg:
>     minItems: 1
>     maxItems: 2
> 
>   interrupts:
>     minItems: 1
>     maxItems: 2
> 
>   clocks:
>     minItems: 1
>     maxItems: 3
> 
>   clock-names:
>     minItems: 1
>     maxItems: 3
> ---
> 
> For most of the devices, property reg, interrupts, clocks and clock-names
> has 1 item. So these properties can set maxItems to 1. For special devices,
> I should list them standalone, is this your expectation?

Just like you constrain clocks for new variant, your variant should have
constrained reg, interrupts and whatever else is there variable. I don't
require fixing all the devices in this binding, but at least your new
one and preferably other NXP as well.


Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Xu Yang]

Hi Krzysztof,

> 
> On 16/01/2024 08:49, Xu Yang wrote:
> > Hi Krzysztof,
> >
> >>
> >> On 12/01/2024 12:17, 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
> >>> ---
> >>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
> >>>  1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> >> hdrc-usb2.yaml
> >>> index b7e664f7395b..6e75099b6394 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:
> >>> @@ -412,6 +413,21 @@ allOf:
> >>>          samsung,picophy-pre-emp-curr-control: false
> >>>          samsung,picophy-dc-vol-level-adjust: false
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            const: fsl,imx93-usb
> >>> +    then:
> >>> +      properties:
> >>> +        clock-names:
> >>> +          items:
> >>> +            - const: usb_ctrl_root_clk
> >>> +            - const: usb_wakeup_clk
> >>> +        clocks:
> >>> +          minItems: 2
> >>> +          maxItems: 2
> >>
> >> Nothing improved regarding my comments. Why do you allow
> >> any/unspecific/unconstrain interrupts and reg?
> >>
> >> You said:
> >> "However, reset, reg and interrupts property is not special for imx93."
> >> but what does it even mean? How they can be special or not special?
> >>
> >> My comments from previous version apply. If you do not want to work on
> >> existing technical debt, BTW added by another NXP employee, then I will
> >> NAK any new submissions.
> >
> > You want me to adjust below properties to be more common properties
> > and add device specific limitations, right?
> 
> Yes
> 
> >
> > ---
> >   reg:
> >     minItems: 1
> >     maxItems: 2
> >
> >   interrupts:
> >     minItems: 1
> >     maxItems: 2
> >
> >   clocks:
> >     minItems: 1
> >     maxItems: 3
> >
> >   clock-names:
> >     minItems: 1
> >     maxItems: 3
> > ---
> >
> > For most of the devices, property reg, interrupts, clocks and clock-names
> > has 1 item. So these properties can set maxItems to 1. For special devices,
> > I should list them standalone, is this your expectation?
> 
> Just like you constrain clocks for new variant, your variant should have
> constrained reg, interrupts and whatever else is there variable. I don't
> require fixing all the devices in this binding, but at least your new
> one and preferably other NXP as well.

Okay. I will do this and test it.

Thanks,
Xu Yang

RE: [EXT] Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Xu Yang]

Hi Krzysztof,

> 
> On 16/01/2024 08:49, Xu Yang wrote:
> > Hi Krzysztof,
> >
> >>
> >> On 12/01/2024 12:17, 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
> >>> ---
> >>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
> >>>  1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> >> hdrc-usb2.yaml
> >>> index b7e664f7395b..6e75099b6394 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:
> >>> @@ -412,6 +413,21 @@ allOf:
> >>>          samsung,picophy-pre-emp-curr-control: false
> >>>          samsung,picophy-dc-vol-level-adjust: false
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            const: fsl,imx93-usb
> >>> +    then:
> >>> +      properties:
> >>> +        clock-names:
> >>> +          items:
> >>> +            - const: usb_ctrl_root_clk
> >>> +            - const: usb_wakeup_clk
> >>> +        clocks:
> >>> +          minItems: 2
> >>> +          maxItems: 2
> >>
> >> Nothing improved regarding my comments. Why do you allow
> >> any/unspecific/unconstrain interrupts and reg?
> >>
> >> You said:
> >> "However, reset, reg and interrupts property is not special for imx93."
> >> but what does it even mean? How they can be special or not special?
> >>
> >> My comments from previous version apply. If you do not want to work on
> >> existing technical debt, BTW added by another NXP employee, then I will
> >> NAK any new submissions.
> >
> > You want me to adjust below properties to be more common properties
> > and add device specific limitations, right?
> 
> Yes
> 
> >
> > ---
> >   reg:
> >     minItems: 1
> >     maxItems: 2
> >
> >   interrupts:
> >     minItems: 1
> >     maxItems: 2
> >
> >   clocks:
> >     minItems: 1
> >     maxItems: 3
> >
> >   clock-names:
> >     minItems: 1
> >     maxItems: 3
> > ---
> >
> > For most of the devices, property reg, interrupts, clocks and clock-names
> > has 1 item. So these properties can set maxItems to 1. For special devices,
> > I should list them standalone, is this your expectation?
> 
> Just like you constrain clocks for new variant, your variant should have
> constrained reg, interrupts and whatever else is there variable. I don't
> require fixing all the devices in this binding, but at least your new
> one and preferably other NXP as well.
> 

I'm tring to set the maxItems of property reg, interrupts, clocks and 
clock-names to 1, then constrain the minItems and maxItems to 3 for
one soc later like below, in such way I needn't to constrain reg and
interrupts for most of the socs.
But dt-validate failed at the first place when validate clocks property.

Is there a way to achieve this?

---
  reg:
    maxItems: 1

  interrupts:
    maxItems: 1

  clocks:
    maxItems: 1

  clock-names:
    maxItems: 1

allOf:
  - if:
      properties:
        compatible:
          oneOf:
            - items:
                - const: fsl,imx27-usb
    then:
      properties:
        clocks:
          minItems: 3
          maxItems: 3
        clock-names:
          minItems: 3
          maxItems: 3
---

Thanks,
Xu Yang

[PATCH v3 5/6] arm64: dts: imx93: add usb nodes

[Xu Yang]

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

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
---
 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 34c0540276d1..10124758c579 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -171,6 +171,20 @@ cm33: remoteproc-cm33 {
 		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>;
@@ -1054,6 +1068,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_clk";
+			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_clk";
+			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

Re: [PATCH v3 5/6] arm64: dts: imx93: add usb nodes

[Alexander Stein]

Am Freitag, 12. Januar 2024, 12:17:46 CET schrieb Xu Yang:
> ********************
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen,
> dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie
> die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
> Attention external email: Open attachments and links only if you know that
> they are from a secure source and are safe. In doubt forward the email to
> the IT-Helpdesk to check it. ********************
> 
> There are 2 USB controllers on i.MX93. Add them.
> 
> 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
> ---
>  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 34c0540276d1..10124758c579
> 100644
> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> @@ -171,6 +171,20 @@ cm33: remoteproc-cm33 {
>  		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>;
> @@ -1054,6 +1068,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_clk";
> +			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";

Please fix the alignment here.

> +			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_clk";
> +			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";

Please fix the alignment here as well.
With both changes applied:

Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa9352LA/CA

> +			reg = <0x4c200200 0x200>;
> +			#index-cells = <1>;
> +		};
> +
>  		ddr-pmu@4e300dc0 {
>  			compatible = "fsl,imx93-ddr-pmu";
>  			reg = <0x4e300dc0 0x200>;


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

[PATCH v3 6/6] 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
---
 .../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 2b9d47716f75..87c7cfacdc7b 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"
 
 / {
@@ -103,6 +104,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>;
@@ -155,6 +230,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>;
@@ -221,6 +332,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