[PATCH 0/5] ASoC: SOF: cleanups
Minor cleanups for error formats, missing cases, useless functions and simplifications.
Curtis Malainey (2): ASoC: SOF: add missing pm debug ASoC: SOF: fix string format for errors
Fred Oh (1): ASoC: SOF: ext_manifest: use explicit number for elem_type
Guennadi Liakhovetski (2): ASoC: SOF: remove unused functions ASoC: SOF: HDA: (cosmetic) simplify hda_dsp_d0i3_work()
include/sound/sof/ext_manifest.h | 6 ++-- sound/soc/sof/intel/hda-compress.c | 2 +- sound/soc/sof/intel/hda-dsp.c | 18 +++++------ sound/soc/sof/intel/hda-loader.c | 6 ++-- sound/soc/sof/intel/hda-pcm.c | 2 +- sound/soc/sof/intel/hda-trace.c | 4 +-- sound/soc/sof/intel/hda.c | 50 ------------------------------ sound/soc/sof/intel/hda.h | 1 - sound/soc/sof/ipc.c | 2 ++ 9 files changed, 19 insertions(+), 72 deletions(-)
From: Curtis Malainey cujomalainey@chromium.org
Type is not part of debugging parse code. Add it so unknown types don't show up while debugging
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Curtis Malainey cujomalainey@chromium.org Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index fc13bb06dbf3..1bc3d6282f16 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -106,6 +106,8 @@ static void ipc_log_header(struct device *dev, u8 *text, u32 cmd) str2 = "CLK_REQ"; break; case SOF_IPC_PM_CORE_ENABLE: str2 = "CORE_ENABLE"; break; + case SOF_IPC_PM_GATE: + str2 = "GATE"; break; default: str2 = "unknown type"; break; }
From: Curtis Malainey cujomalainey@chromium.org
These are negative error return values, printing them as hex is annoying and not beneficial. Switch back to signed type to make error lookup simpler.
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@gmail.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Curtis Malainey cujomalainey@chromium.org Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-compress.c | 2 +- sound/soc/sof/intel/hda-dsp.c | 2 +- sound/soc/sof/intel/hda-loader.c | 6 +++--- sound/soc/sof/intel/hda-pcm.c | 2 +- sound/soc/sof/intel/hda-trace.c | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda-compress.c b/sound/soc/sof/intel/hda-compress.c index c6cf7b313934..fe2f3f7d236b 100644 --- a/sound/soc/sof/intel/hda-compress.c +++ b/sound/soc/sof/intel/hda-compress.c @@ -82,7 +82,7 @@ int hda_probe_compr_set_params(struct snd_sof_dev *sdev,
ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL); if (ret < 0) { - dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret); + dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); return ret; }
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index e92e9750b720..72c0b8e9a196 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -732,7 +732,7 @@ int hda_dsp_resume(struct snd_sof_dev *sdev) ret = snd_hdac_ext_bus_link_power_up(hlink); if (ret < 0) { dev_dbg(sdev->dev, - "error %x in %s: failed to power up links", + "error %d in %s: failed to power up links", ret, __func__); return ret; } diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index d744952f6954..fc25ee8f68dc 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -47,7 +47,7 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig /* allocate DMA buffer */ ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, &pci->dev, size, dmab); if (ret < 0) { - dev_err(sdev->dev, "error: memory alloc failed: %x\n", ret); + dev_err(sdev->dev, "error: memory alloc failed: %d\n", ret); goto error; }
@@ -58,13 +58,13 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig if (direction == SNDRV_PCM_STREAM_CAPTURE) { ret = hda_dsp_iccmax_stream_hw_params(sdev, dsp_stream, dmab, NULL); if (ret < 0) { - dev_err(sdev->dev, "error: iccmax stream prepare failed: %x\n", ret); + dev_err(sdev->dev, "error: iccmax stream prepare failed: %d\n", ret); goto error; } } else { ret = hda_dsp_stream_hw_params(sdev, dsp_stream, dmab, NULL); if (ret < 0) { - dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret); + dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); goto error; } hda_dsp_stream_spib_config(sdev, dsp_stream, HDA_DSP_SPIB_ENABLE, size); diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 689934131a68..df00db8369c7 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -111,7 +111,7 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev,
ret = hda_dsp_stream_hw_params(sdev, stream, dmab, params); if (ret < 0) { - dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret); + dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret); return ret; }
diff --git a/sound/soc/sof/intel/hda-trace.c b/sound/soc/sof/intel/hda-trace.c index 81d76d3debc6..29e3da3c63db 100644 --- a/sound/soc/sof/intel/hda-trace.c +++ b/sound/soc/sof/intel/hda-trace.c @@ -32,7 +32,7 @@ static int hda_dsp_trace_prepare(struct snd_sof_dev *sdev)
ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL); if (ret < 0) - dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret); + dev_err(sdev->dev, "error: hdac prepare failed: %d\n", ret);
return ret; } @@ -59,7 +59,7 @@ int hda_dsp_trace_init(struct snd_sof_dev *sdev, u32 *stream_tag) */ ret = hda_dsp_trace_prepare(sdev); if (ret < 0) { - dev_err(sdev->dev, "error: hdac trace init failed: %x\n", ret); + dev_err(sdev->dev, "error: hdac trace init failed: %d\n", ret); hda_dsp_stream_put(sdev, SNDRV_PCM_STREAM_CAPTURE, *stream_tag); hda->dtrace_stream = NULL; *stream_tag = 0;
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
hda_dsp_dump_skl() is never used and hda_dsp_get_status_skl() is only called from that function. Remove both functions.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Keyon Jie yang.jie@intel.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda.c | 50 --------------------------------------- sound/soc/sof/intel/hda.h | 1 - 2 files changed, 51 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 447163494b05..57853ef89cd1 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -317,26 +317,6 @@ static const struct hda_dsp_msg_code hda_dsp_rom_msg[] = { {HDA_DSP_ROM_NULL_FW_ENTRY, "error: null FW entry point"}, };
-static void hda_dsp_get_status_skl(struct snd_sof_dev *sdev) -{ - u32 status; - int i; - - status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, - HDA_ADSP_FW_STATUS_SKL); - - for (i = 0; i < ARRAY_SIZE(hda_dsp_rom_msg); i++) { - if (status == hda_dsp_rom_msg[i].code) { - dev_err(sdev->dev, "%s - code %8.8x\n", - hda_dsp_rom_msg[i].msg, status); - return; - } - } - - /* not for us, must be generic sof message */ - dev_dbg(sdev->dev, "unknown ROM status value %8.8x\n", status); -} - static void hda_dsp_get_status(struct snd_sof_dev *sdev) { u32 status; @@ -385,36 +365,6 @@ static void hda_dsp_get_registers(struct snd_sof_dev *sdev, stack_words * sizeof(u32)); }
-void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags) -{ - struct sof_ipc_dsp_oops_xtensa xoops; - struct sof_ipc_panic_info panic_info; - u32 stack[HDA_DSP_STACK_DUMP_SIZE]; - u32 status, panic; - - /* try APL specific status message types first */ - hda_dsp_get_status_skl(sdev); - - /* now try generic SOF status messages */ - status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, - HDA_ADSP_ERROR_CODE_SKL); - - /*TODO: Check: there is no define in spec, but it is used in the code*/ - panic = snd_sof_dsp_read(sdev, HDA_DSP_BAR, - HDA_ADSP_ERROR_CODE_SKL + 0x4); - - if (sdev->fw_state == SOF_FW_BOOT_COMPLETE) { - hda_dsp_get_registers(sdev, &xoops, &panic_info, stack, - HDA_DSP_STACK_DUMP_SIZE); - snd_sof_get_status(sdev, status, panic, &xoops, &panic_info, - stack, HDA_DSP_STACK_DUMP_SIZE); - } else { - dev_err(sdev->dev, "error: status = 0x%8.8x panic = 0x%8.8x\n", - status, panic); - hda_dsp_get_status_skl(sdev); - } -} - /* dump the first 8 dwords representing the extended ROM status */ static void hda_dsp_dump_ext_rom_status(struct snd_sof_dev *sdev, u32 flags) { diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 488c0ace4d28..d1c38c37bc9d 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -518,7 +518,6 @@ int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev); int hda_dsp_runtime_resume(struct snd_sof_dev *sdev); int hda_dsp_runtime_idle(struct snd_sof_dev *sdev); int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev); -void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags); void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags); void hda_ipc_dump(struct snd_sof_dev *sdev); void hda_ipc_irq_dump(struct snd_sof_dev *sdev);
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
Simplify hda_dsp_d0i3_work() by returning immediately from it if D0i3 cannot be set.
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Libin Yang libin.yang@intel.com Reviewed-by: Keyon Jie yang.jie@intel.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 72c0b8e9a196..5788fe356960 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -934,19 +934,15 @@ void hda_dsp_d0i3_work(struct work_struct *work) d0i3_work.work); struct hdac_bus *bus = &hdev->hbus.core; struct snd_sof_dev *sdev = dev_get_drvdata(bus->dev); - struct sof_dsp_power_state target_state; + struct sof_dsp_power_state target_state = { + .state = SOF_DSP_PM_D0, + .substate = SOF_HDA_DSP_PM_D0I3, + }; int ret;
- target_state.state = SOF_DSP_PM_D0; - /* DSP can enter D0I3 iff only D0I3-compatible streams are active */ - if (snd_sof_dsp_only_d0i3_compatible_stream_active(sdev)) - target_state.substate = SOF_HDA_DSP_PM_D0I3; - else - target_state.substate = SOF_HDA_DSP_PM_D0I0; - - /* remain in D0I0 */ - if (target_state.substate == SOF_HDA_DSP_PM_D0I0) + if (!snd_sof_dsp_only_d0i3_compatible_stream_active(sdev)) + /* remain in D0I0 */ return;
/* This can fail but error cannot be propagated */
From: Fred Oh fred.oh@linux.intel.com
Use explicit number to define elem_type enum instead of using SOF_IPC_EXT_*.
Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Reviewed-by: Karol Trzciński karolx.trzcinski@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Fred Oh fred.oh@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- include/sound/sof/ext_manifest.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/sound/sof/ext_manifest.h b/include/sound/sof/ext_manifest.h index 7abc4f0bd3ad..2a7e055584f9 100644 --- a/include/sound/sof/ext_manifest.h +++ b/include/sound/sof/ext_manifest.h @@ -58,9 +58,9 @@ struct sof_ext_man_header { /* Extended manifest elements types */ enum sof_ext_man_elem_type { SOF_EXT_MAN_ELEM_FW_VERSION = 0, - SOF_EXT_MAN_ELEM_WINDOW = SOF_IPC_EXT_WINDOW, - SOF_EXT_MAN_ELEM_CC_VERSION = SOF_IPC_EXT_CC_INFO, - SOF_EXT_MAN_ELEM_DBG_ABI = SOF_IPC_EXT_USER_ABI_INFO, + SOF_EXT_MAN_ELEM_WINDOW = 1, + SOF_EXT_MAN_ELEM_CC_VERSION = 2, + SOF_EXT_MAN_ELEM_DBG_ABI = 4, SOF_EXT_MAN_ELEM_CONFIG_DATA = 5, /**< ABI3.17 */ SOF_EXT_MAN_ELEM_PLATFORM_CONFIG_DATA = 6, };
On Mon, 8 Feb 2021 17:21:44 -0600, Pierre-Louis Bossart wrote:
Minor cleanups for error formats, missing cases, useless functions and simplifications.
Curtis Malainey (2): ASoC: SOF: add missing pm debug ASoC: SOF: fix string format for errors
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] ASoC: SOF: add missing pm debug commit: a8f50cd9be7cc4c57f29c1390568225ebee90eda [2/5] ASoC: SOF: fix string format for errors commit: ce1f55bac5534aa518e26b94728173ee45f91a8c [3/5] ASoC: SOF: remove unused functions commit: 3be46fa21088740ae5790d84b882e5a3c98fce41 [4/5] ASoC: SOF: HDA: (cosmetic) simplify hda_dsp_d0i3_work() commit: f1bb023525fd654121f18f6e2587eeee84c9db04 [5/5] ASoC: SOF: ext_manifest: use explicit number for elem_type commit: cc11626dd9f894d93ed15d78b04452ca9acbb52b
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Pierre-Louis Bossart