2022-02-23 09:22:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v7 05/11] dt-bindings: pwm: add microchip corepwm binding

On 23/02/2022 07:20, Uwe Kleine-König wrote:
> On Mon, Feb 14, 2022 at 01:58:35PM +0000, [email protected] wrote:
>> From: Conor Dooley <[email protected]>
>>
>> Add device tree bindings for the Microchip fpga fabric based "core" PWM
>> controller.
>>
>> Reviewed-by: Rob Herring <[email protected]>
>> Signed-off-by: Conor Dooley <[email protected]>
>> Acked-by: Palmer Dabbelt <[email protected]>
>
> I like it:
>
> Acked-by: Uwe Kleine-König <[email protected]>
>
> nitpick: Put your S-o-b last in the commit log. (This doesn't justify a
> resend IMHO)

It should be the opposite - the first. First author signs the patch,
then comes review and finally an ack. Putting SoB at then suggests that
tags were accumulated before sending patch, out of mailing list.


Best regards,
Krzysztof


2022-02-24 01:04:39

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v7 05/11] dt-bindings: pwm: add microchip corepwm binding

On Wed, Feb 23, 2022 at 08:12:49AM +0100, Krzysztof Kozlowski wrote:
> On 23/02/2022 07:20, Uwe Kleine-K?nig wrote:
> > On Mon, Feb 14, 2022 at 01:58:35PM +0000, [email protected] wrote:
> >> From: Conor Dooley <[email protected]>
> >>
> >> Add device tree bindings for the Microchip fpga fabric based "core" PWM
> >> controller.
> >>
> >> Reviewed-by: Rob Herring <[email protected]>
> >> Signed-off-by: Conor Dooley <[email protected]>
> >> Acked-by: Palmer Dabbelt <[email protected]>
> >
> > I like it:
> >
> > Acked-by: Uwe Kleine-K?nig <[email protected]>
> >
> > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a
> > resend IMHO)
>
> It should be the opposite - the first. First author signs the patch,
> then comes review and finally an ack. Putting SoB at then suggests that
> tags were accumulated before sending patch, out of mailing list.

well, or in an earlier revision of this patch as is the case here. One
of the ideas of S-o-b is that the order shows the flow of the patch
states and if this patch ends in git with:

Referred-by: Rob Herring <[email protected]>
Singed-off-by: Conor Dooley <[email protected]>
Backed-by: Palmer Dabbelt <[email protected]>
Singed-off-by: Peter Maintainer <[email protected]>

I'd expect that Backed-by was added by Peter, not Conor.
(Modified the tags on purpose to not interfere with b4's tag pickup, I
guess you humans still get the point.)

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (1.65 kB)
signature.asc (499.00 B)
Download all attachments

2022-02-24 01:41:44

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 05/11] dt-bindings: pwm: add microchip corepwm binding

On Wed, 23 Feb 2022, Uwe Kleine-König wrote:

> On Wed, Feb 23, 2022 at 08:12:49AM +0100, Krzysztof Kozlowski wrote:
> > On 23/02/2022 07:20, Uwe Kleine-König wrote:
> > > On Mon, Feb 14, 2022 at 01:58:35PM +0000, [email protected] wrote:
> > >> From: Conor Dooley <[email protected]>
> > >>
> > >> Add device tree bindings for the Microchip fpga fabric based "core" PWM
> > >> controller.
> > >>
> > >> Reviewed-by: Rob Herring <[email protected]>
> > >> Signed-off-by: Conor Dooley <[email protected]>
> > >> Acked-by: Palmer Dabbelt <[email protected]>
> > >
> > > I like it:
> > >
> > > Acked-by: Uwe Kleine-König <[email protected]>
> > >
> > > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a
> > > resend IMHO)
> >
> > It should be the opposite - the first. First author signs the patch,
> > then comes review and finally an ack. Putting SoB at then suggests that
> > tags were accumulated before sending patch, out of mailing list.
>
> well, or in an earlier revision of this patch as is the case here. One
> of the ideas of S-o-b is that the order shows the flow of the patch
> states and if this patch ends in git with:
>
> Referred-by: Rob Herring <[email protected]>
> Singed-off-by: Conor Dooley <[email protected]>
> Backed-by: Palmer Dabbelt <[email protected]>
> Singed-off-by: Peter Maintainer <[email protected]>
>
> I'd expect that Backed-by was added by Peter, not Conor.
> (Modified the tags on purpose to not interfere with b4's tag pickup, I
> guess you humans still get the point.)

I tend to like *-by tags to appear chronologically.

Suggested (suggested-by)
Authored (signed-off-by)
Co-Authored (signed-off-by/co-developed-by)
Reviewed/Acked/Tested (reviewed-by/acked-by/tested-by)
Committed (signed-off-by)

--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog