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

From: Xingyu Wu <xingyu.wu@starfivetech.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Claudiu Beznea <Claudiu.Beznea@microchip.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor.dooley@microchip.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"linux-sound@vger.kernel.org" <linux-sound@vger.kernel.org>
Subject: RE: 回复: [PATCH v2 1/2] ASoC: dt-bindings: Add bindings for Cadence I2S-MC controller
Date: Tue, 26 Mar 2024 13:43:21 +0000	[thread overview]
Message-ID: <NTZPR01MB0956230296D881F112F92D119F35A@NTZPR01MB0956.CHNPR01.prod.partner.outlook.cn> (raw)
In-Reply-To: <9b1a5100-8536-4b59-b2e7-d6ebd2ba9e66@linaro.org>

On 26/03/2024 14:46, Krzysztof Kozlowski wrote:
> 
> On 26/03/2024 07:29, Xingyu Wu wrote:
> >>
> >> On 20/03/2024 10:02, Xingyu Wu wrote:
> >>> Add bindings for the Multi-Channel I2S controller of Cadence.
> 
> Your email app responds very weird. You bypassed all possible filters and it is
> simply not visible that you answer to me. You Reply to everyone, not to me with
> Cc to others. There is no standard header whom do you quote, e.g. "On
> 26/03/2024 07:29, Xingyu Wu wrote:"
> 
> Please use some decent email system, otherwise I will miss all replies from you.

Thank you for reminding me. I will correct it.

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

> 
> >
> >>
> >>> +      - 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?
> 
> Drop description. Keep property, if your hardware has interrupts.
> 

Will drop.

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

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

Best regards,
Xingyu Wu