All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	linux-acpi@vger.kernel.org, Vinod Koul <vinod.koul@intel.com>,
	dmaengine@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH v2 2/7] ACPI / LPSS: allow to use specific PM domain during ->probe()
Date: Fri, 27 Nov 2015 11:56:13 +0200	[thread overview]
Message-ID: <1448618173.15393.104.camel@linux.intel.com> (raw)
In-Reply-To: <10907323.ZOUAMFPIfU@vostro.rjw.lan>

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 is an amendment to previously pushed commit 01ac170ba29a
> > > > (ACPI
> > > > / LPSS:
> > > > allow to use specific PM domain during ->probe()). We can't
> > > > assign
> > > > anything to
> > > > the platform device on ADD_DEVICE stage since it might be
> > > > changed
> > > > during
> > > > unbound / bind cycle.
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.c
> > > > om>
> > > > ---
> > > >   drivers/acpi/acpi_lpss.c | 9 +++++++--
> > > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/acpi/acpi_lpss.c
> > > > b/drivers/acpi/acpi_lpss.c
> > > > index f9e0d09..e840229 100644
> > > > --- a/drivers/acpi/acpi_lpss.c
> > > > +++ b/drivers/acpi/acpi_lpss.c
> > > > @@ -705,8 +705,14 @@ static int
> > > > acpi_lpss_platform_notify(struct
> > > > notifier_block *nb,
> > > >   	}
> > > > 
> > > >   	switch (action) {
> > > > -	case BUS_NOTIFY_ADD_DEVICE:
> > > > +	case BUS_NOTIFY_BIND_DRIVER:
> > > >   		pdev->dev.pm_domain = &acpi_lpss_pm_domain;
> > > > +		break;
> > > > +	case BUS_NOTIFY_BIND_DRIVER_ERROR:
> > > > +	case BUS_NOTIFY_UNBOUND_DRIVER:
> > > > +		pdev->dev.pm_domain = NULL;
> > > > +		break;
> > > > +	case BUS_NOTIFY_ADD_DEVICE:
> > > 
> > > This won't fix like revert of original commit does.
> 
> Which commit exactly are you talking about, for reference?

commit 01ac170ba29a9903ee590e1ef2d8e6b27b49a16c
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Wed Nov 5 18:34:46 2014 +0200

    ACPI / LPSS: allow to use specific PM domain during ->probe()
    

> > > 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.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	linux-acpi@vger.kernel.org, Vinod Koul <vinod.koul@intel.com>,
	dmaengine@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH v2 2/7] ACPI / LPSS: allow to use specific PM domain during ->probe()
Date: Fri, 27 Nov 2015 11:56:13 +0200	[thread overview]
Message-ID: <1448618173.15393.104.camel@linux.intel.com> (raw)
In-Reply-To: <10907323.ZOUAMFPIfU@vostro.rjw.lan>

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 is an amendment to previously pushed commit 01ac170ba29a
> > > > (ACPI
> > > > / LPSS:
> > > > allow to use specific PM domain during ->probe()). We can't
> > > > assign
> > > > anything to
> > > > the platform device on ADD_DEVICE stage since it might be
> > > > changed
> > > > during
> > > > unbound / bind cycle.
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.c
> > > > om>
> > > > ---
> > > >   drivers/acpi/acpi_lpss.c | 9 +++++++--
> > > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/acpi/acpi_lpss.c
> > > > b/drivers/acpi/acpi_lpss.c
> > > > index f9e0d09..e840229 100644
> > > > --- a/drivers/acpi/acpi_lpss.c
> > > > +++ b/drivers/acpi/acpi_lpss.c
> > > > @@ -705,8 +705,14 @@ static int
> > > > acpi_lpss_platform_notify(struct
> > > > notifier_block *nb,
> > > >   	}
> > > > 
> > > >   	switch (action) {
> > > > -	case BUS_NOTIFY_ADD_DEVICE:
> > > > +	case BUS_NOTIFY_BIND_DRIVER:
> > > >   		pdev->dev.pm_domain = &acpi_lpss_pm_domain;
> > > > +		break;
> > > > +	case BUS_NOTIFY_BIND_DRIVER_ERROR:
> > > > +	case BUS_NOTIFY_UNBOUND_DRIVER:
> > > > +		pdev->dev.pm_domain = NULL;
> > > > +		break;
> > > > +	case BUS_NOTIFY_ADD_DEVICE:
> > > 
> > > This won't fix like revert of original commit does.
> 
> Which commit exactly are you talking about, for reference?

commit 01ac170ba29a9903ee590e1ef2d8e6b27b49a16c
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Wed Nov 5 18:34:46 2014 +0200

    ACPI / LPSS: allow to use specific PM domain during ->probe()
    

> > > 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.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy


  reply	other threads:[~2015-11-27  9:56 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-26 15:19 [PATCH v2 0/7] ACPI / LPSS: fix system hangup on BYT/BSW/CHT Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 1/7] device core: add BUS_NOTIFY_BIND_DRIVER_ERROR notification Andy Shevchenko
2015-11-26 23:09   ` Rafael J. Wysocki
2015-11-27  9:46     ` Andy Shevchenko
2015-11-27  9:46       ` Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 2/7] ACPI / LPSS: allow to use specific PM domain during ->probe() Andy Shevchenko
2015-11-26 16:30   ` Jarkko Nikula
2015-11-26 16:45     ` Andy Shevchenko
2015-11-26 16:45       ` Andy Shevchenko
2015-11-26 23:15       ` Rafael J. Wysocki
2015-11-27  9:56         ` Andy Shevchenko [this message]
2015-11-27  9:56           ` Andy Shevchenko
2015-12-03 19:29           ` Shevchenko, Andriy
2015-12-03 19:29             ` Shevchenko, Andriy
2015-12-04 13:04             ` Jarkko Nikula
2015-11-27  7:05       ` Jarkko Nikula
2015-11-27 10:01         ` Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 3/7] ACPI / LPSS: do delay for all LPSS devices when D3->D0 Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 4/7] ACPI / LPSS: override power state for LPSS DMA device Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 5/7] dmaengine: dw: platform: power on device on shutdown Andy Shevchenko
2015-11-26 17:01   ` Vinod Koul
2015-11-26 17:24     ` Andy Shevchenko
2015-11-26 17:24       ` Andy Shevchenko
2015-11-26 17:41       ` Vinod Koul
2015-11-26 17:41         ` Vinod Koul
2015-11-26 17:58         ` Andy Shevchenko
2015-11-26 17:58           ` Andy Shevchenko
2015-11-26 18:11           ` Shevchenko, Andriy
2015-11-26 18:11             ` Shevchenko, Andriy
2015-11-26 15:19 ` [PATCH v2 6/7] dmaengine: dw: return immediately from IRQ when DMA isn't in use Andy Shevchenko
2015-11-26 15:19 ` [PATCH v2 7/7] Revert "dmaengine: dw: platform: provide platform data for Intel" Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1448618173.15393.104.camel@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.