| Message ID | 20240822025258.53263-2-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:53 AM > Subject: [PATCH 1/2] i2c: emev2: 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, remove > the goto label "err_clk:", and use its return value to return the error code. > > Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> > --- > drivers/i2c/busses/i2c-emev2.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c index > 557409410445..20efe0b0cb85 100644 > --- a/drivers/i2c/busses/i2c-emev2.c > +++ b/drivers/i2c/busses/i2c-emev2.c > @@ -373,14 +373,10 @@ static int em_i2c_probe(struct platform_device *pdev) > > strscpy(priv->adap.name, "EMEV2 I2C", sizeof(priv->adap.name)); > > - priv->sclk = devm_clk_get(&pdev->dev, "sclk"); > + priv->sclk = devm_clk_get_enabled(&pdev->dev, "sclk"); nit: After your change sclk can be local. You can drop sclk from struct em_i2c_device Maybe send another patch to fix this. Cheers, Biju > if (IS_ERR(priv->sclk)) > return PTR_ERR(priv->sclk); > > - ret = clk_prepare_enable(priv->sclk); > - if (ret) > - return ret; > - > priv->adap.timeout = msecs_to_jiffies(100); > priv->adap.retries = 5; > priv->adap.dev.parent = &pdev->dev; > @@ -397,26 +393,22 @@ static int em_i2c_probe(struct platform_device *pdev) > > ret = platform_get_irq(pdev, 0); > if (ret < 0) > - goto err_clk; > + return ret; > priv->irq = ret; > ret = devm_request_irq(&pdev->dev, priv->irq, em_i2c_irq_handler, 0, > "em_i2c", priv); > if (ret) > - goto err_clk; > + return ret; > > ret = i2c_add_adapter(&priv->adap); > > if (ret) > - goto err_clk; > + return ret; > > dev_info(&pdev->dev, "Added i2c controller %d, irq %d\n", priv->adap.nr, > priv->irq); > > return 0; > - > -err_clk: > - clk_disable_unprepare(priv->sclk); > - return ret; > } > > static void em_i2c_remove(struct platform_device *dev) @@ -424,7 +416,6 @@ static void > em_i2c_remove(struct platform_device *dev) > struct em_i2c_device *priv = platform_get_drvdata(dev); > > i2c_del_adapter(&priv->adap); > - clk_disable_unprepare(priv->sclk); > } > > static const struct of_device_id em_i2c_ids[] = { > -- > 2.39.0 >
在 2024/8/22 17:19, 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:53 AM >> Subject: [PATCH 1/2] i2c: emev2: 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, remove >> the goto label "err_clk:", and use its return value to return the error code. >> >> Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> >> --- >> drivers/i2c/busses/i2c-emev2.c | 17 ++++------------- >> 1 file changed, 4 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c index >> 557409410445..20efe0b0cb85 100644 >> --- a/drivers/i2c/busses/i2c-emev2.c >> +++ b/drivers/i2c/busses/i2c-emev2.c >> @@ -373,14 +373,10 @@ static int em_i2c_probe(struct platform_device *pdev) >> >> strscpy(priv->adap.name, "EMEV2 I2C", sizeof(priv->adap.name)); >> >> - priv->sclk = devm_clk_get(&pdev->dev, "sclk"); >> + priv->sclk = devm_clk_get_enabled(&pdev->dev, "sclk"); > nit: > After your change sclk can be local. You can drop sclk from struct em_i2c_device > Maybe send another patch to fix this. Thanks for taking the time to reply. I will try to do this in the next version. > > Cheers, > Biju > >> if (IS_ERR(priv->sclk)) >> return PTR_ERR(priv->sclk); >> >> - ret = clk_prepare_enable(priv->sclk); >> - if (ret) >> - return ret; >> - >> priv->adap.timeout = msecs_to_jiffies(100); >> priv->adap.retries = 5; >> priv->adap.dev.parent = &pdev->dev; >> @@ -397,26 +393,22 @@ static int em_i2c_probe(struct platform_device *pdev) >> >> ret = platform_get_irq(pdev, 0); >> if (ret < 0) >> - goto err_clk; >> + return ret; >> priv->irq = ret; >> ret = devm_request_irq(&pdev->dev, priv->irq, em_i2c_irq_handler, 0, >> "em_i2c", priv); >> if (ret) >> - goto err_clk; >> + return ret; >> >> ret = i2c_add_adapter(&priv->adap); >> >> if (ret) >> - goto err_clk; >> + return ret; >> >> dev_info(&pdev->dev, "Added i2c controller %d, irq %d\n", priv->adap.nr, >> priv->irq); >> >> return 0; >> - >> -err_clk: >> - clk_disable_unprepare(priv->sclk); >> - return ret; >> } >> >> static void em_i2c_remove(struct platform_device *dev) @@ -424,7 +416,6 @@ static void >> em_i2c_remove(struct platform_device *dev) >> struct em_i2c_device *priv = platform_get_drvdata(dev); >> >> i2c_del_adapter(&priv->adap); >> - clk_disable_unprepare(priv->sclk); >> } >> >> static const struct of_device_id em_i2c_ids[] = { >> -- >> 2.39.0 >>
diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c index 557409410445..20efe0b0cb85 100644 --- a/drivers/i2c/busses/i2c-emev2.c +++ b/drivers/i2c/busses/i2c-emev2.c @@ -373,14 +373,10 @@ static int em_i2c_probe(struct platform_device *pdev) strscpy(priv->adap.name, "EMEV2 I2C", sizeof(priv->adap.name)); - priv->sclk = devm_clk_get(&pdev->dev, "sclk"); + priv->sclk = devm_clk_get_enabled(&pdev->dev, "sclk"); if (IS_ERR(priv->sclk)) return PTR_ERR(priv->sclk); - ret = clk_prepare_enable(priv->sclk); - if (ret) - return ret; - priv->adap.timeout = msecs_to_jiffies(100); priv->adap.retries = 5; priv->adap.dev.parent = &pdev->dev; @@ -397,26 +393,22 @@ static int em_i2c_probe(struct platform_device *pdev) ret = platform_get_irq(pdev, 0); if (ret < 0) - goto err_clk; + return ret; priv->irq = ret; ret = devm_request_irq(&pdev->dev, priv->irq, em_i2c_irq_handler, 0, "em_i2c", priv); if (ret) - goto err_clk; + return ret; ret = i2c_add_adapter(&priv->adap); if (ret) - goto err_clk; + return ret; dev_info(&pdev->dev, "Added i2c controller %d, irq %d\n", priv->adap.nr, priv->irq); return 0; - -err_clk: - clk_disable_unprepare(priv->sclk); - return ret; } static void em_i2c_remove(struct platform_device *dev) @@ -424,7 +416,6 @@ static void em_i2c_remove(struct platform_device *dev) struct em_i2c_device *priv = platform_get_drvdata(dev); i2c_del_adapter(&priv->adap); - clk_disable_unprepare(priv->sclk); } static const struct of_device_id em_i2c_ids[] = {
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, remove the goto label "err_clk:", and use its return value to return the error code. Signed-off-by: Rong Qianfeng <rongqianfeng@vivo.com> --- drivers/i2c/busses/i2c-emev2.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)