[PATCH 0/7] ASoC: SOF: code cleanups for 5.14

[PATCH 0/7] ASoC: SOF: code cleanups for 5.14

[Kai Vehmanen]

Set of code cleanups for issues found in code review.

Jaska Uimonen (1):
  ASoC: SOF: topology: fix assignment to use le32_to_cpu

Keyon Jie (1):
  ASoC: SOF: ops: print out the polling register

Peter Ujfalusi (4):
  ASoC: SOF: Check desc->ops directly in acpi/pci/of probe functions
  ASoC: SOF: pci: No need to cast second time to save the desc
  ASoC: SOF: loader: Use snd_sof_dsp_block_read() instead
    sof_block_read()
  ASoC: SOF: Intel: hda: Remove conditions against CONFIG_PCI

Pierre-Louis Bossart (1):
  ASoC: SOF: ops: don't return void value

 sound/soc/sof/intel/hda.c    |  8 +++-----
 sound/soc/sof/loader.c       |  2 +-
 sound/soc/sof/ops.h          | 10 ++++++----
 sound/soc/sof/sof-acpi-dev.c |  5 +----
 sound/soc/sof/sof-of-dev.c   |  5 +----
 sound/soc/sof/sof-pci-dev.c  |  7 ++-----
 sound/soc/sof/topology.c     |  2 +-
 7 files changed, 15 insertions(+), 24 deletions(-)


base-commit: aa736700f42fa0813e286ca2f9274ffaa25163b9
-- 
2.31.0

[PATCH 1/7] ASoC: SOF: Check desc->ops directly in acpi/pci/of probe functions

[Kai Vehmanen]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

We can check for the desc->ops directly in the probe functions, the ops
is not used directly in the functions.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/sof-acpi-dev.c | 5 +----
 sound/soc/sof/sof-of-dev.c   | 5 +----
 sound/soc/sof/sof-pci-dev.c  | 5 +----
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/sound/soc/sof/sof-acpi-dev.c b/sound/soc/sof/sof-acpi-dev.c
index 7fbf09f9f17e..74982c04497b 100644
--- a/sound/soc/sof/sof-acpi-dev.c
+++ b/sound/soc/sof/sof-acpi-dev.c
@@ -60,7 +60,6 @@ int sof_acpi_probe(struct platform_device *pdev, const struct sof_dev_desc *desc
 {
 	struct device *dev = &pdev->dev;
 	struct snd_sof_pdata *sof_pdata;
-	const struct snd_sof_dsp_ops *ops;
 
 	dev_dbg(dev, "ACPI DSP detected");
 
@@ -68,9 +67,7 @@ int sof_acpi_probe(struct platform_device *pdev, const struct sof_dev_desc *desc
 	if (!sof_pdata)
 		return -ENOMEM;
 
-	/* get ops for platform */
-	ops = desc->ops;
-	if (!ops) {
+	if (!desc->ops) {
 		dev_err(dev, "error: no matching ACPI descriptor ops\n");
 		return -ENODEV;
 	}
diff --git a/sound/soc/sof/sof-of-dev.c b/sound/soc/sof/sof-of-dev.c
index c9c70645b377..d1a21edfa05d 100644
--- a/sound/soc/sof/sof-of-dev.c
+++ b/sound/soc/sof/sof-of-dev.c
@@ -70,7 +70,6 @@ static int sof_of_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const struct sof_dev_desc *desc;
 	struct snd_sof_pdata *sof_pdata;
-	const struct snd_sof_dsp_ops *ops;
 
 	dev_info(&pdev->dev, "DT DSP detected");
 
@@ -82,9 +81,7 @@ static int sof_of_probe(struct platform_device *pdev)
 	if (!desc)
 		return -ENODEV;
 
-	/* get ops for platform */
-	ops = desc->ops;
-	if (!ops) {
+	if (!desc->ops) {
 		dev_err(dev, "error: no matching DT descriptor ops\n");
 		return -ENODEV;
 	}
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index 3489dc1b48f6..a1db973d5673 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -116,14 +116,11 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
 	const struct sof_dev_desc *desc =
 		(const struct sof_dev_desc *)pci_id->driver_data;
 	struct snd_sof_pdata *sof_pdata;
-	const struct snd_sof_dsp_ops *ops;
 	int ret;
 
 	dev_dbg(&pci->dev, "PCI DSP detected");
 
-	/* get ops for platform */
-	ops = desc->ops;
-	if (!ops) {
+	if (!desc->ops) {
 		dev_err(dev, "error: no matching PCI descriptor ops\n");
 		return -ENODEV;
 	}
-- 
2.31.0

[PATCH 2/7] ASoC: SOF: pci: No need to cast second time to save the desc

[Kai Vehmanen]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

At the start of the function we already have the desc, no need to cast it
again from pci_id->driver_data to save it to sof_pdata.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/sof-pci-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index a1db973d5673..03119462f9e2 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -138,7 +138,7 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
 		return ret;
 
 	sof_pdata->name = pci_name(pci);
-	sof_pdata->desc = (struct sof_dev_desc *)pci_id->driver_data;
+	sof_pdata->desc = desc;
 	sof_pdata->dev = dev;
 	sof_pdata->fw_filename = desc->default_fw_filename;
 
-- 
2.31.0

[PATCH 3/7] ASoC: SOF: ops: print out the polling register

[Kai Vehmanen]

From: Keyon Jie <yang.jie@linux.intel.com>

Print the register offset out to provide more useful information for the
register polling debugging.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/ops.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index 323a0b3f561b..2763059c3d4b 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -546,14 +546,16 @@ static inline const struct snd_sof_dsp_ops
 		(val) = snd_sof_dsp_read(sdev, bar, offset);		\
 		if (cond) { \
 			dev_dbg(sdev->dev, \
-				"FW Poll Status: reg=%#x successful\n", (val)); \
+				"FW Poll Status: reg[%#x]=%#x successful\n", \
+				(offset), (val)); \
 			break; \
 		} \
 		if (__timeout_us && \
 		    ktime_compare(ktime_get(), __timeout) > 0) { \
 			(val) = snd_sof_dsp_read(sdev, bar, offset); \
 			dev_dbg(sdev->dev, \
-				"FW Poll Status: reg=%#x timedout\n", (val)); \
+				"FW Poll Status: reg[%#x]=%#x timedout\n", \
+				(offset), (val)); \
 			break; \
 		} \
 		if (__sleep_us) \
-- 
2.31.0

[PATCH 4/7] ASoC: SOF: loader: Use snd_sof_dsp_block_read() instead sof_block_read()

[Kai Vehmanen]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

SOF core should use the IO functions via callbacks and not directly to
ensure that it remains platform independent.

Fixes: 83ee7ab1627b7 ("ASoC: SOF: Intel: byt: Refactor fw ready / mem windows creation")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c
index 6efaf766f2ab..2b38a77cd594 100644
--- a/sound/soc/sof/loader.c
+++ b/sound/soc/sof/loader.c
@@ -517,7 +517,7 @@ int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
 		return 0;
 
 	/* copy data from the DSP FW ready offset */
-	sof_block_read(sdev, bar, offset, fw_ready, sizeof(*fw_ready));
+	snd_sof_dsp_block_read(sdev, bar, offset, fw_ready, sizeof(*fw_ready));
 
 	/* make sure ABI version is compatible */
 	ret = snd_sof_ipc_valid(sdev);
-- 
2.31.0

[PATCH 5/7] ASoC: SOF: topology: fix assignment to use le32_to_cpu

[Kai Vehmanen]

From: Jaska Uimonen <jaska.uimonen@linux.intel.com>

Fix sparse warning by using le32_to_cpu.

Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index 92d346bbd357..cc9585bfa4e9 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -3338,7 +3338,7 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
 	/* Copy common data to all config ipc structs */
 	for (i = 0; i < num_conf; i++) {
 		config[i].hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG;
-		config[i].format = hw_config[i].fmt;
+		config[i].format = le32_to_cpu(hw_config[i].fmt);
 		config[i].type = common_config.type;
 		config[i].dai_index = common_config.dai_index;
 	}
-- 
2.31.0

[PATCH 6/7] ASoC: SOF: ops: don't return void value

[Kai Vehmanen]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Sparse throws the following warning:

sound/soc/sof/ops.h:247:17: error: returning void-valued expression

Remove the useless returns.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Paul Olaru <paul.olaru@oss.nxp.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/ops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index 2763059c3d4b..4a5d6e497f05 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -244,13 +244,13 @@ snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
 static inline void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags)
 {
 	if (sof_ops(sdev)->dbg_dump)
-		return sof_ops(sdev)->dbg_dump(sdev, flags);
+		sof_ops(sdev)->dbg_dump(sdev, flags);
 }
 
 static inline void snd_sof_ipc_dump(struct snd_sof_dev *sdev)
 {
 	if (sof_ops(sdev)->ipc_dump)
-		return sof_ops(sdev)->ipc_dump(sdev);
+		sof_ops(sdev)->ipc_dump(sdev);
 }
 
 /* register IO */
-- 
2.31.0

[PATCH 7/7] ASoC: SOF: Intel: hda: Remove conditions against CONFIG_PCI

[Kai Vehmanen]

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

The HDA support can only be compiled when SND_SOC_SOF_PCI is enabled which
depends on CONFIG_PCI.

This makes the IS_ENABLED(CONFIG_PCI) checks redundant in the code, they
will resolve to true all the time.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/sof/intel/hda.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 5658e4b6273d..126232a76a10 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -277,10 +277,12 @@ struct hda_dsp_msg_code {
 	const char *msg;
 };
 
-static bool hda_use_msi = IS_ENABLED(CONFIG_PCI);
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG)
+static bool hda_use_msi = true;
 module_param_named(use_msi, hda_use_msi, bool, 0444);
 MODULE_PARM_DESC(use_msi, "SOF HDA use PCI MSI mode");
+#else
+#define hda_use_msi	(1)
 #endif
 
 static char *hda_model;
@@ -485,9 +487,7 @@ static int hda_init(struct snd_sof_dev *sdev)
 
 	/* initialise hdac bus */
 	bus->addr = pci_resource_start(pci, 0);
-#if IS_ENABLED(CONFIG_PCI)
 	bus->remap_addr = pci_ioremap_bar(pci, 0);
-#endif
 	if (!bus->remap_addr) {
 		dev_err(bus->dev, "error: ioremap error\n");
 		return -ENXIO;
@@ -799,9 +799,7 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
 		goto hdac_bus_unmap;
 
 	/* DSP base */
-#if IS_ENABLED(CONFIG_PCI)
 	sdev->bar[HDA_DSP_BAR] = pci_ioremap_bar(pci, HDA_DSP_BAR);
-#endif
 	if (!sdev->bar[HDA_DSP_BAR]) {
 		dev_err(sdev->dev, "error: ioremap error\n");
 		ret = -ENXIO;
-- 
2.31.0

Re: [PATCH 0/7] ASoC: SOF: code cleanups for 5.14

[Mark Brown]

On Fri, 21 May 2021 12:27:57 +0300, Kai Vehmanen wrote:
> Set of code cleanups for issues found in code review.
> 
> Jaska Uimonen (1):
>   ASoC: SOF: topology: fix assignment to use le32_to_cpu
> 
> Keyon Jie (1):
>   ASoC: SOF: ops: print out the polling register
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/7] ASoC: SOF: Check desc->ops directly in acpi/pci/of probe functions
      commit: fd979ec12eebcfb718f2c7c28b336d891d439f85
[2/7] ASoC: SOF: pci: No need to cast second time to save the desc
      commit: e5eaa4e66f538b8ba4928785a62edf8ffcf7c053
[3/7] ASoC: SOF: ops: print out the polling register
      commit: 3b2e93ed12381fa1c33169202f2cdffbb18157c4
[4/7] ASoC: SOF: loader: Use snd_sof_dsp_block_read() instead sof_block_read()
      commit: c03459415c5120fe03dd7d9824880acc8b7f2693
[5/7] ASoC: SOF: topology: fix assignment to use le32_to_cpu
      commit: ccaea61a8d1b8180cc3c470e383381884e4bc1f2
[6/7] ASoC: SOF: ops: don't return void value
      commit: 4f50f16e9414ea41d5c142fd880faab060472a6b
[7/7] ASoC: SOF: Intel: hda: Remove conditions against CONFIG_PCI
      commit: 9d5536e0e1ca8409665bdd80d951941d5ce19b8a

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