* [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage
2014-12-11 4:15 [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency Ben Zhang
@ 2014-12-11 4:15 ` Ben Zhang
2014-12-12 13:27 ` Mark Brown
2014-12-11 4:15 ` [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source Ben Zhang
2014-12-12 13:25 ` [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency Mark Brown
2 siblings, 1 reply; 8+ messages in thread
From: Ben Zhang @ 2014-12-11 4:15 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, linux-kernel, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage
2014-12-11 4:15 ` [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage Ben Zhang
@ 2014-12-12 13:27 ` Mark Brown
2014-12-12 23:48 ` Ben Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2014-12-12 13:27 UTC (permalink / raw)
To: Ben Zhang
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage
2014-12-12 13:27 ` Mark Brown
@ 2014-12-12 23:48 ` Ben Zhang
2014-12-15 12:20 ` Mark Brown
0 siblings, 1 reply; 8+ messages in thread
From: Ben Zhang @ 2014-12-12 23:48 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, Linux Kernel Mailing List
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?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage
2014-12-12 23:48 ` Ben Zhang
@ 2014-12-15 12:20 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-12-15 12:20 UTC (permalink / raw)
To: Ben Zhang
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, Linux Kernel Mailing List
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source
2014-12-11 4:15 [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency Ben Zhang
2014-12-11 4:15 ` [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage Ben Zhang
@ 2014-12-11 4:15 ` Ben Zhang
2014-12-12 13:34 ` Mark Brown
2014-12-12 13:25 ` [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency Mark Brown
2 siblings, 1 reply; 8+ messages in thread
From: Ben Zhang @ 2014-12-11 4:15 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, linux-kernel, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source
2014-12-11 4:15 ` [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source Ben Zhang
@ 2014-12-12 13:34 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-12-12 13:34 UTC (permalink / raw)
To: Ben Zhang
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency
2014-12-11 4:15 [PATCH 1/3] ASoC: rt5677: add REGMAP_I2C and REGMAP_IRQ dependency Ben Zhang
2014-12-11 4:15 ` [PATCH 2/3] ASoC: rt5677: add a platform config option for MICBIAS voltage Ben Zhang
2014-12-11 4:15 ` [PATCH 3/3] ASoC: rt5677: add a platform config option for DACREF source Ben Zhang
@ 2014-12-12 13:25 ` Mark Brown
2 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2014-12-12 13:25 UTC (permalink / raw)
To: Ben Zhang
Cc: alsa-devel, Liam Girdwood, Bard Liao, Oder Chiou, Anatol Pomozov,
Dylan Reid, Albert Chen, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread