On 2019-06-26 11:38, Pawel Harlozinski wrote:
When DSP boots with timeout error, reinitialize, transfer and boot firmware to recover audio.
This is so called "recovery". Say "why" we do it, not just "what" the sequence is.
Signed-off-by: Szymon Mielczarek szymonx.mielczarek@intel.com
sound/soc/intel/skylake/bxt-sst.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 440bca7afbf1..a2844bdca1b8 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -455,13 +455,18 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id)
What about its neighbour, cnl_set_dsp_D0?
/* If core 1 was turned on for booting core 0, turn it off */ skl_dsp_core_power_down(ctx, SKL_DSP_CORE_MASK(1)); if (ret == 0) {
dev_err(ctx->dev, "%s: DSP boot timeout\n", __func__);dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n",
dev_warn(ctx->dev,"DSP boot timeout: Error code=0x%x: FW status=0x%x\n",
Log change (--log_level) without mention in commit msg. Maybe split this into separate commit? The entire log-polish thingy could be a theme for a patchset as this is not the only place where such changes are welcome.
sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE), sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS));
dev_err(ctx->dev, "Failed to set core0 to D0 state\n");ret = -EIO;goto err;
ret = bxt_sst_init_fw(skl->dev, skl);dev_warn(ctx->dev, "Reload fw status: %d\n", ret);if (ret < 0) {dev_err(ctx->dev, "Failed to set core0 to D0 state\n");ret = -EIO;goto err;}
Double message is unnecessary, routine is verbose enough. Either leave "always report stuff" and resign from mentioning anything at all on failure -or- do it only on failure explicitly.
}}