* [v5, 1/4] dt-binding: mediatek: watchdog: fix the description of compatible
2020-09-29 3:20 [v5,0/4] watchdog: mt8192: add wdt support Crystal Guo
@ 2020-09-29 3:20 ` Crystal Guo
2020-09-29 3:20 ` [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document Crystal Guo
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Crystal Guo @ 2020-09-29 3:20 UTC (permalink / raw)
To: linux, robh+dt, matthias.bgg
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, Crystal Guo, linux-arm-kernel
The watchdog driver for MT2712 and MT8183 relies on DT data, so
the fallback compatible MT6589 won't work.
Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
index 4dd36bd3f1ad..45eedc2c3141 100644
--- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
@@ -4,13 +4,13 @@ Required properties:
- compatible should contain:
"mediatek,mt2701-wdt", "mediatek,mt6589-wdt": for MT2701
- "mediatek,mt2712-wdt", "mediatek,mt6589-wdt": for MT2712
+ "mediatek,mt2712-wdt": for MT2712
"mediatek,mt6589-wdt": for MT6589
"mediatek,mt6797-wdt", "mediatek,mt6589-wdt": for MT6797
"mediatek,mt7622-wdt", "mediatek,mt6589-wdt": for MT7622
"mediatek,mt7623-wdt", "mediatek,mt6589-wdt": for MT7623
"mediatek,mt7629-wdt", "mediatek,mt6589-wdt": for MT7629
- "mediatek,mt8183-wdt", "mediatek,mt6589-wdt": for MT8183
+ "mediatek,mt8183-wdt": for MT8183
"mediatek,mt8516-wdt", "mediatek,mt6589-wdt": for MT8516
- reg : Specifies base physical address and size of the registers.
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document
2020-09-29 3:20 [v5,0/4] watchdog: mt8192: add wdt support Crystal Guo
2020-09-29 3:20 ` [v5, 1/4] dt-binding: mediatek: watchdog: fix the description of compatible Crystal Guo
@ 2020-09-29 3:20 ` Crystal Guo
2020-10-02 9:28 ` Matthias Brugger
2020-09-29 3:20 ` [v5,3/4] dt-binding: mt8192: add toprgu reset-controller head file Crystal Guo
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Crystal Guo @ 2020-09-29 3:20 UTC (permalink / raw)
To: linux, robh+dt, matthias.bgg
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, Crystal Guo, linux-arm-kernel
update mtk-wdt document for MT8192 platform
Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
index 45eedc2c3141..e36ba60de829 100644
--- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
@@ -12,6 +12,7 @@ Required properties:
"mediatek,mt7629-wdt", "mediatek,mt6589-wdt": for MT7629
"mediatek,mt8183-wdt": for MT8183
"mediatek,mt8516-wdt", "mediatek,mt6589-wdt": for MT8516
+ "mediatek,mt8192-wdt": for MT8192
- reg : Specifies base physical address and size of the registers.
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document
2020-09-29 3:20 ` [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document Crystal Guo
@ 2020-10-02 9:28 ` Matthias Brugger
2020-10-09 3:32 ` Crystal Guo
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Brugger @ 2020-10-02 9:28 UTC (permalink / raw)
To: Crystal Guo, linux, robh+dt
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, linux-arm-kernel
On 29/09/2020 05:20, Crystal Guo wrote:
> update mtk-wdt document for MT8192 platform
>
> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
You added Guenters Reviewed-by in v4 of this series, but I don't see that on
Guenter provided this tag. In the future please make sure that you don't add
tags to your patches that were not provided. This creates great confusion.
Regards,
Matthias
> ---
> Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> index 45eedc2c3141..e36ba60de829 100644
> --- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> @@ -12,6 +12,7 @@ Required properties:
> "mediatek,mt7629-wdt", "mediatek,mt6589-wdt": for MT7629
> "mediatek,mt8183-wdt": for MT8183
> "mediatek,mt8516-wdt", "mediatek,mt6589-wdt": for MT8516
> + "mediatek,mt8192-wdt": for MT8192
>
> - reg : Specifies base physical address and size of the registers.
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document
2020-10-02 9:28 ` Matthias Brugger
@ 2020-10-09 3:32 ` Crystal Guo
0 siblings, 0 replies; 12+ messages in thread
From: Crystal Guo @ 2020-10-09 3:32 UTC (permalink / raw)
To: Matthias Brugger, linux, Wim Van Sebroeck
Cc: linux-watchdog, srv_heupstream,
Seiya Wang (王迺君),
linux-kernel, robh+dt, linux-mediatek, linux-arm-kernel
On Fri, 2020-10-02 at 17:28 +0800, Matthias Brugger wrote:
>
> On 29/09/2020 05:20, Crystal Guo wrote:
> > update mtk-wdt document for MT8192 platform
> >
> > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> You added Guenters Reviewed-by in v4 of this series, but I don't see that on
> Guenter provided this tag. In the future please make sure that you don't add
> tags to your patches that were not provided. This creates great confusion.
>
> Regards,
> Matthias
>
Hi Guenter,
Should I remove the "Reviewed-by:Guenter" tag, and re-submit this patch
for your review, or keeping the status quo and wait for Wim's comment?
Thanks
Crystal
> > ---
> > Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> > index 45eedc2c3141..e36ba60de829 100644
> > --- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> > @@ -12,6 +12,7 @@ Required properties:
> > "mediatek,mt7629-wdt", "mediatek,mt6589-wdt": for MT7629
> > "mediatek,mt8183-wdt": for MT8183
> > "mediatek,mt8516-wdt", "mediatek,mt6589-wdt": for MT8516
> > + "mediatek,mt8192-wdt": for MT8192
> >
> > - reg : Specifies base physical address and size of the registers.
> >
> >
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* [v5,3/4] dt-binding: mt8192: add toprgu reset-controller head file
2020-09-29 3:20 [v5,0/4] watchdog: mt8192: add wdt support Crystal Guo
2020-09-29 3:20 ` [v5, 1/4] dt-binding: mediatek: watchdog: fix the description of compatible Crystal Guo
2020-09-29 3:20 ` [v5,2/4] dt-binding: mediatek: mt8192: update mtk-wdt document Crystal Guo
@ 2020-09-29 3:20 ` Crystal Guo
2020-09-29 3:20 ` [v5,4/4] watchdog: mt8192: add wdt support Crystal Guo
2020-10-01 14:23 ` [v5,0/4] " Matthias Brugger
4 siblings, 0 replies; 12+ messages in thread
From: Crystal Guo @ 2020-09-29 3:20 UTC (permalink / raw)
To: linux, robh+dt, matthias.bgg
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, Crystal Guo, linux-arm-kernel
add toprgu reset-controller head file for MT8192 platform
Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
.../reset-controller/mt8192-resets.h | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 include/dt-bindings/reset-controller/mt8192-resets.h
diff --git a/include/dt-bindings/reset-controller/mt8192-resets.h b/include/dt-bindings/reset-controller/mt8192-resets.h
new file mode 100644
index 000000000000..be9a7ca245b9
--- /dev/null
+++ b/include/dt-bindings/reset-controller/mt8192-resets.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ * Author: Yong Liang <yong.liang@mediatek.com>
+ */
+
+#ifndef _DT_BINDINGS_RESET_CONTROLLER_MT8192
+#define _DT_BINDINGS_RESET_CONTROLLER_MT8192
+
+#define MT8192_TOPRGU_MM_SW_RST 1
+#define MT8192_TOPRGU_MFG_SW_RST 2
+#define MT8192_TOPRGU_VENC_SW_RST 3
+#define MT8192_TOPRGU_VDEC_SW_RST 4
+#define MT8192_TOPRGU_IMG_SW_RST 5
+#define MT8192_TOPRGU_MD_SW_RST 7
+#define MT8192_TOPRGU_CONN_SW_RST 9
+#define MT8192_TOPRGU_CONN_MCU_SW_RST 12
+#define MT8192_TOPRGU_IPU0_SW_RST 14
+#define MT8192_TOPRGU_IPU1_SW_RST 15
+#define MT8192_TOPRGU_AUDIO_SW_RST 17
+#define MT8192_TOPRGU_CAMSYS_SW_RST 18
+#define MT8192_TOPRGU_MJC_SW_RST 19
+#define MT8192_TOPRGU_C2K_S2_SW_RST 20
+#define MT8192_TOPRGU_C2K_SW_RST 21
+#define MT8192_TOPRGU_PERI_SW_RST 22
+#define MT8192_TOPRGU_PERI_AO_SW_RST 23
+
+#define MT8192_TOPRGU_SW_RST_NUM 23
+
+#endif /* _DT_BINDINGS_RESET_CONTROLLER_MT8192 */
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [v5,4/4] watchdog: mt8192: add wdt support
2020-09-29 3:20 [v5,0/4] watchdog: mt8192: add wdt support Crystal Guo
` (2 preceding siblings ...)
2020-09-29 3:20 ` [v5,3/4] dt-binding: mt8192: add toprgu reset-controller head file Crystal Guo
@ 2020-09-29 3:20 ` Crystal Guo
2020-10-01 14:23 ` [v5,0/4] " Matthias Brugger
4 siblings, 0 replies; 12+ messages in thread
From: Crystal Guo @ 2020-09-29 3:20 UTC (permalink / raw)
To: linux, robh+dt, matthias.bgg
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, Crystal Guo, linux-arm-kernel
Add support for watchdog device found in MT8192 SoC
Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/mtk_wdt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index d6a6393f609d..aef0c2db6a11 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -11,6 +11,7 @@
#include <dt-bindings/reset-controller/mt2712-resets.h>
#include <dt-bindings/reset-controller/mt8183-resets.h>
+#include <dt-bindings/reset-controller/mt8192-resets.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -76,6 +77,10 @@ static const struct mtk_wdt_data mt8183_data = {
.toprgu_sw_rst_num = MT8183_TOPRGU_SW_RST_NUM,
};
+static const struct mtk_wdt_data mt8192_data = {
+ .toprgu_sw_rst_num = MT8192_TOPRGU_SW_RST_NUM,
+};
+
static int toprgu_reset_update(struct reset_controller_dev *rcdev,
unsigned long id, bool assert)
{
@@ -322,6 +327,7 @@ static const struct of_device_id mtk_wdt_dt_ids[] = {
{ .compatible = "mediatek,mt2712-wdt", .data = &mt2712_data },
{ .compatible = "mediatek,mt6589-wdt" },
{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_data },
+ { .compatible = "mediatek,mt8192-wdt", .data = &mt8192_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_wdt_dt_ids);
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [v5,0/4] watchdog: mt8192: add wdt support
2020-09-29 3:20 [v5,0/4] watchdog: mt8192: add wdt support Crystal Guo
` (3 preceding siblings ...)
2020-09-29 3:20 ` [v5,4/4] watchdog: mt8192: add wdt support Crystal Guo
@ 2020-10-01 14:23 ` Matthias Brugger
2020-10-01 15:16 ` Guenter Roeck
4 siblings, 1 reply; 12+ messages in thread
From: Matthias Brugger @ 2020-10-01 14:23 UTC (permalink / raw)
To: Crystal Guo, linux, robh+dt
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
linux-mediatek, Wim Van Sebroeck, linux-arm-kernel
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/
>
> 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
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5,0/4] watchdog: mt8192: add wdt support
2020-10-01 14:23 ` [v5,0/4] " Matthias Brugger
@ 2020-10-01 15:16 ` Guenter Roeck
2020-10-02 9:51 ` Matthias Brugger
0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2020-10-01 15:16 UTC (permalink / raw)
To: Matthias Brugger
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
robh+dt, linux-mediatek, Crystal Guo, Wim Van Sebroeck,
linux-arm-kernel
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.
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
> >
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5,0/4] watchdog: mt8192: add wdt support
2020-10-01 15:16 ` Guenter Roeck
@ 2020-10-02 9:51 ` Matthias Brugger
2020-10-02 14:41 ` Guenter Roeck
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Brugger @ 2020-10-02 9:51 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
robh+dt, linux-mediatek, Crystal Guo, Wim Van Sebroeck,
linux-arm-kernel
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
>>>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5,0/4] watchdog: mt8192: add wdt support
2020-10-02 9:51 ` Matthias Brugger
@ 2020-10-02 14:41 ` Guenter Roeck
2020-10-09 3:20 ` Crystal Guo
0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2020-10-02 14:41 UTC (permalink / raw)
To: Matthias Brugger
Cc: linux-watchdog, srv_heupstream, seiya.wang, linux-kernel,
robh+dt, linux-mediatek, Crystal Guo, Wim Van Sebroeck,
linux-arm-kernel
On 10/2/20 2:51 AM, Matthias Brugger wrote:
>
>
> 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.
>
That may be correct, but I am not a DT expert, and it happened often enough
that I approved a DT change and Rob later raised concerns that I don't do it
anymore.
> 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 :)
>
If my Reviewed-by is indeed in all patches, as you state, even if I didn't give it
to some of those and the submitter just added it (is that acceptable nowadays ?),
there should be no problem and Wim should pick up the series. And if the submitter
had bothered to write a proper change log instead of expecting me to do the work
I would have noticed right away.
No, this was very appropriately put to the end of my review queue.
Guenter
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [v5,0/4] watchdog: mt8192: add wdt support
2020-10-02 14:41 ` Guenter Roeck
@ 2020-10-09 3:20 ` Crystal Guo
0 siblings, 0 replies; 12+ messages in thread
From: Crystal Guo @ 2020-10-09 3:20 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-watchdog, srv_heupstream,
Seiya Wang (王迺君),
linux-kernel, robh+dt, linux-mediatek, Matthias Brugger,
Wim Van Sebroeck, linux-arm-kernel
On Fri, 2020-10-02 at 22:41 +0800, Guenter Roeck wrote:
> On 10/2/20 2:51 AM, Matthias Brugger wrote:
> >
> >
> > 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.
> >
>
> That may be correct, but I am not a DT expert, and it happened often enough
> that I approved a DT change and Rob later raised concerns that I don't do it
> anymore.
>
> > 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 :)
> >
>
> If my Reviewed-by is indeed in all patches, as you state, even if I didn't give it
> to some of those and the submitter just added it (is that acceptable nowadays ?),
> there should be no problem and Wim should pick up the series. And if the submitter
> had bothered to write a proper change log instead of expecting me to do the work
> I would have noticed right away.
>
> No, this was very appropriately put to the end of my review queue.
>
> Guenter
Sorry to cause you trouble, I will pay attention to these points in the
future.
Crystal
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread