On 02/08/2023 10:42, Xingyu Wu wrote: > Add the StarFive JH7110 (TX0/TX1/RX channel) SoC support in the bindings > of Designware I2S controller. The I2S controller needs two reset items'' Thank you for your patch. There is something to discuss/improve. > > resets: > items: > - description: Optional controller resets > + - description: controller reset of Sampling rate > + minItems: 1 > > dmas: > items: > @@ -51,6 +75,17 @@ properties: > - const: rx > minItems: 1 > > + starfive,syscon: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to System Register Controller sys_syscon node. > + - description: I2S-rx enabled control offset of SYS_SYSCONSAIF__SYSCFG register. > + - description: I2S-rx enabled control mask > + description: > + The phandle to System Register Controller syscon node and the I2S-rx(ADC) > + enabled control offset and mask of SYS_SYSCONSAIF__SYSCFG register. > + > allOf: > - $ref: dai-common.yaml# > - if: > @@ -66,6 +101,66 @@ allOf: > properties: > "#sound-dai-cells": > const: 0 You need to constrain clocks and resets also for all other existing variants. > + - if: > + properties: > + compatible: > + contains: > + const: snps,designware-i2s > + then: > + properties: > + clocks: > + maxItems: 1 > + clock-names: > + maxItems: 1 > + resets: > + maxItems: 1 > + else: > + properties: > + resets: > + minItems: 2 > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-i2stx0 > + then: > + properties: > + clocks: > + minItems: 5 Also maxItems > + clock-names: > + minItems: 5 Also maxItems What about resets? 1 or 2 items? > + required: > + - resets > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-i2stx1 > + then: > + properties: > + clocks: > + minItems: 9 > + clock-names: > + minItems: 9 resets? > + required: > + - resets > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-i2srx > + then: > + properties: > + clocks: > + minItems: 9 > + clock-names: > + minItems: 9 resets? > + required: > + - resets > + - starfive,syscon > + else: > + properties: > + starfive,syscon: false > > required: > - compatible Best regards, Krzysztof
