[PATCH 0/7] ASoC: SOF: code cleanups for 5.14
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
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; }
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;
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) \
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);
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; }
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 */
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;
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
participants (2)
-
Kai Vehmanen -
Mark Brown