[PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency

[PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency

[Ben Zhang]

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
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage

[Ben Zhang]

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);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source

[Ben Zhang]

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);
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

On Wed, Dec 10, 2014 at 08:15:25PM -0800, Ben Zhang wrote:
> The codec driver uses regmap to do i2c read/write.
> The codec driver started to use REGMAP_IRQ since:

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage

[Ben Zhang]

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?

Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]