* [PATCH v2 0/3] dts: imx8qxp add cdns usb3 port
@ 2023-03-16 21:27 Frank Li
2023-03-16 21:27 ` [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings Frank Li
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Frank Li @ 2023-03-16 21:27 UTC (permalink / raw)
To: shawnguo
Cc: Frank.Li, devicetree, festevam, imx, kernel,
krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx,
linux-kernel, robh+dt, s.hauer
cdns driver code already upstreamed. but missed dts part.
Change from v1 to v2:
1. Add binding docoument.
2. Fixed all shawn's comments
Frank Li (3):
dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings
arm64: dts: imx8qxp: add cadence usb3 support
arm64: dts: freescale: imx8qxp-mek: enable cadence usb3
.../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 ++++++++++++++++++
.../boot/dts/freescale/imx8-ss-conn.dtsi | 72 +++++++++++
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 85 ++++++++++++
3 files changed, 279 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-16 21:27 [PATCH v2 0/3] dts: imx8qxp add cdns usb3 port Frank Li @ 2023-03-16 21:27 ` Frank Li 2023-03-17 9:09 ` Krzysztof Kozlowski 2023-03-16 21:27 ` [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support Frank Li 2023-03-16 21:27 ` [PATCH v2 3/3] arm64: dts: freescale: imx8qxp-mek: enable cadence usb3 Frank Li 2 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-16 21:27 UTC (permalink / raw) To: shawnguo Cc: Frank.Li, devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx, linux-kernel, robh+dt, s.hauer NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml new file mode 100644 index 000000000000..fc24df1e4483 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml @@ -0,0 +1,122 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (c) 2020 NXP +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/fsl,imx8qm-cdns3.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP iMX8QM Soc USB Controller + +maintainers: + - Frank Li <Frank.Li@nxp.com> + +properties: + compatible: + const: fsl,imx8qm-usb3 + + reg: + items: + - description: Address and length of the register set for iMX USB3 Platform Control + + "#address-cells": + enum: [ 1, 2 ] + + "#size-cells": + enum: [ 1, 2 ] + + ranges: true + + clocks: + description: + A list of phandle and clock-specifier pairs for the clocks + listed in clock-names. + items: + - description: Standby clock. Used during ultra low power states. + - description: USB bus clock for usb3 controller. + - description: AXI clock for AXI interface. + - description: ipg clock for register access. + - description: Core clock for usb3 controller. + + clock-names: + items: + - const: usb3_lpm_clk + - const: usb3_bus_clk + - const: usb3_aclk + - const: usb3_ipg_clk + - const: usb3_core_pclk + + assigned-clocks: + items: + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MISC. + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MST_BUS. + + assigned-clock-rates: + items: + - description: Must be 125 Mhz. + - description: Must be 12 Mhz. + - description: Must be 250 Mhz. + + power-domains: + maxItems: 1 + +# Required child node: + +patternProperties: + "^usb@[0-9a-f]+$": + $ref: cdns,usb3.yaml# + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + - ranges + - clocks + - clock-names + - power-domains + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/imx8-lpcg.h> + #include <dt-bindings/firmware/imx/rsrc.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + usbotg3: usb@5b110000 { + compatible = "fsl,imx8qm-usb3"; + ranges; + reg = <0x5b110000 0x10000>; + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, + <&usb3_lpcg IMX_LPCG_CLK_0>, + <&usb3_lpcg IMX_LPCG_CLK_7>, + <&usb3_lpcg IMX_LPCG_CLK_4>, + <&usb3_lpcg IMX_LPCG_CLK_5>; + clock-names = "usb3_lpm_clk", "usb3_bus_clk", "usb3_aclk", + "usb3_ipg_clk", "usb3_core_pclk"; + assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; + assigned-clock-rates = <125000000>, <12000000>, <250000000>; + power-domains = <&pd IMX_SC_R_USB_2>; + #address-cells = <1>; + #size-cells = <1>; + status = "disabled"; + + usbotg3_cdns3: usb@5b120000 { + compatible = "cdns,usb3"; + interrupt-parent = <&gic>; + interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "host", "peripheral", "otg", "wakeup"; + reg = <0x5b120000 0x10000>, /* memory area for OTG/DRD registers */ + <0x5b130000 0x10000>, /* memory area for HOST registers */ + <0x5b140000 0x10000>; /* memory area for DEVICE registers */ + reg-names = "otg", "xhci", "dev"; + phys = <&usb3_phy>; + phy-names = "cdns3,usb3-phy"; + }; + }; -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-16 21:27 ` [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings Frank Li @ 2023-03-17 9:09 ` Krzysztof Kozlowski 2023-03-17 14:55 ` [EXT] " Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-17 9:09 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx, linux-kernel, robh+dt, s.hauer On 16/03/2023 22:27, Frank Li wrote: > NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. > Subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 ++++++++++++++++++ > 1 file changed, 122 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml > > diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml > new file mode 100644 > index 000000000000..fc24df1e4483 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml > @@ -0,0 +1,122 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (c) 2020 NXP > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/usb/fsl,imx8qm-cdns3.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP iMX8QM Soc USB Controller > + > +maintainers: > + - Frank Li <Frank.Li@nxp.com> > + > +properties: > + compatible: > + const: fsl,imx8qm-usb3 > + > + reg: > + items: > + - description: Address and length of the register set for iMX USB3 Platform Control Drop "Address and length of the"... or actually just maxItems: 1, because the description is a bit obvious. > + > + "#address-cells": > + enum: [ 1, 2 ] > + > + "#size-cells": > + enum: [ 1, 2 ] > + > + ranges: true > + > + clocks: > + description: > + A list of phandle and clock-specifier pairs for the clocks > + listed in clock-names. Drop description. > + items: > + - description: Standby clock. Used during ultra low power states. > + - description: USB bus clock for usb3 controller. > + - description: AXI clock for AXI interface. > + - description: ipg clock for register access. > + - description: Core clock for usb3 controller. > + > + clock-names: > + items: > + - const: usb3_lpm_clk > + - const: usb3_bus_clk > + - const: usb3_aclk > + - const: usb3_ipg_clk > + - const: usb3_core_pclk > + > + assigned-clocks: > + items: > + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. > + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MISC. > + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MST_BUS. > + > + assigned-clock-rates: > + items: > + - description: Must be 125 Mhz. > + - description: Must be 12 Mhz. > + - description: Must be 250 Mhz. I would argue that both properties above are not needed. If your hardware requires fixed frequencies, clock provider can fix them, can't it? > + > + power-domains: > + maxItems: 1 > + > +# Required child node: > + > +patternProperties: > + "^usb@[0-9a-f]+$": > + $ref: cdns,usb3.yaml# > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + - ranges > + - clocks > + - clock-names > + - power-domains > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/imx8-lpcg.h> > + #include <dt-bindings/firmware/imx/rsrc.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + usbotg3: usb@5b110000 { Drop label, unused > + compatible = "fsl,imx8qm-usb3"; > + ranges; > + reg = <0x5b110000 0x10000>; reg is second property > + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, > + <&usb3_lpcg IMX_LPCG_CLK_0>, > + <&usb3_lpcg IMX_LPCG_CLK_7>, > + <&usb3_lpcg IMX_LPCG_CLK_4>, > + <&usb3_lpcg IMX_LPCG_CLK_5>; > + clock-names = "usb3_lpm_clk", "usb3_bus_clk", "usb3_aclk", > + "usb3_ipg_clk", "usb3_core_pclk"; > + assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; > + assigned-clock-rates = <125000000>, <12000000>, <250000000>; > + power-domains = <&pd IMX_SC_R_USB_2>; > + #address-cells = <1>; > + #size-cells = <1>; > + status = "disabled"; Drop status > + > + usbotg3_cdns3: usb@5b120000 { Drop label > + compatible = "cdns,usb3"; > + interrupt-parent = <&gic>; > + interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "host", "peripheral", "otg", "wakeup"; > + reg = <0x5b120000 0x10000>, /* memory area for OTG/DRD registers */ > + <0x5b130000 0x10000>, /* memory area for HOST registers */ > + <0x5b140000 0x10000>; /* memory area for DEVICE registers */ > + reg-names = "otg", "xhci", "dev"; reg is second property, reg-names third. > + phys = <&usb3_phy>; > + phy-names = "cdns3,usb3-phy"; > + }; > + }; Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-17 9:09 ` Krzysztof Kozlowski @ 2023-03-17 14:55 ` Frank Li 2023-03-19 11:13 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-17 14:55 UTC (permalink / raw) To: Krzysztof Kozlowski, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Friday, March 17, 2023 4:09 AM > To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org > Cc: devicetree@vger.kernel.org; festevam@gmail.com; imx@lists.linux.dev; > kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm- > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux- > kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de > Subject: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 16/03/2023 22:27, Frank Li wrote: > > NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. > > > > Subject: drop second/last, redundant "bindings". The "dt-bindings" > prefix is already stating that these are bindings. > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 ++++++++++++++++++ > > 1 file changed, 122 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm- > cdns3.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm- > cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm- > cdns3.yaml > > new file mode 100644 > > index 000000000000..fc24df1e4483 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml > > @@ -0,0 +1,122 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +# Copyright (c) 2020 NXP > > +%YAML 1.2 > > +--- > > +$id: > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice > tree.org%2Fschemas%2Fusb%2Ffsl%2Cimx8qm- > cdns3.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9af3d617dc4cf > 14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 38146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C > &sdata=EczZhjyMUGnnp7ZGfSvTj4lmOUuGlOtWYIsxxXIlNXw%3D&reserved > =0 > > +$schema: > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice > tree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9a > f3d617dc4cf14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > 0%7C0%7C638146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > %7C%7C%7C&sdata=uTNYuDm%2ByhZ56oQET2pX8sHpEqVvsUYtmOBCPXEK > v40%3D&reserved=0 > > + > > +title: NXP iMX8QM Soc USB Controller > > + > > +maintainers: > > + - Frank Li <Frank.Li@nxp.com> > > + > > +properties: > > + compatible: > > + const: fsl,imx8qm-usb3 > > + > > + reg: > > + items: > > + - description: Address and length of the register set for iMX USB3 > Platform Control > > Drop "Address and length of the"... or actually just maxItems: 1, > because the description is a bit obvious. > > > + > > + "#address-cells": > > + enum: [ 1, 2 ] > > + > > + "#size-cells": > > + enum: [ 1, 2 ] > > + > > + ranges: true > > + > > + clocks: > > + description: > > + A list of phandle and clock-specifier pairs for the clocks > > + listed in clock-names. > > Drop description. > > > + items: > > + - description: Standby clock. Used during ultra low power states. > > + - description: USB bus clock for usb3 controller. > > + - description: AXI clock for AXI interface. > > + - description: ipg clock for register access. > > + - description: Core clock for usb3 controller. > > + > > + clock-names: > > + items: > > + - const: usb3_lpm_clk > > + - const: usb3_bus_clk > > + - const: usb3_aclk > > + - const: usb3_ipg_clk > > + - const: usb3_core_pclk > > + > > + assigned-clocks: > > + items: > > + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. > > + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MISC. > > + - description: Phandle and clock specifoer of > IMX_SC_PM_CLK_MST_BUS. > > + > > + assigned-clock-rates: > > + items: > > + - description: Must be 125 Mhz. > > + - description: Must be 12 Mhz. > > + - description: Must be 250 Mhz. > > I would argue that both properties above are not needed. If your > hardware requires fixed frequencies, clock provider can fix them, can't it? Clock provider don't know fixed value and turn on only used by client. > > > + > > + power-domains: > > + maxItems: 1 > > + > > +# Required child node: > > + > > +patternProperties: > > + "^usb@[0-9a-f]+$": > > + $ref: cdns,usb3.yaml# > > + > > +required: > > + - compatible > > + - reg > > + - "#address-cells" > > + - "#size-cells" > > + - ranges > > + - clocks > > + - clock-names > > + - power-domains > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/imx8-lpcg.h> > > + #include <dt-bindings/firmware/imx/rsrc.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + usbotg3: usb@5b110000 { > > Drop label, unused > > > + compatible = "fsl,imx8qm-usb3"; > > + ranges; > > + reg = <0x5b110000 0x10000>; > > reg is second property > > > + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, > > + <&usb3_lpcg IMX_LPCG_CLK_0>, > > + <&usb3_lpcg IMX_LPCG_CLK_7>, > > + <&usb3_lpcg IMX_LPCG_CLK_4>, > > + <&usb3_lpcg IMX_LPCG_CLK_5>; > > + clock-names = "usb3_lpm_clk", "usb3_bus_clk", "usb3_aclk", > > + "usb3_ipg_clk", "usb3_core_pclk"; > > + assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; > > + assigned-clock-rates = <125000000>, <12000000>, <250000000>; > > + power-domains = <&pd IMX_SC_R_USB_2>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + status = "disabled"; > > Drop status > > > + > > + usbotg3_cdns3: usb@5b120000 { > > Drop label > > > + compatible = "cdns,usb3"; > > + interrupt-parent = <&gic>; > > + interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "host", "peripheral", "otg", "wakeup"; > > + reg = <0x5b120000 0x10000>, /* memory area for OTG/DRD > registers */ > > + <0x5b130000 0x10000>, /* memory area for HOST registers */ > > + <0x5b140000 0x10000>; /* memory area for DEVICE registers */ > > + reg-names = "otg", "xhci", "dev"; > > reg is second property, reg-names third. > > > + phys = <&usb3_phy>; > > + phy-names = "cdns3,usb3-phy"; > > + }; > > + }; > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-17 14:55 ` [EXT] " Frank Li @ 2023-03-19 11:13 ` Krzysztof Kozlowski 2023-03-20 14:49 ` Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-19 11:13 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer On 17/03/2023 15:55, Frank Li wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: Friday, March 17, 2023 4:09 AM >> To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org >> Cc: devicetree@vger.kernel.org; festevam@gmail.com; imx@lists.linux.dev; >> kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm- >> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux- >> kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de >> Subject: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add >> imx8qm cdns3 glue bindings >> >> Caution: EXT Email >> >> On 16/03/2023 22:27, Frank Li wrote: >>> NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. >>> >> >> Subject: drop second/last, redundant "bindings". The "dt-bindings" >> prefix is already stating that these are bindings. >> >>> Signed-off-by: Frank Li <Frank.Li@nxp.com> >>> --- >>> .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 ++++++++++++++++++ >>> 1 file changed, 122 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm- >> cdns3.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm- >> cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm- >> cdns3.yaml >>> new file mode 100644 >>> index 000000000000..fc24df1e4483 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml >>> @@ -0,0 +1,122 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +# Copyright (c) 2020 NXP >>> +%YAML 1.2 >>> +--- >>> +$id: >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice >> tree.org%2Fschemas%2Fusb%2Ffsl%2Cimx8qm- >> cdns3.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9af3d617dc4cf >> 14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 >> 38146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD >> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C >> &sdata=EczZhjyMUGnnp7ZGfSvTj4lmOUuGlOtWYIsxxXIlNXw%3D&reserved >> =0 >>> +$schema: >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice >> tree.org%2Fmeta- >> schemas%2Fcore.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9a >> f3d617dc4cf14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C >> 0%7C0%7C638146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM >> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 >> %7C%7C%7C&sdata=uTNYuDm%2ByhZ56oQET2pX8sHpEqVvsUYtmOBCPXEK >> v40%3D&reserved=0 >>> + >>> +title: NXP iMX8QM Soc USB Controller >>> + >>> +maintainers: >>> + - Frank Li <Frank.Li@nxp.com> >>> + >>> +properties: >>> + compatible: >>> + const: fsl,imx8qm-usb3 >>> + >>> + reg: >>> + items: >>> + - description: Address and length of the register set for iMX USB3 >> Platform Control >> >> Drop "Address and length of the"... or actually just maxItems: 1, >> because the description is a bit obvious. >> >>> + >>> + "#address-cells": >>> + enum: [ 1, 2 ] >>> + >>> + "#size-cells": >>> + enum: [ 1, 2 ] >>> + >>> + ranges: true >>> + >>> + clocks: >>> + description: >>> + A list of phandle and clock-specifier pairs for the clocks >>> + listed in clock-names. >> >> Drop description. >> >>> + items: >>> + - description: Standby clock. Used during ultra low power states. >>> + - description: USB bus clock for usb3 controller. >>> + - description: AXI clock for AXI interface. >>> + - description: ipg clock for register access. >>> + - description: Core clock for usb3 controller. >>> + >>> + clock-names: >>> + items: >>> + - const: usb3_lpm_clk >>> + - const: usb3_bus_clk >>> + - const: usb3_aclk >>> + - const: usb3_ipg_clk >>> + - const: usb3_core_pclk >>> + >>> + assigned-clocks: >>> + items: >>> + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. >>> + - description: Phandle and clock specifoer of IMX_SC_PM_CLK_MISC. >>> + - description: Phandle and clock specifoer of >> IMX_SC_PM_CLK_MST_BUS. >>> + >>> + assigned-clock-rates: >>> + items: >>> + - description: Must be 125 Mhz. >>> + - description: Must be 12 Mhz. >>> + - description: Must be 250 Mhz. >> >> I would argue that both properties above are not needed. If your >> hardware requires fixed frequencies, clock provider can fix them, can't it? > > Clock provider don't know fixed value and turn on only used by client. So maybe fix the clock provider? Or this device driver? Requiring by binding specific frequencies for every board is a bit redundant. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-19 11:13 ` Krzysztof Kozlowski @ 2023-03-20 14:49 ` Frank Li 2023-03-20 15:23 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-20 14:49 UTC (permalink / raw) To: Krzysztof Kozlowski, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Sunday, March 19, 2023 6:13 AM > To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org > Cc: devicetree@vger.kernel.org; festevam@gmail.com; imx@lists.linux.dev; > kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm- > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux- > kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de > Subject: Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 17/03/2023 15:55, Frank Li wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Friday, March 17, 2023 4:09 AM > >> To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org > >> Cc: devicetree@vger.kernel.org; festevam@gmail.com; > imx@lists.linux.dev; > >> kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm- > >> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux- > >> kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de > >> Subject: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add > >> imx8qm cdns3 glue bindings > >> > >> Caution: EXT Email > >> > >> On 16/03/2023 22:27, Frank Li wrote: > >>> NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. > >>> > >> > >> Subject: drop second/last, redundant "bindings". The "dt-bindings" > >> prefix is already stating that these are bindings. > >> > >>> Signed-off-by: Frank Li <Frank.Li@nxp.com> > >>> --- > >>> .../bindings/usb/fsl,imx8qm-cdns3.yaml | 122 > ++++++++++++++++++ > >>> 1 file changed, 122 insertions(+) > >>> create mode 100644 > Documentation/devicetree/bindings/usb/fsl,imx8qm- > >> cdns3.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8qm- > >> cdns3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8qm- > >> cdns3.yaml > >>> new file mode 100644 > >>> index 000000000000..fc24df1e4483 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml > >>> @@ -0,0 +1,122 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +# Copyright (c) 2020 NXP > >>> +%YAML 1.2 > >>> +--- > >>> +$id: > >> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice > %2F&data=05%7C01%7Cfrank.li%40nxp.com%7Cc2d76d3694194fba130a08db > 286ae90e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381482118 > 66106607%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Lo > JUcnJnBaGjywN1zF%2BuUpFVUmldixTci96NPzVuio0%3D&reserved=0 > >> tree.org%2Fschemas%2Fusb%2Ffsl%2Cimx8qm- > >> > cdns3.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9af3d617dc4cf > >> > 14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > >> > 38146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > >> > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C > >> > &sdata=EczZhjyMUGnnp7ZGfSvTj4lmOUuGlOtWYIsxxXIlNXw%3D&reserved > >> =0 > >>> +$schema: > >> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice > %2F&data=05%7C01%7Cfrank.li%40nxp.com%7Cc2d76d3694194fba130a08db > 286ae90e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381482118 > 66106607%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Lo > JUcnJnBaGjywN1zF%2BuUpFVUmldixTci96NPzVuio0%3D&reserved=0 > >> tree.org%2Fmeta- > >> > schemas%2Fcore.yaml%23&data=05%7C01%7CFrank.Li%40nxp.com%7Cac9a > >> > f3d617dc4cf14baf08db26c74b07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > >> > 0%7C0%7C638146409617970248%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > >> > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > >> %7C%7C%7C&sdata=uTNYuDm%2ByhZ56oQET2pX8sHpEqVvsUYtmOBCPX > EK > >> v40%3D&reserved=0 > >>> + > >>> +title: NXP iMX8QM Soc USB Controller > >>> + > >>> +maintainers: > >>> + - Frank Li <Frank.Li@nxp.com> > >>> + > >>> +properties: > >>> + compatible: > >>> + const: fsl,imx8qm-usb3 > >>> + > >>> + reg: > >>> + items: > >>> + - description: Address and length of the register set for iMX USB3 > >> Platform Control > >> > >> Drop "Address and length of the"... or actually just maxItems: 1, > >> because the description is a bit obvious. > >> > >>> + > >>> + "#address-cells": > >>> + enum: [ 1, 2 ] > >>> + > >>> + "#size-cells": > >>> + enum: [ 1, 2 ] > >>> + > >>> + ranges: true > >>> + > >>> + clocks: > >>> + description: > >>> + A list of phandle and clock-specifier pairs for the clocks > >>> + listed in clock-names. > >> > >> Drop description. > >> > >>> + items: > >>> + - description: Standby clock. Used during ultra low power states. > >>> + - description: USB bus clock for usb3 controller. > >>> + - description: AXI clock for AXI interface. > >>> + - description: ipg clock for register access. > >>> + - description: Core clock for usb3 controller. > >>> + > >>> + clock-names: > >>> + items: > >>> + - const: usb3_lpm_clk > >>> + - const: usb3_bus_clk > >>> + - const: usb3_aclk > >>> + - const: usb3_ipg_clk > >>> + - const: usb3_core_pclk > >>> + > >>> + assigned-clocks: > >>> + items: > >>> + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. > >>> + - description: Phandle and clock specifoer of > IMX_SC_PM_CLK_MISC. > >>> + - description: Phandle and clock specifoer of > >> IMX_SC_PM_CLK_MST_BUS. > >>> + > >>> + assigned-clock-rates: > >>> + items: > >>> + - description: Must be 125 Mhz. > >>> + - description: Must be 12 Mhz. > >>> + - description: Must be 250 Mhz. > >> > >> I would argue that both properties above are not needed. If your > >> hardware requires fixed frequencies, clock provider can fix them, can't it? > > > > Clock provider don't know fixed value and turn on only used by client. > > So maybe fix the clock provider? Or this device driver? Requiring by > binding specific frequencies for every board is a bit redundant. It is not for every boards, it is common for a chip family. Only a place to set for QM and QXP. The similar case is network driver, which require a specific frequency at clock assign. Generally frequency is fixed, clock source name may change at difference chips. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 14:49 ` Frank Li @ 2023-03-20 15:23 ` Krzysztof Kozlowski 2023-03-20 16:22 ` Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-20 15:23 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer On 20/03/2023 15:49, Frank Li wrote: >>>>> + clock-names: >>>>> + items: >>>>> + - const: usb3_lpm_clk >>>>> + - const: usb3_bus_clk >>>>> + - const: usb3_aclk >>>>> + - const: usb3_ipg_clk >>>>> + - const: usb3_core_pclk >>>>> + >>>>> + assigned-clocks: >>>>> + items: >>>>> + - description: Phandle and clock specifier of IMX_SC_PM_CLK_PER. >>>>> + - description: Phandle and clock specifoer of >> IMX_SC_PM_CLK_MISC. >>>>> + - description: Phandle and clock specifoer of >>>> IMX_SC_PM_CLK_MST_BUS. >>>>> + >>>>> + assigned-clock-rates: >>>>> + items: >>>>> + - description: Must be 125 Mhz. >>>>> + - description: Must be 12 Mhz. >>>>> + - description: Must be 250 Mhz. >>>> >>>> I would argue that both properties above are not needed. If your >>>> hardware requires fixed frequencies, clock provider can fix them, can't it? >>> >>> Clock provider don't know fixed value and turn on only used by client. >> >> So maybe fix the clock provider? Or this device driver? Requiring by >> binding specific frequencies for every board is a bit redundant. > > It is not for every boards, it is common for a chip family. Only a place to set for > QM and QXP. > > The similar case is network driver, which require a specific frequency at clock assign. > Generally frequency is fixed, clock source name may change at difference chips. If frequency is always fixed, I don't understand why this is in DT bindings. I would even say it should not be in DTS. We don't put into DTS properties which are always the same, because otherwise they would grow crazy big. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 15:23 ` Krzysztof Kozlowski @ 2023-03-20 16:22 ` Frank Li 2023-03-20 16:27 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-20 16:22 UTC (permalink / raw) To: Krzysztof Kozlowski, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > >>>>> + assigned-clocks: > >>>>> + items: > >>>>> + - description: Phandle and clock specifier of > IMX_SC_PM_CLK_PER. > >>>>> + - description: Phandle and clock specifoer of > >> IMX_SC_PM_CLK_MISC. > >>>>> + - description: Phandle and clock specifoer of > >>>> IMX_SC_PM_CLK_MST_BUS. > >>>>> + > >>>>> + assigned-clock-rates: > >>>>> + items: > >>>>> + - description: Must be 125 Mhz. > >>>>> + - description: Must be 12 Mhz. > >>>>> + - description: Must be 250 Mhz. > >>>> > >>>> I would argue that both properties above are not needed. If your > >>>> hardware requires fixed frequencies, clock provider can fix them, can't > it? > >>> > >>> Clock provider don't know fixed value and turn on only used by client. > >> > >> So maybe fix the clock provider? Or this device driver? Requiring by > >> binding specific frequencies for every board is a bit redundant. > > > > It is not for every boards, it is common for a chip family. Only a place to set > for > > QM and QXP. > > > > The similar case is network driver, which require a specific frequency at > clock assign. > > Generally frequency is fixed, clock source name may change at difference > chips. > > If frequency is always fixed, I don't understand why this is in DT > bindings. I would even say it should not be in DTS. We don't put into > DTS properties which are always the same, because otherwise they would > grow crazy big. Although frequency is fixed, clock name may change for difference platform. assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; assigned-clock-rates = <125000000>, <12000000>, <250000000>; some platform use IMX_SC_R_USB_2, other platform may use IMX_SC_R_USB_3. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 16:22 ` Frank Li @ 2023-03-20 16:27 ` Krzysztof Kozlowski 2023-03-20 17:02 ` Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-20 16:27 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer On 20/03/2023 17:22, Frank Li wrote: >>>>>>> + assigned-clocks: >>>>>>> + items: >>>>>>> + - description: Phandle and clock specifier of >> IMX_SC_PM_CLK_PER. >>>>>>> + - description: Phandle and clock specifoer of >>>> IMX_SC_PM_CLK_MISC. >>>>>>> + - description: Phandle and clock specifoer of >>>>>> IMX_SC_PM_CLK_MST_BUS. >>>>>>> + >>>>>>> + assigned-clock-rates: >>>>>>> + items: >>>>>>> + - description: Must be 125 Mhz. >>>>>>> + - description: Must be 12 Mhz. >>>>>>> + - description: Must be 250 Mhz. >>>>>> >>>>>> I would argue that both properties above are not needed. If your >>>>>> hardware requires fixed frequencies, clock provider can fix them, can't >> it? >>>>> >>>>> Clock provider don't know fixed value and turn on only used by client. >>>> >>>> So maybe fix the clock provider? Or this device driver? Requiring by >>>> binding specific frequencies for every board is a bit redundant. >>> >>> It is not for every boards, it is common for a chip family. Only a place to set >> for >>> QM and QXP. >>> >>> The similar case is network driver, which require a specific frequency at >> clock assign. >>> Generally frequency is fixed, clock source name may change at difference >> chips. >> >> If frequency is always fixed, I don't understand why this is in DT >> bindings. I would even say it should not be in DTS. We don't put into >> DTS properties which are always the same, because otherwise they would >> grow crazy big. > > Although frequency is fixed, clock name may change for difference platform. > > assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; > assigned-clock-rates = <125000000>, <12000000>, <250000000>; > > some platform use IMX_SC_R_USB_2, other platform may use IMX_SC_R_USB_3. This I understand, you wrote it above, so nothing new and my concerns are still there. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 16:27 ` Krzysztof Kozlowski @ 2023-03-20 17:02 ` Frank Li 2023-03-20 17:28 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-20 17:02 UTC (permalink / raw) To: Krzysztof Kozlowski, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Monday, March 20, 2023 11:27 AM > To: Frank Li <frank.li@nxp.com>; shawnguo@kernel.org > Cc: devicetree@vger.kernel.org; festevam@gmail.com; imx@lists.linux.dev; > kernel@pengutronix.de; krzysztof.kozlowski+dt@linaro.org; linux-arm- > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; linux- > kernel@vger.kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de > Subject: Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 20/03/2023 17:22, Frank Li wrote: > >>>>>>> + assigned-clocks: > >>>>>>> + items: > >>>>>>> + - description: Phandle and clock specifier of > >> IMX_SC_PM_CLK_PER. > >>>>>>> + - description: Phandle and clock specifoer of > >>>> IMX_SC_PM_CLK_MISC. > >>>>>>> + - description: Phandle and clock specifoer of > >>>>>> IMX_SC_PM_CLK_MST_BUS. > >>>>>>> + > >>>>>>> + assigned-clock-rates: > >>>>>>> + items: > >>>>>>> + - description: Must be 125 Mhz. > >>>>>>> + - description: Must be 12 Mhz. > >>>>>>> + - description: Must be 250 Mhz. > >>>>>> > >>>>>> I would argue that both properties above are not needed. If your > >>>>>> hardware requires fixed frequencies, clock provider can fix them, > can't > >> it? > >>>>> > >>>>> Clock provider don't know fixed value and turn on only used by client. > >>>> > >>>> So maybe fix the clock provider? Or this device driver? Requiring by > >>>> binding specific frequencies for every board is a bit redundant. > >>> > >>> It is not for every boards, it is common for a chip family. Only a place to > set > >> for > >>> QM and QXP. > >>> > >>> The similar case is network driver, which require a specific frequency at > >> clock assign. > >>> Generally frequency is fixed, clock source name may change at > difference > >> chips. > >> > >> If frequency is always fixed, I don't understand why this is in DT > >> bindings. I would even say it should not be in DTS. We don't put into > >> DTS properties which are always the same, because otherwise they would > >> grow crazy big. > > > > Although frequency is fixed, clock name may change for difference > platform. > > > > assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > > <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > > <&clk IMX_SC_R_USB_2 > IMX_SC_PM_CLK_MST_BUS>; > > assigned-clock-rates = <125000000>, <12000000>, <250000000>; > > > > some platform use IMX_SC_R_USB_2, other platform may use > IMX_SC_R_USB_3. > > This I understand, you wrote it above, so nothing new and my concerns > are still there. I think Fixed value is not good reason. All reg base address, irq number are all for fixed number. The same Logic can be applied to irq-provider driver. But why still be descript in dts? It is hardware property. https://elixir.bootlin.com/linux/v4.8/source/Documentation/devicetree/bindings/clock/clock-bindings.txt have not said that can't set to fixed clock frequency. This is quick common case for network, USB, SATA, PCIE, which protocol defined Frequency. https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/ata/qcom-sata.txt https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml Such frequency information is necessary. We can put to dts or clock drivers. The clock driver Become bigger, or dts become bigger. I think the key point is if property to descript hardware information. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 17:02 ` Frank Li @ 2023-03-20 17:28 ` Krzysztof Kozlowski 2023-03-20 19:59 ` Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-20 17:28 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer On 20/03/2023 18:02, Frank Li wrote: >>> Although frequency is fixed, clock name may change for difference >> platform. >>> >>> assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, >>> <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, >>> <&clk IMX_SC_R_USB_2 >> IMX_SC_PM_CLK_MST_BUS>; >>> assigned-clock-rates = <125000000>, <12000000>, <250000000>; >>> >>> some platform use IMX_SC_R_USB_2, other platform may use >> IMX_SC_R_USB_3. >> >> This I understand, you wrote it above, so nothing new and my concerns >> are still there. > > I think Fixed value is not good reason. All reg base address, irq number are all for fixed number. The same No, because one device - IP block - could have different addresses, depending how it is wired/implemented in given SoC. Also our representation of devices in the kernel requires regs/interrupts coming from DTS, thus DTS is also answer to entire design of kernel and other SW. That's not the case here at all. > Logic can be applied to irq-provider driver. But why still be descript in dts? It is hardware property. > > https://elixir.bootlin.com/linux/v4.8/source/Documentation/devicetree/bindings/clock/clock-bindings.txt > have not said that can't set to fixed clock frequency. I don't understand this. > > This is quick common case for network, USB, SATA, PCIE, which protocol defined > Frequency. And they do not define fixed values in the bindings, so? There are exceptions, but it's usually not argument, right? > > https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/ata/qcom-sata.txt > https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml The second is a good example - as you can see, there is a choice of values, so they are not exactly fixed. > > Such frequency information is necessary. We can put to dts or clock drivers. The clock driver If this is the argument, then the answer is NAK. Sorry, but DTS is not for offloading fixed stuff just because you do not want to work on drivers. The same for discoverable stuff. > Become bigger, or dts become bigger. I think the key point is if property to descript hardware information. You have to understand that with your binding you are not allowing to any changes of these frequencies. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 17:28 ` Krzysztof Kozlowski @ 2023-03-20 19:59 ` Frank Li 2023-03-21 6:37 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-20 19:59 UTC (permalink / raw) To: Krzysztof Kozlowski, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > > The second is a good example - as you can see, there is a choice of > values, so they are not exactly fixed. > > > > > Such frequency information is necessary. We can put to dts or clock drivers. > The clock driver > > If this is the argument, then the answer is NAK. Sorry, but DTS is not > for offloading fixed stuff just because you do not want to work on > drivers. The same for discoverable stuff. > > > Become bigger, or dts become bigger. I think the key point is if property to > descript hardware information. > > You have to understand that with your binding you are not allowing to > any changes of these frequencies. Do you means it should be okay if one of clocks is not fixed? Previous owner already left nxp. I double checked our documents and scfw source code. I miss understood a clock SC_PM_CLK_MST_BUS, which actually mapped to IP's aclk, which ware 100Mhz to 600Mhz. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings 2023-03-20 19:59 ` Frank Li @ 2023-03-21 6:37 ` Krzysztof Kozlowski 0 siblings, 0 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-03-21 6:37 UTC (permalink / raw) To: Frank Li, shawnguo Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer On 20/03/2023 20:59, Frank Li wrote: >> >> The second is a good example - as you can see, there is a choice of >> values, so they are not exactly fixed. >> >>> >>> Such frequency information is necessary. We can put to dts or clock drivers. >> The clock driver >> >> If this is the argument, then the answer is NAK. Sorry, but DTS is not >> for offloading fixed stuff just because you do not want to work on >> drivers. The same for discoverable stuff. >> >>> Become bigger, or dts become bigger. I think the key point is if property to >> descript hardware information. >> >> You have to understand that with your binding you are not allowing to >> any changes of these frequencies. > > Do you means it should be okay if one of clocks is not fixed? We have here long discussion why does your binding require fixed frequencies, because this is something unusual and not recommended. And if they are really fixed, then device driver probably should make the choice of frequencies. > > Previous owner already left nxp. I double checked our documents and scfw source code. > I miss understood a clock SC_PM_CLK_MST_BUS, which actually mapped to IP's aclk, which > ware 100Mhz to 600Mhz. > >> >> Best regards, >> Krzysztof > Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support 2023-03-16 21:27 [PATCH v2 0/3] dts: imx8qxp add cdns usb3 port Frank Li 2023-03-16 21:27 ` [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings Frank Li @ 2023-03-16 21:27 ` Frank Li 2023-03-17 6:39 ` Alexander Stein 2023-03-16 21:27 ` [PATCH v2 3/3] arm64: dts: freescale: imx8qxp-mek: enable cadence usb3 Frank Li 2 siblings, 1 reply; 17+ messages in thread From: Frank Li @ 2023-03-16 21:27 UTC (permalink / raw) To: shawnguo Cc: Frank.Li, devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx, linux-kernel, robh+dt, s.hauer There are cadence usb3.0 controller in 8qxp and 8qm. Add usb3 node at common connect subsystem. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- .../boot/dts/freescale/imx8-ss-conn.dtsi | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi index 4852760adeee..389f52f16a5c 100644 --- a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi @@ -138,6 +138,56 @@ fec2: ethernet@5b050000 { status = "disabled"; }; + usbotg3: usb@5b110000 { + compatible = "fsl,imx8qm-usb3"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + reg = <0x5b110000 0x10000>; + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, + <&usb3_lpcg IMX_LPCG_CLK_0>, + <&usb3_lpcg IMX_LPCG_CLK_7>, + <&usb3_lpcg IMX_LPCG_CLK_4>, + <&usb3_lpcg IMX_LPCG_CLK_5>; + clock-names = "usb3_lpm_clk", "usb3_bus_clk", "usb3_aclk", + "usb3_ipg_clk", "usb3_core_pclk"; + assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; + assigned-clock-rates = <125000000>, <12000000>, <250000000>; + power-domains = <&pd IMX_SC_R_USB_2>; + status = "disabled"; + + usbotg3_cdns3: usb@5b120000 { + compatible = "cdns,usb3"; + #address-cells = <1>; + #size-cells = <1>; + interrupt-parent = <&gic>; + interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "host", "peripheral", "otg", "wakeup"; + reg = <0x5b130000 0x10000>, /* memory area for HOST registers */ + <0x5b140000 0x10000>, /* memory area for DEVICE registers */ + <0x5b120000 0x10000>; /* memory area for OTG/DRD registers */ + reg-names = "xhci", "dev", "otg"; + phys = <&usb3_phy>; + phy-names = "cdns3,usb3-phy"; + status = "disabled"; + }; + }; + + usb3_phy: usb-phy@5b160000 { + compatible = "nxp,salvo-phy"; + reg = <0x5b160000 0x40000>; + clocks = <&usb3_lpcg IMX_LPCG_CLK_6>; + clock-names = "salvo_phy_clk"; + power-domains = <&pd IMX_SC_R_USB_2_PHY>; + #phy-cells = <0>; + status = "disabled"; + }; + /* LPCG clocks */ sdhc0_lpcg: clock-controller@5b200000 { compatible = "fsl,imx8qxp-lpcg"; @@ -234,4 +284,26 @@ usb2_lpcg: clock-controller@5b270000 { clock-output-names = "usboh3_ahb_clk", "usboh3_phy_ipg_clk"; power-domains = <&pd IMX_SC_R_USB_0_PHY>; }; + + usb3_lpcg: clock-controller@5b280000 { + compatible = "fsl,imx8qxp-lpcg"; + reg = <0x5b280000 0x10000>; + #clock-cells = <1>; + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_1>, + <IMX_LPCG_CLK_4>, <IMX_LPCG_CLK_5>, + <IMX_LPCG_CLK_6>, <IMX_LPCG_CLK_7>; + clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, + <&conn_ipg_clk>, + <&conn_ipg_clk>, + <&conn_ipg_clk>, + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; + clock-output-names = "usb3_app_clk", + "usb3_lpm_clk", + "usb3_ipg_clk", + "usb3_core_pclk", + "usb3_phy_clk", + "usb3_aclk"; + power-domains = <&pd IMX_SC_R_USB_2_PHY>; + }; }; -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support 2023-03-16 21:27 ` [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support Frank Li @ 2023-03-17 6:39 ` Alexander Stein 2023-03-17 15:05 ` [EXT] " Frank Li 0 siblings, 1 reply; 17+ messages in thread From: Alexander Stein @ 2023-03-17 6:39 UTC (permalink / raw) To: shawnguo, linux-arm-kernel Cc: Frank.Li, devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx, linux-kernel, robh+dt, s.hauer, Frank Li Am Donnerstag, 16. März 2023, 22:27:10 CET schrieb Frank Li: > There are cadence usb3.0 controller in 8qxp and 8qm. > Add usb3 node at common connect subsystem. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > .../boot/dts/freescale/imx8-ss-conn.dtsi | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi index > 4852760adeee..389f52f16a5c 100644 > --- a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > @@ -138,6 +138,56 @@ fec2: ethernet@5b050000 { > status = "disabled"; > }; > > + usbotg3: usb@5b110000 { > + compatible = "fsl,imx8qm-usb3"; Mh, is imx8qm considered a subset of imx8qxp or vice versa? Maybe it's worth adding a dedicated compatible for imx8qxp as well. Best regards, Alexander > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + reg = <0x5b110000 0x10000>; > + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, > + <&usb3_lpcg IMX_LPCG_CLK_0>, > + <&usb3_lpcg IMX_LPCG_CLK_7>, > + <&usb3_lpcg IMX_LPCG_CLK_4>, > + <&usb3_lpcg IMX_LPCG_CLK_5>; > + clock-names = "usb3_lpm_clk", "usb3_bus_clk", "usb3_aclk", > + "usb3_ipg_clk", "usb3_core_pclk"; > + assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; > + assigned-clock-rates = <125000000>, <12000000>, <250000000>; > + power-domains = <&pd IMX_SC_R_USB_2>; > + status = "disabled"; > + > + usbotg3_cdns3: usb@5b120000 { > + compatible = "cdns,usb3"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <&gic>; > + interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "host", "peripheral", "otg", "wakeup"; > + reg = <0x5b130000 0x10000>, /* memory area for HOST registers */ > + <0x5b140000 0x10000>, /* memory area for DEVICE registers */ > + <0x5b120000 0x10000>; /* memory area for OTG/DRD registers */ > + reg-names = "xhci", "dev", "otg"; > + phys = <&usb3_phy>; > + phy-names = "cdns3,usb3-phy"; > + status = "disabled"; > + }; > + }; > + > + usb3_phy: usb-phy@5b160000 { > + compatible = "nxp,salvo-phy"; > + reg = <0x5b160000 0x40000>; > + clocks = <&usb3_lpcg IMX_LPCG_CLK_6>; > + clock-names = "salvo_phy_clk"; > + power-domains = <&pd IMX_SC_R_USB_2_PHY>; > + #phy-cells = <0>; > + status = "disabled"; > + }; > + > /* LPCG clocks */ > sdhc0_lpcg: clock-controller@5b200000 { > compatible = "fsl,imx8qxp-lpcg"; > @@ -234,4 +284,26 @@ usb2_lpcg: clock-controller@5b270000 { > clock-output-names = "usboh3_ahb_clk", "usboh3_phy_ipg_clk"; > power-domains = <&pd IMX_SC_R_USB_0_PHY>; > }; > + > + usb3_lpcg: clock-controller@5b280000 { > + compatible = "fsl,imx8qxp-lpcg"; > + reg = <0x5b280000 0x10000>; > + #clock-cells = <1>; > + clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_1>, > + <IMX_LPCG_CLK_4>, <IMX_LPCG_CLK_5>, > + <IMX_LPCG_CLK_6>, <IMX_LPCG_CLK_7>; > + clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>, > + <&conn_ipg_clk>, > + <&conn_ipg_clk>, > + <&conn_ipg_clk>, > + <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MST_BUS>; > + clock-output-names = "usb3_app_clk", > + "usb3_lpm_clk", > + "usb3_ipg_clk", > + "usb3_core_pclk", > + "usb3_phy_clk", > + "usb3_aclk"; > + power-domains = <&pd IMX_SC_R_USB_2_PHY>; > + }; > }; -- 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/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [EXT] Re: [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support 2023-03-17 6:39 ` Alexander Stein @ 2023-03-17 15:05 ` Frank Li 0 siblings, 0 replies; 17+ messages in thread From: Frank Li @ 2023-03-17 15:05 UTC (permalink / raw) To: Alexander Stein, shawnguo, linux-arm-kernel Cc: devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, dl-linux-imx, linux-kernel, robh+dt, s.hauer > > Caution: EXT Email > > Am Donnerstag, 16. März 2023, 22:27:10 CET schrieb Frank Li: > > There are cadence usb3.0 controller in 8qxp and 8qm. > > Add usb3 node at common connect subsystem. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > .../boot/dts/freescale/imx8-ss-conn.dtsi | 72 +++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > > b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi index > > 4852760adeee..389f52f16a5c 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi > > @@ -138,6 +138,56 @@ fec2: ethernet@5b050000 { > > status = "disabled"; > > }; > > > > + usbotg3: usb@5b110000 { > > + compatible = "fsl,imx8qm-usb3"; > > Mh, is imx8qm considered a subset of imx8qxp or vice versa? > Maybe it's worth adding a dedicated compatible for imx8qxp as well. Imx8qxp is subset of imx8qm. This part is the same between qxp and qm. If exact same, I think not necessary to add compatible string. > > Best regards, > Alexander > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + reg = <0x5b110000 0x10000>; > > + clocks = <&usb3_lpcg IMX_LPCG_CLK_1>, > > + <&usb3_lpcg IMX_LPCG_CLK_0>, > > + <&usb3_lpcg IMX_LPCG_CLK_7>, > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 3/3] arm64: dts: freescale: imx8qxp-mek: enable cadence usb3 2023-03-16 21:27 [PATCH v2 0/3] dts: imx8qxp add cdns usb3 port Frank Li 2023-03-16 21:27 ` [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings Frank Li 2023-03-16 21:27 ` [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support Frank Li @ 2023-03-16 21:27 ` Frank Li 2 siblings, 0 replies; 17+ messages in thread From: Frank Li @ 2023-03-16 21:27 UTC (permalink / raw) To: shawnguo Cc: Frank.Li, devicetree, festevam, imx, kernel, krzysztof.kozlowski+dt, linux-arm-kernel, linux-imx, linux-kernel, robh+dt, s.hauer Enable USB3 controller, phy and typec related nodes. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts index afa883389456..9ba4c72f0006 100644 --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts @@ -6,6 +6,7 @@ /dts-v1/; #include "imx8qxp.dtsi" +#include <dt-bindings/usb/pd.h> / { model = "Freescale i.MX8QXP MEK"; @@ -28,6 +29,21 @@ reg_usdhc2_vmmc: usdhc2-vmmc { gpio = <&lsio_gpio4 19 GPIO_ACTIVE_HIGH>; enable-active-high; }; + + gpio-sbu-mux { + compatible = "gpio-sbu-mux"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_typec_mux>; + select-gpios = <&lsio_gpio5 9 GPIO_ACTIVE_HIGH>; + enable-gpios = <&pca9557_a 7 GPIO_ACTIVE_LOW>; + orientation-switch; + + port { + usb3_data_ss: endpoint { + remote-endpoint = <&typec_con_ss>; + }; + }; + }; }; &dsp { @@ -127,6 +143,42 @@ light-sensor@44 { }; }; }; + + ptn5110: tcpc@50 { + compatible = "nxp,ptn5110"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_typec>; + reg = <0x50>; + interrupt-parent = <&lsio_gpio1>; + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; + + port { + typec_dr_sw: endpoint { + remote-endpoint = <&usb3_drd_sw>; + }; + }; + + usb_con1: connector { + compatible = "usb-c-connector"; + label = "USB-C"; + power-role = "source"; + data-role = "dual"; + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@1 { + reg = <1>; + typec_con_ss: endpoint { + remote-endpoint = <&usb3_data_ss>; + }; + }; + }; + }; + }; + }; &lpuart0 { @@ -204,6 +256,27 @@ &usdhc2 { status = "okay"; }; +&usb3_phy { + status = "okay"; +}; + +&usbotg3 { + status = "okay"; +}; + +&usbotg3_cdns3 { + dr_mode = "otg"; + usb-role-switch; + status = "okay"; + + port { + usb3_drd_sw: endpoint { + remote-endpoint = <&typec_dr_sw>; + }; + }; +}; + + &vpu { compatible = "nxp,imx8qxp-vpu"; status = "okay"; @@ -267,6 +340,18 @@ IMX8QXP_UART0_TX_ADMA_UART0_TX 0x06000020 >; }; + pinctrl_typec: typecgrp { + fsl,pins = < + IMX8QXP_SPI2_SCK_LSIO_GPIO1_IO03 0x06000021 + >; + }; + + pinctrl_typec_mux: typecmuxgrp { + fsl,pins = < + IMX8QXP_ENET0_REFCLK_125M_25M_LSIO_GPIO5_IO09 0x60 + >; + }; + pinctrl_usdhc1: usdhc1grp { fsl,pins = < IMX8QXP_EMMC0_CLK_CONN_EMMC0_CLK 0x06000041 -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-03-21 6:38 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-16 21:27 [PATCH v2 0/3] dts: imx8qxp add cdns usb3 port Frank Li 2023-03-16 21:27 ` [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings Frank Li 2023-03-17 9:09 ` Krzysztof Kozlowski 2023-03-17 14:55 ` [EXT] " Frank Li 2023-03-19 11:13 ` Krzysztof Kozlowski 2023-03-20 14:49 ` Frank Li 2023-03-20 15:23 ` Krzysztof Kozlowski 2023-03-20 16:22 ` Frank Li 2023-03-20 16:27 ` Krzysztof Kozlowski 2023-03-20 17:02 ` Frank Li 2023-03-20 17:28 ` Krzysztof Kozlowski 2023-03-20 19:59 ` Frank Li 2023-03-21 6:37 ` Krzysztof Kozlowski 2023-03-16 21:27 ` [PATCH v2 2/3] arm64: dts: imx8qxp: add cadence usb3 support Frank Li 2023-03-17 6:39 ` Alexander Stein 2023-03-17 15:05 ` [EXT] " Frank Li 2023-03-16 21:27 ` [PATCH v2 3/3] arm64: dts: freescale: imx8qxp-mek: enable cadence usb3 Frank Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).