Unable to open hostless PCM device after introduction of commit - ASoC: Stop dummy from overriding hwparams

[Amadeusz Sławiński]

On 11/18/2022 6:17 PM, Patrick Lai wrote:
> Hi Amadeusz,
> 
> On the product I am working on, a hostless PCM device is defined for 
> purpose of activating CODEC driver to setup the path inside CODEC. So, 
> CPU DAI and PCM Platform are defined to use dummy dai & DMA supplied by 
> sound/soc/soc-utils.c.
> 
> After upgrading to newer kernel, hostless PCM device failed to open. 
> After doing a bit of digging, the root cause is that dummy_dma_hardware 
> is not set in dummy_dma_open() due to new conditional check logic 
> introduced in this commit - 6c504663ba2ee2abeaf5622e27082819326c1bd4.
> 
> In order to fix problem I am encountering properly without regressing 
> your scenario, I would like to get a better understanding of problem you 
> were addressing. My understanding, from looking through other drivers 
> under sound/soc, is that pcm hardware info is usually set by PCM 
> platform/DMA drivers. For your scenario, do you have other component e.g 
> CPU/CODEC DAI, set PCM hardware definition? I am not sure conditional 
> check logic from 6c504663ba2ee2abeaf5622e27082819326c1bd4 guarantees 
> that other component would be setting pcm hardware info. Appreciate if 
> you can provide more insight to your scenario?
> 
> Thanks
> Patrick

Hi Patrick,

Call path is: ... -> __soc_pcm_open() -> soc_pcm_components_open -> 
snd_soc_component_open -> open callback, where for dummy device open 
callback is dummy_dma_open.

Expanding on the issue in question which was cause of the patch.

With following debug log:
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index e12f8244242b..b086ec05da25 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -290,6 +290,8 @@ int snd_soc_component_open(struct snd_soc_component 
*component,
  {
         int ret = 0;

+       pr_err("%s\n", component->name);
+
         if (component->driver->open)
                 ret = component->driver->open(component, substream);

that's what I get in dmesg on one of our test platforms:
[   95.522577] avs_rt274.1-platform
[   95.526019] i2c-INT34C2:00
[   95.528837] snd_soc_core:dpcm_fe_dai_startup:  audio: ASoC: open FE audio
[   95.528849] avs_rt274.1-platform
[   95.532249] snd-soc-dummy
[   95.534989] snd-soc-dummy
[   95.537800] snd_soc_avs:avs_dai_fe_startup: snd_soc_avs 0000:00:1f.3: 
avs_dai_fe_startup fe STARTUP tag 1 str 0000000064defd29

"avs_rt274.1-platform" component is handled in sound/soc/intel/avs/pcm.c 
it calls avs_component_open() which sets hwparams to generic set 
supported by i2s devices in AVS driver.

"i2c-INT34C2:00" is codec driver sound/soc/codecs/rt274.c it does not 
have open callback.

And finally "snd-soc-dummy" which as mentioned above calls 
dummy_dma_open which originally overridden hwparams set in 
avs_component_open() with its own limited ones.

(When topology is loaded it also creates FEs, which further limit 
allowed hwparams, they are a subset of the ones set above).

As mentioned in the patch:
"Alternative approach would be to copy whole dummy handling and rename 
it to "snd-soc-null" or something similar. And remove hwparams 
assignment to make it really do nothing."

However, looking at it again, I would consider the existence of 
dummy_dma_open() to be scope creep. If component is really a dummy one 
it should not affect any other components in any way. And if any drivers 
depends on dummy setting parameters for it, I would consider it 
partially broken. And would say that issue should rather be fixed on 
driver side by making a dedicated component for it instead of using a 
dummy one.

I hope that I cleared up situation a bit.

Thanks,
Amadeusz