| Message ID | 20230321151951.2784286-2-Frank.Li@nxp.com |
|---|---|
| State | Accepted |
| Commit | 8486eb8068ccfea99864544327a7d18f8a1e231e |
| Headers | show |
| Series | [v3,1/3] dt-bindings: usb: cdns-imx8qm: add imx8qm cdns3 glue bindings | expand |
On 22/03/2023 15:34, Frank Li wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: Wednesday, March 22, 2023 2:32 AM >> To: Frank Li <frank.li@nxp. >>> + - const: usb3_aclk >>> + - const: usb3_ipg_clk >>> + - const: usb3_core_pclk >>> + >>> + assigned-clocks: >>> + items: >>> + - description: Phandle and clock specifoer of >> IMX_SC_PM_CLK_MST_BUS. >> >> Drop useless pieces so "Phandle and clock specifoer of " and name the >> hardware, not the syntax. >> >>> + >>> + assigned-clock-rates: >>> + items: >>> + - description: Should be in Range 100 - 600 Mhz. >> >> That's better but I still do not understand why do you need it in the >> bindings. You never actually answered this question. > > I am not sure 100% sure the reason. > I think difference system target's axi bus frequency is difference, > And just one time work, needn't software to manage it. > Following other driver's code style may be another reason. That's the reason of heaving it in DTS. But I am asking about bindings. You do understand you define here interface? Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, March 22, 2023 4:32 PM > To: Frank Li <frank.li@nxp.com> > 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; > shawnguo@kernel.org > Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 22/03/2023 15:34, Frank Li wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Wednesday, March 22, 2023 2:32 AM > >> To: Frank Li <frank.li@nxp. > >>> + - const: usb3_aclk > >>> + - const: usb3_ipg_clk > >>> + - const: usb3_core_pclk > >>> + > >>> + assigned-clocks: > >>> + items: > >>> + - description: Phandle and clock specifoer of > >> IMX_SC_PM_CLK_MST_BUS. > >> > >> Drop useless pieces so "Phandle and clock specifoer of " and name the > >> hardware, not the syntax. > >> > >>> + > >>> + assigned-clock-rates: > >>> + items: > >>> + - description: Should be in Range 100 - 600 Mhz. > >> > >> That's better but I still do not understand why do you need it in the > >> bindings. You never actually answered this question. > > > > I am not sure 100% sure the reason. > > I think difference system target's axi bus frequency is difference, > > And just one time work, needn't software to manage it. > > Following other driver's code style may be another reason. > > That's the reason of heaving it in DTS. But I am asking about bindings. > You do understand you define here interface? I defined here is descript AXI frequency for usb controller. Supposed difference Platform will have difference working frequency. > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, March 22, 2023 4:38 PM > To: Frank Li <frank.li@nxp.com> > 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; > shawnguo@kernel.org > Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 22/03/2023 22:36, Frank Li wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Wednesday, March 22, 2023 4:32 PM > >> To: Frank Li <frank.li@nxp.com> > >> 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; > >> shawnguo@kernel.org > >> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > >> imx8qm cdns3 glue bindings > >> > >> Caution: EXT Email > >> > >> On 22/03/2023 15:34, Frank Li wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>> Sent: Wednesday, March 22, 2023 2:32 AM > >>>> To: Frank Li <frank.li@nxp. > >>>>> + - const: usb3_aclk > >>>>> + - const: usb3_ipg_clk > >>>>> + - const: usb3_core_pclk > >>>>> + > >>>>> + assigned-clocks: > >>>>> + items: > >>>>> + - description: Phandle and clock specifoer of > >>>> IMX_SC_PM_CLK_MST_BUS. > >>>> > >>>> Drop useless pieces so "Phandle and clock specifoer of " and name the > >>>> hardware, not the syntax. > >>>> > >>>>> + > >>>>> + assigned-clock-rates: > >>>>> + items: > >>>>> + - description: Should be in Range 100 - 600 Mhz. > >>>> > >>>> That's better but I still do not understand why do you need it in the > >>>> bindings. You never actually answered this question. > >>> > >>> I am not sure 100% sure the reason. > >>> I think difference system target's axi bus frequency is difference, > >>> And just one time work, needn't software to manage it. > >>> Following other driver's code style may be another reason. > >> > >> That's the reason of heaving it in DTS. But I am asking about bindings. > >> You do understand you define here interface? > > > > I defined here is descript AXI frequency for usb controller. Supposed > difference > > Platform will have difference working frequency. > > I don't understand how does this answer my concerns of having it in DT > bindings. If you do not add it, you "will have difference working > frequency", so what's the point? For example: imx8qxp, it need set to 250Mhz, i.MX8QM need set to 200Mhz. Maybe future chip can set to 400Mhz. > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, March 22, 2023 4:43 PM > To: Frank Li <frank.li@nxp.com> > 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; > shawnguo@kernel.org > Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 22/03/2023 22:40, Frank Li wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Wednesday, March 22, 2023 4:38 PM > >> To: Frank Li <frank.li@nxp.com> > >> 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; > >> shawnguo@kernel.org > >> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > >> imx8qm cdns3 glue bindings > >> > >> Caution: EXT Email > >> > >> On 22/03/2023 22:36, Frank Li wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>> Sent: Wednesday, March 22, 2023 4:32 PM > >>>> To: Frank Li <frank.li@nxp.com> > >>>> 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; > >>>> shawnguo@kernel.org > >>>> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: > add > >>>> imx8qm cdns3 glue bindings > >>>> > >>>> Caution: EXT Email > >>>> > >>>> On 22/03/2023 15:34, Frank Li wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>>>> Sent: Wednesday, March 22, 2023 2:32 AM > >>>>>> To: Frank Li <frank.li@nxp. > >>>>>>> + - const: usb3_aclk > >>>>>>> + - const: usb3_ipg_clk > >>>>>>> + - const: usb3_core_pclk > >>>>>>> + > >>>>>>> + assigned-clocks: > >>>>>>> + items: > >>>>>>> + - description: Phandle and clock specifoer of > >>>>>> IMX_SC_PM_CLK_MST_BUS. > >>>>>> > >>>>>> Drop useless pieces so "Phandle and clock specifoer of " and name > the > >>>>>> hardware, not the syntax. > >>>>>> > >>>>>>> + > >>>>>>> + assigned-clock-rates: > >>>>>>> + items: > >>>>>>> + - description: Should be in Range 100 - 600 Mhz. > >>>>>> > >>>>>> That's better but I still do not understand why do you need it in the > >>>>>> bindings. You never actually answered this question. > >>>>> > >>>>> I am not sure 100% sure the reason. > >>>>> I think difference system target's axi bus frequency is difference, > >>>>> And just one time work, needn't software to manage it. > >>>>> Following other driver's code style may be another reason. > >>>> > >>>> That's the reason of heaving it in DTS. But I am asking about bindings. > >>>> You do understand you define here interface? > >>> > >>> I defined here is descript AXI frequency for usb controller. Supposed > >> difference > >>> Platform will have difference working frequency. > >> > >> I don't understand how does this answer my concerns of having it in DT > >> bindings. If you do not add it, you "will have difference working > >> frequency", so what's the point? > > > > For example: imx8qxp, it need set to 250Mhz, i.MX8QM need set to > 200Mhz. > > Maybe future chip can set to 400Mhz. > > And? So as you can see you will still have different frequencies, so > what's the point? What is the benefit? Dunno, maybe we do not understand > each other, because I don't think you are answering my questions at all. Benefit: New chip just need change dts file for the same IP, like change base Reg address and irq number. Your question is: "why need this assigned-clock-rates IMX_SC_PM_CLK_MST_BUS property?" My answer: it is one of hardware property, like reg base address and irq number. If can't match your expectation, can you change another words or provide me an example? > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, March 22, 2023 5:09 PM > To: Frank Li <frank.li@nxp.com> > 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; > shawnguo@kernel.org > Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > imx8qm cdns3 glue bindings > > Caution: EXT Email > > On 22/03/2023 22:57, Frank Li wrote: > > > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: Wednesday, March 22, 2023 4:43 PM > >> To: Frank Li <frank.li@nxp.com> > >> 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; > >> shawnguo@kernel.org > >> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: add > >> imx8qm cdns3 glue bindings > >> > >> Caution: EXT Email > >> > >> On 22/03/2023 22:40, Frank Li wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>> Sent: Wednesday, March 22, 2023 4:38 PM > >>>> To: Frank Li <frank.li@nxp.com> > >>>> 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; > >>>> shawnguo@kernel.org > >>>> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: > add > >>>> imx8qm cdns3 glue bindings > >>>> > >>>> Caution: EXT Email > >>>> > >>>> On 22/03/2023 22:36, Frank Li wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>>>> Sent: Wednesday, March 22, 2023 4:32 PM > >>>>>> To: Frank Li <frank.li@nxp.com> > >>>>>> 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; > >>>>>> shawnguo@kernel.org > >>>>>> Subject: Re: [EXT] Re: [PATCH v3 1/3] dt-bindings: usb: cdns-imx8qm: > >> add > >>>>>> imx8qm cdns3 glue bindings > >>>>>> > >>>>>> Caution: EXT Email > >>>>>> > >>>>>> On 22/03/2023 15:34, Frank Li wrote: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>>>>>>> Sent: Wednesday, March 22, 2023 2:32 AM > >>>>>>>> To: Frank Li <frank.li@nxp. > >>>>>>>>> + - const: usb3_aclk > >>>>>>>>> + - const: usb3_ipg_clk > >>>>>>>>> + - const: usb3_core_pclk > >>>>>>>>> + > >>>>>>>>> + assigned-clocks: > >>>>>>>>> + items: > >>>>>>>>> + - description: Phandle and clock specifoer of > >>>>>>>> IMX_SC_PM_CLK_MST_BUS. > >>>>>>>> > >>>>>>>> Drop useless pieces so "Phandle and clock specifoer of " and > name > >> the > >>>>>>>> hardware, not the syntax. > >>>>>>>> > >>>>>>>>> + > >>>>>>>>> + assigned-clock-rates: > >>>>>>>>> + items: > >>>>>>>>> + - description: Should be in Range 100 - 600 Mhz. > >>>>>>>> > >>>>>>>> That's better but I still do not understand why do you need it in > the > >>>>>>>> bindings. You never actually answered this question. > >>>>>>> > >>>>>>> I am not sure 100% sure the reason. > >>>>>>> I think difference system target's axi bus frequency is difference, > >>>>>>> And just one time work, needn't software to manage it. > >>>>>>> Following other driver's code style may be another reason. > >>>>>> > >>>>>> That's the reason of heaving it in DTS. But I am asking about bindings. > >>>>>> You do understand you define here interface? > >>>>> > >>>>> I defined here is descript AXI frequency for usb controller. Supposed > >>>> difference > >>>>> Platform will have difference working frequency. > >>>> > >>>> I don't understand how does this answer my concerns of having it in DT > >>>> bindings. If you do not add it, you "will have difference working > >>>> frequency", so what's the point? > >>> > >>> For example: imx8qxp, it need set to 250Mhz, i.MX8QM need set to > >> 200Mhz. > >>> Maybe future chip can set to 400Mhz. > >> > >> And? So as you can see you will still have different frequencies, so > >> what's the point? What is the benefit? Dunno, maybe we do not > understand > >> each other, because I don't think you are answering my questions at all. > > > > Benefit: New chip just need change dts file for the same IP, like change > base > > Reg address and irq number. > > To remind - the question was: > "That's better but I still do not understand why do you need it in the > bindings." > If you drop it from the bindings the benefit is still there, so what do > you want to prove? > > > > > Your question is: "why need this assigned-clock-rates > IMX_SC_PM_CLK_MST_BUS property?" > > This was the previous thread. Now, related but slightly different, why > do you still need it in the bindings? > > > > My answer: it is one of hardware property, like reg base address and irq > number. > > Sure, it is, I know, and bindings already allow it. Just look at many > DTS and their bindings. Do you see the bindings defining this property? > No. So why do you think it is needed here? I am asking this since like 6 > emails and your answers are not related to bindings at all. > > > > > If can't match your expectation, can you change another words or provide > me an example? > > Yeah, just open several DTS and look for assigned-clock, then open their > bindings and answer - why do you need to add it to the binding but all > other bindings did not have to? If you have the answer, sure, bring > these parts of bindings. Do you means, Needn't add assigned-clock in binding *document* yaml file? And dts file still can use assigned-clock property. > > Best regards, > Krzysztof
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..d876e3dab608 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml @@ -0,0 +1,112 @@ +# 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: Register set for iMX USB3 Platform Control + + "#address-cells": + enum: [ 1, 2 ] + + "#size-cells": + enum: [ 1, 2 ] + + ranges: true + + clocks: + 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 specifoer of IMX_SC_PM_CLK_MST_BUS. + + assigned-clock-rates: + items: + - description: Should be in Range 100 - 600 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> + + usb@5b110000 { + compatible = "fsl,imx8qm-usb3"; + reg = <0x5b110000 0x10000>; + ranges; + 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_MST_BUS>; + assigned-clock-rates = <250000000>; + power-domains = <&pd IMX_SC_R_USB_2>; + #address-cells = <1>; + #size-cells = <1>; + + usb@5b120000 { + compatible = "cdns,usb3"; + 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"; + 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"; + phys = <&usb3_phy>; + phy-names = "cdns3,usb3-phy"; + }; + };
NXP imx8qm integrates 1 cdns3 IP. This is glue layer device bindings. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Change from v2 to v3 - Drop two fixed frequency clocks, it is system reset value, no need set now. If need, futher work/discuss on driver or dts change. It will not block this basic enablement work. - Drop lable - Drop some descriptions - Reg as second property. Change from v1 to v2. - new add binding doc .../bindings/usb/fsl,imx8qm-cdns3.yaml | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.yaml