public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neil@brown.name>
To: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>,
	linux-kernel@vger.kernel.org,
	GTA04 owners <gta04-owner@goldelico.com>,
	NeilBrown <neilb@suse.de>, Pavel Machek <pavel@ucw.cz>,
	linux-omap@vger.kernel.org
Subject: [PATCH 1/6] phy: twl4030-usb: make runtime pm more reliable.
Date: Thu, 16 Apr 2015 18:03:04 +1000	[thread overview]
Message-ID: <20150416080304.23307.91491.stgit@notabene.brown> (raw)
In-Reply-To: <20150416075945.23307.24424.stgit@notabene.brown>

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

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 bc42d6a8939f..3078f80bf520 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)
+{
+	return stat == OMAP_MUSB_VBUS_VALID ||
+		stat == OMAP_MUSB_ID_GROUND;
+}
+
 struct twl4030_usb {
 	struct usb_phy		phy;
 	struct device		*dev;
@@ -536,8 +546,10 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 
 	mutex_lock(&twl->lock);
 	if (status >= 0 && status != twl->linkstat) {
+		status_changed =
+			cable_present(twl->linkstat) !=
+			cable_present(status);
 		twl->linkstat = status;
-		status_changed = true;
 	}
 	mutex_unlock(&twl->lock);
 
@@ -553,15 +565,11 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 		 * USB_LINK_VBUS state.  musb_hdrc won't care until it
 		 * starts to handle softconnect right.
 		 */
-		if ((status == OMAP_MUSB_VBUS_VALID) ||
-		    (status == OMAP_MUSB_ID_GROUND)) {
-			if (pm_runtime_suspended(twl->dev))
-				pm_runtime_get_sync(twl->dev);
+		if (cable_present(status)) {
+			pm_runtime_get_sync(twl->dev);
 		} else {
-			if (pm_runtime_active(twl->dev)) {
-				pm_runtime_mark_last_busy(twl->dev);
-				pm_runtime_put_autosuspend(twl->dev);
-			}
+			pm_runtime_mark_last_busy(twl->dev);
+			pm_runtime_put_autosuspend(twl->dev);
 		}
 		omap_musb_mailbox(status);
 	}
@@ -767,6 +775,9 @@ static int twl4030_usb_remove(struct platform_device *pdev)
 
 	/* disable complete OTG block */
 	twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
+
+	if (cable_present(twl->linkstat))
+		pm_runtime_put_noidle(twl->dev);
 	pm_runtime_mark_last_busy(twl->dev);
 	pm_runtime_put(twl->dev);
 



  parent reply	other threads:[~2015-04-16  8:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-16  8:03 [PATCH 0/6] Enhancements to twl4030 phy to support better charging NeilBrown
2015-04-16  8:03 ` [PATCH 3/6] phy: twl4030-usb: remove incorrect pm_runtime_get_sync() in probe function NeilBrown
2015-04-16  8:03 ` NeilBrown [this message]
2015-04-16  8:03 ` [PATCH 5/6] phy: twl4030-usb: add support for reading resistor on ID pin NeilBrown
2015-06-01 13:36   ` Kishon Vijay Abraham I
2015-06-01 21:37     ` NeilBrown
2015-06-02 13:49       ` Kishon Vijay Abraham I
2015-06-02 14:06         ` [Gta04-owner] " Dr. H. Nikolaus Schaller
2015-06-02 20:11           ` Pavel Machek
2015-06-02 20:47             ` Dr. H. Nikolaus Schaller
2015-06-06 13:10       ` Pavel Machek
2015-06-08  3:45         ` Felipe Balbi
2015-06-23  9:09       ` [Gta04-owner] " Dr. H. Nikolaus Schaller
2015-04-16  8:03 ` [PATCH 6/6] phy: twl4030-usb: add extcon to report cable connections NeilBrown
2015-05-11 13:38   ` Kishon Vijay Abraham I
2015-05-11 15:58     ` Chanwoo Choi
2015-04-16  8:03 ` [PATCH 2/6] phy: twl4030-usb: remove pointless 'suspended' test in 'suspend' callback NeilBrown
2015-04-16  8:03 ` [PATCH 4/6] phy: twl4030-usb: add ABI documentation NeilBrown
2015-04-17 22:14   ` Pavel Machek
2015-04-17 22:34     ` 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=20150416080304.23307.91491.stgit@notabene.brown \
    --to=neil@brown.name \
    --cc=gta04-owner@goldelico.com \
    --cc=kishon@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=pavel@ucw.cz \
    --cc=tony@atomide.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox