Re: [alsa-devel] [PATCH 26/44] fireworks: Add PCM interface

[Clemens Ladisch <clemens@ladisch.de>]

Takashi Sakamoto wrote:
> (Apr 4 2014 17:48), Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> +    if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
>>> +        substream->runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
>>> +    } else {
>>> +        substream->runtime->hw.formats = AMDTP_OUT_PCM_FORMAT_BITS;
>>
>> The should have been a similar symbol for AMDTP capture streams.
>
> Do you suggest to add AMDTP_IN_PCM_FORMAT_BITS?

Yes; in the ideal case, this driver does not need to know which formats
are actually supported.

>> /*
>>  * Align the period size to SYT_INTERVAL to align the period interrupts
>>  * with the packet boundaries.  Align the buffer size to SYT_INTERVAL to
>>  * avoid having a buffer boundary inside a packet.
>>  */
>
> Currently firewire-lib exports a table for SYT_INTERVAL so we can make
> better rules for this, can't we?

Yes, we could make better rules, if we wanted to.

>>> +static int pcm_open(struct snd_pcm_substream *substream)
>>> +{
>>> +    ...
>>> +    /*
>>> +     * When source of clock is not internal or any PCM streams are running,
>>> +     * available sampling rate is limited at current sampling rate.
>>> +     */
>>> +    if ((clock_source != SND_EFW_CLOCK_SOURCE_INTERNAL) ||
>>> +        amdtp_stream_pcm_running(&efw->tx_stream) ||
>>> +        amdtp_stream_pcm_running(&efw->rx_stream)) {
>>
>> Opening the playback and capture streams of a single PCM device is
>> protected with the same mutex, but this does not help against races with
>> the MIDI callbacks.
>>
>> This substream management code must be protected with a mutex.
>> (Also in hw_params and hw_free.)
>
> Hm. I have no ideas for such races, except for substream counter.

There is already a mutex for everything in start_duplex(), so the only
unprotected piece of data is this counter.


Regards,
Clemens

------------------------------------------------------------------------------