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).
Jean-Jacques Hiblot (3):
ASoC: tas6424: Allow disabling auto diagnostics for faster power-on
ASoC: tas6424: Add support for the standby pin
ASoC: tas6424: Add support for the mute pin
.../devicetree/bindings/sound/ti,tas6424.txt | 4 ++
sound/soc/codecs/tas6424.c | 74 +++++++++++++++++++++-
sound/soc/codecs/tas6424.h | 3 +
3 files changed, 79 insertions(+), 2 deletions(-)
--
2.7.4
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 <[email protected]>
---
.../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 bf2ff98..82c6d48 100644
--- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt
+++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt
@@ -8,6 +8,7 @@ Required properties:
- sound-dai-cells: must be equal to 0
- disable-auto-diagnostics: disable DC auto diagnostics (faster power
on, but less safe as shortage won't be detected)
+ - standby-gpio: GPIO used to shut the TAS6424 down.
Example:
diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c
index 5cee400..926259a 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>
@@ -44,6 +45,7 @@ struct tas6424_data {
unsigned int last_fault2;
unsigned int last_warn;
bool no_auto_diags;
+ struct gpio_desc *standby_gpio;
};
/*
@@ -632,6 +634,22 @@ static int tas6424_i2c_probe(struct i2c_client *client,
tas6424->no_auto_diags = of_property_read_bool(dev->of_node,
"disable-auto-diagnostics");
+ /*
+ * 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 standvby 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),
@@ -691,6 +709,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
mute can be connected to GPIO. In that case we have to drive it to the
correct value
Signed-off-by: Jean-Jacques Hiblot <[email protected]>
---
.../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 82c6d48..527e356 100644
--- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt
+++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt
@@ -9,6 +9,7 @@ Required properties:
- disable-auto-diagnostics: disable DC auto diagnostics (faster power
on, but less safe as shortage won't be detected)
- standby-gpio: GPIO used to shut the TAS6424 down.
+ - mute-gpio: GPIO used to mute all the outputs
Example:
diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c
index 926259a..cf84b1c 100644
--- a/sound/soc/codecs/tas6424.c
+++ b/sound/soc/codecs/tas6424.c
@@ -46,6 +46,7 @@ struct tas6424_data {
unsigned int last_warn;
bool no_auto_diags;
struct gpio_desc *standby_gpio;
+ struct gpio_desc *mute_gpio;
};
/*
@@ -252,10 +253,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 ? 1 : 0);
+ return 0;
+ }
+
if (mute)
val = TAS6424_ALL_STATE_MUTE;
else
@@ -290,6 +297,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);
@@ -306,7 +314,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. 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
@@ -650,6 +669,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
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 DC diagnostics can be bypassed by adding the
property "disable-auto-diagnostics".
Signed-off-by: Jean-Jacques Hiblot <[email protected]>
---
.../devicetree/bindings/sound/ti,tas6424.txt | 2 ++
sound/soc/codecs/tas6424.c | 22 +++++++++++++++++++++-
sound/soc/codecs/tas6424.h | 3 +++
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/ti,tas6424.txt b/Documentation/devicetree/bindings/sound/ti,tas6424.txt
index 1c4ada0..bf2ff98 100644
--- a/Documentation/devicetree/bindings/sound/ti,tas6424.txt
+++ b/Documentation/devicetree/bindings/sound/ti,tas6424.txt
@@ -6,6 +6,8 @@ Required properties:
- compatible: "ti,tas6424" - TAS6424
- reg: I2C slave address
- sound-dai-cells: must be equal to 0
+ - disable-auto-diagnostics: disable DC auto diagnostics (faster power
+ on, but less safe as shortage won't be detected)
Example:
diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c
index 4f3a16c..5cee400 100644
--- a/sound/soc/codecs/tas6424.c
+++ b/sound/soc/codecs/tas6424.c
@@ -43,6 +43,7 @@ struct tas6424_data {
unsigned int last_fault1;
unsigned int last_fault2;
unsigned int last_warn;
+ bool no_auto_diags;
};
/*
@@ -308,7 +309,8 @@ static int tas6424_power_on(struct snd_soc_component *component)
/* any time we come out of HIZ, the output channels automatically run DC
* load diagnostics, wait here until this completes
*/
- msleep(230);
+ if (!tas6424->no_auto_diags)
+ msleep(230);
return 0;
}
@@ -627,6 +629,9 @@ static int tas6424_i2c_probe(struct i2c_client *client,
return ret;
}
+ tas6424->no_auto_diags = of_property_read_bool(dev->of_node,
+ "disable-auto-diagnostics");
+
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),
@@ -651,6 +656,21 @@ static int tas6424_i2c_probe(struct i2c_client *client,
return ret;
}
+ if (tas6424->no_auto_diags) {
+ /*
+ * Disable DC auto-diagnostics to save time when channel leave
+ * Hi-Z state
+ */
+ ret = regmap_update_bits(tas6424->regmap,
+ TAS6424_DC_DIAG_CTRL1,
+ 0xff, TAS6424_LDGBYPASS);
+ if (ret) {
+ dev_err(dev, "failed to disable auto-diags: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
INIT_DELAYED_WORK(&tas6424->fault_check_work, tas6424_fault_check_work);
ret = devm_snd_soc_register_component(dev, &soc_codec_dev_tas6424,
diff --git a/sound/soc/codecs/tas6424.h b/sound/soc/codecs/tas6424.h
index 4305883..3aea0f7 100644
--- a/sound/soc/codecs/tas6424.h
+++ b/sound/soc/codecs/tas6424.h
@@ -111,6 +111,9 @@
TAS6424_CH3_STATE_DIAG | \
TAS6424_CH4_STATE_DIAG)
+/* TAS6424_DC_DIAG_CTRL1 */
+#define TAS6424_LDGBYPASS BIT(0)
+
/* TAS6424_GLOB_FAULT1_REG */
#define TAS6424_FAULT_CLOCK BIT(4)
#define TAS6424_FAULT_PVDD_OV BIT(3)
--
2.7.4
On Fri, Apr 20, 2018 at 12:04:43PM +0200, Jean-Jacques Hiblot wrote:
> + - standby-gpio: GPIO used to shut the TAS6424 down.
All GPIO property names are supposed to be called -gpios regardless of
if there can ever be more than one GPIO (though the framework does
support the singular, I wonder if it's not more helpful to just get the
spec relaxed here).
> + dev_info(dev, "failed to get standvby GPIO: %ld\n",
Typo.
On Fri, Apr 20, 2018 at 12:04:44PM +0200, Jean-Jacques Hiblot wrote:
> + - mute-gpio: GPIO used to mute all the outputs
Same thing with the plural here.
> + if (tas6424->mute_gpio) {
> + gpiod_set_value_cansleep(tas6424->mute_gpio, mute ? 1 : 0);
> + return 0;
> + }
Just use mute directly, the ternery operator is doing nothing for
legibility here and C does the integer to boolean thing for you.
> + if (tas6424->mute_gpio) {
> + gpiod_set_value_cansleep(tas6424->mute_gpio, 0);
> + /*
> + * channels are muted via the mute pin, don't also. Don't also
> + * mute them via the registers so that subsequent register
> + * access is not necessary to un-mute the channels
Extra "don't also" in there.
On Fri, Apr 20, 2018 at 12:04:42PM +0200, Jean-Jacques Hiblot wrote:
> 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 DC diagnostics can be bypassed by adding the
> property "disable-auto-diagnostics".
This is making me think we should be smarter here and either only run
the diagnostics once during boot or provide a user control for the
diagnostics. It seems like something that is more of a runtime software
decision than something that's fixed in hardware design, is there
anything about the hardware design that'd make it impossible to run
diagnostics?
> + if (tas6424->no_auto_diags) {
> + /*
> + * Disable DC auto-diagnostics to save time when channel leave
> + * Hi-Z state
> + */
> + ret = regmap_update_bits(tas6424->regmap,
> + TAS6424_DC_DIAG_CTRL1,
> + 0xff, TAS6424_LDGBYPASS);
This could just be exposed to userspace as a simple switch control
couldn't it? I do note that it masks more bits than it sets though...
On 23/04/2018 17:44, Mark Brown wrote:
> On Fri, Apr 20, 2018 at 12:04:42PM +0200, Jean-Jacques Hiblot wrote:
>> 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 DC diagnostics can be bypassed by adding the
>> property "disable-auto-diagnostics".
> This is making me think we should be smarter here and either only run
> the diagnostics once during boot or provide a user control for the
> diagnostics. It seems like something that is more of a runtime software
> decision than something that's fixed in hardware design, is there
> anything about the hardware design that'd make it impossible to run
> diagnostics?
I can't think of anything that would make it unusable.
This auto diagnostic is really useful in cars where wires can get
disconnected or shorted to ground.
In quieter environment it may not be as useful, and a test at startup
may be sufficient.
Diagnostics can also be triggered at will.
Do you think a sysfs interface would be better suited to disable the
autodiag ?
Thanks for your comment on the series.
JJ
>
>> + if (tas6424->no_auto_diags) {
>> + /*
>> + * Disable DC auto-diagnostics to save time when channel leave
>> + * Hi-Z state
>> + */
>> + ret = regmap_update_bits(tas6424->regmap,
>> + TAS6424_DC_DIAG_CTRL1,
>> + 0xff, TAS6424_LDGBYPASS);
> This could just be exposed to userspace as a simple switch control
> couldn't it? I do note that it masks more bits than it sets though...
On Mon, Apr 23, 2018 at 09:00:18PM +0200, Jean-Jacques Hiblot wrote:
> I can't think of anything that would make it unusable.
> This auto diagnostic is really useful in cars where wires can get
> disconnected or shorted to ground.
> In quieter environment it may not be as useful, and a test at startup may be
> sufficient.
> Diagnostics can also be triggered at will.
> Do you think a sysfs interface would be better suited to disable the
> autodiag ?
I'd just make it an ALSA control TBH. sysfs should work as well though.