Re: [alsa-devel] [PATCH] ASoC: codecs: Add DA9055 codec driver
On Thu, Sep 13, 2012 at 05:38:36PM +0530, Ashish Chavan wrote: > On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote: > > > + aif_ctrl |= (DA9055_AIF_OE | DA9055_AIF_EN); > > DAPM. > Here the trouble in making it DAPM based is that there is no separate > control for AIF input and output. It is confirmed that AIF_EN is the > master control bit, which enables both output as well as input. The AIF > _OE bit is redundant. If we use the same control bit(i.e AIF_EN) for > both DAPM_AIF_IN and DAPM_AIF_OUT, there will be obvious side effects. > Is there any other way to take care of this? Or should we leave it > outside DAPM? Use a supply widget. -- 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] ASoC: codecs: Add DA9055 codec driver
On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote: > > > + aif_ctrl |= (DA9055_AIF_OE | DA9055_AIF_EN); > > DAPM. Here the trouble in making it DAPM based is that there is no separate control for AIF input and output. It is confirmed that AIF_EN is the master control bit, which enables both output as well as input. The AIF _OE bit is redundant. If we use the same control bit(i.e AIF_EN) for both DAPM_AIF_IN and DAPM_AIF_OUT, there will be obvious side effects. Is there any other way to take care of this? Or should we leave it outside DAPM? -- 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] ASoC: codecs: Add DA9055 codec driver
FYI. -Original Message- From: Mark Brown [mailto:broo...@opensource.wolfsonmicro.com] Sent: Wednesday, September 12, 2012 6:20 PM To: Ashish P. Chavan Cc: lrg; alsa-devel; linux-kernel; David Dajun Chen Subject: Re: [alsa-devel] [PATCH] ASoC: codecs: Add DA9055 codec driver On Wed, Sep 12, 2012 at 06:26:18PM +0530, Ashish Chavan wrote: > On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote: > > Why is any of this being exposed to userspace? If this should be > > configured I'd expect it to be static platform data, not something > > that gets changed at runtime. > These parameters are exposed considering the fact that DMIC itself is > not part of the codec. Codec only provides DMIC interface using which > an external DMIC can be attached. These parameters depend on the > actual DMIC hardware and hence kept configurable to allow runtime plug > in of any DMIC hardware. Doesn't it make sense to keep them runtime > configurable? No. The only realistic way to attach a new DMIC to a board is to solder it down, that's not something people are going to do while the system is actve. It's something that's fixed at PCB design time. > > > + /* In slave mode, there is only one set of divisors */ > > > + if (!da9055->master) > > > + fout = 2822400; > > Should check the user supplied this value > Can you explain which value / user supplied value you are referring to? > It is not quite clear to me. The specified output frequency, you just totally ignore it. > For other things like input mixers, Headphone and Lineouts, DAPM is > already used to control power specific bits. The confusion is because > there are two separate control bits for these blocks. One bit is for > "Enabling" that block and other is for "Enabling Amplifier" of that > block. > e.g for headphone, one bit is for "output enable" while other is for > "output amplifier enable". So document this. -- 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] ASoC: codecs: Add DA9055 codec driver
On Wed, Sep 12, 2012 at 06:26:18PM +0530, Ashish Chavan wrote: > On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote: > > Why is any of this being exposed to userspace? If this should be > > configured I'd expect it to be static platform data, not something that > > gets changed at runtime. > These parameters are exposed considering the fact that DMIC itself is > not part of the codec. Codec only provides DMIC interface using which an > external DMIC can be attached. These parameters depend on the actual > DMIC hardware and hence kept configurable to allow runtime plug in of > any DMIC hardware. Doesn't it make sense to keep them runtime > configurable? No. The only realistic way to attach a new DMIC to a board is to solder it down, that's not something people are going to do while the system is actve. It's something that's fixed at PCB design time. > > > + /* In slave mode, there is only one set of divisors */ > > > + if (!da9055->master) > > > + fout = 2822400; > > Should check the user supplied this value > Can you explain which value / user supplied value you are referring to? > It is not quite clear to me. The specified output frequency, you just totally ignore it. > For other things like input mixers, Headphone and Lineouts, DAPM is > already used to control power specific bits. The confusion is because > there are two separate control bits for these blocks. One bit is for > "Enabling" that block and other is for "Enabling Amplifier" of that > block. > e.g for headphone, one bit is for "output enable" while other is for > "output amplifier enable". So document this. -- 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] ASoC: codecs: Add DA9055 codec driver
On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote:
> > +/* Digital MIC clock rate select */
> > +static const char * const da9055_dmic_clk_rate_txt[] = {
> > + "3MHz", "1.5MHz"
> > +};
> > +
> > +static const struct soc_enum da9055_dmic_clk_rate =
> > + SOC_ENUM_SINGLE(DA9055_MIC_CONFIG, 2, 2, da9055_dmic_clk_rate_txt);
> > +
> > +/* Digital MIC sample phase select */
> > +static const char * const da9055_dmic_phase_txt[] = {
> > + "Sample on DMICCLK edges", "Sample between DMICCLK edges"
> > +};
> > +
> > +static const struct soc_enum da9055_dmic_phase =
> > + SOC_ENUM_SINGLE(DA9055_MIC_CONFIG, 1, 2, da9055_dmic_phase_txt);
> > +
> > +/* Digital MIC channel select */
> > +static const char * const da9055_dmic_channel_select_txt[] = {
> > + "Rising Left Falling Right", "Falling Left Rising Right"
> > +};
>
> Why is any of this being exposed to userspace? If this should be
> configured I'd expect it to be static platform data, not something that
> gets changed at runtime.
>
These parameters are exposed considering the fact that DMIC itself is
not part of the codec. Codec only provides DMIC interface using which an
external DMIC can be attached. These parameters depend on the actual
DMIC hardware and hence kept configurable to allow runtime plug in of
any DMIC hardware. Doesn't it make sense to keep them runtime
configurable?
>
> > + /* In slave mode, there is only one set of divisors */
> > + if (!da9055->master)
> > + fout = 2822400;
>
> Should check the user supplied this value
Can you explain which value / user supplied value you are referring to?
It is not quite clear to me.
> - also, what happens if the
> user sets the device to slave mode after setting up the PLL?
Will add required checks in set_fmt().
> > + /* Enable Mic Bias */
> > + snd_soc_write(codec, DA9055_MIC_BIAS_CTRL, DA9055_MIC_BIAS_EN);
>
> DAPM for this and most of the rest of this funciton.
I guess here a DAPM_SUPPLY widget should be used and its routing should
be part of machine driver instead of this codec driver, right?
For other things like input mixers, Headphone and Lineouts, DAPM is
already used to control power specific bits. The confusion is because
there are two separate control bits for these blocks. One bit is for
"Enabling" that block and other is for "Enabling Amplifier" of that
block.
e.g for headphone, one bit is for "output enable" while other is for
"output amplifier enable".
Chip designers have confirmed that "Amplifier Enable" bits are ones
which should be used for power management. This is the reason why
"Enable" bits are being set in probe() and "Amplifier Enable" bits are
being taken care by DAPM. May be I need to put enough comments in code
to clarify this.
--
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] ASoC: codecs: Add DA9055 codec driver
On Tue, Sep 11, 2012 at 08:33:43PM +0530, Ashish Chavan wrote:
> +/* LDO voltage level value */
> +static const char * const da9055_ldo_lvl_select_txt[] = {
> + "1.05V", "1.1V", "1.2V", "1.4V"
> +};
> +static const struct soc_enum da9055_ldo_lvl_select =
> + SOC_ENUM_SINGLE(DA9055_LDO_CTRL, 4, 4, da9055_ldo_lvl_select_txt);
There's a regulator API, if configuration is required we should be using
that.
> +/* Digital MIC clock rate select */
> +static const char * const da9055_dmic_clk_rate_txt[] = {
> + "3MHz", "1.5MHz"
> +};
> +
> +static const struct soc_enum da9055_dmic_clk_rate =
> + SOC_ENUM_SINGLE(DA9055_MIC_CONFIG, 2, 2, da9055_dmic_clk_rate_txt);
> +
> +/* Digital MIC sample phase select */
> +static const char * const da9055_dmic_phase_txt[] = {
> + "Sample on DMICCLK edges", "Sample between DMICCLK edges"
> +};
> +
> +static const struct soc_enum da9055_dmic_phase =
> + SOC_ENUM_SINGLE(DA9055_MIC_CONFIG, 1, 2, da9055_dmic_phase_txt);
> +
> +/* Digital MIC channel select */
> +static const char * const da9055_dmic_channel_select_txt[] = {
> + "Rising Left Falling Right", "Falling Left Rising Right"
> +};
Why is any of this being exposed to userspace? If this should be
configured I'd expect it to be static platform data, not something that
gets changed at runtime.
> +/* MIC bias voltage level select */
> +static const char * const da9055_mic_bias_level_txt[] = {
> + "1.6V", "1.8V", "2.1V", "2.2V"
> +};
Again, why is this being exposed to userspace/ I'm fairly sure we went
through similar stuff with your last driver...
> + SOC_DOUBLE_R_TLV("HeadPhone Volume",
> + DA9055_HP_L_GAIN, DA9055_HP_R_GAIN,
> + 0, 0x3f, 0, hp_vol_tlv),
Headphone.
> + /* Mute controls */
> + SOC_DOUBLE_R("Mic Mute Switch", DA9055_MIC_L_CTRL,
> + DA9055_MIC_R_CTRL, 6, 1, 0),
> + SOC_DOUBLE_R("Aux Mute Switch", DA9055_AUX_L_CTRL,
> + DA9055_AUX_R_CTRL, 6, 1, 0),
> + SOC_DOUBLE_R("Mixin PGA Mute Switch", DA9055_MIXIN_L_CTRL,
> + DA9055_MIXIN_R_CTRL, 6, 1, 0),
> + SOC_DOUBLE_R("ADC Mute Switch", DA9055_ADC_L_CTRL,
> + DA9055_ADC_R_CTRL, 6, 1, 0),
> + SOC_DOUBLE_R("HeadPhone Mute Switch", DA9055_HP_L_CTRL,
> + DA9055_HP_R_CTRL, 6, 1, 0),
> + SOC_SINGLE("Lineout Mute Switch", DA9055_LINE_CTRL, 6, 1, 0),
> + SOC_SINGLE("DAC Soft Mute Switch", DA9055_DAC_FILTERS5, 7, 1, 0),
No "Mute". Again, I'm fairly sure we had the same issue last time.
> + /* LDO control */
> + SOC_SINGLE("LDO Enable", DA9055_LDO_CTRL, 7, 1, 0),
> + SOC_ENUM("LDO Level Select", da9055_ldo_lvl_select),
The LDO enable should absolutely not be being exposed to userspace!
> + /* DMIC controls */
> + SOC_DOUBLE_R("DMIC Enable", DA9055_MIXIN_L_SELECT,
> + DA9055_MIXIN_R_SELECT, 7, 1, 0),
Switch if this is a mute.
> + /* MIC PGA input selection controls */
> + SOC_ENUM("Mic Left Input Select", da9055_mic_l_select),
> + SOC_ENUM("Mic Right Input Select", da9055_mic_r_select),
DAPM.
> +
> + /* ALC Controls */
> + SOC_DOUBLE_EXT("ALC Enable", DA9055_ALC_CTRL1, 3, 7, 1, 0,
> +snd_soc_get_volsw, da9055_put_alc_sw),
ALC Switch. All enable controls should be switches.
> +static int da9055_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_codec *codec = dai->codec;
> + struct da9055_priv *da9055 = snd_soc_codec_get_drvdata(codec);
> + u8 aif_ctrl, fs;
> + u32 sysclk;
> +
> + /* Set AIF source to Left and Right ADC */
> + snd_soc_write(codec, DA9055_DIG_ROUTING_AIF,
> + DA9055_AIF_L_SRC | DA9055_AIF_R_SRC);
This should be in DAPM.
> + aif_ctrl = snd_soc_read(codec, DA9055_AIF_CTRL) & 0xf3;
Use snd_soc_update_bits() later on.
> + aif_ctrl |= (DA9055_AIF_OE | DA9055_AIF_EN);
DAPM.
> + /* In slave mode, there is only one set of divisors */
> + if (!da9055->master)
> + fout = 2822400;
Should check the user supplied this value - also, what happens if the
user sets the device to slave mode after setting up the PLL?
> + /* Enable VMID reference & master bias */
> + snd_soc_write(codec, DA9055_REFERENCES,
> + DA9055_VMID_EN | DA9055_BIAS_EN);
set_bias_level()
> + /* Enable Mic Bias */
> + snd_soc_write(codec, DA9055_MIC_BIAS_CTRL, DA9055_MIC_BIAS_EN);
DAPM for this and most of the rest of this funciton.
> + da9055->regmap = regmap_init_i2c(i2c, &da9055_regmap_config);
devm_regmap_init_i2c()
--
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/
[alsa-devel] [PATCH] ASoC: codecs: Add DA9055 codec driver
This patch adds support for Dialog semiconductor's DA9055 audio codec. This has been tested on DA9055 EVB with Samsung SMDK6410 board. Signed-off-by: Ashish Chavan Signed-off-by: David Dajun Chen --- sound/soc/codecs/Kconfig |4 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/da9055.c | 1507 + 3 files changed, 1513 insertions(+), 0 deletions(-) diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 3684255..b92759a 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -37,6 +37,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_CX20442 select SND_SOC_DA7210 if I2C select SND_SOC_DA732X if I2C + select SND_SOC_DA9055 if I2C select SND_SOC_DFBMCS320 select SND_SOC_ISABELLE if I2C select SND_SOC_JZ4740_CODEC @@ -239,6 +240,9 @@ config SND_SOC_DA7210 config SND_SOC_DA732X tristate +config SND_SOC_DA9055 + tristate + config SND_SOC_DFBMCS320 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index ca508b2..9bd4d95 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -24,6 +24,7 @@ snd-soc-cs4271-objs := cs4271.o snd-soc-cx20442-objs := cx20442.o snd-soc-da7210-objs := da7210.o snd-soc-da732x-objs := da732x.o +snd-soc-da9055-objs := da9055.o snd-soc-dfbmcs320-objs := dfbmcs320.o snd-soc-dmic-objs := dmic.o snd-soc-isabelle-objs := isabelle.o @@ -144,6 +145,7 @@ obj-$(CONFIG_SND_SOC_CS4271)+= snd-soc-cs4271.o obj-$(CONFIG_SND_SOC_CX20442) += snd-soc-cx20442.o obj-$(CONFIG_SND_SOC_DA7210) += snd-soc-da7210.o obj-$(CONFIG_SND_SOC_DA732X) += snd-soc-da732x.o +obj-$(CONFIG_SND_SOC_DA9055) += snd-soc-da9055.o obj-$(CONFIG_SND_SOC_DFBMCS320)+= snd-soc-dfbmcs320.o obj-$(CONFIG_SND_SOC_DMIC) += snd-soc-dmic.o obj-$(CONFIG_SND_SOC_ISABELLE) += snd-soc-isabelle.o diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c new file mode 100644 index 000..be5c9e6 --- /dev/null +++ b/sound/soc/codecs/da9055.c @@ -0,0 +1,1507 @@ +/* + * DA9055 ALSA Soc codec driver + * + * Copyright (c) 2012 Dialog Semiconductor + * + * Tested on (Samsung SMDK6410 board + DA9055 EVB) using I2S and I2C + * Written by David Chen and + * Ashish Chavan + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* DA9055 register space */ + +/* Status Registers */ +#define DA9055_STATUS1 0x02 +#define DA9055_PLL_STATUS 0x03 +#define DA9055_AUX_L_GAIN_STATUS 0x04 +#define DA9055_AUX_R_GAIN_STATUS 0x05 +#define DA9055_MIC_L_GAIN_STATUS 0x06 +#define DA9055_MIC_R_GAIN_STATUS 0x07 +#define DA9055_MIXIN_L_GAIN_STATUS 0x08 +#define DA9055_MIXIN_R_GAIN_STATUS 0x09 +#define DA9055_ADC_L_GAIN_STATUS 0x0A +#define DA9055_ADC_R_GAIN_STATUS 0x0B +#define DA9055_DAC_L_GAIN_STATUS 0x0C +#define DA9055_DAC_R_GAIN_STATUS 0x0D +#define DA9055_HP_L_GAIN_STATUS0x0E +#define DA9055_HP_R_GAIN_STATUS0x0F +#define DA9055_LINE_GAIN_STATUS0x10 + +/* System Initialisation Registers */ +#define DA9055_CIF_CTRL0x20 +#define DA9055_DIG_ROUTING_AIF 0X21 +#define DA9055_SR 0x22 +#define DA9055_REFERENCES 0x23 +#define DA9055_PLL_FRAC_TOP0x24 +#define DA9055_PLL_FRAC_BOT0x25 +#define DA9055_PLL_INTEGER 0x26 +#define DA9055_PLL_CTRL0x27 +#define DA9055_AIF_CLK_MODE0x28 +#define DA9055_AIF_CTRL0x29 +#define DA9055_DIG_ROUTING_DAC 0x2A +#define DA9055_ALC_CTRL1 0x2B + +/* Input - Gain, Select and Filter Registers */ +#define DA9055_AUX_L_GAIN 0x30 +#define DA9055_AUX_R_GAIN 0x31 +#define DA9055_MIXIN_L_SELECT 0x32 +#define DA9055_MIXIN_R_SELECT 0x33 +#define DA9055_MIXIN_L_GAIN0x34 +#define DA9055_MIXIN_R_GAIN0x35 +#define DA9055_ADC_L_GAIN 0x36 +#define DA9055_ADC_R_GAIN 0x37 +#define DA9055_ADC_FILTERS10x38 +#define DA9055_MIC_L_GAIN 0x39 +#define DA9055_MIC_R_GAIN 0x3A + +/* Output - Gain, Select and Filter Registers */ +#define DA9055_DAC_FILTERS50x40 +#define DA9055_DAC_FILTERS20x41 +#define DA9055_DAC_FILTERS30x42 +#define DA9055_DAC_FILTERS40x43 +#define DA9055_DAC_FILTERS10x44 +#define DA9055_DAC_L_GAIN 0x45 +#define DA9055_DAC_R_GAIN 0x46 +#define DA9055_CP_CTRL
