diff mbox series

[v3,5/6] arm64: dtsi: imx8mp: add usb nodes

Message ID 1600771612-30727-6-git-send-email-jun.li@nxp.com (mailing list archive)
State Superseded
Headers show
Series add NXP imx8mp usb support | expand

Commit Message

Jun Li Sept. 22, 2020, 10:46 a.m. UTC
imx8mp integrates 2 identical dwc3 based USB3 controllers and
Synopsys phys, each instance has additional wakeup logic to
support low power mode, so the glue layer need a node with dwc3
core sub node.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 88 +++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

Comments

Krzysztof Kozlowski Sept. 22, 2020, 2:39 p.m. UTC | #1
On Tue, 22 Sep 2020 at 12:56, Li Jun <jun.li@nxp.com> wrote:
>
> imx8mp integrates 2 identical dwc3 based USB3 controllers and
> Synopsys phys, each instance has additional wakeup logic to
> support low power mode, so the glue layer need a node with dwc3
> core sub node.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 88 +++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 9de2aa1..1b7ed4c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -750,5 +750,93 @@
>                         reg = <0x3d800000 0x400000>;
>                         interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
>                 };
> +
> +               usb3_phy0: usb-phy@381f0040 {
> +                       compatible = "fsl,imx8mp-usb-phy";
> +                       reg = <0x381f0040 0x40>;
> +                       clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
> +                       clock-names = "phy";
> +                       assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> +                       assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> +                       #phy-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               usb3_0: usb@32f10100 {
> +                       compatible = "fsl,imx8mp-dwc3";
> +                       reg = <0x32f10100 0x8>;
> +                       clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> +                                <&clk IMX8MP_CLK_USB_ROOT>;
> +                       clock-names = "hsio", "suspend";
> +                       interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges;
> +                       status = "disabled";
> +
> +                       usb_dwc3_0: dwc3@38100000 {
> +                               compatible = "snps,dwc3";
> +                               reg = <0x38100000 0x10000>;
> +                               clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> +                                        <&clk IMX8MP_CLK_USB_CORE_REF>,
> +                                        <&clk IMX8MP_CLK_USB_ROOT>;
> +                               clock-names = "bus_early", "ref", "suspend";
> +                               assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> +                               assigned-clock-rates = <500000000>;
> +                               interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> +                               phys = <&usb3_phy0>, <&usb3_phy0>;
> +                               phy-names = "usb2-phy", "usb3-phy";
> +                               snps,dis-u2-freeclk-exists-quirk;
> +                               xhci-64bit-support-disable;
> +                               status = "disabled";
> +                       };
> +
> +               };
> +
> +               usb3_phy1: usb-phy@382f0040 {
> +                       compatible = "fsl,imx8mp-usb-phy";
> +                       reg = <0x382f0040 0x40>;
> +                       clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
> +                       clock-names = "phy";
> +                       assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> +                       assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> +                       #phy-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               usb3_1: usb@32f10108 {
> +                       compatible = "fsl,imx8mp-dwc3";
> +                       reg = <0x32f10108 0x8>;
> +                       clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> +                                <&clk IMX8MP_CLK_USB_ROOT>;
> +                       clock-names = "hsio", "suspend";
> +                       assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI_SRC>;

This does not validate against your own schema:
arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml: usb@32f10108:
'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' do
not match any of the regexes: '^dwc3@[0-9a-f]+$', 'pinctrl-[0-9]+'

Please, stop adding new schema and DTS which from day one have
warnings/violations.

It's really a lot of effort (and commits) to clean this up later.

Best regards,
Krzysztof
Sascha Hauer Sept. 23, 2020, 7:50 a.m. UTC | #2
On Tue, Sep 22, 2020 at 06:46:51PM +0800, Li Jun wrote:
> imx8mp integrates 2 identical dwc3 based USB3 controllers and
> Synopsys phys, each instance has additional wakeup logic to
> support low power mode, so the glue layer need a node with dwc3
> core sub node.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 88 +++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 9de2aa1..1b7ed4c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +		usb3_1: usb@32f10108 {
> +			compatible = "fsl,imx8mp-dwc3";
> +			reg = <0x32f10108 0x8>;
> +			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> +				 <&clk IMX8MP_CLK_USB_ROOT>;
> +			clock-names = "hsio", "suspend";
> +			assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI_SRC>;

In Linux-5.9-rc6 this clock doesn't exist anymore. Should be
IMX8MP_CLK_HSIO_AXI

> +			assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> +			assigned-clock-rates = <500000000>;
> +			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			status = "disabled";
> +
> +			usb_dwc3_1: dwc3@38200000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x38200000 0x10000>;
> +				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> +					 <&clk IMX8MP_CLK_USB_CORE_REF>,
> +					 <&clk IMX8MP_CLK_USB_ROOT>;
> +				clock-names = "bus_early", "ref", "suspend";
> +				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> +				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> +				assigned-clock-rates = <500000000>;
> +				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +				phys = <&usb3_phy1>, <&usb3_phy1>;
> +				phy-names = "usb2-phy", "usb3-phy";
> +				snps,dis-u2-freeclk-exists-quirk;
> +				xhci-64bit-support-disable;
> +				status = "disabled";

Does it make sense for a board to enable the parent node and leave this
one disabled? If not you can drop this status = "disabled" here.

Sascha
Jun Li Sept. 23, 2020, 8:49 a.m. UTC | #3
> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Wednesday, September 23, 2020 3:51 PM
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; shawnguo@kernel.org; balbi@kernel.org;
> mathias.nyman@intel.com; gregkh@linuxfoundation.org;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; Anson Huang <anson.huang@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; Peng Fan <peng.fan@nxp.com>; Andy Duan
> <fugang.duan@nxp.com>; Joakim Zhang <qiangqing.zhang@nxp.com>; Horia
> Geanta <horia.geanta@nxp.com>; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v3 5/6] arm64: dtsi: imx8mp: add usb nodes
> 
> On Tue, Sep 22, 2020 at 06:46:51PM +0800, Li Jun wrote:
> > imx8mp integrates 2 identical dwc3 based USB3 controllers and Synopsys
> > phys, each instance has additional wakeup logic to support low power
> > mode, so the glue layer need a node with dwc3 core sub node.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 88
> > +++++++++++++++++++++++++++++++
> >  1 file changed, 88 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 9de2aa1..1b7ed4c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +		usb3_1: usb@32f10108 {
> > +			compatible = "fsl,imx8mp-dwc3";
> > +			reg = <0x32f10108 0x8>;
> > +			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > +				 <&clk IMX8MP_CLK_USB_ROOT>;
> > +			clock-names = "hsio", "suspend";
> > +			assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI_SRC>;
> 
> In Linux-5.9-rc6 this clock doesn't exist anymore. Should be
> IMX8MP_CLK_HSIO_AXI

Will change.

> 
> > +			assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > +			assigned-clock-rates = <500000000>;
> > +			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges;
> > +			status = "disabled";
> > +
> > +			usb_dwc3_1: dwc3@38200000 {
> > +				compatible = "snps,dwc3";
> > +				reg = <0x38200000 0x10000>;
> > +				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > +					 <&clk IMX8MP_CLK_USB_CORE_REF>,
> > +					 <&clk IMX8MP_CLK_USB_ROOT>;
> > +				clock-names = "bus_early", "ref", "suspend";
> > +				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > +				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > +				assigned-clock-rates = <500000000>;
> > +				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > +				phys = <&usb3_phy1>, <&usb3_phy1>;
> > +				phy-names = "usb2-phy", "usb3-phy";
> > +				snps,dis-u2-freeclk-exists-quirk;
> > +				xhci-64bit-support-disable;
> > +				status = "disabled";
> 
> Does it make sense for a board to enable the parent node and leave this one
> disabled? If not you can drop this status = "disabled" here.

OK, will drop it.

Thanks
Li Jun
> 
> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=02%7C01%7Cjun.li%40nxp.com%7Cadbbbfe2edec4cca1
> 38908d85f956382%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6373644425
> 20571896&amp;sdata=07ku8kTQCv8lv8qdRiOcU3CX4lly3503LO4bUDIbjow%3D&amp;r
> eserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9de2aa1..1b7ed4c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -750,5 +750,93 @@ 
 			reg = <0x3d800000 0x400000>;
 			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		usb3_phy0: usb-phy@381f0040 {
+			compatible = "fsl,imx8mp-usb-phy";
+			reg = <0x381f0040 0x40>;
+			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
+			clock-names = "phy";
+			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
+			assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		usb3_0: usb@32f10100 {
+			compatible = "fsl,imx8mp-dwc3";
+			reg = <0x32f10100 0x8>;
+			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
+				 <&clk IMX8MP_CLK_USB_ROOT>;
+			clock-names = "hsio", "suspend";
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			status = "disabled";
+
+			usb_dwc3_0: dwc3@38100000 {
+				compatible = "snps,dwc3";
+				reg = <0x38100000 0x10000>;
+				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
+					 <&clk IMX8MP_CLK_USB_CORE_REF>,
+					 <&clk IMX8MP_CLK_USB_ROOT>;
+				clock-names = "bus_early", "ref", "suspend";
+				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+				assigned-clock-rates = <500000000>;
+				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+				phys = <&usb3_phy0>, <&usb3_phy0>;
+				phy-names = "usb2-phy", "usb3-phy";
+				snps,dis-u2-freeclk-exists-quirk;
+				xhci-64bit-support-disable;
+				status = "disabled";
+			};
+
+		};
+
+		usb3_phy1: usb-phy@382f0040 {
+			compatible = "fsl,imx8mp-usb-phy";
+			reg = <0x382f0040 0x40>;
+			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
+			clock-names = "phy";
+			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
+			assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		usb3_1: usb@32f10108 {
+			compatible = "fsl,imx8mp-dwc3";
+			reg = <0x32f10108 0x8>;
+			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
+				 <&clk IMX8MP_CLK_USB_ROOT>;
+			clock-names = "hsio", "suspend";
+			assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI_SRC>;
+			assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+			assigned-clock-rates = <500000000>;
+			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			status = "disabled";
+
+			usb_dwc3_1: dwc3@38200000 {
+				compatible = "snps,dwc3";
+				reg = <0x38200000 0x10000>;
+				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
+					 <&clk IMX8MP_CLK_USB_CORE_REF>,
+					 <&clk IMX8MP_CLK_USB_ROOT>;
+				clock-names = "bus_early", "ref", "suspend";
+				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+				assigned-clock-rates = <500000000>;
+				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+				phys = <&usb3_phy1>, <&usb3_phy1>;
+				phy-names = "usb2-phy", "usb3-phy";
+				snps,dis-u2-freeclk-exists-quirk;
+				xhci-64bit-support-disable;
+				status = "disabled";
+			};
+		};
 	};
 };