Re: [PATCH] ALSA: sb: remove needless evaluation in implementation for copy callback

[Takashi Iwai <tiwai@suse.de>]

On Tue, 23 May 2017 15:40:42 +0200,
Takashi Sakamoto wrote:
> 
> In design of ALSA pcm core, 'struct snd_pcm_ops.copy' is expected to
> copy PCM frames, according to frame alignment on intermediate buffer for
> userspace and dedicated buffer for data transmission. In this callback,
> value of 'channel' argument depends on the frame alignment, which drivers
> registers to runtime of PCM substream. When target devices can handle
> non-interleaved buffer, this value has positive value, otherwise negative.
> 
> ALSA driver for PCM component of EMU8000 chip is programmed with local
> macro to switch the frame alignment. The 'copy' operation in
> non-interleaved side has evaluation of the 'channel' argument (actually
> it's 'voice' argument). This is useless.
> 
> This commit remove the evaluation.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Passing -1 to the channel was valid even for non-interleaved access.
It was meant to apply to all channels.

The call with channel = -1 itself might have been dropped meanwhile,
so removing it may be OK now.  But the description is confusing as if
it were incorrectly implemented.


Takashi


> ---
>  sound/isa/sb/emu8000_pcm.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c
> index 32f234f494e5..bab796d75dbd 100644
> --- a/sound/isa/sb/emu8000_pcm.c
> +++ b/sound/isa/sb/emu8000_pcm.c
> @@ -450,20 +450,8 @@ static int emu8k_pcm_copy(struct snd_pcm_substream *subs,
>  	struct snd_emu8000 *emu = rec->emu;
>  
>  	snd_emu8000_write_wait(emu, 1);
> -	if (voice == -1) {
> -		unsigned short *buf = src;
> -		int i, err;
> -		count /= rec->voices;
> -		for (i = 0; i < rec->voices; i++) {
> -			err = emu8k_transfer_block(emu, pos + rec->loop_start[i], buf, count);
> -			if (err < 0)
> -				return err;
> -			buf += count;
> -		}
> -		return 0;
> -	} else {
> -		return emu8k_transfer_block(emu, pos + rec->loop_start[voice], src, count);
> -	}
> +	return emu8k_transfer_block(emu, pos + rec->loop_start[voice], src,
> +				    count);
>  }
>  
>  /* make a channel block silence */
> @@ -487,17 +475,9 @@ static int emu8k_pcm_silence(struct snd_pcm_substream *subs,
>  	struct snd_emu8000 *emu = rec->emu;
>  
>  	snd_emu8000_write_wait(emu, 1);
> -	if (voice == -1 && rec->voices == 1)
> +	if (rec->voices == 1)
>  		voice = 0;
> -	if (voice == -1) {
> -		int err;
> -		err = emu8k_silence_block(emu, pos + rec->loop_start[0], count / 2);
> -		if (err < 0)
> -			return err;
> -		return emu8k_silence_block(emu, pos + rec->loop_start[1], count / 2);
> -	} else {
> -		return emu8k_silence_block(emu, pos + rec->loop_start[voice], count);
> -	}
> +	return emu8k_silence_block(emu, pos + rec->loop_start[voice], count);
>  }
>  
>  #else /* interleave */
> -- 
> 2.11.0
>