From: Crystal Guo <crystal.guo@mediatek.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Matthias Brugger" <matthias.bgg@gmail.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
srv_heupstream <srv_heupstream@mediatek.com>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
"Seiya Wang (王迺君)" <seiya.wang@mediatek.com>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>
Subject: Re: [v5,0/4] watchdog: mt8192: add wdt support
Date: Fri, 9 Oct 2020 11:20:21 +0800 [thread overview]
Message-ID: <1602213621.14806.43.camel@mhfsdcap03> (raw)
In-Reply-To: <9d493ec4-6b3e-12a1-01f3-34c09a826290@roeck-us.net>
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
WARNING: multiple messages have this Message-ID (diff)
From: Crystal Guo <crystal.guo@mediatek.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
srv_heupstream <srv_heupstream@mediatek.com>,
"Seiya Wang (王迺君)" <seiya.wang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [v5,0/4] watchdog: mt8192: add wdt support
Date: Fri, 9 Oct 2020 11:20:21 +0800 [thread overview]
Message-ID: <1602213621.14806.43.camel@mhfsdcap03> (raw)
In-Reply-To: <9d493ec4-6b3e-12a1-01f3-34c09a826290@roeck-us.net>
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
WARNING: multiple messages have this Message-ID (diff)
From: Crystal Guo <crystal.guo@mediatek.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
srv_heupstream <srv_heupstream@mediatek.com>,
"Seiya Wang (王迺君)" <seiya.wang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [v5,0/4] watchdog: mt8192: add wdt support
Date: Fri, 9 Oct 2020 11:20:21 +0800 [thread overview]
Message-ID: <1602213621.14806.43.camel@mhfsdcap03> (raw)
In-Reply-To: <9d493ec4-6b3e-12a1-01f3-34c09a826290@roeck-us.net>
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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-10-09 3:20 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` 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, 1/4] " 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
2020-09-29 3:20 ` Crystal Guo
2020-09-29 3:20 ` Crystal Guo
2020-10-02 9:28 ` Matthias Brugger
2020-10-02 9:28 ` Matthias Brugger
2020-10-02 9:28 ` Matthias Brugger
2020-10-09 3:32 ` Crystal Guo
2020-10-09 3:32 ` Crystal Guo
2020-10-09 3:32 ` Crystal Guo
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-09-29 3:20 ` Crystal Guo
2020-09-29 3:20 ` [v5,4/4] watchdog: mt8192: add wdt support Crystal Guo
2020-09-29 3:20 ` Crystal Guo
2020-09-29 3:20 ` Crystal Guo
2020-10-01 14:23 ` [v5,0/4] " Matthias Brugger
2020-10-01 14:23 ` Matthias Brugger
2020-10-01 14:23 ` Matthias Brugger
2020-10-01 15:16 ` Guenter Roeck
2020-10-01 15:16 ` Guenter Roeck
2020-10-01 15:16 ` Guenter Roeck
2020-10-02 9:51 ` Matthias Brugger
2020-10-02 9:51 ` Matthias Brugger
2020-10-02 9:51 ` Matthias Brugger
2020-10-02 14:41 ` Guenter Roeck
2020-10-02 14:41 ` Guenter Roeck
2020-10-02 14:41 ` Guenter Roeck
2020-10-09 3:20 ` Crystal Guo [this message]
2020-10-09 3:20 ` Crystal Guo
2020-10-09 3:20 ` Crystal Guo
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=1602213621.14806.43.camel@mhfsdcap03 \
--to=crystal.guo@mediatek.com \
--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=robh+dt@kernel.org \
--cc=seiya.wang@mediatek.com \
--cc=srv_heupstream@mediatek.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.