| Message ID | fa667d6870976a2cf2d60f06e262982872349d74.1665066397.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: Support ROHM/Kionix kx022a | expand |
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); > +}
On Thu, 6 Oct 2022 19:17:39 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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); > > +} >
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.
On Sun, Oct 09, 2022 at 01:24:21PM +0100, Jonathan Cameron wrote: > On Thu, 6 Oct 2022 19:17:39 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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?
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).
Hi Jonathan, On 10/9/22 15:24, Jonathan Cameron wrote: > On Thu, 6 Oct 2022 19:17:39 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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
On Mon, 10 Oct 2022 12:24:51 +0300 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > Hi Jonathan, > > On 10/9/22 15:24, Jonathan Cameron wrote: > > On Thu, 6 Oct 2022 19:17:39 +0300 > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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(?) I'll be messy, but the majority of that fix series will need to go in during rcX.. The last patch (refactoring one) will need to wait until those are upstream and have worked there way around to be in my togreg branch (I usually rebase that tree only after pull requests, but given this is going on I may do an earlier pull request than normal.) That refactoring patch will probably need to cover the new driver. I can do that as a fixup whilst applying it though once we get that far. Jonathan > > Yours > -- Matti >
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, ®ulator_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) {
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 <mazziesaccount@gmail.com> (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(+)