Re: [alsa-devel] [PATCH v3] ASoC: Add support for NAU8825 codec to ASoC
Hi Anatol, Thanks for your suggestion. We are working on different architectures to support NAU8825 ALSA driver, and I also think it is better we can have a cooperation for driver development and upstream. We have checked your code, and we think your implement version should more follow the ALSA upstream coding standards. We have another version of ALSA driver, which also supports most features as you listed. And I think you already got it from our HW engineers. The major difference between our architectures should be the flow and sequence in sleep mode, and some other architecture dependence codes. For the upstream, we think it is better to have only one version source and could support different architectures. Do you agree to use your version to go on the upstream in future? If yes, we can port your driver to our test platform, and then give the feedback to you. On 2015/7/23 上午 01:57, Anatol Pomozov wrote: > Hi Chih-Chiang > > On Mon, Jul 13, 2015 at 12:33 AM, Chih-Chiang Chang > wrote: >> 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. > > At ChromeOS we work with Nuvoton hw engineers on driver for this nice > chip as well. > > Here is current driver code > https://github.com/anatol/linux/blob/nau8825/sound/soc/codecs/nau8825.c > > The functionality is mostly ready. Our driver handles playback, > capture, mic jack detection, an button presses (4 buttons according > Android specification). We need to resolve a few remaining issues, > such as > - chip needs better way to recognize high-impedance input (e.g. Bose > Quiet Comfort 15 headset) > - implement headset cross talk automatic configuration > - general code cleanup > > But otherwise driver is functional and used for testing in our next > generation device. > > > What do you think about joining efforts on this software development > to make great driver for this chip? > >> Signed-off-by: Chih-Chiang Chang >> --- >> v3->v2: >>- fix the wrong definition of reg_default >>- fix the flow of set_sys_clk() and nau8825_set_bias_level() >>- remove unnecessary code in nau8825_volatile_register() and >> nau8825_i2c_probe() >>- add trigger function for ADC/DAC control >>- add some register definitions >>- fix some coding style issues >> 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 | 724 >> + >> sound/soc/codecs/nau8825.h | 399 + >> 4 files changed, 1130 insertions(+) >> create mode 100644 sound/soc/codecs/nau8825.c >> create mode 100644 sound/soc/codecs/nau8825.h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH v3] ASoC: Add support for NAU8825 codec to ASoC
Hi Anatol, Thanks for your suggestion. We are working on different architectures to support NAU8825 ALSA driver, and I also think it is better we can have a cooperation for driver development and upstream. We have checked your code, and we think your implement version should more follow the ALSA upstream coding standards. We have another version of ALSA driver, which also supports most features as you listed. And I think you already got it from our HW engineers. The major difference between our architectures should be the flow and sequence in sleep mode, and some other architecture dependence codes. For the upstream, we think it is better to have only one version source and could support different architectures. Do you agree to use your version to go on the upstream in future? If yes, we can port your driver to our test platform, and then give the feedback to you. On 2015/7/23 上午 01:57, Anatol Pomozov wrote: Hi Chih-Chiang On Mon, Jul 13, 2015 at 12:33 AM, Chih-Chiang Chang ccchan...@nuvoton.com wrote: 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. At ChromeOS we work with Nuvoton hw engineers on driver for this nice chip as well. Here is current driver code https://github.com/anatol/linux/blob/nau8825/sound/soc/codecs/nau8825.c The functionality is mostly ready. Our driver handles playback, capture, mic jack detection, an button presses (4 buttons according Android specification). We need to resolve a few remaining issues, such as - chip needs better way to recognize high-impedance input (e.g. Bose Quiet Comfort 15 headset) - implement headset cross talk automatic configuration - general code cleanup But otherwise driver is functional and used for testing in our next generation device. What do you think about joining efforts on this software development to make great driver for this chip? Signed-off-by: Chih-Chiang Chang ccchan...@nuvoton.com --- v3-v2: - fix the wrong definition of reg_default - fix the flow of set_sys_clk() and nau8825_set_bias_level() - remove unnecessary code in nau8825_volatile_register() and nau8825_i2c_probe() - add trigger function for ADC/DAC control - add some register definitions - fix some coding style issues 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 | 724 + sound/soc/codecs/nau8825.h | 399 + 4 files changed, 1130 insertions(+) create mode 100644 sound/soc/codecs/nau8825.c create mode 100644 sound/soc/codecs/nau8825.h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] 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. Signed-off-by: Chih-Chiang Chang --- v3->v2: - fix the wrong definition of reg_default - fix the flow of set_sys_clk() and nau8825_set_bias_level() - remove unnecessary code in nau8825_volatile_register() and nau8825_i2c_probe() - add trigger function for ADC/DAC control - add some register definitions - fix some coding style issues 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 | 724 + sound/soc/codecs/nau8825.h | 399 + 4 files changed, 1130 insertions(+) create mode 100644 sound/soc/codecs/nau8825.c create mode 100644 sound/soc/codecs/nau8825.h diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index efaafce..1edb781 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -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 @@ -470,6 +471,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 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index cf160d9..db0a4ec 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -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 @@ -256,6 +257,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 diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 000..d3b306a --- /dev/null +++ b/sound/soc/codecs/nau8825.c @@ -0,0 +1,724 @@ +/* + * nau8825.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "nau8825.h" + +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -5400, 100, 0); +static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -10350, 50, 0); + +static const struct snd_kcontrol_new nau8825_snd_controls[] = { +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), +}; + +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), +}; + +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { +/* Input */ +SND_SOC_DAPM_INPUT("MIC"), +SND_SOC_DAPM_SUPPLY("MIC BIAS", NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0, +NULL, 0), +/* Audio Interface */ +SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0, SND_SOC_NOPM, 0, 0), +SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0, SND_SOC_NOPM, 0, 0), +/* DACs */ +SND_S
[PATCH v3] 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. Signed-off-by: Chih-Chiang Chang ccchan...@nuvoton.com --- v3-v2: - fix the wrong definition of reg_default - fix the flow of set_sys_clk() and nau8825_set_bias_level() - remove unnecessary code in nau8825_volatile_register() and nau8825_i2c_probe() - add trigger function for ADC/DAC control - add some register definitions - fix some coding style issues 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 | 724 + sound/soc/codecs/nau8825.h | 399 + 4 files changed, 1130 insertions(+) create mode 100644 sound/soc/codecs/nau8825.c create mode 100644 sound/soc/codecs/nau8825.h diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index efaafce..1edb781 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -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 @@ -470,6 +471,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 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index cf160d9..db0a4ec 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -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 @@ -256,6 +257,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 diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 000..d3b306a --- /dev/null +++ b/sound/soc/codecs/nau8825.c @@ -0,0 +1,724 @@ +/* + * nau8825.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo mh...@nuvoton.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/init.h +#include linux/delay.h +#include linux/pm.h +#include linux/i2c.h +#include linux/gpio.h +#include linux/device.h +#include linux/clk.h +#include linux/regmap.h +#include linux/acpi.h +#include asm/div64.h +#include sound/jack.h +#include sound/core.h +#include sound/pcm.h +#include sound/pcm_params.h +#include sound/soc.h +#include sound/soc-dapm.h +#include sound/initval.h +#include sound/tlv.h +#include linux/delay.h +#include nau8825.h + +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -5400, 100, 0); +static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -10350, 50, 0); + +static const struct snd_kcontrol_new nau8825_snd_controls[] = { +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), +}; + +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), +}; + +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { +/* Input */ +SND_SOC_DAPM_INPUT(MIC), +SND_SOC_DAPM_SUPPLY(MIC BIAS, NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0, +NULL, 0), +/* Audio Interface */ +SND_SOC_DAPM_AIF_IN(AIF1RX, AIF1 Playback, 0
Re: [PATCH v2] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/25 上午 02:28, Mark Brown wrote: > On Thu, Apr 16, 2015 at 05:56:26PM +0800, Chih-Chiang Chang wrote: > >> +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), }; >> +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); >> +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}, > > 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, 0x}, {0x001, 0x00ff}, {0x003, 0x0050}, {0x004, 0x}, {0x005, 0x3126}, {0x006, 0x0008}, {0x007, 0x0010}, {0x008, 0x}, {0x009, 0x6000}, {0x00a, 0xf13c}, {0x00c, 0x000c}, {0x00d, 0x}, {0x00f, 0x0800}, {0x010, 0x}, {0x011, 0x}, {0x012, 0x0010}, {0x013, 0x0015}, {0x014, 0x0110}, {0x015, 0x}, ... >> +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: >> +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:
Re: [alsa-devel] [PATCH v2] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/20 下午 10:57, Lars-Peter Clausen wrote: > On 04/16/2015 11:56 AM, Chih-Chiang Chang wrote: >> 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 > > Looks pretty good now. > >> --- >> 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 >> > [...] >> +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. >> +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), >> +}; > [...] >> + >> +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 */ >> +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. >> +/* select divied VCO input */ >> +regmap_update_bits(nau8825->regmap, NAU8825_FLL_5, >> +NAU8825_FLL_FILTER_SW_MASK, 0x); >> +/* 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) >> +{ >> +struct snd_soc_codec *codec = dai->codec; >> + >> +switch (clk_id) { >> +cas
Re: [PATCH v2] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/25 上午 02:28, Mark Brown wrote: On Thu, Apr 16, 2015 at 05:56:26PM +0800, Chih-Chiang Chang wrote: +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), }; +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); +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}, 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, 0x}, {0x001, 0x00ff}, {0x003, 0x0050}, {0x004, 0x}, {0x005, 0x3126}, {0x006, 0x0008}, {0x007, 0x0010}, {0x008, 0x}, {0x009, 0x6000}, {0x00a, 0xf13c}, {0x00c, 0x000c}, {0x00d, 0x}, {0x00f, 0x0800}, {0x010, 0x}, {0x011, 0x}, {0x012, 0x0010}, {0x013, 0x0015}, {0x014, 0x0110}, {0x015, 0x}, ... +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: +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; } } +/* 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); +/*writing initial register values
Re: [alsa-devel] [PATCH v2] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/20 下午 10:57, Lars-Peter Clausen wrote: On 04/16/2015 11:56 AM, Chih-Chiang Chang wrote: 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 ccchan...@nuvoton.com Looks pretty good now. --- 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 [...] +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. +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), +}; [...] + +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 */ +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. +/* select divied VCO input */ +regmap_update_bits(nau8825-regmap, NAU8825_FLL_5, +NAU8825_FLL_FILTER_SW_MASK, 0x); +/* 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) +{ +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 ... 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); +break; +case NAU8825_INTERNALCLOCK: +set_sys_clk(codec, clk_id); ... and here The same answer to previous. +break; +default: +dev_err(codec-dev, Wrong clock src\n); +return -EINVAL
[PATCH v2] 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. Signed-off-by: Chih-Chiang Chang --- 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 diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 061c465..ad7e205 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -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 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index abe2d7e..9663e1c 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -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 diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 000..f19c117 --- /dev/null +++ b/sound/soc/codecs/nau8825.c @@ -0,0 +1,563 @@ +/* + * nau8825.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "nau8825.h" + +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -5400, 100, 0); +static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -10350, 50, 0); + +static const struct snd_kcontrol_new nau8825_snd_controls[] = { + + 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), +}; + +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), +}; + +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { + + /* Input */ + SND_SOC_DAPM_INPUT("MIC"), + SND_SOC_DAPM_SUPPLY("MIC BIAS", NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0, + NULL, 0), + /* Audio Interface */ + SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0, SND_SOC_NOPM, 0, 0), + /* DACs */ + SND_SOC_DAPM_DAC("DAC L1", NULL, NAU8825_DAC_CTRL, + NAU8825_DAC_L_SFT, 0), + SND_SOC_DAPM_DAC("DAC R1", NULL, NAU8825_DAC_CTRL, +
[PATCH v2] 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. Signed-off-by: Chih-Chiang Chang ccchan...@nuvoton.com --- 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 diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 061c465..ad7e205 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -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 diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index abe2d7e..9663e1c 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -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 diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 000..f19c117 --- /dev/null +++ b/sound/soc/codecs/nau8825.c @@ -0,0 +1,563 @@ +/* + * nau8825.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo mh...@nuvoton.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/init.h +#include linux/delay.h +#include linux/pm.h +#include linux/i2c.h +#include linux/gpio.h +#include linux/device.h +#include linux/clk.h +#include linux/regmap.h +#include linux/acpi.h +#include asm/div64.h +#include sound/jack.h +#include sound/core.h +#include sound/pcm.h +#include sound/pcm_params.h +#include sound/soc.h +#include sound/soc-dapm.h +#include sound/initval.h +#include sound/tlv.h +#include linux/delay.h +#include nau8825.h + +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -5400, 100, 0); +static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -10350, 50, 0); + +static const struct snd_kcontrol_new nau8825_snd_controls[] = { + + 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), +}; + +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), +}; + +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { + + /* Input */ + SND_SOC_DAPM_INPUT(MIC), + SND_SOC_DAPM_SUPPLY(MIC BIAS, NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0, + NULL, 0), + /* Audio Interface */ + SND_SOC_DAPM_AIF_IN(AIF1RX, AIF1 Playback, 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_OUT(AIF1TX, AIF1 Capture, 0, SND_SOC_NOPM, 0, 0), + /* DACs */ + SND_SOC_DAPM_DAC(DAC L1, NULL, NAU8825_DAC_CTRL
Re: [PATCH] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/10 下午 04:38, Lars-Peter Clausen wrote: > On 04/10/2015 09:21 AM, Chih-Chiang Chang wrote: >> 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 >> --- >> include/sound/nau8825_plat.h | 22 ++ >> sound/soc/codecs/Kconfig | 5 + >> sound/soc/codecs/Makefile| 2 + >> sound/soc/codecs/nau8825.c | 593 >> +++ >> sound/soc/codecs/nau8825.h | 150 +++ >> 5 files changed, 772 insertions(+) >> create mode 100644 include/sound/nau8825_plat.h >> create mode 100644 sound/soc/codecs/nau8825.c >> create mode 100644 sound/soc/codecs/nau8825.h >> >> diff --git a/include/sound/nau8825_plat.h b/include/sound/nau8825_plat.h >> new file mode 100644 >> index 000..87afcd3 >> --- /dev/null >> +++ b/include/sound/nau8825_plat.h > > the preferred place for platform_data files is include/linux/platform_data/, > but the driver doesn't seem to use the platform data anyway, so maybe drop it? The data is reserved for future use, so drop it now. For the preferred place for platform_data files, there are more than 300 files in the sound/soc/codecs folder. But I find only few files refer the platform_data files in include/linux/platform_data/ folder, most files put platform_data files in include/sound/ folder. cczhang@MS00-Linux:~/broonie_linux/sound/sound/soc/codecs$ grep -nr "linux/platform_data/" * adau1761.c:20:#include adau1781.c:20:#include adau17x1.h:5:#include adau1977.c:16:#include ssm2518.c:17:#include > > [...] >> diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c >> new file mode 100644 >> index 000..a8c8f59 >> --- /dev/null >> +++ b/sound/soc/codecs/nau8825.c >> @@ -0,0 +1,593 @@ >> +/* >> + * linux/sound/soc/codecs/nau8825.c > > No need to include the file path in the header of the file, this will just > become outdated if the file is ever moved. Removed the file path in source. > > [...] >> +static int nau8825_hw_params(struct snd_pcm_substream *substream, >> +struct snd_pcm_hw_params *params, >> +struct snd_soc_dai *dai); >> +static int nau8825_set_dai_fmt(struct snd_soc_dai *codec_dai, >> +unsigned int fmt); >> +static int nau8825_set_bias_level(struct snd_soc_codec *codec, >> +enum snd_soc_bias_level level); >> +static int nau8825_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, >> +unsigned int freq, int dir); > > These forward declarations don't seem to be necessary. Removed. > > [...] >> +static const struct snd_kcontrol_new nau8825_snd_controls[] = { >> + > >> +SOC_SINGLE("HP Class OP", NAU8825_CLASS_G_CTRL, NAU8825_CLASS_G_SHIFT, >> + 1, 0), > > What is "Class OP"? "Class OP" means amplifier. The code is removed due to DAC DAPM widgets have the same bit control for NAU8825_CLASS_G_CTRL. > >> +SOC_SINGLE("DAC Right", NAU8825_DAC_CTRL, NAU8825_DAC_R_SFT, 1, 0), >> +SOC_SINGLE("DAC Left", NAU8825_DAC_CTRL, NAU8825_DAC_L_SFT, 1, 0), > > The same bits are controlled by the DAC DAPM widgets, there shouldn't be any > controls for them. Removed. > >> +SOC_SINGLE("DAC Right Clock", NAU8825_DAC_CTRL, NAU8825_DAC_CLK_R_SFT, >> +1, 0), >> +SOC_SINGLE("DAC Left Clock", NAU8825_DAC_CTRL, NAU8825_DAC_CLK_L_SFT, >> +1, 0), > > The clock controls should probably not be user controllable but rather be > DAPM supply widgets. The code is not used, removed. > >> +}; >> + >> +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), >> +}; >> + >> +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { >> + >> +SND_SOC_DAPM_INPUT("Mic Jack"), > > Input and output widgets should have the same name as the matching pin of > the CODEC. Modified as below. SND_SOC_DAPM_INPUT("MIC"), > >> +SND_SOC_DAPM_MICBIAS("MIC BIAS",
Re: [PATCH] ASoC: Add support for NAU8825 codec to ASoC
On 2015/4/10 下午 04:38, Lars-Peter Clausen wrote: On 04/10/2015 09:21 AM, Chih-Chiang Chang wrote: 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 ccchan...@nuvoton.com --- include/sound/nau8825_plat.h | 22 ++ sound/soc/codecs/Kconfig | 5 + sound/soc/codecs/Makefile| 2 + sound/soc/codecs/nau8825.c | 593 +++ sound/soc/codecs/nau8825.h | 150 +++ 5 files changed, 772 insertions(+) create mode 100644 include/sound/nau8825_plat.h create mode 100644 sound/soc/codecs/nau8825.c create mode 100644 sound/soc/codecs/nau8825.h diff --git a/include/sound/nau8825_plat.h b/include/sound/nau8825_plat.h new file mode 100644 index 000..87afcd3 --- /dev/null +++ b/include/sound/nau8825_plat.h the preferred place for platform_data files is include/linux/platform_data/, but the driver doesn't seem to use the platform data anyway, so maybe drop it? The data is reserved for future use, so drop it now. For the preferred place for platform_data files, there are more than 300 files in the sound/soc/codecs folder. But I find only few files refer the platform_data files in include/linux/platform_data/ folder, most files put platform_data files in include/sound/ folder. cczhang@MS00-Linux:~/broonie_linux/sound/sound/soc/codecs$ grep -nr linux/platform_data/ * adau1761.c:20:#include linux/platform_data/adau17x1.h adau1781.c:20:#include linux/platform_data/adau17x1.h adau17x1.h:5:#include linux/platform_data/adau17x1.h adau1977.c:16:#include linux/platform_data/adau1977.h ssm2518.c:17:#include linux/platform_data/ssm2518.h [...] diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 000..a8c8f59 --- /dev/null +++ b/sound/soc/codecs/nau8825.c @@ -0,0 +1,593 @@ +/* + * linux/sound/soc/codecs/nau8825.c No need to include the file path in the header of the file, this will just become outdated if the file is ever moved. Removed the file path in source. [...] +static int nau8825_hw_params(struct snd_pcm_substream *substream, +struct snd_pcm_hw_params *params, +struct snd_soc_dai *dai); +static int nau8825_set_dai_fmt(struct snd_soc_dai *codec_dai, +unsigned int fmt); +static int nau8825_set_bias_level(struct snd_soc_codec *codec, +enum snd_soc_bias_level level); +static int nau8825_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, +unsigned int freq, int dir); These forward declarations don't seem to be necessary. Removed. [...] +static const struct snd_kcontrol_new nau8825_snd_controls[] = { + +SOC_SINGLE(HP Class OP, NAU8825_CLASS_G_CTRL, NAU8825_CLASS_G_SHIFT, + 1, 0), What is Class OP? Class OP means amplifier. The code is removed due to DAC DAPM widgets have the same bit control for NAU8825_CLASS_G_CTRL. +SOC_SINGLE(DAC Right, NAU8825_DAC_CTRL, NAU8825_DAC_R_SFT, 1, 0), +SOC_SINGLE(DAC Left, NAU8825_DAC_CTRL, NAU8825_DAC_L_SFT, 1, 0), The same bits are controlled by the DAC DAPM widgets, there shouldn't be any controls for them. Removed. +SOC_SINGLE(DAC Right Clock, NAU8825_DAC_CTRL, NAU8825_DAC_CLK_R_SFT, +1, 0), +SOC_SINGLE(DAC Left Clock, NAU8825_DAC_CTRL, NAU8825_DAC_CLK_L_SFT, +1, 0), The clock controls should probably not be user controllable but rather be DAPM supply widgets. The code is not used, removed. +}; + +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), +}; + +static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = { + +SND_SOC_DAPM_INPUT(Mic Jack), Input and output widgets should have the same name as the matching pin of the CODEC. Modified as below. SND_SOC_DAPM_INPUT(MIC), +SND_SOC_DAPM_MICBIAS(MIC BIAS, NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0), New drivers shouldn't use DAPM_MICBIAS widgets as they are known to be broken. Use a DAPM_SUPPLY widget instead. Modified as below. SND_SOC_DAPM_SUPPLY(MIC BIAS, NAU8825_BOOST, NAU8825_G_BIAS_SFT, 0, NULL, 0), +SND_SOC_DAPM_SUPPLY(micbias, SND_SOC_NOPM, 0, 0, +NULL, SND_SOC_DAPM_PRE_PMU +| SND_SOC_DAPM_POST_PMD), +SND_SOC_DAPM_SUPPLY(vmid, SND_SOC_NOPM, 0, 0, NULL, +SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD), Both the vmid
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
Hi Mark, Sorry late to the response. On 3/6/2015 1:07 PM, Mark Brown wrote: > On Fri, Mar 06, 2015 at 03:28:33PM +0800, Chih-Chiang Chang wrote: > > Please fix your mailer to word wrap within paragraphs, it makes things a > lot easier to read. This seems to violate the kernel's rule. I am using the Thunderbird to do upstream. And in kernel's documentation, it shows we should set "mailnews.wraplength" from "72" to "0". Any way, for your convenience, I already modify the "mailnews.wraplength" back to "72". > >> On 2015/3/4 下午 08:55, Mark Brown wrote: >>> On Wed, Mar 04, 2015 at 08:35:52PM +0800, Chih-Chiang Chang wrote: >>>> On 2015/2/24 下午 10:13, Mark Brown wrote: >>> Add relevant control types if you need them, it's important to have >>> proper stereo controls available to userspace. >> We cannot find suitable macro in file "include\sound\soc.h", so we want to >> add below two macro for our chip. >> SOC_DOUBLE_L_R_VALUE >> SOC_DOUBLE_L_R_TLV > Sounds good. > >>>>> This looks like you're reimplementing regmap's register sequence >>>>> stuff... It's also a *very* large sequence you have, are you sure it's >>>>> all required? It seems like this may be doing a bunch of machine >>>>> specific configuration but since it's all magic numbers it's hard to >>>>> tell. >>>> Initial settings are arranged in order >>> This doesn't answer or address my concern. >> These large number of register setting is used to initial our codec, >> and some of other codec have the same behavior. We will remove few >> unnecessary register default setting and add some remark for >> registers. > I'd really like to have a better understanding of what this is doing - > it can be valid to do this but there are some warning signs here such as > the volume of writes being large in comparison with the set of controls > the driver exposes which mean I'd like to be sure the use matches > expectations. Normally this sort of thing is a small number of fixes > for undocumented registers or updates to register defaults changed in > later revisions of the chip. We have tried to reduce the sequence recently, but it got some issues in the tests. We think these large number of register settings are necessary to our NAU8824 codec. We will provide the comments of all values in source to have a better understanding, is it acceptable to you? > >>> Don't include noise like this in upstream communication, if your company >>> won't fix this then please use an external mail account for upstream >>> communication. >> Our MIS report they have disabled to append message in mail. Hope you do not >> see it in this mail. > It's gone, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
Hi Mark, Sorry late to the response. On 3/6/2015 1:07 PM, Mark Brown wrote: On Fri, Mar 06, 2015 at 03:28:33PM +0800, Chih-Chiang Chang wrote: Please fix your mailer to word wrap within paragraphs, it makes things a lot easier to read. This seems to violate the kernel's rule. I am using the Thunderbird to do upstream. And in kernel's documentation, it shows we should set mailnews.wraplength from 72 to 0. Any way, for your convenience, I already modify the mailnews.wraplength back to 72. On 2015/3/4 下午 08:55, Mark Brown wrote: On Wed, Mar 04, 2015 at 08:35:52PM +0800, Chih-Chiang Chang wrote: On 2015/2/24 下午 10:13, Mark Brown wrote: Add relevant control types if you need them, it's important to have proper stereo controls available to userspace. We cannot find suitable macro in file include\sound\soc.h, so we want to add below two macro for our chip. SOC_DOUBLE_L_R_VALUE SOC_DOUBLE_L_R_TLV Sounds good. This looks like you're reimplementing regmap's register sequence stuff... It's also a *very* large sequence you have, are you sure it's all required? It seems like this may be doing a bunch of machine specific configuration but since it's all magic numbers it's hard to tell. Initial settings are arranged in order This doesn't answer or address my concern. These large number of register setting is used to initial our codec, and some of other codec have the same behavior. We will remove few unnecessary register default setting and add some remark for registers. I'd really like to have a better understanding of what this is doing - it can be valid to do this but there are some warning signs here such as the volume of writes being large in comparison with the set of controls the driver exposes which mean I'd like to be sure the use matches expectations. Normally this sort of thing is a small number of fixes for undocumented registers or updates to register defaults changed in later revisions of the chip. We have tried to reduce the sequence recently, but it got some issues in the tests. We think these large number of register settings are necessary to our NAU8824 codec. We will provide the comments of all values in source to have a better understanding, is it acceptable to you? Don't include noise like this in upstream communication, if your company won't fix this then please use an external mail account for upstream communication. Our MIS report they have disabled to append message in mail. Hope you do not see it in this mail. It's gone, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC
On 2015/3/5 上午 06:32, Paul Bolle wrote: > Chih-Chiang Chang schreef op wo 04-03-2015 om 20:53 [+0800]: >> From fe37688e226f83ba477a3c2fbc1e64946cd4ec4e Mon Sep 17 00:00:00 2001 >> From: Chih-Chiang Chang >> Date: Wed, 4 Mar 2015 20:03:21 +0800 >> Subject: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC > > It seems that none of those lines were needed. Sorry for the wrong patch format, will remove these lines in next submit. > >> --- /dev/null >> +++ b/include/sound/nau8824.h >> @@ -0,0 +1,22 @@ >> +/* >> + * linux/sound/nau8824.h -- Platform data for NAU8824 >> + * >> + * Copyright 2015 Nuvoton Technology Corp. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#ifndef __LINUX_SND_NAU8824_H >> +#define __LINUX_SND_NAU8824_H >> + >> +struct nau8824_platform_data { >> + unsigned int audio_mclk1; >> + unsigned int gpio_irq; >> + int naudint_irq; >> + int headset_detect; >> + int button_press_detect; >> +}; >> + >> +#endif > > In the future something other than just sound/soc/codecs/nau8824.h is > going to include this header, right? > >> --- /dev/null >> +++ b/sound/soc/codecs/nau8824.c >> @@ -0,0 +1,807 @@ >> +/* >> + * linux/sound/soc/codecs/nau8824.c >> + * >> + * Copyright 2015 Nuvoton Technology Corp. >> + * Author: Meng-Huang Kuo >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ > > This states the license is GPL v2. (So do the two headers this patch > adds.) > >> +MODULE_LICENSE("GPL"); > > So that should probably be > MODULE_LICENSE("GPL v2"); We will modify the code to be GPL v2. > > > Paul Bolle > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
On 2015/3/4 下午 08:55, Mark Brown wrote: > On Wed, Mar 04, 2015 at 08:35:52PM +0800, Chih-Chiang Chang wrote: >> On 2015/2/24 下午 10:13, Mark Brown wrote: > >>> I would have expected the headphone volume control to be a stereo >>> (double) control - same for speakers. > >> The nau8824 related registers which control left/right volume are located >> in different addresses and different shift bits. Since there is no available >> preprocessor macro to meet our requirements, the driver consists of >> left/right >> volume control separately. > > Add relevant control types if you need them, it's important to have > proper stereo controls available to userspace. We cannot find suitable macro in file "include\sound\soc.h", so we want to add below two macro for our chip. SOC_DOUBLE_L_R_VALUE SOC_DOUBLE_L_R_TLV > >>>> +struct nau8824_init_reg { >>>> +u8 reg; >>>> +u16 val; >>>> +}; > >>> This looks like you're reimplementing regmap's register sequence >>> stuff... It's also a *very* large sequence you have, are you sure it's >>> all required? It seems like this may be doing a bunch of machine >>> specific configuration but since it's all magic numbers it's hard to >>> tell. > >> Initial settings are arranged in order > > This doesn't answer or address my concern. These large number of register setting is used to initial our codec, and some of other codec have the same behavior. We will remove few unnecessary register default setting and add some remark for registers. > >>>> +/* Dynamic Headset detection enabled */ >>>> +snd_soc_update_bits(codec, 0x01, 0x400, 0x400); >>>> +snd_soc_update_bits(codec, 0x02, 0x0008, 0x0008); >>>> +snd_soc_update_bits(codec, 0x0f, 0x0300, 0x0100); >>>> +snd_soc_write(codec, 0x09, 0xE000); >>>> +snd_soc_write(codec, NAU8824_IRQ_SETTING, 0x1006); >>>> +snd_soc_write(codec, 0x13, 0x1615); >>>> +snd_soc_write(codec, 0x15, 0x0414); >>>> +snd_soc_update_bits(codec, 0x16, 0xFF00, 0x5900); >>>> +snd_soc_update_bits(codec, 0x66, 0x0070, 0x0060); > >>> Too many magic numbers here I think and this looks like it should be >>> configured using platform data and/or the machine driver (what if the >>> headphone detection/IRQ aren't wired up?). I'd also expect to see >>> reporting via the standard interfaces for jack reporting. > >> The above initial settings are for jack detection. As for other jack >> detection flow, it will be implemented in machine driver but not be included >> in >> this application. > > Please either remove this for now or implement it properly. We will remove it. > >> === >> The privileged confidential information contained in this email is intended >> for use only by the addressees as indicated by the original sender of this >> email. If you are not the addressee indicated in this email or are not >> responsible for delivery of the email to such a person, please kindly reply >> to the sender indicating this fact and delete all copies of it from your >> computer and network server immediately. Your cooperation is highly >> appreciated. It is advised that any unauthorized use of confidential >> information of Nuvoton is strictly prohibited; and any information in this >> email irrelevant to the official business of Nuvoton shall be deemed as >> neither given nor endorsed by Nuvoton. > > Don't include noise like this in upstream communication, if your company > won't fix this then please use an external mail account for upstream > communication. Our MIS report they have disabled to append message in mail. Hope you do not see it in this mail. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
On 2015/3/4 下午 08:55, Mark Brown wrote: On Wed, Mar 04, 2015 at 08:35:52PM +0800, Chih-Chiang Chang wrote: On 2015/2/24 下午 10:13, Mark Brown wrote: I would have expected the headphone volume control to be a stereo (double) control - same for speakers. The nau8824 related registers which control left/right volume are located in different addresses and different shift bits. Since there is no available preprocessor macro to meet our requirements, the driver consists of left/right volume control separately. Add relevant control types if you need them, it's important to have proper stereo controls available to userspace. We cannot find suitable macro in file include\sound\soc.h, so we want to add below two macro for our chip. SOC_DOUBLE_L_R_VALUE SOC_DOUBLE_L_R_TLV +struct nau8824_init_reg { +u8 reg; +u16 val; +}; This looks like you're reimplementing regmap's register sequence stuff... It's also a *very* large sequence you have, are you sure it's all required? It seems like this may be doing a bunch of machine specific configuration but since it's all magic numbers it's hard to tell. Initial settings are arranged in order This doesn't answer or address my concern. These large number of register setting is used to initial our codec, and some of other codec have the same behavior. We will remove few unnecessary register default setting and add some remark for registers. +/* Dynamic Headset detection enabled */ +snd_soc_update_bits(codec, 0x01, 0x400, 0x400); +snd_soc_update_bits(codec, 0x02, 0x0008, 0x0008); +snd_soc_update_bits(codec, 0x0f, 0x0300, 0x0100); +snd_soc_write(codec, 0x09, 0xE000); +snd_soc_write(codec, NAU8824_IRQ_SETTING, 0x1006); +snd_soc_write(codec, 0x13, 0x1615); +snd_soc_write(codec, 0x15, 0x0414); +snd_soc_update_bits(codec, 0x16, 0xFF00, 0x5900); +snd_soc_update_bits(codec, 0x66, 0x0070, 0x0060); Too many magic numbers here I think and this looks like it should be configured using platform data and/or the machine driver (what if the headphone detection/IRQ aren't wired up?). I'd also expect to see reporting via the standard interfaces for jack reporting. The above initial settings are for jack detection. As for other jack detection flow, it will be implemented in machine driver but not be included in this application. Please either remove this for now or implement it properly. We will remove it. === The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton. Don't include noise like this in upstream communication, if your company won't fix this then please use an external mail account for upstream communication. Our MIS report they have disabled to append message in mail. Hope you do not see it in this mail. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC
On 2015/3/5 上午 06:32, Paul Bolle wrote: Chih-Chiang Chang schreef op wo 04-03-2015 om 20:53 [+0800]: From fe37688e226f83ba477a3c2fbc1e64946cd4ec4e Mon Sep 17 00:00:00 2001 From: Chih-Chiang Chang ccchan...@nuvoton.com Date: Wed, 4 Mar 2015 20:03:21 +0800 Subject: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC It seems that none of those lines were needed. Sorry for the wrong patch format, will remove these lines in next submit. --- /dev/null +++ b/include/sound/nau8824.h @@ -0,0 +1,22 @@ +/* + * linux/sound/nau8824.h -- Platform data for NAU8824 + * + * Copyright 2015 Nuvoton Technology Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __LINUX_SND_NAU8824_H +#define __LINUX_SND_NAU8824_H + +struct nau8824_platform_data { + unsigned int audio_mclk1; + unsigned int gpio_irq; + int naudint_irq; + int headset_detect; + int button_press_detect; +}; + +#endif In the future something other than just sound/soc/codecs/nau8824.h is going to include this header, right? --- /dev/null +++ b/sound/soc/codecs/nau8824.c @@ -0,0 +1,807 @@ +/* + * linux/sound/soc/codecs/nau8824.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo mh...@nuvoton.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ This states the license is GPL v2. (So do the two headers this patch adds.) +MODULE_LICENSE(GPL); So that should probably be MODULE_LICENSE(GPL v2); We will modify the code to be GPL v2. Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ASoC: Add support for NAU8824 codec to ASoC
>From fe37688e226f83ba477a3c2fbc1e64946cd4ec4e Mon Sep 17 00:00:00 2001 From: Chih-Chiang Chang Date: Wed, 4 Mar 2015 20:03:21 +0800 Subject: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC Signed-off-by: Chih-Chiang Chang --- include/sound/nau8824.h| 22 ++ sound/soc/codecs/Kconfig | 5 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/nau8824.c | 807 + sound/soc/codecs/nau8824.h | 380 + 5 files changed, 1216 insertions(+) create mode 100644 include/sound/nau8824.h create mode 100644 sound/soc/codecs/nau8824.c create mode 100644 sound/soc/codecs/nau8824.h diff --git a/include/sound/nau8824.h b/include/sound/nau8824.h new file mode 100644 index 000..edb5f08 --- /dev/null +++ b/include/sound/nau8824.h @@ -0,0 +1,22 @@ +/* + * linux/sound/nau8824.h -- Platform data for NAU8824 + * + * Copyright 2015 Nuvoton Technology Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __LINUX_SND_NAU8824_H +#define __LINUX_SND_NAU8824_H + +struct nau8824_platform_data { + unsigned int audio_mclk1; + unsigned int gpio_irq; + int naudint_irq; + int headset_detect; + int button_press_detect; +}; + +#endif diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 064e6c1..862b7bd 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -75,6 +75,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_NAU8824 if I2C select SND_SOC_HDMI_CODEC select SND_SOC_PCM1681 if I2C select SND_SOC_PCM1792A if SPI_MASTER @@ -463,6 +464,10 @@ config SND_SOC_MAX98357A config SND_SOC_MAX9850 tristate +config SND_SOC_NAU8824 + tristate "Nuvoton NAU8824 CODEC" + depends on I2C + config SND_SOC_PCM1681 tristate "Texas Instruments PCM1681 CODEC" depends on I2C diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 69b8666..acb7bda 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -68,6 +68,7 @@ snd-soc-max98357a-objs := max98357a.o snd-soc-max9850-objs := max9850.o snd-soc-mc13783-objs := mc13783.o snd-soc-ml26124-objs := ml26124.o +snd-soc-nau8824-objs := nau8824.o snd-soc-hdmi-codec-objs := hdmi.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm1792a-codec-objs := pcm1792a.o @@ -250,6 +251,7 @@ obj-$(CONFIG_SND_SOC_MAX98357A) += snd-soc-max98357a.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_NAU8824) += snd-soc-nau8824.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 diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c new file mode 100644 index 000..8a19b77 --- /dev/null +++ b/sound/soc/codecs/nau8824.c @@ -0,0 +1,807 @@ +/* + * linux/sound/soc/codecs/nau8824.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "nau8824.h" + +static int nau8824_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai); +static int nau8824_set_dai_fmt(struct snd_soc_dai *codec_dai, + unsigned int fmt); +static int nau8824_set_bias_level(struct snd_soc_codec *codec, + enum snd_soc_bias_level level); +static int nau8824_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, + unsigned int freq, int dir); +static const DECLARE_TLV_DB_SCALE(out_spk_vol_tlv, 0, 100, 0); +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -3000, 100, 0); +static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -12800, 50, 0); +static const DECLARE_TLV_DB_SCALE(in_vol_tlv, -3450, 150, 0); +static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -12800, 50, 0); +static const DECLARE_TLV_DB_SCALE(adc_left_vol_tlv, -12750, 50, 1); +static const DECLARE_TLV_DB_SCALE(adc_right_vol_tlv, -12750, 50, 1); + +static const struct snd_kcontrol_new nau8824_snd_controls[] = { + /* SP Class-D mute control *
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
On 2015/2/24 下午 10:13, Mark Brown wrote: > On Sun, Feb 15, 2015 at 03:49:30PM +0800, Wan Zongshun wrote: > >> +/* SP Class-D mute control */ >> +SOC_DOUBLE("HP Playback Switch", NAU8824_HP_MUTE, NAU8824_L_MUTE_SFT, >> +NAU8824_R_MUTE_SFT, 1, 1), >> +SOC_SINGLE_TLV("HP Left Volume", NAU8824_HP_VOL, NAU8824_L_VOL_SFT, >> +NAU8824_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), >> +SOC_SINGLE_TLV("HP Right Volume", NAU8824_HP_VOL, NAU8824_R_VOL_SFT, >> +NAU8824_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), > > I would have expected the headphone volume control to be a stereo > (double) control - same for speakers. The nau8824 related registers which control left/right volume are located in different addresses and different shift bits. Since there is no available preprocessor macro to meet our requirements, the driver consists of left/right volume control separately. > >> +/* DMIC control */ >> +SOC_DOUBLE("DMIC L R Switch", NAU8824_ENA_CTRL, NAU8824_DMIC_CH0_SFT, >> +NAU8824_DMIC_CH1_SFT, 1, 0), >> +SOC_SINGLE("DMIC Enable", NAU8824_BIAS_ADJ, NAU8824_DMIC1_SFT, 1, 0), >> +SOC_SINGLE("VMID Switch", NAU8824_BIAS_ADJ, NAU8824_VMID_SFT, 1, 0), >> +SOC_SINGLE("BIAS Switch", NAU8824_BOOST, NAU8824_G_BIAS_SFT, 1, 0), > > This all looks like stuff that should be done with DAPM... The above controls have been done with DAPM in next patch submit. > >> +SOC_DOUBLE_R_TLV("MIC L R Gain", NAU8824_ADC_CH0_DGAIN_CTRL, >> +NAU8824_ADC_CH1_DGAIN_CTRL, 0, >> +NAU8824_ADC_VOL_RSCL_RANGE, 0, adc_vol_tlv), > > All volume controls should be called Volume. The naming has been changed. > >> +static int set_dmic_clk(struct snd_soc_dapm_widget *w, >> +struct snd_kcontrol *kcontrol, int event) >> +{ >> +struct snd_soc_codec *codec = w->codec; > > w->codec is going away, use snd_soc_dapm_to_codec(). You should always > submit against current code. Modified done in next patch submit. > >> +static const struct snd_kcontrol_new nau8824_rec_l_mix[] = { >> +SOC_DAPM_SINGLE("BST1 Switch", SND_SOC_NOPM, >> +0, 0, 0), >> +}; > > Please use normal indentation, a single tab is enough. Modified done in next patch submit. > >> +static int nau8824_hp_event(struct snd_soc_dapm_widget *w, >> +struct snd_kcontrol *kcontrol, int event) >> +{ >> +return 0; >> +} > > Remove empty functions. Modified done in next patch submit. > >> +switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { >> +case SND_SOC_DAIFMT_CBM_CFM: >> +reg_val_2 |= NAU8824_I2S_MS_M; >> +break; >> +case SND_SOC_DAIFMT_CBS_CFS: >> +break; >> +case SND_SOC_DAIFMT_CBS_CFM: >> +break; > > These two clocking configurations appear not to differ in terms of what > we do to the device - this suggests that only one of them is actually > supported. Modified done in next patch submit. > >> +static int nau8824_FLL_freerun_mode(struct snd_soc_codec *codec, int >> IsEnable) >> +{ >> +if (IsEnable == true) { > > This doesn't look like Linux coding style... Remove this function > >> +void set_sys_clk(struct snd_soc_codec *codec, int sys_clk) >> +{ >> +pr_debug("%s sys_clk=%x\n", __func__, sys_clk); >> +switch (sys_clk) { >> +case NAU8824_INTERNALCLOCK: >> +nau8824_FLL_freerun_mode(codec, true); >> +break; >> + >> +case NAU8824_MCLK: >> +default: >> +nau8824_FLL_freerun_mode(codec, false); >> +break; >> +} >> +} > > ...and I don't entirely see why it's a separate function to this (which > is itself an internal wrapper function which possibly shouldn't exist)> Remove "nau8824_FLL_freerun_mode()" function > >> +static int nau8824_set_sysclk(struct snd_soc_codec *codec, >> +int clk_id, int source, unsigned int freq, int dir) >> +{ >> +int divider = 1; >> + >> +if (clk_id == NAU8824_MCLK) { >> +set_sys_clk(codec, NAU8824_MCLK); >> +dev_dbg(codec->dev, "%s: input freq = %d divider = %d", >> +__func__, freq, divider); >> + >> +} else if (clk_id == NAU8824_INTERNALCLOCK) { >> +set_sys_clk(codec, NAU8824_INTERNALCLOCK); >> +} else { >> +dev_err(codec->dev, "Wrong clock src\n"); >> +return -EINVAL; >> +} > > Use switch statements rather than if trees like other drivers. It looks > like clk_id should actually be source since we're selecting the clock to > use rather than selecting which clock to configure. Modified done in next patch submit. > >> +static int nau8824_set_bias_level(struct snd_soc_codec *codec, >> +enum snd_soc_bias_level level) >> +{ >> +dev_dbg(codec->dev, "## nau8824_set_bias_level %d\n", level); >> +if (level ==
[PATCH v2] ASoC: Add support for NAU8824 codec to ASoC
From fe37688e226f83ba477a3c2fbc1e64946cd4ec4e Mon Sep 17 00:00:00 2001 From: Chih-Chiang Chang ccchan...@nuvoton.com Date: Wed, 4 Mar 2015 20:03:21 +0800 Subject: [PATCH v2] ASoC: Add support for NAU8824 codec to ASoC Signed-off-by: Chih-Chiang Chang ccchan...@nuvoton.com --- include/sound/nau8824.h| 22 ++ sound/soc/codecs/Kconfig | 5 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/nau8824.c | 807 + sound/soc/codecs/nau8824.h | 380 + 5 files changed, 1216 insertions(+) create mode 100644 include/sound/nau8824.h create mode 100644 sound/soc/codecs/nau8824.c create mode 100644 sound/soc/codecs/nau8824.h diff --git a/include/sound/nau8824.h b/include/sound/nau8824.h new file mode 100644 index 000..edb5f08 --- /dev/null +++ b/include/sound/nau8824.h @@ -0,0 +1,22 @@ +/* + * linux/sound/nau8824.h -- Platform data for NAU8824 + * + * Copyright 2015 Nuvoton Technology Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __LINUX_SND_NAU8824_H +#define __LINUX_SND_NAU8824_H + +struct nau8824_platform_data { + unsigned int audio_mclk1; + unsigned int gpio_irq; + int naudint_irq; + int headset_detect; + int button_press_detect; +}; + +#endif diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 064e6c1..862b7bd 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -75,6 +75,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_NAU8824 if I2C select SND_SOC_HDMI_CODEC select SND_SOC_PCM1681 if I2C select SND_SOC_PCM1792A if SPI_MASTER @@ -463,6 +464,10 @@ config SND_SOC_MAX98357A config SND_SOC_MAX9850 tristate +config SND_SOC_NAU8824 + tristate Nuvoton NAU8824 CODEC + depends on I2C + config SND_SOC_PCM1681 tristate Texas Instruments PCM1681 CODEC depends on I2C diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 69b8666..acb7bda 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -68,6 +68,7 @@ snd-soc-max98357a-objs := max98357a.o snd-soc-max9850-objs := max9850.o snd-soc-mc13783-objs := mc13783.o snd-soc-ml26124-objs := ml26124.o +snd-soc-nau8824-objs := nau8824.o snd-soc-hdmi-codec-objs := hdmi.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm1792a-codec-objs := pcm1792a.o @@ -250,6 +251,7 @@ obj-$(CONFIG_SND_SOC_MAX98357A) += snd-soc-max98357a.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_NAU8824) += snd-soc-nau8824.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 diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c new file mode 100644 index 000..8a19b77 --- /dev/null +++ b/sound/soc/codecs/nau8824.c @@ -0,0 +1,807 @@ +/* + * linux/sound/soc/codecs/nau8824.c + * + * Copyright 2015 Nuvoton Technology Corp. + * Author: Meng-Huang Kuo mh...@nuvoton.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/init.h +#include linux/delay.h +#include linux/pm.h +#include linux/i2c.h +#include linux/gpio.h +#include linux/device.h +#include linux/clk.h +#include linux/regmap.h +#include linux/acpi.h +#include asm/div64.h +#include sound/jack.h +#include sound/core.h +#include sound/pcm.h +#include sound/pcm_params.h +#include sound/soc.h +#include sound/soc-dapm.h +#include sound/initval.h +#include sound/tlv.h +#include linux/delay.h + +#include nau8824.h + +static int nau8824_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai); +static int nau8824_set_dai_fmt(struct snd_soc_dai *codec_dai, + unsigned int fmt); +static int nau8824_set_bias_level(struct snd_soc_codec *codec, + enum snd_soc_bias_level level); +static int nau8824_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, + unsigned int freq, int dir); +static const DECLARE_TLV_DB_SCALE(out_spk_vol_tlv, 0, 100, 0); +static const DECLARE_TLV_DB_SCALE(out_hp_vol_tlv, -3000, 100, 0); +static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -12800, 50, 0); +static const DECLARE_TLV_DB_SCALE(in_vol_tlv, -3450, 150, 0); +static const
Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
On 2015/2/24 下午 10:13, Mark Brown wrote: On Sun, Feb 15, 2015 at 03:49:30PM +0800, Wan Zongshun wrote: +/* SP Class-D mute control */ +SOC_DOUBLE(HP Playback Switch, NAU8824_HP_MUTE, NAU8824_L_MUTE_SFT, +NAU8824_R_MUTE_SFT, 1, 1), +SOC_SINGLE_TLV(HP Left Volume, NAU8824_HP_VOL, NAU8824_L_VOL_SFT, +NAU8824_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), +SOC_SINGLE_TLV(HP Right Volume, NAU8824_HP_VOL, NAU8824_R_VOL_SFT, +NAU8824_VOL_RSCL_RANGE, 1, out_hp_vol_tlv), I would have expected the headphone volume control to be a stereo (double) control - same for speakers. The nau8824 related registers which control left/right volume are located in different addresses and different shift bits. Since there is no available preprocessor macro to meet our requirements, the driver consists of left/right volume control separately. +/* DMIC control */ +SOC_DOUBLE(DMIC L R Switch, NAU8824_ENA_CTRL, NAU8824_DMIC_CH0_SFT, +NAU8824_DMIC_CH1_SFT, 1, 0), +SOC_SINGLE(DMIC Enable, NAU8824_BIAS_ADJ, NAU8824_DMIC1_SFT, 1, 0), +SOC_SINGLE(VMID Switch, NAU8824_BIAS_ADJ, NAU8824_VMID_SFT, 1, 0), +SOC_SINGLE(BIAS Switch, NAU8824_BOOST, NAU8824_G_BIAS_SFT, 1, 0), This all looks like stuff that should be done with DAPM... The above controls have been done with DAPM in next patch submit. +SOC_DOUBLE_R_TLV(MIC L R Gain, NAU8824_ADC_CH0_DGAIN_CTRL, +NAU8824_ADC_CH1_DGAIN_CTRL, 0, +NAU8824_ADC_VOL_RSCL_RANGE, 0, adc_vol_tlv), All volume controls should be called Volume. The naming has been changed. +static int set_dmic_clk(struct snd_soc_dapm_widget *w, +struct snd_kcontrol *kcontrol, int event) +{ +struct snd_soc_codec *codec = w-codec; w-codec is going away, use snd_soc_dapm_to_codec(). You should always submit against current code. Modified done in next patch submit. +static const struct snd_kcontrol_new nau8824_rec_l_mix[] = { +SOC_DAPM_SINGLE(BST1 Switch, SND_SOC_NOPM, +0, 0, 0), +}; Please use normal indentation, a single tab is enough. Modified done in next patch submit. +static int nau8824_hp_event(struct snd_soc_dapm_widget *w, +struct snd_kcontrol *kcontrol, int event) +{ +return 0; +} Remove empty functions. Modified done in next patch submit. +switch (fmt SND_SOC_DAIFMT_MASTER_MASK) { +case SND_SOC_DAIFMT_CBM_CFM: +reg_val_2 |= NAU8824_I2S_MS_M; +break; +case SND_SOC_DAIFMT_CBS_CFS: +break; +case SND_SOC_DAIFMT_CBS_CFM: +break; These two clocking configurations appear not to differ in terms of what we do to the device - this suggests that only one of them is actually supported. Modified done in next patch submit. +static int nau8824_FLL_freerun_mode(struct snd_soc_codec *codec, int IsEnable) +{ +if (IsEnable == true) { This doesn't look like Linux coding style... Remove this function +void set_sys_clk(struct snd_soc_codec *codec, int sys_clk) +{ +pr_debug(%s sys_clk=%x\n, __func__, sys_clk); +switch (sys_clk) { +case NAU8824_INTERNALCLOCK: +nau8824_FLL_freerun_mode(codec, true); +break; + +case NAU8824_MCLK: +default: +nau8824_FLL_freerun_mode(codec, false); +break; +} +} ...and I don't entirely see why it's a separate function to this (which is itself an internal wrapper function which possibly shouldn't exist) Remove nau8824_FLL_freerun_mode() function +static int nau8824_set_sysclk(struct snd_soc_codec *codec, +int clk_id, int source, unsigned int freq, int dir) +{ +int divider = 1; + +if (clk_id == NAU8824_MCLK) { +set_sys_clk(codec, NAU8824_MCLK); +dev_dbg(codec-dev, %s: input freq = %d divider = %d, +__func__, freq, divider); + +} else if (clk_id == NAU8824_INTERNALCLOCK) { +set_sys_clk(codec, NAU8824_INTERNALCLOCK); +} else { +dev_err(codec-dev, Wrong clock src\n); +return -EINVAL; +} Use switch statements rather than if trees like other drivers. It looks like clk_id should actually be source since we're selecting the clock to use rather than selecting which clock to configure. Modified done in next patch submit. +static int nau8824_set_bias_level(struct snd_soc_codec *codec, +enum snd_soc_bias_level level) +{ +dev_dbg(codec-dev, ## nau8824_set_bias_level %d\n, level); +if (level == codec-dapm.bias_level) { +dev_dbg(codec-dev, ##set_bias_level: level returning...\r\n); +return -EINVAL; +} Why is this here - other drivers don't do this and it doesn't look device specific? Remove it