linux-next: Fixes tag needs some work in the clk tree

linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

[I am experimenting with checking the Fixes tags in commits in linux-next.
Please let me know if you think I am being too strict.]

Hi all,

Commit

  b488517b28a4 ("clk: socfpga: stratix10: fix naming convention for the fixed-clocks")

has a malformed Fixes tag:

  Fixes: 07afb8db7340 ("clk: socfpga: stratix10: add clock driver for
  Stratix10 platform")

It should not be split over 2 lines.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Boyd]

Quoting Stephen Rothwell (2019-01-15 13:34:40)
> [I am experimenting with checking the Fixes tags in commits in linux-next.
> Please let me know if you think I am being too strict.]
> 
> Hi all,
> 
> Commit
> 
>   b488517b28a4 ("clk: socfpga: stratix10: fix naming convention for the fixed-clocks")
> 
> has a malformed Fixes tag:
> 
>   Fixes: 07afb8db7340 ("clk: socfpga: stratix10: add clock driver for
>   Stratix10 platform")
> 
> It should not be split over 2 lines.

Ok sure. Does this matter to scripts? Is it documented in
Documentation/process/submitting-patches.rst? I've seen both types, one
line or many lines, so I'm happy to mandate a policy as long as it's
documented. Maybe also checkpatch can complain about this so that people
don't submit patches "the wrong way".

Re: linux-next: Fixes tag needs some work in the clk tree

[Fabio Estevam]

On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:

> Ok sure. Does this matter to scripts? Is it documented in
> Documentation/process/submitting-patches.rst? I've seen both types, one

From Documentation/process/submitting-patches.rst"

"If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary.  For example::

Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"

Re: linux-next: Fixes tag needs some work in the clk tree

[Fabio Estevam]

On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:

> Ok sure. Does this matter to scripts? Is it documented in
> Documentation/process/submitting-patches.rst? I've seen both types, one

>From Documentation/process/submitting-patches.rst"

"If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary.  For example::

Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Boyd]

Quoting Fabio Estevam (2019-01-16 17:25:21)
> On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:
> 
> > Ok sure. Does this matter to scripts? Is it documented in
> > Documentation/process/submitting-patches.rst? I've seen both types, one
> 
> From Documentation/process/submitting-patches.rst"
> 
> "If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> the SHA-1 ID, and the one line summary.  For example::
> 
> Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"

Yes but 'one line summary' is not clear to me. Does it mean the summary
line? Or the one line summary on one line regardless of commit message
column wrapping? Sorry to be pedantic but I can see how this can be
misunderstood.

Re: linux-next: Fixes tag needs some work in the clk tree

[Michael Ellerman]

Stephen Boyd <sboyd@kernel.org> writes:

> Quoting Fabio Estevam (2019-01-16 17:25:21)
>> On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:
>> 
>> > Ok sure. Does this matter to scripts? Is it documented in
>> > Documentation/process/submitting-patches.rst? I've seen both types, one
>> 
>> From Documentation/process/submitting-patches.rst"
>> 
>> "If your patch fixes a bug in a specific commit, e.g. you found an issue using
>> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>> the SHA-1 ID, and the one line summary.  For example::
>> 
>> Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"
>
> Yes but 'one line summary' is not clear to me. Does it mean the summary
> line? Or the one line summary on one line regardless of commit message
> column wrapping? Sorry to be pedantic but I can see how this can be
> misunderstood.

Yeah "one line summary" is referring to the subject of the commit.

So it doesn't explicitly say not to split it across lines, it probably
should.

eg: ?

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 30dc00a364e8..12a283ba3929 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -186,6 +186,8 @@ If your patch fixes a bug in a specific commit, e.g. you found an issue using
 
 	Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
 
+To ease parsing of the 'Fixes:' tag please don't split it across multiple lines.
+
 The following ``git config`` settings can be used to add a pretty format for
 outputting the above style in the ``git log`` or ``git show`` commands::
 


cheers

Re: linux-next: Fixes tag needs some work in the clk tree

[Fabio Estevam]

Hi Michael,

