2022-10-06 14:40:03

by Matti Vaittinen

[permalink] [raw]
Subject: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

A few regulator consumer drivers seem to be just getting a regulator,
enabling it and registering a devm-action to disable the regulator at
the driver detach and then forget about it.

We can simplify this a bit by adding a devm-helper for this pattern.
Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()

Signed-off-by: Matti Vaittinen <[email protected]>
(cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)

---

Already in Mark's regulator tree. Not to be merged. Included just for
the sake of the completeness. Will be dropped when series is rebased on
top of the 6.1-rc1
---
drivers/regulator/devres.c | 59 ++++++++++++++++++++++++++++++
include/linux/regulator/consumer.h | 13 +++++++
2 files changed, 72 insertions(+)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 32823a87fd40..3cb3eb49ad8a 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -70,6 +70,65 @@ struct regulator *devm_regulator_get_exclusive(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_regulator_get_exclusive);

+static void regulator_action_disable(void *d)
+{
+ struct regulator *r = (struct regulator *)d;
+
+ regulator_disable(r);
+}
+
+static int _devm_regulator_get_enable(struct device *dev, const char *id,
+ int get_type)
+{
+ struct regulator *r;
+ int ret;
+
+ r = _devm_regulator_get(dev, id, get_type);
+ if (IS_ERR(r))
+ return PTR_ERR(r);
+
+ ret = regulator_enable(r);
+ if (!ret)
+ ret = devm_add_action_or_reset(dev, &regulator_action_disable, r);
+
+ if (ret)
+ devm_regulator_put(r);
+
+ return ret;
+}
+
+/**
+ * devm_regulator_get_enable_optional - Resource managed regulator get and enable
+ * @dev: device to supply
+ * @id: supply name or regulator ID.
+ *
+ * Get and enable regulator for duration of the device life-time.
+ * regulator_disable() and regulator_put() are automatically called on driver
+ * detach. See regulator_get_optional() and regulator_enable() for more
+ * information.
+ */
+int devm_regulator_get_enable_optional(struct device *dev, const char *id)
+{
+ return _devm_regulator_get_enable(dev, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_get_enable_optional);
+
+/**
+ * devm_regulator_get_enable - Resource managed regulator get and enable
+ * @dev: device to supply
+ * @id: supply name or regulator ID.
+ *
+ * Get and enable regulator for duration of the device life-time.
+ * regulator_disable() and regulator_put() are automatically called on driver
+ * detach. See regulator_get() and regulator_enable() for more
+ * information.
+ */
+int devm_regulator_get_enable(struct device *dev, const char *id)
+{
+ return _devm_regulator_get_enable(dev, id, NORMAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_get_enable);
+
/**
* devm_regulator_get_optional - Resource managed regulator_get_optional()
* @dev: device to supply
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index bc6cda706d1f..8e6cf65851b0 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -207,6 +207,8 @@ struct regulator *__must_check regulator_get_optional(struct device *dev,
const char *id);
struct regulator *__must_check devm_regulator_get_optional(struct device *dev,
const char *id);
+int devm_regulator_get_enable(struct device *dev, const char *id);
+int devm_regulator_get_enable_optional(struct device *dev, const char *id);
void regulator_put(struct regulator *regulator);
void devm_regulator_put(struct regulator *regulator);

@@ -354,6 +356,17 @@ devm_regulator_get_exclusive(struct device *dev, const char *id)
return ERR_PTR(-ENODEV);
}

+static inline int devm_regulator_get_enable(struct device *dev, const char *id)
+{
+ return -ENODEV;
+}
+
+static inline int devm_regulator_get_enable_optional(struct device *dev,
+ const char *id)
+{
+ return -ENODEV;
+}
+
static inline struct regulator *__must_check
regulator_get_optional(struct device *dev, const char *id)
{
--
2.37.3


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]


Attachments:
(No filename) (4.42 kB)
signature.asc (499.00 B)
Download all attachments

2022-10-06 16:23:11

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:
> A few regulator consumer drivers seem to be just getting a regulator,
> enabling it and registering a devm-action to disable the regulator at
> the driver detach and then forget about it.
>
> We can simplify this a bit by adding a devm-helper for this pattern.
> Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()

...

> (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)

Not sure:
- why this is in the commit message
- what it points to, since
$ git show b6058e052b842a19c8bb639798d8692cd0e7589f
fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f

> Already in Mark's regulator tree. Not to be merged. Included just for
> the sake of the completeness. Will be dropped when series is rebased on
> top of the 6.1-rc1

Ah, I see, but does it mean the commit has been rebased or you used wrong SHA?

...

> +static void regulator_action_disable(void *d)
> +{
> + struct regulator *r = (struct regulator *)d;

Cast is not needed.

> + regulator_disable(r);
> +}

--
With Best Regards,
Andy Shevchenko


2022-10-09 12:37:32

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

On Thu, 6 Oct 2022 19:17:39 +0300
Andy Shevchenko <[email protected]> wrote:

> On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:
> > A few regulator consumer drivers seem to be just getting a regulator,
> > enabling it and registering a devm-action to disable the regulator at
> > the driver detach and then forget about it.
> >
> > We can simplify this a bit by adding a devm-helper for this pattern.
> > Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()
>
> ...
>
> > (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)
>
> Not sure:
> - why this is in the commit message
> - what it points to, since
> $ git show b6058e052b842a19c8bb639798d8692cd0e7589f
> fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f

These are now upstream in Linus' tree and in my testing branch.
I'd not normally advocate working on top of that (because I rebase it), but
if it is useful for this series go ahead.

Jonathan

>
> > Already in Mark's regulator tree. Not to be merged. Included just for
> > the sake of the completeness. Will be dropped when series is rebased on
> > top of the 6.1-rc1
>
> Ah, I see, but does it mean the commit has been rebased or you used wrong SHA?
>
> ...
>
> > +static void regulator_action_disable(void *d)
> > +{
> > + struct regulator *r = (struct regulator *)d;
>
> Cast is not needed.
>
> > + regulator_disable(r);
> > +}
>

2022-10-10 04:35:13

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

Hi Andy,

On 10/6/22 19:17, Andy Shevchenko wrote:
> On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:
>> A few regulator consumer drivers seem to be just getting a regulator,
>> enabling it and registering a devm-action to disable the regulator at
>> the driver detach and then forget about it.
>>
>> We can simplify this a bit by adding a devm-helper for this pattern.
>> Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()
>
> ...
>
>> (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)
>
> Not sure:
> - why this is in the commit message
> - what it points to, since
> $ git show b6058e052b842a19c8bb639798d8692cd0e7589f
> fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f
>
>> Already in Mark's regulator tree. Not to be merged. Included just for
>> the sake of the completeness. Will be dropped when series is rebased on
>> top of the 6.1-rc1
>
> Ah, I see, but does it mean the commit has been rebased or you used wrong SHA?

I did probably cherry-pick this from my local development branch and not
from Mark's tree. Sorry for the confusion. I thought people would ignore
these first two patches when reviewing as was requested in cover-letter.

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

2022-10-10 06:31:22

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

On Mon, Oct 10, 2022 at 07:13:23AM +0300, Matti Vaittinen wrote:
> On 10/6/22 19:17, Andy Shevchenko wrote:
> > On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:
> > > A few regulator consumer drivers seem to be just getting a regulator,
> > > enabling it and registering a devm-action to disable the regulator at
> > > the driver detach and then forget about it.
> > >
> > > We can simplify this a bit by adding a devm-helper for this pattern.
> > > Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()

...

> > > (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)
> >
> > Not sure:
> > - why this is in the commit message
> > - what it points to, since
> > $ git show b6058e052b842a19c8bb639798d8692cd0e7589f
> > fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f
> >
> > > Already in Mark's regulator tree. Not to be merged. Included just for
> > > the sake of the completeness. Will be dropped when series is rebased on
> > > top of the 6.1-rc1
> >
> > Ah, I see, but does it mean the commit has been rebased or you used wrong SHA?
>
> I did probably cherry-pick this from my local development branch and not
> from Mark's tree. Sorry for the confusion. I thought people would ignore
> these first two patches when reviewing as was requested in cover-letter.

The solution as pointed out by LKP and which will removes the need of the noise
in email and a lot of confusions is to use --base parameter to the patch(es).

--
With Best Regards,
Andy Shevchenko


2022-10-10 06:31:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

On Sun, Oct 09, 2022 at 01:24:21PM +0100, Jonathan Cameron wrote:
> On Thu, 6 Oct 2022 19:17:39 +0300
> Andy Shevchenko <[email protected]> wrote:
> > On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:

...

> > > (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)
> >
> > Not sure:
> > - why this is in the commit message
> > - what it points to, since
> > $ git show b6058e052b842a19c8bb639798d8692cd0e7589f
> > fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f
>
> These are now upstream in Linus' tree and in my testing branch.

I don't see them. As I pointed out the commit IDs are not in the any of the
official trees (subsystem maintainer's or Linus'). Again, read my doubts about
the above commit message.

> I'd not normally advocate working on top of that (because I rebase it), but
> if it is useful for this series go ahead.

> > > Already in Mark's regulator tree. Not to be merged. Included just for
> > > the sake of the completeness. Will be dropped when series is rebased on
> > > top of the 6.1-rc1
> >
> > Ah, I see, but does it mean the commit has been rebased or you used wrong SHA?

--
With Best Regards,
Andy Shevchenko


2022-10-10 10:15:37

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/5] regulator: Add devm helpers for get and enable

Hi Jonathan,

On 10/9/22 15:24, Jonathan Cameron wrote:
> On Thu, 6 Oct 2022 19:17:39 +0300
> Andy Shevchenko <[email protected]> wrote:
>
>> On Thu, Oct 06, 2022 at 05:36:52PM +0300, Matti Vaittinen wrote:
>>> A few regulator consumer drivers seem to be just getting a regulator,
>>> enabling it and registering a devm-action to disable the regulator at
>>> the driver detach and then forget about it.
>>>
>>> We can simplify this a bit by adding a devm-helper for this pattern.
>>> Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()
>>
>> ...
>>
>>> (cherry picked from commit b6058e052b842a19c8bb639798d8692cd0e7589f)
>>
>> Not sure:
>> - why this is in the commit message
>> - what it points to, since
>> $ git show b6058e052b842a19c8bb639798d8692cd0e7589f
>> fatal: bad object b6058e052b842a19c8bb639798d8692cd0e7589f
>
> These are now upstream in Linus' tree and in my testing branch.
> I'd not normally advocate working on top of that (because I rebase it), but
> if it is useful for this series go ahead.

Thanks for the explanation :)

This series will conflict with my fixup series for triggered-buffer
attributes. Hence I though I might combine these two series into one if
I need to respin the fixup series. I thought of using the v6.1-rc1 when
it is out. (I think the 6.1-rc1 should not be that far away)

OTOH, I just read your another mail which told that there will be one
more driver which will conflict with the fixup coming in during this
cycle. If that driver lands in your tree before the fix - then I guess I
need to rebase the fixup series (and maybe this too) on top of your tree
+ add conversion of this new driver. I don't think that would be the
testing branch though(?)

Yours
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~