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

Guru Das Srinagesh <[email protected]> Mon, 10 Aug 2026 00:13:03 -0700
Newsgroups org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> > ---
> 
> 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.