linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] clk: provide new devm helpers for prepared and enabled clocks
@ 2021-03-30 18:17 Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 1/6] clk: generalize devm_clk_get() a bit Uwe Kleine-König
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2021-03-30 18:17 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, kernel, Claudiu Beznea, Thierry Reding, Lee Jones,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-pwm,
	linux-arm-kernel, Alessandro Zummo, linux-rtc, Mark Brown,
	linux-spi

Hello,

this series contains new helpers for devm managed clocks. Since v3 this
is a series that also contains four example conversions to show the
simplification that can be achieved. It was not hard to find these
candidates, there are drivers all over that can benefit.

The idea to provide these helpers is already quite old, I sent v1 back
in October and unfortunately didn't receive any feedback from the clk
maintainers yet on any of the patch series. It would be great if this
series is considered obviously good enough to consider it.

Best regards
Uwe

Uwe Kleine-König (6):
  clk: generalize devm_clk_get() a bit
  clk: Provide new devm_clk_helpers for prepared and enabled clocks
  pwm: atmel: Simplify using devm_clk_get_prepared()
  rtc: at91sma9: Simplify using devm_clk_get_enabled()
  i2c: imx: Simplify using devm_clk_get_enableded()
  spi: davinci: Simplify using devm_clk_get_enabled()

 drivers/clk/clk-devres.c     | 96 ++++++++++++++++++++++++++++++------
 drivers/i2c/busses/i2c-imx.c | 11 +----
 drivers/pwm/pwm-atmel.c      | 15 +-----
 drivers/rtc/rtc-at91sam9.c   | 22 ++-------
 drivers/spi/spi-davinci.c    | 11 +----
 include/linux/clk.h          | 87 +++++++++++++++++++++++++++++++-
 6 files changed, 176 insertions(+), 66 deletions(-)

base-commit: a38fd8748464831584a19438cbb3082b5a2dab15
-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/6] clk: generalize devm_clk_get() a bit
  2021-03-30 18:17 [PATCH v4 0/6] clk: provide new devm helpers for prepared and enabled clocks Uwe Kleine-König
@ 2021-03-30 18:17 ` Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 2/6] clk: Provide new devm_clk_helpers for prepared and enabled clocks Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled() Uwe Kleine-König
  2 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2021-03-30 18:17 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, kernel, Claudiu Beznea, Thierry Reding, Lee Jones,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-pwm,
	linux-arm-kernel, Alessandro Zummo, linux-rtc, Mark Brown,
	linux-spi

Allow to add an exit hook to devm managed clocks. Also use
clk_get_optional() in devm_clk_get_optional instead of open coding it.
The generalisation will be used in the next commit to add some more
devm_clk helpers.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/clk/clk-devres.c | 67 ++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index be160764911b..91c995815b57 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -4,39 +4,72 @@
 #include <linux/export.h>
 #include <linux/gfp.h>
 
+struct devm_clk_state {
+	struct clk *clk;
+	void (*exit)(struct clk *clk);
+};
+
 static void devm_clk_release(struct device *dev, void *res)
 {
-	clk_put(*(struct clk **)res);
+	struct devm_clk_state *state = *(struct devm_clk_state **)res;
+
+	if (state->exit)
+		state->exit(state->clk);
+
+	clk_put(state->clk);
 }
 
-struct clk *devm_clk_get(struct device *dev, const char *id)
+static struct clk *__devm_clk_get(struct device *dev, const char *id,
+				  struct clk *(*get)(struct device *dev, const char *id),
+				  int (*init)(struct clk *clk),
+				  void (*exit)(struct clk *clk))
 {
-	struct clk **ptr, *clk;
+	struct devm_clk_state *state;
+	struct clk *clk;
+	int ret;
 
-	ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
+	state = devres_alloc(devm_clk_release, sizeof(*state), GFP_KERNEL);
+	if (!state)
 		return ERR_PTR(-ENOMEM);
 
-	clk = clk_get(dev, id);
-	if (!IS_ERR(clk)) {
-		*ptr = clk;
-		devres_add(dev, ptr);
-	} else {
-		devres_free(ptr);
+	clk = get(dev, id);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto err_clk_get;
 	}
 
+	if (init) {
+		ret = init(clk);
+		if (ret)
+			goto err_clk_init;
+	}
+
+	state->clk = clk;
+	state->exit = exit;
+
+	devres_add(dev, state);
+
 	return clk;
+
+err_clk_init:
+
+	clk_put(clk);
+err_clk_get:
+
+	devres_free(state);
+	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL(devm_clk_get);
 
-struct clk *devm_clk_get_optional(struct device *dev, const char *id)
+struct clk *devm_clk_get(struct device *dev, const char *id)
 {
-	struct clk *clk = devm_clk_get(dev, id);
+	return __devm_clk_get(dev, id, clk_get, NULL, NULL);
 
-	if (clk == ERR_PTR(-ENOENT))
-		return NULL;
+}
+EXPORT_SYMBOL(devm_clk_get);
 
-	return clk;
+struct clk *devm_clk_get_optional(struct device *dev, const char *id)
+{
+	return __devm_clk_get(dev, id, clk_get_optional, NULL, NULL);
 }
 EXPORT_SYMBOL(devm_clk_get_optional);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/6] clk: Provide new devm_clk_helpers for prepared and enabled clocks
  2021-03-30 18:17 [PATCH v4 0/6] clk: provide new devm helpers for prepared and enabled clocks Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 1/6] clk: generalize devm_clk_get() a bit Uwe Kleine-König
@ 2021-03-30 18:17 ` Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled() Uwe Kleine-König
  2 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2021-03-30 18:17 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, kernel, Claudiu Beznea, Thierry Reding, Lee Jones,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-pwm,
	linux-arm-kernel, Alessandro Zummo, linux-rtc, Mark Brown,
	linux-spi

