[PATCH 0/5] ASoC: SOF: cleanups

[PATCH 0/5] ASoC: SOF: cleanups

[Pierre-Louis Bossart]

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(-)

-- 
2.25.1

[PATCH 1/5] ASoC: SOF: add missing pm debug

[Pierre-Louis Bossart]

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;
 		}
-- 
2.25.1

[PATCH 2/5] ASoC: SOF: fix string format for errors

[Pierre-Louis Bossart]

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;
-- 
2.25.1

[PATCH 3/5] ASoC: SOF: remove unused functions

[Pierre-Louis Bossart]

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);
-- 
2.25.1

[PATCH 4/5] ASoC: SOF: HDA: (cosmetic) simplify hda_dsp_d0i3_work()

[Pierre-Louis Bossart]

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 */
-- 
2.25.1

[PATCH 5/5] ASoC: SOF: ext_manifest: use explicit number for elem_type

[Pierre-Louis Bossart]

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,
 };
-- 
2.25.1

Re: [PATCH 0/5] ASoC: SOF: cleanups

[Mark Brown]

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