On 22/05/2024 15:30, Krzysztof Kozlowski wrote: > > On 08/05/2024 10:03, Krzysztof Kozlowski wrote: > > On 08/05/2024 09:04, Xingyu Wu wrote: > >> Add bindings for the Multi-Channel I2S controller of Cadence. > >> > >> The Multi-Channel I2S (I2S-MC) implements a function of the 8-channel > >> I2S bus interfasce. Each channel can become receiver or transmitter. > >> Four I2S instances are used on the StarFive > >> JH8100 SoC. One instance of them is limited to 2 channels, two > >> instance are limited to 4 channels, and the other one can use most 8 > >> channels. Add a unique property about 'starfive,i2s-max-channels' to > >> distinguish each instance. > >> > >> Signed-off-by: Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx> > > > > > >> + > >> + starfive,i2s-max-channels: > >> + description: > >> + Number of I2S max stereo channels supported on the StarFive > >> + JH8100 SoC. > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + enum: [2, 4, 8] > >> + > >> +allOf: > >> + - $ref: dai-common.yaml# > >> + - if: > >> + properties: > >> + compatible: > >> + contains: > >> + const: starfive,jh8100-i2s > >> + then: > >> + required: > >> + - starfive,i2s-max-channels > >> + else: > >> + properties: > >> + starfive,i2s-max-channels: false > >> + > >> +required: > > > > I asked to put it after properties: block, not after allOf:. See > > example-schema for preferred order. Why? Because we are used to it and > > it makes reading the schema easier for us. > > > > Rest looks good, so with the re-order: > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > > Since you do not plan to fix it and already started pinging mark, I retract my > review. > > Unreviewed-by. > > Implement the feedback I already asked you BEFORE. > > Best regards, > Krzysztof Sorry, I thought my reply email went out but it didn't. I will re-order it and put the required before the allOf in the next version. I thought it would be nice to update both bindings and driver patches in the next version, so I ping mark. Best regards, Xingyu Wu
