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
