| Message ID | 20190822113616.22702-2-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ca964edf0ddbfec2cb10b3d251d09598e7ca9b13 |
| Headers | show |
| Series | ASoC: Intel: Haswell: Adjust machine device private | expand |
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)); > if (IS_ERR(sst_acpi->pdev_mach)) > return PTR_ERR(sst_acpi->pdev_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
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);
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(-)