All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: Len Brown <len.brown@intel.com>,
	linux-samsung-soc@vger.kernel.org,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	linux-pm@vger.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@linaro.org>, Pavel Machek <pavel@ucw.cz>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
Date: Mon, 17 Nov 2014 18:54:44 +0200	[thread overview]
Message-ID: <546A2854.3020204@ti.com> (raw)
In-Reply-To: <20141117153257.GS4042@n2100.arm.linux.org.uk>

Hi Russell,

On 11/17/2014 05:32 PM, Russell King - ARM Linux wrote:
> On Mon, Nov 17, 2014 at 04:19:10PM +0100, Ulf Hansson wrote:
>> The amba bus, amba drivers and a vast amount of platform drivers which
>> enables runtime PM, don't invoke a pm_runtime_get_sync() while probing
>> their devices.
>>
>> Instead, once they have turned on their PM resourses during ->probe()
>> and are ready to handle I/O, these invokes pm_runtime_set_active() to
>> synchronize its state towards the runtime PM core.
> 
> The above is misleading for amba.  The code sequence is:
> 
>                  pm_runtime_get_noresume(dev);
>                  pm_runtime_set_active(dev);
>                  pm_runtime_enable(dev);
> 
>                  ret = pcdrv->probe(pcdev, id);
> 
> AMBA drivers should never call pm_runtime_set_active(), as the runtime PM
> state has already been initialised by the bus code.  Platform drivers are
> different; the platform code provides zero help for runtime PM.
> 
> The sequence used by AMBA bus code is the sequence which was used by PCI
> (as per commit f3ec4f87d607) at the time the runtime PM support was
> written for AMBA.  PCI assumes that unbound devices are already powered
> up, and as far as I'm aware, that's also true of AMBA devices as well.
> I have yet to have access to a platform where this isn't true, neither
> has anyone reported that such a platform exists.
> 

I'd be very appreciated if you would be able to clarify one point to me as
I'm not familiar with amba hw?

I've found at least 2 AMBA drivers where secondary clock is used 
to enable/disable device in addition to "apb_pclk":
 - drivers/mmc/host/mmci.c 
 - drivers/spi/spi-pl022.c
So, form the code point of view, the assumption that "unbound AMBA devices are
already powered up" is not always true. And "apb_pclk" is just an interface clock
in such cases. 
>From another side above statement is true for mailbox/pl320-ipc.c which seems like
is simple device.
Am I wrong?

Thanks in advance. 

regards,
-grygorii


WARNING: multiple messages have this Message-ID (diff)
From: grygorii.strashko@ti.com (Grygorii Strashko)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PM / Domains: Power on the PM domain right after attach completes
Date: Mon, 17 Nov 2014 18:54:44 +0200	[thread overview]
Message-ID: <546A2854.3020204@ti.com> (raw)
In-Reply-To: <20141117153257.GS4042@n2100.arm.linux.org.uk>

Hi Russell,

On 11/17/2014 05:32 PM, Russell King - ARM Linux wrote:
> On Mon, Nov 17, 2014 at 04:19:10PM +0100, Ulf Hansson wrote:
>> The amba bus, amba drivers and a vast amount of platform drivers which
>> enables runtime PM, don't invoke a pm_runtime_get_sync() while probing
>> their devices.
>>
>> Instead, once they have turned on their PM resourses during ->probe()
>> and are ready to handle I/O, these invokes pm_runtime_set_active() to
>> synchronize its state towards the runtime PM core.
> 
> The above is misleading for amba.  The code sequence is:
> 
>                  pm_runtime_get_noresume(dev);
>                  pm_runtime_set_active(dev);
>                  pm_runtime_enable(dev);
> 
>                  ret = pcdrv->probe(pcdev, id);
> 
> AMBA drivers should never call pm_runtime_set_active(), as the runtime PM
> state has already been initialised by the bus code.  Platform drivers are
> different; the platform code provides zero help for runtime PM.
> 
> The sequence used by AMBA bus code is the sequence which was used by PCI
> (as per commit f3ec4f87d607) at the time the runtime PM support was
> written for AMBA.  PCI assumes that unbound devices are already powered
> up, and as far as I'm aware, that's also true of AMBA devices as well.
> I have yet to have access to a platform where this isn't true, neither
> has anyone reported that such a platform exists.
> 

I'd be very appreciated if you would be able to clarify one point to me as
I'm not familiar with amba hw?

I've found at least 2 AMBA drivers where secondary clock is used 
to enable/disable device in addition to "apb_pclk":
 - drivers/mmc/host/mmci.c 
 - drivers/spi/spi-pl022.c
So, form the code point of view, the assumption that "unbound AMBA devices are
already powered up" is not always true. And "apb_pclk" is just an interface clock
in such cases. 
>From another side above statement is true for mailbox/pl320-ipc.c which seems like
is simple device.
Am I wrong?

Thanks in advance. 

