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 v4 1/4] platform/x86: intel_scu_ipc: Check status after timeout in busy_loop()
Date: Wed, 13 Sep 2023 14:27:19 -0700 [thread overview]
Message-ID: <20230913212723.3055315-2-swboyd@chromium.org> (raw)
In-Reply-To: <20230913212723.3055315-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); // status = IPC_STATUS_BUSY
<long time scheduled away>
if (!(status & IPC_STATUS_BUSY))
If this happens, then the status bit could change while the task is
scheduled away and this function would never read the status again after
timing out. Instead, the function will return -ETIMEDOUT when it's
possible that scheduling didn't work out and the status bit was cleared.
Bit polling code should always check the bit being polled one more time
after the timeout in case this happens.
Fix this by reading the status once more after the while loop breaks.
The readl_poll_timeout() macro implements all of this, and it is
shorter, so use that macro here to consolidate code and fix this.
There were some concerns with using readl_poll_timeout() because it uses
timekeeping, and timekeeping isn't running early on or during the late
stages of system suspend or early stages of system resume, but an audit
of the code concluded that this code isn't called during those times so
it is safe to use the macro.
Cc: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@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..4c774ee8bb1b 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;
+ u8 status;
+ int err;
- do {
- u32 status;
+ err = readx_poll_timeout(ipc_read_status, scu, status, !(status & IPC_STATUS_BUSY),
+ 100, jiffies_to_usecs(IPC_TIMEOUT));
+ if (err)
+ return err;
- status = ipc_read_status(scu);
- if (!(status & IPC_STATUS_BUSY))
- return (status & IPC_STATUS_ERR) ? -EIO : 0;
-
- usleep_range(50, 100);
- } while (time_before(jiffies, end));
-
- return -ETIMEDOUT;
+ return (status & IPC_STATUS_ERR) ? -EIO : 0;
}
/* Wait till ipc ioc interrupt is received or timeout in 10 HZ */
--
https://chromeos.dev
next prev parent reply other threads:[~2023-09-13 21:27 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-13 21:27 [PATCH v4 0/4] platform/x86: intel_scu_ipc: Timeout fixes Stephen Boyd
2023-09-13 21:27 ` Stephen Boyd [this message]
2023-09-15 13:42 ` [PATCH v4 1/4] platform/x86: intel_scu_ipc: Check status after timeout in busy_loop() Ilpo Järvinen
2023-09-13 21:27 ` [PATCH v4 2/4] platform/x86: intel_scu_ipc: Check status upon timeout in ipc_wait_for_interrupt() Stephen Boyd
2023-09-15 13:49 ` Ilpo Järvinen
2023-09-18 13:10 ` Hans de Goede
2023-09-18 13:26 ` Ilpo Järvinen
2023-09-13 21:27 ` [PATCH v4 3/4] platform/x86: intel_scu_ipc: Don't override scu in intel_scu_ipc_dev_simple_command() Stephen Boyd
2023-09-15 14:45 ` Ilpo Järvinen
2023-09-16 11:14 ` Andy Shevchenko
2023-09-13 21:27 ` [PATCH v4 4/4] platform/x86: intel_scu_ipc: Fail IPC send if still busy Stephen Boyd
2023-09-15 14:49 ` Ilpo Järvinen
2023-09-18 13:15 ` [PATCH v4 0/4] platform/x86: intel_scu_ipc: Timeout fixes Hans de Goede
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=20230913212723.3055315-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).