[alsa-devel] [PATCH 0/4] ASoC: Intel: Haswell: Adjust machine device private
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.
Caught by recent cleanups where content of sst_pdata was moved. Currently, despite the incorrect cast, dereferenced field points happily to NULL (uninitialized field), so no panics were observed.
Cezary Rojewski (4): ASoC: Intel: Haswell: Adjust machine device private context ASoC: Intel: haswell: Simplify device probe ASoC: Intel: bdw-rt5677: Simplify device probe ASoC: Intel: broadwell: Simplify device probe
sound/soc/intel/boards/bdw-rt5677.c | 6 +----- sound/soc/intel/boards/broadwell.c | 6 +----- sound/soc/intel/boards/haswell.c | 6 +----- sound/soc/intel/common/sst-acpi.c | 3 ++- 4 files changed, 5 insertions(+), 16 deletions(-)
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.
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@intel.com --- 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);
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...
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@intel.com
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));
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@intel.com
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);
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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
On 2019-08-22 17:58, 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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
On 2019-08-22 18:42, Pierre-Louis Bossart wrote:
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
Sure thing.
Code: /* register machine driver */ sst_acpi->pdev_mach = platform_device_register_data(dev, mach->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_probe (/sound/soc/intel/common/sst-acpi.c:145) generates new platform_device - which represents machine board - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on machine board probe, e.g.: broadwell_audio_probe (/sound/soc/intel/boards/broadwell.c:270). Involved platform is called: broadwell-audio. Requested private data type by broadwell_audio_probe: struct snd_soc_acpi_mach *. MISMATCH.
Code:
/* register PCM and DAI driver */ sst_acpi->pdev_pcm = platform_device_register_data(dev, desc->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_fw_cb (/sound/soc/intel/common/sst_acpi_fw_cb:47) generates new platform_device - which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on dai .init - broadwell_rtd_init - invocation when card is instantiated by ASoC code. As you can see on (/sound/soc/intel/boards/broadwell.c:162), platform tied with it is: haswell-pcm-audio. Requested private data type by broadwell_rtd_init - struct sst_pdata *. MATCH.
Czarek
On 8/22/19 12:14 PM, Cezary Rojewski wrote:
On 2019-08-22 18:42, Pierre-Louis Bossart wrote:
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
Sure thing.
Code: /* register machine driver */ sst_acpi->pdev_mach = platform_device_register_data(dev, mach->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_probe (/sound/soc/intel/common/sst-acpi.c:145) generates new platform_device - which represents machine board - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on machine board probe, e.g.: broadwell_audio_probe (/sound/soc/intel/boards/broadwell.c:270). Involved platform is called: broadwell-audio. Requested private data type by broadwell_audio_probe: struct snd_soc_acpi_mach *. MISMATCH.
Code:
/* register PCM and DAI driver */ sst_acpi->pdev_pcm = platform_device_register_data(dev, desc->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_fw_cb (/sound/soc/intel/common/sst_acpi_fw_cb:47) generates new platform_device - which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on dai .init - broadwell_rtd_init - invocation when card is instantiated by ASoC code. As you can see on (/sound/soc/intel/boards/broadwell.c:162), platform tied with it is: haswell-pcm-audio. Requested private data type by broadwell_rtd_init - struct sst_pdata *. MATCH.
the machine drivers uses snd_soc_rtdcom_lookup(rtd, DRV_NAME);
How is DRV_NAME connected to haswell-pcm-audio?
I must be missing something in your logic.
On 2019-08-22 20:44, Pierre-Louis Bossart wrote:
On 8/22/19 12:14 PM, Cezary Rojewski wrote:
On 2019-08-22 18:42, Pierre-Louis Bossart wrote:
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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.
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@intel.com
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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
Sure thing.
Code: /* register machine driver */ sst_acpi->pdev_mach = platform_device_register_data(dev, mach->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_probe (/sound/soc/intel/common/sst-acpi.c:145) generates new platform_device - which represents machine board - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on machine board probe, e.g.: broadwell_audio_probe (/sound/soc/intel/boards/broadwell.c:270). Involved platform is called: broadwell-audio. Requested private data type by broadwell_audio_probe: struct snd_soc_acpi_mach *. MISMATCH.
Code:
/* register PCM and DAI driver */ sst_acpi->pdev_pcm = platform_device_register_data(dev, desc->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_fw_cb (/sound/soc/intel/common/sst_acpi_fw_cb:47) generates new platform_device - which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on dai .init
- broadwell_rtd_init - invocation when card is instantiated by ASoC
code. As you can see on (/sound/soc/intel/boards/broadwell.c:162), platform tied with it is: haswell-pcm-audio. Requested private data type by broadwell_rtd_init - struct sst_pdata *. MATCH.
the machine drivers uses snd_soc_rtdcom_lookup(rtd, DRV_NAME);
How is DRV_NAME connected to haswell-pcm-audio?
I must be missing something in your logic.
Please checkout sst-acpi.c file and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.
On 8/22/19 2:02 PM, Cezary Rojewski wrote:
On 2019-08-22 20:44, Pierre-Louis Bossart wrote:
On 8/22/19 12:14 PM, Cezary Rojewski wrote:
On 2019-08-22 18:42, Pierre-Louis Bossart wrote:
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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. > > 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@intel.com > --- > 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));
I now agree that the code I added is incorrect and probably accesses memory offsets that aren't right. I have absolutely no idea why I added this comment that 'legacy does not pass parameters' when it most definitively does. Good catch on your side.
That said, doesn't the proposed fix introduce another issue?
In the machine drivers, you still get pdata directly, so aren't you missing an indirection to get back to pdata from mach?
static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev); struct sst_hsw *broadwell = pdata->dsp;
<<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
Sure thing.
Code: /* register machine driver */ sst_acpi->pdev_mach = platform_device_register_data(dev, mach->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_probe (/sound/soc/intel/common/sst-acpi.c:145) generates new platform_device - which represents machine board - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on machine board probe, e.g.: broadwell_audio_probe (/sound/soc/intel/boards/broadwell.c:270). Involved platform is called: broadwell-audio. Requested private data type by broadwell_audio_probe: struct snd_soc_acpi_mach *. MISMATCH.
Code:
/* register PCM and DAI driver */ sst_acpi->pdev_pcm = platform_device_register_data(dev, desc->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_fw_cb (/sound/soc/intel/common/sst_acpi_fw_cb:47) generates new platform_device - which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on dai .init - broadwell_rtd_init - invocation when card is instantiated by ASoC code. As you can see on (/sound/soc/intel/boards/broadwell.c:162), platform tied with it is: haswell-pcm-audio. Requested private data type by broadwell_rtd_init
- struct sst_pdata *. MATCH.
the machine drivers uses snd_soc_rtdcom_lookup(rtd, DRV_NAME);
How is DRV_NAME connected to haswell-pcm-audio?
I must be missing something in your logic.
Please checkout sst-acpi.c file and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.
what happens in sst-acpi.c stays in sst-acpi.c I don't get how you retrieve the pdata in the machine driver from *another* driver. Different devices, different platform data.
On 2019-08-22 22:44, Pierre-Louis Bossart wrote:
On 8/22/19 2:02 PM, Cezary Rojewski wrote:
On 2019-08-22 20:44, Pierre-Louis Bossart wrote:
On 8/22/19 12:14 PM, Cezary Rojewski wrote:
On 2019-08-22 18:42, Pierre-Louis Bossart wrote:
On 8/22/19 11:05 AM, Cezary Rojewski wrote:
On 2019-08-22 17:58, 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. >> >> 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@intel.com >> --- >> 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)); > > I now agree that the code I added is incorrect and probably > accesses memory offsets that aren't right. I have absolutely no > idea why I added this comment that 'legacy does not pass > parameters' when it most definitively does. Good catch on your side. > > That said, doesn't the proposed fix introduce another issue? > > In the machine drivers, you still get pdata directly, so aren't > you missing an indirection to get back to pdata from mach? > > static int bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd) > { > struct snd_soc_component *component = > snd_soc_rtdcom_lookup(rtd, DRV_NAME); > struct sst_pdata *pdata = dev_get_platdata(component->dev); > struct sst_hsw *broadwell = pdata->dsp; > > <<< so here you took the wrong pointer, no?
Both Baytrail and Haswell are enumerated in a bit different fashion than SKL equivalents.
There is an in-place registration for machine device - whose private_data gets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63). _rtd_init makes use of the latter of two.
I don't get your explanations. can you elaborate on what this does now that pdata is no longer passed as an argument to the machine driver:
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); struct sst_pdata *pdata = dev_get_platdata(component->dev);
the 'component' here is not the PCM one, is it?
Sure thing.
Code: /* register machine driver */ sst_acpi->pdev_mach = platform_device_register_data(dev, mach->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_probe (/sound/soc/intel/common/sst-acpi.c:145) generates new platform_device - which represents machine board - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on machine board probe, e.g.: broadwell_audio_probe (/sound/soc/intel/boards/broadwell.c:270). Involved platform is called: broadwell-audio. Requested private data type by broadwell_audio_probe: struct snd_soc_acpi_mach *. MISMATCH.
Code:
/* register PCM and DAI driver */ sst_acpi->pdev_pcm = platform_device_register_data(dev, desc->drv_name, -1, sst_pdata, sizeof(*sst_pdata));
Found in sst_acpi_fw_cb (/sound/soc/intel/common/sst_acpi_fw_cb:47) generates new platform_device - which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of struct sst_pdata type. This data gets used on dai .init - broadwell_rtd_init - invocation when card is instantiated by ASoC code. As you can see on (/sound/soc/intel/boards/broadwell.c:162), platform tied with it is: haswell-pcm-audio. Requested private data type by broadwell_rtd_init
- struct sst_pdata *. MATCH.
the machine drivers uses snd_soc_rtdcom_lookup(rtd, DRV_NAME);
How is DRV_NAME connected to haswell-pcm-audio?
I must be missing something in your logic.
Please checkout sst-acpi.c file and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.
what happens in sst-acpi.c stays in sst-acpi.c I don't get how you retrieve the pdata in the machine driver from *another* driver. Different devices, different platform data.
DAI is tied with platform device called "haswell-pcm-audio" whereas machine board is represented by "broadwell-audio" platform deivce. Which part is still unclear?
On 2019-08-23 09:27, Cezary Rojewski wrote:
On 2019-08-22 22:44, Pierre-Louis Bossart wrote:
Please checkout sst-acpi.c file and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.
what happens in sst-acpi.c stays in sst-acpi.c I don't get how you retrieve the pdata in the machine driver from *another* driver. Different devices, different platform data.
DAI is tied with platform device called "haswell-pcm-audio" whereas machine board is represented by "broadwell-audio" platform deivce. Which part is still unclear?
Did what you ask and must say, results are not entirely unexpected..
Change:
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index db7e1e87156d..ee52564437c3 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -126,7 +126,8 @@ static const struct snd_soc_ops broadwell_rt286_ops = { static int broadwell_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); - struct sst_pdata *pdata = dev_get_platdata(component->dev); + struct snd_soc_acpi_mach *mach = dev_get_platdata(component->dev); + struct sst_pdata *pdata = mach->pdata; struct sst_hsw *broadwell = pdata->dsp; int ret;
Generates:
[ 24.841747] hsw-acpi INT3438:00: DesignWare DMA Controller, 8 channels [ 24.862260] haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2 [ 24.862320] haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2) [ 24.862924] haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19 [ 24.946651] rt286 i2c-INT343A:00: ASoC: sink widget DMIC1 overwritten [ 24.946882] rt286 i2c-INT343A:00: ASoC: source widget DMIC1 overwritten [ 24.948251] ================================================================== [ 24.948275] BUG: KASAN: user-memory-access in _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948290] Write of size 4 at addr 0000010400000000 by task systemd-udevd/292
[ 24.948313] CPU: 1 PID: 292 Comm: systemd-udevd Not tainted 5.3.0-rc4+ #111 [ 24.948317] Hardware name: Intel Corporation Broadwell Client platform/Pearl Valley, BIOS BDW-E1R1.86C.0119.R01.1503252201 03/25/2015 [ 24.948319] Call Trace: [ 24.948327] dump_stack+0x71/0xab [ 24.948334] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948339] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948346] __kasan_report+0x176/0x192 [ 24.948352] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948359] kasan_report+0xe/0x20 [ 24.948366] check_memory_region+0x149/0x1a0 [ 24.948372] _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948378] ? _raw_write_lock_bh+0xe0/0xe0 [ 24.948426] ? snd_soc_dapm_add_route+0x2da/0x4f0 [snd_soc_core] [ 24.948435] ipc_tx_message+0xa8/0x540 [snd_soc_sst_ipc] [ 24.948485] ? snd_soc_dapm_add_path+0x9c0/0x9c0 [snd_soc_core] [ 24.948490] ? 0xffffffffc0bc0000 [ 24.948512] ? snd_ctl_dev_free+0x80/0x80 [snd] [ 24.948522] sst_ipc_tx_message_wait+0x63/0xb0 [snd_soc_sst_ipc] [ 24.948545] sst_hsw_device_set_config+0x13f/0x2d0 [snd_soc_sst_haswell_pcm] [ 24.948552] ? mutex_unlock+0x1d/0x40 [ 24.948572] ? hsw_notification_work+0x2c0/0x2c0 [snd_soc_sst_haswell_pcm] [ 24.948578] ? strcmp+0x30/0x50 [ 24.948584] ? strcmp+0x30/0x50 [ 24.948597] broadwell_rtd_init+0x68/0xa0 [snd_soc_sst_broadwell] [ 24.948642] snd_soc_instantiate_card+0xd81/0x1720 [snd_soc_core] [ 24.948690] ? soc_cleanup_card_resources+0x5a0/0x5a0 [snd_soc_core] [ 24.948697] ? __kasan_kmalloc.constprop.8+0xa0/0xd0 [ 24.948703] ? __kmalloc_node_track_caller+0xf3/0x320 [ 24.948749] snd_soc_register_card+0x25b/0x280 [snd_soc_core] [ 24.948756] ? devres_alloc_node+0x55/0x70 [ 24.948801] devm_snd_soc_register_card+0x3c/0x80 [snd_soc_core] [ 24.948809] platform_drv_probe+0x4d/0xb0 [ 24.948816] really_probe+0x35c/0x5c0 [ 24.948824] driver_probe_device+0x181/0x1b0 [ 24.948831] device_driver_attach+0x8a/0x90 [ 24.948838] ? device_driver_attach+0x90/0x90 [ 24.948843] __driver_attach+0xc1/0x190 [ 24.948849] ? device_driver_attach+0x90/0x90 [ 24.948854] bus_for_each_dev+0xe6/0x140 [ 24.948859] ? _raw_write_trylock+0xe0/0xe0 [ 24.948865] ? subsys_dev_iter_exit+0x10/0x10 [ 24.948871] ? klist_node_init+0x61/0x90 [ 24.948878] bus_add_driver+0x212/0x310 [ 24.948887] driver_register+0xcf/0x1b0 [ 24.948892] ? 0xffffffffc09d8000 [ 24.948900] do_one_initcall+0x8b/0x2b4 [ 24.948907] ? trace_event_raw_event_initcall_finish+0x140/0x140 [ 24.948914] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948921] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948926] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948935] do_init_module+0xe5/0x364 [ 24.948941] load_module+0x4385/0x4b80 [ 24.948960] ? module_frob_arch_sections+0x20/0x20 [ 24.948965] ? ima_read_file+0x10/0x10 [ 24.948971] ? vfs_read+0xc2/0x1a0 [ 24.948978] ? kernel_read+0x95/0xb0 [ 24.948984] ? kernel_read_file+0x14a/0x330 [ 24.948992] ? get_unmapped_area+0x16c/0x1c0 [ 24.949000] ? __do_sys_finit_module+0x193/0x1c0 [ 24.949005] __do_sys_finit_module+0x193/0x1c0 [ 24.949012] ? __ia32_sys_init_module+0x40/0x40 [ 24.949019] ? __do_sys_newfstat+0x7c/0xd0 [ 24.949029] do_syscall_64+0x73/0x1b0 [ 24.949037] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 24.949042] RIP: 0033:0x7f03ff6124d9 [ 24.949049] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8f 29 2c 00 f7 d8 64 89 01 48 [ 24.949052] RSP: 002b:00007ffd2a5fcce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 24.949058] RAX: ffffffffffffffda RBX: 000055c89fafe8e0 RCX: 00007f03ff6124d9 [ 24.949062] RDX: 0000000000000000 RSI: 00007f03ffb08e23 RDI: 0000000000000012 [ 24.949065] RBP: 00007f03ffb08e23 R08: 0000000000000000 R09: 0000000000000000 [ 24.949068] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000 [ 24.949072] R13: 000055c89fb33d50 R14: 0000000000020000 R15: 000000000aba9500 [ 24.949077] ================================================================== [ 24.949092] Disabling lock debugging due to kernel taint [ 24.949100] BUG: unable to handle page fault for address: 0000010400000000 [ 24.949113] #PF: supervisor write access in kernel mode [ 24.949125] #PF: error_code(0x0002) - not-present page [ 24.949135] PGD 0 P4D 0 [ 24.949147] Oops: 0002 [#1] SMP KASAN PTI [ 24.949160] CPU: 1 PID: 292 Comm: systemd-udevd Tainted: G B 5.3.0-rc4+ #111 [ 24.949176] Hardware name: Intel Corporation Broadwell Client platform/Pearl Valley, BIOS BDW-E1R1.86C.0119.R01.1503252201 03/25/2015 [ 24.949198] RIP: 0010:_raw_spin_lock_irqsave+0x96/0xf0 [ 24.949212] Code: be 04 00 00 00 c7 44 24 20 00 00 00 00 e8 82 00 3b ff 48 8d 7c 24 20 be 04 00 00 00 e8 73 00 3b ff ba 01 00 00 00 8b 44 24 20 <f0> 0f b1 13 75 2f 48 b8 00 00 00 00 00 fc ff df 48 c7 44 05 00 00 [ 24.949238] RSP: 0018:ffff888116d9f350 EFLAGS: 00010097 [ 24.949251] RAX: 0000000000000000 RBX: 0000010400000000 RCX: ffffffffad01224d [ 24.949264] RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888116d9f370 [ 24.949277] RBP: 1ffff11022db3e6a R08: 0000000000000004 R09: ffffed1022db3e6e [ 24.949291] R10: 0000000000000001 R11: ffffed1022db3e6e R12: 0000000000000246 [ 24.949304] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000010 [ 24.949320] FS: 00007f04007898c0(0000) GS:ffff888129480000(0000) knlGS:0000000000000000 [ 24.949335] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.949348] CR2: 0000010400000000 CR3: 00000001277cc006 CR4: 00000000003606e0 [ 24.949360] Call Trace: [ 24.949372] ? _raw_write_lock_bh+0xe0/0xe0 [ 24.949426] ? snd_soc_dapm_add_route+0x2da/0x4f0 [snd_soc_core] [ 24.949445] ipc_tx_message+0xa8/0x540 [snd_soc_sst_ipc] [ 24.949501] ? snd_soc_dapm_add_path+0x9c0/0x9c0 [snd_soc_core] [ 24.949515] ? 0xffffffffc0bc0000 [ 24.949543] ? snd_ctl_dev_free+0x80/0x80 [snd] [ 24.949560] sst_ipc_tx_message_wait+0x63/0xb0 [snd_soc_sst_ipc] [ 24.949592] sst_hsw_device_set_config+0x13f/0x2d0 [snd_soc_sst_haswell_pcm] [ 24.949609] ? mutex_unlock+0x1d/0x40 [ 24.949636] ? hsw_notification_work+0x2c0/0x2c0 [snd_soc_sst_haswell_pcm] [ 24.949651] ? strcmp+0x30/0x50 [ 24.949664] ? strcmp+0x30/0x50 [ 24.949682] broadwell_rtd_init+0x68/0xa0 [snd_soc_sst_broadwell] [ 24.949736] snd_soc_instantiate_card+0xd81/0x1720 [snd_soc_core] [ 24.949795] ? soc_cleanup_card_resources+0x5a0/0x5a0 [snd_soc_core] [ 24.949811] ? __kasan_kmalloc.constprop.8+0xa0/0xd0 [ 24.949825] ? __kmalloc_node_track_caller+0xf3/0x320 [ 24.949879] snd_soc_register_card+0x25b/0x280 [snd_soc_core] [ 24.949894] ? devres_alloc_node+0x55/0x70 [ 24.949947] devm_snd_soc_register_card+0x3c/0x80 [snd_soc_core] [ 24.949964] platform_drv_probe+0x4d/0xb0 [ 24.949978] really_probe+0x35c/0x5c0 [ 24.949992] driver_probe_device+0x181/0x1b0 [ 24.950006] device_driver_attach+0x8a/0x90 [ 24.950020] ? device_driver_attach+0x90/0x90 [ 24.950033] __driver_attach+0xc1/0x190 [ 24.950047] ? device_driver_attach+0x90/0x90 [ 24.950059] bus_for_each_dev+0xe6/0x140 [ 24.950071] ? _raw_write_trylock+0xe0/0xe0 [ 24.950084] ? subsys_dev_iter_exit+0x10/0x10 [ 24.950097] ? klist_node_init+0x61/0x90 [ 24.950111] bus_add_driver+0x212/0x310 [ 24.950126] driver_register+0xcf/0x1b0 [ 24.950138] ? 0xffffffffc09d8000 [ 24.950151] do_one_initcall+0x8b/0x2b4 [ 24.950165] ? trace_event_raw_event_initcall_finish+0x140/0x140 [ 24.950181] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950196] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950209] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950224] do_init_module+0xe5/0x364 [ 24.950238] load_module+0x4385/0x4b80 [ 24.950262] ? module_frob_arch_sections+0x20/0x20 [ 24.950275] ? ima_read_file+0x10/0x10 [ 24.950288] ? vfs_read+0xc2/0x1a0 [ 24.950300] ? kernel_read+0x95/0xb0 [ 24.950314] ? kernel_read_file+0x14a/0x330 [ 24.950328] ? get_unmapped_area+0x16c/0x1c0 [ 24.950343] ? __do_sys_finit_module+0x193/0x1c0 [ 24.950356] __do_sys_finit_module+0x193/0x1c0 [ 24.950369] ? __ia32_sys_init_module+0x40/0x40 [ 24.950384] ? __do_sys_newfstat+0x7c/0xd0 [ 24.950401] do_syscall_64+0x73/0x1b0 [ 24.950415] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 24.950427] RIP: 0033:0x7f03ff6124d9 [ 24.950440] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8f 29 2c 00 f7 d8 64 89 01 48 [ 24.950467] RSP: 002b:00007ffd2a5fcce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 24.950484] RAX: ffffffffffffffda RBX: 000055c89fafe8e0 RCX: 00007f03ff6124d9 [ 24.950497] RDX: 0000000000000000 RSI: 00007f03ffb08e23 RDI: 0000000000000012 [ 24.950510] RBP: 00007f03ffb08e23 R08: 0000000000000000 R09: 0000000000000000 [ 24.950523] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000 [ 24.950537] R13: 000055c89fb33d50 R14: 0000000000020000 R15: 000000000aba9500 [ 24.950551] Modules linked in: snd_soc_sst_broadwell(+) intel_rapl_msr snd_soc_sst_haswell_pcm snd_soc_sst_ipc intel_rapl_common x86_pkg_temp_thermal snd_soc_sst_firmware intel_powerclamp coretemp snd_soc_rt298 kvm_intel kvm irqbypass snd_soc_rt286 snd_soc_rl6347a crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_soc_core snd_pcm_dmaengine ac97_bus aesni_intel snd_pcm aes_x86_64 crypto_simd input_leds cryptd glue_helper snd_seq_midi serio_raw snd_seq_midi_event intel_pch_thermal snd_rawmidi mei_me mei lpc_ich snd_seq snd_seq_device soc_button_array intel_vbtn snd_timer snd_soc_hsw_acpi snd snd_soc_sst_dsp snd_soc_acpi_intel_match dw_dmac snd_soc_acpi soundcore 8250_dw intel_hid intel_pmc_core sparse_keymap acpi_pad parport_pc ppdev lp parport autofs4 i915 ahci e1000e libahci sdhci_acpi video sdhci [ 24.950717] CR2: 0000010400000000 [ 24.950728] ---[ end trace 7c9279db22368aac ]--- [ 24.950742] RIP: 0010:_raw_spin_lock_irqsave+0x96/0xf0 [ 24.950756] Code: be 04 00 00 00 c7 44 24 20 00 00 00 00 e8 82 00 3b ff 48 8d 7c 24 20 be 04 00 00 00 e8 73 00 3b ff ba 01 00 00 00 8b 44 24 20 <f0> 0f b1 13 75 2f 48 b8 00 00 00 00 00 fc ff df 48 c7 44 05 00 00 [ 24.950783] RSP: 0018:ffff888116d9f350 EFLAGS: 00010097 [ 24.950795] RAX: 0000000000000000 RBX: 0000010400000000 RCX: ffffffffad01224d [ 24.950808] RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888116d9f370 [ 24.950822] RBP: 1ffff11022db3e6a R08: 0000000000000004 R09: ffffed1022db3e6e [ 24.950835] R10: 0000000000000001 R11: ffffed1022db3e6e R12: 0000000000000246 [ 24.950848] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000010 [ 24.950863] FS: 00007f04007898c0(0000) GS:ffff888129480000(0000) knlGS:0000000000000000 [ 24.950879] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.950891] CR2: 0000010400000000 CR3: 00000001277cc006 CR4: 00000000003606e0
On 8/28/19 4:38 AM, Cezary Rojewski wrote:
On 2019-08-23 09:27, Cezary Rojewski wrote:
On 2019-08-22 22:44, Pierre-Louis Bossart wrote:
Please checkout sst-acpi.c file and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.
what happens in sst-acpi.c stays in sst-acpi.c I don't get how you retrieve the pdata in the machine driver from *another* driver. Different devices, different platform data.
DAI is tied with platform device called "haswell-pcm-audio" whereas machine board is represented by "broadwell-audio" platform deivce. Which part is still unclear?
Did what you ask and must say, results are not entirely unexpected..
Change:
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index db7e1e87156d..ee52564437c3 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -126,7 +126,8 @@ static const struct snd_soc_ops broadwell_rt286_ops = { static int broadwell_rtd_init(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME); - struct sst_pdata *pdata = dev_get_platdata(component->dev); + struct snd_soc_acpi_mach *mach = dev_get_platdata(component->dev);
that's not what I had in mind, I was talking about using rtd->card->dev and point to the same argument that is passed during the probe.
Anyways it's not much better so let's forget about it.
+ struct sst_pdata *pdata = mach->pdata; struct sst_hsw *broadwell = pdata->dsp; int ret;
Generates:
[ 24.841747] hsw-acpi INT3438:00: DesignWare DMA Controller, 8 channels [ 24.862260] haswell-pcm-audio haswell-pcm-audio: Direct firmware load for intel/IntcPP01.bin failed with error -2 [ 24.862320] haswell-pcm-audio haswell-pcm-audio: fw image intel/IntcPP01.bin not available(-2) [ 24.862924] haswell-pcm-audio haswell-pcm-audio: FW loaded, mailbox readback FW info: type 01, - version: 00.00, build 77, source commit id: 876ac6906f31a43b6772b23c7c983ce9dcb18a19 [ 24.946651] rt286 i2c-INT343A:00: ASoC: sink widget DMIC1 overwritten [ 24.946882] rt286 i2c-INT343A:00: ASoC: source widget DMIC1 overwritten [ 24.948251] ================================================================== [ 24.948275] BUG: KASAN: user-memory-access in _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948290] Write of size 4 at addr 0000010400000000 by task systemd-udevd/292
[ 24.948313] CPU: 1 PID: 292 Comm: systemd-udevd Not tainted 5.3.0-rc4+ #111 [ 24.948317] Hardware name: Intel Corporation Broadwell Client platform/Pearl Valley, BIOS BDW-E1R1.86C.0119.R01.1503252201 03/25/2015 [ 24.948319] Call Trace: [ 24.948327] dump_stack+0x71/0xab [ 24.948334] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948339] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948346] __kasan_report+0x176/0x192 [ 24.948352] ? _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948359] kasan_report+0xe/0x20 [ 24.948366] check_memory_region+0x149/0x1a0 [ 24.948372] _raw_spin_lock_irqsave+0x7e/0xf0 [ 24.948378] ? _raw_write_lock_bh+0xe0/0xe0 [ 24.948426] ? snd_soc_dapm_add_route+0x2da/0x4f0 [snd_soc_core] [ 24.948435] ipc_tx_message+0xa8/0x540 [snd_soc_sst_ipc] [ 24.948485] ? snd_soc_dapm_add_path+0x9c0/0x9c0 [snd_soc_core] [ 24.948490] ? 0xffffffffc0bc0000 [ 24.948512] ? snd_ctl_dev_free+0x80/0x80 [snd] [ 24.948522] sst_ipc_tx_message_wait+0x63/0xb0 [snd_soc_sst_ipc] [ 24.948545] sst_hsw_device_set_config+0x13f/0x2d0 [snd_soc_sst_haswell_pcm] [ 24.948552] ? mutex_unlock+0x1d/0x40 [ 24.948572] ? hsw_notification_work+0x2c0/0x2c0 [snd_soc_sst_haswell_pcm] [ 24.948578] ? strcmp+0x30/0x50 [ 24.948584] ? strcmp+0x30/0x50 [ 24.948597] broadwell_rtd_init+0x68/0xa0 [snd_soc_sst_broadwell] [ 24.948642] snd_soc_instantiate_card+0xd81/0x1720 [snd_soc_core] [ 24.948690] ? soc_cleanup_card_resources+0x5a0/0x5a0 [snd_soc_core] [ 24.948697] ? __kasan_kmalloc.constprop.8+0xa0/0xd0 [ 24.948703] ? __kmalloc_node_track_caller+0xf3/0x320 [ 24.948749] snd_soc_register_card+0x25b/0x280 [snd_soc_core] [ 24.948756] ? devres_alloc_node+0x55/0x70 [ 24.948801] devm_snd_soc_register_card+0x3c/0x80 [snd_soc_core] [ 24.948809] platform_drv_probe+0x4d/0xb0 [ 24.948816] really_probe+0x35c/0x5c0 [ 24.948824] driver_probe_device+0x181/0x1b0 [ 24.948831] device_driver_attach+0x8a/0x90 [ 24.948838] ? device_driver_attach+0x90/0x90 [ 24.948843] __driver_attach+0xc1/0x190 [ 24.948849] ? device_driver_attach+0x90/0x90 [ 24.948854] bus_for_each_dev+0xe6/0x140 [ 24.948859] ? _raw_write_trylock+0xe0/0xe0 [ 24.948865] ? subsys_dev_iter_exit+0x10/0x10 [ 24.948871] ? klist_node_init+0x61/0x90 [ 24.948878] bus_add_driver+0x212/0x310 [ 24.948887] driver_register+0xcf/0x1b0 [ 24.948892] ? 0xffffffffc09d8000 [ 24.948900] do_one_initcall+0x8b/0x2b4 [ 24.948907] ? trace_event_raw_event_initcall_finish+0x140/0x140 [ 24.948914] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948921] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948926] ? kasan_unpoison_shadow+0x30/0x40 [ 24.948935] do_init_module+0xe5/0x364 [ 24.948941] load_module+0x4385/0x4b80 [ 24.948960] ? module_frob_arch_sections+0x20/0x20 [ 24.948965] ? ima_read_file+0x10/0x10 [ 24.948971] ? vfs_read+0xc2/0x1a0 [ 24.948978] ? kernel_read+0x95/0xb0 [ 24.948984] ? kernel_read_file+0x14a/0x330 [ 24.948992] ? get_unmapped_area+0x16c/0x1c0 [ 24.949000] ? __do_sys_finit_module+0x193/0x1c0 [ 24.949005] __do_sys_finit_module+0x193/0x1c0 [ 24.949012] ? __ia32_sys_init_module+0x40/0x40 [ 24.949019] ? __do_sys_newfstat+0x7c/0xd0 [ 24.949029] do_syscall_64+0x73/0x1b0 [ 24.949037] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 24.949042] RIP: 0033:0x7f03ff6124d9 [ 24.949049] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8f 29 2c 00 f7 d8 64 89 01 48 [ 24.949052] RSP: 002b:00007ffd2a5fcce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 24.949058] RAX: ffffffffffffffda RBX: 000055c89fafe8e0 RCX: 00007f03ff6124d9 [ 24.949062] RDX: 0000000000000000 RSI: 00007f03ffb08e23 RDI: 0000000000000012 [ 24.949065] RBP: 00007f03ffb08e23 R08: 0000000000000000 R09: 0000000000000000 [ 24.949068] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000 [ 24.949072] R13: 000055c89fb33d50 R14: 0000000000020000 R15: 000000000aba9500 [ 24.949077] ================================================================== [ 24.949092] Disabling lock debugging due to kernel taint [ 24.949100] BUG: unable to handle page fault for address: 0000010400000000 [ 24.949113] #PF: supervisor write access in kernel mode [ 24.949125] #PF: error_code(0x0002) - not-present page [ 24.949135] PGD 0 P4D 0 [ 24.949147] Oops: 0002 [#1] SMP KASAN PTI [ 24.949160] CPU: 1 PID: 292 Comm: systemd-udevd Tainted: G B 5.3.0-rc4+ #111 [ 24.949176] Hardware name: Intel Corporation Broadwell Client platform/Pearl Valley, BIOS BDW-E1R1.86C.0119.R01.1503252201 03/25/2015 [ 24.949198] RIP: 0010:_raw_spin_lock_irqsave+0x96/0xf0 [ 24.949212] Code: be 04 00 00 00 c7 44 24 20 00 00 00 00 e8 82 00 3b ff 48 8d 7c 24 20 be 04 00 00 00 e8 73 00 3b ff ba 01 00 00 00 8b 44 24 20 <f0> 0f b1 13 75 2f 48 b8 00 00 00 00 00 fc ff df 48 c7 44 05 00 00 [ 24.949238] RSP: 0018:ffff888116d9f350 EFLAGS: 00010097 [ 24.949251] RAX: 0000000000000000 RBX: 0000010400000000 RCX: ffffffffad01224d [ 24.949264] RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888116d9f370 [ 24.949277] RBP: 1ffff11022db3e6a R08: 0000000000000004 R09: ffffed1022db3e6e [ 24.949291] R10: 0000000000000001 R11: ffffed1022db3e6e R12: 0000000000000246 [ 24.949304] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000010 [ 24.949320] FS: 00007f04007898c0(0000) GS:ffff888129480000(0000) knlGS:0000000000000000 [ 24.949335] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.949348] CR2: 0000010400000000 CR3: 00000001277cc006 CR4: 00000000003606e0 [ 24.949360] Call Trace: [ 24.949372] ? _raw_write_lock_bh+0xe0/0xe0 [ 24.949426] ? snd_soc_dapm_add_route+0x2da/0x4f0 [snd_soc_core] [ 24.949445] ipc_tx_message+0xa8/0x540 [snd_soc_sst_ipc] [ 24.949501] ? snd_soc_dapm_add_path+0x9c0/0x9c0 [snd_soc_core] [ 24.949515] ? 0xffffffffc0bc0000 [ 24.949543] ? snd_ctl_dev_free+0x80/0x80 [snd] [ 24.949560] sst_ipc_tx_message_wait+0x63/0xb0 [snd_soc_sst_ipc] [ 24.949592] sst_hsw_device_set_config+0x13f/0x2d0 [snd_soc_sst_haswell_pcm] [ 24.949609] ? mutex_unlock+0x1d/0x40 [ 24.949636] ? hsw_notification_work+0x2c0/0x2c0 [snd_soc_sst_haswell_pcm] [ 24.949651] ? strcmp+0x30/0x50 [ 24.949664] ? strcmp+0x30/0x50 [ 24.949682] broadwell_rtd_init+0x68/0xa0 [snd_soc_sst_broadwell] [ 24.949736] snd_soc_instantiate_card+0xd81/0x1720 [snd_soc_core] [ 24.949795] ? soc_cleanup_card_resources+0x5a0/0x5a0 [snd_soc_core] [ 24.949811] ? __kasan_kmalloc.constprop.8+0xa0/0xd0 [ 24.949825] ? __kmalloc_node_track_caller+0xf3/0x320 [ 24.949879] snd_soc_register_card+0x25b/0x280 [snd_soc_core] [ 24.949894] ? devres_alloc_node+0x55/0x70 [ 24.949947] devm_snd_soc_register_card+0x3c/0x80 [snd_soc_core] [ 24.949964] platform_drv_probe+0x4d/0xb0 [ 24.949978] really_probe+0x35c/0x5c0 [ 24.949992] driver_probe_device+0x181/0x1b0 [ 24.950006] device_driver_attach+0x8a/0x90 [ 24.950020] ? device_driver_attach+0x90/0x90 [ 24.950033] __driver_attach+0xc1/0x190 [ 24.950047] ? device_driver_attach+0x90/0x90 [ 24.950059] bus_for_each_dev+0xe6/0x140 [ 24.950071] ? _raw_write_trylock+0xe0/0xe0 [ 24.950084] ? subsys_dev_iter_exit+0x10/0x10 [ 24.950097] ? klist_node_init+0x61/0x90 [ 24.950111] bus_add_driver+0x212/0x310 [ 24.950126] driver_register+0xcf/0x1b0 [ 24.950138] ? 0xffffffffc09d8000 [ 24.950151] do_one_initcall+0x8b/0x2b4 [ 24.950165] ? trace_event_raw_event_initcall_finish+0x140/0x140 [ 24.950181] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950196] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950209] ? kasan_unpoison_shadow+0x30/0x40 [ 24.950224] do_init_module+0xe5/0x364 [ 24.950238] load_module+0x4385/0x4b80 [ 24.950262] ? module_frob_arch_sections+0x20/0x20 [ 24.950275] ? ima_read_file+0x10/0x10 [ 24.950288] ? vfs_read+0xc2/0x1a0 [ 24.950300] ? kernel_read+0x95/0xb0 [ 24.950314] ? kernel_read_file+0x14a/0x330 [ 24.950328] ? get_unmapped_area+0x16c/0x1c0 [ 24.950343] ? __do_sys_finit_module+0x193/0x1c0 [ 24.950356] __do_sys_finit_module+0x193/0x1c0 [ 24.950369] ? __ia32_sys_init_module+0x40/0x40 [ 24.950384] ? __do_sys_newfstat+0x7c/0xd0 [ 24.950401] do_syscall_64+0x73/0x1b0 [ 24.950415] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 24.950427] RIP: 0033:0x7f03ff6124d9 [ 24.950440] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8f 29 2c 00 f7 d8 64 89 01 48 [ 24.950467] RSP: 002b:00007ffd2a5fcce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 24.950484] RAX: ffffffffffffffda RBX: 000055c89fafe8e0 RCX: 00007f03ff6124d9 [ 24.950497] RDX: 0000000000000000 RSI: 00007f03ffb08e23 RDI: 0000000000000012 [ 24.950510] RBP: 00007f03ffb08e23 R08: 0000000000000000 R09: 0000000000000000 [ 24.950523] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000 [ 24.950537] R13: 000055c89fb33d50 R14: 0000000000020000 R15: 000000000aba9500 [ 24.950551] Modules linked in: snd_soc_sst_broadwell(+) intel_rapl_msr snd_soc_sst_haswell_pcm snd_soc_sst_ipc intel_rapl_common x86_pkg_temp_thermal snd_soc_sst_firmware intel_powerclamp coretemp snd_soc_rt298 kvm_intel kvm irqbypass snd_soc_rt286 snd_soc_rl6347a crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_soc_core snd_pcm_dmaengine ac97_bus aesni_intel snd_pcm aes_x86_64 crypto_simd input_leds cryptd glue_helper snd_seq_midi serio_raw snd_seq_midi_event intel_pch_thermal snd_rawmidi mei_me mei lpc_ich snd_seq snd_seq_device soc_button_array intel_vbtn snd_timer snd_soc_hsw_acpi snd snd_soc_sst_dsp snd_soc_acpi_intel_match dw_dmac snd_soc_acpi soundcore 8250_dw intel_hid intel_pmc_core sparse_keymap acpi_pad parport_pc ppdev lp parport autofs4 i915 ahci e1000e libahci sdhci_acpi video sdhci [ 24.950717] CR2: 0000010400000000 [ 24.950728] ---[ end trace 7c9279db22368aac ]--- [ 24.950742] RIP: 0010:_raw_spin_lock_irqsave+0x96/0xf0 [ 24.950756] Code: be 04 00 00 00 c7 44 24 20 00 00 00 00 e8 82 00 3b ff 48 8d 7c 24 20 be 04 00 00 00 e8 73 00 3b ff ba 01 00 00 00 8b 44 24 20 <f0> 0f b1 13 75 2f 48 b8 00 00 00 00 00 fc ff df 48 c7 44 05 00 00 [ 24.950783] RSP: 0018:ffff888116d9f350 EFLAGS: 00010097 [ 24.950795] RAX: 0000000000000000 RBX: 0000010400000000 RCX: ffffffffad01224d [ 24.950808] RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffff888116d9f370 [ 24.950822] RBP: 1ffff11022db3e6a R08: 0000000000000004 R09: ffffed1022db3e6e [ 24.950835] R10: 0000000000000001 R11: ffffed1022db3e6e R12: 0000000000000246 [ 24.950848] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000010 [ 24.950863] FS: 00007f04007898c0(0000) GS:ffff888129480000(0000) knlGS:0000000000000000 [ 24.950879] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.950891] CR2: 0000010400000000 CR3: 00000001277cc006 CR4: 00000000003606e0 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
The patch
ASoC: Intel: Haswell: Adjust machine device private context
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
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
From ca964edf0ddbfec2cb10b3d251d09598e7ca9b13 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 22 Aug 2019 13:36:13 +0200 Subject: [PATCH] ASoC: Intel: Haswell: Adjust machine device private context
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.
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@intel.com Link: https://lore.kernel.org/r/20190822113616.22702-2-cezary.rojewski@intel.com Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 0e8e0a7a11df..5854868650b9 100644 --- a/sound/soc/intel/common/sst-acpi.c +++ b/sound/soc/intel/common/sst-acpi.c @@ -141,11 +141,12 @@ static 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);
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/haswell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c index 4d3822cff98c..3dadf9bff796 100644 --- a/sound/soc/intel/boards/haswell.c +++ b/sound/soc/intel/boards/haswell.c @@ -188,18 +188,14 @@ static struct snd_soc_card haswell_rt5640 = { static int haswell_audio_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
haswell_rt5640.dev = &pdev->dev;
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&haswell_rt5640, - platform_name); + mach->mach_params.platform); if (ret) return ret;
The patch
ASoC: Intel: haswell: Simplify device probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
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
From 1fc3e6b2ac3f8125ae54685c600d594d690a268a Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 22 Aug 2019 13:36:14 +0200 Subject: [PATCH] ASoC: Intel: haswell: Simplify device probe
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Link: https://lore.kernel.org/r/20190822113616.22702-3-cezary.rojewski@intel.com Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/haswell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c index 4d3822cff98c..3dadf9bff796 100644 --- a/sound/soc/intel/boards/haswell.c +++ b/sound/soc/intel/boards/haswell.c @@ -188,18 +188,14 @@ static struct snd_soc_card haswell_rt5640 = { static int haswell_audio_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
haswell_rt5640.dev = &pdev->dev;
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&haswell_rt5640, - platform_name); + mach->mach_params.platform); if (ret) return ret;
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw-rt5677.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index e8e9c3dc82a5..4a4d3353e26d 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -340,7 +340,6 @@ static int bdw_rt5677_probe(struct platform_device *pdev) { struct bdw_rt5677_priv *bdw_rt5677; struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
bdw_rt5677_card.dev = &pdev->dev; @@ -355,11 +354,8 @@ static int bdw_rt5677_probe(struct platform_device *pdev)
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt5677_card, - platform_name); + mach->mach_params.platform); if (ret) return ret;
The patch
ASoC: Intel: bdw-rt5677: Simplify device probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
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
From c25e93bba90b3f194c43a37fe2fcdb0727c4ab84 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 22 Aug 2019 13:36:15 +0200 Subject: [PATCH] ASoC: Intel: bdw-rt5677: Simplify device probe
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Link: https://lore.kernel.org/r/20190822113616.22702-4-cezary.rojewski@intel.com Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bdw-rt5677.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index e8e9c3dc82a5..4a4d3353e26d 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -340,7 +340,6 @@ static int bdw_rt5677_probe(struct platform_device *pdev) { struct bdw_rt5677_priv *bdw_rt5677; struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
bdw_rt5677_card.dev = &pdev->dev; @@ -355,11 +354,8 @@ static int bdw_rt5677_probe(struct platform_device *pdev)
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&bdw_rt5677_card, - platform_name); + mach->mach_params.platform); if (ret) return ret;
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/broadwell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index ab38ef30dfff..db7e1e87156d 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -270,18 +270,14 @@ static struct snd_soc_card broadwell_rt286 = { static int broadwell_audio_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
broadwell_rt286.dev = &pdev->dev;
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&broadwell_rt286, - platform_name); + mach->mach_params.platform); if (ret) return ret;
The patch
ASoC: Intel: broadwell: Simplify device probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
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
From 54d037d5a466ce518ee4bb81f39b79ac7d843ba6 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 22 Aug 2019 13:36:16 +0200 Subject: [PATCH] ASoC: Intel: broadwell: Simplify device probe
With legacy ADSP private context adjusted, there is no need for double safety.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Link: https://lore.kernel.org/r/20190822113616.22702-5-cezary.rojewski@intel.com Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/broadwell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index ab38ef30dfff..db7e1e87156d 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -270,18 +270,14 @@ static struct snd_soc_card broadwell_rt286 = { static int broadwell_audio_probe(struct platform_device *pdev) { struct snd_soc_acpi_mach *mach; - const char *platform_name = NULL; int ret;
broadwell_rt286.dev = &pdev->dev;
/* override plaform name, if required */ mach = (&pdev->dev)->platform_data; - if (mach) /* extra check since legacy does not pass parameters */ - platform_name = mach->mach_params.platform; - ret = snd_soc_fixup_dai_links_platform_name(&broadwell_rt286, - platform_name); + mach->mach_params.platform); if (ret) return ret;
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.
Caught by recent cleanups where content of sst_pdata was moved. Currently, despite the incorrect cast, dereferenced field points happily to NULL (uninitialized field), so no panics were observed.
No issues seen on Broadwell w/ rt5677 (Samus) so
Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
I think the two broadwell/bdw-rt5677 cases are the only ones used in products, I am not aware of anyone using the haswell machine driver.
Sorry for being thick with the review, I couldn't figure out from the patch itself how the code worked and what this DRV_NAME represented. It's definitively legit when looking at the entire tree, but I discovered that the 'DRV_NAME' is defined 3 times, see below. It's just lunacy to use the same define with different strings in different Intel drivers, we should clean this up with a prefix to avoid ambiguities.
atom/sst-mfld-platform.h:#define DRV_NAME "sst" baytrail/sst-baytrail-pcm.c:#define DRV_NAME "byt-dai" haswell/sst-haswell-ipc.h:#define DRV_NAME "haswell-dai"
Cezary Rojewski (4): ASoC: Intel: Haswell: Adjust machine device private context ASoC: Intel: haswell: Simplify device probe ASoC: Intel: bdw-rt5677: Simplify device probe ASoC: Intel: broadwell: Simplify device probe
sound/soc/intel/boards/bdw-rt5677.c | 6 +----- sound/soc/intel/boards/broadwell.c | 6 +----- sound/soc/intel/boards/haswell.c | 6 +----- sound/soc/intel/common/sst-acpi.c | 3 ++- 4 files changed, 5 insertions(+), 16 deletions(-)
participants (3)
-
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart