Re: [PATCH v2 1/2] ASoC: dt-bindings: Add bindings for Cadence I2S-MC controller

On 20/03/2024 10:02, 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

.../bindings/sound/cdns,i2s-mc.yaml | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml

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..0a1b0122a246 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cdns,i2s-mc.yaml @@ -0,0 +1,110 @@ +# 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:
• enum:

•  - cdns,i2s-mc
  

Why did this appear? Who asked for this? Usually these blocks are not usable on their own.

•  - 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
  

•  using DMA.
  

That's still wrong. You already got comment on this. Either you have interrupt or not. You do not add interrupts, based on your choice or not of having DMA. Drop the comment.

• 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

+allOf:

• • $ref: dai-common.yaml#
• • if:

•  properties:
  

•    compatible:
  

•      contains:
  

•        const: starfive,jh8100-i2s
  

• then:

•  properties:
  

•    starfive,i2s-max-channels:
  

•      description:
  

•        Number of I2S max stereo channels supported on the StarFive
  

•        JH8100 SoC.
  

•      $ref: /schemas/types.yaml#/definitions/uint32
  

Properties must be defined in top-level. You can disallow the property for other variants, but I claim you won't have here other variants.

https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindi...

•      enum: [2, 4, 8]
  

•  required:
  

•    - starfive,i2s-max-channels
  

+required:

required goes after properties.

• • compatible
• • reg
• • clocks
• • clock-names
• • resets

+oneOf:

• • required:

•  - dmas
  

•  - dma-names
  

• • required:

•  - interrupts
  

This won't work. Provide both interrupts and dmas, and then test your DTS.

+unevaluatedProperties: false

Best regards, Krzysztof