On 05/12/2022 09:00, Geert Uytterhoeven wrote: > Hi Krzysztof, > > Thanks for your comments! > > On Sat, Dec 3, 2022 at 1:13 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> On 02/12/2022 13:55, Geert Uytterhoeven wrote: >>> Convert the Texas Instruments PCM3168A Audio Codec Device Tree binding >>> documentation to json-schema. >>> >>> Add missing properties. >>> Drop unneeded pinctrl properties from example. >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +description: >>> + The Texas Instruments PCM3168A is a 24-bit Multi-channel Audio CODEC with >>> + 96/192kHz sampling rate, supporting both SPI and I2C bus access. >>> + >>> +properties: >>> + compatible: >>> + const: ti,pcm3168a >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + items: >>> + - description: System clock input >>> + >>> + clock-names: >>> + items: >>> + - const: scki >>> + >>> + reset-gpios: >>> + items: >>> + - description: | >>> + GPIO line connected to the active-low RST pin of the codec. >>> + RST = low: device power-down >>> + RST = high: device is enabled >>> + >>> + "#sound-dai-cells": >>> + enum: [0, 1] >> >> This is a bit unexpected. Looking at DTSes: >> 1. I see in ulcb-kf.dtsi with cells==0, but two endpoints. The dai cells >> seem unused? In such case shall dai-cells be skipped if we have endpoints? >> >> 2. in k3-j721e-common-proc-board.dts has cells=1, but user's phandle >> does not have an argument, so practically it is ==0? The user - >> sound/soc/ti/j721e-evm.c - just gets the node and does not use cells, >> right? So even though dai-cells==1, it does not matter, because user >> gets its own parsing? >> >> 3. The pcm3168a driver also does not have any xlate function, but it >> registers to DAIs, so all uses should be with cells==1 to select proper >> DAI... > > I have no idea (DAI is magic to me), and hope the audio experts > can provide some input... It is a bit of magic to me too. Yet I think the correct usage of this is with dai-cells=1. I think we can skip the choice here between sound-dai-cells and endpoints (to allow only one) and only fix the value here to =1. This would also require changing arch/arm64/boot/dts/renesas/ulcb-kf.dtsi to dai-cells=1. Best regards, Krzysztof