On 27/03/2024 13:12, Krzysztof Kozlowski wrote: > On 26/03/2024 14:43, Xingyu Wu wrote: > >>>>> + > >>>>> +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"; > > 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@xxxxxxxxxxxxx/ Or should I only keep the compatible 'starfive,jh8110-i2s' and change the bindings name to same to this compatible? > > > > >> > >>> > >>>> > >>>>> + - 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. > > 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? > > > > > 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
