All of lore.kernel.org
 help / color / mirror / Atom feed
From: peter.chen@freescale.com (Peter Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 01/11] usb: chipidea: Add power management support
Date: Tue, 15 Oct 2013 10:18:15 +0800	[thread overview]
Message-ID: <20131015021814.GA3254@shlinux1.ap.freescale.net> (raw)
In-Reply-To: <20131014110108.GL25034@n2100.arm.linux.org.uk>

On Mon, Oct 14, 2013 at 12:01:08PM +0100, Russell King - ARM Linux wrote:
> On Sat, Oct 12, 2013 at 05:35:03PM +0800, Peter Chen wrote:
> > This commit adds runtime and system power management support for
> > chipidea core. The runtime pm support is controlled by glue
> > layer, it can be enabled by flag CI_HDRC_SUPPORTS_RUNTIME_PM.
> 
> Let's look at the locking.
> 
> 1. Runtime PM.  These callbacks are locked with a spinlock, which holds
>    dev->power.lock.  This lock is taken either with or without disabling
>    IRQs depending on whether runtime PM is IRQ safe or not.
> 
> 2. Normal PM.  These callbacks are locked by holding dev->mutex.
> 
> Now, there's a little bit of protection between these two operations -
> when normal PM places a device into a low power state, it 'gets' a
> reference on the runtime PM to ensure no runtime PM transitions occur
> while normal PM is active.  (See pm_runtime_get_noresume() in
> device_prepare().)  This is only dropped when the normal PM resumes the
> device.
> 
> Moreover, all runtime PM events are flushed before the suspend callback
> occurs (see the pm_runtime_barrier() in __device_suspend()).
> 
> What that means is that you can't receive any runtime PM events while
> you are in your suspend/resume callbacks.  So, each call is mutually
> exclusive.
> 
> So, runtime PM callbacks vs normal PM callbacks for any single device
> are all called with mutual exclusion - you won't have two running at
> any time.
> 
> Hence, for the reasons stated previously about the non-atomic nature of
> atomic_read()/atomic_set(), there's even more reasons that their use
> here is just mere obfuscation: the accesses to this state tracking
> variable is already guaranteed to be single-threaded by core code, so
> the use of atomic_read()/atomic_set() just adds additional needless
> confusion to this code.
> 

Many Thanks, Russell.

So, the lessons for this topic are:

- If one atomic variable's operation only includes one instruction like
atomic_read and atomic_set, it is not meaningful for using atomic
operation, we can just use bool instead of it.

- The runtime pm itself, normal pm itself, runtime pm and normal pm
are all already exclusion, we don't need protection for variables
who are only used at pm situation.

-- 

Best Regards,
Peter Chen

  reply	other threads:[~2013-10-15  2:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-12  9:35 [PATCH 00/11] Add power management support for chipidea Peter Chen
2013-10-12  9:35 ` [PATCH 01/11] usb: chipidea: Add power management support Peter Chen
2013-10-14  8:04   ` Lothar Waßmann
2013-10-14  7:55     ` Peter Chen
2013-10-14  8:42       ` Sascha Hauer
2013-10-14  9:04         ` Peter Chen
2013-10-14 10:23           ` Sascha Hauer
2013-10-14 10:46           ` Russell King - ARM Linux
2013-10-14 10:44       ` Russell King - ARM Linux
2013-10-14 11:01   ` Russell King - ARM Linux
2013-10-15  2:18     ` Peter Chen [this message]
2013-10-15 11:15       ` Russell King - ARM Linux
2013-10-12  9:35 ` [PATCH 02/11] usb: chipidea: imx: add " Peter Chen
2013-10-12  9:35 ` [PATCH 03/11] usb: chipidea: usbmisc_imx: remove the controller's clock information Peter Chen
2013-10-12  9:35 ` [PATCH 04/11] usb: chipidea: add wakeup interrupt handler Peter Chen
2013-10-12  9:35 ` [PATCH 05/11] usb: chipidea: usbmisc_imx: add set_wakup API Peter Chen
2013-10-12  9:35 ` [PATCH 06/11] usb: chipidea: imx: call set_wakeup when necessary Peter Chen
2013-10-12  9:35 ` [PATCH 07/11] usb: chipidea: host: add quirk for ehci operation Peter Chen
2013-10-12  9:35 ` [PATCH 08/11] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
2013-10-12  9:35 ` [PATCH 09/11] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs Peter Chen
2013-10-12  9:35 ` [PATCH 10/11] usb: chipidea: imx: add binding for supporting runtime pm Peter Chen
2013-10-12 14:40   ` Alan Stern
2013-10-14  1:22     ` Peter Chen
2013-10-14  1:39       ` Marek Vasut
2013-10-14  1:33         ` Peter Chen
2013-10-12  9:35 ` [PATCH 11/11] ARM: dts: imx6qdl-sabresd: Enable runtime pm for usbotg and usb host 1 Peter Chen

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=20131015021814.GA3254@shlinux1.ap.freescale.net \
    --to=peter.chen@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.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.