Re: [Sound-open-firmware] [PATCH v4 05/14] ASoC: SOF: Add PCM operations support - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: Daniel Baluta <daniel.baluta@gmail.com>,
	andriy.shevchenko@intel.com, alsa-devel@alsa-project.org,
	liam.r.girdwood@linux.intel.com, vkoul@kernel.org,
	broonie@kernel.org, Alan Cox <alan@linux.intel.com>,
	sound-open-firmware@alsa-project.org
Subject: Re: [Sound-open-firmware] [PATCH v4 05/14] ASoC: SOF: Add PCM operations support
Date: Thu, 14 Feb 2019 14:42:51 -0600	[thread overview]
Message-ID: <707d8855-2f89-b0fb-4b1e-7e3cd133bfab@linux.intel.com> (raw)
In-Reply-To: <0092ac90-10fe-fbf8-9a77-dff05e0c4525@linux.intel.com>

Missed the comments below in my earlier answer.
>>> +    ret = 
>>> snd_pcm_lib_preallocate_pages(pcm->streams[stream].substream,
>>> +                        SNDRV_DMA_TYPE_DEV_SG, sdev->dev,
>>> + le32_to_cpu(caps->buffer_size_min),
>>> + le32_to_cpu(caps->buffer_size_max));
>>> +    if (ret) {
>>> +        dev_err(sdev->dev, "error: can't alloc DMA buffer size 
>>> 0x%x/0x%x for %s %d\n",
>>> +            caps->buffer_size_min, caps->buffer_size_max,
>>> +            caps->name, ret);
>>> +        return ret;
>>> +    }
>>
>> The error check here is redundant, please drop.
>> snd_pcm_lib_preallocate_pages() was changed to be void function
>> recently, so it'll be a build error.
yes indeed. It's not on Mark's for-next branch so I missed it. Fixed 
now, thanks
>>
>>> +
>>> +        snd_mask_none(fmt);
>>> +        snd_mask_set(fmt, (__force int)SNDRV_PCM_FORMAT_S16_LE);
>>
>> Use snd_mask_set_format() macro.  That avoids the ugly cast.

Ah yes. Ironically I was the one who reported this ugly cast and didn't 
update our code with the new helper. Oops. Fixed now.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel