From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Daniel Golle <daniel@makrotopia.org>
Cc: AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
Wim Van Sebroeck <wim@linux-watchdog.org>,
Guenter Roeck <linux@roeck-us.net>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu
Date: Fri, 10 Nov 2023 20:58:57 +0100 [thread overview]
Message-ID: <8846adad-9504-4ffd-b9c3-f5e78082d261@linaro.org> (raw)
In-Reply-To: <ZU5jU-T0m5QW4ZeF@makrotopia.org>
On 10/11/2023 18:07, Daniel Golle wrote:
> On Fri, Nov 10, 2023 at 04:21:35PM +0100, Krzysztof Kozlowski wrote:
>> On 10/11/2023 16:15, Krzysztof Kozlowski wrote:
>>>>>> So adding the file to include/dt-bindings/reset/ should go into a
>>>>>> seperate patch? Because including it with the driver itself gave me
>>>>>> a checkpath warning telling me that dt-bindings should go seperate,
>>>>>> which is why I included it with the binding docs.
>>>>>
>>>>> No, I said the hunk should be dropped. Removed.
>>>>
>>>> I guess we are somehow misunderstanding each other.
>>>> Lets go with an example. I can put the header into a commit of its own,
>>>> just like commit
>>>> 5794dda109fc8 dt-bindings: reset: mt7986: Add reset-controller header file
>>>> https://lore.kernel.org/r/20220105100456.7126-2-sam.shih@mediatek.com
>>>>
>>>> Would that be acceptable? And if not, why?
>>>
>>> ...this question.
>
> ... which you didn't answer. Sorry, but it's not helpful to be polemic
> or ironic in a code review involving non-native English speakers
> trying to understand each others.
Why do you keep skipping - third time - my earlier reply? The earlier
paragraph?
Cutting lines out of context is not how this should be discussed.
So let me bring it:
>> FOO
> Here is the answer to...
>> BAR
> ...this question.
The "FOO" Is the answer. Go open the emails and read the answer.
>
>>>
>>> Again, whether this is separate patch - it is still hunk which I think
>>> should be removed. I gave the reason "why" in this mail thread and in
>>> multiple other discussions.
>>
>> I gave you clear reasoning 7 hours ago:
>> https://lore.kernel.org/all/59629ec1-cc0c-4c5a-87cc-ea30d64ec191@linaro.org/
>> to which you did not respond.
>
> Because it doesn't match anything existing regarding MediaTek reset
> drivers, and I was assuming there must be some kind of misunderstanding,
> which is why I replied to your later email in the same thread.
>
> My assumption that the problem was merely having documentation and
> header combined in a single commit stems from the fact that a very
> similar patch for MT7986[1] was Ack'ed by Rob Herring about a year and
> a half ago; hence the rule you apply here may have always existed, but
> apparently then hasn't been applied in the past.
>
> Literally *all* existing dt-binding headers for MediaTek SoCs follow a
> direct 1:1 mapping of reset bit in hardware and reset number in the
> header files. The driver is simple, all it cares about is the maximum
> number defined in the header (and I like that, because it makes it very
> easy to add new SoCs). At this point the abstraction needed to
> fulfill your request doesn't exist, not for any of the SoCs using
> mtk_wdt.c. It can be implemented, surely, it's a problem computers can
> solve. If that's what you (and current maintainers of that driver)
> would want me to implement, please say so clearly and spell it out.
>
> Also be clear about if all the other existing headers need to be
> converted, mappings for all SoCs created in the driver, ... all before
> support for MT7988 can go in?
> Or should the existing headers for other MediaTek SoCs remain
> untouched because they are already considered stable API or something?
I am repeating myself... but I don't know how to put it other way. I did
not ask you to rewrite your driver. I asked to drop the change to
bindings, because it is entirely pointless.
Drop this change, only this. No need to rewrite drivers, they stay the same.
Best regards,
Krzysztof
next prev parent reply other threads:[~2023-11-10 19:59 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-10 0:30 [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu Daniel Golle
2023-11-10 0:30 ` [PATCH 2/2] watchdog: mediatek: mt7988: add wdt support Daniel Golle
2023-11-10 5:24 ` Guenter Roeck
2023-11-10 11:55 ` AngeloGioacchino Del Regno
2023-11-10 15:13 ` Guenter Roeck
2023-11-10 8:09 ` [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu Krzysztof Kozlowski
2023-11-10 15:20 ` Krzysztof Kozlowski
2023-11-10 20:00 ` Krzysztof Kozlowski
2023-11-10 20:45 ` Daniel Golle
2023-11-11 7:55 ` Krzysztof Kozlowski
2023-11-10 11:56 ` AngeloGioacchino Del Regno
2023-11-10 14:17 ` Daniel Golle
2023-11-10 14:20 ` Krzysztof Kozlowski
2023-11-10 14:40 ` Daniel Golle
2023-11-10 14:46 ` Krzysztof Kozlowski
2023-11-10 14:51 ` Daniel Golle
2023-11-10 15:07 ` Krzysztof Kozlowski
2023-11-10 15:12 ` Daniel Golle
2023-11-10 15:15 ` Krzysztof Kozlowski
2023-11-10 15:21 ` Krzysztof Kozlowski
2023-11-10 17:07 ` Daniel Golle
2023-11-10 19:58 ` Krzysztof Kozlowski [this message]
2023-11-10 20:18 ` Daniel Golle
2023-11-10 20:21 ` 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=8846adad-9504-4ffd-b9c3-f5e78082d261@linaro.org \
--to=krzysztof.kozlowski@linaro.org \
--cc=angelogioacchino.delregno@collabora.com \
--cc=conor+dt@kernel.org \
--cc=daniel@makrotopia.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=matthias.bgg@gmail.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--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 read-only IMAP folder(s) and NNTP newsgroup(s).