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 ccchang12@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 0000000..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?
[...]
diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c new file mode 100644 index 0000000..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.
[...]
+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.
[...]
+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"?
- 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.
- 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.
+};
+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.
- 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.
- 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 and micbias supply seem to be unused and don't do anything either.
- /* ADCs */
- SND_SOC_DAPM_ADC("ADC L", NULL, SND_SOC_NOPM, 0, 0),
- SND_SOC_DAPM_ADC("ADC R", NULL, SND_SOC_NOPM, 0, 0),
- /* ADC IF1 */
- SND_SOC_DAPM_PGA("IF1 ADC", SND_SOC_NOPM, 0, 0, NULL, 0),
This one seems to be unused, and doesn't do anything either.
- /* 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_E("DAC L1", NULL, NAU8825_DAC_CTRL,
NAU8825_DAC_L_SFT, 0, NULL,SND_SOC_DAPM_PRE_PMD),
Just use DAC instead of DAC_E if you don't have to implement a callback
- SND_SOC_DAPM_DAC_E("DAC R1", NULL, NAU8825_DAC_CTRL,
NAU8825_DAC_R_SFT, 0, NULL,SND_SOC_DAPM_PRE_PMD),
Same here.
- /* SPO/HPO/LOUT/Mono Mixer */
- SND_SOC_DAPM_MIXER("HPO MIX", SND_SOC_NOPM, 0, 0, nau8825_hpo_mix,
ARRAY_SIZE(nau8825_hpo_mix)),- SND_SOC_DAPM_PGA_S("HP amp", 1, NAU8825_CLASS_G_CTRL,
NAU8825_CLASS_G_SHIFT, 0, NULL, 0),
No need for _S if there is no sub-sequencing involved.
- /* Output Lines */
- SND_SOC_DAPM_OUTPUT("HPOL"),
- SND_SOC_DAPM_OUTPUT("HPOR"),
+};
[...
+static int nau8825_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *tmp)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_codec *codec = rtd->codec;
rtd->codec does not necessarily point to your CODEC device, use dai->codec instead. As rule of thumb you should never have to look at the snd_soc_pcm_runtime in a CODEC driver.
[...]
+static void config_fll_clk_12m(struct snd_soc_codec *codec) +{
- snd_soc_update_bits(codec, NAU8825_CLK_DIVIDER, 0x000F, 0x0003);
- snd_soc_update_bits(codec, NAU8825_FL_1, 0x007F, 0x0001);
- snd_soc_write(codec, NAU8825_FL_2, 0xC49B);
- snd_soc_update_bits(codec, NAU8825_FL_3, 0x03FF, 0x0020);
- snd_soc_update_bits(codec, NAU8825_FL_4, 0x0C00, 0x0800);
- snd_soc_update_bits(codec, NAU8825_FL_5, 0x2000, 0x0000);
- snd_soc_update_bits(codec, NAU8825_FL_6, 0x4000, 0x4000);
So what do these magic values do?
+}
+void set_sys_clk(struct snd_soc_codec *codec, int sys_clk)
static
+{
- pr_debug("%s :: sys_clk=%x\n", __func__, sys_clk);
[...
+static int nau8825_codec_suspend(struct snd_soc_codec *codec) +{
- nau8825_set_bias_level(codec, SND_SOC_BIAS_OFF);
- return 0;
+}
set the suspend_bias_off flag in your codec driver to let the core take care of this.
+static int nau8825_resume(struct snd_soc_codec *codec) +{
- nau8825_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
- return 0;
+}
This is not necessary the core already takes care of it.
[...]
+static int nau8825_codec_probe(struct snd_soc_codec *codec) +{
- int i;
- struct nau8825_priv *nau8825;
- nau8825 = snd_soc_codec_get_drvdata(codec);
- nau8825->codec = codec;
- /*writing initial register values to the codec*/
- for (i = 0; i < ARRAY_SIZE(nau8825_reg); i++)
snd_soc_write(codec, nau8825_reg[i].reg, nau8825_reg[i].def);
If that is really necessary use regmap_sync() and preferably in the i2c probe function. But I'd expect that a soft reset should put the device in the default register configuration?
- return 0;
+}
[...]
+static struct snd_soc_codec_driver soc_codec_driver_nau8825 = {
const
- .probe = nau8825_codec_probe,
- .remove = nau8825_codec_remove,
- .suspend = nau8825_codec_suspend,
- .resume = nau8825_resume,
- .set_bias_level = nau8825_set_bias_level,
- .controls = nau8825_snd_controls,
- .num_controls = ARRAY_SIZE(nau8825_snd_controls),
- .dapm_widgets = nau8825_dapm_widgets,
- .num_dapm_widgets = ARRAY_SIZE(nau8825_dapm_widgets),
- .dapm_routes = nau8825_dapm_routes,
- .num_dapm_routes = ARRAY_SIZE(nau8825_dapm_routes),
+};
+static struct snd_soc_dai_ops nau8825_dai_ops = {
const
- .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",.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,- }
+};
+static int nau8825_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *i2c_id)+{
- struct nau8825_platform_data *pdata = dev_get_platdata(&i2c->dev);
- struct nau8825_priv *nau8825;
- int ret;
- nau8825 = devm_kzalloc(&i2c->dev, sizeof(struct nau8825_priv),
preferred form for this is sizeof(*nau8825)
[...]
+MODULE_DESCRIPTION("ASoC NAU8825 codec driver"); +MODULE_AUTHOR("Nuvoton"); +MODULE_LICENSE("GPL v2");
No need for the extra new lines at the end
diff --git a/sound/soc/codecs/nau8825.h b/sound/soc/codecs/nau8825.h new file mode 100644 index 0000000..b16d4d1 --- /dev/null +++ b/sound/soc/codecs/nau8825.h @@ -0,0 +1,150 @@ +/*
[...]
+struct nau8825_priv {
- struct snd_soc_codec *codec;
- struct nau8825_platform_data pdata;
- struct regmap *regmap;
- struct i2c_client *i2c;
- struct snd_soc_jack *hp_jack;
- struct snd_soc_jack *mic_jack;
- struct delayed_work delayed_work;
- struct workqueue_struct *workqueue;
- struct mutex mutex;
- unsigned int irq;
- bool jd_status;
- bool bp_status;
- int jack_type;
+};
It looks like pretty much all of the fields in the struct are not used by the driver.
+#endif /* _NAU8825_H */