On Wed, Oct 20, 2010 at 12:13 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
struct s3c_audio_pdata is shared among I2S, AC97, SPDIF and PCM. Here the idea is that controller drivers add specific structures to the union as and when they need it.
Right, what I'm saying is that it'd seem more natural to do things the other way around and embed the common structure within a device specific pdata structure instead of having the union.
Ok, will do it.
Can we do this stuff with a variable storing the mask to use? It might compress the code a lot but I've not looked at what the bits actually are.
sorry, I am unable to understand what you suggest
If we have something in the driver data struct specifying the masks to use then set these at probe time rather than having the if statements - probably the same mask can be used for playback & record if the bitfields are lined up similarly. This would then be:
con |= data->active_mask; con &= ~data->pause_mask;
or similar, possibly with some shifts.
Let me see if the space saved is worth the complication.
- /* Allow LRCLK-inverted version of the supported formats */
- switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
- case SND_SOC_DAIFMT_NB_NF:
- break;
- case SND_SOC_DAIFMT_NB_IF:
- if (tmp & MOD_LR_RLOW)
- tmp &= ~MOD_LR_RLOW;
- else
- tmp |= MOD_LR_RLOW;
- break;
This looks a bit odd - it'll flip MOD_LR_FLOW if the frame is inverted, I'd expect it to want to set a specific value?
The frame polarity is specified by the Format(I2S, LSB, MSB), so we set/clear MOD_LR_RLOW acc to the Format requested. The Inversion request works relative to the Format -- if Frame inversion is requested, we simply flip it from the value set during Format setting.
Please add a comment explaining that you're inverting the orientation you set previously - it's really surprising when reading the code.
Ok, I'll add a comment. Btw, isn't it the standard way? Don't other I2S CPU drivers do the same thing ?
ASoC machine drivers need to index the secondary dai that is automatically created and registered by the cpu driver. So, it needs to be available outside. I'll make it SAMSUNG_I2S_SECOFF -- secondary offset of I2S(?)
How would the index be used with multi-component?
For example, please look at [Patch 23/25]
+ /* Secondary is at offset MAX_I2S from Primary */ + str = (char *)smdk_dai[SEC_PLAYBACK].cpu_dai_name; + str[strlen(str) - 1] = '0' + MAX_I2S;
Thanks.