| Message ID | 1373559359-31607-2-git-send-email-richard.genoud@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 11, 2013 at 06:15:53PM +0200, Richard Genoud wrote: Please always try to use commit logs that look like normal commit logs for the subsystem. > switch (freq) { > - case 11289600: > case 12000000: > + wm8731->constraints = &wm8731_constraints_12000000; > + break; > case 12288000: > - case 16934400: > case 18432000: > - wm8731->sysclk = freq; > + wm8731->constraints = &wm8731_constraints_12288000_18432000; > + break; > + case 16934400: > + case 11289600: > + wm8731->constraints = &wm8731_constraints_11289600_16934400; > break; > default: > return -EINVAL; > } This isn't going to work with systems which have a variable clock as the input to the CODEC. If it's imposing constraints the driver needs to allow setting the clock to zero as a way of removing constraints (and any existing drivers should be updated to do this if needed).
2013/7/12 Mark Brown <broonie@kernel.org>: > On Thu, Jul 11, 2013 at 06:15:53PM +0200, Richard Genoud wrote: > > Please always try to use commit logs that look like normal commit logs > for the subsystem. Ok, I'll pay attention to that. >> switch (freq) { >> - case 11289600: >> case 12000000: >> + wm8731->constraints = &wm8731_constraints_12000000; >> + break; >> case 12288000: >> - case 16934400: >> case 18432000: >> - wm8731->sysclk = freq; >> + wm8731->constraints = &wm8731_constraints_12288000_18432000; >> + break; >> + case 16934400: >> + case 11289600: >> + wm8731->constraints = &wm8731_constraints_11289600_16934400; >> break; >> default: >> return -EINVAL; >> } > > This isn't going to work with systems which have a variable clock as the > input to the CODEC. If it's imposing constraints the driver needs to > allow setting the clock to zero as a way of removing constraints (and > any existing drivers should be updated to do this if needed). Maybe I'm wrong, but I didn't find any system using variable clock with this codec. The sam9g20ek (soc/atmel/sam9g20_wm8731.c) is not using a crystal, but it's using a fixed clock anyway. But there's soc/pxa/corgi.c and soc/pxa/poodle.c that puzzle me. They seems to use a crystal, but they are setting a different sysclk depending on the rate. That seems wrong, but as I'm a newbie in ASoC...
On Mon, Jul 15, 2013 at 04:53:46PM +0200, Richard Genoud wrote: > 2013/7/12 Mark Brown <broonie@kernel.org>: > > This isn't going to work with systems which have a variable clock as the > > input to the CODEC. If it's imposing constraints the driver needs to > > allow setting the clock to zero as a way of removing constraints (and > > any existing drivers should be updated to do this if needed). > Maybe I'm wrong, but I didn't find any system using variable clock > with this codec. The driver should be written with that possibility in mind even if there were no users; it only takes a couple of lines of code. > The sam9g20ek (soc/atmel/sam9g20_wm8731.c) is not using a crystal, but > it's using a fixed clock anyway. > But there's soc/pxa/corgi.c and soc/pxa/poodle.c that puzzle me. > They seems to use a crystal, but they are setting a different sysclk > depending on the rate. > That seems wrong, but as I'm a newbie in ASoC... Note that the CPU is clock master for those - it's going to be outputting a clock based on the sample rate selected automatically. These boards would be broken by your change as it stands.
diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c index 5276062..fc031ed 100644 --- a/sound/soc/codecs/wm8731.c +++ b/sound/soc/codecs/wm8731.c @@ -45,6 +45,7 @@ static const char *wm8731_supply_names[WM8731_NUM_SUPPLIES] = { struct wm8731_priv { struct regmap *regmap; struct regulator_bulk_data supplies[WM8731_NUM_SUPPLIES]; + const struct snd_pcm_hw_constraint_list *constraints; unsigned int sysclk; int sysclk_type; int playback_fs; @@ -290,6 +291,36 @@ static const struct _coeff_div coeff_div[] = { {12000000, 88200, 136, 0xf, 0x1, 0x1}, }; +/* rates constraints */ +static const unsigned int wm8731_rates_12000000[] = { + 8000, 32000, 44100, 48000, 96000, 88200, +}; + +static const unsigned int wm8731_rates_12288000_18432000[] = { + 8000, 32000, 48000, 96000, +}; + +static const unsigned int wm8731_rates_11289600_16934400[] = { + 8000, 44100, 88200, +}; + +static const struct snd_pcm_hw_constraint_list wm8731_constraints_12000000 = { + .list = wm8731_rates_12000000, + .count = ARRAY_SIZE(wm8731_rates_12000000), +}; + +static const +struct snd_pcm_hw_constraint_list wm8731_constraints_12288000_18432000 = { + .list = wm8731_rates_12288000_18432000, + .count = ARRAY_SIZE(wm8731_rates_12288000_18432000), +}; + +static const +struct snd_pcm_hw_constraint_list wm8731_constraints_11289600_16934400 = { + .list = wm8731_rates_11289600_16934400, + .count = ARRAY_SIZE(wm8731_rates_11289600_16934400), +}; + static inline int get_coeff(int mclk, int rate) { int i; @@ -362,17 +393,23 @@ static int wm8731_set_dai_sysclk(struct snd_soc_dai *codec_dai, } switch (freq) { - case 11289600: case 12000000: + wm8731->constraints = &wm8731_constraints_12000000; + break; case 12288000: - case 16934400: case 18432000: - wm8731->sysclk = freq; + wm8731->constraints = &wm8731_constraints_12288000_18432000; + break; + case 16934400: + case 11289600: + wm8731->constraints = &wm8731_constraints_11289600_16934400; break; default: return -EINVAL; } + wm8731->sysclk = freq; + snd_soc_dapm_sync(&codec->dapm); return 0; @@ -475,12 +512,26 @@ static int wm8731_set_bias_level(struct snd_soc_codec *codec, return 0; } +static int wm8731_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(dai->codec); + + if (wm8731->constraints) + snd_pcm_hw_constraint_list(substream->runtime, 0, + SNDRV_PCM_HW_PARAM_RATE, + wm8731->constraints); + + return 0; +} + #define WM8731_RATES SNDRV_PCM_RATE_8000_96000 #define WM8731_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\ SNDRV_PCM_FMTBIT_S24_LE) static const struct snd_soc_dai_ops wm8731_dai_ops = { + .startup = wm8731_startup, .hw_params = wm8731_hw_params, .digital_mute = wm8731_mute, .set_sysclk = wm8731_set_dai_sysclk,
Depending on the mclk (or crystal) selected, the wm8731 codec have some constraints on its data sampling rates: e.g. with a 12.288MHz or 18.432MHz crystal, the authorized rates are 8KHz, 32KHz, 48KHz and 96KHz. Signed-off-by: Richard Genoud <richard.genoud@gmail.com> --- sound/soc/codecs/wm8731.c | 57 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 3 deletions(-)