On 14/02/2023 14:02, Alexander Sverdlin wrote: > Hello Krzysztof, > > thank you for the quick review! > > On Tue, 2023-02-14 at 11:58 +0100, Krzysztof Kozlowski wrote: >>> +properties: >>> + compatible: >>> + const: cirrus,ep9301-i2s >>> + >>> + '#sound-dai-cells': >>> + const: 0 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + minItems: 3 >> >> maxItems instead > > reg and clocks are required, I suppose I should include both minItems > and maxItems for both of them? No. minItems is implied. > >>> + >>> + clock-names: >>> + items: >>> + - const: mclk >>> + - const: sclk >>> + - const: lrclk >> >> >> The clk suffixes are quite redundant. Don't these inputs have some >> meaningful name? > > They are actually meaningful, as they are usually named in I2S, please > refer to the EP93xx User's Guide: > https://cdn.embeddedts.com/resource-attachments/ts-7000_ep9301-ug.pdf > page 71, for instance. OK, but then I like the example - if datasheet would use name "clk_clk_this_is_clk" would you still find it meaningful? Every clock input in clocks is a clock. There is usually no need to say that a clock is a clock... > Best regards, Krzysztof