On Thu 24 Sep 2009 05:48, [email protected] pondered:
> --- trunk/sound/soc/blackfin/bf5xx-adau1371.c (rev
0)
> +++ trunk/sound/soc/blackfin/bf5xx-adau1371.c 2009-09-24 09:48:30 UTC (rev
7448)
[snip]
> +static int bf5xx_adau1371_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
> + struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> + int ret = 0;
> + /* Change input crystal source here */
> + unsigned int clk = 12288000;
> + /* unsigned int clk = 11289600; */
Doesn't this belong in the ASOC equivalent of the platform data? (or
is ./soc/blackfin/ where that goes?)
If so - shouldn't there be some comment about "this is the crystal on the
ADAU1371 demo board"?
[snip]
> --- trunk/sound/soc/codecs/adau1371.c (rev 0)
> +++ trunk/sound/soc/codecs/adau1371.c 2009-09-24 09:48:30 UTC (rev 7448)
> @@ -0,0 +1,1105 @@
[snip]
> +/*
> + * adau1371 register cache
> + * We can't read the adau1371 register space when we are
> + * using 2 wire for device control, so we cache them instead.
> + * There is no point in caching the reset register
> + */
> +static const u8 adau1371_reg[ADAU1371_CACHEREGNUM] = {
> + 0x00, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, /* 0x00-0x07 */
> + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x00, /* 0x08-0x0f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x10-0x17 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x18-0x1f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x20-0x27 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x28-0x2f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x30-0x37 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x38-0x3f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x40-0x47 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x48-0x4f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x50-0x57 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x58-0x5f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x60-0x67 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x68-0x6f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x70-0x77 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x78-0x7f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x80-0x87 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x88-0x8f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x90-0x97 */
> + 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0x98-0x9f */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xa0-0xa7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xa8-0xaf */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xb0-0xb7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xb8-0xbf */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xc0-0xc7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xc8-0xcf */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xd0-0xd7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xd8-0xdf */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xe0-0xe7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xe8-0xef */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 0xf0-0xf7 */
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 /* 0xf8-0xff */
> +};
these don't seem to be reset on the reset function - should they be?
> +struct _pll_settings {
> + u32 mclk;
> + u32 rate;
> + u16 n; /* N and M registers are inverted on REVB*/
I was told - although we only have rev B today - there is no plan to release
rev B to production, and that we should only plan on supporting (long term)
rev C....
So, I guess this needs to stay for now, but can be dropped before sending
upstream.
> + u16 m;
> + u8 input_div:2;
> + u8 integer:4;
> + u8 type:1;
> +};
> +
> +/* PLL settings coefficients, add more here... */
> +static const struct _pll_settings pll_settings[] = {
> + /* 96k */
> + /* {12288000, 96000, 0, 0, 0x0, 0x8, 0x0}, */
> + /* 88.2k */
> + /* {12288000, 88200, 20, 7, 0x0, 0x7, 0x1}, */
> + /* 48k */
> + {12288000, 48000, 0, 0, 0x0, 0x4, 0x0},
> + /* 44.1k */
> + {12288000, 44100, 40, 27, 0x0, 0x3, 0x1},
> + {11289600, 44100, 0, 0, 0x0, 0x4, 0x0},
> + /* 22.050k */
> + {12288000, 22050, 40, 27, 0x1, 0x3, 0x1},
> + {11289600, 22050, 0, 0, 0x0, 0x2, 0x0},
> + /* 8k */
> + /* {12288000, 8000, 3, 2, 0x3, 0x2, 0x1}, */
> +
> +};
Don't these need to be calculated - as they depend on the crystal input?
[snip]
> +static int adau1371_init(struct snd_soc_device *socdev)
> +{
> + struct snd_soc_codec *codec = socdev->card->codec;
> + int reg = 0, ret = 0;
> + struct adau1371_priv *adau1371 = codec->private_data;
> + codec->name = "adau1371";
> + codec->owner = THIS_MODULE;
> + codec->read = adau1371_read_reg_cache;
> + codec->write = adau1371_write;
> + codec->set_bias_level = adau1371_set_bias_level;
> + codec->dai = &adau1371_dai;
> + codec->num_dai = 1;
> + codec->reg_cache_size = sizeof(adau1371_reg);
> + codec->reg_cache = kmemdup(adau1371_reg, sizeof(adau1371_reg),
> + GFP_KERNEL);
> + if (codec->reg_cache == NULL)
> + return -ENOMEM;
> +
> + adau1371_reset(codec);
> + /* By default line out and input A are enabled*/
> + adau1371->out_chan_mask = PB_LINE;
> + adau1371->in_chan_mask = CAP_INPA;
> + /* register pcms */
> + ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
> + if (ret < 0) {
> + pr_err("adau1371: failed to create pcms\n");
> + goto pcm_err;
> + }
> + /* Playback mix settings, line out switched to DACs*/
> + adau1371_write(codec, ADAU1371_LLINEMIX, LDAC_SIGNAL_ENA);
> + adau1371_write(codec, ADAU1371_RLINEMIX, RDAC_SIGNAL_ENA);
> + /* Line out volume gain:0 db by default */
0db by default, or should things be muted to start? (to reduce click/pop)
> + adau1371_write(codec, ADAU1371_LLINEVOL, 0x1f);
> + adau1371_write(codec, ADAU1371_RLINEVOL, 0x1f);
> + adau1371_write(codec, ADAU1371_DAIAPBLVOL, 0x80);
> + adau1371_write(codec, ADAU1371_DAIAPBRVOL, 0x80);
> +
> + /* Capture mix settings, AIN1 switched to ADCs */
> + adau1371_write(codec, ADAU1371_LADCMIX, AIN1_SIGNAL_ENA);
> + adau1371_write(codec, ADAU1371_RADCMIX, AIN1_SIGNAL_ENA);
> + /* Input volume gain:0 db by default */
> + adau1371_write(codec, ADAU1371_INPUTMODE, 0x00);
> + adau1371_write(codec, ADAU1371_INALVOL, 0x1f);
> + adau1371_write(codec, ADAU1371_INARVOL, 0x1f);
> + adau1371_write(codec, ADAU1371_DAIRECLVOL, 0x80);
> + adau1371_write(codec, ADAU1371_DAIRECRVOL, 0x80);
> + /* Shoul be set on REVB to enable ADCs*/
> + adau1371_write(codec, ADAU1371_CODECCTL, 0x10);
> + /* 64 bits per frame*/
> + adau1371_write(codec, ADAU1371_BCLKDIV, BCLKDIV_BPFA64);
> + /* Use PLL, the clock divisor is 4 */
> + adau1371_write(codec, ADAU1371_CLKSDIV, (3 << CLKSDIV_CLKDIV_SHIFT));
> + /* PWR on input port A,right and left ADCs.*/
> + reg = PWRCTLA_INAPD | PWRCTLA_MICBPD | PWRCTLA_LADCPD | PWRCTLA_RADCPD;
> + adau1371_write(codec, ADAU1371_PWRCTLA, reg);
> + /* PWR on line out,right and left DACs and whole chip.*/
> + reg = PWRCTLB_RLNPD | PWRCTLB_LLNPD | PWRCTLB_RDACPD |\
> + PWRCTLB_LDACPD | PWRCTLB_PWDB;
> + adau1371_write(codec, ADAU1371_PWRCTLB, reg);
I'm just a little concerned that we are setting board specific info in the
codec driver.
_______________________________________________
Linux-kernel-commits mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits