Skip to content

Commit

Permalink
platform/x86: intel_scu_ipc: Fail IPC send if still busy
Browse files Browse the repository at this point in the history
[ Upstream commit 85e654c ]

It's possible for interrupts to get significantly delayed to the point
that callers of intel_scu_ipc_dev_command() and friends can call the
function once, hit a timeout, and call it again while the interrupt
still hasn't been processed. This driver will get seriously confused if
the interrupt is finally processed after the second IPC has been sent
with ipc_command(). It won't know which IPC has been completed. This
could be quite disastrous if calling code assumes something has happened
upon return from intel_scu_ipc_dev_simple_command() when it actually
hasn't.

Let's avoid this scenario by simply returning -EBUSY in this case.
Hopefully higher layers will know to back off or fail gracefully when
this happens. It's all highly unlikely anyway, but it's better to be
correct here as we have no way to know which IPC the status register is
telling us about if we send a second IPC while the previous IPC is still
processing.

Cc: Prashant Malani <pmalani@chromium.org>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Fixes: ed12f29 ("ipc: Added support for IPC interrupt mode")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20230913212723.3055315-5-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 b34121a commit 7f301aa
Showing 1 changed file with 28 additions and 12 deletions.
40 changes: 28 additions & 12 deletions drivers/platform/x86/intel_scu_ipc.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,24 @@ static int intel_scu_ipc_check_status(struct intel_scu_ipc_dev *scu)
return scu->irq > 0 ? ipc_wait_for_interrupt(scu) : busy_loop(scu);
}

static struct intel_scu_ipc_dev *intel_scu_ipc_get(struct intel_scu_ipc_dev *scu)
{
u8 status;

if (!scu)
scu = ipcdev;
if (!scu)
return ERR_PTR(-ENODEV);

status = ipc_read_status(scu);
if (status & IPC_STATUS_BUSY) {
dev_dbg(&scu->dev, "device is busy\n");
return ERR_PTR(-EBUSY);
}

return scu;
}

/* Read/Write power control(PMIC in Langwell, MSIC in PenWell) registers */
static int pwr_reg_rdwr(struct intel_scu_ipc_dev *scu, u16 *addr, u8 *data,
u32 count, u32 op, u32 id)
Expand All @@ -279,11 +297,10 @@ static int pwr_reg_rdwr(struct intel_scu_ipc_dev *scu, u16 *addr, u8 *data,
memset(cbuf, 0, sizeof(cbuf));

mutex_lock(&ipclock);
if (!scu)
scu = ipcdev;
if (!scu) {
scu = intel_scu_ipc_get(scu);
if (IS_ERR(scu)) {
mutex_unlock(&ipclock);
return -ENODEV;
return PTR_ERR(scu);
}

for (nc = 0; nc < count; nc++, offset += 2) {
Expand Down Expand Up @@ -438,12 +455,12 @@ int intel_scu_ipc_dev_simple_command(struct intel_scu_ipc_dev *scu, int cmd,
int err;

mutex_lock(&ipclock);
if (!scu)
scu = ipcdev;
if (!scu) {
scu = intel_scu_ipc_get(scu);
if (IS_ERR(scu)) {
mutex_unlock(&ipclock);
return -ENODEV;
return PTR_ERR(scu);
}

cmdval = sub << 12 | cmd;
ipc_command(scu, cmdval);
err = intel_scu_ipc_check_status(scu);
Expand Down Expand Up @@ -483,11 +500,10 @@ int intel_scu_ipc_dev_command_with_size(struct intel_scu_ipc_dev *scu, int cmd,
return -EINVAL;

mutex_lock(&ipclock);
if (!scu)
scu = ipcdev;
if (!scu) {
scu = intel_scu_ipc_get(scu);
if (IS_ERR(scu)) {
mutex_unlock(&ipclock);
return -ENODEV;
return PTR_ERR(scu);
}

memcpy(inbuf, in, inlen);
Expand Down

0 comments on commit 7f301aa

Please sign in to comment.