linux-kernel - Re: [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8846adad-9504-4ffd-b9c3-f5e78082d261@linaro.org>
Date:   Fri, 10 Nov 2023 20:58:57 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Daniel Golle <daniel@...rotopia.org>
Cc:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988
 watchdog and toprgu

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