[alsa-devel] [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency
The codec driver uses regmap to do i2c read/write. The codec driver started to use REGMAP_IRQ since:
5e3363ad1b7b2e1f197a3f56b01e21cb155ad454 ASoC: rt5677: add GPIO IRQ support
Signed-off-by: Ben Zhang benzh@chromium.org --- sound/soc/codecs/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 6f21a76..60f9425 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -503,6 +503,8 @@ config SND_SOC_RT5670
config SND_SOC_RT5677 tristate + select REGMAP_I2C + select REGMAP_IRQ
config SND_SOC_RT5677_SPI tristate
The MICBIAS voltage for IN1 can be set to 1.476V/2.970V/1.242V/2.475V
Signed-off-by: Ben Zhang benzh@chromium.org --- Documentation/devicetree/bindings/sound/rt5677.txt | 4 ++++ include/sound/rt5677.h | 9 +++++++++ sound/soc/codecs/rt5677.c | 6 ++++++ 3 files changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rt5677.txt b/Documentation/devicetree/bindings/sound/rt5677.txt index 740ff77..f54d0dd 100644 --- a/Documentation/devicetree/bindings/sound/rt5677.txt +++ b/Documentation/devicetree/bindings/sound/rt5677.txt @@ -19,6 +19,9 @@ Optional properties:
- realtek,pow-ldo2-gpio : The GPIO that controls the CODEC's POW_LDO2 pin.
+- realtek,micbias1 + Select 0/1/2/3 to set MICBIAS1 voltage to 1.476V/2.970V/1.242V/2.475V + - realtek,in1-differential - realtek,in2-differential - realtek,lout1-differential @@ -70,6 +73,7 @@ rt5677 {
realtek,pow-ldo2-gpio = <&gpio TEGRA_GPIO(V, 3) GPIO_ACTIVE_HIGH>; + realtek,micbias1 = <1> /* MICBIAS1 = 2.970V */ realtek,in1-differential = "true"; realtek,gpio-config = /bits/ 8 <0 0 0 0 0 2>; /* pull up GPIO6 */ realtek,jd2-gpio = <3>; /* Enables Jack detection for GPIO6 */ diff --git a/include/sound/rt5677.h b/include/sound/rt5677.h index d9eb7d8..efa74bb 100644 --- a/include/sound/rt5677.h +++ b/include/sound/rt5677.h @@ -12,6 +12,13 @@ #ifndef __LINUX_SND_RT5677_H #define __LINUX_SND_RT5677_H
+enum rt5677_micbias { + RT5677_MICBIAS_1_476V = 0, + RT5677_MICBIAS_2_970V = 1, + RT5677_MICBIAS_1_242V = 2, + RT5677_MICBIAS_2_475V = 3, +}; + enum rt5677_dmic2_clk { RT5677_DMIC_CLK1 = 0, RT5677_DMIC_CLK2 = 1, @@ -19,6 +26,8 @@ enum rt5677_dmic2_clk {
struct rt5677_platform_data { + /* MICBIAS output voltage control */ + enum rt5677_micbias micbias1; /* IN1/IN2/LOUT1/LOUT2/LOUT3 can optionally be differential */ bool in1_diff; bool in2_diff; diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c index 81fe146..ac4bee8 100644 --- a/sound/soc/codecs/rt5677.c +++ b/sound/soc/codecs/rt5677.c @@ -4551,6 +4551,7 @@ MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np) { + of_property_read_u32(np, "realtek,micbias1", &rt5677->pdata.micbias1); rt5677->pdata.in1_diff = of_property_read_bool(np, "realtek,in1-differential"); rt5677->pdata.in2_diff = of_property_read_bool(np, @@ -4722,6 +4723,11 @@ static int rt5677_i2c_probe(struct i2c_client *i2c, if (ret != 0) dev_warn(&i2c->dev, "Failed to apply regmap patch: %d\n", ret);
+ regmap_update_bits(rt5677->regmap, RT5677_MICBIAS, + RT5677_MICBIAS1_OUTVOLT_MASK | + RT5677_MICBIAS1_CTRL_VDD_MASK, + rt5677->pdata.micbias1 << RT5677_MICBIAS1_CTRL_VDD_SFT); + if (rt5677->pdata.in1_diff) regmap_update_bits(rt5677->regmap, RT5677_IN1, RT5677_IN_DF1, RT5677_IN_DF1);
On Wed, Dec 10, 2014 at 08:15:26PM -0800, Ben Zhang wrote:
The MICBIAS voltage for IN1 can be set to 1.476V/2.970V/1.242V/2.475V
The changelog says "platform config" but this is adding DT binding.
+- realtek,micbias1
- Select 0/1/2/3 to set MICBIAS1 voltage to 1.476V/2.970V/1.242V/2.475V
Why is this being specified as some magic number rather than using the voltage (or at least providing defines for the voltage) - this is going to do little to make the DT legible and...
+enum rt5677_micbias {
- RT5677_MICBIAS_1_476V = 0,
- RT5677_MICBIAS_2_970V = 1,
- RT5677_MICBIAS_1_242V = 2,
- RT5677_MICBIAS_2_475V = 3,
+};
...I see there are defined for platform data.
On Fri, Dec 12, 2014 at 5:27 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Dec 10, 2014 at 08:15:26PM -0800, Ben Zhang wrote:
The MICBIAS voltage for IN1 can be set to 1.476V/2.970V/1.242V/2.475V
The changelog says "platform config" but this is adding DT binding.
+- realtek,micbias1
- Select 0/1/2/3 to set MICBIAS1 voltage to 1.476V/2.970V/1.242V/2.475V
Why is this being specified as some magic number rather than using the voltage (or at least providing defines for the voltage) - this is going to do little to make the DT legible and...
+enum rt5677_micbias {
RT5677_MICBIAS_1_476V = 0,RT5677_MICBIAS_2_970V = 1,RT5677_MICBIAS_1_242V = 2,RT5677_MICBIAS_2_475V = 3,+};
...I see there are defined for platform data.
This patch adds both an entry to the platform data and a DT binding for MICBIAS level selection. The 4 voltage options (1.476V/2.970V/1.242V/2.475V) are the only ones supported by the codec hardware, so it seems an enum is better than specifying the exact voltage directly. I was following the two examples below:
Documentation/devicetree/bindings/sound/cs42l52.txt (cirrus,micbias-lvl) Documentation/devicetree/bindings/sound/tlv320aic3x.txt (ai3x-micbias-vg)
I'm new to devicetree bindings. Is there something like an enum in DT?
On Fri, Dec 12, 2014 at 03:48:51PM -0800, Ben Zhang wrote:
On Fri, Dec 12, 2014 at 5:27 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Dec 10, 2014 at 08:15:26PM -0800, Ben Zhang wrote:
Why is this being specified as some magic number rather than using the voltage (or at least providing defines for the voltage) - this is going to do little to make the DT legible and...
+enum rt5677_micbias {
RT5677_MICBIAS_1_476V = 0,RT5677_MICBIAS_2_970V = 1,RT5677_MICBIAS_1_242V = 2,RT5677_MICBIAS_2_475V = 3,+};
...I see there are defined for platform data.
This patch adds both an entry to the platform data and a DT binding for MICBIAS level selection. The 4 voltage options (1.476V/2.970V/1.242V/2.475V) are the only ones supported by the codec hardware, so it seems an enum is better than specifying the exact
It's not that clear that an enum *is* better, and a magic numbers enum is definitely worse for anyone who has to read the resulting DT.
voltage directly. I was following the two examples below: Documentation/devicetree/bindings/sound/cs42l52.txt (cirrus,micbias-lvl) Documentation/devicetree/bindings/sound/tlv320aic3x.txt (ai3x-micbias-vg)
Old DT bindings are old and often not best practice.
I'm new to devicetree bindings. Is there something like an enum in DT?
include/dt-bindings
DACREF power source can come from external 1.8V or codec internal 1.8V. This patch adds the option to enable the internal DACREF power source.
Signed-off-by: Ben Zhang benzh@chromium.org --- Documentation/devicetree/bindings/sound/rt5677.txt | 3 +++ include/sound/rt5677.h | 2 ++ sound/soc/codecs/rt5677.c | 9 +++++++++ 3 files changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rt5677.txt b/Documentation/devicetree/bindings/sound/rt5677.txt index f54d0dd..f06d52a 100644 --- a/Documentation/devicetree/bindings/sound/rt5677.txt +++ b/Documentation/devicetree/bindings/sound/rt5677.txt @@ -22,6 +22,9 @@ Optional properties: - realtek,micbias1 Select 0/1/2/3 to set MICBIAS1 voltage to 1.476V/2.970V/1.242V/2.475V
+- realtek,internal-dacref-en + Select codec internal 1.8V as DACREF source optionally. + - realtek,in1-differential - realtek,in2-differential - realtek,lout1-differential diff --git a/include/sound/rt5677.h b/include/sound/rt5677.h index efa74bb..42866f3 100644 --- a/include/sound/rt5677.h +++ b/include/sound/rt5677.h @@ -28,6 +28,8 @@ enum rt5677_dmic2_clk { struct rt5677_platform_data { /* MICBIAS output voltage control */ enum rt5677_micbias micbias1; + /* Select codec internal 1.8V as DACREF source optionally */ + bool internal_dacref_en; /* IN1/IN2/LOUT1/LOUT2/LOUT3 can optionally be differential */ bool in1_diff; bool in2_diff; diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c index ac4bee8..e6d7bb4 100644 --- a/sound/soc/codecs/rt5677.c +++ b/sound/soc/codecs/rt5677.c @@ -4552,6 +4552,8 @@ MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id); static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np) { of_property_read_u32(np, "realtek,micbias1", &rt5677->pdata.micbias1); + rt5677->pdata.internal_dacref_en = of_property_read_bool(np, + "realtek,internal-dacref-en"); rt5677->pdata.in1_diff = of_property_read_bool(np, "realtek,in1-differential"); rt5677->pdata.in2_diff = of_property_read_bool(np, @@ -4728,6 +4730,13 @@ static int rt5677_i2c_probe(struct i2c_client *i2c, RT5677_MICBIAS1_CTRL_VDD_MASK, rt5677->pdata.micbias1 << RT5677_MICBIAS1_CTRL_VDD_SFT);
+ if (rt5677->pdata.internal_dacref_en) { + regmap_update_bits(rt5677->regmap, RT5677_PR_BASE + + RT5677_TEST_CTRL1, 1 << 9, 1 << 9); + regmap_update_bits(rt5677->regmap, RT5677_PR_BASE + + RT5677_SOFT_DEPOP_DAC_CLK_CTRL, 1 << 5, 1 << 5); + } + if (rt5677->pdata.in1_diff) regmap_update_bits(rt5677->regmap, RT5677_IN1, RT5677_IN_DF1, RT5677_IN_DF1);
On Wed, Dec 10, 2014 at 08:15:27PM -0800, Ben Zhang wrote:
DACREF power source can come from external 1.8V or codec internal 1.8V. This patch adds the option to enable the internal DACREF power source.
+- realtek,internal-dacref-en
- Select codec internal 1.8V as DACREF source optionally.
Why is this not done using the regulator API - don't we need to ensure that the external DACREF is enabled if it's there? I don't think we need to model the internal reference as a regulator (we can just set the registers up in that case) but if it's off device I'd expect to be making sure that it's turned on.
participants (3)
-
Ben Zhang -
Ben Zhang -
Mark Brown