On 2023/12/21 21:53, Conor Dooley wrote: > Xingyu, Mark, > > On Thu, Dec 21, 2023 at 11:32:22AM +0800, Xingyu Wu wrote: >> Add bindings for the Multi-Channel I2S controller of Cadence >> on the StarFive JH8100 SoC. >> >> Signed-off-by: Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx> >> --- >> .../bindings/sound/cdns,jh8100-i2s.yaml | 100 ++++++++++++++++++ >> 1 file changed, 100 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/sound/cdns,jh8100-i2s.yaml >> >> diff --git a/Documentation/devicetree/bindings/sound/cdns,jh8100-i2s.yaml b/Documentation/devicetree/bindings/sound/cdns,jh8100-i2s.yaml >> new file mode 100644 >> index 000000000000..5d95d9ab3e45 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/cdns,jh8100-i2s.yaml >> @@ -0,0 +1,100 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/cdns,jh8100-i2s.yaml# > > Filename matching the compatible please. Noted. > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Cadence multi-channel I2S controller for StarFive JH8100 SoC >> + >> +description: | > > You only need the | if there is formatting to preserve. Will drop. > >> + 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. >> + It is used in the StarFive JH8100 SoC. >> + >> +maintainers: >> + - Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx> >> + - Walker Chen <walker.chen@xxxxxxxxxxxxxxxx> >> + >> +properties: >> + compatible: >> + const: starfive,jh8100-i2s >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + description: | >> + The interrupt line number for the I2S controller. Add this >> + parameter if the I2S controller that you are using does not >> + support DMA. > > You've got one i2s controller here, you should know if it supports DMA > or not. The I2S already supports interrupt handler, but if the SoC supports DMA controller to be use, it can optionally use DMA. > >> + 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 >> + >> + cdns,i2s-max-channels: >> + description: | >> + Number of I2S max stereo channels supported by the hardware. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + minimum: 1 >> + maximum: 8 > > Mark, is there no common property for this kind of thing? That said, > there's one device here so the number is known at present. > Another note, this property is not required, so it should have a > default. > > It's kinda hard to know with this binding - it is touted as being for a > particular Cadence IP, and some aspects are pretty generic, but at the > same time there's only one device here so it's hard to tell what is > variable between implementations and what is not. > Are there no other implementations of this controller? Unless it is > brand new, I find that hard to believe. > > Cheers, > Conor. > Sorry, It does not seem to be common property. The Cadence I2S supports 8 channels. There are four I2S controllers on the JH8100 SoC, and two of them just provide 4 channels to use, one of them just provide 2 channels. It seems to depend on the SoC. Thanks, Xingyu Wu >> + >> + "#sound-dai-cells": >> + const: 0 >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - resets >> + >> +oneOf: >> + - required: >> + - dmas >> + - dma-names >> + - required: >> + - interrupts >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + i2s@122b0000 { >> + compatible = "starfive,jh8100-i2s"; >> + 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"; >> + cdns,i2s-max-channels = <2>; >> + #sound-dai-cells = <0>; >> + }; >> -- >> 2.25.1 >> >>
