patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Mark Gross <markgross@kernel.org>
Cc: linux-kernel@vger.kernel.org, patches@lists.linux.dev,
	platform-driver-x86@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	Prashant Malani <pmalani@chromium.org>
Subject: [PATCH 1/3] platform/x86: intel_scu_ipc: Check status after timeouts in busy_loop()
Date: Wed, 30 Aug 2023 18:14:01 -0700	[thread overview]
Message-ID: <20230831011405.3246849-2-swboyd@chromium.org> (raw)
In-Reply-To: <20230831011405.3246849-1-swboyd@chromium.org>

It's possible for the polling loop in busy_loop() to get scheduled away
for a long time.

  status = ipc_read_status(scu);
  <long time scheduled away>
  if (!(status & IPC_STATUS_BUSY))

If this happens, then the status bit could change and this function
would never test it again after checking the jiffies against the timeout
limit. Polling code should check the condition one more time after the
timeout in case this happens.

The read_poll_timeout() helper implements this logic, and is shorter, so
simply use that helper here.

Cc: Prashant Malani <pmalani@chromium.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Fixes: e7b7ab3847c9 ("platform/x86: intel_scu_ipc: Sleeping is fine when polling")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/platform/x86/intel_scu_ipc.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 6851d10d6582..5a37becc65aa 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
@@ -231,19 +232,15 @@ static inline u32 ipc_data_readl(struct intel_scu_ipc_dev *scu, u32 offset)
 /* Wait till scu status is busy */
 static inline int busy_loop(struct intel_scu_ipc_dev *scu)
 {
-	unsigned long end = jiffies + IPC_TIMEOUT;
-
-	do {
-		u32 status;
-
-		status = ipc_read_status(scu);
-		if (!(status & IPC_STATUS_BUSY))
-			return (status & IPC_STATUS_ERR) ? -EIO : 0;
+	u8 status;
+	int err;
 
-		usleep_range(50, 100);
-	} while (time_before(jiffies, end));
+	err = read_poll_timeout(ipc_read_status, status, !(status & IPC_STATUS_BUSY),
+				100, jiffies_to_usecs(IPC_TIMEOUT), false, scu);
+	if (err)
+		return err;
 
-	return -ETIMEDOUT;
+	return (status & IPC_STATUS_ERR) ? -EIO : 0;
 }
 
 /* Wait till ipc ioc interrupt is received or timeout in 10 HZ */
-- 
https://chromeos.dev


  reply	other threads:[~2023-08-31  1:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31  1:14 [PATCH 0/3] platform/x86: intel_scu_ipc: Timeout fixes Stephen Boyd
2023-08-31  1:14 ` Stephen Boyd [this message]
2023-08-31 13:53   ` [PATCH 1/3] platform/x86: intel_scu_ipc: Check status after timeouts in busy_loop() Andy Shevchenko
2023-09-05 22:24     ` Stephen Boyd
2023-09-06 13:58       ` Andy Shevchenko
2023-08-31 14:15   ` Kuppuswamy Sathyanarayanan
2023-09-01  5:50   ` Mika Westerberg
2023-09-05 22:27     ` Stephen Boyd
2023-09-06 13:46       ` Andy Shevchenko
2023-09-06 14:31         ` Mika Westerberg
2023-08-31  1:14 ` [PATCH 2/3] platform/x86: intel_scu_ipc: Check status upon timeout in ipc_wait_for_interrupt() Stephen Boyd
2023-08-31 13:58   ` Andy Shevchenko
2023-09-05 22:36     ` Stephen Boyd
2023-08-31 14:27   ` Kuppuswamy Sathyanarayanan
2023-09-05 22:56     ` Stephen Boyd
2023-09-01  6:04   ` Mika Westerberg
2023-08-31  1:14 ` [PATCH 3/3] platform/x86: intel_scu_ipc: Fail IPC send if still busy Stephen Boyd
2023-08-31 14:07   ` Andy Shevchenko
2023-09-01  6:06     ` Mika Westerberg
2023-09-05 22:42       ` Stephen Boyd
2023-08-31  3:28 ` [PATCH 0/3] platform/x86: intel_scu_ipc: Timeout fixes Kuppuswamy Sathyanarayanan
2023-09-05 22:55   ` Stephen Boyd

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=20230831011405.3246849-2-swboyd@chromium.org \
    --to=swboyd@chromium.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=patches@lists.linux.dev \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=pmalani@chromium.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.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;
as well as URLs for NNTP newsgroup(s).