| Message ID | 20230104064136.1695102-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: imx8m: fix wrong return value check of of_clk_get_by_name() | expand |
Hi, Am Mittwoch, 4. Januar 2023, 07:41:36 CET schrieb Yang Yingliang: > of_clk_get_by_name() never returns NULL pointer, it will return > ERR_PTR() when it fails, so replace the check with IS_ERR(). > > Fixes: 836fb30949d9 ("soc: imx8m: Enable OCOTP clock before reading the > register") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/soc/imx/soc-imx8m.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c > index 28144c699b0c..32ed9dc88e45 100644 > --- a/drivers/soc/imx/soc-imx8m.c > +++ b/drivers/soc/imx/soc-imx8m.c > @@ -66,8 +66,8 @@ static u32 __init imx8mq_soc_revision(void) > ocotp_base = of_iomap(np, 0); > WARN_ON(!ocotp_base); > clk = of_clk_get_by_name(np, NULL); > - if (!clk) { > - WARN_ON(!clk); > + if (IS_ERR(clk)) { Nice catch. But wouldn't > if (WARN_ON(IS_ERR(clk))) be even better instead of doing the check twice? Best regards, Alexander > + WARN_ON(IS_ERR(clk)); > return 0; > }
On 2023/1/4 15:50, Alexander Stein wrote: > Hi, > > Am Mittwoch, 4. Januar 2023, 07:41:36 CET schrieb Yang Yingliang: >> of_clk_get_by_name() never returns NULL pointer, it will return >> ERR_PTR() when it fails, so replace the check with IS_ERR(). >> >> Fixes: 836fb30949d9 ("soc: imx8m: Enable OCOTP clock before reading the >> register") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/soc/imx/soc-imx8m.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c >> index 28144c699b0c..32ed9dc88e45 100644 >> --- a/drivers/soc/imx/soc-imx8m.c >> +++ b/drivers/soc/imx/soc-imx8m.c >> @@ -66,8 +66,8 @@ static u32 __init imx8mq_soc_revision(void) >> ocotp_base = of_iomap(np, 0); >> WARN_ON(!ocotp_base); >> clk = of_clk_get_by_name(np, NULL); >> - if (!clk) { >> - WARN_ON(!clk); >> + if (IS_ERR(clk)) { > Nice catch. But wouldn't >> if (WARN_ON(IS_ERR(clk))) > be even better instead of doing the check twice? Yes, it's better, I will send a new version. Thanks, Yang > > Best regards, > Alexander > >> + WARN_ON(IS_ERR(clk)); >> return 0; >> } > > > > .
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c index 28144c699b0c..32ed9dc88e45 100644 --- a/drivers/soc/imx/soc-imx8m.c +++ b/drivers/soc/imx/soc-imx8m.c @@ -66,8 +66,8 @@ static u32 __init imx8mq_soc_revision(void) ocotp_base = of_iomap(np, 0); WARN_ON(!ocotp_base); clk = of_clk_get_by_name(np, NULL); - if (!clk) { - WARN_ON(!clk); + if (IS_ERR(clk)) { + WARN_ON(IS_ERR(clk)); return 0; }
of_clk_get_by_name() never returns NULL pointer, it will return ERR_PTR() when it fails, so replace the check with IS_ERR(). Fixes: 836fb30949d9 ("soc: imx8m: Enable OCOTP clock before reading the register") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/soc/imx/soc-imx8m.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)