ASoC: Intel: Haswell: Adjust machine device private
Subsystems (3)
Cezary Rojewski
maintainercover 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(-)
Signature · 1 line
2.17.1
Cezary Rojewski
maintainerPATCH 1/4 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.
--- sound/soc/intel/common/sst-acpi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
sound/soc/intel/common/sst-acpi.c +2 -1
@@ -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);
Signature · 1 line
2.17.1
Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 11 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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...
Quoted 27 lines
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 <[email protected]>--- sound/soc/intel/common/sst-acpi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +2
-1@@ -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);
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 21 lines · Pierre-Louis Bossart
Quoted 11 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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)."
Quoted 8 lines
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
Quoted 8 lines
Quoted 8 lines
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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 17 lines
Quoted 9 lines
sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 7 lines
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))
returnPTR_ERR(sst_acpi->pdev_mach);
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
In reply to Cezary RojewskiQuoted 25 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>--- sound/soc/intel/common/sst-acpi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +0
-0
sound/soc/intel/common/sst-acpi.c int sst_acpi_probe(struct platform_device *pdev) · line 117
Quoted 10 lines
sound/soc/intel/common/sst-acpi.c +2
-1@@ -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?
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 43 lines · Pierre-Louis Bossart
Quoted 29 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 8 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd){
structsnd_soc_component*component = snd_soc_rtdcom_lookup(rtd,DRV_NAME);
structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 50 lines · Cezary Rojewski
Quoted 49 lines · Pierre-Louis Bossart
Quoted 29 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_hsw*broadwell =pdata->dsp;
Quoted 5 lines
Quoted 1 line
<<< 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_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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?
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 57 lines · Pierre-Louis Bossart
Quoted 54 lines · Cezary Rojewski
Quoted 43 lines · Pierre-Louis Bossart
Quoted 29 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 7 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
Quoted 10 lines
Quoted 1 line
There is an in-place registration for machine device - whose
private_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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); structsst_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
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 81 lines · Cezary Rojewski
Quoted 57 lines · Pierre-Louis Bossart
Quoted 54 lines · Cezary Rojewski
Quoted 43 lines · Pierre-Louis Bossart
Quoted 29 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 7 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
Quoted 10 lines
Quoted 1 line
There is an in-place registration for machine device - whose
private_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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); structsst_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 newplatform_device- which represents machine board - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_audio_probe: structsnd_soc_acpi_mach*. MISMATCH.
Quoted 8 lines
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 newplatform_device- which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_rtd_init- structsst_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.
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 97 lines · Pierre-Louis Bossart
Quoted 84 lines · Cezary Rojewski
Quoted 58 lines · Pierre-Louis Bossart
Quoted 55 lines · Cezary Rojewski
Quoted 44 lines · Pierre-Louis Bossart
Quoted 30 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 7 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
Quoted 10 lines
Quoted 1 line
There is an in-place registration for machine device - whose
private_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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); structsst_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 newplatform_device- which represents machine board - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_audio_probe: structsnd_soc_acpi_mach*. MISMATCH.Code:
Quoted 8 lines
/* 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 newplatform_device- which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_rtd_init- structsst_pdata*. MATCH.
Quoted 6 lines
the machine drivers uses
snd_soc_rtdcom_lookup(rtd, DRV_NAME);How is
DRV_NAMEconnected 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.
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 97 lines · Cezary Rojewski
Quoted 96 lines · Pierre-Louis Bossart
Quoted 91 lines · Cezary Rojewski
Quoted 58 lines · Pierre-Louis Bossart
Quoted 55 lines · Cezary Rojewski
Quoted 44 lines · Pierre-Louis Bossart
Quoted 30 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 7 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
Quoted 10 lines
Quoted 1 line
There is an in-place registration for machine device - whose
private_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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); structsst_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 newplatform_device- which represents machine board - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_audio_probe: structsnd_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 newplatform_device- which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_rtd_init
- structsst_pdata*. MATCH.
Quoted 8 lines
Quoted 6 lines
the machine drivers uses
snd_soc_rtdcom_lookup(rtd, DRV_NAME);How is
DRV_NAMEconnected to haswell-pcm-audio?I must be missing something in your logic.
Please checkout
sst-acpi.cfile 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.
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 105 lines · Pierre-Louis Bossart
Quoted 102 lines · Cezary Rojewski
Quoted 101 lines · Pierre-Louis Bossart
Quoted 85 lines · Cezary Rojewski
Quoted 59 lines · Pierre-Louis Bossart
Quoted 56 lines · Cezary Rojewski
Quoted 45 lines · Pierre-Louis Bossart
Quoted 31 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_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 <[email protected]>---
sound/soc/intel/common/sst-acpi.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)sound/soc/intel/common/sst-acpi.c +1
-0@@ -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 */Quoted 4 lines
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?
Quoted 7 lines
static int
bdw_rt5677_rtd_init(struct snd_soc_pcm_runtime *rtd)
{
structsnd_soc_component*component =snd_soc_rtdcom_lookup(rtd, DRV_NAME); structsst_pdata*pdata =dev_get_platdata(component->dev); structsst_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.
Quoted 10 lines
Quoted 1 line
There is an in-place registration for machine device - whose
private_datagets used in machine probe - and pcm device which happens on firmware load callback (/sound/soc/intel/common/sst-acpi:63)._rtd_initmakes 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); structsst_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 newplatform_device- which represents machine board - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_audio_probe: structsnd_soc_acpi_mach*. MISMATCH.Code:
Quoted 7 lines
/* 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 newplatform_device- which represents Haswell PCM, you may treat it as Skylake equivalent - with its private data set to pointer to instance of structsst_pdatatype. 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 bybroadwell_rtd_init
- structsst_pdata*. MATCH.the machine drivers uses
snd_soc_rtdcom_lookup(rtd, DRV_NAME);How is
DRV_NAMEconnected to haswell-pcm-audio?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
I must be missing something in your logic.
Please checkout
sst-acpi.cfile and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.what happens in
sst-acpi.cstays insst-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?
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Cezary Rojewski
maintainerRe: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 8 lines · Cezary Rojewski
Quoted 5 lines · Pierre-Louis Bossart
Quoted 2 lines
Please checkout
sst-acpi.cfile and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.what happens in
sst-acpi.cstays insst-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:
sound/soc/intel/boards/broadwell.c +0 -0
@@ -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: 00000000003606e0Re: [PATCH 1/4] ASoC: Intel: Haswell: Adjust machine device private context
Quoted 20 lines · Cezary Rojewski
Quoted 8 lines · Cezary Rojewski
Quoted 5 lines · Pierre-Louis Bossart
Quoted 2 lines
Please checkout
sst-acpi.cfile and see declaration of legacy platform descriptors. See the names of PCM devices (platform devices) being declared.what happens in
sst-acpi.cstays insst-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:
sound/soc/intel/boards/broadwell.c +0
-0
sound/soc/intel/boards/broadwell.c static const struct snd_soc_ops broadwell_rt286_ops = { · line 130
Quoted 6 lines
sound/soc/intel/boards/broadwell.c +1
-1@@ -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.
Quoted 182 lines
sound/soc/intel/boards/broadwell.c +1
-0+ 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: 00000246ORIG_RAX: 0000000000000139
[ 24.949058] RAX: ffffffffffffffda RBX:000055c89fafe8e0RCX:00007f03ff6124d9
[ 24.949062] RDX: 0000000000000000 RSI:00007f03ffb08e23RDI: 0000000000000012
[ 24.949065] RBP:00007f03ffb08e23R08: 0000000000000000 R09: 0000000000000000
[ 24.949068] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000
[ 24.949072] R13:000055c89fb33d50R14: 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:1ffff11022db3e6aR08: 0000000000000004 R09:ffffed1022db3e6e
[ 24.949291] R10: 0000000000000001 R11:ffffed1022db3e6eR12: 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:00000001277cc006CR4: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: 00000246ORIG_RAX: 0000000000000139
[ 24.950484] RAX: ffffffffffffffda RBX:000055c89fafe8e0RCX:00007f03ff6124d9
[ 24.950497] RDX: 0000000000000000 RSI:00007f03ffb08e23RDI: 0000000000000012
[ 24.950510] RBP:00007f03ffb08e23R08: 0000000000000000 R09: 0000000000000000
[ 24.950523] R10: 0000000000000012 R11: 0000000000000246 R12: 0000000000000000
[ 24.950537] R13:000055c89fb33d50R14: 0000000000020000 R15:000000000aba9500
[ 24.950551] Modules linked in:snd_soc_sst_broadwell(+)intel_rapl_msrsnd_soc_sst_haswell_pcmsnd_soc_sst_ipcintel_rapl_commonx86_pkg_temp_thermalsnd_soc_sst_firmwareintel_powerclampcoretempsnd_soc_rt298kvm_intelkvm irqbypasssnd_soc_rt286snd_soc_rl6347acrct10dif_pclmulcrc32_pclmulghash_clmulni_intelsnd_soc_coresnd_pcm_dmaengineac97_busaesni_intelsnd_pcmaes_x86_64crypto_simdinput_ledscryptdglue_helpersnd_seq_midiserio_rawsnd_seq_midi_eventintel_pch_thermalsnd_rawmidimei_memeilpc_ichsnd_seqsnd_seq_devicesoc_button_arrayintel_vbtnsnd_timersnd_soc_hsw_acpisndsnd_soc_sst_dspsnd_soc_acpi_intel_matchdw_dmacsnd_soc_acpisoundcore 8250_dwintel_hidintel_pmc_coresparse_keymapacpi_padparport_pcppdev lp parport autofs4 i915 ahci e1000e libahcisdhci_acpivideo sdhci [ 24.950717] CR2: 0000010400000000
[ 24.950728] ---[ end trace7c9279db22368aac]---
[ 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:1ffff11022db3e6aR08: 0000000000000004 R09:ffffed1022db3e6e
[ 24.950835] R10: 0000000000000001 R11:ffffed1022db3e6eR12: 0000000000000246
Quoted 8 lines
[ 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:00000001277cc006CR4:00000000003606e0
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Mark Brown
maintainerApplied "ASoC: Intel: Haswell: Adjust machine device private context" to the asoc tree
In reply to Cezary RojewskiThe 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
Quoted 1 line
From
ca964edf0ddbfec2cb10b3d251d09598e7ca9b13Mon Sep 17 00:00:00 2001
From: Cezary Rojewski <[email protected]>
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.
--- sound/soc/intel/common/sst-acpi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
sound/soc/intel/common/sst-acpi.c +2 -1
@@ -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);
Signature · 1 line
2.20.1
Cezary Rojewski
maintainerPATCH 2/4 ASoC: Intel: haswell: Simplify device probe
In reply to Cezary RojewskiWith legacy ADSP private context adjusted, there is no need for double safety.
--- sound/soc/intel/boards/haswell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/haswell.c +1 -5
@@ -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;
Signature · 1 line
2.17.1
Mark Brown
maintainerApplied "ASoC: Intel: haswell: Simplify device probe" to the asoc tree
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
Quoted 1 line
From
1fc3e6b2ac3f8125ae54685c600d594d690a268aMon Sep 17 00:00:00 2001
From: Cezary Rojewski <[email protected]>
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.
--- sound/soc/intel/boards/haswell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/haswell.c +1 -5
@@ -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;
Signature · 1 line
2.20.1
Cezary Rojewski
maintainerPATCH 3/4 ASoC: Intel: bdw-rt5677: Simplify device probe
In reply to Cezary RojewskiWith legacy ADSP private context adjusted, there is no need for double safety.
--- sound/soc/intel/boards/bdw-rt5677.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/bdw-rt5677.c +1 -5
@@ -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;
Signature · 1 line
2.17.1
Mark Brown
maintainerApplied "ASoC: Intel: bdw-rt5677: Simplify device probe" to the asoc tree
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
Quoted 1 line
From
c25e93bba90b3f194c43a37fe2fcdb0727c4ab84Mon Sep 17 00:00:00 2001
From: Cezary Rojewski <[email protected]>
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.
--- sound/soc/intel/boards/bdw-rt5677.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/bdw-rt5677.c +1 -5
@@ -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;
Signature · 1 line
2.20.1
Cezary Rojewski
maintainerPATCH 4/4 ASoC: Intel: broadwell: Simplify device probe
In reply to Cezary RojewskiWith legacy ADSP private context adjusted, there is no need for double safety.
--- sound/soc/intel/boards/broadwell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/broadwell.c +1 -5
@@ -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;
Signature · 1 line
2.17.1
Mark Brown
maintainerApplied "ASoC: Intel: broadwell: Simplify device probe" to the asoc tree
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
Quoted 1 line
From
54d037d5a466ce518ee4bb81f39b79ac7d843ba6Mon Sep 17 00:00:00 2001
From: Cezary Rojewski <[email protected]>
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.
--- sound/soc/intel/boards/broadwell.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
sound/soc/intel/boards/broadwell.c +1 -5
@@ -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;
Signature · 1 line
2.20.1
Quoted 12 lines · Cezary Rojewski
Apart from Haswell machines, all other devices have their private data set to
snd_soc_acpi_machinstance.Changes for HSW/ BDW boards introduced with series:
https://patchwork.kernel.org/cover/10782035/added support for
dai_linkplatform_nameadjustments within card probe routines. These take for grantedprivate_datapoints tosnd_soc_acpi_machwhereas for Haswell, it'ssst_pdatainstead. Change private context ofplatform_device- representing machine board - to address this.
Quoted 3 lines
Caught by recent cleanups where content of
sst_pdatawas 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
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"
Quoted 13 lines
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 probesound/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(-)