[PATCH] drivers: clk: keystone: Fix parameter judgment in clk_prepare

ping.gao posted 1 patch 2 weeks, 2 days ago
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] drivers: clk: keystone: Fix parameter judgment in clk_prepare
Posted by ping.gao 2 weeks, 2 days ago
    The clk may return NULL or an ERR_PTR. Don't
    treat an ERR_PTR as valid.

    for example: biu_clk in dwmmc driver request fail, but it's ERR_PTR,
    not null,it will panic when call clk_prepare
    log is below:
    [  438.400868] [7:   binder:436_2: 4998] Unable to handle kernel paging request at virtual address fffffffffffffffe
    [  438.400877] [7:   binder:436_2: 4998] Mem abort info:
    [  438.400881] [7:   binder:436_2: 4998]   ESR = 0x0000000096000005
    [  438.400887] [7:   binder:436_2: 4998]   EC = 0x25: DABT (current EL), IL = 32 bits
    [  438.400894] [7:   binder:436_2: 4998]   SET = 0, FnV = 0
    [  438.400899] [7:   binder:436_2: 4998]   EA = 0, S1PTW = 0
    [  438.400904] [7:   binder:436_2: 4998]   FSC = 0x05: level 1 translation fault
    ...
    [  438.409424] [7:   binder:436_2: 4998] Call trace:
    [  438.409429] [7:   binder:436_2: 4998]  clk_prepare+0x10/0x24
    [  438.409439] [7:   binder:436_2: 4998]  dw_mci_runtime_resume+0x50/0x2d8 [dw_mmc_samsung cd210e210975263404c28fc89778f369f8398f0c]
    [  438.409471] [7:   binder:436_2: 4998]  dw_mci_exynos_runtime_resume+0x18/0x58 [dw_mmc_exynos_samsung 2735a594c7c9c9e8c65b0b87523fbf70dcaabfff]
    [  438.409496] [7:   binder:436_2: 4998]  pm_generic_runtime_resume+0x40/0x58
    [  438.409506] [7:   binder:436_2: 4998]  pm_runtime_force_resume+0x9c/0x134
    [  438.409517] [7:   binder:436_2: 4998]  platform_pm_resume+0x40/0x8c
    [  438.409529] [7:   binder:436_2: 4998]  dpm_run_callback+0x64/0x230
    [  438.409540] [7:   binder:436_2: 4998]  __device_resume+0x1d8/0x394
    [  438.409551] [7:   binder:436_2: 4998]  dpm_resume+0x110/0x2b8
    [  438.409561] [7:   binder:436_2: 4998]  dpm_resume_end+0x1c/0x38
    [  438.409570] [7:   binder:436_2: 4998]  suspend_devices_and_enter+0x828/0xab0
    [  438.409582] [7:   binder:436_2: 4998]  pm_suspend+0x334/0x618
    [  438.409592] [7:   binder:436_2: 4998]  state_store+0x104/0x144
    [  438.409601] [7:   binder:436_2: 4998]  kobj_attr_store+0x30/0x48
    [  438.409610] [7:   binder:436_2: 4998]  sysfs_kf_write+0x54/0x6c
    [  438.409619] [7:   binder:436_2: 4998]  kernfs_fop_write_iter+0x104/0x1a8
    [  438.409628] [7:   binder:436_2: 4998]  vfs_write+0x24c/0x2f4
    [  438.409640] [7:   binder:436_2: 4998]  ksys_write+0x78/0xe8
    [  438.409652] [7:   binder:436_2: 4998]  __arm64_sys_write+0x1c/0x2c
    [  438.409664] [7:   binder:436_2: 4998]  invoke_syscall+0x58/0x114
    [  438.409676] [7:   binder:436_2: 4998]  el0_svc_common+0xac/0xe0
    [  438.409687] [7:   binder:436_2: 4998]  do_el0_svc+0x1c/0x28
    [  438.409698] [7:   binder:436_2: 4998]  el0_svc+0x38/0x68
    [  438.409705] [7:   binder:436_2: 4998]  el0t_64_sync_handler+0x68/0xbc
    [  438.409712] [7:   binder:436_2: 4998]  el0t_64_sync+0x1a8/0x1ac

