> +void intel_alsa_reset_ssp_status(void)
> +{
> + s_stream_status.ssp_handle = NULL;
> + s_stream_status.stream_info = 0;
> + spin_lock_init(&s_stream_status.lock);
> +}
It looks like a lot of these can be static ?
> + spin_lock_bh(&s_stream_status.lock);
Take the lock
> + if (s_stream_status.stream_info == 0) {
> + open_ssp = true;
> + } else if ((((str_info->stream_index == 0)) && (test_bit(1,
> &s_stream_status.stream_info))) ||
> + ((str_info->stream_index == 1) &&
> (test_bit(0, &s_stream_status.stream_info)))) {
> + /*
> + * do nothing be cause already Open by sibling
> substream
> + */
> + pr_debug("ALSA SSP: Open DO NOTHING\n");
> +
> + } else {
> + WARN(1, "ALSA SSP: Open unsupported Config\n");
Return with the lock held
So you've not actually tested this ?
>
> + /* Algorithm that detects conflicting call of open /close*/
> + spin_lock_bh(&s_stream_status.lock);
> +
> + if (s_stream_status.stream_info == 0) {
> + WARN(1, "ALSA SSP: Close before Open\n");
Ditto
> + return -EBUSY;
> + }
> + clear_bit(str_info->stream_index,
> &s_stream_status.stream_info); +
> + if (s_stream_status.stream_info == 0)
> + close_ssp = true;
What guarantees the right thing occurs if the other user opens first,
closes first ?
> +
> + spin_unlock_bh(&s_stream_status.lock);
> +
> + /*
> + * the Close SSP is performed out of lock
> + */
> + if (close_ssp) {
> + intel_mid_i2s_close(s_stream_status.ssp_handle);
> + s_stream_status.ssp_handle = NULL;
> + }
> +
> + return 0;
> +} /* intel_alsa_ssp_close */
> +
> +/*
> + * intel_alsa_ssp_control - Set Control params
> + *
> + * Input parameters
> + * command : command to execute
> + * value: pointer to a structure
If you've gone to the trouble of documenting the arguments (which is
nice) you might as well put them into kernel-doc format so the doc tools
can extract them.
> + * Output parameters
> + * ret_val : status
> + */
> +int intel_alsa_ssp_control(int command, struct
> intel_alsa_ssp_stream_info *str_info) +{
> + int retval = 0;
> +
> + switch (command) {
> + case INTEL_ALSA_SSP_CTRL_SND_OPEN:
> + retval = intel_alsa_ssp_open(str_info);
> + break;
> + /*
> + * SND_PAUSE & SND_RESUME not supported in this version
> + */
> + case INTEL_ALSA_SSP_CTRL_SND_PAUSE:
> + case INTEL_ALSA_SSP_CTRL_SND_RESUME:
> + break;
> +
> + case INTEL_ALSA_SSP_CTRL_SND_CLOSE:
> + intel_alsa_ssp_close(str_info);
> + break;
> +
> + default:
> + /* Illegal case */
> + WARN(1, "ALSA_SSP: intel_alsa_ssp_control Error: Bad
> Control ID\n");
Not sure this should be warning - it's user triggerable ?
> + p_stream_info = (struct intel_alsa_ssp_stream_info *)
> info_substream; +
Its void * anyway so you don't need the casts, and Linux generally
omits them for such cases
> + /*
> + * Allocates the DMA buffer for the substream
> + * This callback could be called several time
> + * snd_pcm_lib_malloc_pages allows to avoid memory leak
> + * as it release already allocated memory when already
> allocated
> + */
> + ret_val = snd_pcm_lib_malloc_pages(substream,
> + params_buffer_bytes(hw_params));
> +
> + memset(substream->runtime->dma_area, 0,
> params_buffer_bytes(hw_params)); +
And if the allocation failed ....
_______________________________________________
Meego-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel
