回复: [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.

I wonder if I should keep the original IP compatible. Do I not need it?

...

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

Do I keep this property and drop this description?

...

• 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... xample-schema.yaml#L212

Will fix.

...

•      enum: [2, 4, 8]
  

•  required:
  

•    - starfive,i2s-max-channels
  

+required:

required goes after properties.

Will fix.

...

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

I provided both properties in the DTS and test by dtbs_check. Then it printed that: 'More than one condition true in one of shema: ...'

...

+unevaluatedProperties: false

Best regards, Krzysztof

Best regards, Xingyu Wu