Signed-off-by: ping.gao <ping.gao@samsung.com>
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 85d2f2481acf..6d62f69323b5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1171,7 +1171,7 @@ static int clk_core_prepare_lock(struct clk_core *core)
  */
 int clk_prepare(struct clk *clk)
 {
-	if (!clk)
+	if (IS_ERR_OR_NULL(clk))
 		return 0;
 
 	return clk_core_prepare_lock(clk->core);
-- 
2.50.1
Re: [PATCH] drivers: clk: keystone: Fix parameter judgment in clk_prepare
Posted by Brian Masney 2 weeks, 1 day ago
Hi Ping,

Thanks for the patch!

On Thu, Jan 22, 2026 at 06:11:24PM +0800, ping.gao wrote:
>     The clk may return NULL or an ERR_PTR. Don't
>     treat an ERR_PTR as valid.
> 
>     for example: biu_clk in dwmmc driver request fail, but it's ERR_PTR,
>     not null,it will panic when call clk_prepare
>     log is below:

The patch subject has 'keystone:' that should be dropped. (Along with
the extra tab.)

This is also a nitpick but the extra spaces at the beginning of each
line in the commit message should be removed. Just start it at the
beginning of the line. When the patch is applied, 'git log' will
correctly indent your commit message properly for you.

>     [  438.400868] [7:   binder:436_2: 4998] Unable to handle kernel paging request at virtual address fffffffffffffffe
>     [  438.400877] [7:   binder:436_2: 4998] Mem abort info:
>     [  438.400881] [7:   binder:436_2: 4998]   ESR = 0x0000000096000005
>     [  438.400887] [7:   binder:436_2: 4998]   EC = 0x25: DABT (current EL), IL = 32 bits
>     [  438.400894] [7:   binder:436_2: 4998]   SET = 0, FnV = 0
>     [  438.400899] [7:   binder:436_2: 4998]   EA = 0, S1PTW = 0
>     [  438.400904] [7:   binder:436_2: 4998]   FSC = 0x05: level 1 translation fault
>     ...
>     [  438.409424] [7:   binder:436_2: 4998] Call trace:
>     [  438.409429] [7:   binder:436_2: 4998]  clk_prepare+0x10/0x24
>     [  438.409439] [7:   binder:436_2: 4998]  dw_mci_runtime_resume+0x50/0x2d8 [dw_mmc_samsung cd210e210975263404c28fc89778f369f8398f0c]
>     [  438.409471] [7:   binder:436_2: 4998]  dw_mci_exynos_runtime_resume+0x18/0x58 [dw_mmc_exynos_samsung 2735a594c7c9c9e8c65b0b87523fbf70dcaabfff]
>     [  438.409496] [7:   binder:436_2: 4998]  pm_generic_runtime_resume+0x40/0x58
>     [  438.409506] [7:   binder:436_2: 4998]  pm_runtime_force_resume+0x9c/0x134
>     [  438.409517] [7:   binder:436_2: 4998]  platform_pm_resume+0x40/0x8c
>     [  438.409529] [7:   binder:436_2: 4998]  dpm_run_callback+0x64/0x230
>     [  438.409540] [7:   binder:436_2: 4998]  __device_resume+0x1d8/0x394
>     [  438.409551] [7:   binder:436_2: 4998]  dpm_resume+0x110/0x2b8
>     [  438.409561] [7:   binder:436_2: 4998]  dpm_resume_end+0x1c/0x38
>     [  438.409570] [7:   binder:436_2: 4998]  suspend_devices_and_enter+0x828/0xab0
>     [  438.409582] [7:   binder:436_2: 4998]  pm_suspend+0x334/0x618
>     [  438.409592] [7:   binder:436_2: 4998]  state_store+0x104/0x144
>     [  438.409601] [7:   binder:436_2: 4998]  kobj_attr_store+0x30/0x48
>     [  438.409610] [7:   binder:436_2: 4998]  sysfs_kf_write+0x54/0x6c
>     [  438.409619] [7:   binder:436_2: 4998]  kernfs_fop_write_iter+0x104/0x1a8
>     [  438.409628] [7:   binder:436_2: 4998]  vfs_write+0x24c/0x2f4
>     [  438.409640] [7:   binder:436_2: 4998]  ksys_write+0x78/0xe8
>     [  438.409652] [7:   binder:436_2: 4998]  __arm64_sys_write+0x1c/0x2c
>     [  438.409664] [7:   binder:436_2: 4998]  invoke_syscall+0x58/0x114
>     [  438.409676] [7:   binder:436_2: 4998]  el0_svc_common+0xac/0xe0
>     [  438.409687] [7:   binder:436_2: 4998]  do_el0_svc+0x1c/0x28
>     [  438.409698] [7:   binder:436_2: 4998]  el0_svc+0x38/0x68
>     [  438.409705] [7:   binder:436_2: 4998]  el0t_64_sync_handler+0x68/0xbc
>     [  438.409712] [7:   binder:436_2: 4998]  el0t_64_sync+0x1a8/0x1ac
> 
> Signed-off-by: ping.gao <ping.gao@samsung.com>
> ---
>  drivers/clk/clk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 85d2f2481acf..6d62f69323b5 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1171,7 +1171,7 @@ static int clk_core_prepare_lock(struct clk_core *core)
>   */
>  int clk_prepare(struct clk *clk)
>  {
> -	if (!clk)
> +	if (IS_ERR_OR_NULL(clk))
>  		return 0;
>  
>  	return clk_core_prepare_lock(clk->core);

With a cleaned up commit message:

Reviewed-by: Brian Masney <bmasney@redhat.com>

Just to help Stephen: It looks like this is what ultimately causes the
problem:

dw_mci_probe() in drivers/mmc/host/dw_mmc.c sets up host->ciu_clk, and
if an error is returned by devm_clk_get(), it leaves the error pointer in
host->ciu_clk, and the probe continues normally. Later
dw_mci_runtime_resume() calls clk_prepare_enable() on host->ciu_clk with
the error pointer.

Additionally, clk_unprepare() already has this IS_ERR_OR_NULL() check.

Stephen: Do you think that clk_enable() should also be updated as well?
I see that clk_disable() also has the IS_ERR_OR_NULL() check.

Brian
Re: [PATCH] drivers: clk: keystone: Fix parameter judgment in clk_prepare
Posted by Stephen Boyd 2 weeks, 1 day ago
Quoting Brian Masney (2026-01-22 16:31:46)
> 
> Just to help Stephen: It looks like this is what ultimately causes the
> problem:
> 
> dw_mci_probe() in drivers/mmc/host/dw_mmc.c sets up host->ciu_clk, and
> if an error is returned by devm_clk_get(), it leaves the error pointer in
> host->ciu_clk, and the probe continues normally. Later
> dw_mci_runtime_resume() calls clk_prepare_enable() on host->ciu_clk with
> the error pointer.
> 
> Additionally, clk_unprepare() already has this IS_ERR_OR_NULL() check.
> 
> Stephen: Do you think that clk_enable() should also be updated as well?
> I see that clk_disable() also has the IS_ERR_OR_NULL() check.

No. We ignore error pointers in the disable and unprepare case to
simplify error paths, see commit  63589e92c2d9 ("clk: Ignore error and
NULL pointers passed to clk_{unprepare, disable}()"). With the usage of
devm though I'm not sure that error paths are a real thing anymore, but
it's nice to let drivers have one goto label instead of a million
different ones if they're not using devm.

Not ignoring error pointers in the enable and prepare path is
intentional. Fix the driver.