Re: [cocci] [PATCH 56/61] clk: Prefer IS_ERR_OR_NULL over manual NULL check

Chen-Yu Tsai <[email protected]> Wed, 11 Mar 2026 11:07:21 +0900
Newsgroups fr.inria.cocci,dev.linux.lists.dm-devel,dev.linux.lists.gfs2,dev.linux.lists.iommu,dev.linux.lists.ntfs3,dev.linux.lists.sched-ext,dev.linux.lists.v9fs,org.freedesktop.lists.amd-gfx,org.freedesktop.lists.dri-devel,org.freedesktop.lists.intel-gfx,org.infradead.lists.linux-mtd,org.infradead.lists.linux-phy,org.infradead.lists.linux-rockchip,org.kernel.vger.bpf,org.kernel.vger.ceph-devel,org.kernel.vger.kvm,org.kernel.vger.linux-block,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-cifs,org.kernel.vger.linux-clk,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-gpio,org.kernel.vger.linux-hyperv,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel,org.kernel.vger.linux-leds,org.kernel.vger.linux-media,org.kernel.vger.linux-mips,org.kernel.vger.linux-modules,org.kernel.vger.linux-nfs,org.kernel.vger.linux-omap,org.kernel.vger.linux-pm,org.kernel.vger.linux-s390,org.kernel.vger.linux-scsi,org.kernel.vger.linux-sctp,org.kernel.vger.linux-security-module,org.kernel.vger.linux-sh,org.kernel.vger.linux-sound,org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-usb,org.kernel.vger.linux-wireless,org.kernel.vger.netdev,org.kernel.vger.target-devel,org.kvack.linux-mm,org.osuosl.intel-wired-lan,org.ozlabs.lists.linux-erofs
Message-ID <CAGXv+5FQAVaJjqhv+Xq-ysOc4SHQn2mCNTgCAp8XocmWBWGGoA@mail.gmail.com> 
On Tue, Mar 10, 2026 at 9:57 PM Philipp Hahn <[email protected]> wrote:
>
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Semantich change: Previously the code only printed the warning on error,
> but not when the pointer was NULL. Now the warning is printed in both
> cases!
>
> Change found with coccinelle.
>
> To: Michael Turquette <[email protected]>
> To: Stephen Boyd <[email protected]>
> To: Daniel Lezcano <[email protected]>
> To: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Philipp Hahn <[email protected]>
> ---
>  drivers/clk/clk.c               | 4 ++--
>  drivers/clocksource/timer-pxa.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 47093cda9df32223c1120c3710261296027c4cd3..35146e3869a7dd93741d10b7223d4488a9216ed1 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4558,7 +4558,7 @@ void clk_unregister(struct clk *clk)
>         unsigned long flags;
>         const struct clk_ops *ops;
>
> -       if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> +       if (WARN_ON_ONCE(IS_ERR_OR_NULL(clk)))
>                 return;
>
>         clk_debug_unregister(clk->core);
> @@ -4744,7 +4744,7 @@ void __clk_put(struct clk *clk)
>  {
>         struct module *owner;
>
> -       if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
> +       if (WARN_ON_ONCE(IS_ERR_OR_NULL(clk)))

clk_get_optional() returns NULL if the clk isn't present.

Drivers would just pass this to clk_put(). Your change here would cause
this pattern to emit a very big warning.

I don't think this change should be landed.


ChenYu

>                 return;
>
>         clk_prepare_lock();
> diff --git a/drivers/clocksource/timer-pxa.c b/drivers/clocksource/timer-pxa.c
> index 7ad0e5adb2ffac4125c34710fc67f4b45f30331d..f65fb0b7fc318b766227e5e7a4c0fb08ba11c8f9 100644
> --- a/drivers/clocksource/timer-pxa.c
> +++ b/drivers/clocksource/timer-pxa.c
> @@ -218,7 +218,7 @@ void __init pxa_timer_nodt_init(int irq, void __iomem *base)
>
>         timer_base = base;
>         clk = clk_get(NULL, "OSTIMER0");
> -       if (clk && !IS_ERR(clk)) {
> +       if (!IS_ERR_OR_NULL(clk)) {
>                 clk_prepare_enable(clk);
>                 pxa_timer_common_init(irq, clk_get_rate(clk));
>         } else {
>
> --
> 2.43.0
>
>