[alsa-devel] [PATCH 11/25] ASoC: Samsung: Add common I2S driver
broonie at opensource.wolfsonmicro.com
Wed Oct 20 06:01:42 CEST 2010
On Wed, Oct 20, 2010 at 12:27:43PM +0900, Jassi Brar wrote:
> On Wed, Oct 20, 2010 at 12:13 PM, Mark Brown
> > 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.
I was thinking it should make something similar by making it clear that
we're just doing the same thing to a different location rather than lots
of conditional code to follow.
> > 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 ?
Not commonly - normally people don't implement anything except plain I2S
mode, or have explict mode and inversion bits (so the mode sets the
expected polarity and then the inversion bit swaps that).
> > 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;
Hrm, slightly fiddly. Perhaps we want a function people can call which
will generate the secondary DAI name for you? Might be more directly
what people want.
More information about the Alsa-devel