On Fri, Jan 18, 2019 at 10:41 AM Michael Ellerman <mpe@ellerman.id.au> wrote:

> Yeah "one line summary" is referring to the subject of the commit.
>
> So it doesn't explicitly say not to split it across lines, it probably
> should.
>
> eg: ?
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 30dc00a364e8..12a283ba3929 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -186,6 +186,8 @@ If your patch fixes a bug in a specific commit, e.g. you found an issue using
>
>         Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>
> +To ease parsing of the 'Fixes:' tag please don't split it across multiple lines.
> +
>  The following ``git config`` settings can be used to add a pretty format for
>  outputting the above style in the ``git log`` or ``git show`` commands::

Yes, I think this make it clearer. Thanks

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Boyd]

Quoting Michael Ellerman (2019-01-18 04:41:41)
> Stephen Boyd <sboyd@kernel.org> writes:
> 
> > Quoting Fabio Estevam (2019-01-16 17:25:21)
> >> On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >> 
> >> > Ok sure. Does this matter to scripts? Is it documented in
> >> > Documentation/process/submitting-patches.rst? I've seen both types, one
> >> 
> >> From Documentation/process/submitting-patches.rst"
> >> 
> >> "If your patch fixes a bug in a specific commit, e.g. you found an issue using
> >> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> >> the SHA-1 ID, and the one line summary.  For example::
> >> 
> >> Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"
> >
> > Yes but 'one line summary' is not clear to me. Does it mean the summary
> > line? Or the one line summary on one line regardless of commit message
> > column wrapping? Sorry to be pedantic but I can see how this can be
> > misunderstood.
> 
> Yeah "one line summary" is referring to the subject of the commit.
> 
> So it doesn't explicitly say not to split it across lines, it probably
> should.
> 
> eg: ?

Yes, thanks!

> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 30dc00a364e8..12a283ba3929 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -186,6 +186,8 @@ If your patch fixes a bug in a specific commit, e.g. you found an issue using
>  
>         Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>  
> +To ease parsing of the 'Fixes:' tag please don't split it across multiple lines.
> +
>  The following ``git config`` settings can be used to add a pretty format for
>  outputting the above style in the ``git log`` or ``git show`` commands::
>  
> 

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Do you have a patch for checkpatch too? Maybe Joe can solve my problem.

Re: linux-next: Fixes tag needs some work in the clk tree

[Nathan Chancellor]

On Fri, Jan 18, 2019 at 09:37:26AM -0800, Stephen Boyd wrote:
> Quoting Michael Ellerman (2019-01-18 04:41:41)
> > Stephen Boyd <sboyd@kernel.org> writes:
> > 
> > > Quoting Fabio Estevam (2019-01-16 17:25:21)
> > >> On Wed, Jan 16, 2019 at 10:44 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > >> 
> > >> > Ok sure. Does this matter to scripts? Is it documented in
> > >> > Documentation/process/submitting-patches.rst? I've seen both types, one
> > >> 
> > >> From Documentation/process/submitting-patches.rst"
> > >> 
> > >> "If your patch fixes a bug in a specific commit, e.g. you found an issue using
> > >> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> > >> the SHA-1 ID, and the one line summary.  For example::
> > >> 
> > >> Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")"
> > >
> > > Yes but 'one line summary' is not clear to me. Does it mean the summary
> > > line? Or the one line summary on one line regardless of commit message
> > > column wrapping? Sorry to be pedantic but I can see how this can be
> > > misunderstood.
> > 
> > Yeah "one line summary" is referring to the subject of the commit.
> > 
> > So it doesn't explicitly say not to split it across lines, it probably
> > should.
> > 
> > eg: ?
> 
> Yes, thanks!
> 
> > 
> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> > index 30dc00a364e8..12a283ba3929 100644
> > --- a/Documentation/process/submitting-patches.rst
> > +++ b/Documentation/process/submitting-patches.rst
> > @@ -186,6 +186,8 @@ If your patch fixes a bug in a specific commit, e.g. you found an issue using
> >  
> >         Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
> >  
> > +To ease parsing of the 'Fixes:' tag please don't split it across multiple lines.
> > +
> >  The following ``git config`` settings can be used to add a pretty format for
> >  outputting the above style in the ``git log`` or ``git show`` commands::
> >  
> > 
> 
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> 
> Do you have a patch for checkpatch too? Maybe Joe can solve my problem.
> 

