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@xxxxxxxxx>
---
sound/soc/intel/common/sst-acpi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/common/sst-acpi.c
b/sound/soc/intel/common/sst-acpi.c
index 15f2b27e643f..c34f628c7987 100644
--- a/sound/soc/intel/common/sst-acpi.c
+++ b/sound/soc/intel/common/sst-acpi.c
@@ -109,11 +109,12 @@ int sst_acpi_probe(struct platform_device
*pdev)
}
platform_set_drvdata(pdev, sst_acpi);
+ mach->pdata = sst_pdata;
/* register machine driver */
sst_acpi->pdev_mach =
platform_device_register_data(dev, mach->drv_name, -1,
- sst_pdata, sizeof(*sst_pdata));
+ mach, sizeof(*mach));
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?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
