[PATCH v2] coccinelle: Detect clk_register() anti-pattern
Guru Das Srinagesh <[email protected]> Mon, 03 Aug 2026 23:20:57 -0700
| Newsgroups | org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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()).
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]>
---
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;
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 and verified to have zero false positives.
- "MODE=patch" verified separately on scratch copies of affected files
to confirm minimal, correct diffs.
checkpatch flagged that this new file needs a MAINTAINERS entry, and I'd
like to maintain it, so this adds a standalone entry rather than leaving
the file uncovered. There's no direct precedent for an individual .cocci
file getting its own entry - the only other named .cocci file in
MAINTAINERS, scripts/coccinelle/api/string_choices.cocci, was added to
the existing GENERIC STRING LIBRARY entry by that subsystem's
maintainer, not as a new one. Happy to fold this into COMMON CLK
FRAMEWORK or drop it entirely depending on what the maintainers prefer.
---
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/[email protected]
---
MAINTAINERS | 5 +
scripts/coccinelle/api/clk_register.cocci | 153 ++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 716acfc3d7c1..26788ccbf98c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6363,6 +6363,11 @@ L: [email protected]
S: Maintained
F: include/linux/clk.h
+CLK_REGISTER() COCCINELLE CHECK
+M: Guru Das Srinagesh <[email protected]>
+S: Maintained
+F: scripts/coccinelle/api/clk_register.cocci
+
CLOCKSOURCE, CLOCKEVENT DRIVERS
M: Daniel Lezcano <[email protected]>
M: Thomas Gleixner <[email protected]>
diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
new file mode 100644
index 000000000000..86a8be92f2a2
--- /dev/null
+++ b/scripts/coccinelle/api/clk_register.cocci
@@ -0,0 +1,153 @@
+// 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)$";
+@@
+
+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)$";
+@@
+
+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;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+
+@r1 depends on org || report@
+position p1;
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+voidfn@p1(...);
+...
+}
+
+@depends on context@
+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)$";
+@@
+
+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;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+
+@r2 depends on org || report@
+position p2;
+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@p2(...);
+...
+}
+
+@script:python depends on org@
+p1 << r1.p1;
+@@
+
+cocci.print_main(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;
+@@
+
+cocci.print_main(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 <[email protected]>