Joe posted a checkpatch diff in another thread: https://lore.kernel.org/lkml/40bfc40958fca6e2cc9b86101153aa0715fac4f7.camel@perches.com/

Nathan

Re: linux-next: Fixes tag needs some work in the clk tree

[Joe Perches]

On Fri, 2019-01-18 at 10:41 -0700, Nathan Chancellor wrote:
> On Fri, Jan 18, 2019 at 09:37:26AM -0800, Stephen Boyd wrote:
> > Do you have a patch for checkpatch too? Maybe Joe can solve my problem.
> Joe posted a checkpatch diff in another thread:
> https://lore.kernel.org/lkml/40bfc40958fca6e2cc9b86101153aa0715fac4f7.camel@perches.com/

That proposal has the negative that a Fixes: line
is often after a Signed-off-by: line and the test
does not validate Fixes lines after Signed-off-by:s.

I would still appreciate some external testing of
the proposed patch.

Re: linux-next: Fixes tag needs some work in the clk tree

[Geert Uytterhoeven]

Hi Stephen,

On Sun, Dec 20, 2020 at 10:35 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> On Thu, 10 Dec 2020 08:52:41 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > trees can be pulled into linux-next? That would find this earlier.
> >
> > That sounds like a great idea, also for pinctrl.
> > Can you please add the following:
> >     git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> > renesas-clk
> >     git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> > renesas-pinctrl
> > ?
>
> Added from today.  Called clk-renesas and pinctrl-renesas respectively.

Thank you, the result looks good.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Hi Geert,

On Thu, 10 Dec 2020 08:52:41 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> > trees can be pulled into linux-next? That would find this earlier.  
> 
> That sounds like a great idea, also for pinctrl.
> Can you please add the following:
>     git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> renesas-clk
>     git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> renesas-pinctrl
> ?

Added from today.  Called clk-renesas and pinctrl-renesas respectively.

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
     * submitted under GPL v2 (or later) and include the Contributor's
        Signed-off-by,
     * posted to the relevant mailing list,
     * reviewed by you (or another maintainer of your subsystem tree),
     * successfully unit tested, and 
     * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Fixes tag needs some work in the clk tree

[Geert Uytterhoeven]

Hi Stephen²,

On Wed, Dec 9, 2020 at 6:29 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2020-12-08 00:37:00)
> > On Mon, Dec 7, 2020 at 11:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > In commit
> > >
> > >   c3f207f6d23d ("clk: renesas: r8a779a0: Make rcar_r8a779a0_cpg_clk_register() static")
> > >
> > > Fixes tag
> > >
> > >   Fixes: c07439dea94050b6 ("clk: renesas: cpg-mssr: Add support for R-Car V3U")
> > >
> > > has these problem(s):
> > >
> > >   - Target SHA1 does not exist
> >
> > Oops, my bad.
> >
> > > Maybe you meant
> > >
> > > Fixes: 17bcc8035d2d ("clk: renesas: cpg-mssr: Add support for R-Car V3U")
> >
> > Yes I did.
> >
> > Mike/Stephen: do you want me to respin my pull requests?
>
> Sure a respin is fine. I can fix it up in clk tree. Any chance your

Done, sorry for the mess.

> trees can be pulled into linux-next? That would find this earlier.

That sounds like a great idea, also for pinctrl.
Can you please add the following:
    git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
renesas-clk
    git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
renesas-pinctrl
?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Boyd]

Quoting Geert Uytterhoeven (2020-12-08 00:37:00)
> Hi Stephen,
> 
> On Mon, Dec 7, 2020 at 11:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > In commit
> >
> >   c3f207f6d23d ("clk: renesas: r8a779a0: Make rcar_r8a779a0_cpg_clk_register() static")
> >
> > Fixes tag
> >
> >   Fixes: c07439dea94050b6 ("clk: renesas: cpg-mssr: Add support for R-Car V3U")
> >
> > has these problem(s):
> >
> >   - Target SHA1 does not exist
> 
> Oops, my bad.
> 
> > Maybe you meant
> >
> > Fixes: 17bcc8035d2d ("clk: renesas: cpg-mssr: Add support for R-Car V3U")
> 
> Yes I did.
> 
> Mike/Stephen: do you want me to respin my pull requests?
> 

Sure a respin is fine. I can fix it up in clk tree. Any chance your
trees can be pulled into linux-next? That would find this earlier.

Re: linux-next: Fixes tag needs some work in the clk tree

[Geert Uytterhoeven]

Hi Stephen,

On Mon, Dec 7, 2020 at 11:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> In commit
>
>   c3f207f6d23d ("clk: renesas: r8a779a0: Make rcar_r8a779a0_cpg_clk_register() static")
>
> Fixes tag
>
>   Fixes: c07439dea94050b6 ("clk: renesas: cpg-mssr: Add support for R-Car V3U")
>
> has these problem(s):
>
>   - Target SHA1 does not exist

Oops, my bad.

> Maybe you meant
>
> Fixes: 17bcc8035d2d ("clk: renesas: cpg-mssr: Add support for R-Car V3U")

Yes I did.

Mike/Stephen: do you want me to respin my pull requests?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hi all,

In commit

  c3f207f6d23d ("clk: renesas: r8a779a0: Make rcar_r8a779a0_cpg_clk_register() static")

Fixes tag

  Fixes: c07439dea94050b6 ("clk: renesas: cpg-mssr: Add support for R-Car V3U")

has these problem(s):

  - Target SHA1 does not exist

Maybe you meant

Fixes: 17bcc8035d2d ("clk: renesas: cpg-mssr: Add support for R-Car V3U")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

Hi Stephen,

On Wed, 30 Sep 2020 17:22:06 -0700 Stephen Boyd <sboyd@kernel.org> wrote:
>
> Thanks for pointing that out. Is this part of checkpatch? I run
> checkpatch on most patches and didn't see any complaint.

This is not part of checkpatch, just some scripts I run after fetching
each tree each day.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Fixes tag needs some work in the clk tree

[Stephen Boyd]

Quoting Stephen Rothwell (2020-09-23 04:46:32)
> Hi all,
> 
> In commit
> 
>   b02cf0c4736c ("clk: socfpga: stratix10: fix the divider for the emac_ptp_free_clk")
> 
> Fixes tag
> 
>   Fixes: 07afb8db7340 ("clk: socfpga: stratix10: add clock driver for
> 
> has these problem(s):
> 
>   - Subject has leading but no trailing parentheses
>   - Subject has leading but no trailing quotes
> 
> Please do not split Fixes tags over more that one line.
> 

Thanks for pointing that out. Is this part of checkpatch? I run
checkpatch on most patches and didn't see any complaint.

linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

Hi all,

In commit

  b02cf0c4736c ("clk: socfpga: stratix10: fix the divider for the emac_ptp_free_clk")

Fixes tag

  Fixes: 07afb8db7340 ("clk: socfpga: stratix10: add clock driver for

has these problem(s):

  - Subject has leading but no trailing parentheses
  - Subject has leading but no trailing quotes

Please do not split Fixes tags over more that one line.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

Hi all,

In commit

  55c13463b652 ("clk: rockchip: fix mmc get phase")

Fixes tag

  Fixes: 2760878662a2 ("clk: Bail out when calculating phase fails during clk

has these problem(s):

  - Subject has leading but no trailing parentheses
  - Subject has leading but no trailing quotes

Please do not split Fixes tags over more than one line.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

linux-next: Fixes tag needs some work in the clk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

Hi all,

In commit

  b3fddd5b100e ("clk: imx: imx8mm: fix int pll clk gate")

Fixes tag

  Fixes: ba5625c3e27 ("clk: imx: Add clock driver support for imx8mm")

has these problem(s):

  - SHA1 should be at least 12 digits long
    Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
    or later) just making sure it is not set (or set to "auto").

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]