public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Would devm_regulator_enable be useful ?
@ 2014-02-02  0:23 Guenter Roeck
  2014-02-03 18:21 ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2014-02-02  0:23 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org; +Cc: Liam Girdwood, Mark Brown

Hi all,

while working with regulators, I noticed that there is no devm_regulator_enable() API.

Seems to me it would be useful to have it, but then devm_clk_enable() doesn't exist either,
so I wonder if there is a reason for not having it.

I'll be happy to submit a patch if people think it is useful; on the other side,
I would like to understand the reason for not having it if there is one.

Thanks,
Guenter

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

* Re: Would devm_regulator_enable be useful ?
  2014-02-02  0:23 Would devm_regulator_enable be useful ? Guenter Roeck
@ 2014-02-03 18:21 ` Mark Brown
  2014-02-03 22:27   ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-02-03 18:21 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-kernel@vger.kernel.org, Liam Girdwood

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On Sat, Feb 01, 2014 at 04:23:59PM -0800, Guenter Roeck wrote:

As previously mentioned please fix your mailer to word wrap at a
sensible limit.

> Seems to me it would be useful to have it, but then devm_clk_enable()
> doesn't exist either, so I wonder if there is a reason for not having
> it.

In both cases enabling and then leaving the resource enabled throughout
the runtime of the device isn't normally the best practice for using
them.  You usually want to enable and disable at runtime with mechanisms
like runtime PM when the device is idle rather than burning power all
the time and once you start doing that managed resources don't fit so
well.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: Would devm_regulator_enable be useful ?
  2014-02-03 18:21 ` Mark Brown
@ 2014-02-03 22:27   ` Guenter Roeck
  2014-02-04 11:10     ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2014-02-03 22:27 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel@vger.kernel.org, Liam Girdwood

On Mon, Feb 03, 2014 at 06:21:52PM +0000, Mark Brown wrote:
> On Sat, Feb 01, 2014 at 04:23:59PM -0800, Guenter Roeck wrote:
> 
> As previously mentioned please fix your mailer to word wrap at a
> sensible limit.
> 
I thought I did ;-). I'll try to make sure I only send e-mail to you
using mutt in the future ... but I notice that your line length is
less than the one I configured, so maybe that is the problem here.

> > Seems to me it would be useful to have it, but then devm_clk_enable()
> > doesn't exist either, so I wonder if there is a reason for not having
> > it.
> 
> In both cases enabling and then leaving the resource enabled throughout
> the runtime of the device isn't normally the best practice for using
> them.  You usually want to enable and disable at runtime with mechanisms
> like runtime PM when the device is idle rather than burning power all
> the time and once you start doing that managed resources don't fit so
> well.

Ok, I accept that. I thought that was what devm_xxx_[disable,remove] etc
was for, though.

Thanks,
Guenter

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

* Re: Would devm_regulator_enable be useful ?
  2014-02-03 22:27   ` Guenter Roeck
@ 2014-02-04 11:10     ` Mark Brown
  2014-02-04 14:22       ` Guenter Roeck
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-02-04 11:10 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-kernel@vger.kernel.org, Liam Girdwood

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

On Mon, Feb 03, 2014 at 02:27:26PM -0800, Guenter Roeck wrote:
> On Mon, Feb 03, 2014 at 06:21:52PM +0000, Mark Brown wrote:

> > As previously mentioned please fix your mailer to word wrap at a
> > sensible limit.

> I thought I did ;-). I'll try to make sure I only send e-mail to you
> using mutt in the future ... but I notice that your line length is
> less than the one I configured, so maybe that is the problem here.

You need to allow some room for quoting.

> > In both cases enabling and then leaving the resource enabled throughout
> > the runtime of the device isn't normally the best practice for using
> > them.  You usually want to enable and disable at runtime with mechanisms
> > like runtime PM when the device is idle rather than burning power all
> > the time and once you start doing that managed resources don't fit so
> > well.

