On 2019-08-22 16:07, Pierre-Louis Bossart wrote:
On 8/22/19 6:36 AM, Cezary Rojewski wrote:
Apart from Haswell machines, all other devices have their private data
set to snd_soc_acpi_mach instance.
Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/
added support for dai_link platform_name adjustments within card probe
routines. These take for granted private_data points to
snd_soc_acpi_mach whereas for Haswell, it's sst_pdata instead. Change
private context of platform_device - representing machine board - to
address this.
Cezary, see the comments of the initial series:
"Note that byt-max98080, byt-rt5640 were not modified since they are
deprecated. bytcht-nocodec and the Skylake/Kabylake machine drivers
changes were not changed since SOF does not support them. There may be
additional changes if and when Skylake/Kabylake are supported by SOF
(largely a firmware authentication issue, not technical difficulty)."
I intentionally did not touch the Haswell and Baytrail legacy since both
drivers do not update the platform name, this is only done for cases
where SOF is used.
So while I don't mind a change, it's got to come with tests for each
variant, and if you do the changes for Haswell then you want to change
Baytrail legacy machine drivers as well. And are we going to change the
SKL/KBL machine drivers to allow for this platform name rewrite?
Also the information below is misleading: nothing is broken in the
current solution and -stable kernels do not need to pick this patchset.
This is a code alignment and the behavior is identical.
Or as an alternative we leave the code as is...
Guess I wasn't clear enough:
- this code fixes panic generated by series found under link above.
Following code added within machine probe for broadwell.c:
/* override plaform name, if required */
mach = (&pdev->dev)->platform_data;
pdata = (&pdev->dev)->platform_data;
if (mach) /* extra check since legacy does not pass parameters */ {
platform_name = mach->mach_params.platform;
dev_warn(&pdev->dev, "Broadwell platform_name: %s, %s, %s, %s\n",
mach->id, mach->drv_name, mach->fw_filename, platform_name);
dev_warn(&pdev->dev, "Broadwell id and res_idx: %x, %d\n", pdata->id,
pdata->resindex_dma_base);
}
Generates:
[ 25.982151] broadwell-audio broadwell-audio: Broadwell platform_name:
, (null), (efault), (null)
[ 25.982157] broadwell-audio broadwell-audio: Broadwell id and
res_idx: 3438, 1040384
Conslusion:
0x3438 == BDW_ID
1040384 -> 0x0FE000 -> WPT_DSP_DMA_ADDR_OFFSET
confirms the claim.
As stated, during cleanups and moving stuff around, code you've added
generates panics. Right now it works only because of offsets of
miscasted object pointing to uninitialized variable (luckily).
platform_name is initialized as NULL for all SKL+ and legacy platforms
and thus the snd_soc_fixup_dai_links_platform_name returns immediately.
So by all means, change is not only save, but required.
Code is being tested on 2x BDW-Y which live now a happy live in our lab
with the rest of the platforms.
Czarek
Fixes: e87055d732e3 ("ASoC: Intel: haswell: platform name fixup support")
Fixes: 7e40ddcf974a ("ASoC: Intel: bdw-rt5677: platform name fixup
support")
Fixes: 2d067b2807f9 ("ASoC: Intel: broadwell: platform name fixup
support")
Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
---
sound/soc/intel/common/sst-acpi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/common/sst-acpi.c
b/sound/soc/intel/common/sst-acpi.c
index 15f2b27e643f..c34f628c7987 100644
--- a/sound/soc/intel/common/sst-acpi.c
+++ b/sound/soc/intel/common/sst-acpi.c
@@ -109,11 +109,12 @@ int sst_acpi_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, sst_acpi);
+ mach->pdata = sst_pdata;
/* register machine driver */
sst_acpi->pdev_mach =
platform_device_register_data(dev, mach->drv_name, -1,
- sst_pdata, sizeof(*sst_pdata));
+ mach, sizeof(*mach));
if (IS_ERR(sst_acpi->pdev_mach))
return PTR_ERR(sst_acpi->pdev_mach);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
