| Message ID | 20240508070406.286159-2-xingyu.wu@starfivetech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Cadence I2S-MC controller driver | expand |
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 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@linaro.org> Best regards, Krzysztof
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 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@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
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 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@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
diff --git a/Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml b/Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml new file mode 100644 index 000000000000..94ebce7fd5e4 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/cdns,i2s-mc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cadence multi-channel I2S controller + +description: + The Cadence I2S Controller implements a function of the multi-channel + (up to 8-channel) bus. It combines functions of a transmitter and a receiver. + +maintainers: + - Xingyu Wu <xingyu.wu@starfivetech.com> + +properties: + compatible: + items: + - enum: + - starfive,jh8100-i2s + - const: cdns,i2s-mc + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: Bit clock + - description: Main ICG clock + - description: Inner master clock + + clock-names: + items: + - const: bclk + - const: icg + - const: mclk_inner + + resets: + maxItems: 1 + + dmas: + items: + - description: TX DMA Channel + - description: RX DMA Channel + minItems: 1 + + dma-names: + items: + - const: tx + - const: rx + minItems: 1 + + "#sound-dai-cells": + const: 0 + + 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: + - compatible + - reg + - clocks + - clock-names + - resets + - dmas + - dma-names + - interrupts + - "#sound-dai-cells" + +unevaluatedProperties: false + +examples: + - | + i2s@122b0000 { + compatible = "starfive,jh8100-i2s", "cdns,i2s-mc"; + reg = <0x122b0000 0x1000>; + clocks = <&syscrg_ne 133>, + <&syscrg_ne 170>, + <&syscrg 50>; + clock-names = "bclk", "icg", + "mclk_inner"; + resets = <&syscrg_ne 43>; + dmas = <&dma 7>, <&dma 6>; + dma-names = "tx", "rx"; + interrupts = <59>; + #sound-dai-cells = <0>; + starfive,i2s-max-channels = <4>; + };
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> --- .../bindings/sound/cdns,i2s-mc.yaml | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml