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

Julia Lawall <[email protected]>
Newsgroups fr.inria.cocci,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

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 <[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.

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.
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.