Re: ASoC updates for v4.2

[Takashi Iwai <tiwai@suse.de>]

At Mon, 22 Jun 2015 15:43:38 +0100,
Mark Brown wrote:
> 
> On Mon, Jun 22, 2015 at 04:10:32PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> > > > Mark Brown wrote:
> > > > > On Mon, Jun 22, 2015 at 11:58:24AM +0200, Takashi Iwai wrote:
> 
> > > > > > And, looking at the code, it seems calling runtime suspend in the
> > > > > > following way at probe:
> 
> > > > > I'm confused, where's the call to runtime suspend?
> 
> > > Sorry, I'm still confused about what you're seeing in the probe - I know
> > > where the callbacks for runtime PM are registered but I'm not seeing a
> > > call to suspend (or something that I'd expect to trigger one) in the
> > > above?
> 
> > There is no place calling runtime suspend manually, that's why the
> > compiler catches and warns.
> 
> Right, that's why I was confused - you said it was calling runtime
> suspend.

Ah sorry, missed that; I meant runtime resume, of course.

> > > > But my concern above isn't about the warning itself.  I just stumbled
> > > > on the code invoking runtime resume while looking at this warning, and
> > > > wondered the behavior with CONFIG_PM=n.
> 
> > > > Usually this kind of warning could be simply fixed by adding a proper
> > > > ifdef.  But, this driver calls runtime resume in the probe manually.
> 
> > > Sure, that's a fairly common pattern though?
> 
> > Depends.  The more common pattern seems to call pm_runtime_resume().
> > And this will skip the call of runtime PM when CONFIG_PM=n.
> 
> That's another way of doing the same thing but it still leaves the same
> thing with sharing the runtime code - if the runtime suspend and resume
> paths are the same as the normal power up/down sequence you need to have
> an explicit call to the shared power up function somewhere in probe and
> can't ifdef things.
>
> It does also mean that there's always going to be a bounce on of the
> power in probe which is a bit sad though hardly the end of the world.

Yep.  Also, basically the check of pm_runtime_enabled() is
superfluous, too, once when everything gets coded in a proper
balance.


Takashi