ASoC: tlv320aic31xx: Add MICBIAS off setting
PATCH 1/1 ASoC: tlv320aic31xx: Add MICBIAS off setting
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. Although it may not make much sense to enable a microphone here without any bias, it is a valid setting that can be chosen by DT and may be needed for some boards.
--- 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(+)
Documentation/devicetree/bindings/sound/tlv320aic31xx.txt +1 -0
@@ -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
include/dt-bindings/sound/tlv320aic31xx-micbias.h +1 -0
@@ -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
sound/soc/codecs/tlv320aic31xx.c +1 -0
@@ -1421,6 +1421,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 :
Signature · 1 line
2.18.0
Mark Brown
maintainerQuoted 5 lines · Andrew F. Davis
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. Although it may not make much sense to enable a microphone here without any bias, it is a valid setting that can be chosen by DT and may be needed for some boards.
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Quoted 7 lines · Mark Brown
Quoted 5 lines · Andrew F. Davis
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. Although it may not make much sense to enable a microphone here without any bias, it is a valid setting that can be chosen by DT and may be needed for some boards.
Quoted 6 lines
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Turning on bias is controlled separately, automatically in user-space in many cases, not based on the board. The DT needs to have a way to state that 0v is the needed bias on this board, without this you can not set 0v bias and 2v is chosen by default (which is IMHO should be 0v but that would change existing behavior so I won't touch that).
Mark Brown
maintainerQuoted 8 lines · Andrew F. Davis
Quoted 5 lines · Mark Brown
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Quoted 5 lines
Turning on bias is controlled separately, automatically in user-space in many cases, not based on the board. The DT needs to have a way to state that 0v is the needed bias on this board, without this you can not set 0v bias and 2v is chosen by default (which is IMHO should be 0v but that would change existing behavior so I won't touch that).
Surely turning on MICBIAS at 0V is equivalent to leaving the bias off?
Quoted 9 lines · Mark Brown
Quoted 7 lines · Andrew F. Davis
Quoted 5 lines · Mark Brown
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Quoted 8 lines
Quoted 5 lines
Turning on bias is controlled separately, automatically in user-space in many cases, not based on the board. The DT needs to have a way to state that 0v is the needed bias on this board, without this you can not set 0v bias and 2v is chosen by default (which is IMHO should be 0v but that would change existing behavior so I won't touch that).
Surely turning on MICBIAS at 0V is equivalent to leaving the bias off?
It is, but DT doesn't control when or why MICBIAS is turned on or off, only what voltage on should be.
Mark Brown
maintainerQuoted 14 lines · Andrew F. Davis
Quoted 11 lines · Mark Brown
Quoted 11 lines
Quoted 5 lines
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Turning on bias is controlled separately, automatically in user-space in many cases, not based on the board. The DT needs to have a way to state that 0v is the needed bias on this board, without this you can not set 0v bias and 2v is chosen by default (which is IMHO should be 0v but that would change existing behavior so I won't touch that).
Quoted 4 lines
Quoted 1 line
Surely turning on MICBIAS at 0V is equivalent to leaving the bias off?
It is, but DT doesn't control when or why MICBIAS is turned on or off, only what voltage on should be.
The DT controls this in so far as it will arrange for the bias to be connected to something, if it's not connected to anything then it will only be turned on if the machine driver explicitly forces it on which is a potential source of problems but seems comfortably in "you broke it, you get to keep the pieces" territory. If we really want to have a way of explicitly specifying that some widgets should never be turned on then it feels like rather than have something device and widget specific like this we should instead have a higher level way of doing that which can be applied to any widget, that is something that could be useful especially with things like speaker drivers where there's real potential for physical damage to the system.
Like I said in my original reply I'm also worried that this will break existing boards by causing them to change to a voltage of 0 when they had managed to end up with the default of 2V which happened to work for them.
Quoted 20 lines · Mark Brown
Quoted 14 lines · Andrew F. Davis
Quoted 11 lines · Mark Brown
Quoted 11 lines
Quoted 5 lines
If we really want to pay attention to something setting this up we'd need to completely remove the widget - what the code is doing at the minute is setting the voltage that the bias will go to when enabled, there's still a widget for turning it on and off. There's some chance that this will break existing boards.
Turning on bias is controlled separately, automatically in user-space in many cases, not based on the board. The DT needs to have a way to state that 0v is the needed bias on this board, without this you can not set 0v bias and 2v is chosen by default (which is IMHO should be 0v but that would change existing behavior so I won't touch that).
Quoted 4 lines
Quoted 1 line
Surely turning on MICBIAS at 0V is equivalent to leaving the bias off?
It is, but DT doesn't control when or why MICBIAS is turned on or off, only what voltage on should be.
Quoted 12 lines
The DT controls this in so far as it will arrange for the bias to be connected to something, if it's not connected to anything then it will only be turned on if the machine driver explicitly forces it on which is a potential source of problems but seems comfortably in "you broke it, you get to keep the pieces" territory. If we really want to have a way of explicitly specifying that some widgets should never be turned on then it feels like rather than have something device and widget specific like this we should instead have a higher level way of doing that which can be applied to any widget, that is something that could be useful especially with things like speaker drivers where there's real potential for physical damage to the system.
Not sure how that could even look, it would need to be in DT as only that layer knows the connections, but then the it would also have to have internal knowledge of the widgets used in the driver..
Quoted 5 lines
Like I said in my original reply I'm also worried that this will break existing boards by causing them to change to a voltage of 0 when they had managed to end up with the default of 2V which happened to work for them.
Then only time this could happen is if they specified MICBIAS_OFF in DT but got the default 2v instead. In which case, yes, the behavior would change, but it would also be changing to the correct behavior. We can't avoid fixing code on the off chance someone depended on the broken behavior, no progress could ever be made.
Mark Brown
maintainerQuoted 10 lines · Andrew F. Davis
Quoted 7 lines · Mark Brown
you get to keep the pieces" territory. If we really want to have a way of explicitly specifying that some widgets should never be turned on then it feels like rather than have something device and widget specific like this we should instead have a higher level way of doing that which can be applied to any widget, that is something that could be useful especially with things like speaker drivers where there's real potential for physical damage to the system.
Quoted 3 lines
Not sure how that could even look, it would need to be in DT as only that layer knows the connections, but then the it would also have to have internal knowledge of the widgets used in the driver..
We already have a bunch of DT code that knows about and uses the external widgets the device has (which are I'd guess the only ones where this is an issue)? I was thinking just have a property that lists the widgets that should never be turned on.
Quoted 10 lines
Quoted 4 lines
Like I said in my original reply I'm also worried that this will break existing boards by causing them to change to a voltage of 0 when they had managed to end up with the default of 2V which happened to work for them.
Then only time this could happen is if they specified
MICBIAS_OFFin DT but got the default 2v instead. In which case, yes, the behavior would change, but it would also be changing to the correct behavior. We can't avoid fixing code on the off chance someone depended on the broken behavior, no progress could ever be made.
I'm really having a lot of trouble seeing MICBIAS_OFF as a useful voltage to specify in DT in the first place.
Quoted 25 lines · Mark Brown
Quoted 10 lines · Andrew F. Davis
Quoted 7 lines · Mark Brown
you get to keep the pieces" territory. If we really want to have a way of explicitly specifying that some widgets should never be turned on then it feels like rather than have something device and widget specific like this we should instead have a higher level way of doing that which can be applied to any widget, that is something that could be useful especially with things like speaker drivers where there's real potential for physical damage to the system.
Quoted 3 lines
Not sure how that could even look, it would need to be in DT as only that layer knows the connections, but then the it would also have to have internal knowledge of the widgets used in the driver..
We already have a bunch of DT code that knows about and uses the external widgets the device has (which are I'd guess the only ones where this is an issue)? I was thinking just have a property that lists the widgets that should never be turned on.
Quoted 4 lines
Quoted 4 lines
Like I said in my original reply I'm also worried that this will break existing boards by causing them to change to a voltage of 0 when they had managed to end up with the default of 2V which happened to work for them.
Quoted 9 lines
Quoted 5 lines
Then only time this could happen is if they specified
MICBIAS_OFFin DT but got the default 2v instead. In which case, yes, the behavior would change, but it would also be changing to the correct behavior. We can't avoid fixing code on the off chance someone depended on the broken behavior, no progress could ever be made.I'm really having a lot of trouble seeing
MICBIAS_OFFas a useful voltage to specify in DT in the first place.
I don't see the usefulness in specifying any bias voltage in DT at all, it is a configuration and can be made at runtime, it has no place in DT. But it is already here, so lets allow all available voltages a board may need and the CODEC can supply, even 0.
Mark Brown
maintainerQuoted 9 lines · Andrew F. Davis
Quoted 2 lines · Mark Brown
I'm really having a lot of trouble seeing
MICBIAS_OFFas a useful voltage to specify in DT in the first place.I don't see the usefulness in specifying any bias voltage in DT at all, it is a configuration and can be made at runtime, it has no place in DT. But it is already here, so lets allow all available voltages a board may need and the CODEC can supply, even 0.
It is very rare for it to be useful to select the bias voltage at runtime - it's usually something that's decided by the electrical engineering at system design time and linked to selection of passive components rather than something that a user could usefully vary. I suspect in the situations where it is useful to vary it you'd want a layer of indirection mapping it onto some user observable behaviour (or the system should just do this autonomously as with the various low power mic detect solutions out there). What situations are you aware of where runtime configuration is useful?