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@starfivetech.com
- starfive,i2s-max-channels:
- description:
Number of I2S max stereo channels supported on the StarFiveJH8100 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@linaro.org
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