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

On 27/03/2024 13:12, Krzysztof Kozlowski wrote:

On 26/03/2024 14:43, Xingyu Wu wrote:

quoted

quoted

quoted

quoted

quoted

+
+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?

As I said, it is not usable on its own, so unless you have other arguments then

no.

quoted

quoted

But my point was that no one asked for this.

I want to keep the original IP compatible which can distinguish from the JH8100

SoC.

quoted

Can I write it like this:
compatible:
   enum:
          - starfive,jh8100-i2s
   const: cdns,i2s-mc

and I write this in the DTS:
compatible = "starfive,jh8100-i2s", "cdns,i2s-mc";

Can you provide any rationale for this? I asked "unless you have other
arguments", so where are the arguments?

Nothing was explained in patch changelog. Nothing was provided in this email
thread.

I don't know if I understood what mark said[1]. Is it to keep the original IP and
add other compatible?

[1] https://lore.kernel.org/all/27155281-573c-493d-96fe-1f28ebb0ce5e@sirena.org.uk/ (local)

Or should I only keep the compatible 'starfive,jh8110-i2s' and change the
bindings name to same to this compatible?

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

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?

Drop description. Keep property, if your hardware has interrupts.

Will drop.

quoted

...

quoted

quoted

quoted

+  - 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:

quoted

'More than one condition true in one of shema: ...'

Exactly. Having both properties is a correct DTS. Interrupts do not
disappear just because you decide to describe DMA. It is OS choice
what to use if both are provided.

But this I2S can only use either DMA or interrupts.

Just like many other components. DTS should reflect hardware. Hardware has
interrupts and DMA, right?

Yes. The hardware can use interrupts and provide the handshake interface of
DMA to DMA controller. In software, we can choose only one between them.
Do I keep them both in the bindings and provide the selection in the driver?

quoted

Can I use the config (like SND_SOC_CADENCE_I2S_MC_PCM)  to choose DMA
or interrupt if having both them in DTS?

Don't know, I tend to focus here on bindings.

Best regards,
Xingyu Wu