Re: [PATCH v2 2/7] ACPI / LPSS: allow to use specific PM domain during ->probe()

["Shevchenko, Andriy" <andriy.shevchenko@intel.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3160 bytes --]

On Fri, 2015-11-27 at 11:56 +0200, Andy Shevchenko wrote:
> > > > > 
> On Fri, 2015-11-27 at 00:15 +0100, Rafael J. Wysocki wrote:
> > On Thursday, November 26, 2015 06:45:17 PM Andy Shevchenko wrote:
> > > On Thu, 2015-11-26 at 18:30 +0200, Jarkko Nikula wrote:
> > > > On 11/26/2015 05:19 PM, Andy Shevchenko wrote:
> > > > This won't fix like revert of original commit does.

Jarkko, I will split this one to the revert (with Fixes tag) and new
patch to target DMA issue.
    

> > > > Primary problem here 
> > > > is that there is no explicit power on at all during LPSS device
> > > > probe
> > > > because dev->pm_domain is set before probing.
> > > 
> > > And we can't do this as in very original code of acpi_lpss.c
> > > since
> > > DMA
> > > has to be sure it's powered on while probing. We could guarantee
> > > this
> > > only in case when PM domain is assigned already and we do our
> > > quirk
> > > for
> > > it.
> > > 
> > > From my point of view we have to fix hang first since it's most
> > > painful
> > > case for users and their experience. Though I'm open to any
> > > better
> > > solution if you have any in mind.
> > > 
> > > > 
> > > > driver_probe_device
> > > >    platform_drv_prove
> > > >      dev_pm_domain_attach
> > > >        acpi_dev_pm_attach
> > > >          returns instantly because of dev->pm_domain is set
> > 
> > This looks like a candidate for the new PM domain callbacks,
> > ->activate and
> > ->dismiss.
> > 
> > ->activate() is called before the probe, so it may power up things.
> > 
> > ->dismiss() in turn is called in the failed probe case, so it can
> > do
> > the
> > cleanup.
> > 
> > Have you considered using these?
> 
> Thanks for the hint. We will check this.

I briefly checked this for DMA issue. It will not help anyhow, so we
*have to* move a power domain assignment to the BIND stage.

For I2C and rest LPSS devices this might help (though didn't look
deeply). My understanding that we assign those callbacks in the LPSS
custom PM domain and call them explicitly in acpi_lpss.c.

The code will be the same as we are using now to bring device from
runtime suspend resume. This means whenever we call probe for e.g. I2C
we end up in a sequence similar to:
 pm_runtime_resume(I2C);
 ->probe(I2C);
 pm_runtime_suspend(I2C);

I will try to mock up this and check if it will work, though have no
idea what to do if I2C during probe calls pm_runtime_forbid().

Jarkko, what do you think?

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