| Message ID | 298162bfa2e7225ccc753865e1ffa39ce2722b2a.1649443080.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MIPS: DTS: fix some findings by "make ci20_defconfig dt_binding_check dtbs_check" | expand |
On 08/04/2022 20:37, H. Nikolaus Schaller wrote: > arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed: > ['ingenic,jz4780-otg', 'snps,dwc2'] is too long > ['ingenic,jz4780-otg', 'snps,dwc2'] is too short > 'brcm,bcm2835-usb' was expected > 'hisilicon,hi6220-usb' was expected > 'rockchip,rk3066-usb' was expected > 'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] > 'lantiq,arx100-usb' was expected > 'lantiq,xrx200-usb' was expected > 'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg'] > 'amcc,dwc-otg' was expected > 'apm,apm82181-dwc-otg' was expected > 'snps,dwc2' was expected > 'st,stm32f4x9-fsotg' was expected > 'st,stm32f4x9-hsotg' was expected > 'st,stm32f7-hsotg' was expected > 'st,stm32mp15-fsotg' was expected > 'st,stm32mp15-hsotg' was expected > 'samsung,s3c6400-hsotg' was expected > 'intel,socfpga-agilex-hsotg' was expected You really don't need to paste entire warning. > From schema: Documentation/devicetree/bindings/usb/dwc2.yaml > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi > index 2021836983c96..c5124459678b7 100644 > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi > @@ -576,7 +576,7 @@ bch: bch@134d0000 { > }; > > otg: usb@13500000 { > - compatible = "ingenic,jz4780-otg", "snps,dwc2"; > + compatible = "snps,dwc2"; This looks wrong, the block usually should have a specific compatible. Please mention why it does not. Best regards, Best regards, Krzysztof
Hi, Le sam., avril 9 2022 at 13:15:37 +0200, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> a écrit : > On 08/04/2022 20:37, H. Nikolaus Schaller wrote: >> arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: >> 'oneOf' conditional failed, one must be fixed: >> ['ingenic,jz4780-otg', 'snps,dwc2'] is too long >> ['ingenic,jz4780-otg', 'snps,dwc2'] is too short >> 'brcm,bcm2835-usb' was expected >> 'hisilicon,hi6220-usb' was expected >> 'rockchip,rk3066-usb' was expected >> 'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', >> 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', >> 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', >> 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] >> 'lantiq,arx100-usb' was expected >> 'lantiq,xrx200-usb' was expected >> 'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', >> 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', >> 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg'] >> 'amcc,dwc-otg' was expected >> 'apm,apm82181-dwc-otg' was expected >> 'snps,dwc2' was expected >> 'st,stm32f4x9-fsotg' was expected >> 'st,stm32f4x9-hsotg' was expected >> 'st,stm32f7-hsotg' was expected >> 'st,stm32mp15-fsotg' was expected >> 'st,stm32mp15-hsotg' was expected >> 'samsung,s3c6400-hsotg' was expected >> 'intel,socfpga-agilex-hsotg' was expected > > You really don't need to paste entire warning. > > >> From schema: Documentation/devicetree/bindings/usb/dwc2.yaml >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi >> b/arch/mips/boot/dts/ingenic/jz4780.dtsi >> index 2021836983c96..c5124459678b7 100644 >> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi >> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi >> @@ -576,7 +576,7 @@ bch: bch@134d0000 { >> }; >> >> otg: usb@13500000 { >> - compatible = "ingenic,jz4780-otg", "snps,dwc2"; >> + compatible = "snps,dwc2"; > > This looks wrong, the block usually should have a specific compatible. > Please mention why it does not. Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP. Cheers, -Paul
> Am 09.04.2022 um 13:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: > > On 08/04/2022 20:37, H. Nikolaus Schaller wrote: >> arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed: >> ['ingenic,jz4780-otg', 'snps,dwc2'] is too long >> ['ingenic,jz4780-otg', 'snps,dwc2'] is too short >> 'brcm,bcm2835-usb' was expected >> 'hisilicon,hi6220-usb' was expected >> 'rockchip,rk3066-usb' was expected >> 'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] >> 'lantiq,arx100-usb' was expected >> 'lantiq,xrx200-usb' was expected >> 'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg'] >> 'amcc,dwc-otg' was expected >> 'apm,apm82181-dwc-otg' was expected >> 'snps,dwc2' was expected >> 'st,stm32f4x9-fsotg' was expected >> 'st,stm32f4x9-hsotg' was expected >> 'st,stm32f7-hsotg' was expected >> 'st,stm32mp15-fsotg' was expected >> 'st,stm32mp15-hsotg' was expected >> 'samsung,s3c6400-hsotg' was expected >> 'intel,socfpga-agilex-hsotg' was expected > > You really don't need to paste entire warning. > > >> From schema: Documentation/devicetree/bindings/usb/dwc2.yaml >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi >> index 2021836983c96..c5124459678b7 100644 >> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi >> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi >> @@ -576,7 +576,7 @@ bch: bch@134d0000 { >> }; >> >> otg: usb@13500000 { >> - compatible = "ingenic,jz4780-otg", "snps,dwc2"; >> + compatible = "snps,dwc2"; > > This looks wrong, the block usually should have a specific compatible. > Please mention why it does not. Well, I did not even have that idea that it could need an explanation. There is no "ingenic,jz4780-otg" and none is needed here to make it work. Therefore the generic "snps,dwc2" is sufficient.
On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >> >> This looks wrong, the block usually should have a specific compatible. >> Please mention why it does not. > > Well, I did not even have that idea that it could need an explanation. > > There is no "ingenic,jz4780-otg" and none is needed here to make it work. Make it work in what terms? We talk about hardware description, right? > > Therefore the generic "snps,dwc2" is sufficient. No, you are mixing now driver behavior (is sufficient) with hardware description. Most of licensed blocks require the specific compatible to differentiate it. Best regards, Krzysztof
Hi, > Am 09.04.2022 um 14:27 schrieb Paul Cercueil <paul@crapouillou.net>: > > Hi, > > Le sam., avril 9 2022 at 13:15:37 +0200, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> a écrit : >> On 08/04/2022 20:37, H. Nikolaus Schaller wrote: >>> arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed: >>> ['ingenic,jz4780-otg', 'snps,dwc2'] is too long >>> ['ingenic,jz4780-otg', 'snps,dwc2'] is too short >>> 'brcm,bcm2835-usb' was expected >>> 'hisilicon,hi6220-usb' was expected >>> 'rockchip,rk3066-usb' was expected >>> 'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] >>> 'lantiq,arx100-usb' was expected >>> 'lantiq,xrx200-usb' was expected >>> 'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg'] >>> 'amcc,dwc-otg' was expected >>> 'apm,apm82181-dwc-otg' was expected >>> 'snps,dwc2' was expected >>> 'st,stm32f4x9-fsotg' was expected >>> 'st,stm32f4x9-hsotg' was expected >>> 'st,stm32f7-hsotg' was expected >>> 'st,stm32mp15-fsotg' was expected >>> 'st,stm32mp15-hsotg' was expected >>> 'samsung,s3c6400-hsotg' was expected >>> 'intel,socfpga-agilex-hsotg' was expected >> You really don't need to paste entire warning. >>> From schema: Documentation/devicetree/bindings/usb/dwc2.yaml >>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >>> --- >>> arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi >>> index 2021836983c96..c5124459678b7 100644 >>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi >>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi >>> @@ -576,7 +576,7 @@ bch: bch@134d0000 { >>> }; >>> otg: usb@13500000 { >>> - compatible = "ingenic,jz4780-otg", "snps,dwc2"; >>> + compatible = "snps,dwc2"; >> This looks wrong, the block usually should have a specific compatible. >> Please mention why it does not. > > Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP. Well, there is no specifc implementation and driver for it. So no need to uniquely identify it. BR and thanks, Nikolaus
On 09/04/2022 15:18, H. Nikolaus Schaller wrote: hould have a specific compatible. >>> Please mention why it does not. >> >> Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP. > > Well, there is no specifc implementation and driver for it. So no need to uniquely identify it. Specific implementation and driver are not arguments here. This does not matter. It's really unrelated argument. Bindings are not about implementation in Linux. Implementation can change, so bindings should also? Best regards, Krzysztof
> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: > > On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>> >>> This looks wrong, the block usually should have a specific compatible. >>> Please mention why it does not. >> >> Well, I did not even have that idea that it could need an explanation. >> >> There is no "ingenic,jz4780-otg" and none is needed here to make it work. > > Make it work in what terms? We talk about hardware description, right? Yes. > >> >> Therefore the generic "snps,dwc2" is sufficient. > > No, you are mixing now driver behavior (is sufficient) with hardware > description. No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior. > Most of licensed blocks require the specific compatible to > differentiate it. If there is a need to differentiate.
> Am 09.04.2022 um 15:23 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: > > On 09/04/2022 15:18, H. Nikolaus Schaller wrote: > hould have a specific compatible. >>>> Please mention why it does not. >>> >>> Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP. >> >> Well, there is no specifc implementation and driver for it. So no need to uniquely identify it. > > Specific implementation and driver are not arguments here. This does not > matter. It's really unrelated argument. The argumentation is in reverse: if there is no need for a specialized driver or implementation, why is there is a need to define a specialization. Your argument was: "there is no way to uniquely identify the JZ4780 implementation of the IP" My question is: "what do we need that for?" > Bindings are not about implementation in Linux. Implementation can > change, so bindings should also? No. Implementations should be agnostic.
Le sam., avril 9 2022 at 15:37:51 +0200, H. Nikolaus Schaller <hns@goldelico.com> a écrit : > > >> Am 09.04.2022 um 15:23 schrieb Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org>: >> >> On 09/04/2022 15:18, H. Nikolaus Schaller wrote: >> hould have a specific compatible. >>>>> Please mention why it does not. >>>> >>>> Agreed. The "snps,dwc2" should be a fallback string, otherwise >>>> there is no way to uniquely identify the JZ4780 implementation of >>>> the IP. >>> >>> Well, there is no specifc implementation and driver for it. So no >>> need to uniquely identify it. >> >> Specific implementation and driver are not arguments here. This >> does not >> matter. It's really unrelated argument. > > The argumentation is in reverse: if there is no need for a > specialized driver or implementation, > why is there is a need to define a specialization. > > Your argument was: > "there is no way to uniquely identify the JZ4780 implementation of > the IP" > > My question is: > "what do we need that for?" You may not need the differenciation now, but if you need it in the future and you had only the "snps,dwc2" compatible previously, then you're screwed, since your driver must support older device tree blobs. -Paul >> Bindings are not about implementation in Linux. Implementation can >> change, so bindings should also? > > No. Implementations should be agnostic. >
On 09/04/2022 15:32, H. Nikolaus Schaller wrote: > > >> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >> >> On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>>> >>>> This looks wrong, the block usually should have a specific compatible. >>>> Please mention why it does not. >>> >>> Well, I did not even have that idea that it could need an explanation. >>> >>> There is no "ingenic,jz4780-otg" and none is needed here to make it work. >> >> Make it work in what terms? We talk about hardware description, right? > > Yes. > >> >>> >>> Therefore the generic "snps,dwc2" is sufficient. >> >> No, you are mixing now driver behavior (is sufficient) with hardware >> description. > > No. "snps,dwc2" is a hardware description for a licensed block. > Not a driver behavior. snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure? > >> Most of licensed blocks require the specific compatible to >> differentiate it. > > If there is a need to differentiate. No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes. Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)? Best regards, Krzysztof
> Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: > > On 09/04/2022 15:32, H. Nikolaus Schaller wrote: >> >> >>> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>> >>> On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>>>> >>>>> This looks wrong, the block usually should have a specific compatible. >>>>> Please mention why it does not. >>>> >>>> Well, I did not even have that idea that it could need an explanation. >>>> >>>> There is no "ingenic,jz4780-otg" and none is needed here to make it work. >>> >>> Make it work in what terms? We talk about hardware description, right? >> >> Yes. >> >>> >>>> >>>> Therefore the generic "snps,dwc2" is sufficient. >>> >>> No, you are mixing now driver behavior (is sufficient) with hardware >>> description. >> >> No. "snps,dwc2" is a hardware description for a licensed block. >> Not a driver behavior. > > snps,dwc2 matches the original block, not necessarily this > implementation. Unless you are sure? I assume. Nobody has reported an issue without having any specific jz4780 driver in place. Well, that is only evidence, not bullet proof. > >> >>> Most of licensed blocks require the specific compatible to >>> differentiate it. >> >> If there is a need to differentiate. > > No, regardless whether there is a need currently, most of them have > specific compatibles, because there are some minor differences. Even if > difference is not visible from programming model or wiring, it might > justify it's own specific compatible. For example because maybe once > that tiny difference will require some changes. > > Someone added the ingenic compatible, so why do you assume that one tool > (bindings) is correct but other piece of code (using specific > compatible) is not? You use the argument "bindings warning" which is not > enough. Argument that blocks are 100% same, is good enough, if you are > sure. Just use it in commit msg. But are you sure that these are the > same? Same pins, same programming model (entire model, not used by Linux)? The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings. BR and thanks, Nikolaus
Hi folks, On 2022/4/9 下午9:53, H. Nikolaus Schaller wrote: > >> Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >> >> On 09/04/2022 15:32, H. Nikolaus Schaller wrote: >>> >>>> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>>> >>>> On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>>>>> This looks wrong, the block usually should have a specific compatible. >>>>>> Please mention why it does not. >>>>> Well, I did not even have that idea that it could need an explanation. >>>>> >>>>> There is no "ingenic,jz4780-otg" and none is needed here to make it work. >>>> Make it work in what terms? We talk about hardware description, right? >>> Yes. >>> >>>>> Therefore the generic "snps,dwc2" is sufficient. >>>> No, you are mixing now driver behavior (is sufficient) with hardware >>>> description. >>> No. "snps,dwc2" is a hardware description for a licensed block. >>> Not a driver behavior. >> snps,dwc2 matches the original block, not necessarily this >> implementation. Unless you are sure? > I assume. Nobody has reported an issue without having any specific jz4780 driver in place. > Well, that is only evidence, not bullet proof. > >>>> Most of licensed blocks require the specific compatible to >>>> differentiate it. >>> If there is a need to differentiate. >> No, regardless whether there is a need currently, most of them have >> specific compatibles, because there are some minor differences. Even if >> difference is not visible from programming model or wiring, it might >> justify it's own specific compatible. For example because maybe once >> that tiny difference will require some changes. >> >> Someone added the ingenic compatible, so why do you assume that one tool >> (bindings) is correct but other piece of code (using specific >> compatible) is not? You use the argument "bindings warning" which is not >> enough. Argument that blocks are 100% same, is good enough, if you are >> sure. Just use it in commit msg. But are you sure that these are the >> same? Same pins, same programming model (entire model, not used by Linux)? > The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. > Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings. It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it to the mailing list, and then I received some revision comments, but for some personal reasons I didn't continue to improve it. I'll finish these modifications as soon as possible and send them out. Then after they merge into the mainline, this problem will be solved. Thanks and best regards! > > BR and thanks, > Nikolaus >
Hi, > Am 10.04.2022 um 18:32 schrieb Zhou Yanjie <zhouyanjie@wanyeetech.com>: > > Hi folks, > > On 2022/4/9 下午9:53, H. Nikolaus Schaller wrote: >> >>> Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>> >>> On 09/04/2022 15:32, H. Nikolaus Schaller wrote: >>>> >>>>> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>>>> >>>>> On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>>>>>> This looks wrong, the block usually should have a specific compatible. >>>>>>> Please mention why it does not. >>>>>> Well, I did not even have that idea that it could need an explanation. >>>>>> >>>>>> There is no "ingenic,jz4780-otg" and none is needed here to make it work. >>>>> Make it work in what terms? We talk about hardware description, right? >>>> Yes. >>>> >>>>>> Therefore the generic "snps,dwc2" is sufficient. >>>>> No, you are mixing now driver behavior (is sufficient) with hardware >>>>> description. >>>> No. "snps,dwc2" is a hardware description for a licensed block. >>>> Not a driver behavior. >>> snps,dwc2 matches the original block, not necessarily this >>> implementation. Unless you are sure? >> I assume. Nobody has reported an issue without having any specific jz4780 driver in place. >> Well, that is only evidence, not bullet proof. >> >>>>> Most of licensed blocks require the specific compatible to >>>>> differentiate it. >>>> If there is a need to differentiate. >>> No, regardless whether there is a need currently, most of them have >>> specific compatibles, because there are some minor differences. Even if >>> difference is not visible from programming model or wiring, it might >>> justify it's own specific compatible. For example because maybe once >>> that tiny difference will require some changes. >>> >>> Someone added the ingenic compatible, so why do you assume that one tool >>> (bindings) is correct but other piece of code (using specific >>> compatible) is not? You use the argument "bindings warning" which is not >>> enough. Argument that blocks are 100% same, is good enough, if you are >>> sure. Just use it in commit msg. But are you sure that these are the >>> same? Same pins, same programming model (entire model, not used by Linux)? >> The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. >> Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings. > > > It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it > to the mailing list, and then I received some revision comments, but for > some personal reasons I didn't continue to improve it. > > I'll finish these modifications as soon as possible and send them out. > Then after they merge into the mainline, this problem will be solved. No need to apologize. If you agree I can add "ingenic,jz4780-otg" to the schema file and keep the .dts in the v2 of my series. And I'll add you to the list of reviewers, so you can please comment v2 if it is correct or if we are still missing something. Best regards and thanks, Nikolaus
Hi Nikolaus, On 2022/4/11 上午3:13, H. Nikolaus Schaller wrote: > Hi, > >> Am 10.04.2022 um 18:32 schrieb Zhou Yanjie <zhouyanjie@wanyeetech.com>: >> >> Hi folks, >> >> On 2022/4/9 下午9:53, H. Nikolaus Schaller wrote: >>>> Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>>> >>>> On 09/04/2022 15:32, H. Nikolaus Schaller wrote: >>>>>> Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>: >>>>>> >>>>>> On 09/04/2022 15:05, H. Nikolaus Schaller wrote: >>>>>>>> This looks wrong, the block usually should have a specific compatible. >>>>>>>> Please mention why it does not. >>>>>>> Well, I did not even have that idea that it could need an explanation. >>>>>>> >>>>>>> There is no "ingenic,jz4780-otg" and none is needed here to make it work. >>>>>> Make it work in what terms? We talk about hardware description, right? >>>>> Yes. >>>>> >>>>>>> Therefore the generic "snps,dwc2" is sufficient. >>>>>> No, you are mixing now driver behavior (is sufficient) with hardware >>>>>> description. >>>>> No. "snps,dwc2" is a hardware description for a licensed block. >>>>> Not a driver behavior. >>>> snps,dwc2 matches the original block, not necessarily this >>>> implementation. Unless you are sure? >>> I assume. Nobody has reported an issue without having any specific jz4780 driver in place. >>> Well, that is only evidence, not bullet proof. >>> >>>>>> Most of licensed blocks require the specific compatible to >>>>>> differentiate it. >>>>> If there is a need to differentiate. >>>> No, regardless whether there is a need currently, most of them have >>>> specific compatibles, because there are some minor differences. Even if >>>> difference is not visible from programming model or wiring, it might >>>> justify it's own specific compatible. For example because maybe once >>>> that tiny difference will require some changes. >>>> >>>> Someone added the ingenic compatible, so why do you assume that one tool >>>> (bindings) is correct but other piece of code (using specific >>>> compatible) is not? You use the argument "bindings warning" which is not >>>> enough. Argument that blocks are 100% same, is good enough, if you are >>>> sure. Just use it in commit msg. But are you sure that these are the >>>> same? Same pins, same programming model (entire model, not used by Linux)? >>> The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. >>> Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings. >> >> It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it >> to the mailing list, and then I received some revision comments, but for >> some personal reasons I didn't continue to improve it. >> >> I'll finish these modifications as soon as possible and send them out. >> Then after they merge into the mainline, this problem will be solved. > No need to apologize. > > If you agree I can add "ingenic,jz4780-otg" to the schema file and keep > the .dts in the v2 of my series. Sure. Or you can wait a bit, I plan to send out new patches later today, it contains "ingenic,jz4780-otg". > And I'll add you to the list of reviewers, so you can please comment v2 > if it is correct or if we are still missing something. Okay, thanks! > > Best regards and thanks, > Nikolaus
Hi Yanjie, > Am 12.04.2022 um 11:49 schrieb Zhou Yanjie <zhouyanjie@wanyeetech.com>: > > Hi Nikolaus, > > On 2022/4/11 上午3:13, H. Nikolaus Schaller wrote: >> Hi, >> >> >> If you agree I can add "ingenic,jz4780-otg" to the schema file and keep >> the .dts in the v2 of my series. > > > Sure. > > Or you can wait a bit, I plan to send out new patches later today, it contains "ingenic,jz4780-otg". Would be fine! I think you will be faster than me :) So I'll check with your patch and drop mine from my v2 series. BR and thanks, Nikolaus
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 2021836983c96..c5124459678b7 100644 --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi @@ -576,7 +576,7 @@ bch: bch@134d0000 { }; otg: usb@13500000 { - compatible = "ingenic,jz4780-otg", "snps,dwc2"; + compatible = "snps,dwc2"; reg = <0x13500000 0x40000>; interrupt-parent = <&intc>;
arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed: ['ingenic,jz4780-otg', 'snps,dwc2'] is too long ['ingenic,jz4780-otg', 'snps,dwc2'] is too short 'brcm,bcm2835-usb' was expected 'hisilicon,hi6220-usb' was expected 'rockchip,rk3066-usb' was expected 'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] 'lantiq,arx100-usb' was expected 'lantiq,xrx200-usb' was expected 'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg'] 'amcc,dwc-otg' was expected 'apm,apm82181-dwc-otg' was expected 'snps,dwc2' was expected 'st,stm32f4x9-fsotg' was expected 'st,stm32f4x9-hsotg' was expected 'st,stm32f7-hsotg' was expected 'st,stm32mp15-fsotg' was expected 'st,stm32mp15-hsotg' was expected 'samsung,s3c6400-hsotg' was expected 'intel,socfpga-agilex-hsotg' was expected From schema: Documentation/devicetree/bindings/usb/dwc2.yaml Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)