[alsa-devel] [PATCH 18/44] fireworks: Add connection and stream management

[Clemens Ladisch]

Takashi Sakamoto wrote:
> Fireworks manages connections by CMP and can transmit/receive AMDTP streams
> with a few quirks. This commit adds functionality to start/stop the streams.
>
> +++ b/sound/firewire/fireworks/fireworks.c
> +static int
> +init_stream(struct snd_efw *efw, struct amdtp_stream *stream)
> +{
> +	...
> +	err = amdtp_stream_init(stream, efw->unit, s_dir, CIP_BLOCKING);

amdtp_stream_destroy() must be called.

> +static int
> +start_stream(struct snd_efw *efw, struct amdtp_stream *stream,
> +	     unsigned int sampling_rate)
> +{
> +	...
> +	err = amdtp_stream_start(stream,
> +				 conn->resources.channel,
> +				 conn->speed);
> +	if (err < 0)
> +		stop_stream(efw, stream);
> +
> +	/* wait first callback */
> +	if (!amdtp_stream_wait_callback(stream, CALLBACK_TIMEOUT)) {
> +		stop_stream(efw, stream);
> +		err = -ETIMEDOUT;
> +	}
> +end:
> +	return err;
> +}

If amdtp_stream_start() fails, this function will try to wait for
the stream anyway.

> +int snd_efw_stream_init_duplex(struct snd_efw *efw)
> +{
> +	int err;
> +
> +	err = init_stream(efw, &efw->tx_stream);
> +	if (err < 0)
> +		goto end;
> +
> +	err = init_stream(efw, &efw->rx_stream);
> +	if (err < 0)
> +		goto end;

If the second init_stream() fails, this function will return with only one
of the streams initialized.

> +
> +	/* set IEC61883 compliant mode */
> +	err = snd_efw_command_set_tx_mode(efw, SND_EFW_TRANSPORT_MODE_IEC61883);
> +end:
> +	return err;

And if this fails, this function will return an error, but the two streams
will still be initialized.

In the error cases, any so-far initialized stream must be destroyed.

> +int snd_efw_stream_stop_duplex(struct snd_efw *efw)
> +{
> +	struct amdtp_stream *master, *slave;
> +	enum cip_flags sync_mode;
> +	unsigned int slave_substreams;
> +	int err;
> +
> +	mutex_lock(&efw->mutex);
> +
> +	err = get_roles(efw, &sync_mode, &master, &slave);
> +	if (err < 0)
> +		goto end;

snd_efw_stream_stop_duplex() must always succeed so that the
resources can be freed properly.  Therefore, it should not
try to ask the device for anything (the device might be
resetting or be unplugged).

Either cache the master/slave pointers when starting the stream,
or rewrite get_roles() so that it does not access the device.


Regards,
Clemens