ASoC: Add support for NAU8825 codec to ASoC
PATCH v2 1/1 ASoC: Add support for NAU8825 codec to ASoC
The NAU88L25 is an ultra-low power high performance audio codec designed for smartphone, tablet PC, and other portable devices by Nuvoton, now add linux driver support for it.
---v2->v1:
- fixes according to Lars-Peter Clausen's review comments
- removes unused platform data file
- corrects the naming of DAPM input widget
- fixes some wrong coding of SOC widgets and other codes
- adds definition and remark for config FLL clock
- moves the code of reset hardware registers from codec_probe() to i2c_probe()
- removes unused codes
sound/soc/codecs/Kconfig | 5 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/nau8825.c | 563 +++++++++++++++++++++++++++++++++++++++++++++ sound/soc/codecs/nau8825.h | 170 ++++++++++++++ 4 files changed, 740 insertions(+) create mode 100644 sound/soc/codecs/nau8825.c create mode 100644 sound/soc/codecs/nau8825.h
sound/soc/codecs/Kconfig +5 -0
@@ -76,6 +76,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_MAX9877 if I2C select SND_SOC_MC13783 if MFD_MC13XXX select SND_SOC_ML26124 if I2C + select SND_SOC_NAU8825 if I2C select SND_SOC_HDMI_CODEC select SND_SOC_PCM1681 if I2C select SND_SOC_PCM1792A if SPI_MASTER
@@ -468,6 +469,10 @@ config SND_SOC_MAX98925 config SND_SOC_MAX9850 tristate +config SND_SOC_NAU8825 + tristate "Nuvoton NAU8825 CODEC" + depends on I2C + config SND_SOC_PCM1681 tristate "Texas Instruments PCM1681 CODEC" depends on I2C
sound/soc/codecs/Makefile +2 -0
@@ -69,6 +69,7 @@ snd-soc-max98925-objs := max98925.o snd-soc-max9850-objs := max9850.o snd-soc-mc13783-objs := mc13783.o snd-soc-ml26124-objs := ml26124.o +snd-soc-nau8825-objs := nau8825.o snd-soc-hdmi-codec-objs := hdmi.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm1792a-codec-objs := pcm1792a.o
@@ -254,6 +255,7 @@ obj-$(CONFIG_SND_SOC_MAX98925) += snd-soc-max98925.o obj-$(CONFIG_SND_SOC_MAX9850) += snd-soc-max9850.o obj-$(CONFIG_SND_SOC_MC13783) += snd-soc-mc13783.o obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o +obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_HDMI_CODEC) += snd-soc-hdmi-codec.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM1792A) += snd-soc-pcm1792a-codec.o
sound/soc/codecs/nau8825.c new file +563 -0
563 lines not on this page · raw message
sound/soc/codecs/nau8825.h new file +170 -0
170 lines not on this page · raw message
Signature · 1 line
1.9.3
Quoted 5 lines · Chih-Chiang Chang
The NAU88L25 is an ultra-low power high performance audio codec designed for smartphone, tablet PC, and other portable devices by Nuvoton, now add linux driver support for it.
Signed-off-by Chih-Chiang Chang <[email protected]>
Looks pretty good now.
Quoted 10 lines
---
v2->v1:
- fixes according to Lars-Peter Clausen's review comments
- removes unused platform data file
- corrects the naming of DAPM input widget
- fixes some wrong coding of SOC widgets and other codes
- adds definition and remark for config FLL clock
- moves the code of reset hardware registers fromcodec_probe()toi2c_probe()
- removes unused codes
[...]
Review
Quoted 2 lines
(quoted diff) +2
-0+static const struct snd_kcontrol_new nau8825_snd_controls[] = { +
Here and a few other places you leave the first line in the struct declaration empty. No need for that.
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+SOC_SINGLE_TLV ("MIC Volume" ,NAU8825_ADC_DGAIN_CTRL , +NAU8825_ADC_DGAIN_SFT , +NAU8825_ADC_VOL_RSCL_RANGE ,0 , adc_vol_tlv), +SOC_DOUBLE_TLV ("HP Volume" ,NAU8825_HSVOL_CTRL , +NAU8825_L_HSVOL_SFT ,NAU8825_R_HSVOL_SFT , + NAU8825_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), +};
[...]
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+ +static void config_fll_clk_12m (struct snd_soc_codec *codec) +{ +struct nau8825_priv *nau8825 =snd_soc_codec_get_drvdata (codec); + +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_CLK_MCLK_SRC_MASK ,0x0003 );
sound/soc/codecs/nau8825.c line 196
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+regmap_update_bits (nau8825->regmap,NAU8825_FLL_1 , +NAU8825_FLL_RATIO_MASK ,0x0001 ); +/* FLL 16-bit fractional input */ +regmap_write (nau8825->regmap,NAU8825_FLL_2 ,0xC49B ); +/* FLL 10-bit integer input */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_3 , +NAU8825_FLL_INTEGER_MASK ,0x0020 ); +/* FLL pre-scaler */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_4 , +NAU8825_FLL_REF_DIV_MASK ,0x0800 );
This seems to use some constant dividers and multipliers for the FLL. Does this expect a specific reference clock from somewhere?
Quoted 33 lines
sound/soc/codecs/nau8825.c new file +33
-0+/* select divied VCO input */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_5 , +NAU8825_FLL_FILTER_SW_MASK ,0x0000 ); +/* FLL sigma delta modulator enable */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_SDM_EN_MASK ,0x4000 ); +} + +static void set_sys_clk (struct snd_soc_codec *codec,int sys_clk) +{ +struct nau8825_priv *nau8825 =snd_soc_codec_get_drvdata (codec); + +pr_debug ("%s :: sys_clk=%x\n" , __func__, sys_clk); +switch (sys_clk) { +case NAU8825_INTERNALCLOCK : +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_DCO_EN_MASK ,NAU8825_DCO_EN ); +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_EN ); +break ; +case NAU8825_MCLK : +default : +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_DCO_EN_MASK ,NAU8825_DCO_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_EN ); +break ; + } +}
sound/soc/codecs/nau8825.c line 239
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+ +static int nau8825_dai_set_sysclk (struct snd_soc_dai *dai, +int clk_id,unsigned int freq,int dir) +{ +struct snd_soc_codec *codec = dai->codec; + +switch (clk_id) { +case NAU8825_MCLK : +config_fll_clk_12m (codec); +set_sys_clk (codec, clk_id);
Maybe just inline the contents of set_sys_clk here ...
sound/soc/codecs/nau8825.c line 239
Quoted 3 lines
sound/soc/codecs/nau8825.c new file +3
-0+break ; +case NAU8825_INTERNALCLOCK : +set_sys_clk (codec, clk_id);
... and here
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+break ; +default : +dev_err (codec->dev,"Wrong clock src\n" ); +return -EINVAL ; + } +return 0 ; +}
[...]
Quoted 40 lines
sound/soc/codecs/nau8825.c new file +40
-0+static const struct reg_default nau8825_reg[] = { +/* enable clock source */ + {0x0001 ,0x07FF }, +/* enable VMID and Bias */ + {0x0076 ,0x3140 }, +/* setup clock divider */ + {0x0003 ,0x0050 }, +/* jack detection configuration */ + {0x000C ,0x0004 }, + {0x000D ,0x00E0 }, + {0x000F ,0x0801 }, + {0x0012 ,0x0010 }, +/* keypad detection configuration */ + {0x0013 ,0x0280 }, + {0x0014 ,0x7310 }, + {0x0015 ,0x050E }, + {0x0016 ,0x1B2A }, +/* audio format configuration */ + {0x001A ,0x0800 }, + {0x001C ,0x000E }, + {0x001D ,0x0010 }, +/* sampling rate control */ + {0x002B ,0x0012 }, +/* DAC sampling rate control */ + {0x002C ,0x0082 }, +/* ADC and DAC mixer control */ + {0x0030 ,0x00D2 }, + {0x0033 ,0x00CF }, + {0x0034 ,0x02CF }, +/* DAC class-G control */ + {0x006A ,0x1003 }, + {0x0050 ,0x2007 }, +/* ADC PGA control */ + {0x0072 ,0x0260 }, +/* DAC power down enabled */ + {0x0080 ,0x03A0 }, +/* enable DAC clock */ + {0x0073 ,0x336C }, +/* enable MIC Bias */ + {0x0074 ,0x5502 },
sound/soc/codecs/nau8825.c line 330
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+/* powerup output driver */ + {0x007F, 0x473C}, + {0x007F, 0x473F}, +/* DAC power down disabled */ + {0x0080, 0x00A0}, +/* adjust MIC Bias */ + {0x0066, 0x2060}, + {0x0066, 0x2060}, + {0x0066, 0x0060}, +};
The register defaults should hold the content of the registers in the power-on reset state of the chip. This is not meant to be used as a initialization sequence.
Quoted 1 line
+
[...]
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+static struct snd_soc_dai_driver nau8825_dai_driver[] = { + { + .name ="nau8825-aif1" , + .playback = { + .stream_name ="AIF1 Playback" , + .channels_min =1 , + .channels_max =2 ,
sound/soc/codecs/nau8825.c line 492
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+ .rates = NAU8825_RATES, + .formats = NAU8825_FORMATS, + }, + .capture = { + .stream_name = "AIF1 Capture", + .channels_min = 1, + .channels_max = 2, + .rates = NAU8825_RATES, + .formats = NAU8825_FORMATS, + },
The indent is a bit strange above.
Quoted 12 lines
sound/soc/codecs/nau8825.c new file +12
-0+ .ops = &nau8825_dai_ops, + } +}; + + +static int nau8825_i2c_probe (struct i2c_client *i2c, +const struct i2c_device_id *i2c_id) +{ +struct nau8825_priv *nau8825; +int i, ret; + + nau8825 = devm_kzalloc(&i2c->dev, sizeof(*nau8825),
sound/soc/codecs/nau8825.c line 514
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+ GFP_KERNEL); +if (nau8825 ==NULL ) +return -ENOMEM ; + nau8825->i2c = i2c; +i2c_set_clientdata (i2c, nau8825); + nau8825->regmap =devm_regmap_init_i2c (i2c, &nau8825_regmap); +if (IS_ERR (nau8825->regmap)) { + ret =PTR_ERR (nau8825->regmap); +dev_err (&i2c->dev,"Failed to allocate regmap: %d\n" , ret); + goto err_enable;
Since there is nothing to do at err_enable, just return ret directly and get rid of the label.
Quoted 16 lines
sound/soc/codecs/nau8825.c new file +16
-0+ } +/* software reset */ +regmap_write (nau8825->regmap,NAU8825_RESET ,0x01 ); +regmap_write (nau8825->regmap,NAU8825_RESET ,0x02 ); +/*writing initial register values to the codec*/ +for (i =0 ; i <ARRAY_SIZE (nau8825_reg); i++) +regmap_write (nau8825->regmap, nau8825_reg[i].reg, + nau8825_reg[i].def); + + ret =snd_soc_register_codec (&i2c->dev, &soc_codec_driver_nau8825, + nau8825_dai_driver, +ARRAY_SIZE (nau8825_dai_driver)); +err_enable: +return ret; +} +
[...]
sound/soc/codecs/nau8825.h line 161
Quoted 3 lines
sound/soc/codecs/nau8825.h new file +3
-0+#define NAU8825_RATES SNDRV_PCM_RATE_8000_192000 +#define NAU8825_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE \ + | SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
These defines should probably be moved to the C file close to their users.
sound/soc/codecs/nau8825.h line 168
Quoted 7 lines
sound/soc/codecs/nau8825.h new file +7
-0+ +struct nau8825_priv { +struct snd_soc_codec *codec; +struct regmap *regmap; +struct i2c_client *i2c; +struct snd_soc_jack *jack; +struct delayed_work jack_detect_work;
All fields in here except for the regmap field are still unused.
Quoted 3 lines
(quoted diff) +2
-0+}; +#endif /* _NAU8825_H */
Mark Brown
maintainersound/soc/codecs/nau8825.c line 53
Quoted 6 lines · Chih-Chiang Chang
sound/soc/codecs/nau8825.c new file +6
-0+static const struct snd_kcontrol_new nau8825_hpo_mix[] = { + +SOC_DAPM_SINGLE ("HP L Switch" ,NAU8825_HSVOL_CTRL , +NAU8825_L_MUTE_SFT ,1 ,1 ), +SOC_DAPM_SINGLE ("HP R Switch" ,NAU8825_HSVOL_CTRL , +NAU8825_R_MUTE_SFT ,1 ,1 ),
Indentation - the continuation lines should be lined up with the (.
Review
Quoted 2 lines
(quoted diff) +2
-0+static void set_sys_clk(struct snd_soc_codec *codec, int sys_clk) +{
This is only used in set_sysclk(), just merge it into there.
Quoted 11 lines
sound/soc/codecs/nau8825.c new file +11
-0+static const struct reg_default nau8825_reg[] = { +/* enable clock source */ + {0x0001 ,0x07FF }, +/* enable VMID and Bias */ + {0x0076 ,0x3140 }, +/* setup clock divider */ + {0x0003 ,0x0050 }, +/* jack detection configuration */ + {0x000C ,0x0004 }, + {0x000D ,0x00E0 }, + {0x000F ,0x0801 },
sound/soc/codecs/nau8825.c line 301
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+ {0x0012, 0x0010}, +/* keypad detection configuration */ + {0x0013, 0x0280}, + {0x0014, 0x7310}, + {0x0015, 0x050E}, + {0x0016, 0x1B2A}, +/* audio format configuration */ + {0x001A, 0x0800}, + {0x001C, 0x000E}, + {0x001D, 0x0010},
This all looks like normal configuration of the device done through a fixed magic number table which isn't what patches are for at all - they are for erratum fixes and similar. Most if not all of this should be configured either from userspace or by the machine driver.
Quoted 8 lines
sound/soc/codecs/nau8825.c new file +8
-0+static bool nau8825_volatile_register (struct device *dev,unsigned int reg) +{ +switch (reg) { +case NAU8825_RESET : +case NAU8825_ENA_CTRL : +case NAU8825_CLK_EN : +case NAU8825_CLK_DIVIDER : +case NAU8825_FLL_1 :
sound/soc/codecs/nau8825.c line 424
Quoted 10 lines
sound/soc/codecs/nau8825.c new file +10
-0+case NAU8825_FLL_2 : +case NAU8825_FLL_3 : +case NAU8825_FLL_4 : +case NAU8825_FLL_5 : +case NAU8825_FLL_6 : +case NAU8825_HEADSET_CTRL : +case NAU8825_JACK_DET_CTRL : +case NAU8825_IRQ_MASK : +case NAU8825_IRQ_CLEAR : +case NAU8825_IRQ_CTRL :
Are you *sure* all these registers are volatile? It isn't impossible of course but it seems like it'd be some very special hardware design. It looks like all the registers are being marked as volatile.
sound/soc/codecs/nau8825.c line 518
Quoted 3 lines
sound/soc/codecs/nau8825.c new file +3
-0+/* software reset */ +regmap_write (nau8825->regmap,NAU8825_RESET ,0x01 ); +regmap_write (nau8825->regmap,NAU8825_RESET ,0x02 );
We did the reset differently in the removal path...
sound/soc/codecs/nau8825.c line 522
Quoted 4 lines
sound/soc/codecs/nau8825.c new file +4
-0+/*writing initial register values to the codec*/ +for (i =0 ; i <ARRAY_SIZE (nau8825_reg); i++) +regmap_write (nau8825->regmap, nau8825_reg[i].reg, + nau8825_reg[i].def);
We should use the reset values the CODEC has.
Quoted 19 lines · Lars-Peter Clausen
Quoted 5 lines · Chih-Chiang Chang
The NAU88L25 is an ultra-low power high performance audio codec designed for smartphone, tablet PC, and other portable devices by Nuvoton, now add linux driver support for it.
Signed-off-by Chih-Chiang Chang <[email protected]>Looks pretty good now.
Quoted 10 lines
---
v2->v1:
- fixes according to Lars-Peter Clausen's review comments
- removes unused platform data file
- corrects the naming of DAPM input widget
- fixes some wrong coding of SOC widgets and other codes
- adds definition and remark for config FLL clock
- moves the code of reset hardware registers fromcodec_probe()toi2c_probe()
- removes unused codes
Quoted 7 lines
[...]
Review
Quoted 2 lines
(quoted diff) +2
-0+static const struct snd_kcontrol_new nau8825_snd_controls[] = { +Here and a few other places you leave the first line in the struct declaration empty. No need for that.
Removed the space line in declarations.
Quoted 20 lines
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+SOC_SINGLE_TLV ("MIC Volume" ,NAU8825_ADC_DGAIN_CTRL , +NAU8825_ADC_DGAIN_SFT , +NAU8825_ADC_VOL_RSCL_RANGE ,0 , adc_vol_tlv), +SOC_DOUBLE_TLV ("HP Volume" ,NAU8825_HSVOL_CTRL , +NAU8825_L_HSVOL_SFT ,NAU8825_R_HSVOL_SFT , + NAU8825_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), +};[...]
Quoted 12 lines
sound/soc/codecs/nau8825.c new file +12
-0+ +static void config_fll_clk_12m (struct snd_soc_codec *codec) +{ +struct nau8825_priv *nau8825 =snd_soc_codec_get_drvdata (codec); + +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_CLK_MCLK_SRC_MASK ,0x0003 ); +regmap_update_bits (nau8825->regmap,NAU8825_FLL_1 , +NAU8825_FLL_RATIO_MASK ,0x0001 ); +/* FLL 16-bit fractional input */ +regmap_write (nau8825->regmap,NAU8825_FLL_2 ,0xC49B ); +/* FLL 10-bit integer input */
Quoted 9 lines
sound/soc/codecs/nau8825.c line 196
Quoted 5 lines
sound/soc/codecs/nau8825.c new file +5
-0+regmap_update_bits (nau8825->regmap,NAU8825_FLL_3 , +NAU8825_FLL_INTEGER_MASK ,0x0020 ); +/* FLL pre-scaler */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_4 , +NAU8825_FLL_REF_DIV_MASK ,0x0800 );This seems to use some constant dividers and multipliers for the FLL. Does this expect a specific reference clock from somewhere?
Yes, current code expects ASoC will send 12 MHz MCLK to the codec. But finally, we will support all possible reference clock frequencies.
Quoted 37 lines
Quoted 37 lines
sound/soc/codecs/nau8825.c new file +37
-0+/* select divied VCO input */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_5 , +NAU8825_FLL_FILTER_SW_MASK ,0x0000 ); +/* FLL sigma delta modulator enable */ +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_SDM_EN_MASK ,0x4000 ); +} + +static void set_sys_clk (struct snd_soc_codec *codec,int sys_clk) +{ +struct nau8825_priv *nau8825 =snd_soc_codec_get_drvdata (codec); + +pr_debug ("%s :: sys_clk=%x\n" , __func__, sys_clk); +switch (sys_clk) { +case NAU8825_INTERNALCLOCK : +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_DCO_EN_MASK ,NAU8825_DCO_EN ); +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_EN ); +break ; +case NAU8825_MCLK : +default : +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_FLL_6 , +NAU8825_DCO_EN_MASK ,NAU8825_DCO_DIS ); +regmap_update_bits (nau8825->regmap,NAU8825_CLK_DIVIDER , +NAU8825_SYSCLK_EN_MASK ,NAU8825_SYSCLK_EN ); +break ; + } +} + +static int nau8825_dai_set_sysclk (struct snd_soc_dai *dai, +int clk_id,unsigned int freq,int dir) +{
Quoted 9 lines
sound/soc/codecs/nau8825.c line 239
Quoted 6 lines
sound/soc/codecs/nau8825.c new file +6
-0+struct snd_soc_codec *codec = dai->codec; + +switch (clk_id) { +case NAU8825_MCLK : +config_fll_clk_12m (codec); +set_sys_clk (codec, clk_id);Maybe just inline the contents of
set_sys_clkhere ...
Since another code as below still uses set_sys_clk(), we reserve function calling here.
case SND_SOC_BIAS_OFF: dev_dbg(codec->dev, "###nau8825_set_bias_level OFF\n"); set_sys_clk(codec, NAU8825_INTERNALCLOCK); regmap_update_bits(nau8825->regmap, NAU8825_BIAS_ADJ, NAU8825_VMID_MASK, NAU8825_VMID_DIS);
Quoted 6 lines
sound/soc/codecs/nau8825.c line 239
Quoted 3 lines
sound/soc/codecs/nau8825.c new file +3
-0+break ; +case NAU8825_INTERNALCLOCK : +set_sys_clk (codec, clk_id);... and here
The same answer to previous.
Quoted 54 lines
Quoted 7 lines
sound/soc/codecs/nau8825.c new file +7
-0+break ; +default : +dev_err (codec->dev,"Wrong clock src\n" ); +return -EINVAL ; + } +return 0 ; +}[...]
Quoted 46 lines
sound/soc/codecs/nau8825.c new file +46
-0+static const struct reg_default nau8825_reg[] = { +/* enable clock source */ + {0x0001 ,0x07FF }, +/* enable VMID and Bias */ + {0x0076 ,0x3140 }, +/* setup clock divider */ + {0x0003 ,0x0050 }, +/* jack detection configuration */ + {0x000C ,0x0004 }, + {0x000D ,0x00E0 }, + {0x000F ,0x0801 }, + {0x0012 ,0x0010 }, +/* keypad detection configuration */ + {0x0013 ,0x0280 }, + {0x0014 ,0x7310 }, + {0x0015 ,0x050E }, + {0x0016 ,0x1B2A }, +/* audio format configuration */ + {0x001A ,0x0800 }, + {0x001C ,0x000E }, + {0x001D ,0x0010 }, +/* sampling rate control */ + {0x002B ,0x0012 }, +/* DAC sampling rate control */ + {0x002C ,0x0082 }, +/* ADC and DAC mixer control */ + {0x0030 ,0x00D2 }, + {0x0033 ,0x00CF }, + {0x0034 ,0x02CF }, +/* DAC class-G control */ + {0x006A ,0x1003 }, + {0x0050 ,0x2007 }, +/* ADC PGA control */ + {0x0072 ,0x0260 }, +/* DAC power down enabled */ + {0x0080 ,0x03A0 }, +/* enable DAC clock */ + {0x0073 ,0x336C }, +/* enable MIC Bias */ + {0x0074 ,0x5502 }, +/* powerup output driver */ + {0x007F ,0x473C }, + {0x007F ,0x473F }, +/* DAC power down disabled */ + {0x0080 ,0x00A0 }, +/* adjust MIC Bias */
Quoted 9 lines
sound/soc/codecs/nau8825.c line 330
Quoted 4 lines
sound/soc/codecs/nau8825.c new file +4
-0+ {0x0066, 0x2060}, + {0x0066, 0x2060}, + {0x0066, 0x0060}, +};The register defaults should hold the content of the registers in the power-on reset state of the chip. This is not meant to be used as a initialization sequence.
Modified code to make register defaults hold the register values in the power-on reset state.
static const struct reg_default nau8825_reg[] = {
{0x000, 0x0000},
{0x001, 0x00ff},
{0x003, 0x0050},
{0x004, 0x0000},
{0x005, 0x3126},
{0x006, 0x0008},
{0x007, 0x0010},
{0x008, 0x0000},
{0x009, 0x6000},
{0x00a, 0xf13c},
{0x00c, 0x000c},
{0x00d, 0x0000},
{0x00f, 0x0800},
{0x010, 0x0000},
{0x011, 0x0000},
{0x012, 0x0010},
{0x013, 0x0015},
{0x014, 0x0110},
{0x015, 0x0000},
...Quoted 13 lines
Quoted 1 line
+
[...]
Quoted 11 lines
sound/soc/codecs/nau8825.c new file +11
-0+static struct snd_soc_dai_driver nau8825_dai_driver[] = { + { + .name ="nau8825-aif1" , + .playback = { + .stream_name ="AIF1 Playback" , + .channels_min =1 , + .channels_max =2 , + .rates =NAU8825_RATES , + .formats =NAU8825_FORMATS , + }, + .capture = {
Quoted 9 lines
sound/soc/codecs/nau8825.c line 492
Quoted 6 lines
sound/soc/codecs/nau8825.c new file +6
-0+ .stream_name = "AIF1 Capture", + .channels_min = 1, + .channels_max = 2, + .rates = NAU8825_RATES, + .formats = NAU8825_FORMATS, + },The indent is a bit strange above.
Fixed the code as below.
static struct snd_soc_dai_driver nau8825_dai_driver[] = {
{
.name = "nau8825-aif1",
.playback = {
.stream_name = "AIF1 Playback",
.channels_min = 1,
.channels_max = 2,
.rates = NAU8825_RATES,
.formats = NAU8825_FORMATS,
},
.capture = {
.stream_name = "AIF1 Capture",
.channels_min = 1,
.channels_max = 2,
.rates = NAU8825_RATES,
.formats = NAU8825_FORMATS,
},
.ops = &nau8825_dai_ops,
}
};Quoted 17 lines
Quoted 17 lines
sound/soc/codecs/nau8825.c new file +17
-0+ .ops = &nau8825_dai_ops, + } +}; + + +static int nau8825_i2c_probe (struct i2c_client *i2c, +const struct i2c_device_id *i2c_id) +{ +struct nau8825_priv *nau8825; +int i, ret; + + nau8825 =devm_kzalloc (&i2c->dev,sizeof (*nau8825), +GFP_KERNEL ); +if (nau8825 ==NULL ) +return -ENOMEM ; + nau8825->i2c = i2c; +i2c_set_clientdata (i2c, nau8825);
Quoted 9 lines
sound/soc/codecs/nau8825.c line 514
Quoted 5 lines
sound/soc/codecs/nau8825.c new file +5
-0+ nau8825->regmap =devm_regmap_init_i2c (i2c, &nau8825_regmap); +if (IS_ERR (nau8825->regmap)) { + ret =PTR_ERR (nau8825->regmap); +dev_err (&i2c->dev,"Failed to allocate regmap: %d\n" , ret); + goto err_enable;Since there is nothing to do at
err_enable, just return ret directly and get rid of the label.
Modified the code as below.
i2c_set_clientdata(i2c, nau8825);
nau8825->regmap = devm_regmap_init_i2c(i2c, &nau8825_regmap);
if (IS_ERR(nau8825->regmap)) {
ret = PTR_ERR(nau8825->regmap);
dev_err(&i2c->dev, "Failed to allocate regmap: %d\n", ret);
return ret;
}
/* software reset */
regmap_write(nau8825->regmap, NAU8825_RESET, 0x00);
regmap_write(nau8825->regmap, NAU8825_RESET, 0x00);
/* register sound card */
ret = snd_soc_register_codec(&i2c->dev, &soc_codec_driver_nau8825,
nau8825_dai_driver,
ARRAY_SIZE(nau8825_dai_driver));
return ret;
}Quoted 17 lines
Quoted 16 lines
sound/soc/codecs/nau8825.c new file +16
-0+ } +/* software reset */ +regmap_write (nau8825->regmap,NAU8825_RESET ,0x01 ); +regmap_write (nau8825->regmap,NAU8825_RESET ,0x02 ); +/*writing initial register values to the codec*/ +for (i =0 ; i <ARRAY_SIZE (nau8825_reg); i++) +regmap_write (nau8825->regmap, nau8825_reg[i].reg, + nau8825_reg[i].def); + + ret =snd_soc_register_codec (&i2c->dev, &soc_codec_driver_nau8825, + nau8825_dai_driver, +ARRAY_SIZE (nau8825_dai_driver)); +err_enable: +return ret; +} +[...]
Quoted 6 lines
sound/soc/codecs/nau8825.h line 161
Quoted 3 lines
sound/soc/codecs/nau8825.h new file +3
-0+#define NAU8825_RATES SNDRV_PCM_RATE_8000_192000 +#define NAU8825_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE \ + | SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)These defines should probably be moved to the C file close to their users.
Move the defines to C file as below.
#define NAU8825_RATES SNDRV_PCM_RATE_8000_192000
#define NAU8825_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
SNDRV_PCM_FMTBIT_S20_3LE \ | SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
static const struct snd_soc_dai_ops nau8825_dai_ops = {
.trigger = nau8825_trigger,
.hw_params = nau8825_hw_params,
.set_sysclk = nau8825_dai_set_sysclk,
.set_fmt = nau8825_set_dai_fmt,
.digital_mute = nau8825_dac_mute,
};static struct snd_soc_dai_driver nau8825_dai_driver[] = {
{
.name = "nau8825-aif1",Quoted 10 lines
sound/soc/codecs/nau8825.h line 168
Quoted 7 lines
sound/soc/codecs/nau8825.h new file +7
-0+ +struct nau8825_priv { +struct snd_soc_codec *codec; +struct regmap *regmap; +struct i2c_client *i2c; +struct snd_soc_jack *jack; +struct delayed_work jack_detect_work;All fields in here except for the regmap field are still unused.
Remove some unused field. But the i2c field is used in nau8825_i2c_probe().
struct nau8825_priv {
struct regmap *regmap;
struct i2c_client *i2c;
};static int nau8825_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *i2c_id)
{
struct nau8825_priv *nau8825;
int ret;
nau8825 = devm_kzalloc(&i2c->dev, sizeof(*nau8825),
GFP_KERNEL);
if (nau8825 == NULL)
return -ENOMEM;
nau8825->i2c = i2c;
i2c_set_clientdata(i2c, nau8825);
nau8825->regmap = devm_regmap_init_i2c(i2c, &nau8825_regmap);Quoted 4 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+}; +#endif /* _NAU8825_H */
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 11 lines · Mark Brown
sound/soc/codecs/nau8825.c line 53
Quoted 6 lines · Chih-Chiang Chang
sound/soc/codecs/nau8825.c new file +6
-0+static const struct snd_kcontrol_new nau8825_hpo_mix[] = { + +SOC_DAPM_SINGLE ("HP L Switch" ,NAU8825_HSVOL_CTRL , +NAU8825_L_MUTE_SFT ,1 ,1 ), +SOC_DAPM_SINGLE ("HP R Switch" ,NAU8825_HSVOL_CTRL , +NAU8825_R_MUTE_SFT ,1 ,1 ),Indentation - the continuation lines should be lined up with the (.
Modified code as below.
static const struct snd_kcontrol_new nau8825_hpo_mix[] = {
SOC_DAPM_SINGLE("HP L Switch", NAU8825_HSVOL_CTRL,
NAU8825_L_MUTE_SFT, 1, 1),
SOC_DAPM_SINGLE("HP R Switch", NAU8825_HSVOL_CTRL,
NAU8825_R_MUTE_SFT, 1, 1),
};Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+static void set_sys_clk(struct snd_soc_codec *codec, int sys_clk) +{This is only used in
set_sysclk(), just merge it into there.
it is also used in the set_bias_level().
case SND_SOC_BIAS_OFF: dev_dbg(codec->dev, "###nau8825_set_bias_level OFF\n"); set_sys_clk(codec, NAU8825_INTERNALCLOCK); regmap_update_bits(nau8825->regmap, NAU8825_BIAS_ADJ, NAU8825_VMID_MASK, NAU8825_VMID_DIS);
Quoted 18 lines
Quoted 18 lines
sound/soc/codecs/nau8825.c new file +18
-0+static const struct reg_default nau8825_reg[] = { +/* enable clock source */ + {0x0001 ,0x07FF }, +/* enable VMID and Bias */ + {0x0076 ,0x3140 }, +/* setup clock divider */ + {0x0003 ,0x0050 }, +/* jack detection configuration */ + {0x000C ,0x0004 }, + {0x000D ,0x00E0 }, + {0x000F ,0x0801 }, + {0x0012 ,0x0010 }, +/* keypad detection configuration */ + {0x0013 ,0x0280 }, + {0x0014 ,0x7310 }, + {0x0015 ,0x050E }, + {0x0016 ,0x1B2A }, +/* audio format configuration */
Quoted 9 lines
sound/soc/codecs/nau8825.c line 301
Quoted 3 lines
sound/soc/codecs/nau8825.c new file +3
-0+ {0x001A, 0x0800}, + {0x001C, 0x000E}, + {0x001D, 0x0010},This all looks like normal configuration of the device done through a fixed magic number table which isn't what patches are for at all - they are for erratum fixes and similar. Most if not all of this should be configured either from userspace or by the machine driver.
In original code, the reg_default hold the registers of initialization sequence. Modified code to make reg_default hold the register values in the power-on reset state. And remove the code of writing initial register values in i2c_probe() function.
static const struct reg_default nau8825_reg[] = {
{0x000, 0x0000},
{0x001, 0x00ff},
{0x003, 0x0050},
{0x004, 0x0000},
{0x005, 0x3126},
{0x006, 0x0008},
{0x007, 0x0010},
{0x008, 0x0000},
{0x009, 0x6000},
{0x00a, 0xf13c},
{0x00c, 0x000c},
{0x00d, 0x0000},
{0x00f, 0x0800},
{0x010, 0x0000},
{0x011, 0x0000},
{0x012, 0x0010},
{0x013, 0x0015},
{0x014, 0x0110},
{0x015, 0x0000},
...Quoted 14 lines
Quoted 14 lines
sound/soc/codecs/nau8825.c new file +14
-0+static bool nau8825_volatile_register (struct device *dev,unsigned int reg) +{ +switch (reg) { +case NAU8825_RESET : +case NAU8825_ENA_CTRL : +case NAU8825_CLK_EN : +case NAU8825_CLK_DIVIDER : +case NAU8825_FLL_1 : +case NAU8825_FLL_2 : +case NAU8825_FLL_3 : +case NAU8825_FLL_4 : +case NAU8825_FLL_5 : +case NAU8825_FLL_6 : +case NAU8825_HEADSET_CTRL :
Quoted 9 lines
sound/soc/codecs/nau8825.c line 372
Quoted 4 lines
sound/soc/codecs/nau8825.c new file +4
-0+case NAU8825_JACK_DET_CTRL : +case NAU8825_IRQ_MASK : +case NAU8825_IRQ_CLEAR : +case NAU8825_IRQ_CTRL :Are you *sure* all these registers are volatile? It isn't impossible of course but it seems like it'd be some very special hardware design. It looks like all the registers are being marked as volatile.
Remove some unnecessary registers.
static bool nau8825_volatile_register(struct device *dev, unsigned int reg)
{
switch (reg) {
case NAU8825_RESET:
case NAU8825_CLK_DIVIDER:
case NAU8825_FLL_1:
case NAU8825_FLL_2:
case NAU8825_FLL_3:
case NAU8825_FLL_4:
case NAU8825_FLL_5:
case NAU8825_FLL_6:
case NAU8825_ANALOG_CTRL_2:
case NAU8825_ANALOG_ADC_1:
case NAU8825_ANALOG_ADC_2:
case NAU8825_DAC_CTRL:
case NAU8825_MIC_BIAS:
case NAU8825_BOOST:
case NAU8825_CLASSG_CTRL:
case NAU8825_I2C_DEVICE_ID:
case NAU8825_BIAS_ADJ:
case NAU8825_POWER_UP_CTRL:
case NAU8825_CHARGE_BUMP_CTRL:
case NAU8825_GENERAL_STATUS:
return true;
default:
return false;
}
}Quoted 6 lines
sound/soc/codecs/nau8825.c line 518
Quoted 3 lines
sound/soc/codecs/nau8825.c new file +3
-0+/* software reset */ +regmap_write (nau8825->regmap,NAU8825_RESET ,0x01 ); +regmap_write (nau8825->regmap,NAU8825_RESET ,0x02 );We did the reset differently in the removal path...
For software reset, our codec must write twice in any value. Sync the code as below in both of i2c_probe() and codec_remove().
regmap_write(nau8825->regmap, NAU8825_RESET, 0x00); regmap_write(nau8825->regmap, NAU8825_RESET, 0x00);
Quoted 7 lines
sound/soc/codecs/nau8825.c line 522
Quoted 4 lines
sound/soc/codecs/nau8825.c new file +4
-0+/*writing initial register values to the codec*/ +for (i =0 ; i <ARRAY_SIZE (nau8825_reg); i++) +regmap_write (nau8825->regmap, nau8825_reg[i].reg, + nau8825_reg[i].def);We should use the reset values the CODEC has.
Yes, removed the code.