| Message ID | 20240221145846.1611627-4-xu.yang_2@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v6,1/9] dt-bindings: usb: usbmisc-imx: add fsl,imx8ulp-usbmisc compatible | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, Feb 21, 2024 at 10:58:41PM +0800, Xu Yang wrote: > As more and more NXP i.MX chips come out, it becomes harder to maintain > ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to > this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add > a new schema for NXP ChipIdea USB2 Controller. > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > Changes in v6: > - new patch > --- > .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > new file mode 100644 > index 000000000000..2ec62f564bf5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/usb/ci-hdrc-usb2-imx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP USB2 ChipIdea USB controller > + > +maintainers: > + - Xu Yang <xu.yang_2@nxp.com> > + > +properties: > + compatible: > + oneOf: > + - enum: > + - fsl,imx27-usb > + - items: > + - enum: > + - fsl,imx23-usb > + - fsl,imx25-usb > + - fsl,imx28-usb > + - fsl,imx35-usb > + - fsl,imx50-usb > + - fsl,imx51-usb > + - fsl,imx53-usb > + - fsl,imx6q-usb > + - fsl,imx6sl-usb > + - fsl,imx6sx-usb > + - fsl,imx6ul-usb > + - fsl,imx7d-usb > + - fsl,vf610-usb > + - const: fsl,imx27-usb > + - items: > + - enum: > + - fsl,imx8dxl-usb > + - fsl,imx8ulp-usb > + - const: fsl,imx7ulp-usb > + - const: fsl,imx6ul-usb > + - items: > + - enum: > + - fsl,imx8mm-usb > + - fsl,imx8mn-usb > + - const: fsl,imx7d-usb > + - const: fsl,imx27-usb > + - items: > + - enum: > + - fsl,imx6sll-usb > + - fsl,imx7ulp-usb > + - const: fsl,imx6ul-usb > + - const: fsl,imx27-usb Now you just duplicated all the compatibles, and now any new compatibles have to be added in 2 places. For this to work, you have to split ci-hdrc-usb2.yaml into 2 files. One with all the common properties and one with compatibles (minus imx). This is also needed if imx has any extra properties the other don't. Didn't I say this already? Rob
Hi Rob, > > On Wed, Feb 21, 2024 at 10:58:41PM +0800, Xu Yang wrote: > > As more and more NXP i.MX chips come out, it becomes harder to maintain > > ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to > > this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add > > a new schema for NXP ChipIdea USB2 Controller. > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > > > --- > > Changes in v6: > > - new patch > > --- > > .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml b/Documentation/devicetree/bindings/usb/ci- > hdrc-usb2-imx.yaml > > new file mode 100644 > > index 000000000000..2ec62f564bf5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > @@ -0,0 +1,75 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/usb/ci-hdrc- > usb2- > imx.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3bc2b4c6fa92c > d99c5c301635%7C0%7C0%7C638442937830606824%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi > LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7p1DzvYmBsTgN44jypH7lc56z9hVBsFBYXUwsblk9z8%3D&reserv > ed=0 > > +$schema: http://devicetree.org/meta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3 > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638442937830615622%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=fWWy9enbGK5yeKiovday7go3Gss5L%2F%2Fe%2F > OZcANny0QA%3D&reserved=0 > > + > > +title: NXP USB2 ChipIdea USB controller > > + > > +maintainers: > > + - Xu Yang <xu.yang_2@nxp.com> > > + > > +properties: > > + compatible: > > + oneOf: > > + - enum: > > + - fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx23-usb > > + - fsl,imx25-usb > > + - fsl,imx28-usb > > + - fsl,imx35-usb > > + - fsl,imx50-usb > > + - fsl,imx51-usb > > + - fsl,imx53-usb > > + - fsl,imx6q-usb > > + - fsl,imx6sl-usb > > + - fsl,imx6sx-usb > > + - fsl,imx6ul-usb > > + - fsl,imx7d-usb > > + - fsl,vf610-usb > > + - const: fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx8dxl-usb > > + - fsl,imx8ulp-usb > > + - const: fsl,imx7ulp-usb > > + - const: fsl,imx6ul-usb > > + - items: > > + - enum: > > + - fsl,imx8mm-usb > > + - fsl,imx8mn-usb > > + - const: fsl,imx7d-usb > > + - const: fsl,imx27-usb > > + - items: > > + - enum: > > + - fsl,imx6sll-usb > > + - fsl,imx7ulp-usb > > + - const: fsl,imx6ul-usb > > + - const: fsl,imx27-usb > > Now you just duplicated all the compatibles, and now any new compatibles > have to be added in 2 places. For this to work, you have to split > ci-hdrc-usb2.yaml into 2 files. One with all the common properties and > one with compatibles (minus imx). This is also needed if imx has any > extra properties the other don't. > > Didn't I say this already? > Yes, I know. But according to your words, I need to split ci-hdrc-usb2.yaml into 1 common file and more than 1 vendor specific files (imx, nvidia, qcom, nuvoton and others). In this patchset, I only focus on imx part and KK said he or someone will take over other parts, therefore I just duplicated all the imx compatibles. If I only create imx specific yaml file and remove all compatilbles from common file, nvidia, qcom, nuvoton and others compatible info will be lost, is this feasible? Or should I create mutiple vendor specific files at the same time? Thanks, Xu Yang
On Fri, Feb 23, 2024 at 7:56 AM Xu Yang <xu.yang_2@nxp.com> wrote: > > Hi Rob, > > > > > On Wed, Feb 21, 2024 at 10:58:41PM +0800, Xu Yang wrote: > > > As more and more NXP i.MX chips come out, it becomes harder to maintain > > > ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to > > > this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add > > > a new schema for NXP ChipIdea USB2 Controller. > > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > > > > > --- > > > Changes in v6: > > > - new patch > > > --- > > > .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ > > > 1 file changed, 75 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml b/Documentation/devicetree/bindings/usb/ci- > > hdrc-usb2-imx.yaml > > > new file mode 100644 > > > index 000000000000..2ec62f564bf5 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > > > @@ -0,0 +1,75 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/usb/ci-hdrc- > > usb2- > > imx.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3bc2b4c6fa92c > > d99c5c301635%7C0%7C0%7C638442937830606824%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi > > LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7p1DzvYmBsTgN44jypH7lc56z9hVBsFBYXUwsblk9z8%3D&reserv > > ed=0 > > > +$schema: http://devicetree.org/meta- > > schemas%2Fcore.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3 > > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638442937830615622%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > > JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=fWWy9enbGK5yeKiovday7go3Gss5L%2F%2Fe%2F > > OZcANny0QA%3D&reserved=0 > > > + > > > +title: NXP USB2 ChipIdea USB controller > > > + > > > +maintainers: > > > + - Xu Yang <xu.yang_2@nxp.com> > > > + > > > +properties: > > > + compatible: > > > + oneOf: > > > + - enum: > > > + - fsl,imx27-usb > > > + - items: > > > + - enum: > > > + - fsl,imx23-usb > > > + - fsl,imx25-usb > > > + - fsl,imx28-usb > > > + - fsl,imx35-usb > > > + - fsl,imx50-usb > > > + - fsl,imx51-usb > > > + - fsl,imx53-usb > > > + - fsl,imx6q-usb > > > + - fsl,imx6sl-usb > > > + - fsl,imx6sx-usb > > > + - fsl,imx6ul-usb > > > + - fsl,imx7d-usb > > > + - fsl,vf610-usb > > > + - const: fsl,imx27-usb > > > + - items: > > > + - enum: > > > + - fsl,imx8dxl-usb > > > + - fsl,imx8ulp-usb > > > + - const: fsl,imx7ulp-usb > > > + - const: fsl,imx6ul-usb > > > + - items: > > > + - enum: > > > + - fsl,imx8mm-usb > > > + - fsl,imx8mn-usb > > > + - const: fsl,imx7d-usb > > > + - const: fsl,imx27-usb > > > + - items: > > > + - enum: > > > + - fsl,imx6sll-usb > > > + - fsl,imx7ulp-usb > > > + - const: fsl,imx6ul-usb > > > + - const: fsl,imx27-usb > > > > Now you just duplicated all the compatibles, and now any new compatibles > > have to be added in 2 places. For this to work, you have to split > > ci-hdrc-usb2.yaml into 2 files. One with all the common properties and > > one with compatibles (minus imx). This is also needed if imx has any > > extra properties the other don't. > > > > Didn't I say this already? > > > > Yes, I know. > > But according to your words, I need to split ci-hdrc-usb2.yaml into 1 common > file and more than 1 vendor specific files (imx, nvidia, qcom, nuvoton and > others). In this patchset, I only focus on imx part and KK said he or someone > will take over other parts, therefore I just duplicated all the imx compatibles. This series is just wasting our time because it can't be applied without that. > If I only create imx specific yaml file and remove all compatilbles from common > file, nvidia, qcom, nuvoton and others compatible info will be lost, is this > feasible? Or should I create mutiple vendor specific files at the same time? You don't have to split each vendor to a separate vendor schema file. Just move everything common to ci-hdrc-usb2-common.yaml and add a reference to it. Then imx can reference the common file. Rob
On 23/02/2024 15:56, Xu Yang wrote: > Hi Rob, > >> >> On Wed, Feb 21, 2024 at 10:58:41PM +0800, Xu Yang wrote: >>> As more and more NXP i.MX chips come out, it becomes harder to maintain >>> ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to >>> this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add >>> a new schema for NXP ChipIdea USB2 Controller. >>> >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> >>> >>> --- >>> Changes in v6: >>> - new patch >>> --- >>> .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ >>> 1 file changed, 75 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml b/Documentation/devicetree/bindings/usb/ci- >> hdrc-usb2-imx.yaml >>> new file mode 100644 >>> index 000000000000..2ec62f564bf5 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml >>> @@ -0,0 +1,75 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/usb/ci-hdrc- >> usb2- >> imx.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3bc2b4c6fa92c >> d99c5c301635%7C0%7C0%7C638442937830606824%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi >> LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7p1DzvYmBsTgN44jypH7lc56z9hVBsFBYXUwsblk9z8%3D&reserv >> ed=0 >>> +$schema: http://devicetree.org/meta- >> schemas%2Fcore.yaml%23&data=05%7C02%7Cxu.yang_2%40nxp.com%7C4ac0c60cd4b4433f0f9f08dc34782572%7C686ea1d3 >> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638442937830615622%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC >> JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=fWWy9enbGK5yeKiovday7go3Gss5L%2F%2Fe%2F >> OZcANny0QA%3D&reserved=0 >>> + >>> +title: NXP USB2 ChipIdea USB controller >>> + >>> +maintainers: >>> + - Xu Yang <xu.yang_2@nxp.com> >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - enum: >>> + - fsl,imx27-usb >>> + - items: >>> + - enum: >>> + - fsl,imx23-usb >>> + - fsl,imx25-usb >>> + - fsl,imx28-usb >>> + - fsl,imx35-usb >>> + - fsl,imx50-usb >>> + - fsl,imx51-usb >>> + - fsl,imx53-usb >>> + - fsl,imx6q-usb >>> + - fsl,imx6sl-usb >>> + - fsl,imx6sx-usb >>> + - fsl,imx6ul-usb >>> + - fsl,imx7d-usb >>> + - fsl,vf610-usb >>> + - const: fsl,imx27-usb >>> + - items: >>> + - enum: >>> + - fsl,imx8dxl-usb >>> + - fsl,imx8ulp-usb >>> + - const: fsl,imx7ulp-usb >>> + - const: fsl,imx6ul-usb >>> + - items: >>> + - enum: >>> + - fsl,imx8mm-usb >>> + - fsl,imx8mn-usb >>> + - const: fsl,imx7d-usb >>> + - const: fsl,imx27-usb >>> + - items: >>> + - enum: >>> + - fsl,imx6sll-usb >>> + - fsl,imx7ulp-usb >>> + - const: fsl,imx6ul-usb >>> + - const: fsl,imx27-usb >> >> Now you just duplicated all the compatibles, and now any new compatibles >> have to be added in 2 places. For this to work, you have to split >> ci-hdrc-usb2.yaml into 2 files. One with all the common properties and >> one with compatibles (minus imx). This is also needed if imx has any >> extra properties the other don't. >> >> Didn't I say this already? >> > > Yes, I know. > > But according to your words, I need to split ci-hdrc-usb2.yaml into 1 common > file and more than 1 vendor specific files (imx, nvidia, qcom, nuvoton and > others). In this patchset, I only focus on imx part and KK said he or someone > will take over other parts, therefore I just duplicated all the imx compatibles. Someone will take over their bits, but it does not mean you can duplicate IMX. Why you cannot use the approach I asked - move IMX out of that schema? https://lore.kernel.org/all/a0134089-a283-488b-8d7f-3f59dd938b60@linaro.org/ Look: "... and move IMX to own file." Move is not "copy". Please run: `man mv` > If I only create imx specific yaml file and remove all compatilbles from common Why would you remove ALL of others? What is the point of this. > file, nvidia, qcom, nuvoton and others compatible info will be lost, is this > feasible? Or should I create mutiple vendor specific files at the same time? You can take a look how it is done in my recent Qualcomm PCI dt-bindings series. Best regards, Krzysztof
> > On 23/02/2024 15:56, Xu Yang wrote: > > Hi Rob, > > > >> > >> On Wed, Feb 21, 2024 at 10:58:41PM +0800, Xu Yang wrote: > >>> As more and more NXP i.MX chips come out, it becomes harder to maintain > >>> ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to > >>> this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add > >>> a new schema for NXP ChipIdea USB2 Controller. > >>> > >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > >>> > >>> --- > >>> Changes in v6: > >>> - new patch > >>> --- > >>> .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ > >>> 1 file changed, 75 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > b/Documentation/devicetree/bindings/usb/ci- > >> hdrc-usb2-imx.yaml > >>> new file mode 100644 > >>> index 000000000000..2ec62f564bf5 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml > >>> @@ -0,0 +1,75 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 [...] > > > If I only create imx specific yaml file and remove all compatilbles from common > > Why would you remove ALL of others? What is the point of this. I originally thought ci-hdrc-usb2.yaml should be removed and only ci-hdrc-usb2-comm.yaml is kept. So I'm not sure where other compatibles should go. I looked though your Qualcomm PCI dt-bindings series and now I know how to do. Thanks, Xu Yang
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml new file mode 100644 index 000000000000..2ec62f564bf5 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/ci-hdrc-usb2-imx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP USB2 ChipIdea USB controller + +maintainers: + - Xu Yang <xu.yang_2@nxp.com> + +properties: + compatible: + oneOf: + - enum: + - fsl,imx27-usb + - items: + - enum: + - fsl,imx23-usb + - fsl,imx25-usb + - fsl,imx28-usb + - fsl,imx35-usb + - fsl,imx50-usb + - fsl,imx51-usb + - fsl,imx53-usb + - fsl,imx6q-usb + - fsl,imx6sl-usb + - fsl,imx6sx-usb + - fsl,imx6ul-usb + - fsl,imx7d-usb + - fsl,vf610-usb + - const: fsl,imx27-usb + - items: + - enum: + - fsl,imx8dxl-usb + - fsl,imx8ulp-usb + - const: fsl,imx7ulp-usb + - const: fsl,imx6ul-usb + - items: + - enum: + - fsl,imx8mm-usb + - fsl,imx8mn-usb + - const: fsl,imx7d-usb + - const: fsl,imx27-usb + - items: + - enum: + - fsl,imx6sll-usb + - fsl,imx7ulp-usb + - const: fsl,imx6ul-usb + - const: fsl,imx27-usb + +allOf: + - $ref: ci-hdrc-usb2.yaml# + +required: + - compatible + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/imx7d-clock.h> + + usb@30b10000 { + compatible = "fsl,imx7d-usb", "fsl,imx27-usb"; + reg = <0x30b10000 0x200>; + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clks IMX7D_USB_CTRL_CLK>; + fsl,usbphy = <&usbphynop1>; + fsl,usbmisc = <&usbmisc1 0>; + phy-clkgate-delay-us = <400>; + }; + +...
As more and more NXP i.MX chips come out, it becomes harder to maintain ci-hdrc-usb2.yaml if more stuffs like property restrictions are added to this file. This will separate i.MX parts out of ci-hdrc-usb2.yaml and add a new schema for NXP ChipIdea USB2 Controller. Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- Changes in v6: - new patch --- .../bindings/usb/ci-hdrc-usb2-imx.yaml | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2-imx.yaml