2022-05-19 17:47:31

by Guo Zhengkui

Subject: [PATCH] media: atmel: atmel-isc: remove unneeded IS_ERR()

There has already been error check in clk_unregister(), so remove error
check before calling clk_unregister().

Signed-off-by: Guo Zhengkui <[email protected]>
---
drivers/media/platform/atmel/atmel-isc-clk.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc-clk.c b/drivers/media/platform/atmel/atmel-isc-clk.c
index 2059fe376b00..9decd8fb03d5 100644
--- a/drivers/media/platform/atmel/atmel-isc-clk.c
+++ b/drivers/media/platform/atmel/atmel-isc-clk.c
@@ -304,8 +304,7 @@ void isc_clk_cleanup(struct isc_device *isc)
for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
struct isc_clk *isc_clk = &isc->isc_clks[i];

- if (!IS_ERR(isc_clk->clk))
- clk_unregister(isc_clk->clk);
+ clk_unregister(isc_clk->clk);
}
}
EXPORT_SYMBOL_GPL(isc_clk_cleanup);
--
2.20.1

2022-06-09 09:49:27

by Hans Verkuil

Subject: Re: [PATCH] media: atmel: atmel-isc: remove unneeded IS_ERR()

Hi Guo Zhengkui,

On 5/19/22 08:52, Guo Zhengkui wrote:
> There has already been error check in clk_unregister(), so remove error
> check before calling clk_unregister().
>
> Signed-off-by: Guo Zhengkui <[email protected]>
> ---
> drivers/media/platform/atmel/atmel-isc-clk.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isc-clk.c b/drivers/media/platform/atmel/atmel-isc-clk.c
> index 2059fe376b00..9decd8fb03d5 100644
> --- a/drivers/media/platform/atmel/atmel-isc-clk.c
> +++ b/drivers/media/platform/atmel/atmel-isc-clk.c
> @@ -304,8 +304,7 @@ void isc_clk_cleanup(struct isc_device *isc)
> for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
> struct isc_clk *isc_clk = &isc->isc_clks[i];
>
> - if (!IS_ERR(isc_clk->clk))
> - clk_unregister(isc_clk->clk);
> + clk_unregister(isc_clk->clk);

Actually, clk_unregister does this:

if (!clk || WARN_ON_ONCE(IS_ERR(clk)))
return;

So if IS_ERR(clk) is true, then a warning will be issued, and that's not what you want.

I'm rejecting this patch.

Regards,

Hans


> }
> }
> EXPORT_SYMBOL_GPL(isc_clk_cleanup);