regards,
-grygorii

  reply	other threads:[~2014-11-17 16:55 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 15:19 [PATCH] PM / Domains: Power on the PM domain right after attach completes Ulf Hansson
2014-11-17 15:19 ` Ulf Hansson
2014-11-17 15:32 ` Russell King - ARM Linux
2014-11-17 15:32   ` Russell King - ARM Linux
2014-11-17 16:54   ` Grygorii Strashko [this message]
2014-11-17 16:54     ` Grygorii Strashko
2014-11-17 17:07     ` Russell King - ARM Linux
2014-11-17 17:07       ` Russell King - ARM Linux
2014-11-17 18:28 ` Kevin Hilman
2014-11-17 18:28   ` Kevin Hilman
2014-11-17 19:06   ` Alan Stern
2014-11-17 19:06     ` Alan Stern
2014-11-17 19:17     ` Dmitry Torokhov
2014-11-17 19:17       ` Dmitry Torokhov
2014-11-17 19:54       ` Alan Stern
2014-11-17 19:54         ` Alan Stern
2014-11-17 20:28         ` Dmitry Torokhov
2014-11-17 20:28           ` Dmitry Torokhov
2014-11-17 20:49           ` Alan Stern
2014-11-17 20:49             ` Alan Stern
2014-11-17 21:11             ` Dmitry Torokhov
2014-11-17 21:11               ` Dmitry Torokhov
2014-11-17 21:44               ` Alan Stern
2014-11-17 21:44                 ` Alan Stern
2014-11-17 22:02                 ` Dmitry Torokhov
2014-11-17 22:02                   ` Dmitry Torokhov
2014-11-17 22:12                   ` Alan Stern
2014-11-17 22:12                     ` Alan Stern
2014-11-17 22:17                     ` Dmitry Torokhov
2014-11-17 22:17                       ` Dmitry Torokhov
2014-11-17 23:28                       ` Rafael J. Wysocki
2014-11-17 23:28                         ` Rafael J. Wysocki
2014-11-17 23:26                         ` Dmitry Torokhov
2014-11-17 23:26                           ` Dmitry Torokhov
2014-11-18  0:26                           ` Rafael J. Wysocki
2014-11-18  0:26                             ` Rafael J. Wysocki
2014-11-18  2:16                             ` Rafael J. Wysocki
2014-11-18  2:16                               ` Rafael J. Wysocki
2014-11-18 14:05                               ` Ulf Hansson
2014-11-18 14:05                                 ` Ulf Hansson
2014-11-18 20:29                                 ` Rafael J. Wysocki
2014-11-18 20:29                                   ` Rafael J. Wysocki
2014-11-19  8:54                                   ` Ulf Hansson
2014-11-19  8:54                                     ` Ulf Hansson
2014-11-20  0:35                                     ` Rafael J. Wysocki
2014-11-20  0:35                                       ` Rafael J. Wysocki
2014-11-20 10:13                                       ` Ulf Hansson
2014-11-20 10:13                                         ` Ulf Hansson
2014-11-20 20:56                                         ` Rafael J. Wysocki
2014-11-20 20:56                                           ` Rafael J. Wysocki
2014-11-20 12:17                                     ` Grygorii Strashko
2014-11-20 12:17                                       ` Grygorii Strashko
2014-11-20 13:01                                       ` Ulf Hansson
2014-11-20 13:01                                         ` Ulf Hansson
2014-11-20 15:06                                         ` Grygorii Strashko
2014-11-20 15:06                                           ` Grygorii Strashko
2014-11-18 16:13                       ` Alan Stern
2014-11-18 16:13                         ` Alan Stern
2014-11-18 17:18                         ` Dmitry Torokhov
2014-11-18 17:18                           ` Dmitry Torokhov
2014-11-18 17:44                           ` Alan Stern
2014-11-18 17:44                             ` Alan Stern
2014-11-18 17:55                             ` Dmitry Torokhov
2014-11-18 17:55                               ` Dmitry Torokhov
2014-11-18 20:14                               ` Rafael J. Wysocki
2014-11-18 20:14                                 ` Rafael J. Wysocki
2014-11-18 20:04                                 ` Dmitry Torokhov
2014-11-18 20:04                                   ` Dmitry Torokhov
2014-11-18 21:03                                   ` Rafael J. Wysocki
2014-11-18 21:03                                     ` Rafael J. Wysocki
2014-11-18 21:17                                     ` Rafael J. Wysocki
2014-11-18 21:17                                       ` Rafael J. Wysocki
2014-11-18 21:02                                       ` Dmitry Torokhov
2014-11-18 21:02                                         ` Dmitry Torokhov
2014-11-18 21:58                                         ` Rafael J. Wysocki
2014-11-18 21:58                                           ` Rafael J. Wysocki
2014-11-18 21:44                                           ` Dmitry Torokhov
2014-11-18 21:44                                             ` Dmitry Torokhov
2014-11-18 22:10                                           ` Rafael J. Wysocki
2014-11-18 22:10                                             ` Rafael J. Wysocki

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=546A2854.3020204@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=khilman@linaro.org \
    --cc=len.brown@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=s.nawrocki@samsung.com \
    --cc=ulf.hansson@linaro.org \
    /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.