On 17/01/2024 10:37, Tim Lunn wrote: >> You can drop the description. > Yes just 1 clock, i will fix this. >> >>> + >>> + clock-names: >>> + description: >>> + The clock name for the codec clock. >> Drop. > Just drop the description? I dont think can drop the clock names as the > driver use the name to lookup clock: Description. But anyway the problem is that adding clocks should be separate patch with its own explanation. > > devm_clk_get(pdev->dev.parent, "mclk"); >> >>> + items: >>> + - const: mclk >>> + >>> + '#sound-dai-cells': >>> + description: >>> + Needed for the interpretation of sound dais. >> Common property, don't need the description. > Ok >> >>> + const: 0 >>> + >>> + codec: >>> + description: | >>> + The child node for the codec to hold additional properties. If no >>> + additional properties are required for the codec, this node can be >>> + omitted. >> Why do you need a child node here? Just put the 1 property in the parent >> node. > This is how the existing rk817 codec driver was setup. I suppose it was > copied from downstream, where there are more properties than just the > one. I don't know if there was any intention (or need) to implement > those other properties. >> You need to clearly express ABI requirements in the commit msg. Otherwise you will get a review like for new bindings. Best regards, Krzysztof
