On 31/01/2024 09:24, Xu Yang wrote: > Hi Krzysztof, > >> >> On 19/01/2024 08:19, Xu Yang wrote: >>> Change reg, interrupts, clock and clock-names as common properties and add >>> restrictions on them for different compatibles. >>> >>> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx> >>> >>> --- >>> Changes in v4: >>> - new patch since v3's discussion >>> - split the reg, interrupts, clock and clock-names properties into >>> common part and device-specific >>> --- >>> .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 +++++++++++++++--- >>> 1 file changed, 102 insertions(+), 16 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci- >> hdrc-usb2.yaml >>> index b7e664f7395b..78e30ca0a8ca 100644 >>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml >>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml >>> @@ -73,22 +73,10 @@ properties: >>> - nuvoton,npcm845-udc >>> - const: nuvoton,npcm750-udc >>> >>> - reg: >>> - minItems: 1 >>> - maxItems: 2 >>> - >>> - interrupts: >>> - minItems: 1 >>> - maxItems: 2 >>> - >>> - clocks: >>> - minItems: 1 >>> - maxItems: 3 >>> - >>> - clock-names: >>> - minItems: 1 >>> - maxItems: 3 >> >> Why all these are gone? They are supposed to be here. Your if:then: only >> customizes them. > > I have also concerns of whether to make this part common. > I will revert this later. Revert? No. This patch must be correct. > >> >>> - >>> + reg: true >>> + interrupts: true >>> + clocks: true >>> + clock-names: true >> >> No. These are not booleans on other variants. > > Okay. > >> >>> dr_mode: true >>> >>> power-domains: >>> @@ -412,6 +400,104 @@ allOf: >>> samsung,picophy-pre-emp-curr-control: false >>> samsung,picophy-dc-vol-level-adjust: false >>> >>> + - if: >>> + properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: fsl,imx27-usb >> >> No, the syntax you need is contains:. >> >> Look at existing code - there is no single binding with oneOf: in if: block. > > I wonder why 'make dt_binding_check' does not report this issue if the syntax > is not correct? I did not say syntax is incorrect. > > So I need to add contains as below, right? > > - if: > properties: > compatible: > contains: > oneOf: > - items: > - const: fsl,imx27-usb > - items: > - enum: > - fsl,imx25-usb > - fsl,imx35-usb > - const: fsl,imx27-usb > > The purpose of this code is to match: > > - compatible = "fsl,imx27-usb"; > - compatible = "fsl,imx25-usb", "fsl,imx27-usb"; > - compatible = "fsl,imx35-usb", "fsl,imx27-usb"; > > but should not match: > > - compatible = "fsl,imx7d-usb", "fsl,imx27-usb"; > > Is this feasible? So maybe they are not compatible? Your patch creates some unusual constraints for all the variants, which is probably result of huge one binding for all implementations of re-used IP block. I don't think that this huge if: you add here and further in the patch helps. Just like for other re-used IP blocks, this should have common part and per-device/per-family/per-implementation binding. Best regards, Krzysztof
