Skip to content

Commit

Permalink
platform/x86: intel_scu_ipc: Check status after timeout in busy_loop()
Browse files Browse the repository at this point in the history
[ Upstream commit e0b4ab3 ]

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: e7b7ab3 ("platform/x86: intel_scu_ipc: Sleeping is fine when polling")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20230913212723.3055315-2-swboyd@chromium.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
bebarino authored and gregkh committed Oct 6, 2023
1 parent 8ef5cc6 commit 98a5a7f
Showing 1 changed file with 8 additions and 11 deletions.
19 changes: 8 additions & 11 deletions drivers/platform/x86/intel_scu_ipc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -232,19 +233,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 = readx_poll_timeout(ipc_read_status, scu, status, !(status & IPC_STATUS_BUSY),
100, jiffies_to_usecs(IPC_TIMEOUT));
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 */
Expand Down

0 comments on commit 98a5a7f

Please sign in to comment.