Skip to contentSkip to conversation

ASoC: rt5640: add more settings for rt5639

PATCH In review Oder Chiou 3 participants 4 messages Apr 9, 2014 – Apr 10, 2014

Subsystems (4)
Device tree bindings Sound ASoC Realtek audio codecs
lore
Share & tools
lore

Oder Chiou

author maintainer
#0 lore raw

PATCH 1/1 ASoC: rt5640: add more settings for rt5639

The patch adds the the list of OF compatible strings, the binding document and the list of I2C device IDs for rt5639.

Signed-off-by Oder Chiou <[email protected]>
---
 Documentation/devicetree/bindings/sound/rt5639.txt |  48 ++++++++++
 sound/soc/codecs/rt5640.c                          | 103 ++++++++++++++++++++-
 2 files changed, 147 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rt5639.txt
Documentation/devicetree/bindings/sound/rt5639.txt new file +48 -0
new file mode 100644
index 0000000..d63c662
@@ -0,0 +1,48 @@
+RT5639 audio CODEC
+
+This device supports I2C only.
+
+Required properties:
+
+- compatible : "realtek,rt5639".
+
+- reg : The I2C address of the device.
+
+- interrupts : The CODEC's interrupt output.
+
+Optional properties:
+
+- realtek,in1-differential
+- realtek,in2-differential
+  Boolean. Indicate MIC1/2 input are differential, rather than single-ended.
+
+- realtek,ldo1-en-gpios : The GPIO that controls the CODEC's LDO1_EN pin.
+
+Pins on the device (for linking into audio routes):
+
+  * DMIC1
+  * DMIC2
+  * MICBIAS1
+  * IN1P
+  * IN1R
+  * IN2P
+  * IN2R
+  * HPOL
+  * HPOR
+  * LOUTL
+  * LOUTR
+  * SPOLP
+  * SPOLN
+  * SPORP
+  * SPORN
+
+Example:
+
+rt5639 {
+	compatible = "realtek,rt5639";
+	reg = <0x1c>;
+	interrupt-parent = <&gpio>;
+	interrupts = <TEGRA_GPIO(W, 3) GPIO_ACTIVE_HIGH>;
+	realtek,ldo1-en-gpios =
+		<&gpio TEGRA_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
+};
sound/soc/codecs/rt5640.c +99 -4

142 lines not on this page · raw message

Signature · 1 line
1.8.1.1.439.g50a6b54
#1 lore raw

Documentation/devicetree/bindings/sound/rt5639.txt

Quoted 5 lines · Oder Chiou

The patch adds the the list of OF compatible strings, the binding document and the list of I2C device IDs for rt5639.

Documentation/devicetree/bindings/sound/rt5639.txt new file +0 -0
new file mode 100644

Why not just add the new compatible value to the existing rt5640.txt?

Quoted 7 lines
Documentation/devicetree/bindings/sound/rt5639.txt new file +7 -0
+Pins on the device (for linking into audio routes):
+
+  * DMIC1
+  * DMIC2
+  * MICBIAS1
+  * IN1P
+  * IN1R

Documentation/devicetree/bindings/sound/rt5639.txt line 37

Quoted 10 lines
Documentation/devicetree/bindings/sound/rt5639.txt new file +10 -0
+  * IN2P
+  * IN2R
+  * HPOL
+  * HPOR
+  * LOUTL
+  * LOUTR
+  * SPOLP
+  * SPOLN
+  * SPORP
+  * SPORN

The one difference is that RT5640 supports MONOP/N, which you could handle by:

Pins on the device (for linking into audio routes) for RT5639/RT5640:

* DMIC
...
* SPORN

Additional pins on the device for RT5640:

* MONOP
* MONON

sound/soc/codecs/rt5640.c static const struct i2c_device_id rt5640_i2c_id[] = { · line 2216

Quoted 11 lines
sound/soc/codecs/rt5640.c +5 -0
@@ -2168,12 +2209,38 @@ static const struct i2c_device_id rt5640_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rt5640_i2c_id);
 
+static const struct i2c_device_id rt5639_i2c_id[] = {
+	{ "rt5639", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, rt5640_i2c_id);

I thought you could put all the values into a single table, along with some device-specific value which probe() could use to differentiate the two if needed?

sound/soc/codecs/rt5640.c static const struct i2c_device_id rt5640_i2c_id[] = { · line 2223

Quoted 6 lines
sound/soc/codecs/rt5640.c +6 -0
+#if defined(CONFIG_OF)
+static const struct of_device_id rt5640_of_match[] = {
+	{ .compatible = "realtek,rt5640", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt5640_of_match);

That's already part of the file.

sound/soc/codecs/rt5640.c static const struct i2c_device_id rt5640_i2c_id[] = { · line 2230

Quoted 6 lines
sound/soc/codecs/rt5640.c +6 -0
+static const struct of_device_id rt5639_of_match[] = {
+	{ .compatible = "realtek,rt5639", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt5639_of_match);
+#endif

Same for of_match; just use a single table with multiple entries.

sound/soc/codecs/rt5640.c static const struct i2c_device_id rt5640_i2c_id[] = { · line 2244

Quoted 6 lines
sound/soc/codecs/rt5640.c +5 -0
+static struct acpi_device_id rt5639_acpi_match[] = {
+	{ "INT33CA", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, rt5639_acpi_match);
 #endif

Same for ACPI, I hope.

sound/soc/codecs/rt5640.c static int rt5640_i2c_probe(struct i2c_client *i2c, · line 2365

Quoted 9 lines
sound/soc/codecs/rt5640.c +3 -2
@@ -2293,8 +2360,18 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
 
 	rt5640->hp_mute = 1;
 
-	ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_rt5640,
-			rt5640_dai, ARRAY_SIZE(rt5640_dai));
+	regmap_read(rt5640->regmap, RT5640_RESET, &val);
+	switch (val) {
+	case RT5640_RESET_ID:

It would be good to validate whether the RT5640_RESET register value matches the name in the i2c/of/ACPI device table that matched, and at least warn if they don't.

Quoted 13 lines
sound/soc/codecs/rt5640.c +7 -1
@@ -2315,6 +2392,9 @@ static struct i2c_driver rt5640_i2c_driver = {
 		.name = "rt5640",
 		.owner = THIS_MODULE,
 		.acpi_match_table = ACPI_PTR(rt5640_acpi_match),
+#if defined(CONFIG_OF)
+		.of_match_table = of_match_ptr(rt5640_of_match),
+#endif
 
-MODULE_DESCRIPTION("ASoC RT5640 driver");
+static struct i2c_driver rt5639_i2c_driver = {
+	.driver = {
+		.name = "rt5639",
+		.owner = THIS_MODULE,

sound/soc/codecs/rt5640.c static struct i2c_driver rt5640_i2c_driver = {

Quoted 10 lines
sound/soc/codecs/rt5640.c +10 -0
+		.acpi_match_table = ACPI_PTR(rt5639_acpi_match),
+#if defined(CONFIG_OF)
+		.of_match_table = of_match_ptr(rt5639_of_match),
+#endif
+	},
+	.probe = rt5640_i2c_probe,
+	.remove   = rt5640_i2c_remove,
+	.id_table = rt5639_i2c_id,
+};
+module_i2c_driver(rt5639_i2c_driver);

I don't think you need a separate i2c_driver structure; just make the existing one handle multiple devices.

author maintainer
#2 lore raw
Quoted 8 lines

sound/soc/codecs/rt5640.c static const struct i2c_device_id rt5640_i2c_id[] = { · line 2223

Quoted 6 lines
sound/soc/codecs/rt5640.c +6 -0
+#if defined(CONFIG_OF)
+static const struct of_device_id rt5640_of_match[] = {
+	{ .compatible = "realtek,rt5640", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt5640_of_match);

That's already part of the file.

The patch will apply in the branch "origin/topic/rt5640" that is not the latest version of rt5640.c, so there is no the of_match_table of rt5640 in it. How could I handle the situation?

maintainer
#3 lore raw
In reply to Oder Chiou

Documentation/devicetree/bindings/sound/rt5639.txt line 1

Quoted 5 lines · Oder Chiou

index 0000000..d63c662

Documentation/devicetree/bindings/sound/rt5639.txt new file +1 -0
@@ -0,0 +1,48 @@
+RT5639 audio CODEC

This doesn't really need to be a separate document, just add the information to the rt5640 document.

Documentation/devicetree/bindings/sound/rt5639.txt line 14

Quoted 13 lines
Documentation/devicetree/bindings/sound/rt5639.txt new file +13 -0
+#if defined(CONFIG_OF)
+static const struct of_device_id rt5640_of_match[] = {
+	{ .compatible = "realtek,rt5640", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt5640_of_match);
+
+static const struct of_device_id rt5639_of_match[] = {
+	{ .compatible = "realtek,rt5639", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rt5639_of_match);
+#endif

This looks broken - you should have one driver with one table.

Documentation/devicetree/bindings/sound/rt5639.txt line 17

Quoted 3 lines
Documentation/devicetree/bindings/sound/rt5639.txt new file +3 -0
+#if defined(CONFIG_OF)
+		.of_match_table = of_match_ptr(rt5640_of_match),
+#endif

Drop the ifdef, the whole point with of_match_ptr() is that you don't need it.

Documentation/devicetree/bindings/sound/rt5639.txt line 21

Quoted 4 lines
Documentation/devicetree/bindings/sound/rt5639.txt new file +4 -0
+static struct i2c_driver rt5639_i2c_driver = {
+	.driver = {
+		.name = "rt5639",
+		.owner = THIS_MODULE,

No, add an ID table to the I2C driver - look at wm8904 for an example.