Hi Krzysztof, > > 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. I mean only this part keeps unchanged like before. > > > >> > >>> - > >>> + 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 Yes, they are not fully compatible. > 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. Actually I've tested all dts files (not only imx parts) against this dt-binding yaml. Then I'll rework this patch to focus on imx parts. I'm not sure if someone will add restrictions for their family/device in the future. Thanks, Xu Yang
