| Message ID | 20230320020714.955800-1-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [linux-next] dt-bindings: usb: snps,dwc3: correct i.MX8MQ support | expand |
On 20/03/2023 03:07, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > The previous i.MX8MQ support breaks rockchip,dwc3 support, > so use select to restrict i.MX8MQ support and avoid break others. > > Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support i.MX8MQ") > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > index 16c7d06c9172..6347a6769ee3 100644 > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > @@ -28,15 +28,22 @@ allOf: > else: > $ref: usb-xhci.yaml# > > +select: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx8mq-dwc3 And what about all snps,dwc3 devices there (without specific compatible)? Previously they were selected and now they are not... so you just disabled schema for all of them. > + required: > + - compatible > + > properties: > compatible: > - oneOf: > - - items: > - - const: fsl,imx8mq-dwc3 I don't understand why you remove your compatible. Best regards, Krzysztof
> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct > i.MX8MQ support > > On 20/03/2023 03:07, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > The previous i.MX8MQ support breaks rockchip,dwc3 support, so use > > select to restrict i.MX8MQ support and avoid break others. > > > > Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support i.MX8MQ") > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > index 16c7d06c9172..6347a6769ee3 100644 > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > @@ -28,15 +28,22 @@ allOf: > > else: > > $ref: usb-xhci.yaml# > > > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8mq-dwc3 > > And what about all snps,dwc3 devices there (without specific compatible)? > Previously they were selected and now they are not... so you just disabled > schema for all of them. [Peng Fan] I am not sure how to address: https://lore.kernel.org/all/CAL_Jsq+nrqTX5ND3=+kFUmStx-+b=qpE_WsL_Le-YX8c285-5A@mail.gmail.com/ Rob suggested use select, but I am not sure how to use it here. Use an extra yaml file with select or else. > > > + required: > > + - compatible > > + > > properties: > > compatible: > > - oneOf: > > - - items: > > - - const: fsl,imx8mq-dwc3 > > I don't understand why you remove your compatible. [Peng Fan] My last change drops 'contains' by mistake, and breaks rockchip,dwc3, so just bring back the original content Thanks, Peng. > > Best regards, > Krzysztof
On 20/03/2023 12:02, Peng Fan wrote: >> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct >> i.MX8MQ support >> >> On 20/03/2023 03:07, Peng Fan (OSS) wrote: >>> From: Peng Fan <peng.fan@nxp.com> >>> >>> The previous i.MX8MQ support breaks rockchip,dwc3 support, so use >>> select to restrict i.MX8MQ support and avoid break others. >>> >>> Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support i.MX8MQ") >>> Signed-off-by: Peng Fan <peng.fan@nxp.com> >>> --- >>> .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++------- >>> 1 file changed, 14 insertions(+), 7 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> index 16c7d06c9172..6347a6769ee3 100644 >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> @@ -28,15 +28,22 @@ allOf: >>> else: >>> $ref: usb-xhci.yaml# >>> >>> +select: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - fsl,imx8mq-dwc3 >> >> And what about all snps,dwc3 devices there (without specific compatible)? >> Previously they were selected and now they are not... so you just disabled >> schema for all of them. > [Peng Fan] > > I am not sure how to address: > https://lore.kernel.org/all/CAL_Jsq+nrqTX5ND3=+kFUmStx-+b=qpE_WsL_Le-YX8c285-5A@mail.gmail.com/ > > Rob suggested use select, but I am not sure how to use it here. Use > an extra yaml file with select or else. > >> >>> + required: >>> + - compatible >>> + >>> properties: >>> compatible: >>> - oneOf: >>> - - items: >>> - - const: fsl,imx8mq-dwc3 >> >> I don't understand why you remove your compatible. > [Peng Fan] > > My last change drops 'contains' by mistake, and breaks This I understand. > rockchip,dwc3, so just bring back the original content This is not a reason to make your compatible undocumented. You basically revert your patch. It does not make any sense to me. Best regards, Krzysztof
> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct > i.MX8MQ support > > On 20/03/2023 12:02, Peng Fan wrote: > >> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct > >> i.MX8MQ support > >> > >> On 20/03/2023 03:07, Peng Fan (OSS) wrote: > >>> From: Peng Fan <peng.fan@nxp.com> > >>> > >>> The previous i.MX8MQ support breaks rockchip,dwc3 support, so use > >>> select to restrict i.MX8MQ support and avoid break others. > >>> > >>> Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support i.MX8MQ") > >>> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >>> --- > >>> .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++------- > >>> 1 file changed, 14 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>> index 16c7d06c9172..6347a6769ee3 100644 > >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>> @@ -28,15 +28,22 @@ allOf: > >>> else: > >>> $ref: usb-xhci.yaml# > >>> > >>> +select: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - fsl,imx8mq-dwc3 > >> > >> And what about all snps,dwc3 devices there (without specific compatible)? > >> Previously they were selected and now they are not... so you just > >> disabled schema for all of them. > > [Peng Fan] > > > > I am not sure how to address: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore > > .kernel.org%2Fall%2FCAL_Jsq%2BnrqTX5ND3%3D%2BkFUmStx- > %2Bb%3DqpE_WsL_Le > > -YX8c285- > 5A%40mail.gmail.com%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C1e > > > 67df79692243f058fa08db2937befd%7C686ea1d3bc2b4c6fa92cd99c5c30163 > 5%7C0% > > > 7C0%7C638149091586933618%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > 4wLjAwMDAiL > > > CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s > data=r1 > > fesB1c%2B3YxohCGM36xySFucVU25WVYIMlpgGkdoWk%3D&reserved=0 > > > > Rob suggested use select, but I am not sure how to use it here. Use an > > extra yaml file with select or else. > > > >> > >>> + required: > >>> + - compatible > >>> + > >>> properties: > >>> compatible: > >>> - oneOf: > >>> - - items: > >>> - - const: fsl,imx8mq-dwc3 > >> > >> I don't understand why you remove your compatible. > > [Peng Fan] > > > > My last change drops 'contains' by mistake, and breaks > > This I understand. > > > rockchip,dwc3, so just bring back the original content > > This is not a reason to make your compatible undocumented. You basically > revert your patch. It does not make any sense to me. [Peng Fan] ok, let me think more. Honestly not have good idea (: If you have any guidance, that would be helpful. Thanks, Peng. > > > Best regards, > Krzysztof
On 20/03/2023 12:44, Peng Fan wrote: >> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct >> i.MX8MQ support >> >> On 20/03/2023 12:02, Peng Fan wrote: >>>> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct >>>> i.MX8MQ support >>>> >>>> On 20/03/2023 03:07, Peng Fan (OSS) wrote: >>>>> From: Peng Fan <peng.fan@nxp.com> >>>>> >>>>> The previous i.MX8MQ support breaks rockchip,dwc3 support, so use >>>>> select to restrict i.MX8MQ support and avoid break others. >>>>> >>>>> Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support i.MX8MQ") >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com> >>>>> --- >>>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++------- >>>>> 1 file changed, 14 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> index 16c7d06c9172..6347a6769ee3 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> @@ -28,15 +28,22 @@ allOf: >>>>> else: >>>>> $ref: usb-xhci.yaml# >>>>> >>>>> +select: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - fsl,imx8mq-dwc3 >>>> >>>> And what about all snps,dwc3 devices there (without specific compatible)? >>>> Previously they were selected and now they are not... so you just >>>> disabled schema for all of them. >>> [Peng Fan] >>> >>> I am not sure how to address: >>> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore >>> .kernel.org%2Fall%2FCAL_Jsq%2BnrqTX5ND3%3D%2BkFUmStx- >> %2Bb%3DqpE_WsL_Le >>> -YX8c285- >> 5A%40mail.gmail.com%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C1e >>> >> 67df79692243f058fa08db2937befd%7C686ea1d3bc2b4c6fa92cd99c5c30163 >> 5%7C0% >>> >> 7C0%7C638149091586933618%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC >> 4wLjAwMDAiL >>> >> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s >> data=r1 >>> fesB1c%2B3YxohCGM36xySFucVU25WVYIMlpgGkdoWk%3D&reserved=0 >>> >>> Rob suggested use select, but I am not sure how to use it here. Use an >>> extra yaml file with select or else. >>> >>>> >>>>> + required: >>>>> + - compatible >>>>> + >>>>> properties: >>>>> compatible: >>>>> - oneOf: >>>>> - - items: >>>>> - - const: fsl,imx8mq-dwc3 >>>> >>>> I don't understand why you remove your compatible. >>> [Peng Fan] >>> >>> My last change drops 'contains' by mistake, and breaks >> >> This I understand. >> >>> rockchip,dwc3, so just bring back the original content >> >> This is not a reason to make your compatible undocumented. You basically >> revert your patch. It does not make any sense to me. > [Peng Fan] > > ok, let me think more. Honestly not have good idea (: > If you have any guidance, that would be helpful. You should only need a select, however most likely selecting also snps,dwc3 (without specific compatible). Anyway this binding - including how it is referenced by others - is getting unmanagable. I think I commented about this on some other occasion. The point is that no one refactors it thus it grows and grows leading to more of such tricky scenarios. How it should be refactored? Probably just like all other with re-usable components: common part and device specific bindings including it. While snps,dwc3 alone should not be allowed anymore. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: 2023年3月20日 20:06 > To: Peng Fan <peng.fan@nxp.com>; Peng Fan (OSS) > <peng.fan@oss.nxp.com>; gregkh@linuxfoundation.org; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; Xu Yang > <xu.yang_2@nxp.com> > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux- > imx@nxp.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Jun Li > <jun.li@nxp.com> > Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct > i.MX8MQ support > > On 20/03/2023 12:44, Peng Fan wrote: > >> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: correct > >> i.MX8MQ support > >> > >> On 20/03/2023 12:02, Peng Fan wrote: > >>>> Subject: Re: [PATCH linux-next] dt-bindings: usb: snps,dwc3: > >>>> correct i.MX8MQ support > >>>> > >>>> On 20/03/2023 03:07, Peng Fan (OSS) wrote: > >>>>> From: Peng Fan <peng.fan@nxp.com> > >>>>> > >>>>> The previous i.MX8MQ support breaks rockchip,dwc3 support, so use > >>>>> select to restrict i.MX8MQ support and avoid break others. > >>>>> > >>>>> Fixes: 3754c41c7686 ("dt-bindings: usb: snps,dwc3: support > >>>>> i.MX8MQ") > >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >>>>> --- > >>>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 21 ++++++++++++---- > --- > >>>>> 1 file changed, 14 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>>>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>>>> index 16c7d06c9172..6347a6769ee3 100644 > >>>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > >>>>> @@ -28,15 +28,22 @@ allOf: > >>>>> else: > >>>>> $ref: usb-xhci.yaml# > >>>>> > >>>>> +select: > >>>>> + properties: > >>>>> + compatible: > >>>>> + contains: > >>>>> + enum: > >>>>> + - fsl,imx8mq-dwc3 > >>>> > >>>> And what about all snps,dwc3 devices there (without specific > compatible)? > >>>> Previously they were selected and now they are not... so you just > >>>> disabled schema for all of them. > >>> [Peng Fan] > >>> > >>> I am not sure how to address: > >>> > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor > >> > e%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7Cb85a372a1a3d467fbf55 > 08db293b > >> > 699d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63814910733 > 4034389% > >> > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC > JBTiI6I > >> > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=etbl63jX3d0LVM3 > %2B8P8%2BB > >> 2z8dcBddkns%2Fx0O63UHc2E%3D&reserved=0 > >>> .kernel.org%2Fall%2FCAL_Jsq%2BnrqTX5ND3%3D%2BkFUmStx- > >> %2Bb%3DqpE_WsL_Le > >>> -YX8c285- > >> > 5A%40mail.gmail.com%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C1e > >>> > >> > 67df79692243f058fa08db2937befd%7C686ea1d3bc2b4c6fa92cd99c5c30163 > >> 5%7C0% > >>> > >> > 7C0%7C638149091586933618%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > >> 4wLjAwMDAiL > >>> > >> > CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s > >> data=r1 > >>> > fesB1c%2B3YxohCGM36xySFucVU25WVYIMlpgGkdoWk%3D&reserved=0 > >>> > >>> Rob suggested use select, but I am not sure how to use it here. Use > >>> an extra yaml file with select or else. > >>> > >>>> > >>>>> + required: > >>>>> + - compatible > >>>>> + > >>>>> properties: > >>>>> compatible: > >>>>> - oneOf: > >>>>> - - items: > >>>>> - - const: fsl,imx8mq-dwc3 > >>>> > >>>> I don't understand why you remove your compatible. > >>> [Peng Fan] > >>> > >>> My last change drops 'contains' by mistake, and breaks > >> > >> This I understand. > >> > >>> rockchip,dwc3, so just bring back the original content > >> > >> This is not a reason to make your compatible undocumented. You > >> basically revert your patch. It does not make any sense to me. > > [Peng Fan] > > > > ok, let me think more. Honestly not have good idea (: > > If you have any guidance, that would be helpful. > > You should only need a select, however most likely selecting also > snps,dwc3 (without specific compatible). Anyway this binding - including > how it is referenced by others - is getting unmanagable. I think I commented > about this on some other occasion. The point is that no one refactors it thus > it grows and grows leading to more of such tricky scenarios. > > How it should be refactored? Probably just like all other with re-usable > components: common part and device specific bindings including it. While > snps,dwc3 alone should not be allowed anymore. Thanks, I may take rockchip,dwc3.yaml or other vendor,dwc3.yaml as reference to introduce fsl-imx8mq,dwc3.yaml to just add a select, and drop the fsl,imx8mq-dwc3 compatible from snps,dwc3.yaml. Thanks, Peng. > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 16c7d06c9172..6347a6769ee3 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -28,15 +28,22 @@ allOf: else: $ref: usb-xhci.yaml# +select: + properties: + compatible: + contains: + enum: + - fsl,imx8mq-dwc3 + required: + - compatible + properties: compatible: - oneOf: - - items: - - const: fsl,imx8mq-dwc3 - - const: snps,dwc3 - - const: snps,dwc3 - - const: synopsys,dwc3 - deprecated: true + contains: + oneOf: + - const: snps,dwc3 + - const: synopsys,dwc3 + deprecated: true reg: maxItems: 1