linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Kousik Sanagavarapu <five231003@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Guenter Roeck <linux@roeck-us.net>, Nishanth Menon <nm@ti.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema
Date: Mon, 22 Jul 2024 16:08:11 +0200	[thread overview]
Message-ID: <0f1becb9-56e9-4b71-b9ca-263dd6592c43@kernel.org> (raw)
In-Reply-To: <Zp5mkcDca6jRvOnf@five231003>

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



  reply	other threads:[~2024-07-22 14:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-21 16:28 [PATCH 0/3] ti: davinci, keystone: txt to yaml Kousik Sanagavarapu
2024-07-21 16:28 ` [PATCH 1/3] dt-bindings: timer: ti,davinci-timer: convert to dtschema Kousik Sanagavarapu
2024-07-22  8:11   ` Krzysztof Kozlowski
2024-07-22 12:59     ` Kousik Sanagavarapu
2024-07-21 16:28 ` [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: " Kousik Sanagavarapu
2024-07-22  8:15   ` Krzysztof Kozlowski
2024-07-22 13:12     ` Kousik Sanagavarapu
2024-07-22 13:50       ` Krzysztof Kozlowski
2024-07-22 14:02         ` Kousik Sanagavarapu
2024-07-22 14:08           ` Krzysztof Kozlowski [this message]
2024-07-21 16:28 ` [RFC PATCH 3/3] ARM: dts: davinci, keystone: correct watchdog nodenames Kousik Sanagavarapu
2024-07-22  8:13   ` Krzysztof Kozlowski
2024-07-22 13:21     ` Kousik Sanagavarapu
2024-07-22 14:07       ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0f1becb9-56e9-4b71-b9ca-263dd6592c43@kernel.org \
    --to=krzk@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=five231003@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=nm@ti.com \
    --cc=robh@kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).