[alsa-devel] [PATCH 1/2] sound: asoc: Adding support for SPEAr13XX ASoC driver
rajeev
rajeev-dlh.kumar at st.com
Mon Mar 21 12:29:30 CET 2011
Hi Mark
On 3/18/2011 5:13 PM, Mark Brown wrote:
> On Fri, Mar 18, 2011 at 12:49:04PM +0530, rajeev wrote:
>> On 3/17/2011 8:35 PM, Mark Brown wrote:
>>> On Thu, Mar 17, 2011 at 04:53:35PM +0530, Rajeev Kumar wrote:
>
>>>> +static int spear13xx_i2s_hw_params(struct snd_pcm_substream *substream,
>>>> + struct snd_pcm_hw_params *params,
>>>> + struct snd_soc_dai *dai)
>>>> +{
>>>> + struct spear13xx_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
>>>> + u32 channel;
>
>>>> + channel = params_channels(params);
>
>>>> + dev->mode = channel;
>
>>> Having channel here isn't doing a lot...
>
>> I was actually planning to expand this routine in future.
>> Should I remove it from the current version?
>
> It's not too important either way.
>
OK I will remove this function
>>> These constraints don't agree with each other - the rate limits claim to
>>> support 48kHz only, the rates setting tells a different story.
>
>> At present I checked STA529(codec) only with 48Khz,I need to check with other data rates also.
>
> It's fine to expand later but you should make sure that if you want to
> say the device currently only works at 48kHz all the constraints say
> that - the issue is the inconsistency.
>
Ok I will check with rate minimum and rate maximum and update it accordingly.
>>> Rather than passing the channel count through from the I2S driver you
>>> could just get it directly here.
>
>> Are you talking about this "struct spear13xx_runtime_data *prtd = runtime->private_data"
>> Could you please elaborate little bit more.
>
> Yes, you're passing through data that doesn't need to be passed through
> since the DMA driver gets a hw_params() call too.
> .
>
Keeping a copy of substream in struct spear13xx_runtime_data as implementation is base on tasklet.Need to extract
this prtd->substream before dma submit.
Best Regards
Rajeev
More information about the Alsa-devel
mailing list