>-----Original Message-----
>From: Robin Getz [mailto:[email protected]]
>Sent: Friday, September 25, 2009 10:43 PM
>To: [email protected]
>Cc: [email protected]
>Subject: Re: [Linux-kernel-commits] [7448] trunk/sound/soc:
>task[#5443]addADAU1371 support
>
>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"?
This should be moved to platform file.
>[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?
These are default working values,may be removed later.
>> +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.
Will be dropped for the release revision.
>
>> + 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?
>
The rates commented can't be got through PLL
Currently,and Yao Wei said he will check it with me once he is free.
>[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)
This is an initial working value,the click/pop wil be heared the first
time
Codec is powered on,no matter if it is muted or not.
>> + 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.
Some parameters such as volume,mute/unmute,input mux .etc can be changed
through amixer,other system specific parameters will be moved
To platform file.
Cliff
_______________________________________________
Linux-kernel-commits mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
