[alsa-devel] [PATCH 7/8] ALSA: pcm: Add card sync_irq field

[Takashi Iwai]

On Tue, 19 Nov 2019 09:24:33 +0100,
Takashi Iwai wrote:
> 
> On Tue, 19 Nov 2019 08:40:25 +0100,
> Ranjani Sridharan wrote:
> > 
> > > > 
> > > > Hi Takashi,
> > > > 
> > > > I just realized that In the SOF driver, we only set the component
> > > > driver ops. The pcm ops are set when creating the new pcm. So,
> > > > should I
> > > > also add the sync_stop op in the component driver and set the pcm
> > > > sync_stop op to point to the component sync_stop op? Just wanted to
> > > > confirm if I am on the right track.
> > > 
> > > Yes, I didn't touch this yet, but that's the way to go I suppose.
> > > One caveat is that this ops is optional and needs NULL as default,
> > > hence you'd need to set only when defined, like copy_user, page or
> > > mmap ops, at least.
> > Hi Takashi,
> > 
> > This is what I tried in the SOF driver:
> > https://github.com/thesofproject/linux/pull/1513/commits
> > 
> > And it seems to cause the system to hang when I stop the stream and I
> > have no meaningful logs to pinpoint to the problem. Could you please
> > have a look at the 4 commits that I have added to your series and let
> > me know what I could be missing? 
> 
> I couldn't find anything obvious.  Could you try without changing
> snd_sof_pcm_period_elapsed(), i.e. only adding the stuff and calling
> sync_stop, in order to see whether the additional stuff broke
> anything?

... and looking at the code again, I don't think the stop_sync ops
would help in your case.  The PCM ops is called *after* trigger-STOP
is issued, while your case requires the serialization of trigger call
itself.

But I'm still wondering whether the current implementation is safe,
too.  Basically the scheduled work may be executed immediately after
queuing, so if it's about the timing issue, it's not solid solution.
Pushing to work helps if it's about the locking issue, though.


Takashi