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

[PATCH v4] 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.

Assisted-by: LLM
Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
---
Changes in v4:
- Rebased to v7.3
- Drop verbose cover letter examples of transformations
- Match any voidfn instead of specifically only dev_err, pr_err, dev_warn, etc.
  to cover pr_crit, DRM_DEV_ERROR instances
- Simplify functional-final `ret = PTR_ERR(...); return ret;` to `return
  clk_register();` directly such as in clk-pic32.c and clk-sysctrl.c;
  the new rule names the functions literally, per v1, to keep the file
  pre-filter working
- Similarly simplify within-if-body `ret = PTR_ERR(...); return ret;`
- Drop a local left unread by the removed message when it is declared
  and set in the registration loop, as in owl-common.c and clk-rpmh.c
- Running script on whole tree produces a single positive:
  arch/arm/mach-omap1/clock_data.c, where the removed message is the
  only user of a local variable, resulting in an unused variable
  warning. Left this to fix up by hand to keep the script shorter, happy
  to add rules to handle it if preferred.
- microchip-isc-clk.c keeps braces on its "else if" arm which checkpatch
  flags; this is also left to fix up by hand for the same reason.
- Link to v3: https://patch.msgid.link/20260809-cocci-clk-register-v3-1-60d036e13370@gurudas.dev

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

Testing:
- "make coccicheck COCCI=<full/path/to/clk_register.cocci> MODE=patch SPFLAGS="--in-place"
- All four modes (report/context/patch/org) verified via "make
  coccicheck COCCI=<path> MODE=<mode> [M=<path>]" against the
  whole tree on v7.3.
---
 scripts/coccinelle/api/clk_register.cocci | 262 ++++++++++++++++++++++++++++++
 1 file changed, 262 insertions(+)

diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
new file mode 100644
index 000000000000..01d272e2d96f
--- /dev/null
+++ b/scripts/coccinelle/api/clk_register.cocci
@@ -0,0 +1,262 @@
+// 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;
+constant char[] c;
+statement S;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+*voidfn(...,c,...);
+...
+}
+else S
+
+@collapse depends on patch@
+expression list args;
+identifier x;
+identifier voidfn;
+constant char[] c;
+@@
+
+(
+-x = clk_register(args);
++return clk_register(args);
+|
+-x = devm_clk_register(args);
++return devm_clk_register(args);
+|
+-x = clk_hw_register(args);
++return clk_hw_register(args);
+|
+-x = devm_clk_hw_register(args);
++return devm_clk_hw_register(args);
+|
+-x = of_clk_hw_register(args);
++return of_clk_hw_register(args);
+)
+-if ( \( IS_ERR(x) \| x \| x < 0 \) )
+(
+-voidfn(...,c,...);
+|
+-{ voidfn(...,c,...); }
+)
+-return x;
+
+@depends on collapse && patch@
+identifier collapse.x;
+type T;
+@@
+
+- T x;
+  ... when != x
+      when strict
+
+@depends on patch@
+expression clk;
+identifier voidfn;
+constant char[] c;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+-if ( IS_ERR(clk) )
+-voidfn(...,c,...);
+
+@depends on patch@
+expression clk, ret;
+identifier voidfn;
+constant char[] c;
+statement S_else;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+-{
+-ret = PTR_ERR(clk);
+-voidfn(...,c,...);
+-return ret;
++return PTR_ERR(clk);
+-}
+else S_else
+
+@bind_loop depends on patch@
+expression x, E;
+identifier voidfn, n;
+constant char[] c;
+type T;
+statement S, S_else;
+@@
+
+  for (...;...;...) {
+  ... when != n
+- T n;
+  ...
+- n = E;
+  ...
+  x = \(clk_register\|devm_clk_register\|clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+  if ( \( IS_ERR(x) \| x \| x < 0 \) )
+(
+-{
+-voidfn(...,c,...,n,...);
+S
+-}
+|
+{
+...
+-voidfn(...,c,...,n,...);
+...
+}
+)
+  else S_else
+  ... when != n
+  }
+
+@depends on patch@
+expression clk;
+identifier voidfn;
+constant char[] c;
+statement S, S_else;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+(
+-{
+-voidfn(...,c,...);
+S
+-}
+|
+{
+...
+-voidfn(...,c,...);
+...
+}
+)
+else S_else
+
+@r1 depends on org || report@
+position p1;
+expression clk;
+identifier voidfn;
+constant char[] c;
+statement S;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+voidfn@p1(...,c,...);
+...
+}
+else S
+
+@depends on context@
+expression ret;
+identifier voidfn;
+constant char[] c;
+statement S;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+*voidfn(...,c,...);
+...
+}
+else S
+
+@depends on patch@
+expression ret;
+identifier voidfn;
+constant char[] c;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+-if ( \( ret \| ret < 0 \) )
+-voidfn(...,c,...);
+
+@depends on patch@
+expression ret;
+identifier voidfn;
+constant char[] c;
+statement S, S_else;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+(
+-{
+-voidfn(...,c,...);
+S
+-}
+|
+{
+...
+-voidfn(...,c,...);
+...
+}
+)
+else S_else
+
+@r2 depends on org || report@
+position p2;
+expression ret;
+identifier voidfn;
+constant char[] c;
+statement S;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+voidfn@p2(...,c,...);
+...
+}
+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: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20260802-cocci-clk-register-951d94251af4

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] coccinelle: clk_register: missing when != n constraints in bind_loop
--

commit 5a7f966954ec0e1e638d5df96a25b50558993fc6
Author: Guru Das Srinagesh <linux@gurudas.dev>

coccinelle: Detect clk_register() anti-pattern

This commit adds a Coccinelle semantic patch to detect and remove redundant
error messages logged after clk_register(), clk_hw_register(), and their
devm_/of_ variants fail.

> diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
> new file mode 100644
> index 0000000000000..01d272e2d96f7
> --- /dev/null
> +++ b/scripts/coccinelle/api/clk_register.cocci
> @@ -0,0 +1,262 @@

[ ... ]

> +@bind_loop depends on patch@
> +expression x, E;
> +identifier voidfn, n;
> +constant char[] c;
> +type T;
> +statement S, S_else;
> +@@
> +
> +  for (...;...;...) {
> +  ... when != n
> +- T n;
> +  ...
> +- n = E;
> +  ...
> +  x = \(clk_register\|devm_clk_register\|clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
> +  if ( \( IS_ERR(x) \| x \| x < 0 \) )
> +(
> +-{
> +-voidfn(...,c,...,n,...);
> +S
> +-}
> +|
> +{
> +...
> +-voidfn(...,c,...,n,...);
> +...
> +}
> +)
> +  else S_else
> +  ... when != n
> +  }

[Severity: Medium]
Can the bind_loop rule delete the declaration and assignment of n when n is
still referenced elsewhere in the loop?

In SmPL, when != n only constrains the specific ellipsis to which it is
attached. Here, when != n is only placed before T n; (before n is in scope)
and after else S_else. The intermediate ellipses around n = E; and
voidfn(...), the registration call arguments, and the statements S and
S_else are not constrained.

If a loop uses n between n = E; and the end of the if/else block (for
example, passing n to the registration call or using it in S or S_else),
won't running this script in patch mode still remove T n; and n = E;,
leaving behind an undeclared identifier?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018-cocci-clk-register-v4-1-ed611b1c0da6@gurudas.dev?part=1