[PATCH] hwmon: Make use of devm_clk_get_enabled()

[PATCH] hwmon: Make use of devm_clk_get_enabled()

[Uwe Kleine-König]

Several drivers manually register a devm handler to disable their clk.
Convert them to devm_clk_get_enabled().

Acked-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

previously this patch was an example to show the benefits of adding
devm_clk_get_enabled() and its variants. Last submission was on 14 Mar
2022
(https://lore.kernel.org/all/20220314141643.22184-5-u.kleine-koenig@pengutronix.de).

This patch obviously depends on

	7ef9651e9792 clk: Provide new devm_clk helpers for prepared and enabled clocks

and to actually work needs

	8b3d743fc9e2 clk: Fix pointer casting to prevent oops in devm_clk_release()

which are both in Linus' tree now.

Best regards
Uwe

 drivers/hwmon/axi-fan-control.c | 15 +--------------
 drivers/hwmon/ltc2947-core.c    | 17 +----------------
 drivers/hwmon/mr75203.c         | 26 +-------------------------
 drivers/hwmon/sparx5-temp.c     | 19 +------------------
 4 files changed, 4 insertions(+), 73 deletions(-)

diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
index 96c4a5c45291..6724e0dd3088 100644
--- a/drivers/hwmon/axi-fan-control.c
+++ b/drivers/hwmon/axi-fan-control.c
@@ -394,11 +394,6 @@ static int axi_fan_control_init(struct axi_fan_control_data *ctl,
 	return ret;
 }
 
-static void axi_fan_control_clk_disable(void *clk)
-{
-	clk_disable_unprepare(clk);
-}
-
 static const struct hwmon_channel_info *axi_fan_control_info[] = {
 	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT),
 	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_FAULT | HWMON_F_LABEL),
@@ -478,20 +473,12 @@ static int axi_fan_control_probe(struct platform_device *pdev)
 	if (IS_ERR(ctl->base))
 		return PTR_ERR(ctl->base);
 
-	clk = devm_clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "clk_get failed with %ld\n", PTR_ERR(clk));
 		return PTR_ERR(clk);
 	}
 
-	ret = clk_prepare_enable(clk);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(&pdev->dev, axi_fan_control_clk_disable, clk);
-	if (ret)
-		return ret;
-
 	ctl->clk_rate = clk_get_rate(clk);
 	if (!ctl->clk_rate)
 		return -EINVAL;
diff --git a/drivers/hwmon/ltc2947-core.c b/drivers/hwmon/ltc2947-core.c
index 5423466de697..626f5bf2c9c7 100644
--- a/drivers/hwmon/ltc2947-core.c
+++ b/drivers/hwmon/ltc2947-core.c
@@ -956,13 +956,6 @@ static struct attribute *ltc2947_attrs[] = {
 };
 ATTRIBUTE_GROUPS(ltc2947);
 
-static void ltc2947_clk_disable(void *data)
-{
-	struct clk *extclk = data;
-
-	clk_disable_unprepare(extclk);
-}
-
 static int ltc2947_setup(struct ltc2947_data *st)
 {
 	int ret;
@@ -989,7 +982,7 @@ static int ltc2947_setup(struct ltc2947_data *st)
 		return ret;
 
 	/* check external clock presence */
-	extclk = devm_clk_get_optional(st->dev, NULL);
+	extclk = devm_clk_get_optional_enabled(st->dev, NULL);
 	if (IS_ERR(extclk))
 		return dev_err_probe(st->dev, PTR_ERR(extclk),
 				     "Failed to get external clock\n");
@@ -1007,14 +1000,6 @@ static int ltc2947_setup(struct ltc2947_data *st)
 			return -EINVAL;
 		}
 
-		ret = clk_prepare_enable(extclk);
-		if (ret)
-			return ret;
-
-		ret = devm_add_action_or_reset(st->dev, ltc2947_clk_disable,
-					       extclk);
-		if (ret)
-			return ret;
 		/* as in table 1 of the datasheet */
 		if (rate_hz >= LTC2947_CLK_MIN && rate_hz <= 1000000)
 			pre = 0;
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 26278b0f17a9..cf9467d03531 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -451,24 +451,6 @@ static int pvt_get_regmap(struct platform_device *pdev, char *reg_name,
 	return 0;
 }
 
-static void pvt_clk_disable(void *data)
-{
-	struct pvt_device *pvt = data;
-
-	clk_disable_unprepare(pvt->clk);
-}
-
-static int pvt_clk_enable(struct device *dev, struct pvt_device *pvt)
-{
-	int ret;
-
-	ret = clk_prepare_enable(pvt->clk);
-	if (ret)
-		return ret;
-
-	return devm_add_action_or_reset(dev, pvt_clk_disable, pvt);
-}
-
 static void pvt_reset_control_assert(void *data)
 {
 	struct pvt_device *pvt = data;
@@ -505,16 +487,10 @@ static int mr75203_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pvt->clk = devm_clk_get(dev, NULL);
+	pvt->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(pvt->clk))
 		return dev_err_probe(dev, PTR_ERR(pvt->clk), "failed to get clock\n");
 
-	ret = pvt_clk_enable(dev, pvt);
-	if (ret) {
-		dev_err(dev, "failed to enable clock\n");
-		return ret;
-	}
-
 	pvt->rst = devm_reset_control_get_exclusive(dev, NULL);
 	if (IS_ERR(pvt->rst))
 		return dev_err_probe(dev, PTR_ERR(pvt->rst),
diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
index 98be48e3a22a..04fd8505e5d6 100644
--- a/drivers/hwmon/sparx5-temp.c
+++ b/drivers/hwmon/sparx5-temp.c
@@ -26,13 +26,6 @@ struct s5_hwmon {
 	struct clk *clk;
 };
 
-static void s5_temp_clk_disable(void *data)
-{
-	struct clk *clk = data;
-
-	clk_disable_unprepare(clk);
-}
-
 static void s5_temp_enable(struct s5_hwmon *hwmon)
 {
 	u32 val = readl(hwmon->base + TEMP_CFG);
@@ -113,7 +106,6 @@ static int s5_temp_probe(struct platform_device *pdev)
 {
 	struct device *hwmon_dev;
 	struct s5_hwmon *hwmon;
-	int ret;
 
 	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
 	if (!hwmon)
@@ -123,19 +115,10 @@ static int s5_temp_probe(struct platform_device *pdev)
 	if (IS_ERR(hwmon->base))
 		return PTR_ERR(hwmon->base);
 
-	hwmon->clk = devm_clk_get(&pdev->dev, NULL);
+	hwmon->clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(hwmon->clk))
 		return PTR_ERR(hwmon->clk);
 
-	ret = clk_prepare_enable(hwmon->clk);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(&pdev->dev, s5_temp_clk_disable,
-				       hwmon->clk);
-	if (ret)
-		return ret;
-
 	s5_temp_enable(hwmon);
 
 	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,

base-commit: 8b3d743fc9e2542822826890b482afabf0e7522a
-- 
2.36.1

Re: [PATCH] hwmon: Make use of devm_clk_get_enabled()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Mon, Aug 08, 2022 at 08:06:40AM +0200, Uwe Kleine-König wrote:
> Several drivers manually register a devm handler to disable their clk.
> Convert them to devm_clk_get_enabled().
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Oh, the tags by Nuno Sá and Jonathan Cameron are a fake. I picked them
by mistake from the (similar) patch for iio. Please take care about that
before application. (i.e. drop them, or make me resend the patch without
the tags, or make the two give the tags post-factum :-)

Best regards and sorry for the chaos,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] hwmon: Make use of devm_clk_get_enabled()

[Jonathan Cameron]

On Mon, 8 Aug 2022 09:54:08 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> On Mon, Aug 08, 2022 at 08:06:40AM +0200, Uwe Kleine-König wrote:
> > Several drivers manually register a devm handler to disable their clk.
> > Convert them to devm_clk_get_enabled().
> > 
> > Acked-by: Guenter Roeck <linux@roeck-us.net>
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Oh, the tags by Nuno Sá and Jonathan Cameron are a fake. I picked them
> by mistake from the (similar) patch for iio. Please take care about that
> before application. (i.e. drop them, or make me resend the patch without
> the tags, or make the two give the tags post-factum :-)
> 
> Best regards and sorry for the chaos,
> Uwe
> 

Much like the IIO one you may get a request to split it by driver.
I personally prefer per driver patches as Andy suggested, but meh, it is
up to the hmwon maintainer.

I'm feeling particularly unhelpful today. As it's not IIO patches going
through another tree (like the IIO one was when I gave that tag):
+ simple patch to quickly review.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

*evil laugh*

Re: [PATCH] hwmon: Make use of devm_clk_get_enabled()

[Guenter Roeck]

On 8/8/22 07:30, Jonathan Cameron wrote:
> On Mon, 8 Aug 2022 09:54:08 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
>> On Mon, Aug 08, 2022 at 08:06:40AM +0200, Uwe Kleine-König wrote:
>>> Several drivers manually register a devm handler to disable their clk.
>>> Convert them to devm_clk_get_enabled().
>>>
>>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>>> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>>> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>
>> Oh, the tags by Nuno Sá and Jonathan Cameron are a fake. I picked them
>> by mistake from the (similar) patch for iio. Please take care about that
>> before application. (i.e. drop them, or make me resend the patch without
>> the tags, or make the two give the tags post-factum :-)
>>
>> Best regards and sorry for the chaos,
>> Uwe
>>
> 
> Much like the IIO one you may get a request to split it by driver.
> I personally prefer per driver patches as Andy suggested, but meh, it is
> up to the hmwon maintainer.
> 

I don't like those one-patch-for-dozens-of-drivers approach very much
that seems to proliferate recently, and very much prefer per-driver patches.
Which is one reason for letting this hang ...

Guenter

> I'm feeling particularly unhelpful today. As it's not IIO patches going
> through another tree (like the IIO one was when I gave that tag):
> + simple patch to quickly review.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> *evil laugh*
> 

Re: [PATCH] hwmon: Make use of devm_clk_get_enabled()

[Guenter Roeck]

On Mon, Aug 08, 2022 at 08:06:40AM +0200, Uwe Kleine-König wrote:
> Several drivers manually register a devm handler to disable their clk.
> Convert them to devm_clk_get_enabled().
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

[Finally] applied.

Thanks,
Guenter