1. Patchew
  2. linux
  3. View series

[RFC PATCH] reset: Add devm_reset_control_deassert helper

Prabhakar posted 1 patch 1 year, 9 months ago
There is a newer version of this series
drivers/reset/core.c  | 22 ++++++++++++++++++++++
include/linux/reset.h |  6 ++++++
2 files changed, 28 insertions(+)
[RFC PATCH] reset: Add devm_reset_control_deassert helper
Posted by Prabhakar 1 year, 9 months ago 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

A typical code pattern for reset_control_deassert() call is to call it in
the _probe function and to call reset_control_assert() both from _probe
error path and from _remove function.

Add helper function to replace this bolierplate piece of code. Calling
devm_reset_control_deassert() removes the need for calling
reset_control_assert() both in the probe()'s error path and in the
remove() function.

Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/reset/core.c  | 22 ++++++++++++++++++++++
 include/linux/reset.h |  6 ++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index dba74e857be6..a2a6eff8e599 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -592,6 +592,28 @@ int reset_control_deassert(struct reset_control *rstc)
 }
 EXPORT_SYMBOL_GPL(reset_control_deassert);
 
+static void reset_control_assert_action(void *rstc)
+{
+	reset_control_assert(rstc);
+}
+
+/**
+ * devm_reset_control_deassert - devres-enabled version of reset_control_deassert()
+ * @dev: device that requests the reset control
+ * @rstc: reset controller
+ */
+int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc)
+{
+	int ret;
+
+	ret = reset_control_deassert(rstc);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, reset_control_assert_action, rstc);
+}
+EXPORT_SYMBOL_GPL(devm_reset_control_deassert);
+
 /**
  * reset_control_bulk_deassert - deasserts the reset lines in reverse order
  * @num_rstcs: number of entries in rstcs array
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 514ddf003efc..e41e752ba098 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -31,6 +31,7 @@ int reset_control_reset(struct reset_control *rstc);
 int reset_control_rearm(struct reset_control *rstc);
 int reset_control_assert(struct reset_control *rstc);
 int reset_control_deassert(struct reset_control *rstc);
+int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc);
 int reset_control_status(struct reset_control *rstc);
 int reset_control_acquire(struct reset_control *rstc);
 void reset_control_release(struct reset_control *rstc);
@@ -91,6 +92,11 @@ static inline int reset_control_deassert(struct reset_control *rstc)
 	return 0;
 }
 
+static inline int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc)
+{
+	return 0;
+}
+
 static inline int reset_control_status(struct reset_control *rstc)
 {
 	return 0;
-- 
2.34.1
Re: [RFC PATCH] reset: Add devm_reset_control_deassert helper
Posted by Philipp Zabel 1 year, 7 months ago 
Hi,

On Do, 2024-04-18 at 14:26 +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> A typical code pattern for reset_control_deassert() call is to call it in
> the _probe function and to call reset_control_assert() both from _probe
> error path and from _remove function.
> 
> Add helper function to replace this bolierplate piece of code. Calling
> devm_reset_control_deassert() removes the need for calling
> reset_control_assert() both in the probe()'s error path and in the
> remove() function.
> 
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I'm not sure this aligns well with the intended use of devres for
resource acquisition and release.

Note how there is no devm_clk_prepare_enable, devm_regulator_enable,
devm_gpiod_set_value_cansleep, or devm_pwm_enable either.

I've sent an alternative suggestion that adds
devm_reset_control_get..._deasserted calls, similarly to the existing
devm_clk_get..._enabled calls. Please let me know what you think.

regards
Philipp
Re: [RFC PATCH] reset: Add devm_reset_control_deassert helper
Posted by Geert Uytterhoeven 1 year, 7 months ago 
Hi Philipp,

On Fri, Jun 21, 2024 at 4:45 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Do, 2024-04-18 at 14:26 +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > A typical code pattern for reset_control_deassert() call is to call it in
> > the _probe function and to call reset_control_assert() both from _probe
> > error path and from _remove function.
> >
> > Add helper function to replace this bolierplate piece of code. Calling
> > devm_reset_control_deassert() removes the need for calling
> > reset_control_assert() both in the probe()'s error path and in the
> > remove() function.
> >
> > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I'm not sure this aligns well with the intended use of devres for
> resource acquisition and release.
>
> Note how there is no devm_clk_prepare_enable, devm_regulator_enable,
> devm_gpiod_set_value_cansleep, or devm_pwm_enable either.
>
> I've sent an alternative suggestion that adds
> devm_reset_control_get..._deasserted calls, similarly to the existing
> devm_clk_get..._enabled calls. Please let me know what you think.

Thank you, that sounds like a good alternative.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [RFC PATCH] reset: Add devm_reset_control_deassert helper
Posted by Lad, Prabhakar 1 year, 9 months ago 
On Thu, Apr 18, 2024 at 2:26 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> A typical code pattern for reset_control_deassert() call is to call it in
> the _probe function and to call reset_control_assert() both from _probe
> error path and from _remove function.
>
> Add helper function to replace this bolierplate piece of code. Calling
> devm_reset_control_deassert() removes the need for calling
> reset_control_assert() both in the probe()'s error path and in the
> remove() function.
>
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/reset/core.c  | 22 ++++++++++++++++++++++
>  include/linux/reset.h |  6 ++++++
>  2 files changed, 28 insertions(+)
>
Gentle ping.

Cheers,
Prabhakar

> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index dba74e857be6..a2a6eff8e599 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -592,6 +592,28 @@ int reset_control_deassert(struct reset_control *rstc)
>  }
>  EXPORT_SYMBOL_GPL(reset_control_deassert);
>
> +static void reset_control_assert_action(void *rstc)
> +{
> +       reset_control_assert(rstc);
> +}
> +
> +/**
> + * devm_reset_control_deassert - devres-enabled version of reset_control_deassert()
> + * @dev: device that requests the reset control
> + * @rstc: reset controller
> + */
> +int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc)
> +{
> +       int ret;
> +
> +       ret = reset_control_deassert(rstc);
> +       if (ret)
> +               return ret;
> +
> +       return devm_add_action_or_reset(dev, reset_control_assert_action, rstc);
> +}
> +EXPORT_SYMBOL_GPL(devm_reset_control_deassert);
> +
>  /**
>   * reset_control_bulk_deassert - deasserts the reset lines in reverse order
>   * @num_rstcs: number of entries in rstcs array
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index 514ddf003efc..e41e752ba098 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -31,6 +31,7 @@ int reset_control_reset(struct reset_control *rstc);
>  int reset_control_rearm(struct reset_control *rstc);
>  int reset_control_assert(struct reset_control *rstc);
>  int reset_control_deassert(struct reset_control *rstc);
> +int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc);
>  int reset_control_status(struct reset_control *rstc);
>  int reset_control_acquire(struct reset_control *rstc);
>  void reset_control_release(struct reset_control *rstc);
> @@ -91,6 +92,11 @@ static inline int reset_control_deassert(struct reset_control *rstc)
>         return 0;
>  }
>
> +static inline int devm_reset_control_deassert(struct device *dev, struct reset_control *rstc)
> +{
> +       return 0;
> +}
> +
>  static inline int reset_control_status(struct reset_control *rstc)
>  {
>         return 0;
> --
> 2.34.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.