| Message ID | 20240418132602.509313-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [RFC] reset: Add devm_reset_control_deassert helper | expand |
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 >
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
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
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;