> Ok, I accept that. I thought that was what devm_xxx_[disable,remove] etc
> was for, though.

Sort of.  They're there but that doesn't mean that they should be used
in normal operation - they should be special cases, not normal things.
Managed resources are supposed to for things that are more fire and
forget.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: Would devm_regulator_enable be useful ?
  2014-02-04 11:10     ` Mark Brown
@ 2014-02-04 14:22       ` Guenter Roeck
  2014-02-04 20:09         ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2014-02-04 14:22 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel@vger.kernel.org, Liam Girdwood

On 02/04/2014 03:10 AM, Mark Brown wrote:
> On Mon, Feb 03, 2014 at 02:27:26PM -0800, Guenter Roeck wrote:
>> On Mon, Feb 03, 2014 at 06:21:52PM +0000, Mark Brown wrote:
>
>>> As previously mentioned please fix your mailer to word wrap at a
>>> sensible limit.
>
>> I thought I did ;-). I'll try to make sure I only send e-mail to you
>> using mutt in the future ... but I notice that your line length is
>> less than the one I configured, so maybe that is the problem here.
>
> You need to allow some room for quoting.
>
>>> In both cases enabling and then leaving the resource enabled throughout
>>> the runtime of the device isn't normally the best practice for using
>>> them.  You usually want to enable and disable at runtime with mechanisms
>>> like runtime PM when the device is idle rather than burning power all
>>> the time and once you start doing that managed resources don't fit so
>>> well.
>
>> Ok, I accept that. I thought that was what devm_xxx_[disable,remove] etc
>> was for, though.
>
> Sort of.  They're there but that doesn't mean that they should be used
> in normal operation - they should be special cases, not normal things.
> Managed resources are supposed to for things that are more fire and
> forget.
>

Isn't that a bit philosophical ? The drivers I had in mind commonly
call regulator_enable() in probe and regulator_disable() in remove.
Having device managed functions would simplify that code a lot.
If those same drivers implement pm functions, I don't see a problem
using devm_ functions in those. Sure, execution complexity is a bit
higher, but it is not as if pm functions are high volume calls.
And, after all, the existence of devm_ functions doesn't mean
that they _have_ to be used.

Guenter


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

* Re: Would devm_regulator_enable be useful ?
  2014-02-04 14:22       ` Guenter Roeck
@ 2014-02-04 20:09         ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-02-04 20:09 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-kernel@vger.kernel.org, Liam Girdwood

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Tue, Feb 04, 2014 at 06:22:30AM -0800, Guenter Roeck wrote:
> On 02/04/2014 03:10 AM, Mark Brown wrote:

> >Sort of.  They're there but that doesn't mean that they should be used
> >in normal operation - they should be special cases, not normal things.
> >Managed resources are supposed to for things that are more fire and
> >forget.

> Isn't that a bit philosophical ? The drivers I had in mind commonly
> call regulator_enable() in probe and regulator_disable() in remove.
> Having device managed functions would simplify that code a lot.
> If those same drivers implement pm functions, I don't see a problem
> using devm_ functions in those. Sure, execution complexity is a bit
> higher, but it is not as if pm functions are high volume calls.
> And, after all, the existence of devm_ functions doesn't mean
> that they _have_ to be used.

It's partly about what we're encouraging people to do - if the
frameworks are encouraging people to do things we don't want them to do
that's not great, and if there are things that are normally warning
signs that are getting used normally that's a bit worrying.

For what you're talking about it'd seem better to have the core
automatically drop the reference counts on enabled regulators when the
consumer is destroyed all the time rather than having an explicit devm_
function for it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

end of thread, other threads:[~2014-02-04 20:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-02  0:23 Would devm_regulator_enable be useful ? Guenter Roeck
2014-02-03 18:21 ` Mark Brown
2014-02-03 22:27   ` Guenter Roeck
2014-02-04 11:10     ` Mark Brown
2014-02-04 14:22       ` Guenter Roeck
2014-02-04 20:09         ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox