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

[Julia Lawall <julia.lawall@inria.fr>]

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