Re: [PATCH v5 6/9] ALSA: virtio: PCM substream operators

On Mon, 22 Feb 2021 16:34:41 +0100, Anton Yakovlev wrote:

+static int virtsnd_pcm_open(struct snd_pcm_substream *substream) +{

• struct virtio_pcm *vpcm = snd_pcm_substream_chip(substream);
• struct virtio_pcm_substream *vss = NULL;
• if (vpcm) {

• switch (substream->stream) {
  

• case SNDRV_PCM_STREAM_PLAYBACK:
  

• case SNDRV_PCM_STREAM_CAPTURE: {
  

The switch() here looks superfluous. The substream->stream must be a good value in the callback. If any, you can put WARN_ON() there, but I don't think it worth.

+static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,

• 		 struct snd_pcm_hw_params *hw_params)
  

+{

....

• return virtsnd_pcm_msg_alloc(vss, periods, period_bytes);

We have the allocation, but...

+static int virtsnd_pcm_hw_free(struct snd_pcm_substream *substream) +{

• return 0;

... no release at hw_free()? I know that the free is present in the allocator, but it's only for re-allocation case, I suppose.

thanks,

Takashi