[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <89835d94-7f26-bdc3-4760-af00978aba44@gmail.com>
Date: Fri, 2 Oct 2020 11:51:29 +0200
From: Matthias Brugger <matthias.bgg@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Crystal Guo <crystal.guo@...iatek.com>, robh+dt@...nel.org,
srv_heupstream@...iatek.com, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-watchdog@...r.kernel.org, seiya.wang@...iatek.com,
Wim Van Sebroeck <wim@...ux-watchdog.org>
Subject: Re: [v5,0/4] watchdog: mt8192: add wdt support
On 01/10/2020 17:16, Guenter Roeck wrote:
> On Thu, Oct 01, 2020 at 04:23:02PM +0200, Matthias Brugger wrote:
>> Hi Crystal,
>>
>> It seems you forgot to send the email to one of the maintainers, Wim.
>> Please make sure you add all the maintainers from get_maintainers.pl when
>> you send a series.
>>
>> Regards,
>> Matthias
>>
>> On 29/09/2020 05:20, Crystal Guo wrote:
>>> v5 changes:
>>> fix typos on:
>>> https://patchwork.kernel.org/patch/11697493/
>>>
>>> v4 changes:
>>> revise commit messages.
>>>
>>> v3 changes:
>>> https://patchwork.kernel.org/patch/11692731/
>>> https://patchwork.kernel.org/patch/11692767/
>>> https://patchwork.kernel.org/patch/11692729/
>>> https://patchwork.kernel.org/patch/11692771/
>>> https://patchwork.kernel.org/patch/11692733/
>
> This is less than helpful. It doesn't tell me anything. It expects me to
> go back to the earlier versions, download them, and run a diff, to figure
> out what changed. That means the patch or patch series ends at the bottom
> of my pile of patches to review. Which, as it happens, is quite deep.
>
> I will review this and similar patches without change log after (and only
> after) I have reviewed all other patches in my queue.
>
I understand your comments on hard to understand change log. But I think you
acted to quick to put this series to the end of your queue. I'll try to explain:
In v4 you gave your Acked-by and Reviewed-by for the patches that in this series
are 3/4 [1] and 4/4 [2] respectively. You also gave your Reviewed-by for 1/4 [3].
In v4 you stated that you wanted to wait for a review from Rob for the binding
changes. Obviously it's up to you to handle that the way you want. From my point
of view these are rather trivial changes.
In 1/4 are deleting compatible fallbacks in the bindings, as the driver provides
SoC specific platform data, which you reviewed.
One can argue that this will break older devicetree bindings because the driver
using the fallback would work except for the topgru reset controller. But I
think this is the job of the maintainer of the driver as Rob won't be able to
look into all the driver code to decide if any change to the bindings is
backward compatible. With your Reviewed-by I understand that you are OK with
this change. As SoC maintainer I'm fine with the change. MT2701 is a SoC that's
not available to the general public. MT8183 is available, but only on
chromebooks and I don't expect anybody to use an older kernel without watchdog
driver support for mt8183 (enablement is still ongoing). Actually I took the DTS
counter part already through my tree, which was an error, as we now have a DTS
which does not hold to the binding description (until and if you accept 1/4).
The only patch missing patch is now 2/4, where Crystal added your Reviewed-by
which you never gave. But it just adds the compatible to the binding for a
driver you already gave your Reviewed-by. So I think this the series actually
just fall through the cracks.
Sorry for the long mail, but if you got until here, I hope I was able to
convince you to just merge the series :)
Best regards,
Matthias
[1] https://patchwork.kernel.org/patch/11697493/
[2] https://patchwork.kernel.org/patch/11697483/
[3] https://patchwork.kernel.org/patch/11697477/
> Guenter
>
>>>
>>> Crystal Guo (4):
>>> dt-binding: mediatek: watchdog: fix the description of compatible
>>> dt-binding: mediatek: mt8192: update mtk-wdt document
>>> dt-binding: mt8192: add toprgu reset-controller head file
>>> watchdog: mt8192: add wdt support
>>>
>>> .../devicetree/bindings/watchdog/mtk-wdt.txt | 5 ++--
>>> drivers/watchdog/mtk_wdt.c | 6 +++++
>>> .../dt-bindings/reset-controller/mt8192-resets.h | 30 ++++++++++++++++++++++
>>> 3 files changed, 39 insertions(+), 2 deletions(-)
>>> create mode 100644 include/dt-bindings/reset-controller/mt8192-resets.h
>>>
Powered by blists - more mailing lists