[alsa-devel] [PATCH v3 0/6] ASoC: tlv320aic32x4: DT support
Hi,
The series adds basic DT support for the TI tlv320aic32x4 codec. Patch 3 ("ASoC: tlv320aic32x4: Use signed int mixer controls") depends on series "[PATCH 0/2] ASoC: core: volume control using signed register values".
I added the remaining two patches of the "ASoC: tlv320aic32x4 bugfixes" series to the beginning of this series to have all tlv320aic32x4 patches in one series.
Regards,
Markus
Changes in v3: - Rename property gpio-reset to reset-gpios - Use negative constants as minimum volume instead of casting to negative integers
Changes in v2: - Regulator support - Fix documentation for gpio property
Markus Pargmann (6): ASoC: tlv320aic32x4: Fix mono playback ASoC: tlv320aic32x4: Fix MICPGA input configuration ASoC: tlv320aic32x4: Use signed int mixer controls ASoC: tlv320aic32x4: DT support ASoC: tlv320aic32x4: Support for master clock ASoC: tlv320aic32x4: Support for regulators
.../devicetree/bindings/sound/tlv320aic32x4.txt | 28 ++++ sound/soc/codecs/tlv320aic32x4.c | 154 ++++++++++++++++++--- sound/soc/codecs/tlv320aic32x4.h | 3 + 3 files changed, 164 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/tlv320aic32x4.txt
Playback of a mono stream should output the same stream on both channels. At the moment only the left analog signal is valid, the right one is just noise.
This patch maps the left digital channel onto both DACs when receiving a mono stream.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/codecs/tlv320aic32x4.c | 18 +++++++++++------- sound/soc/codecs/tlv320aic32x4.h | 1 + 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 7639601..0a8079b 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -450,6 +450,17 @@ static int aic32x4_hw_params(struct snd_pcm_substream *substream, } snd_soc_write(codec, AIC32X4_IFACE1, data);
+ if (params_channels(params) == 1) { + data = AIC32X4_RDAC2LCHN | AIC32X4_LDAC2LCHN; + } else { + if (aic32x4->swapdacs) + data = AIC32X4_RDAC2LCHN | AIC32X4_LDAC2RCHN; + else + data = AIC32X4_LDAC2LCHN | AIC32X4_RDAC2RCHN; + } + snd_soc_update_bits(codec, AIC32X4_DACSETUP, AIC32X4_DAC_CHAN_MASK, + data); + return 0; }
@@ -606,13 +617,6 @@ static int aic32x4_probe(struct snd_soc_codec *codec) } snd_soc_write(codec, AIC32X4_CMMODE, tmp_reg);
- /* Do DACs need to be swapped? */ - if (aic32x4->swapdacs) { - snd_soc_write(codec, AIC32X4_DACSETUP, AIC32X4_LDAC2RCHN | AIC32X4_RDAC2LCHN); - } else { - snd_soc_write(codec, AIC32X4_DACSETUP, AIC32X4_LDAC2LCHN | AIC32X4_RDAC2RCHN); - } - /* Mic PGA routing */ if (aic32x4->micpga_routing & AIC32X4_MICPGA_ROUTE_LMIC_IN2R_10K) { snd_soc_write(codec, AIC32X4_LMICPGANIN, AIC32X4_LMICPGANIN_IN2R_10K); diff --git a/sound/soc/codecs/tlv320aic32x4.h b/sound/soc/codecs/tlv320aic32x4.h index 3577422..83795af 100644 --- a/sound/soc/codecs/tlv320aic32x4.h +++ b/sound/soc/codecs/tlv320aic32x4.h @@ -138,6 +138,7 @@ #define AIC32X4_LDAC2RCHN (0x02 << 4) #define AIC32X4_LDAC2LCHN (0x01 << 4) #define AIC32X4_RDAC2RCHN (0x01 << 2) +#define AIC32X4_DAC_CHAN_MASK 0x3c
#define AIC32X4_SSTEP2WCLK 0x01 #define AIC32X4_MUTEON 0x0C
On Mon, Jan 27, 2014 at 01:03:05PM +0100, Markus Pargmann wrote:
Playback of a mono stream should output the same stream on both channels. At the moment only the left analog signal is valid, the right one is just noise.
Applied, thanks.
Currently the Negative Terminal Input Routing Configuration is only set when there is a special routing configuration. If we don't use one of the inputs IN1 or IN2 as negative terminal input, the PGA and recording does not work.
This patch adds a route from CM1L/CM1R to the PGA as negative input by default. With this configuration the PGA can amplify all input signals and line-in/mic works again.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/codecs/tlv320aic32x4.c | 10 ++++++---- sound/soc/codecs/tlv320aic32x4.h | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 0a8079b..36a9cb9 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -618,12 +618,14 @@ static int aic32x4_probe(struct snd_soc_codec *codec) snd_soc_write(codec, AIC32X4_CMMODE, tmp_reg);
/* Mic PGA routing */ - if (aic32x4->micpga_routing & AIC32X4_MICPGA_ROUTE_LMIC_IN2R_10K) { + if (aic32x4->micpga_routing & AIC32X4_MICPGA_ROUTE_LMIC_IN2R_10K) snd_soc_write(codec, AIC32X4_LMICPGANIN, AIC32X4_LMICPGANIN_IN2R_10K); - } - if (aic32x4->micpga_routing & AIC32X4_MICPGA_ROUTE_RMIC_IN1L_10K) { + else + snd_soc_write(codec, AIC32X4_LMICPGANIN, AIC32X4_LMICPGANIN_CM1L_10K); + if (aic32x4->micpga_routing & AIC32X4_MICPGA_ROUTE_RMIC_IN1L_10K) snd_soc_write(codec, AIC32X4_RMICPGANIN, AIC32X4_RMICPGANIN_IN1L_10K); - } + else + snd_soc_write(codec, AIC32X4_RMICPGANIN, AIC32X4_RMICPGANIN_CM1R_10K);
aic32x4_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
diff --git a/sound/soc/codecs/tlv320aic32x4.h b/sound/soc/codecs/tlv320aic32x4.h index 83795af..995f033 100644 --- a/sound/soc/codecs/tlv320aic32x4.h +++ b/sound/soc/codecs/tlv320aic32x4.h @@ -120,7 +120,9 @@ #define AIC32X4_MICBIAS_2075V 0x60
#define AIC32X4_LMICPGANIN_IN2R_10K 0x10 +#define AIC32X4_LMICPGANIN_CM1L_10K 0x40 #define AIC32X4_RMICPGANIN_IN1L_10K 0x10 +#define AIC32X4_RMICPGANIN_CM1R_10K 0x40
#define AIC32X4_LMICPGAVOL_NOGAIN 0x80 #define AIC32X4_RMICPGAVOL_NOGAIN 0x80
On Mon, Jan 27, 2014 at 01:03:06PM +0100, Markus Pargmann wrote:
Currently the Negative Terminal Input Routing Configuration is only set when there is a special routing configuration. If we don't use one of the inputs IN1 or IN2 as negative terminal input, the PGA and recording does not work.
Applied, thanks.
There are a number of mixer controls that support negative values. They use signed values for this with different number of bits for the values. Currently they only support the positive range.
This patch replaces the unsigned mixers with signed mixers to support the full range.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/codecs/tlv320aic32x4.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 36a9cb9..c541213 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -68,18 +68,24 @@ struct aic32x4_priv { int rstn_gpio; };
-/* 0dB min, 1dB steps */ -static DECLARE_TLV_DB_SCALE(tlv_step_1, 0, 100, 0); /* 0dB min, 0.5dB steps */ static DECLARE_TLV_DB_SCALE(tlv_step_0_5, 0, 50, 0); +/* -63.5dB min, 0.5dB steps */ +static DECLARE_TLV_DB_SCALE(tlv_pcm, -6350, 50, 0); +/* -6dB min, 1dB steps */ +static DECLARE_TLV_DB_SCALE(tlv_driver_gain, -600, 100, 0); +/* -12dB min, 0.5dB steps */ +static DECLARE_TLV_DB_SCALE(tlv_adc_vol, -1200, 50, 0);
static const struct snd_kcontrol_new aic32x4_snd_controls[] = { - SOC_DOUBLE_R_TLV("PCM Playback Volume", AIC32X4_LDACVOL, - AIC32X4_RDACVOL, 0, 0x30, 0, tlv_step_0_5), - SOC_DOUBLE_R_TLV("HP Driver Gain Volume", AIC32X4_HPLGAIN, - AIC32X4_HPRGAIN, 0, 0x1D, 0, tlv_step_1), - SOC_DOUBLE_R_TLV("LO Driver Gain Volume", AIC32X4_LOLGAIN, - AIC32X4_LORGAIN, 0, 0x1D, 0, tlv_step_1), + SOC_DOUBLE_R_S_TLV("PCM Playback Volume", AIC32X4_LDACVOL, + AIC32X4_RDACVOL, 0, -0x7f, 0x30, 7, 0, tlv_pcm), + SOC_DOUBLE_R_S_TLV("HP Driver Gain Volume", AIC32X4_HPLGAIN, + AIC32X4_HPRGAIN, 0, -0x6, 0x1d, 5, 0, + tlv_driver_gain), + SOC_DOUBLE_R_S_TLV("LO Driver Gain Volume", AIC32X4_LOLGAIN, + AIC32X4_LORGAIN, 0, -0x6, 0x1d, 5, 0, + tlv_driver_gain), SOC_DOUBLE_R("HP DAC Playback Switch", AIC32X4_HPLGAIN, AIC32X4_HPRGAIN, 6, 0x01, 1), SOC_DOUBLE_R("LO DAC Playback Switch", AIC32X4_LOLGAIN, @@ -90,8 +96,8 @@ static const struct snd_kcontrol_new aic32x4_snd_controls[] = { SOC_SINGLE("ADCFGA Left Mute Switch", AIC32X4_ADCFGA, 7, 1, 0), SOC_SINGLE("ADCFGA Right Mute Switch", AIC32X4_ADCFGA, 3, 1, 0),
- SOC_DOUBLE_R_TLV("ADC Level Volume", AIC32X4_LADCVOL, - AIC32X4_RADCVOL, 0, 0x28, 0, tlv_step_0_5), + SOC_DOUBLE_R_S_TLV("ADC Level Volume", AIC32X4_LADCVOL, + AIC32X4_RADCVOL, 0, -0x18, 0x28, 6, 0, tlv_adc_vol), SOC_DOUBLE_R_TLV("PGA Level Volume", AIC32X4_LMICPGAVOL, AIC32X4_RMICPGAVOL, 0, 0x5f, 0, tlv_step_0_5),
On Mon, Jan 27, 2014 at 01:03:07PM +0100, Markus Pargmann wrote:
There are a number of mixer controls that support negative values. They use signed values for this with different number of bits for the values. Currently they only support the positive range.
Applied, thanks.
Add DT support for this codec. The bindings differ a bit from the aic3x codec bindings, so I created a new binding documentation.
Cc: Rob Herring robh+dt@kernel.org Cc: Pawel Moll pawel.moll@arm.com Cc: Mark Rutland mark.rutland@arm.com Cc: Ian Campbell ijc+devicetree@hellion.org.uk Cc: Kumar Gala galak@codeaurora.org Cc: devicetree@vger.kernel.org Signed-off-by: Markus Pargmann mpa@pengutronix.de --- .../devicetree/bindings/sound/tlv320aic32x4.txt | 18 ++++++++++++++++ sound/soc/codecs/tlv320aic32x4.c | 25 ++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tlv320aic32x4.txt
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt new file mode 100644 index 0000000..db05510 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt @@ -0,0 +1,18 @@ +Texas Instruments - tlv320aic32x4 Codec module + +The tlv320aic32x4 serial control bus communicates through I2C protocols + +Required properties: + - compatible: Should be "ti,tlv320aic32x4" + - reg: I2C slave address + +Optional properties: + - reset-gpios: Reset-GPIO phandle with args as described in gpio/gpio.txt + + +Example: + +codec: tlv320aic32x4@18 { + compatible = "ti,tlv320aic32x4"; + reg = <0x18>; +}; diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index c541213..1dd50e4 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -29,6 +29,7 @@ #include <linux/delay.h> #include <linux/pm.h> #include <linux/gpio.h> +#include <linux/of_gpio.h> #include <linux/i2c.h> #include <linux/cdev.h> #include <linux/slab.h> @@ -669,11 +670,22 @@ static struct snd_soc_codec_driver soc_codec_dev_aic32x4 = { .num_dapm_routes = ARRAY_SIZE(aic32x4_dapm_routes), };
+static int aic32x4_parse_dt(struct aic32x4_priv *aic32x4, + struct device_node *np) +{ + aic32x4->swapdacs = false; + aic32x4->micpga_routing = 0; + aic32x4->rstn_gpio = of_get_named_gpio(np, "reset-gpios", 0); + + return 0; +} + static int aic32x4_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct aic32x4_pdata *pdata = i2c->dev.platform_data; struct aic32x4_priv *aic32x4; + struct device_node *np = i2c->dev.of_node; int ret;
aic32x4 = devm_kzalloc(&i2c->dev, sizeof(struct aic32x4_priv), @@ -692,6 +704,12 @@ static int aic32x4_i2c_probe(struct i2c_client *i2c, aic32x4->swapdacs = pdata->swapdacs; aic32x4->micpga_routing = pdata->micpga_routing; aic32x4->rstn_gpio = pdata->rstn_gpio; + } else if (np) { + ret = aic32x4_parse_dt(aic32x4, np); + if (ret) { + dev_err(&i2c->dev, "Failed to parse DT node\n"); + return ret; + } } else { aic32x4->power_cfg = 0; aic32x4->swapdacs = false; @@ -723,10 +741,17 @@ static const struct i2c_device_id aic32x4_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, aic32x4_i2c_id);
+static const struct of_device_id aic32x4_of_id[] = { + { .compatible = "ti,tlv320aic32x4", }, + { /* senitel */ } +}; +MODULE_DEVICE_TABLE(of, aic32x4_of_id); + static struct i2c_driver aic32x4_i2c_driver = { .driver = { .name = "tlv320aic32x4", .owner = THIS_MODULE, + .of_match_table = aic32x4_of_id, }, .probe = aic32x4_i2c_probe, .remove = aic32x4_i2c_remove,
On Mon, Jan 27, 2014 at 01:03:08PM +0100, Markus Pargmann wrote:
Add DT support for this codec. The bindings differ a bit from the aic3x codec bindings, so I created a new binding documentation.
I've applied this but please send a followup removing it from the tlv320aic3x bindings.
Add support for a master clock passed through DT. The master clock of the codec is only active when the codec is in use.
Cc: Rob Herring robh+dt@kernel.org Cc: Pawel Moll pawel.moll@arm.com Cc: Mark Rutland mark.rutland@arm.com Cc: Ian Campbell ijc+devicetree@hellion.org.uk Cc: Kumar Gala galak@codeaurora.org Cc: devicetree@vger.kernel.org Signed-off-by: Markus Pargmann mpa@pengutronix.de --- .../devicetree/bindings/sound/tlv320aic32x4.txt | 4 ++++ sound/soc/codecs/tlv320aic32x4.c | 23 ++++++++++++++++++++++ 2 files changed, 27 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt index db05510..352be7b 100644 --- a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt +++ b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt @@ -8,6 +8,8 @@ Required properties:
Optional properties: - reset-gpios: Reset-GPIO phandle with args as described in gpio/gpio.txt + - clocks/clock-names: Clock named 'mclk' for the master clock of the codec. + See clock/clock-bindings.txt for information about the detailed format.
Example: @@ -15,4 +17,6 @@ Example: codec: tlv320aic32x4@18 { compatible = "ti,tlv320aic32x4"; reg = <0x18>; + clocks = <&clks 201>; + clock-names = "mclk"; }; diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 1dd50e4..4f7830d 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -33,6 +33,7 @@ #include <linux/i2c.h> #include <linux/cdev.h> #include <linux/slab.h> +#include <linux/clk.h>
#include <sound/tlv320aic32x4.h> #include <sound/core.h> @@ -67,6 +68,7 @@ struct aic32x4_priv { u32 micpga_routing; bool swapdacs; int rstn_gpio; + struct clk *mclk; };
/* 0dB min, 0.5dB steps */ @@ -487,8 +489,21 @@ static int aic32x4_mute(struct snd_soc_dai *dai, int mute) static int aic32x4_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { + struct aic32x4_priv *aic32x4 = snd_soc_codec_get_drvdata(codec); + switch (level) { case SND_SOC_BIAS_ON: + /* Switch on master clock */ + if (!IS_ERR(aic32x4->mclk)) { + int ret; + + ret = clk_prepare_enable(aic32x4->mclk); + if (ret) { + dev_err(codec->dev, "Failed to enable master clock\n"); + return ret; + } + } + /* Switch on PLL */ snd_soc_update_bits(codec, AIC32X4_PLLPR, AIC32X4_PLLEN, AIC32X4_PLLEN); @@ -516,6 +531,10 @@ static int aic32x4_set_bias_level(struct snd_soc_codec *codec, case SND_SOC_BIAS_PREPARE: break; case SND_SOC_BIAS_STANDBY: + /* Switch off master clock */ + if (!IS_ERR(aic32x4->mclk)) + clk_disable_unprepare(aic32x4->mclk); + /* Switch off PLL */ snd_soc_update_bits(codec, AIC32X4_PLLPR, AIC32X4_PLLEN, 0); @@ -717,6 +736,10 @@ static int aic32x4_i2c_probe(struct i2c_client *i2c, aic32x4->rstn_gpio = -1; }
+ aic32x4->mclk = devm_clk_get(&i2c->dev, "mclk"); + if (IS_ERR(aic32x4->mclk)) + dev_info(&i2c->dev, "No mclk found, continuing without clock\n"); + if (gpio_is_valid(aic32x4->rstn_gpio)) { ret = devm_gpio_request_one(&i2c->dev, aic32x4->rstn_gpio, GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn");
On Mon, Jan 27, 2014 at 01:03:09PM +0100, Markus Pargmann wrote:
case SND_SOC_BIAS_STANDBY:
/* Switch off master clock */if (!IS_ERR(aic32x4->mclk))clk_disable_unprepare(aic32x4->mclk);- /* Switch off PLL */ snd_soc_update_bits(codec, AIC32X4_PLLPR, AIC32X4_PLLEN, 0);
This looks like it's disabling the MCLK before disabling the PLL - if the two are being disabled together I would expect to see the other way around.
Otherwise this looks good to me.
On Mon, Jan 27, 2014 at 06:17:07PM +0000, Mark Brown wrote:
On Mon, Jan 27, 2014 at 01:03:09PM +0100, Markus Pargmann wrote:
case SND_SOC_BIAS_STANDBY:
/* Switch off master clock */if (!IS_ERR(aic32x4->mclk))clk_disable_unprepare(aic32x4->mclk);- /* Switch off PLL */ snd_soc_update_bits(codec, AIC32X4_PLLPR, AIC32X4_PLLEN, 0);
This looks like it's disabling the MCLK before disabling the PLL - if the two are being disabled together I would expect to see the other way around.
Yes right, should be the other way round, although it doesn't make a difference. I will change it for v4.
Thanks,
Markus
Support regulators to power up the codec. This patch also enables the AVDD LDO if no AV regulator was found.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- .../devicetree/bindings/sound/tlv320aic32x4.txt | 6 +++ sound/soc/codecs/tlv320aic32x4.c | 52 ++++++++++++++++++++++ 2 files changed, 58 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt index 352be7b..d09b065 100644 --- a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt +++ b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt @@ -10,6 +10,12 @@ Optional properties: - reset-gpios: Reset-GPIO phandle with args as described in gpio/gpio.txt - clocks/clock-names: Clock named 'mclk' for the master clock of the codec. See clock/clock-bindings.txt for information about the detailed format. + - supply-*: Optional supply regulators for the codec. Possible regulators are + "ldoin" - LDO power supply + "iov" - digital IO power supply + "dv" - Digital core power supply + "av" - Analog core power supply + See regulator/regulator.txt for more information.
Example: diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 4f7830d..49b5293 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -34,6 +34,7 @@ #include <linux/cdev.h> #include <linux/slab.h> #include <linux/clk.h> +#include <linux/regulator/consumer.h>
#include <sound/tlv320aic32x4.h> #include <sound/core.h> @@ -69,6 +70,11 @@ struct aic32x4_priv { bool swapdacs; int rstn_gpio; struct clk *mclk; + + struct regulator *supply_ldo; + struct regulator *supply_iov; + struct regulator *supply_dv; + struct regulator *supply_av; };
/* 0dB min, 0.5dB steps */ @@ -699,6 +705,46 @@ static int aic32x4_parse_dt(struct aic32x4_priv *aic32x4, return 0; }
+static int aic32x4_i2c_setup_regulators(struct device *dev, + struct aic32x4_priv *aic32x4) +{ + int ret; + + aic32x4->supply_ldo = devm_regulator_get_optional(dev, "ldoin"); + aic32x4->supply_iov = devm_regulator_get_optional(dev, "iov"); + aic32x4->supply_dv = devm_regulator_get_optional(dev, "dv"); + aic32x4->supply_av = devm_regulator_get_optional(dev, "av"); + + if (!IS_ERR(aic32x4->supply_ldo)) { + ret = regulator_enable(aic32x4->supply_ldo); + if (ret) + return ret; + } + + if (!IS_ERR(aic32x4->supply_iov)) { + ret = regulator_enable(aic32x4->supply_iov); + if (ret) + return ret; + } + + if (!IS_ERR(aic32x4->supply_dv)) { + ret = regulator_enable(aic32x4->supply_dv); + if (ret) + return ret; + } + + if (!IS_ERR(aic32x4->supply_av)) { + ret = regulator_enable(aic32x4->supply_av); + if (ret) + return ret; + } + + if (!IS_ERR(aic32x4->supply_ldo) && IS_ERR(aic32x4->supply_av)) + aic32x4->power_cfg |= AIC32X4_PWR_AIC32X4_LDO_ENABLE; + + return 0; +} + static int aic32x4_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { @@ -736,6 +782,12 @@ static int aic32x4_i2c_probe(struct i2c_client *i2c, aic32x4->rstn_gpio = -1; }
+ ret = aic32x4_i2c_setup_regulators(&i2c->dev, aic32x4); + if (ret) { + dev_err(&i2c->dev, "Failed to setup regulators\n"); + return ret; + } + aic32x4->mclk = devm_clk_get(&i2c->dev, "mclk"); if (IS_ERR(aic32x4->mclk)) dev_info(&i2c->dev, "No mclk found, continuing without clock\n");
On Mon, Jan 27, 2014 at 10:03 AM, Markus Pargmann mpa@pengutronix.de wrote:
+static int aic32x4_i2c_setup_regulators(struct device *dev,
struct aic32x4_priv *aic32x4)+{
int ret;aic32x4->supply_ldo = devm_regulator_get_optional(dev, "ldoin");aic32x4->supply_iov = devm_regulator_get_optional(dev, "iov");aic32x4->supply_dv = devm_regulator_get_optional(dev, "dv");aic32x4->supply_av = devm_regulator_get_optional(dev, "av");if (!IS_ERR(aic32x4->supply_ldo)) {ret = regulator_enable(aic32x4->supply_ldo);if (ret)return ret;}if (!IS_ERR(aic32x4->supply_iov)) {ret = regulator_enable(aic32x4->supply_iov);if (ret)return ret;}if (!IS_ERR(aic32x4->supply_dv)) {ret = regulator_enable(aic32x4->supply_dv);if (ret)return ret;}if (!IS_ERR(aic32x4->supply_av)) {ret = regulator_enable(aic32x4->supply_av);if (ret)return ret;
Using regulator_bulk_enable() would simplify the code.
Regards,
Fabio Estevam
On Mon, Jan 27, 2014 at 01:03:10PM +0100, Markus Pargmann wrote:
- aic32x4->supply_ldo = devm_regulator_get_optional(dev, "ldoin");
- aic32x4->supply_iov = devm_regulator_get_optional(dev, "iov");
- aic32x4->supply_dv = devm_regulator_get_optional(dev, "dv");
- aic32x4->supply_av = devm_regulator_get_optional(dev, "av");
These shouldn't be optional, it seems unlikely that the device is going to work without power. This will also be broken for probe deferral since the code will just ignore errors so if the error is -EPROBE_DEFER it'll not actually defer.
Just use the normal bulk functions, as Fabio said this will be simpler too, and there should be code to disable on device removal at least (ideally also on suspend and resume, and while the device is idle if it doesn't have slow startup times).
Hi,
On Mon, Jan 27, 2014 at 03:15:33PM +0000, Mark Brown wrote:
On Mon, Jan 27, 2014 at 01:03:10PM +0100, Markus Pargmann wrote:
- aic32x4->supply_ldo = devm_regulator_get_optional(dev, "ldoin");
- aic32x4->supply_iov = devm_regulator_get_optional(dev, "iov");
- aic32x4->supply_dv = devm_regulator_get_optional(dev, "dv");
- aic32x4->supply_av = devm_regulator_get_optional(dev, "av");
These shouldn't be optional, it seems unlikely that the device is going to work without power. This will also be broken for probe deferral since the code will just ignore errors so if the error is -EPROBE_DEFER it'll not actually defer.
The manual describes the possible power supplies as the following:
dv has to be present if ldoin is not supplied or LDO_SELECT hardware pin is low. av has to be present if ldoin is not supplied. iov is always necessary. ldoin is necessary if both dv and av are not supplied.
I will try to catch all those conditions in the next version and fix the EPROBE_DEFER.
Just use the normal bulk functions, as Fabio said this will be simpler too, and there should be code to disable on device removal at least (ideally also on suspend and resume, and while the device is idle if it doesn't have slow startup times).
Do the normal bulk functions handle optional regulators?
Thanks,
Markus
On Mon, Jan 27, 2014 at 05:10:54PM +0100, Markus Pargmann wrote:
Do the normal bulk functions handle optional regulators?
No, the general idea is that these will need some individual code to decide what to do if they are not present.
participants (3)
-
Fabio Estevam -
Mark Brown -
Markus Pargmann