| Message ID | 20240822140413.65369-4-rongqianfeng@vivo.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Use devm_clk_get_enabled() helpers | expand |
Hi Rong Qianfeng, Thanks for the patch > -----Original Message----- > From: Rong Qianfeng <rongqianfeng@vivo.com> > Sent: Thursday, August 22, 2024 3:04 PM > Subject: [PATCH v2 3/3] i2c: jz4780: Use devm_clk_get_enabled() helpers > > The devm_clk_get_enabled() helpers: > - call devm_clk_get() > - call clk_prepare_enable() and register what is needed in order to > call clk_disable_unprepare() when needed, as a managed resource. > > This simplifies the code and avoids the calls to clk_disable_unprepare(). > > While at it, no more special handling needed here, remove the goto label "err:". > > Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> > Acked-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/i2c/busses/i2c-jz4780.c | 21 ++++++--------------- > 1 file changed, 6 insertions(+), 15 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index > 4aafdfab6305..f5362c5dfb50 100644 > --- a/drivers/i2c/busses/i2c-jz4780.c > +++ b/drivers/i2c/busses/i2c-jz4780.c > @@ -792,26 +792,22 @@ static int jz4780_i2c_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, i2c); > > - i2c->clk = devm_clk_get(&pdev->dev, NULL); > + i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL); > if (IS_ERR(i2c->clk)) > return PTR_ERR(i2c->clk); > > - ret = clk_prepare_enable(i2c->clk); > - if (ret) > - return ret; > - > ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency", > &clk_freq); > if (ret) { > dev_err(&pdev->dev, "clock-frequency not specified in DT\n"); > - goto err; > + return ret; Nitpick: Leaving it to you. If needed, you could send another patch to use dev_err_probe if (ret) return dev_err_probe(&pdev->dev, ret, "clock-frequency not specified in DT\n"); > } > > i2c->speed = clk_freq / 1000; > if (i2c->speed == 0) { > ret = -EINVAL; > dev_err(&pdev->dev, "clock-frequency minimum is 1000\n"); return dev_err_probe(&pdev->dev, -EINVAL, "clock-frequency minimum is 1000\n "); Cheers, Biju > - goto err; > + return ret; > } > jz4780_i2c_set_speed(i2c); > > @@ -827,29 +823,24 @@ static int jz4780_i2c_probe(struct platform_device *pdev) > > ret = platform_get_irq(pdev, 0); > if (ret < 0) > - goto err; > + return ret; > i2c->irq = ret; > ret = devm_request_irq(&pdev->dev, i2c->irq, jz4780_i2c_irq, 0, > dev_name(&pdev->dev), i2c); > if (ret) > - goto err; > + return ret; > > ret = i2c_add_adapter(&i2c->adap); > if (ret < 0) > - goto err; > + return ret; > > return 0; > - > -err: > - clk_disable_unprepare(i2c->clk); > - return ret; > } > > static void jz4780_i2c_remove(struct platform_device *pdev) { > struct jz4780_i2c *i2c = platform_get_drvdata(pdev); > > - clk_disable_unprepare(i2c->clk); > i2c_del_adapter(&i2c->adap); > } > > -- > 2.39.0
在 2024/8/22 22:21, Biju Das 写道: > [Some people who received this message don't often get email from biju.das.jz@bp.renesas.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Hi Rong Qianfeng, > > Thanks for the patch > >> -----Original Message----- >> From: Rong Qianfeng <rongqianfeng@vivo.com> >> Sent: Thursday, August 22, 2024 3:04 PM >> Subject: [PATCH v2 3/3] i2c: jz4780: Use devm_clk_get_enabled() helpers >> >> The devm_clk_get_enabled() helpers: >> - call devm_clk_get() >> - call clk_prepare_enable() and register what is needed in order to >> call clk_disable_unprepare() when needed, as a managed resource. >> >> This simplifies the code and avoids the calls to clk_disable_unprepare(). >> >> While at it, no more special handling needed here, remove the goto label "err:". >> >> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> >> Acked-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/i2c/busses/i2c-jz4780.c | 21 ++++++--------------- >> 1 file changed, 6 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index >> 4aafdfab6305..f5362c5dfb50 100644 >> --- a/drivers/i2c/busses/i2c-jz4780.c >> +++ b/drivers/i2c/busses/i2c-jz4780.c >> @@ -792,26 +792,22 @@ static int jz4780_i2c_probe(struct platform_device *pdev) >> >> platform_set_drvdata(pdev, i2c); >> >> - i2c->clk = devm_clk_get(&pdev->dev, NULL); >> + i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL); >> if (IS_ERR(i2c->clk)) >> return PTR_ERR(i2c->clk); >> >> - ret = clk_prepare_enable(i2c->clk); >> - if (ret) >> - return ret; >> - >> ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency", >> &clk_freq); >> if (ret) { >> dev_err(&pdev->dev, "clock-frequency not specified in DT\n"); >> - goto err; >> + return ret; > Nitpick: > Leaving it to you. > If needed, you could send another patch to use dev_err_probe > > if (ret) > return dev_err_probe(&pdev->dev, ret, "clock-frequency not specified in DT\n"); Thanks for taking the time to review my patch! I will send another patch later. > >> } >> >> i2c->speed = clk_freq / 1000; >> if (i2c->speed == 0) { >> ret = -EINVAL; >> dev_err(&pdev->dev, "clock-frequency minimum is 1000\n"); > return dev_err_probe(&pdev->dev, -EINVAL, "clock-frequency minimum is 1000\n "); > > Cheers, > Biju > >> - goto err; >> + return ret; >> } >> jz4780_i2c_set_speed(i2c); >> >> @@ -827,29 +823,24 @@ static int jz4780_i2c_probe(struct platform_device *pdev) >> >> ret = platform_get_irq(pdev, 0); >> if (ret < 0) >> - goto err; >> + return ret; >> i2c->irq = ret; >> ret = devm_request_irq(&pdev->dev, i2c->irq, jz4780_i2c_irq, 0, >> dev_name(&pdev->dev), i2c); >> if (ret) >> - goto err; >> + return ret; >> >> ret = i2c_add_adapter(&i2c->adap); >> if (ret < 0) >> - goto err; >> + return ret; >> >> return 0; >> - >> -err: >> - clk_disable_unprepare(i2c->clk); >> - return ret; >> } >> >> static void jz4780_i2c_remove(struct platform_device *pdev) { >> struct jz4780_i2c *i2c = platform_get_drvdata(pdev); >> >> - clk_disable_unprepare(i2c->clk); >> i2c_del_adapter(&i2c->adap); >> } >> >> -- >> 2.39.0
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index 4aafdfab6305..f5362c5dfb50 100644 --- a/drivers/i2c/busses/i2c-jz4780.c +++ b/drivers/i2c/busses/i2c-jz4780.c @@ -792,26 +792,22 @@ static int jz4780_i2c_probe(struct platform_device *pdev) platform_set_drvdata(pdev, i2c); - i2c->clk = devm_clk_get(&pdev->dev, NULL); + i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL); if (IS_ERR(i2c->clk)) return PTR_ERR(i2c->clk); - ret = clk_prepare_enable(i2c->clk); - if (ret) - return ret; - ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &clk_freq); if (ret) { dev_err(&pdev->dev, "clock-frequency not specified in DT\n"); - goto err; + return ret; } i2c->speed = clk_freq / 1000; if (i2c->speed == 0) { ret = -EINVAL; dev_err(&pdev->dev, "clock-frequency minimum is 1000\n"); - goto err; + return ret; } jz4780_i2c_set_speed(i2c); @@ -827,29 +823,24 @@ static int jz4780_i2c_probe(struct platform_device *pdev) ret = platform_get_irq(pdev, 0); if (ret < 0) - goto err; + return ret; i2c->irq = ret; ret = devm_request_irq(&pdev->dev, i2c->irq, jz4780_i2c_irq, 0, dev_name(&pdev->dev), i2c); if (ret) - goto err; + return ret; ret = i2c_add_adapter(&i2c->adap); if (ret < 0) - goto err; + return ret; return 0; - -err: - clk_disable_unprepare(i2c->clk); - return ret; } static void jz4780_i2c_remove(struct platform_device *pdev) { struct jz4780_i2c *i2c = platform_get_drvdata(pdev); - clk_disable_unprepare(i2c->clk); i2c_del_adapter(&i2c->adap); }
The devm_clk_get_enabled() helpers: - call devm_clk_get() - call clk_prepare_enable() and register what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the calls to clk_disable_unprepare(). While at it, no more special handling needed here, remove the goto label "err:". Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> Acked-by: Paul Cercueil <paul@crapouillou.net> --- drivers/i2c/busses/i2c-jz4780.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-)