On Tue, Aug 24, 2010 at 22:15, Bob Liu wrote: > On Tue, Aug 24, 2010 at 11:50 PM, Mike Frysinger wrote: >> On Tue, Aug 24, 2010 at 05:10, <[email protected]> wrote: >>> Modified: trunk/sound/soc/blackfin/bf5xx-i2s.c (9095 => 9096) >>> >>> @@ -227,13 +227,13 @@ >>> .suspend = bf5xx_i2s_suspend, >>> .resume = bf5xx_i2s_resume, >>> .playback = { >>> - .channels_min = 2, >>> - .channels_max = 2, >>> + .channels_min = 1, >>> + .channels_max = 1, >>> .rates = BF5XX_I2S_RATES, >>> .formats = BF5XX_I2S_FORMATS,}, >>> .capture = { >>> - .channels_min = 2, >>> - .channels_max = 2, >>> + .channels_min = 1, >>> + .channels_max = 1, >>> .rates = BF5XX_I2S_RATES, >>> .formats = BF5XX_I2S_FORMATS,}, >> >> although, i kind of wonder about the correctness of this patch. the >> bf5xx-i2s interface is used by any codec that has an i2s interface. >> why are we disabling functionality just to make one codec (which isnt >> even in the tree) work ? > > there is a place define codec_dai eg sound/codec/ad73311.c > struct snd_soc_dai ad73311_dai = { > .name = DRV_NAME, > .playback = { > .stream_name = "Playback", > .channels_min = 1, > .channels_max = 1, > .rates = SNDRV_PCM_RATE_8000, > .formats = SNDRV_PCM_FMTBIT_S16_LE, }, > .capture = { > .stream_name = "Capture", > .channels_min = 1, > .channels_max = 1, > .rates = SNDRV_PCM_RATE_8000, > .formats = SNDRV_PCM_FMTBIT_S16_LE, }, > }; > > the soc core code will get the hw->channels_min and max from cpu_dai > and codec_dai by: > runtime->hw.channels_min = max(codec_dai->playback.channels_min, > cpu_dai->playback.channels_min); > runtime->hw.channels_max = min(codec_dai->playback.channels_max, > cpu_dai->playback.channels_max); > > so if don't add this patch ad73311 will get runtime->hw.channels_min=2 > and runtime->hw.channels_max = 1 which makes it can't work.
i'm not saying a change isnt required. just that this is likely not the correct way to go about it. this cpu_dai code is shared between codecs, so any change to it cannot break others even if it fixes one. how about changing the machine driver which links the cpu_dai to the codec_dai ? so in the bf5xx-ad73322 __init function, you can manually change the values from 2 to 1. this will cause much less of a problem. or, if our i2s driver supports 1 or 2 channels, why isnt the max set to 2 and the min set to 1 ? wouldnt that continue to work for everyone ? -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