When a driver keeps a clock prepared (or enabled) during the whole
lifetime of the driver, these helpers allow to simplify the drivers.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/clk/clk-devres.c | 31 ++++++++++++++
 include/linux/clk.h      | 87 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 91c995815b57..b54f7f0f2a35 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -67,12 +67,43 @@ struct clk *devm_clk_get(struct device *dev, const char *id)
 }
 EXPORT_SYMBOL(devm_clk_get);
 
+struct clk *devm_clk_get_prepared(struct device *dev, const char *id)
+{
+	return __devm_clk_get(dev, id, clk_get, clk_prepare, clk_unprepare);
+
+}
+EXPORT_SYMBOL(devm_clk_get_prepared);
+
+struct clk *devm_clk_get_enabled(struct device *dev, const char *id)
+{
+	return __devm_clk_get(dev, id, clk_get,
+			      clk_prepare_enable, clk_disable_unprepare);
+
+}
+EXPORT_SYMBOL(devm_clk_get_enabled);
+
 struct clk *devm_clk_get_optional(struct device *dev, const char *id)
 {
 	return __devm_clk_get(dev, id, clk_get_optional, NULL, NULL);
 }
 EXPORT_SYMBOL(devm_clk_get_optional);
 
+struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *id)
+{
+	return __devm_clk_get(dev, id, clk_get_optional,
+			      clk_prepare, clk_unprepare);
+
+}
+EXPORT_SYMBOL(devm_clk_get_optional_prepared);
+
+struct clk *devm_clk_get_optional_enabled(struct device *dev, const char *id)
+{
+	return __devm_clk_get(dev, id, clk_get_optional,
+			      clk_prepare_enable, clk_disable_unprepare);
+
+}
+EXPORT_SYMBOL(devm_clk_get_optional_enabled);
+
 struct clk_bulk_devres {
 	struct clk_bulk_data *clks;
 	int num_clks;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 266e8de3cb51..b3c5da388b08 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -449,7 +449,7 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
  * the clock producer.  (IOW, @id may be identical strings, but
  * clk_get may return different clock producers depending on @dev.)
  *
- * Drivers must assume that the clock source is not enabled.
+ * Drivers must assume that the clock source is neither prepared nor enabled.
  *
  * devm_clk_get should not be called from within interrupt context.
  *
@@ -458,6 +458,47 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
  */
 struct clk *devm_clk_get(struct device *dev, const char *id);
 
+/**
+ * devm_clk_get_prepared - devm_clk_get() + clk_prepare()
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Returns a struct clk corresponding to the clock producer, or
+ * valid IS_ERR() condition containing errno.  The implementation
+ * uses @dev and @id to determine the clock consumer, and thereby
+ * the clock producer.  (IOW, @id may be identical strings, but
+ * clk_get may return different clock producers depending on @dev.)
+ *
+ * The returned clk (if valid) is prepared. Drivers must however assume that the
+ * clock is not enabled.
+ *
+ * devm_clk_get_prepared should not be called from within interrupt context.
+ *
+ * The clock will automatically be unprepared and freed when the
+ * device is unbound from the bus.
+ */
+struct clk *devm_clk_get_prepared(struct device *dev, const char *id);
+
+/**
+ * devm_clk_get_enabled - devm_clk_get() + clk_prepare_enable()
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Returns a struct clk corresponding to the clock producer, or
+ * valid IS_ERR() condition containing errno.  The implementation
+ * uses @dev and @id to determine the clock consumer, and thereby
+ * the clock producer.  (IOW, @id may be identical strings, but
+ * clk_get may return different clock producers depending on @dev.)
+ *
+ * The returned clk (if valid) is prepared and enabled.
+ *
+ * devm_clk_get_prepared should not be called from within interrupt context.
+ *
+ * The clock will automatically be disabled, unprepared and freed when the
+ * device is unbound from the bus.
+ */
+struct clk *devm_clk_get_enabled(struct device *dev, const char *id);
+
 /**
  * devm_clk_get_optional - lookup and obtain a managed reference to an optional
  *			   clock producer.
@@ -469,6 +510,26 @@ struct clk *devm_clk_get(struct device *dev, const char *id);
  */
 struct clk *devm_clk_get_optional(struct device *dev, const char *id);
 
+/**
+ * devm_clk_get_optional_prepared - devm_clk_get_optional() + clk_prepare()
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Behaves the same as devm_clk_get_prepared() except where there is no clock producer.
+ * In this case, instead of returning -ENOENT, the function returns NULL.
+ */
+struct clk *devm_clk_get_optional_prepared(struct device *dev, const char *id);
+
+/**
+ * devm_clk_get_optional_enabled - devm_clk_get_optional() + clk_prepare_enable()
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Behaves the same as devm_clk_get_enabled() except where there is no clock producer.
+ * In this case, instead of returning -ENOENT, the function returns NULL.
+ */
+struct clk *devm_clk_get_optional_enabled(struct device *dev, const char *id);
+
 /**
  * devm_get_clk_from_child - lookup and obtain a managed reference to a
  *			     clock producer from child node.
@@ -813,12 +874,36 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
 	return NULL;
 }
 
+static inline struct clk *devm_clk_get_prepared(struct device *dev,
+						const char *id)
+{
+	return NULL;
+}
+
+static inline struct clk *devm_clk_get_enabled(struct device *dev,
+					       const char *id)
+{
+	return NULL;
+}
+
 static inline struct clk *devm_clk_get_optional(struct device *dev,
 						const char *id)
 {
 	return NULL;
 }
 
+static inline struct clk *devm_clk_get_optional_prepared(struct device *dev,
+							 const char *id)
+{
+	return NULL;
+}
+
+static inline struct clk *devm_clk_get_optional_enabled(struct device *dev,
+							const char *id)
+{
+	return NULL;
+}
+
 static inline int __must_check devm_clk_bulk_get(struct device *dev, int num_clks,
 						 struct clk_bulk_data *clks)
 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled()
  2021-03-30 18:17 [PATCH v4 0/6] clk: provide new devm helpers for prepared and enabled clocks Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 1/6] clk: generalize devm_clk_get() a bit Uwe Kleine-König
  2021-03-30 18:17 ` [PATCH v4 2/6] clk: Provide new devm_clk_helpers for prepared and enabled clocks Uwe Kleine-König
@ 2021-03-30 18:17 ` Uwe Kleine-König
  2021-04-06 12:55   ` Nicolas Ferre
  2 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2021-03-30 18:17 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Alessandro Zummo,
	Alexandre Belloni, Nicolas Ferre, Ludovic Desroches
  Cc: linux-clk, kernel, linux-rtc, linux-arm-kernel

devm_clk_get_enabled() returns the clk already (prepared and) enabled
and the automatically called cleanup cares for disabling (and
unpreparing). So simplify .probe() and .remove() accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/rtc/rtc-at91sam9.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index 2216be429ab7..b52e7bd26303 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -374,21 +374,14 @@ static int at91_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	rtc->sclk = devm_clk_get(&pdev->dev, NULL);
+	rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(rtc->sclk))
 		return PTR_ERR(rtc->sclk);
 
-	ret = clk_prepare_enable(rtc->sclk);
-	if (ret) {
-		dev_err(&pdev->dev, "Could not enable slow clock\n");
-		return ret;
-	}
-
 	sclk_rate = clk_get_rate(rtc->sclk);
 	if (!sclk_rate || sclk_rate > AT91_RTT_RTPRES) {
 		dev_err(&pdev->dev, "Invalid slow clock rate\n");
-		ret = -EINVAL;
-		goto err_clk;
+		return -EINVAL;
 	}
 
 	mr = rtt_readl(rtc, MR);
@@ -406,7 +399,7 @@ static int at91_rtc_probe(struct platform_device *pdev)
 	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
 	if (IS_ERR(rtc->rtcdev)) {
 		ret = PTR_ERR(rtc->rtcdev);
-		goto err_clk;
+		return ret;
 	}
 
 	rtc->rtcdev->ops = &at91_rtc_ops;
@@ -418,7 +411,7 @@ static int at91_rtc_probe(struct platform_device *pdev)
 			       dev_name(&rtc->rtcdev->dev), rtc);
 	if (ret) {
 		dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
-		goto err_clk;
+		return ret;
 	}
 
 	/* NOTE:  sam9260 rev A silicon has a ROM bug which resets the
@@ -432,11 +425,6 @@ static int at91_rtc_probe(struct platform_device *pdev)
 			 dev_name(&rtc->rtcdev->dev));
 
 	return devm_rtc_register_device(rtc->rtcdev);
-
-err_clk:
-	clk_disable_unprepare(rtc->sclk);
-
-	return ret;
 }
 
 /*
@@ -450,8 +438,6 @@ static int at91_rtc_remove(struct platform_device *pdev)
 	/* disable all interrupts */
 	rtt_writel(rtc, MR, mr & ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
 
-	clk_disable_unprepare(rtc->sclk);
-
 	return 0;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled()
  2021-03-30 18:17 ` [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled() Uwe Kleine-König
@ 2021-04-06 12:55   ` Nicolas Ferre
  0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Ferre @ 2021-04-06 12:55 UTC (permalink / raw)
  To: Uwe Kleine-König, Michael Turquette, Stephen Boyd,
	Alessandro Zummo, Alexandre Belloni, Ludovic Desroches
  Cc: linux-clk, kernel, linux-rtc, linux-arm-kernel

On 30/03/2021 at 20:17, Uwe Kleine-König wrote:
> devm_clk_get_enabled() returns the clk already (prepared and) enabled
> and the automatically called cleanup cares for disabling (and
> unpreparing). So simplify .probe() and .remove() accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I do see the benefit:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Uwe for taking these drivers as examples! Best regards,
   Nicolas

> ---
>   drivers/rtc/rtc-at91sam9.c | 22 ++++------------------
>   1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
> index 2216be429ab7..b52e7bd26303 100644
> --- a/drivers/rtc/rtc-at91sam9.c
> +++ b/drivers/rtc/rtc-at91sam9.c
> @@ -374,21 +374,14 @@ static int at91_rtc_probe(struct platform_device *pdev)
>                  return -ENOMEM;
>          }
> 
> -       rtc->sclk = devm_clk_get(&pdev->dev, NULL);
> +       rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
>          if (IS_ERR(rtc->sclk))
>                  return PTR_ERR(rtc->sclk);
> 
> -       ret = clk_prepare_enable(rtc->sclk);
> -       if (ret) {
> -               dev_err(&pdev->dev, "Could not enable slow clock\n");
> -               return ret;
> -       }
> -
>          sclk_rate = clk_get_rate(rtc->sclk);
>          if (!sclk_rate || sclk_rate > AT91_RTT_RTPRES) {
>                  dev_err(&pdev->dev, "Invalid slow clock rate\n");
> -               ret = -EINVAL;
> -               goto err_clk;
> +               return -EINVAL;
>          }
> 
>          mr = rtt_readl(rtc, MR);
> @@ -406,7 +399,7 @@ static int at91_rtc_probe(struct platform_device *pdev)
>          rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
>          if (IS_ERR(rtc->rtcdev)) {
>                  ret = PTR_ERR(rtc->rtcdev);
> -               goto err_clk;
> +               return ret;
>          }
> 
>          rtc->rtcdev->ops = &at91_rtc_ops;
> @@ -418,7 +411,7 @@ static int at91_rtc_probe(struct platform_device *pdev)
>                                 dev_name(&rtc->rtcdev->dev), rtc);
>          if (ret) {
>                  dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
> -               goto err_clk;
> +               return ret;
>          }
> 
>          /* NOTE:  sam9260 rev A silicon has a ROM bug which resets the
> @@ -432,11 +425,6 @@ static int at91_rtc_probe(struct platform_device *pdev)
>                           dev_name(&rtc->rtcdev->dev));
> 
>          return devm_rtc_register_device(rtc->rtcdev);
> -
> -err_clk:
> -       clk_disable_unprepare(rtc->sclk);
> -
> -       return ret;
>   }
> 
>   /*
> @@ -450,8 +438,6 @@ static int at91_rtc_remove(struct platform_device *pdev)
>          /* disable all interrupts */
>          rtt_writel(rtc, MR, mr & ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
> 
> -       clk_disable_unprepare(rtc->sclk);
> -
>          return 0;
>   }
> 
> --
> 2.30.2
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-06 12:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 18:17 [PATCH v4 0/6] clk: provide new devm helpers for prepared and enabled clocks Uwe Kleine-König
2021-03-30 18:17 ` [PATCH v4 1/6] clk: generalize devm_clk_get() a bit Uwe Kleine-König
2021-03-30 18:17 ` [PATCH v4 2/6] clk: Provide new devm_clk_helpers for prepared and enabled clocks Uwe Kleine-König
2021-03-30 18:17 ` [PATCH v4 4/6] rtc: at91sma9: Simplify using devm_clk_get_enabled() Uwe Kleine-König
2021-04-06 12:55   ` Nicolas Ferre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).