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

Reply via email to