On 22/07/2024 16:02, Kousik Sanagavarapu wrote: > On Mon, Jul 22, 2024 at 03:50:15PM +0200, Krzysztof Kozlowski wrote: >> On 22/07/2024 15:12, Kousik Sanagavarapu wrote: >>> On Mon, Jul 22, 2024 at 10:15:03AM +0200, Krzysztof Kozlowski wrote: >>>> On 21/07/2024 18:28, Kousik Sanagavarapu wrote: >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - ti,davinci-wdt >>>>> + - ti,keystone-wdt >>>> >>>> This does not match the original binding and commit msg did not explain >>>> why such change is necessary. >>> >>> I don't understand. Do you mean both the compatibles are always >>> compulsory? Meaning >>> >>> compatible: >>> items: >>> - const: ti,davinci-wdt >>> - const: ti,keystone-wdt >> >> Yes, this is what old binding said. > > That was what I thought initially too, but the example in the old > binding says otherwise and also the DTS from ti/davinci/da850.dtsi > says > > wdt: watchdog@21000 { > compatible = "ti,davinci-wdt"; > reg = <0x21000 0x1000>; > clocks = <&pll0_auxclk>; > status = "disabled"; > }; > > Or am I seeing it the wrong way? > >>> >>> It is enum because I intended it to align with the subsequent patch >>> which changes DTS. >>> >>>> This also does not match DTS. >>> >>> Yes. I've asked about changing the DTS in the subsequent patch. >>> >> >> Changing the DTS cannot be the reason to affect users and DTS... It's >> tautology. You change DTS because you intent to change DTS? > > Not exactly. I thought that the DTS was wrong when it said > > compatible = "ti,keystone-wdt", "ti,davinci-wdt"; > > while it should have been > > compatible = "ti,keystone-wdt"; > > I was not sure about this though and hence marked both the patches as > RFC, in case I was interpretting them the wrong way. Ah, right, the DTS says keystone+davinci while old binding suggested davinci+keystone. Considering there is no driver binding to keystone, I think the answer is obvious - intention was keystone+davinci. Anyway, commit msg should mention why you are doing something else than pure conversion. Best regards, Krzysztof