- int trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
This isn't used at all in this patch. I found it being used in the later usb-audio patch. If it's the only place, can't it be rather put locally to usb-audio object instead of the common pcm runtime?
It's not limited to USB. We have upcoming hardware where the trigger_tstamp will only be determined with a delay due to IPC. USB is just an example of a common pattern where the trigger_tstamp will be known for sure after a couple of ms.
Well, the question is whether this *must* be put in the common place. In other words, will this field be referred in the common PCM code? If not, it should be recorded rather locally.
well i wasn't sure of what to do here: 1. we can leave the low-lever driver update the trigger_tstamp on its own, and then this field is not needed at the core level, but the core or the application will not be notified that an update is pending 2. or we use this field to let the core know, and possibly let userspace know that the trigger will be updated shortly. However, I didn't find an existing mechanism to notify userspace that the new trigger_tstamp is dirty and has changed so the use of this field is indeed incomplete for now.
I could go either way.
IMO, it's better to clear the flat at the beginning of PCM trigger commonly. Looking at the later patch, you clear in each driver's callback. This should be moved into the common place.
I must admit I don't really understand the logic of all the _pre and _post operations. Did you mean clearing this field in snd_pcm_do_start()?
Yes, either snd_pcm_do_start() and snd_pcm_pre_start(). There aren't much difference in this case.
ok, will cleanup.