[v2,10/19] ASoC: tlv320aic31xx: Add MICBIAS off setting
diff mbox

Message ID 20171129213300.20021-11-afd@ti.com
State New
Headers show

Commit Message

Andrew F. Davis Nov. 29, 2017, 9:32 p.m. UTC
Leaving microphone bias off is a valid setting and even used in the DT
binding document example. Add this setting here and document the same.

Signed-off-by: Andrew F. Davis <afd@ti.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/sound/tlv320aic31xx.txt | 1 +
 include/dt-bindings/sound/tlv320aic31xx-micbias.h         | 1 +
 sound/soc/codecs/tlv320aic31xx.c                          | 1 +
 3 files changed, 3 insertions(+)

Comments

Mark Brown Dec. 1, 2017, 1:30 p.m. UTC | #1
On Wed, Nov 29, 2017 at 03:32:51PM -0600, Andrew F. Davis wrote:
> Leaving microphone bias off is a valid setting and even used in the DT
> binding document example. Add this setting here and document the same.

>  - ai31xx-micbias-vg - MicBias Voltage setting
> +        0 or MICBIAS_OFF - MICBIAS output is powered off
>          1 or MICBIAS_2_0V - MICBIAS output is powered to 2.0V
>          2 or MICBIAS_2_5V - MICBIAS output is powered to 2.5V
>          3 or MICBIAS_AVDD - MICBIAS output is connected to AVDD

This doesn't make much sense.  If we enable the microphone with a
voltage of 0V that's not going to work terribly well - I'd expect this
property to control the voltage set when it's enabled, not if it's
enabled.
Andrew F. Davis Dec. 1, 2017, 2:56 p.m. UTC | #2
On 12/01/2017 07:30 AM, Mark Brown wrote:
> On Wed, Nov 29, 2017 at 03:32:51PM -0600, Andrew F. Davis wrote:
>> Leaving microphone bias off is a valid setting and even used in the DT
>> binding document example. Add this setting here and document the same.
> 
>>  - ai31xx-micbias-vg - MicBias Voltage setting
>> +        0 or MICBIAS_OFF - MICBIAS output is powered off
>>          1 or MICBIAS_2_0V - MICBIAS output is powered to 2.0V
>>          2 or MICBIAS_2_5V - MICBIAS output is powered to 2.5V
>>          3 or MICBIAS_AVDD - MICBIAS output is connected to AVDD
> 
> This doesn't make much sense.  If we enable the microphone with a
> voltage of 0V that's not going to work terribly well - I'd expect this
> property to control the voltage set when it's enabled, not if it's
> enabled.
> 

I agree about this not making much sense, but I'm not sure the right way
forward here.

Personally I wouldn't have put MicBias voltage in DT to begin with, it
is runtime configurable and depends on user runtime inserted microphone,
not really a hardware description :/

The enable/disable is controlled by a DAPM and should be wired to enable
whenever we are recording from the related microphone pin. So then
without MICBIAS_OFF the user is forced to select from the above voltages
whenever recording, which may not be correct if they are using a mic
that doesn't need bias.

The best option I think would be to drop this from DT, but I don't feel
like fighting that battle right now.
Mark Brown Dec. 1, 2017, 5:43 p.m. UTC | #3
On Fri, Dec 01, 2017 at 08:56:31AM -0600, Andrew F. Davis wrote:

> Personally I wouldn't have put MicBias voltage in DT to begin with, it
> is runtime configurable and depends on user runtime inserted microphone,
> not really a hardware description :/

Usually the system as a whole is specified to have some particular kind
of microphone attached (or soldered in) and this follows from that.
There's typically also soldered down analogue components that are
selected based on the bias voltage so a specific system likely won't
have the full flexibility.

> The enable/disable is controlled by a DAPM and should be wired to enable
> whenever we are recording from the related microphone pin. So then
> without MICBIAS_OFF the user is forced to select from the above voltages
> whenever recording, which may not be correct if they are using a mic
> that doesn't need bias.

That'd depend on someone having a board that was able to detect that a
line input was plugged in or a user control for this.  If the board was
permenantly set up for line input presumably the bias wouldn't even be
wired up.

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
index 5b3c33bb99e5..411cc46a2c58 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
@@ -24,6 +24,7 @@  Optional properties:
 
 - reset-gpios - GPIO specification for the active low RESET input.
 - ai31xx-micbias-vg - MicBias Voltage setting
+        0 or MICBIAS_OFF - MICBIAS output is powered off
         1 or MICBIAS_2_0V - MICBIAS output is powered to 2.0V
         2 or MICBIAS_2_5V - MICBIAS output is powered to 2.5V
         3 or MICBIAS_AVDD - MICBIAS output is connected to AVDD
diff --git a/include/dt-bindings/sound/tlv320aic31xx-micbias.h b/include/dt-bindings/sound/tlv320aic31xx-micbias.h
index c6895a18a455..069484070fcf 100644
--- a/include/dt-bindings/sound/tlv320aic31xx-micbias.h
+++ b/include/dt-bindings/sound/tlv320aic31xx-micbias.h
@@ -2,6 +2,7 @@ 
 #ifndef __DT_TLV320AIC31XX_MICBIAS_H
 #define __DT_TLV320AIC31XX_MICBIAS_H
 
+#define MICBIAS_OFF		0
 #define MICBIAS_2_0V		1
 #define MICBIAS_2_5V		2
 #define MICBIAS_AVDDV		3
diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
index 0e00421d363b..252d99af6688 100644
--- a/sound/soc/codecs/tlv320aic31xx.c
+++ b/sound/soc/codecs/tlv320aic31xx.c
@@ -1298,6 +1298,7 @@  static int aic31xx_i2c_probe(struct i2c_client *i2c,
 	fwnode_property_read_u32(aic31xx->dev->fwnode, "ai31xx-micbias-vg",
 				 &micbias_value);
 	switch (micbias_value) {
+	case MICBIAS_OFF:
 	case MICBIAS_2_0V:
 	case MICBIAS_2_5V:
 	case MICBIAS_AVDDV: