[alsa-devel] [PATCH v2 0/3] ASoc: TAS6424: Add support for mute, standby, and faster power on

newer
[alsa-devel] [PATCH] ALSA: dice:...

older
[alsa-devel] [PATCH] ASoC: Intel:...

Jean-Jacques Hiblot

27 Apr 2018 27 Apr '18

4:55 p.m.

mute and standby pins are available on the codec. If they are connected, they should be managed by the driver, instead of relying on gpio hogs or on the initial state of the GPIOs.

This series also includes a patch to improve the start-up time of the channels by disabling built-in DC diagnostics. Those diagnosdtics basically serve to detect : - wires shorted together - wire shorted to ground or vbat - wire disconnected This is not useful for all platforms and the addition to the startup time is quite noticeable (230ms). The diagnostics can be enabled/disabled using a dedicated ALSA control.

Changes in v2: - use a ALSA control to enable/disable the auto diagnostics instead of relying on a DTS property. - fixed typos - use gpios (plural) instead of gpio in the documentation of the bindings

Jean-Jacques Hiblot (3): ASoC: tas6424: Add support for the standby pin ASoC: tas6424: Add support for the mute pin ASoC: tas6424: Allow disabling auto diagnostics for faster power-on

.../devicetree/bindings/sound/ti,tas6424.txt | 2 + sound/soc/codecs/tas6424.c | 78 +++++++++++++++++++++- sound/soc/codecs/tas6424.h | 4 ++ 3 files changed, 81 insertions(+), 3 deletions(-)

-- 2.7.4

Show replies by date

Jean-Jacques Hiblot

27 Apr 27 Apr

4:55 p.m.

New subject: [alsa-devel] [PATCH v2 1/3] ASoC: tas6424: Add support for the standby pin

The standby pin can be connected to a GPIO. In that case we have to drive it to the correct values for the TAS6424 to operate properly.

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- .../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/ti,tas6424.txt b/Documentation/devicetree/bindings/sound/ti,tas6424.txt index 1c4ada0..df71e41 100644 --- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt +++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt @@ -6,6 +6,7 @@ Required properties: - compatible: "ti,tas6424" - TAS6424 - reg: I2C slave address - sound-dai-cells: must be equal to 0 + - standby-gpios: GPIO used to shut the TAS6424 down.

Example:

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 4f3a16c..5abb17f 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -16,6 +16,7 @@ #include <linux/slab.h> #include <linux/regulator/consumer.h> #include <linux/delay.h> +#include <linux/gpio/consumer.h>

#include <sound/pcm.h> #include <sound/pcm_params.h> @@ -43,6 +44,7 @@ struct tas6424_data { unsigned int last_fault1; unsigned int last_fault2; unsigned int last_warn; + struct gpio_desc *standby_gpio; };

/* @@ -627,6 +629,22 @@ static int tas6424_i2c_probe(struct i2c_client *client, return ret; }

+ /* + * Get control of the standby pin and set it LOW to take the codec + * out of the stand-by mode. + * Note: The actual pin polarity is taken care of in the GPIO lib + * according the polarity specified in the DTS. + */ + tas6424->standby_gpio = devm_gpiod_get_optional(dev, "standby", + GPIOD_OUT_LOW); + if (IS_ERR(tas6424->standby_gpio)) { + if (PTR_ERR(tas6424->standby_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_info(dev, "failed to get standby GPIO: %ld\n", + PTR_ERR(tas6424->standby_gpio)); + tas6424->standby_gpio = NULL; + } + for (i = 0; i < ARRAY_SIZE(tas6424->supplies); i++) tas6424->supplies[i].supply = tas6424_supply_names[i]; ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(tas6424->supplies), @@ -671,6 +689,10 @@ static int tas6424_i2c_remove(struct i2c_client *client)

cancel_delayed_work_sync(&tas6424->fault_check_work);

+ /* put the codec in stand-by */ + if (tas6424->standby_gpio) + gpiod_set_value_cansleep(tas6424->standby_gpio, 1); + ret = regulator_bulk_disable(ARRAY_SIZE(tas6424->supplies), tas6424->supplies); if (ret < 0) {

-- 2.7.4

Mark Brown

9:24 p.m.

New subject: [alsa-devel] Applied "ASoC: tas6424: Add support for the standby pin" to the asoc tree

The patch

ASoC: tas6424: Add support for the standby pin

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From e3976aa6fb409aea3cea812f6a0beebb545e5996 Mon Sep 17 00:00:00 2001

From: Jean-Jacques Hiblot jjhiblot@ti.com Date: Fri, 27 Apr 2018 15:55:47 +0200 Subject: [PATCH] ASoC: tas6424: Add support for the standby pin

The standby pin can be connected to a GPIO. In that case we have to drive it to the correct values for the TAS6424 to operate properly.

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Signed-off-by: Mark Brown broonie@kernel.org --- .../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/ti,tas6424.txt b/Documentation/devicetree/bindings/sound/ti,tas6424.txt index 1c4ada0eef4e..df71e414dc5b 100644 --- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt +++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt @@ -6,6 +6,7 @@ Required properties: - compatible: "ti,tas6424" - TAS6424 - reg: I2C slave address - sound-dai-cells: must be equal to 0 + - standby-gpios: GPIO used to shut the TAS6424 down.

Example:

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 4f3a16c520a2..5abb17f8d3dd 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -16,6 +16,7 @@ #include <linux/slab.h> #include <linux/regulator/consumer.h> #include <linux/delay.h> +#include <linux/gpio/consumer.h>

/* @@ -627,6 +629,22 @@ static int tas6424_i2c_probe(struct i2c_client *client, return ret; }

cancel_delayed_work_sync(&tas6424->fault_check_work);

-- 2.17.0

Rob Herring

11:13 p.m.

New subject: [alsa-devel] [PATCH v2 1/3] ASoC: tas6424: Add support for the standby pin

On Fri, Apr 27, 2018 at 03:55:47PM +0200, Jean-Jacques Hiblot wrote:

...

The standby pin can be connected to a GPIO. In that case we have to drive it to the correct values for the TAS6424 to operate properly.

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com

.../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)

Reviewed-by: Rob Herring robh@kernel.org

Jean-Jacques Hiblot

4:55 p.m.

New subject: [alsa-devel] [PATCH v2 2/3] ASoC: tas6424: Add support for the mute pin

mute can be connected to GPIO. In that case we have to drive it to the correct value

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- .../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 37 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/ti,tas6424.txt b/Documentation/devicetree/bindings/sound/ti,tas6424.txt index df71e41..eacb54f 100644 --- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt +++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt @@ -7,6 +7,7 @@ Required properties: - reg: I2C slave address - sound-dai-cells: must be equal to 0 - standby-gpios: GPIO used to shut the TAS6424 down. + - mute-gpios: GPIO used to mute all the outputs

Example:

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 5abb17f..89fd0c1 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -45,6 +45,7 @@ struct tas6424_data { unsigned int last_fault2; unsigned int last_warn; struct gpio_desc *standby_gpio; + struct gpio_desc *mute_gpio; };

/* @@ -251,10 +252,16 @@ static int tas6424_set_dai_tdm_slot(struct snd_soc_dai *dai, static int tas6424_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_component *component = dai->component; + struct tas6424_data *tas6424 = snd_soc_component_get_drvdata(component); unsigned int val;

dev_dbg(component->dev, "%s() mute=%d\n", __func__, mute);

+ if (tas6424->mute_gpio) { + gpiod_set_value_cansleep(tas6424->mute_gpio, mute); + return 0; + } + if (mute) val = TAS6424_ALL_STATE_MUTE; else @@ -289,6 +296,7 @@ static int tas6424_power_on(struct snd_soc_component *component) { struct tas6424_data *tas6424 = snd_soc_component_get_drvdata(component); int ret; + u8 chan_states;

ret = regulator_bulk_enable(ARRAY_SIZE(tas6424->supplies), tas6424->supplies); @@ -305,7 +313,18 @@ static int tas6424_power_on(struct snd_soc_component *component) return ret; }

- snd_soc_component_write(component, TAS6424_CH_STATE_CTRL, TAS6424_ALL_STATE_MUTE); + if (tas6424->mute_gpio) { + gpiod_set_value_cansleep(tas6424->mute_gpio, 0); + /* + * channels are muted via the mute pin. Don't also mute + * them via the registers so that subsequent register + * access is not necessary to un-mute the channels + */ + chan_states = TAS6424_ALL_STATE_PLAY; + } else { + chan_states = TAS6424_ALL_STATE_MUTE; + } + snd_soc_component_write(component, TAS6424_CH_STATE_CTRL, chan_states);

/* any time we come out of HIZ, the output channels automatically run DC * load diagnostics, wait here until this completes @@ -645,6 +664,22 @@ static int tas6424_i2c_probe(struct i2c_client *client, tas6424->standby_gpio = NULL; }

+ /* + * Get control of the mute pin and set it HIGH in order to start with + * all the output muted. + * Note: The actual pin polarity is taken care of in the GPIO lib + * according the polarity specified in the DTS. + */ + tas6424->mute_gpio = devm_gpiod_get_optional(dev, "mute", + GPIOD_OUT_HIGH); + if (IS_ERR(tas6424->mute_gpio)) { + if (PTR_ERR(tas6424->mute_gpio) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_info(dev, "failed to get nmute GPIO: %ld\n", + PTR_ERR(tas6424->mute_gpio)); + tas6424->mute_gpio = NULL; + } + for (i = 0; i < ARRAY_SIZE(tas6424->supplies); i++) tas6424->supplies[i].supply = tas6424_supply_names[i]; ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(tas6424->supplies),

-- 2.7.4

Mark Brown

9:24 p.m.

New subject: [alsa-devel] Applied "ASoC: tas6424: Add support for the mute pin" to the asoc tree

The patch

ASoC: tas6424: Add support for the mute pin

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

...

From e969a6d222c9da87c6b5978674788faf5c58a07d Mon Sep 17 00:00:00 2001

From: Jean-Jacques Hiblot jjhiblot@ti.com Date: Fri, 27 Apr 2018 15:55:48 +0200 Subject: [PATCH] ASoC: tas6424: Add support for the mute pin

mute can be connected to GPIO. In that case we have to drive it to the correct value

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Signed-off-by: Mark Brown broonie@kernel.org --- .../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/ti,tas6424.txt b/Documentation/devicetree/bindings/sound/ti,tas6424.txt index df71e414dc5b..eacb54f34188 100644 --- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt +++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt @@ -7,6 +7,7 @@ Required properties: - reg: I2C slave address - sound-dai-cells: must be equal to 0 - standby-gpios: GPIO used to shut the TAS6424 down. + - mute-gpios: GPIO used to mute all the outputs

Example:

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 5abb17f8d3dd..89fd0c1b3903 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -45,6 +45,7 @@ struct tas6424_data { unsigned int last_fault2; unsigned int last_warn; struct gpio_desc *standby_gpio; + struct gpio_desc *mute_gpio; };

dev_dbg(component->dev, "%s() mute=%d\n", __func__, mute);

ret = regulator_bulk_enable(ARRAY_SIZE(tas6424->supplies), tas6424->supplies); @@ -305,7 +313,18 @@ static int tas6424_power_on(struct snd_soc_component *component) return ret; }

-- 2.17.0

Rob Herring

11:15 p.m.

New subject: [alsa-devel] [PATCH v2 2/3] ASoC: tas6424: Add support for the mute pin

On Fri, Apr 27, 2018 at 03:55:48PM +0200, Jean-Jacques Hiblot wrote:

...

mute can be connected to GPIO. In that case we have to drive it to the correct value

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com

.../devicetree/bindings/sound/ti,tas6424.txt | 1 + sound/soc/codecs/tas6424.c | 37 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring robh@kernel.org

Jean-Jacques Hiblot

4:55 p.m.

New subject: [alsa-devel] [PATCH v2 3/3] ASoC: tas6424: Allow disabling auto diagnostics for faster power-on

The TAS6424 incorporates both DC-load and AC-load diagnostics which are used to determine the status of the load. The DC diagnostics runs when any channel is directed to leave the Hi-Z state and enter the MUTE or PLAY state. The DC diagnostics are turned on by default but, if a fast startup without diagnostics is required, the diagnostics can be disabled using a dedicated ALSA control.

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- sound/soc/codecs/tas6424.c | 19 +++++++++++++++++-- sound/soc/codecs/tas6424.h | 4 ++++ 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 89fd0c1..8eb4842 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -55,6 +55,13 @@ struct tas6424_data { */ static DECLARE_TLV_DB_SCALE(dac_tlv, -10350, 50, 0);

+ +static const char * const diags_enum_texts[] = {"OFF", "ON"}; +static unsigned int diags_enum_values[] = {1, 0}; +SOC_VALUE_ENUM_SINGLE_DECL(diags_enum, TAS6424_DC_DIAG_CTRL1, + TAS6424_LDGBYPASS_SHIFT, TAS6424_LDGBYPASS_MASK, + diags_enum_texts, diags_enum_values); + static const struct snd_kcontrol_new tas6424_snd_controls[] = { SOC_SINGLE_TLV("Speaker Driver CH1 Playback Volume", TAS6424_CH1_VOL_CTRL, 0, 0xff, 0, dac_tlv), @@ -64,6 +71,7 @@ static const struct snd_kcontrol_new tas6424_snd_controls[] = { TAS6424_CH3_VOL_CTRL, 0, 0xff, 0, dac_tlv), SOC_SINGLE_TLV("Speaker Driver CH4 Playback Volume", TAS6424_CH4_VOL_CTRL, 0, 0xff, 0, dac_tlv), + SOC_ENUM("Auto Diagnostics", diags_enum), };

static int tas6424_dac_event(struct snd_soc_dapm_widget *w, @@ -297,6 +305,11 @@ static int tas6424_power_on(struct snd_soc_component *component) struct tas6424_data *tas6424 = snd_soc_component_get_drvdata(component); int ret; u8 chan_states; + int no_auto_diags = 0; + unsigned int reg_val; + + if (!regmap_read(tas6424->regmap, TAS6424_DC_DIAG_CTRL1, &reg_val)) + no_auto_diags = reg_val & TAS6424_LDGBYPASS_MASK;

ret = regulator_bulk_enable(ARRAY_SIZE(tas6424->supplies), tas6424->supplies); @@ -327,9 +340,11 @@ static int tas6424_power_on(struct snd_soc_component *component) snd_soc_component_write(component, TAS6424_CH_STATE_CTRL, chan_states);

/* any time we come out of HIZ, the output channels automatically run DC - * load diagnostics, wait here until this completes + * load diagnostics if autodiagnotics are enabled. wait here until this + * completes. */ - msleep(230); + if (!no_auto_diags) + msleep(230);

return 0; } diff --git a/sound/soc/codecs/tas6424.h b/sound/soc/codecs/tas6424.h index 4305883..b5958c4 100644 --- a/sound/soc/codecs/tas6424.h +++ b/sound/soc/codecs/tas6424.h @@ -111,6 +111,10 @@ TAS6424_CH3_STATE_DIAG | \ TAS6424_CH4_STATE_DIAG)

+/* TAS6424_DC_DIAG_CTRL1 */ +#define TAS6424_LDGBYPASS_SHIFT 0 +#define TAS6424_LDGBYPASS_MASK BIT(TAS6424_LDGBYPASS_SHIFT) + /* TAS6424_GLOB_FAULT1_REG */ #define TAS6424_FAULT_CLOCK BIT(4) #define TAS6424_FAULT_PVDD_OV BIT(3)

-- 2.7.4

Mark Brown

6:30 p.m.

New subject: [alsa-devel] [PATCH v2 3/3] ASoC: tas6424: Allow disabling auto diagnostics for faster power-on

On Fri, Apr 27, 2018 at 03:55:49PM +0200, Jean-Jacques Hiblot wrote:

...

+static const char * const diags_enum_texts[] = {"OFF", "ON"}; +static unsigned int diags_enum_values[] = {1, 0}; +SOC_VALUE_ENUM_SINGLE_DECL(diags_enum, TAS6424_DC_DIAG_CTRL1,
	   TAS6424_LDGBYPASS_SHIFT, TAS6424_LDGBYPASS_MASK,
	   diags_enum_texts, diags_enum_values);

This just just an on/off switch so it should be represented as a standard _SINGLE() control with " Switch" at the end of the name so userspace knows what to do with it.

kbuild test robot

28 Apr 28 Apr

12:21 p.m.

New subject: [alsa-devel] [RFC PATCH] ASoC: tas6424: diags_enum can be static

Fixes: c9f4b53ff449 ("ASoC: tas6424: Allow disabling auto diagnostics for faster power-on") Signed-off-by: Fengguang Wu fengguang.wu@intel.com --- tas6424.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c index 8eb4842..8bca6b2 100644 --- a/sound/soc/codecs/tas6424.c +++ b/sound/soc/codecs/tas6424.c @@ -58,7 +58,7 @@ static DECLARE_TLV_DB_SCALE(dac_tlv, -10350, 50, 0);

static const char * const diags_enum_texts[] = {"OFF", "ON"}; static unsigned int diags_enum_values[] = {1, 0}; -SOC_VALUE_ENUM_SINGLE_DECL(diags_enum, TAS6424_DC_DIAG_CTRL1, +static SOC_VALUE_ENUM_SINGLE_DECL(diags_enum, TAS6424_DC_DIAG_CTRL1, TAS6424_LDGBYPASS_SHIFT, TAS6424_LDGBYPASS_MASK, diags_enum_texts, diags_enum_values);

kbuild test robot

12:21 p.m.

New subject: [alsa-devel] [PATCH v2 3/3] ASoC: tas6424: Allow disabling auto diagnostics for faster power-on

Hi Jean-Jacques,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on asoc/for-next] [also build test WARNING on v4.17-rc2 next-20180426] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Jean-Jacques-Hiblot/ASoc-TAS6424-Ad... base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

...

...
sound/soc/codecs/tas6424.c:61:1: sparse: symbol 'diags_enum' was not declared. Should it be static?

Please review and possibly fold the followup patch.

--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation

2393

Age (days ago)

2394

Last active (days ago)

List overview

Download

10 comments

4 participants

tags (0)

participants (4)

Jean-Jacques Hiblot
kbuild test robot
Mark Brown
Rob Herring