[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0iU5Qqbm6LDNmm=vj_H_sXRasdFN9+xBGto_-eZG6tXrQ@mail.gmail.com>
Date: Thu, 21 Jun 2018 14:02:00 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Roger Quadros <rogerq@...com>
Cc: Samuel Morris <samorris@...mark.com>,
Tony Lindgren <tony@...mide.com>,
Naresh Kamboju <naresh.kamboju@...aro.org>,
Tejun Heo <tj@...nel.org>,
Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
Linux-Next Mailing List <linux-next@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
Arnd Bergmann <arnd@...db.de>,
Santosh Shilimkar <ssantosh@...nel.org>,
Olof Johansson <olof@...om.net>, Tero Kristo <t-kristo@...com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Linux PM <linux-pm@...r.kernel.org>
Subject: Re: 4.18.0-rc1-next-20180619 boot failed on beagle board x15
On Thu, Jun 21, 2018 at 9:58 AM, Roger Quadros <rogerq@...com> wrote:
> +Rafael
>
> On 20/06/18 18:30, Samuel Morris wrote:
>> On Wed, Jun 20, 2018 at 8:58 AM, Roger Quadros <rogerq@...com> wrote:
>>> Tony,
>>>
>>> On 20/06/18 13:29, Tony Lindgren wrote:
>>>> Hi,
>>>>
>>>> * Naresh Kamboju <naresh.kamboju@...aro.org> [180620 05:55]:
>>>>> Linux next (4.18.0-rc1-next-20180619) boot failed on beagle board x15.
>>>>
>>>> Bisect points to commit aece27a2f01b ("ata: ahci_platform: allow disabling of
>>>> hotplug to save power").
>>>>
>>>> Reverting the patch makes things work again. Any ideas what
>>>> might be going wrong here? Things clearly idle but then there
>>>> seems to be some register access with clocks disabled.
>>>
>>> The commit is doing this in probe.
>>>
>>> + pm_runtime_set_active(dev);
>>> pm_runtime_enable(dev);
>>> - pm_runtime_get_sync(dev);
>>> + pm_runtime_forbid(dev);
>>>
>>> On OMAP, the device is not guaranteed to be active at probe and so we can't
>>> say pm_runtime_set_active() and get rid of pm_runtime_get_sync().
>>
>> Okay, by calling set_active(), I'm preventing the rpm_resume from
>> completing that would normally happen in pm_runtime_forbid(). I assume
>> you mean that there are parent devices that need to be resumed before
>
> Actually, in the OMAP case, the AHCI controller device isn't active when probe is called.
> For other platforms this might not be the case. So we need to be careful here.
>
>> this device may be assumed active. I'm going to try removing the
>> set_active(), then move that clause to the end of
>> ahci_platform_init_host(). The pm_runtime_forbid() is effectively the
>> same as get_sync(), it just also sets the runtime_auto flag to false.
>> I don't think we should be saying the device is active until the host
>> is initialized, so that seems like a better, common place for the
>> pm_runtime init callbacks anyway. How does that sound?
>
> Device active and initialized are different things. If the device is powered up
> and can be accessed it is active, even if it is not yet initialized.
> I don't think we should club the two.
>
> Why do you need to call pm_runtime_set_active() at all in the probe sequence?
>
> Documentation for pm_runtime_set_active() says,
> "(it is only valid to use this function if 'power.runtime_error' is set
> or 'power.disable_depth' is greater than zero);"
I guess on some platforms the AHCI controller actually is active
initially and that's a matter of setting the initial status to reflect
the real situation.
If different things can happen on different platforms, there needs to
be a way to discover the initial state instead of making assumptions
on it.
Thanks,
Rafael
Powered by blists - more mailing lists