From: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
To: NeilBrown <neil-+NVA1uvv1dVBDLzU/O5InQ@public.gmane.org>
Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
GTA04 owners
<gta04-owner-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>,
NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>,
Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Subject: Re: [PATCH 1/5] usb: phy: twl4030: make runtime pm more reliable.
Date: Thu, 26 Mar 2015 03:02:21 +0530 [thread overview]
Message-ID: <55132965.3090002@ti.com> (raw)
In-Reply-To: <20150322223523.21765.3199.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
Hi,
On Monday 23 March 2015 04:05 AM, NeilBrown wrote:
> From: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
>
> A construct like:
>
> if (pm_runtime_suspended(twl->dev))
> pm_runtime_get_sync(twl->dev);
>
> is against the spirit of the runtime_pm interface as it
> makes the internal refcounting useless.
>
> In this case it is also racy, particularly as 'put_autosuspend'
> is use to drop a reference.
> When that happens a timer is started and the device is
> runtime-suspended after the timeout.
> If the above code runs in this window, the device will not be
> found to be suspended so no pm_runtime reference is taken.
> When the timer expires the device will be suspended, which is
> against the intention of the code.
>
> So be more direct is taking and dropping references.
> If twl->linkstat is VBUS_VALID or ID_GROUND, then hold a
> pm_runtime reference, otherwise don't.
> Define "cable_present()" to test for this condition.
minor comment below..
>
> Tested-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
> ---
> drivers/phy/phy-twl4030-usb.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> index 8e87f54671f3..1a244f34b748 100644
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -144,6 +144,16 @@
> #define PMBR1 0x0D
> #define GPIO_USB_4PIN_ULPI_2430C (3 << 0)
>
> +/*
> + * If VBUS is valid or ID is ground, then we know a
> + * cable is present and we need to be runtime-enabled
> + */
> +static inline bool cable_present(enum omap_musb_vbus_id_status stat)
twl4030_cable_present?
-Kishon
WARNING: multiple messages have this Message-ID (diff)
From: Kishon Vijay Abraham I <kishon@ti.com>
To: NeilBrown <neil@brown.name>
Cc: Tony Lindgren <tony@atomide.com>, <linux-api@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
GTA04 owners <gta04-owner@goldelico.com>,
NeilBrown <neilb@suse.de>, Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH 1/5] usb: phy: twl4030: make runtime pm more reliable.
Date: Thu, 26 Mar 2015 03:02:21 +0530 [thread overview]
Message-ID: <55132965.3090002@ti.com> (raw)
In-Reply-To: <20150322223523.21765.3199.stgit@notabene.brown>
Hi,
On Monday 23 March 2015 04:05 AM, NeilBrown wrote:
> From: NeilBrown <neilb@suse.de>
>
> A construct like:
>
> if (pm_runtime_suspended(twl->dev))
> pm_runtime_get_sync(twl->dev);
>
> is against the spirit of the runtime_pm interface as it
> makes the internal refcounting useless.
>
> In this case it is also racy, particularly as 'put_autosuspend'
> is use to drop a reference.
> When that happens a timer is started and the device is
> runtime-suspended after the timeout.
> If the above code runs in this window, the device will not be
> found to be suspended so no pm_runtime reference is taken.
> When the timer expires the device will be suspended, which is
> against the intention of the code.
>
> So be more direct is taking and dropping references.
> If twl->linkstat is VBUS_VALID or ID_GROUND, then hold a
> pm_runtime reference, otherwise don't.
> Define "cable_present()" to test for this condition.
minor comment below..
>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> drivers/phy/phy-twl4030-usb.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> index 8e87f54671f3..1a244f34b748 100644
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -144,6 +144,16 @@
> #define PMBR1 0x0D
> #define GPIO_USB_4PIN_ULPI_2430C (3 << 0)
>
> +/*
> + * If VBUS is valid or ID is ground, then we know a
> + * cable is present and we need to be runtime-enabled
> + */
> +static inline bool cable_present(enum omap_musb_vbus_id_status stat)
twl4030_cable_present?
-Kishon
next prev parent reply other threads:[~2015-03-25 21:32 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-22 22:35 [PATCH 0/5] Enhancements to twl4030 phy to support better charging - V2 NeilBrown
2015-03-22 22:35 ` NeilBrown
[not found] ` <20150322223307.21765.62974.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2015-03-22 22:35 ` [PATCH 2/5] usb: phy: twl4030: allow charger to see usb current draw limits NeilBrown
2015-03-22 22:35 ` NeilBrown
2015-03-22 22:35 ` [PATCH 1/5] usb: phy: twl4030: make runtime pm more reliable NeilBrown
2015-03-22 22:35 ` NeilBrown
[not found] ` <20150322223523.21765.3199.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2015-03-25 21:32 ` Kishon Vijay Abraham I [this message]
2015-03-25 21:32 ` Kishon Vijay Abraham I
2015-03-26 6:39 ` Pavel Machek
2015-03-23 21:25 ` [PATCH 0/5] Enhancements to twl4030 phy to support better charging - V2 Pavel Machek
2015-03-23 21:25 ` Pavel Machek
2015-03-25 21:16 ` Kishon Vijay Abraham I
2015-03-25 21:16 ` Kishon Vijay Abraham I
2015-03-25 21:22 ` NeilBrown
2015-03-25 21:22 ` NeilBrown
[not found] ` <20150326082219.510ac598-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2015-03-25 23:59 ` Kishon Vijay Abraham I
2015-03-25 23:59 ` Kishon Vijay Abraham I
[not found] ` <55134BEE.7050406-l0cyMroinI0@public.gmane.org>
2015-03-26 0:16 ` NeilBrown
2015-03-26 0:16 ` NeilBrown
2015-04-01 4:41 ` NeilBrown
2015-04-01 4:41 ` NeilBrown
[not found] ` <20150401154102.5f57ec1e-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2015-04-03 13:38 ` Kishon Vijay Abraham I
2015-04-03 13:38 ` Kishon Vijay Abraham I
[not found] ` <551E97CE.4000501-l0cyMroinI0@public.gmane.org>
2015-04-04 0:28 ` NeilBrown
2015-04-04 0:28 ` NeilBrown
[not found] ` <20150404112816.025d233b-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2015-04-14 10:54 ` Kishon Vijay Abraham I
2015-04-14 10:54 ` Kishon Vijay Abraham I
2015-03-22 22:35 ` [PATCH 5/5] usb: phy: twl4030: test ID resistance to see if charger is present NeilBrown
2015-03-22 22:35 ` [PATCH 4/5] usb: phy: twl4030: add support for reading restore on ID pin NeilBrown
2015-03-22 22:35 ` [PATCH 3/5] usb: phy: twl4030: add ABI documentation NeilBrown
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=55132965.3090002@ti.com \
--to=kishon-l0cymroini0@public.gmane.org \
--cc=gta04-owner-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org \
--cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=neil-+NVA1uvv1dVBDLzU/O5InQ@public.gmane.org \
--cc=neilb-l3A5Bk7waGM@public.gmane.org \
--cc=pavel-+ZI9xUNit7I@public.gmane.org \
--cc=tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.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.