Messages in this thread | | | Date | Tue, 11 Feb 2020 17:41:38 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v5 02/18] platform/x86: intel_scu_ipc: Log more information if SCU IPC command fails |
| |
On Tue, Feb 11, 2020 at 04:25:47PM +0300, Mika Westerberg wrote: > Currently we only log an error if the command times out which makes it > hard to figure out the failing command. This changes the driver to log > command and subcommand with the error code which should make debugging > easier. This also allows us to simplify the callers as they don't need > to log these errors themselves. >
Thanks, my comments below.
...
> scu = ipcdev; > - ipc_command(scu, sub << 12 | cmd); > + cmdval = sub << 12 | cmd; > + ipc_command(scu, cmdval); > err = intel_scu_ipc_check_status(scu);
> + if (err) > + dev_err(&scu->dev, "IPC command %#x failed with %d\n", cmdval, > + err);
We may move it out of the mutex, right? Also, please keep it one line.
> mutex_unlock(&ipclock); > return err;
...
> mutex_lock(&ipclock);
> + cmdval = (inlen << 16) | (sub << 12) | cmd; > + ipc_command(scu, cmdval); > err = intel_scu_ipc_check_status(scu); > - > - if (!err) { > + if (err) { > + dev_err(&scu->dev, "IPC command %#x failed with %d\n", cmdval, > + err); > + } else { > for (i = 0; i < outlen; i++) > *out++ = ipc_data_readl(scu, 4 * i); > }
It's not visible in this context but it looks like above applies here as well.
-- With Best Regards, Andy Shevchenko
| |