Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] UCSI driver fixes
@ 2019-10-04 10:02 Heikki Krogerus
  2019-10-04 10:02 ` [PATCH 1/2] usb: typec: ucsi: ccg: Remove run_isr flag Heikki Krogerus
  2019-10-04 10:02 ` [PATCH 2/2] usb: typec: ucsi: displayport: Fix for the mode entering routine Heikki Krogerus
  0 siblings, 2 replies; 3+ messages in thread
From: Heikki Krogerus @ 2019-10-04 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Ajay Gupta, linux-usb

Hi Greg,

Here are two fixes for the ucsi drivers. The first patch removes a
potential deadlock from ucsi_ccg.c, and the second fixes
ucsi_displayport_enter() function so that it does not return error if
the mode has already been entered by the time the function is called.

Let me know if you want anything to be changed.

thanks,

Heikki Krogerus (2):
  usb: typec: ucsi: ccg: Remove run_isr flag
  usb: typec: ucsi: displayport: Fix for the mode entering routine

 drivers/usb/typec/ucsi/displayport.c |  2 ++
 drivers/usb/typec/ucsi/ucsi_ccg.c    | 42 +++-------------------------
 2 files changed, 6 insertions(+), 38 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] usb: typec: ucsi: ccg: Remove run_isr flag
  2019-10-04 10:02 [PATCH 0/2] UCSI driver fixes Heikki Krogerus
@ 2019-10-04 10:02 ` Heikki Krogerus
  2019-10-04 10:02 ` [PATCH 2/2] usb: typec: ucsi: displayport: Fix for the mode entering routine Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2019-10-04 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Ajay Gupta, linux-usb, stable

The "run_isr" flag is used for preventing the driver from
calling the interrupt service routine in its runtime resume
callback when the driver is expecting completion to a
command, but what that basically does is that it hides the
real problem. The real problem is that the controller is
allowed to suspend in the middle of command execution.

As a more appropriate fix for the problem, using autosuspend
delay time that matches UCSI_TIMEOUT_MS (5s). That prevents
the controller from suspending while still in the middle of
executing a command.

This fixes a potential deadlock. Both ccg_read() and
ccg_write() are called with the mutex already taken at least
from ccg_send_command(). In ccg_read() and ccg_write, the
mutex is only acquired so that run_isr flag can be set.

Fixes: f0e4cd948b91 ("usb: typec: ucsi: ccg: add runtime pm workaround")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 42 +++----------------------------
 1 file changed, 4 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 907e20e1a71e..d772fce51905 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -195,7 +195,6 @@ struct ucsi_ccg {
 
 	/* fw build with vendor information */
 	u16 fw_build;
-	bool run_isr; /* flag to call ISR routine during resume */
 	struct work_struct pm_work;
 };
 
@@ -224,18 +223,6 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 	if (quirks && quirks->max_read_len)
 		max_read_len = quirks->max_read_len;
 
-	if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-	    uc->fw_version <= CCG_OLD_FW_VERSION) {
-		mutex_lock(&uc->lock);
-		/*
-		 * Do not schedule pm_work to run ISR in
-		 * ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-		 * since we are already in ISR path.
-		 */
-		uc->run_isr = false;
-		mutex_unlock(&uc->lock);
-	}
-
 	pm_runtime_get_sync(uc->dev);
 	while (rem_len > 0) {
 		msgs[1].buf = &data[len - rem_len];
@@ -278,18 +265,6 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 	msgs[0].len = len + sizeof(rab);
 	msgs[0].buf = buf;
 
-	if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-	    uc->fw_version <= CCG_OLD_FW_VERSION) {
-		mutex_lock(&uc->lock);
-		/*
-		 * Do not schedule pm_work to run ISR in
-		 * ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-		 * since we are already in ISR path.
-		 */
-		uc->run_isr = false;
-		mutex_unlock(&uc->lock);
-	}
-
 	pm_runtime_get_sync(uc->dev);
 	status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
 	if (status < 0) {
@@ -1130,7 +1105,6 @@ static int ucsi_ccg_probe(struct i2c_client *client,
 	uc->ppm.sync = ucsi_ccg_sync;
 	uc->dev = dev;
 	uc->client = client;
-	uc->run_isr = true;
 	mutex_init(&uc->lock);
 	INIT_WORK(&uc->work, ccg_update_firmware);
 	INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
@@ -1188,6 +1162,8 @@ static int ucsi_ccg_probe(struct i2c_client *client,
 
 	pm_runtime_set_active(uc->dev);
 	pm_runtime_enable(uc->dev);
+	pm_runtime_use_autosuspend(uc->dev);
+	pm_runtime_set_autosuspend_delay(uc->dev, 5000);
 	pm_runtime_idle(uc->dev);
 
 	return 0;
@@ -1229,7 +1205,6 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct ucsi_ccg *uc = i2c_get_clientdata(client);
-	bool schedule = true;
 
 	/*
 	 * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue
@@ -1237,17 +1212,8 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
 	 * Schedule a work to call ISR as a workaround.
 	 */
 	if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-	    uc->fw_version <= CCG_OLD_FW_VERSION) {
-		mutex_lock(&uc->lock);
-		if (!uc->run_isr) {
-			uc->run_isr = true;
-			schedule = false;
-		}
-		mutex_unlock(&uc->lock);
-
-		if (schedule)
-			schedule_work(&uc->pm_work);
-	}
+	    uc->fw_version <= CCG_OLD_FW_VERSION)
+		schedule_work(&uc->pm_work);
 
 	return 0;
 }
-- 
2.23.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] usb: typec: ucsi: displayport: Fix for the mode entering routine
  2019-10-04 10:02 [PATCH 0/2] UCSI driver fixes Heikki Krogerus
  2019-10-04 10:02 ` [PATCH 1/2] usb: typec: ucsi: ccg: Remove run_isr flag Heikki Krogerus
@ 2019-10-04 10:02 ` Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2019-10-04 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Ajay Gupta, linux-usb, stable

Making sure that ucsi_displayport_enter() function does not
return an error if the displayport alternate mode has
already been entered. It's normal that the firmware (or
controller) has already entered the alternate mode by the
time the operating system is notified about the device.

Fixes: af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/displayport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 6c103697c582..d99700cb4dca 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -75,6 +75,8 @@ static int ucsi_displayport_enter(struct typec_altmode *alt)
 
 	if (cur != 0xff) {
 		mutex_unlock(&dp->con->lock);
+		if (dp->con->port_altmode[cur] == alt)
+			return 0;
 		return -EBUSY;
 	}
 
-- 
2.23.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 10:02 [PATCH 0/2] UCSI driver fixes Heikki Krogerus
2019-10-04 10:02 ` [PATCH 1/2] usb: typec: ucsi: ccg: Remove run_isr flag Heikki Krogerus
2019-10-04 10:02 ` [PATCH 2/2] usb: typec: ucsi: displayport: Fix for the mode entering routine Heikki Krogerus

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git