[PATCH v3] coccinelle: Detect clk_register() anti-pattern

[PATCH v3] coccinelle: Detect clk_register() anti-pattern

[Guru Das Srinagesh]

Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
log their own error on failure, so driver-side error prints after these
calls are redundant.

Two independent match families, one per return-value convention:
pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
and int return checked via a nonzero value (clk_hw_register()/
devm_clk_hw_register()/of_clk_hw_register()). Both families match
regardless of whether the redundant message's "if" also has a trailing
"else", via an "else S" clause with S otherwise unused.

In "patch" mode, removing the redundant message also collapses the
enclosing braces when only one statement remains, and deletes the whole
"if" when the message was already the only (braceless) statement.

Assisted-by: Claude:claude-sonnet-5 coccinelle
Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
---
Add a Coccinelle semantic patch enforcing commit 12a0fd23e870 ("clk:
Print an error when clk registration fails"): flags, and in "patch"
mode removes, driver-side error prints that are now redundant after
clk_register()/clk_hw_register() and their devm_/of_ variants.

Two independent match families, one per return-value convention.

Pointer return, IS_ERR()-checked (clk_register()/devm_clk_register()),
e.g. drivers/clk/clk-xgene.c:152-157:

    clk = clk_register(dev, &apmclk->hw);
    if (IS_ERR(clk)) {
-       pr_err("%s: could not register clk %s\n", __func__, name);
        kfree(apmclk);
        return NULL;
    }

Int return, nonzero-checked (clk_hw_register()/devm_clk_hw_register()/
of_clk_hw_register()), e.g. drivers/clk/meson/meson-clkc-utils.c:49-54:

    ret = devm_clk_hw_register(dev, hw);
-   if (ret) {
-           dev_err(dev, "registering %s clock failed\n",
-                   hw->init->name);
+   if (ret)
        return ret;
-   }

Already-braceless single-statement case: the whole "if" is deleted
instead of just the message, e.g. drivers/clk/ux500/clk-sysctrl.c:171-175:

    clk_reg = devm_clk_register(clk->dev, &clk->hw);
-   if (IS_ERR(clk_reg))
-           dev_err(dev, "clk_sysctrl: clk_register failed\n");

    return clk_reg;

Matches regardless of whether the "if" also has a trailing "else", via
an "else S" clause with S otherwise unused, e.g.
drivers/media/platform/microchip/microchip-isc-clk.c:269-275:

    isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
-   if (IS_ERR(isc_clk->clk)) {
-           dev_err(isc->dev, "%s: clock register fail\n", clk_name);
+   if (IS_ERR(isc_clk->clk))
        return PTR_ERR(isc_clk->clk);
-   } else if (id == ISC_MCK) {
+   else if (id == ISC_MCK) {
        of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
    }

Testing:
- Baseline: coccinelle 1.3.1, the Torvalds tree at v7.2-rc5.
- "make coccicheck COCCI=<path> MODE=report M=drivers/clk" produced 73
  hits, unchanged after this revision, and verified to have zero false
  positives.
- All four modes (report/context/patch/org) verified via "make
  coccicheck COCCI=<path> MODE=<mode> [M=<path>]" against the
  drivers/clk baseline and the new else-branch case above.
- "make coccicheck COCCI=<path> MODE=report" (whole tree, no M=) finds
  94 hits; the 21 outside drivers/clk are not part of this series.
---
Changes in v3 (Julia):
- Match an "if" regardless of a trailing "else" (else S, S unused),
  across context/patch/report/org rules for both families. Found via
  this to be a real, previously-invisible case in
  drivers/media/platform/microchip/microchip-isc-clk.c.
- Fix the org-mode script rules: cocci.print_main() takes (message,
  position), not just a position; the previous calls omitted the
  message entirely.
- Drop the MAINTAINERS addition from v2 per Julia's comment that a
  specific maintainer isn't needed for this file.
- Link to v2: https://patch.msgid.link/20260803-cocci-clk-register-v2-1-22e789f75f98@gurudas.dev

Changes in v2 (Julia):
- Use a literal function-name disjunction instead of a regex identifier,
  enabling spatch's file pre-filter optimization.
- In "patch" mode, drop braces when only one statement remains, and
  delete the whole "if" when the message was the only (braceless)
  statement.
- Drop two never-observed condition variants (IS_ERR(clk) == 1, ret !=
  0); keep the one with real precedent (ret < 0).
- Link to v1: https://patch.msgid.link/20260802-cocci-clk-register-v1-1-df68afcb1eef@gurudas.dev
---
 scripts/coccinelle/api/clk_register.cocci | 165 ++++++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)

diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
new file mode 100644
index 000000000000..150279827bf4
--- /dev/null
+++ b/scripts/coccinelle/api/clk_register.cocci
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Remove error messages after clk registration failures, because
+/// clk_register(), clk_hw_register(), and their variants already log
+/// an error when they fail. See commit 12a0fd23e870 ("clk: Print an
+/// error when clk registration fails").
+//
+// Confidence: Medium
+// Options: --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@depends on context@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+*voidfn(...);
+...
+}
+else S
+
+@depends on patch@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+-if ( IS_ERR(clk) )
+-voidfn(...);
+
+@depends on patch@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S, S_else;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+else S_else
+
+@r1 depends on org || report@
+position p1;
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+voidfn@p1(...);
+...
+}
+else S
+
+@depends on context@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+*voidfn(...);
+...
+}
+else S
+
+@depends on patch@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+-if ( \( ret \| ret < 0 \) )
+-voidfn(...);
+
+@depends on patch@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S, S_else;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+else S_else
+
+@r2 depends on org || report@
+position p2;
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+voidfn@p2(...);
+...
+}
+else S
+
+@script:python depends on org@
+p1 << r1.p1;
+@@
+
+msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
+cocci.print_main(msg, p1)
+
+@script:python depends on report@
+p1 << r1.p1;
+@@
+
+msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p2 << r2.p2;
+@@
+
+msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
+cocci.print_main(msg, p2)
+
+@script:python depends on report@
+p2 << r2.p2;
+@@
+
+msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
+coccilib.report.print_report(p2[0], msg)

---
base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
change-id: 20260802-cocci-clk-register-951d94251af4

Best regards,
--  
Guru Das Srinagesh <linux@gurudas.dev>

Re: [PATCH v3] coccinelle: Detect clk_register() anti-pattern

[Julia Lawall]

On Sun, 9 Aug 2026, Guru Das Srinagesh wrote:

> Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
> fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
> log their own error on failure, so driver-side error prints after these
> calls are redundant.
>
> Two independent match families, one per return-value convention:
> pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
> and int return checked via a nonzero value (clk_hw_register()/
> devm_clk_hw_register()/of_clk_hw_register()). Both families match
> regardless of whether the redundant message's "if" also has a trailing
> "else", via an "else S" clause with S otherwise unused.
>
> In "patch" mode, removing the redundant message also collapses the
> enclosing braces when only one statement remains, and deletes the whole
> "if" when the message was already the only (braceless) statement.
>
> Assisted-by: Claude:claude-sonnet-5 coccinelle
> Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
> ---

Hello,

Thanks for the update, which I will look into shortly.

But you are not structuring your patches in the right way.  The part above
the first --- is what will go into the git comit.  That looks fine.

But I don't understand what is the big pile of text below the ---.  When
there is a new version, I want to see what has changed as quickly as
possible.  So the changes in v3 should be the first thing that I see below
the ---, not the last thing.

For the following information, I'm not sure it should be here at all.
Either it is important, in which case it should be above the --- so people
in the future can easily benefit from it, or it is not important, in which
case it should be dropped.

julia


> Add a Coccinelle semantic patch enforcing commit 12a0fd23e870 ("clk:
> Print an error when clk registration fails"): flags, and in "patch"
> mode removes, driver-side error prints that are now redundant after
> clk_register()/clk_hw_register() and their devm_/of_ variants.
>
> Two independent match families, one per return-value convention.
>
> Pointer return, IS_ERR()-checked (clk_register()/devm_clk_register()),
> e.g. drivers/clk/clk-xgene.c:152-157:
>
>     clk = clk_register(dev, &apmclk->hw);
>     if (IS_ERR(clk)) {
> -       pr_err("%s: could not register clk %s\n", __func__, name);
>         kfree(apmclk);
>         return NULL;
>     }
>
> Int return, nonzero-checked (clk_hw_register()/devm_clk_hw_register()/
> of_clk_hw_register()), e.g. drivers/clk/meson/meson-clkc-utils.c:49-54:
>
>     ret = devm_clk_hw_register(dev, hw);
> -   if (ret) {
> -           dev_err(dev, "registering %s clock failed\n",
> -                   hw->init->name);
> +   if (ret)
>         return ret;
> -   }
>
> Already-braceless single-statement case: the whole "if" is deleted
> instead of just the message, e.g. drivers/clk/ux500/clk-sysctrl.c:171-175:
>
>     clk_reg = devm_clk_register(clk->dev, &clk->hw);
> -   if (IS_ERR(clk_reg))
> -           dev_err(dev, "clk_sysctrl: clk_register failed\n");
>
>     return clk_reg;
>
> Matches regardless of whether the "if" also has a trailing "else", via
> an "else S" clause with S otherwise unused, e.g.
> drivers/media/platform/microchip/microchip-isc-clk.c:269-275:
>
>     isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
> -   if (IS_ERR(isc_clk->clk)) {
> -           dev_err(isc->dev, "%s: clock register fail\n", clk_name);
> +   if (IS_ERR(isc_clk->clk))
>         return PTR_ERR(isc_clk->clk);
> -   } else if (id == ISC_MCK) {
> +   else if (id == ISC_MCK) {
>         of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>     }
>
> Testing:
> - Baseline: coccinelle 1.3.1, the Torvalds tree at v7.2-rc5.
> - "make coccicheck COCCI=<path> MODE=report M=drivers/clk" produced 73
>   hits, unchanged after this revision, and verified to have zero false
>   positives.
> - All four modes (report/context/patch/org) verified via "make
>   coccicheck COCCI=<path> MODE=<mode> [M=<path>]" against the
>   drivers/clk baseline and the new else-branch case above.
> - "make coccicheck COCCI=<path> MODE=report" (whole tree, no M=) finds
>   94 hits; the 21 outside drivers/clk are not part of this series.
> ---
> Changes in v3 (Julia):
> - Match an "if" regardless of a trailing "else" (else S, S unused),
>   across context/patch/report/org rules for both families. Found via
>   this to be a real, previously-invisible case in
>   drivers/media/platform/microchip/microchip-isc-clk.c.
> - Fix the org-mode script rules: cocci.print_main() takes (message,
>   position), not just a position; the previous calls omitted the
>   message entirely.
> - Drop the MAINTAINERS addition from v2 per Julia's comment that a
>   specific maintainer isn't needed for this file.
> - Link to v2: https://patch.msgid.link/20260803-cocci-clk-register-v2-1-22e789f75f98@gurudas.dev
>
> Changes in v2 (Julia):
> - Use a literal function-name disjunction instead of a regex identifier,
>   enabling spatch's file pre-filter optimization.
> - In "patch" mode, drop braces when only one statement remains, and
>   delete the whole "if" when the message was the only (braceless)
>   statement.
> - Drop two never-observed condition variants (IS_ERR(clk) == 1, ret !=
>   0); keep the one with real precedent (ret < 0).
> - Link to v1: https://patch.msgid.link/20260802-cocci-clk-register-v1-1-df68afcb1eef@gurudas.dev
> ---
>  scripts/coccinelle/api/clk_register.cocci | 165 ++++++++++++++++++++++++++++++
>  1 file changed, 165 insertions(+)
>
> diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
> new file mode 100644
> index 000000000000..150279827bf4
> --- /dev/null
> +++ b/scripts/coccinelle/api/clk_register.cocci
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Remove error messages after clk registration failures, because
> +/// clk_register(), clk_hw_register(), and their variants already log
> +/// an error when they fail. See commit 12a0fd23e870 ("clk: Print an
> +/// error when clk registration fails").
> +//
> +// Confidence: Medium
> +// Options: --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@depends on context@
> +expression clk;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S;
> +@@
> +
> +clk = \(clk_register\|devm_clk_register\)(...);
> +if ( IS_ERR(clk) )
> +{
> +...
> +*voidfn(...);
> +...
> +}
> +else S
> +
> +@depends on patch@
> +expression clk;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +@@
> +
> +clk = \(clk_register\|devm_clk_register\)(...);
> +-if ( IS_ERR(clk) )
> +-voidfn(...);
> +
> +@depends on patch@
> +expression clk;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S, S_else;
> +@@
> +
> +clk = \(clk_register\|devm_clk_register\)(...);
> +if ( IS_ERR(clk) )
> +(
> +-{
> +-voidfn(...);
> +S
> +-}
> +|
> +{
> +...
> +-voidfn(...);
> +...
> +}
> +)
> +else S_else
> +
> +@r1 depends on org || report@
> +position p1;
> +expression clk;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S;
> +@@
> +
> +clk = \(clk_register\|devm_clk_register\)(...);
> +if ( IS_ERR(clk) )
> +{
> +...
> +voidfn@p1(...);
> +...
> +}
> +else S
> +
> +@depends on context@
> +expression ret;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S;
> +@@
> +
> +ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
> +if ( \( ret \| ret < 0 \) )
> +{
> +...
> +*voidfn(...);
> +...
> +}
> +else S
> +
> +@depends on patch@
> +expression ret;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +@@
> +
> +ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
> +-if ( \( ret \| ret < 0 \) )
> +-voidfn(...);
> +
> +@depends on patch@
> +expression ret;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S, S_else;
> +@@
> +
> +ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
> +if ( \( ret \| ret < 0 \) )
> +(
> +-{
> +-voidfn(...);
> +S
> +-}
> +|
> +{
> +...
> +-voidfn(...);
> +...
> +}
> +)
> +else S_else
> +
> +@r2 depends on org || report@
> +position p2;
> +expression ret;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +statement S;
> +@@
> +
> +ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
> +if ( \( ret \| ret < 0 \) )
> +{
> +...
> +voidfn@p2(...);
> +...
> +}
> +else S
> +
> +@script:python depends on org@
> +p1 << r1.p1;
> +@@
> +
> +msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
> +cocci.print_main(msg, p1)
> +
> +@script:python depends on report@
> +p1 << r1.p1;
> +@@
> +
> +msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p2 << r2.p2;
> +@@
> +
> +msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
> +cocci.print_main(msg, p2)
> +
> +@script:python depends on report@
> +p2 << r2.p2;
> +@@
> +
> +msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
> +coccilib.report.print_report(p2[0], msg)
>
> ---
> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
> change-id: 20260802-cocci-clk-register-951d94251af4
>
> Best regards,
> --
> Guru Das Srinagesh <linux@gurudas.dev>
>
>

Re: [PATCH v3] coccinelle: Detect clk_register() anti-pattern

[Guru Das Srinagesh]

On Mon, Aug 10, 2026 at 08:53:35AM +0200, Julia Lawall wrote:
> 
> 
> On Sun, 9 Aug 2026, Guru Das Srinagesh wrote:
> 
> > Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
> > fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
> > log their own error on failure, so driver-side error prints after these
> > calls are redundant.
> >
> > Two independent match families, one per return-value convention:
> > pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
> > and int return checked via a nonzero value (clk_hw_register()/
> > devm_clk_hw_register()/of_clk_hw_register()). Both families match
> > regardless of whether the redundant message's "if" also has a trailing
> > "else", via an "else S" clause with S otherwise unused.
> >
> > In "patch" mode, removing the redundant message also collapses the
> > enclosing braces when only one statement remains, and deletes the whole
> > "if" when the message was already the only (braceless) statement.
> >
> > Assisted-by: Claude:claude-sonnet-5 coccinelle
> > Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
> > ---
> 
> Hello,
> 
> Thanks for the update, which I will look into shortly.

Thank you for reviewing.

> 
> But you are not structuring your patches in the right way.  The part above
> the first --- is what will go into the git comit.  That looks fine.
> 
> But I don't understand what is the big pile of text below the ---.  When

That is the cover letter for the patch.

I use the b4 tool [1] which is increasingly being used by kernel maintainers and
contributors alike to work with LKML.

When there is only one patch in a patchset/series, b4 puts the cover letter under the
first --- of the patch as described in [2]:

	When you only have a single patch, b4 should “mix-in” the contents of the cover
	letter into the “under-the-cut” portion of the patch itself, where it serves as a
	source of additional information for the reviewers, but never makes it into the
	actual commit.

[1]: https://b4.docs.kernel.org/en/latest/index.html 
[2]: https://b4.docs.kernel.org/en/latest/contributor/prep.html#what-if-the-series-only-has-a-single-patch

> there is a new version, I want to see what has changed as quickly as
> possible.  So the changes in v3 should be the first thing that I see below
> the ---, not the last thing.

Sure thing - I will make sure to put the changelog right under the --- as you
described for any future revisions.

> 
> For the following information, I'm not sure it should be here at all.
> Either it is important, in which case it should be above the --- so people
> in the future can easily benefit from it, or it is not important, in which
> case it should be dropped.
> 
> julia

For reviewers' convenience, the cover letter contains examples of the kind of
transformations that this script will create when used which I think would be too
verbose for inclusion in the commit message but useful for reviewing otherwise.

If you feel it is not required/helpful, I could remove everything but the changelog
from the cover letter for future revisions. Alternatively, I could put the changelog
right at the top so that it appears right under the first --- with the examples and
other stuff following that. Going with the former unless you say otherwise.

Thank you.

Guru Das.

Re: [PATCH v3] coccinelle: Detect clk_register() anti-pattern

[Julia Lawall]

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



On Mon, 10 Aug 2026, Guru Das Srinagesh wrote:

> On Mon, Aug 10, 2026 at 08:53:35AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 9 Aug 2026, Guru Das Srinagesh wrote:
> >
> > > Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
> > > fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
> > > log their own error on failure, so driver-side error prints after these
> > > calls are redundant.
> > >
> > > Two independent match families, one per return-value convention:
> > > pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
> > > and int return checked via a nonzero value (clk_hw_register()/
> > > devm_clk_hw_register()/of_clk_hw_register()). Both families match
> > > regardless of whether the redundant message's "if" also has a trailing
> > > "else", via an "else S" clause with S otherwise unused.
> > >
> > > In "patch" mode, removing the redundant message also collapses the
> > > enclosing braces when only one statement remains, and deletes the whole
> > > "if" when the message was already the only (braceless) statement.
> > >
> > > Assisted-by: Claude:claude-sonnet-5 coccinelle
> > > Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
> > > ---
> >
> > Hello,
> >
> > Thanks for the update, which I will look into shortly.
>
> Thank you for reviewing.
>
> >
> > But you are not structuring your patches in the right way.  The part above
> > the first --- is what will go into the git comit.  That looks fine.
> >
> > But I don't understand what is the big pile of text below the ---.  When
>
> That is the cover letter for the patch.
>
> I use the b4 tool [1] which is increasingly being used by kernel maintainers and
> contributors alike to work with LKML.

OK... I guess I have not been paying enough attention.

I think the conclusion would be that if the cover letter requires more
than say 2 lines of text there should be a real cover letter even if there
is only one patch?

But I would stilll think that the changes should come first.  That's what
the maintainer wants to see.

julia

>
> When there is only one patch in a patchset/series, b4 puts the cover letter under the
> first --- of the patch as described in [2]:
>
> 	When you only have a single patch, b4 should “mix-in” the contents of the cover
> 	letter into the “under-the-cut” portion of the patch itself, where it serves as a
> 	source of additional information for the reviewers, but never makes it into the
> 	actual commit.
>
> [1]: https://b4.docs.kernel.org/en/latest/index.html
> [2]: https://b4.docs.kernel.org/en/latest/contributor/prep.html#what-if-the-series-only-has-a-single-patch
>
> > there is a new version, I want to see what has changed as quickly as
> > possible.  So the changes in v3 should be the first thing that I see below
> > the ---, not the last thing.
>
> Sure thing - I will make sure to put the changelog right under the --- as you
> described for any future revisions.
>
> >
> > For the following information, I'm not sure it should be here at all.
> > Either it is important, in which case it should be above the --- so people
> > in the future can easily benefit from it, or it is not important, in which
> > case it should be dropped.
> >
> > julia
>
> For reviewers' convenience, the cover letter contains examples of the kind of
> transformations that this script will create when used which I think would be too
> verbose for inclusion in the commit message but useful for reviewing otherwise.
>
> If you feel it is not required/helpful, I could remove everything but the changelog
> from the cover letter for future revisions. Alternatively, I could put the changelog
> right at the top so that it appears right under the first --- with the examples and
> other stuff following that. Going with the former unless you say otherwise.
>
> Thank you.
>
> Guru Das.
>