Message ID | 20230913212723.3055315-2-swboyd@chromium.org (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
Series | platform/x86: intel_scu_ipc: Timeout fixes | expand |
On Wed, 13 Sep 2023, Stephen Boyd wrote: > 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> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
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 */