- Sound-open-firmware - mailman.alsa-project.org

Re: [Sound-open-firmware] [PATCH 5/7] soundwire: intel: Don't disable interrupt until children are removed
by Pierre-Louis Bossart 13 Sep '22

13 Sep '22

On 9/7/22 12:14, Richard Fitzgerald wrote: > The cadence_master code needs the interrupt to complete message transfers. > When the bus driver is being removed child drivers are removed, and their > remove actions might need bus transactions. > > Use the sdw_master_ops.remove callback to disable the interrupt handling > only after the child drivers have been removed. > > Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> > --- > drivers/soundwire/intel.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c > index 01be62fa6c83..d5e723a9c80b 100644 > --- a/drivers/soundwire/intel.c > +++ b/drivers/soundwire/intel.c > @@ -1255,6 +1255,13 @@ static int intel_prop_read(struct sdw_bus *bus) > return 0; > } > > +static void intel_bus_remove(struct sdw_bus *bus) > +{ > + struct sdw_cdns *cdns = bus_to_cdns(bus); > + > + sdw_cdns_enable_interrupt(cdns, false); don't you need to check for any on-going transactions on the bus? I wonder if there could be a corner case where there are no child devices but still a device physically attached to the bus. I am not sure if the 'no devices left' is a good-enough indication of no activity on the bus. > +} > + > static struct sdw_master_ops sdw_intel_ops = { > .read_prop = sdw_master_read_prop, > .override_adr = sdw_dmi_override_adr, > @@ -1264,6 +1271,7 @@ static struct sdw_master_ops sdw_intel_ops = { > .set_bus_conf = cdns_bus_conf, > .pre_bank_switch = intel_pre_bank_switch, > .post_bank_switch = intel_post_bank_switch, > + .remove = intel_bus_remove, > }; > > static int intel_init(struct sdw_intel *sdw) > @@ -1502,7 +1510,6 @@ static void intel_link_remove(struct auxiliary_device *auxdev) > */ > if (!bus->prop.hw_disabled) { > intel_debugfs_exit(sdw); > - sdw_cdns_enable_interrupt(cdns, false); > snd_soc_unregister_component(dev); > } > sdw_bus_master_delete(bus);

2 3

Re: [Sound-open-firmware] [PATCH 7/7] soundwire: bus: Fix premature removal of sdw_slave objects
by Pierre-Louis Bossart 12 Sep '22

12 Sep '22

On 9/7/22 12:14, Richard Fitzgerald wrote: > When the bus manager is removed sdw_bus_master_delete() should not > be deleting the struct sdw_slave objects until the bus manager has > been stopped. The first step of removing child drivers should only > be calling device_unregister() on the child. The counterpart to > sdw_drv_probe() is sdw_drv_remove(), not sdw_delete_slave(). > > The sdw_slave objects are created by the bus manager probe() from > ACPI/DT information. They are not created when a child driver probes > so should not be deleted by a child driver remove. > > Change-Id: I25cc145df12fdc7c126f8f594a5f76eedce25488 spurious Change-Id > Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> > --- > drivers/soundwire/bus.c | 30 ++++++++++++++++++++++++++---- > drivers/soundwire/slave.c | 21 +++++++++++++++++---- > 2 files changed, 43 insertions(+), 8 deletions(-) > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index 1327a312be86..5533eb589286 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -146,9 +146,8 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, > } > EXPORT_SYMBOL(sdw_bus_master_add); > > -static int sdw_delete_slave(struct device *dev, void *data) > +static int sdw_delete_slave(struct sdw_slave *slave) > { > - struct sdw_slave *slave = dev_to_sdw_dev(dev); > struct sdw_bus *bus = slave->bus; > > sdw_slave_debugfs_exit(slave); > @@ -163,7 +162,24 @@ static int sdw_delete_slave(struct device *dev, void *data) > list_del_init(&slave->node); > mutex_unlock(&bus->bus_lock); > > + mutex_destroy(&slave->sdw_dev_lock); > + kfree(slave); > + > + return 0; > +} > + > +static int sdw_remove_child(struct device *dev, void *data) > +{ > + /* > + * Do not remove the struct sdw_slave yet. This is created by > + * the bus manager probe() from ACPI information and used by the > + * bus manager to hold status of each peripheral. Its lifetime > + * is that of the bus manager. > + */ > + > + /* This will call sdw_drv_remove() */ > device_unregister(dev); > + > return 0; > } > > @@ -171,16 +187,22 @@ static int sdw_delete_slave(struct device *dev, void *data) > * sdw_bus_master_delete() - delete the bus master instance > * @bus: bus to be deleted > * > - * Remove the instance, delete the child devices. > + * Remove the child devices, remove the master instance. > */ > void sdw_bus_master_delete(struct sdw_bus *bus) > { > - device_for_each_child(bus->dev, NULL, sdw_delete_slave); > + struct sdw_slave *slave, *tmp; > + > + device_for_each_child(bus->dev, NULL, sdw_remove_child); > > /* Children have been removed so it is now safe for the bus to stop */ > if (bus->ops->remove) > bus->ops->remove(bus); > > + /* Now the bus is stopped it is safe to free things */ > + list_for_each_entry_safe(slave, tmp, &bus->slaves, node) > + sdw_delete_slave(slave); > + > sdw_master_device_del(bus); > > sdw_bus_debugfs_exit(bus); > diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c > index c1c1a2ac293a..b6161d002b97 100644 > --- a/drivers/soundwire/slave.c > +++ b/drivers/soundwire/slave.c > @@ -10,10 +10,23 @@ > > static void sdw_slave_release(struct device *dev) > { > - struct sdw_slave *slave = dev_to_sdw_dev(dev); > - > - mutex_destroy(&slave->sdw_dev_lock); > - kfree(slave); > + /* > + * The release() callback should not be empty > + * (see Documentation/core-api/kobject.rst) but the ownership > + * of struct sdw_slave is muddled. It is used for two separate > + * purposes: > + * 1) by the bus driver to track its own state information for > + * physical devices on the bus and found in ACPI/DT, whether > + * or not there is a child driver for it; > + * 2) to hold the child driver object. > + * > + * The struct sdw_slave cannot be freed when the child driver > + * is released because it is holding info used by the bus > + * driver. It is freed when the bus driver is removed. > + * > + * Until the ownership issue is untangled this cannot free > + * the struct sdw_slave object containing the child dev. > + */ > } > > struct device_type sdw_slave_type = {

1 0

Re: [Sound-open-firmware] [PATCH 1/7] soundwire: bus: Do not forcibly disable child pm_runtime
by Pierre-Louis Bossart 12 Sep '22

12 Sep '22

On 9/7/22 12:13, Richard Fitzgerald wrote: > Do not call pm_runtime_disable() of a child driver in > sdw_delete_slave(). We really should never be trying to disable > another driver's pm_runtime - it is up to the child driver to > disable it or the core driver framework cleanup. The driver core > will runtime-resume a driver before calling its remove() so we > shouldn't break that. > > The patch that introduced this is > commit dff70572e9a3 ("soundwire: bus: disable pm_runtime in sdw_slave_delete") > which says: > > "prevent any race condition with the resume being executed after the > bus and slave devices are removed" > > The actual problem is that the bus driver is shutting itself down before > the child drivers have been removed, which is the wrong way around (see > for example I2C and SPI drivers). If this is fixed, the bus driver will > still be operational when the driver framework runtime_resumes the child > drivers to remove them. Then the bus driver will remove() and can shut > down safely. The description of the fix looks good, but "if this is fixed" is very confusing to me. Don't you have a dependency issue here? There should be first a patch to fix the bus issue and then remove this pm_runtime_disable second. > > Also note that the child drivers are not necessarily idle when the bus > driver is removed, so disabling their pm_runtime and stopping the bus > might break more than only their remove(). > > Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> > --- > drivers/soundwire/bus.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index 0bcc2d161eb9..99429892221b 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -151,8 +151,6 @@ static int sdw_delete_slave(struct device *dev, void *data) > struct sdw_slave *slave = dev_to_sdw_dev(dev); > struct sdw_bus *bus = slave->bus; > > - pm_runtime_disable(dev); > - > sdw_slave_debugfs_exit(slave); > > mutex_lock(&bus->bus_lock);

1 0

[Sound-open-firmware] [PATCH 0/5] ASoC: Fixes for MT8195 SOF support
by AngeloGioacchino Del Regno 07 Sep '22

07 Sep '22

This series fixes Sound Open Firmware support for MT8195 by making sure that the sound card driver is actually able to probe and IPC can finally happen. It is now possible to get DSP support for audio. Tested on MT8195 Tomato - Acer Chromebook Spin 513 CP513-2H (Pipewire). AngeloGioacchino Del Regno (5): ASoC: mediatek: mt8195-mt6359: Properly register sound card for SOF ASoC: SOF: mediatek: mt8195: Import namespace SND_SOC_SOF_MTK_COMMON ASoC: SOF: mediatek: mt8195: Add mailbox generic callbacks for IPC ASoC: SOF: mediatek: mt8195: Add generic pcm_{open,close} callbacks ASoC: SOF: mediatek: mt8195: Add devicetree support to select topologies sound/soc/mediatek/mt8195/mt8195-mt6359.c | 6 +++++ sound/soc/sof/mediatek/mt8195/mt8195.c | 32 ++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) -- 2.37.2

2 6

[Sound-open-firmware] [PATCH] ASoC: SOF: Introduce function sof_of_machine_select
by Chunxu Li 05 Sep '22

05 Sep '22

>From current design in sof_machine_check the SOF can only support ACPI type machine. In sof_machine_check if there is no ACPI machine exist, the function will return -ENODEV directly, that's we don't expected if we do not base on ACPI machine. So we add a new function named sof_of_machine_select that we can pass sof_machine_check and obtain info required by snd_sof_new_platform_drv. Signed-off-by: Chunxu Li <chunxu.li(a)mediatek.com> --- sound/soc/sof/sof-audio.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index c18e723435bd..ea9663d448eb 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -785,6 +785,28 @@ int sof_dai_get_bclk(struct snd_soc_pcm_runtime *rtd) } EXPORT_SYMBOL(sof_dai_get_bclk); +static struct snd_sof_of_mach *sof_of_machine_select(struct snd_sof_dev *sdev) +{ + struct snd_sof_pdata *sof_pdata = sdev->pdata; + const struct sof_dev_desc *desc = sof_pdata->desc; + struct snd_sof_of_mach *mach = desc->of_machines; + + if (!mach) + return NULL; + + for (; mach->compatible; mach++) { + if (of_machine_is_compatible(mach->compatible)) { + sof_pdata->tplg_filename = mach->sof_tplg_filename; + if (mach->fw_filename) + sof_pdata->fw_filename = mach->fw_filename; + + return mach; + } + } + + return NULL; +} + /* * SOF Driver enumeration. */ @@ -795,6 +817,7 @@ int sof_machine_check(struct snd_sof_dev *sdev) struct snd_soc_acpi_mach *mach; if (!IS_ENABLED(CONFIG_SND_SOC_SOF_FORCE_NOCODEC_MODE)) { + const struct snd_sof_of_mach *of_mach; /* find machine */ mach = snd_sof_machine_select(sdev); @@ -804,6 +827,12 @@ int sof_machine_check(struct snd_sof_dev *sdev) return 0; } + of_mach = sof_of_machine_select(sdev); + if (of_mach) { + sof_pdata->of_machine = of_mach; + return 0; + } + if (!IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC)) { dev_err(sdev->dev, "error: no matching ASoC machine driver found - aborting probe\n"); return -ENODEV; -- 2.25.1

3 2

[Sound-open-firmware] [PATCH v2 0/2] Introduce sof_of_machine_select
by Chunxu Li 02 Sep '22

02 Sep '22

From: "chunxu.li" <chunxu.li(a)mediatek.com> Change since v1: - remove the callback of_machine_select defined in sof-priv.h - move sof_of_machine_select to common code, and called in sof_machine_check - rename .board field to .compatible in structure snd_sof_of_mach In these patches, we introduce function sof_of_machine_select for SOF Chunxu Li (2): ASoC: SOF: Introduce function sof_of_machine_select ASoC: SOF: mediatek: Add sof_mt8186_machs for mt8186 include/sound/sof.h | 2 ++ sound/soc/sof/mediatek/mt8186/mt8186.c | 9 ++++++++ sound/soc/sof/pcm.c | 8 ++++++- sound/soc/sof/sof-audio.c | 30 ++++++++++++++++++++++++++ sound/soc/sof/sof-of-dev.h | 7 ++++++ 5 files changed, 55 insertions(+), 1 deletion(-) -- 2.25.1

3 4

[Sound-open-firmware] sof-logger status on SOF with Zephyr
by Daniel Baluta 29 Aug '22

29 Aug '22

Hi Liam, What is the plan for sof-logger with SOF-Zephyr? Is it working on Intel platforms? We are now planning on looking at it and we want to understand if it will be used in the future. For now on i.MX the logger is disabled. thanks, daniel.

3 2

[Sound-open-firmware] [PATCH 0/3] ASoC: SOF: mediatek: update SOF driver for mt8186
by Chunxu Li 25 Aug '22

25 Aug '22

In these patches, we update SOF driver for mt8186 Chunxu Li (3): ASoC: SOF: mediatek: Add dai driver for mt8186 ASoC: SOF: mediatek: add snd_sof_dsp_ops callbacks for pcm and mail box ASoC: SOF: mediatek: Use generic implementation for .ipc_msg_data field sound/soc/sof/mediatek/mt8186/mt8186.c | 49 ++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 7 deletions(-) -- 2.25.1

2 4

[Sound-open-firmware] [PATCH -next 1/2] ASoC: SOF: imx8ulp: change es8326_regmap_config to static
by Yang Yingliang 24 Aug '22

24 Aug '22

sof_imx8ulp_ops is only used in imx8ulp.c now, change it to static. Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> --- sound/soc/sof/imx/imx8ulp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/sof/imx/imx8ulp.c b/sound/soc/sof/imx/imx8ulp.c index 02b496165acc..afab7feab5fb 100644 --- a/sound/soc/sof/imx/imx8ulp.c +++ b/sound/soc/sof/imx/imx8ulp.c @@ -413,7 +413,7 @@ static int imx8ulp_dsp_set_power_state(struct snd_sof_dev *sdev, } /* i.MX8 ops */ -struct snd_sof_dsp_ops sof_imx8ulp_ops = { +static struct snd_sof_dsp_ops sof_imx8ulp_ops = { /* probe and remove */ .probe = imx8ulp_probe, .remove = imx8ulp_remove, -- 2.25.1

2 2

[Sound-open-firmware] Zephyr memory allocation from heap
by Daniel Baluta 23 Aug '22

23 Aug '22

Hi guys, Did you encountered any problems with memory allocations when switching to Zephyr? I'm facing a very strange issue with a playback pipeline with module adapter (MP3) where it looks like the DAI hits underruns as soon as I start the pipeline. With normal PCM playback everything works fine. I think it might be a problem in the sense Zephyr allocates memory and/or Cadence memory alignment requirements? Looks like as soon as I change this: -#define PLATFORM_DCACHE_ALIGN sizeof(void *) +#define PLATFORM_DCACHE_ALIGN 32 everything seems to work. But I cannot find any logical explanation.

6 8