ASoC: rt5640: add more settings for rt5639
Subsystems (4)
Oder Chiou
maintainerPATCH 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.
--- 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
@@ -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
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
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.
Oder Chiou
maintainerQuoted 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?
Mark Brown
maintainerDocumentation/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.