On 2023/8/6 5:02, Krzysztof Kozlowski wrote: > 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 The resets of TX0/TX1/RX on JH7110 SoC are mentioned in 'else' here. >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: starfive,jh7110-i2stx0 >> + then: >> + properties: >> + clocks: >> + minItems: 5 > > Also maxItems Will add. > >> + clock-names: >> + minItems: 5 > > Also maxItems Will add. > > What about resets? 1 or 2 items? Mentioned it in the 'else'. Or do you mean I should drop the 'else' and add the resets in here? And is the same for TX1 and RX? > >> + 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, Xingyu Wu
