[alsa-devel] [PATCH 00/11] MFD/ASoC/Input: twl4030-audio submodule DT support
Hello,
The following series adds DT support for the twl4030 audio submodule which provides audio codec and vibra functionality.
The MFD core driver is probed via DT, it will create the needed child devices based on the provided information in the DT blob. Child drivers (vibra, ASoC codec) will parse the core's node if needed to get the needed parameters for their configuration.
In the ASoC codec driver the hs_extmute callback (which was used to toggle a GPIO line) has been removed. The codec driver will receive the GPIO number (if it is needed on the platform) and also the level to disable the mute.
If the series is OK (and no objections from the maintainers), it would be good if this can go via audio. Changed files are well contained within the twl4030-audio stack so I do not expect merge issues later.
The series has been tested on BeagleBoard (with the McBSP DT series, and with the upcoming DT audio support for BeagleBoard).
Regards, Peter --- Peter Ujfalusi (11): MFD: twl4030-audio: Clean up MODULE_* and platform_driver part MFD: twl4030-audio: Convert to use devm_kzalloc MFD: twl4030-audio: Rearange and clean-up the probe function MFD: twl4030-audio: Add DT support Input: twl4030-vibra: Support for DT booted kernel ASoC: twl4030: Move hs_extmute GPIO handling to driver ARM: OMAP/ASoC: Zoom2: Let the codec to handle the hs_extmute GPIO ASoC/MFD: twl4030: Remove set_hs_extmute callback from platform data ASoC: twl4030: Convert to use devm_kzalloc ASoC: twl4030: Add pointer to pdata within the private data ASoC: twl4030: Support for DT booted kernel
.../devicetree/bindings/mfd/twl4030-audio.txt | 51 +++++++++ arch/arm/mach-omap2/board-zoom-peripherals.c | 9 +- arch/arm/mach-omap2/include/mach/board-zoom.h | 2 - drivers/input/misc/twl4030-vibra.c | 20 +++- drivers/mfd/twl4030-audio.c | 114 +++++++++++++------ include/linux/i2c/twl.h | 4 +- sound/soc/codecs/twl4030.c | 117 ++++++++++++++++---- sound/soc/omap/zoom2.c | 4 - 8 files changed, 250 insertions(+), 71 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/twl4030-audio.txt
Place the MODULE_* lines in the same block and add MODULE_DESCRIPTION. Rearange the platform_driver structure at the same time.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl4030-audio.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/twl4030-audio.c b/drivers/mfd/twl4030-audio.c index 838ce4e..ac04b4f 100644 --- a/drivers/mfd/twl4030-audio.c +++ b/drivers/mfd/twl4030-audio.c @@ -250,18 +250,18 @@ static int __devexit twl4030_audio_remove(struct platform_device *pdev) return 0; }
-MODULE_ALIAS("platform:twl4030-audio"); - static struct platform_driver twl4030_audio_driver = { - .probe = twl4030_audio_probe, - .remove = __devexit_p(twl4030_audio_remove), .driver = { .owner = THIS_MODULE, .name = "twl4030-audio", }, + .probe = twl4030_audio_probe, + .remove = __devexit_p(twl4030_audio_remove), };
module_platform_driver(twl4030_audio_driver);
MODULE_AUTHOR("Peter Ujfalusi peter.ujfalusi@ti.com"); +MODULE_DESCRIPTION("TWL4030 audio block MFD driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:twl4030-audio");
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl4030-audio.c | 15 ++++++--------- 1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/twl4030-audio.c b/drivers/mfd/twl4030-audio.c index ac04b4f..efa2d42 100644 --- a/drivers/mfd/twl4030-audio.c +++ b/drivers/mfd/twl4030-audio.c @@ -188,7 +188,8 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) twl_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, val, TWL4030_REG_APLL_CTL);
- audio = kzalloc(sizeof(struct twl4030_audio), GFP_KERNEL); + audio = devm_kzalloc(&pdev->dev, sizeof(struct twl4030_audio), + GFP_KERNEL); if (!audio) return -ENOMEM;
@@ -229,22 +230,18 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) ret = -ENODEV; }
- if (!ret) - return 0; + if (ret) { + platform_set_drvdata(pdev, NULL); + twl4030_audio_dev = NULL; + }
- platform_set_drvdata(pdev, NULL); - kfree(audio); - twl4030_audio_dev = NULL; return ret; }
static int __devexit twl4030_audio_remove(struct platform_device *pdev) { - struct twl4030_audio *audio = platform_get_drvdata(pdev); - mfd_remove_devices(&pdev->dev); platform_set_drvdata(pdev, NULL); - kfree(audio); twl4030_audio_dev = NULL;
return 0;
To facilitate the device tree support the probe function need to be rearanged. Small cleanup in the APLL frequency selection part as well.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl4030-audio.c | 34 ++++++++++++++++------------------ 1 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/mfd/twl4030-audio.c b/drivers/mfd/twl4030-audio.c index efa2d42..ca2d669 100644 --- a/drivers/mfd/twl4030-audio.c +++ b/drivers/mfd/twl4030-audio.c @@ -169,35 +169,30 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) return -EINVAL; }
+ audio = devm_kzalloc(&pdev->dev, sizeof(struct twl4030_audio), + GFP_KERNEL); + if (!audio) + return -ENOMEM; + + mutex_init(&audio->mutex); + audio->audio_mclk = pdata->audio_mclk; + /* Configure APLL_INFREQ and disable APLL if enabled */ - val = 0; - switch (pdata->audio_mclk) { + switch (audio->audio_mclk) { case 19200000: - val |= TWL4030_APLL_INFREQ_19200KHZ; + val = TWL4030_APLL_INFREQ_19200KHZ; break; case 26000000: - val |= TWL4030_APLL_INFREQ_26000KHZ; + val = TWL4030_APLL_INFREQ_26000KHZ; break; case 38400000: - val |= TWL4030_APLL_INFREQ_38400KHZ; + val = TWL4030_APLL_INFREQ_38400KHZ; break; default: dev_err(&pdev->dev, "Invalid audio_mclk\n"); return -EINVAL; } - twl_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - val, TWL4030_REG_APLL_CTL); - - audio = devm_kzalloc(&pdev->dev, sizeof(struct twl4030_audio), - GFP_KERNEL); - if (!audio) - return -ENOMEM; - - platform_set_drvdata(pdev, audio); - - twl4030_audio_dev = pdev; - mutex_init(&audio->mutex); - audio->audio_mclk = pdata->audio_mclk; + twl_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, val, TWL4030_REG_APLL_CTL);
/* Codec power */ audio->resource[TWL4030_AUDIO_RES_POWER].reg = TWL4030_REG_CODEC_MODE; @@ -222,6 +217,9 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) childs++; }
+ platform_set_drvdata(pdev, audio); + twl4030_audio_dev = pdev; + if (childs) ret = mfd_add_devices(&pdev->dev, pdev->id, audio->cells, childs, NULL, 0);
Support for loading the twl4030 audio module via devicetree. Sub devices for codec and vibra will be created as mfd devices once the core MFD driver is loaded when the kernel is booted with a DT blob.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- .../devicetree/bindings/mfd/twl4030-audio.txt | 51 +++++++++++++++ drivers/mfd/twl4030-audio.c | 65 +++++++++++++++++--- 2 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/twl4030-audio.txt
diff --git a/Documentation/devicetree/bindings/mfd/twl4030-audio.txt b/Documentation/devicetree/bindings/mfd/twl4030-audio.txt new file mode 100644 index 0000000..bc45025 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/twl4030-audio.txt @@ -0,0 +1,51 @@ +Texas Instruments TWL family (twl4030) audio module + +The audio module inside the TWL family consist of an audio codec and a vibra +driver. + +Required properties: +- compatible : must be "ti,twl4030-audio" +- ti,audio_mclk: Master clock frequency used on the board. Supported + frequencies: 19200000, 26000000 and 38400000 + +Optional properties, nodes: + +Audio functionality: +- codec { }: Need to be present if the audio functionality is used. Within this + section the following options can be used: +- ti,digimic_delay: Delay need after enabling the digimic to reduce artifacts + from the start of the recorded sample (in ms) +-ti,ramp_delay_value: HS ramp delay configuration to reduce pop noise +-ti,hs_extmute: Use external mute for HS pop reduction +-ti,hs_extmute_gpio: Use external GPIO to control the external mute +-ti,hs_extmute_disable_level: The desired level of the GPIO line when the + external mute is disabled. valuse: 0 or 1 +-ti,offset_cncl_path: Offset cancellation path selection, refer to TRM for the + valid values. + +Vibra functionality +- ti,enable-vibra: Need to be set to <1> if the vibra functionality is used. if + missing or it is 0, the vibra functionality is disabled. + +Example: +&i2c1 { + clock-frequency = <2600000>; + + twl: twl@48 { + reg = <0x48>; + interrupts = <7>; /* SYS_NIRQ cascaded to intc */ + interrupt-parent = <&intc>; + + twl_audio: audio { + compatible = "ti,twl4030-audio"; + ti,audio_mclk = <26000000>; + + ti,enable-vibra = <1>; + + codec { + ti,ramp_delay_value = <3>; + }; + + }; + }; +}; diff --git a/drivers/mfd/twl4030-audio.c b/drivers/mfd/twl4030-audio.c index ca2d669..a4df8b3a 100644 --- a/drivers/mfd/twl4030-audio.c +++ b/drivers/mfd/twl4030-audio.c @@ -28,6 +28,8 @@ #include <linux/kernel.h> #include <linux/fs.h> #include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_platform.h> #include <linux/i2c/twl.h> #include <linux/mfd/core.h> #include <linux/mfd/twl4030-audio.h> @@ -156,15 +158,46 @@ unsigned int twl4030_audio_get_mclk(void) } EXPORT_SYMBOL_GPL(twl4030_audio_get_mclk);
+static bool twl4030_audio_has_codec(struct twl4030_audio_data *pdata, + struct device_node *node) +{ + if (pdata && pdata->codec) + return true; + +#ifdef CONFIG_OF + if (of_find_node_by_name(node, "codec")) + return true; +#endif + + return false; +} + +static bool twl4030_audio_has_vibra(struct twl4030_audio_data *pdata, + struct device_node *node) +{ + int vibra; + + if (pdata && pdata->vibra) + return true; + +#ifdef CONFIG_OF + if (!of_property_read_u32(node, "ti,enable-vibra", &vibra) && vibra) + return true; +#endif + + return false; +} + static int __devinit twl4030_audio_probe(struct platform_device *pdev) { struct twl4030_audio *audio; struct twl4030_audio_data *pdata = pdev->dev.platform_data; + struct device_node *node = pdev->dev.of_node; struct mfd_cell *cell = NULL; int ret, childs = 0; u8 val;
- if (!pdata) { + if (!pdata && !node) { dev_err(&pdev->dev, "Platform data is missing\n"); return -EINVAL; } @@ -175,7 +208,12 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) return -ENOMEM;
mutex_init(&audio->mutex); - audio->audio_mclk = pdata->audio_mclk; + if (pdata) + audio->audio_mclk = pdata->audio_mclk; +#ifdef CONFIG_OF + else + of_property_read_u32(node, "ti,audio_mclk", &audio->audio_mclk); +#endif
/* Configure APLL_INFREQ and disable APLL if enabled */ switch (audio->audio_mclk) { @@ -202,18 +240,22 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) audio->resource[TWL4030_AUDIO_RES_APLL].reg = TWL4030_REG_APLL_CTL; audio->resource[TWL4030_AUDIO_RES_APLL].mask = TWL4030_APLL_EN;
- if (pdata->codec) { + if (twl4030_audio_has_codec(pdata, node)) { cell = &audio->cells[childs]; cell->name = "twl4030-codec"; - cell->platform_data = pdata->codec; - cell->pdata_size = sizeof(*pdata->codec); + if (pdata) { + cell->platform_data = pdata->codec; + cell->pdata_size = sizeof(*pdata->codec); + } childs++; } - if (pdata->vibra) { + if (twl4030_audio_has_vibra(pdata, node)) { cell = &audio->cells[childs]; cell->name = "twl4030-vibra"; - cell->platform_data = pdata->vibra; - cell->pdata_size = sizeof(*pdata->vibra); + if (pdata) { + cell->platform_data = pdata->vibra; + cell->pdata_size = sizeof(*pdata->vibra); + } childs++; }
@@ -245,10 +287,17 @@ static int __devexit twl4030_audio_remove(struct platform_device *pdev) return 0; }
+static const struct of_device_id twl4030_audio_of_match[] = { + {.compatible = "ti,twl4030-audio", }, + { }, +}; +MODULE_DEVICE_TABLE(of, twl4030_audio_of_match); + static struct platform_driver twl4030_audio_driver = { .driver = { .owner = THIS_MODULE, .name = "twl4030-audio", + .of_match_table = twl4030_audio_of_match, }, .probe = twl4030_audio_probe, .remove = __devexit_p(twl4030_audio_remove),
Hi Peter,
On 08/08/2012 11:41 AM, Peter Ujfalusi wrote:
Support for loading the twl4030 audio module via devicetree. Sub devices for codec and vibra will be created as mfd devices once the core MFD driver is loaded when the kernel is booted with a DT blob.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
.../devicetree/bindings/mfd/twl4030-audio.txt | 51 +++++++++++++++ drivers/mfd/twl4030-audio.c | 65 +++++++++++++++++--- 2 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/twl4030-audio.txt
diff --git a/Documentation/devicetree/bindings/mfd/twl4030-audio.txt b/Documentation/devicetree/bindings/mfd/twl4030-audio.txt new file mode 100644 index 0000000..bc45025 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/twl4030-audio.txt @@ -0,0 +1,51 @@ +Texas Instruments TWL family (twl4030) audio module
+The audio module inside the TWL family consist of an audio codec and a vibra +driver.
+Required properties: +- compatible : must be "ti,twl4030-audio" +- ti,audio_mclk: Master clock frequency used on the board. Supported
frequencies: 19200000, 26000000 and 38400000
Since you care about the frequency only you can use the generic attribute: clock-frequency (in your example, I2C is using it).
That being said, since we do have the clock binding now, maybe we should create a clock provider at board level for the sys_clk and thus you will just have to create a phandle to it in your audio description.
In theory OMAP and other components will thus have to use that sys_clock reference as well.
Regards, Benoit
+Optional properties, nodes:
+Audio functionality: +- codec { }: Need to be present if the audio functionality is used. Within this
section the following options can be used:+- ti,digimic_delay: Delay need after enabling the digimic to reduce artifacts
from the start of the recorded sample (in ms)+-ti,ramp_delay_value: HS ramp delay configuration to reduce pop noise +-ti,hs_extmute: Use external mute for HS pop reduction +-ti,hs_extmute_gpio: Use external GPIO to control the external mute +-ti,hs_extmute_disable_level: The desired level of the GPIO line when the
external mute is disabled. valuse: 0 or 1+-ti,offset_cncl_path: Offset cancellation path selection, refer to TRM for the
valid values.+Vibra functionality +- ti,enable-vibra: Need to be set to <1> if the vibra functionality is used. if
missing or it is 0, the vibra functionality is disabled.+Example: +&i2c1 {
- clock-frequency = <2600000>;
- twl: twl@48 {
reg = <0x48>;interrupts = <7>; /* SYS_NIRQ cascaded to intc */interrupt-parent = <&intc>;twl_audio: audio {compatible = "ti,twl4030-audio";ti,audio_mclk = <26000000>;ti,enable-vibra = <1>;codec {ti,ramp_delay_value = <3>;};};- };
+}; diff --git a/drivers/mfd/twl4030-audio.c b/drivers/mfd/twl4030-audio.c index ca2d669..a4df8b3a 100644 --- a/drivers/mfd/twl4030-audio.c +++ b/drivers/mfd/twl4030-audio.c @@ -28,6 +28,8 @@ #include <linux/kernel.h> #include <linux/fs.h> #include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_platform.h> #include <linux/i2c/twl.h> #include <linux/mfd/core.h> #include <linux/mfd/twl4030-audio.h> @@ -156,15 +158,46 @@ unsigned int twl4030_audio_get_mclk(void) } EXPORT_SYMBOL_GPL(twl4030_audio_get_mclk);
+static bool twl4030_audio_has_codec(struct twl4030_audio_data *pdata,
struct device_node *node)+{
- if (pdata && pdata->codec)
return true;+#ifdef CONFIG_OF
- if (of_find_node_by_name(node, "codec"))
return true;+#endif
- return false;
+}
+static bool twl4030_audio_has_vibra(struct twl4030_audio_data *pdata,
struct device_node *node)+{
- int vibra;
- if (pdata && pdata->vibra)
return true;+#ifdef CONFIG_OF
- if (!of_property_read_u32(node, "ti,enable-vibra", &vibra) && vibra)
return true;+#endif
- return false;
+}
static int __devinit twl4030_audio_probe(struct platform_device *pdev) { struct twl4030_audio *audio; struct twl4030_audio_data *pdata = pdev->dev.platform_data;
- struct device_node *node = pdev->dev.of_node; struct mfd_cell *cell = NULL; int ret, childs = 0; u8 val;
- if (!pdata) {
- if (!pdata && !node) { dev_err(&pdev->dev, "Platform data is missing\n"); return -EINVAL; }
@@ -175,7 +208,12 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) return -ENOMEM;
mutex_init(&audio->mutex);
- audio->audio_mclk = pdata->audio_mclk;
- if (pdata)
audio->audio_mclk = pdata->audio_mclk;+#ifdef CONFIG_OF
- else
of_property_read_u32(node, "ti,audio_mclk", &audio->audio_mclk);+#endif
/* Configure APLL_INFREQ and disable APLL if enabled */ switch (audio->audio_mclk) { @@ -202,18 +240,22 @@ static int __devinit twl4030_audio_probe(struct platform_device *pdev) audio->resource[TWL4030_AUDIO_RES_APLL].reg = TWL4030_REG_APLL_CTL; audio->resource[TWL4030_AUDIO_RES_APLL].mask = TWL4030_APLL_EN;
- if (pdata->codec) {
- if (twl4030_audio_has_codec(pdata, node)) { cell = &audio->cells[childs]; cell->name = "twl4030-codec";
cell->platform_data = pdata->codec;cell->pdata_size = sizeof(*pdata->codec);
if (pdata) {cell->platform_data = pdata->codec;cell->pdata_size = sizeof(*pdata->codec);}
- if (pdata->vibra) {
- if (twl4030_audio_has_vibra(pdata, node)) { cell = &audio->cells[childs]; cell->name = "twl4030-vibra";
cell->platform_data = pdata->vibra;cell->pdata_size = sizeof(*pdata->vibra);
if (pdata) {cell->platform_data = pdata->vibra;cell->pdata_size = sizeof(*pdata->vibra);}@@ -245,10 +287,17 @@ static int __devexit twl4030_audio_remove(struct platform_device *pdev) return 0; }
+static const struct of_device_id twl4030_audio_of_match[] = {
- {.compatible = "ti,twl4030-audio", },
- { },
+}; +MODULE_DEVICE_TABLE(of, twl4030_audio_of_match);
static struct platform_driver twl4030_audio_driver = { .driver = { .owner = THIS_MODULE, .name = "twl4030-audio",
.of_match_table = twl4030_audio_of_match,
Hi Benoit,
On 08/08/2012 02:50 PM, Benoit Cousson wrote:
+Required properties: +- compatible : must be "ti,twl4030-audio" +- ti,audio_mclk: Master clock frequency used on the board. Supported
frequencies: 19200000, 26000000 and 38400000Since you care about the frequency only you can use the generic attribute: clock-frequency (in your example, I2C is using it).
I see, I'll use replace the property with clock-frequency.
That being said, since we do have the clock binding now, maybe we should create a clock provider at board level for the sys_clk and thus you will just have to create a phandle to it in your audio description.
In theory OMAP and other components will thus have to use that sys_clock reference as well.
And in the driver I would need to do for example: clkp = clk_get(NULL, "audio_mclk"); clk_rate = clk_get_rate(clkp); clk_put(clkp);
to get the frequency in this way? Note that the twl4030-audio stack itself does not handle clocks at the moment since the clock needed for the audio module is always running (HFCLKIN -> HFCLKOUT -> AUDIO_MCLK (USB_MCLK)). All of this is within twl4030. Do we really want to model this?
On Wed, Aug 08, 2012 at 01:50:53PM +0200, Benoit Cousson wrote:
That being said, since we do have the clock binding now, maybe we should create a clock provider at board level for the sys_clk and thus you will just have to create a phandle to it in your audio description.
Yes, we should be doing this.
On 08/08/2012 03:52 PM, Mark Brown wrote:
On Wed, Aug 08, 2012 at 01:50:53PM +0200, Benoit Cousson wrote:
That being said, since we do have the clock binding now, maybe we should create a clock provider at board level for the sys_clk and thus you will just have to create a phandle to it in your audio description.
Yes, we should be doing this.
Does this needs the common clock framework on the platform? OMAP3 certainly does not have it enabled since it has not been adopted.
On Wed, Aug 08, 2012 at 05:35:34PM +0300, Peter Ujfalusi wrote:
Does this needs the common clock framework on the platform? OMAP3 certainly does not have it enabled since it has not been adopted.
I'd hope at least the binding code for finding clocks has been done at the clkdev level so it can be used on other platforms but perhaps that's not the case.
On 08/08/2012 04:35 PM, Peter Ujfalusi wrote:
On 08/08/2012 03:52 PM, Mark Brown wrote:
On Wed, Aug 08, 2012 at 01:50:53PM +0200, Benoit Cousson wrote:
That being said, since we do have the clock binding now, maybe we should create a clock provider at board level for the sys_clk and thus you will just have to create a phandle to it in your audio description.
Yes, we should be doing this.
Does this needs the common clock framework on the platform? OMAP3 certainly does not have it enabled since it has not been adopted.
Yeah, good point, we are still not there yet. Rajendra is still working on it and I hope we can get something for 3.7, but let's avoid having a strong dependency on that migration for the moment.
Regards, Benoit
On Wed, Aug 08, 2012 at 12:41:20PM +0300, Peter Ujfalusi wrote:
+Required properties: +- compatible : must be "ti,twl4030-audio"
So, as I mentioned before I find this sort of direct mapping of the Linux device representation into the device tree really troubling. I'm just way too aware of the fact that even the Linux split of these things can change over time and often represents internal Linux considerations.
+-ti,hs_extmute: Use external mute for HS pop reduction +-ti,hs_extmute_gpio: Use external GPIO to control the external mute +-ti,hs_extmute_disable_level: The desired level of the GPIO line when the
external mute is disabled. valuse: 0 or 1
This doesn't seem like something that should be in the CODEC driver really, there's a general need for something which can unmute controls at the end of the power up sequence and mute before power down. Also, if this is going to be part of the binding shouldn't we just omit the first property and simply check for the presence of the property which specifies the GPIO?
+#ifdef CONFIG_OF
- if (of_find_node_by_name(node, "codec"))
return true;+#endif
This really seems like we should be stubbing out of_find_node_by_name() to return false in non-OF cases.
+#ifdef CONFIG_OF
- if (!of_property_read_u32(node, "ti,enable-vibra", &vibra) && vibra)
return true;+#endif
Similarly here.
On 08/08/2012 04:13 PM, Mark Brown wrote:
On Wed, Aug 08, 2012 at 12:41:20PM +0300, Peter Ujfalusi wrote:
+Required properties: +- compatible : must be "ti,twl4030-audio"
So, as I mentioned before I find this sort of direct mapping of the Linux device representation into the device tree really troubling. I'm just way too aware of the fact that even the Linux split of these things can change over time and often represents internal Linux considerations.
Yes, I remember. This is however a bit different case compared to the twl6040. The twl4030 series of PMIC includes the following parts: twl4030, twl5030, twl5031, tps65950, tps65930 - contains audio block tps65920, tps65921 - does not contain audio block
So the audio block within these PMICs are really a block, it is present on some PMIC and it is missing on other's.
This is what we discussed regarding to twl6040. I only expose the audio block and not the Linux implementation (that we have separate driver for vibra and codec sub functionality).
+-ti,hs_extmute: Use external mute for HS pop reduction +-ti,hs_extmute_gpio: Use external GPIO to control the external mute +-ti,hs_extmute_disable_level: The desired level of the GPIO line when the
external mute is disabled. valuse: 0 or 1This doesn't seem like something that should be in the CODEC driver really, there's a general need for something which can unmute controls at the end of the power up sequence and mute before power down. Also, if this is going to be part of the binding shouldn't we just omit the first property and simply check for the presence of the property which specifies the GPIO?
The hs_extmute is used for reducing pop noise on the headset. The sequence is described in the TRM and it need to be done within the codec driver since it is within the sequence. Now there's two ways for boards to implement it: using the dedicated pin from twl4030 PMIC (MUTE/GPIO6) - if only "ti,hs_extmute" present we use this mode. Some boards are using the GPIO6 from twl4030 for other purposes, in this case an external GPIO is used for the same purpose - thus we have ti,hs_extmute_gpio and ti,hs_extmute_disable_level to tell the driver this. As for the ti,hs_extmute_disable_level: some boards chosen that the mute is disabled when the signal is high on the GPIO (don't ask me why). This property tells this.
All of this has to be in the driver due to the sequencing requirements.
+#ifdef CONFIG_OF
- if (of_find_node_by_name(node, "codec"))
return true;+#endif
This really seems like we should be stubbing out of_find_node_by_name() to return false in non-OF cases.
Yes, most likely the of_find_node_by_name() deserves the same treatment as some selected of_* function in case CONFIG_OF is not selected. But at the moment this is not the case, we need to protect with ifdef since we might break other randconfigs where CONFIG_OF is not set.
+#ifdef CONFIG_OF
- if (!of_property_read_u32(node, "ti,enable-vibra", &vibra) && vibra)
return true;+#endif
Similarly here.
Here the ifdef is no needed. of_property_read_u32() return -ENOSYS in case CONFIG_OF is not selected.
On Wed, Aug 08, 2012 at 04:43:16PM +0300, Peter Ujfalusi wrote:
On 08/08/2012 04:13 PM, Mark Brown wrote:
This doesn't seem like something that should be in the CODEC driver really, there's a general need for something which can unmute controls at the end of the power up sequence and mute before power down. Also, if this is going to be part of the binding shouldn't we just omit the first property and simply check for the presence of the property which specifies the GPIO?
The hs_extmute is used for reducing pop noise on the headset. The sequence is described in the TRM and it need to be done within the codec driver since it is within the sequence.
Is the point it's at in the sequence the end?
+#ifdef CONFIG_OF
- if (of_find_node_by_name(node, "codec"))
return true;+#endif
This really seems like we should be stubbing out of_find_node_by_name() to return false in non-OF cases.
Yes, most likely the of_find_node_by_name() deserves the same treatment as some selected of_* function in case CONFIG_OF is not selected. But at the moment this is not the case, we need to protect with ifdef since we might break other randconfigs where CONFIG_OF is not set.
We can always add a patch stubbing it out as part of the series, or in a branch in the OF tree that we can pull in.
+#ifdef CONFIG_OF
- if (!of_property_read_u32(node, "ti,enable-vibra", &vibra) && vibra)
return true;+#endif
Similarly here.
Here the ifdef is no needed. of_property_read_u32() return -ENOSYS in case CONFIG_OF is not selected.
-- Péter
On 08/08/2012 04:52 PM, Mark Brown wrote:
The hs_extmute is used for reducing pop noise on the headset. The sequence is described in the TRM and it need to be done within the codec driver since it is within the sequence.
Is the point it's at in the sequence the end?
On both ends :o On HS enable: the extmute has to be enabled, run the power on sequence for the HS, wait for ramp, disable the HS extmute.
On stop: enable the extmute, run the sequence, wait for ramp, release the extmute.
We can always add a patch stubbing it out as part of the series, or in a branch in the OF tree that we can pull in.
If you are OK with the hassle I can add a patch to the series for include/linux/of.h to handle this.
On Wed, Aug 08, 2012 at 05:16:58PM +0300, Peter Ujfalusi wrote:
On 08/08/2012 04:52 PM, Mark Brown wrote:
Is the point it's at in the sequence the end?
On both ends :o On HS enable: the extmute has to be enabled, run the power on sequence for the HS, wait for ramp, disable the HS extmute.
On stop: enable the extmute, run the sequence, wait for ramp, release the extmute.
I'm struggling to see any reason not to just hold the external mute on for the entire time the headset is powered off?
We can always add a patch stubbing it out as part of the series, or in a branch in the OF tree that we can pull in.
If you are OK with the hassle I can add a patch to the series for include/linux/of.h to handle this.
Yes, please.
On 08/08/2012 05:18 PM, Mark Brown wrote:
I'm struggling to see any reason not to just hold the external mute on for the entire time the headset is powered off?
I think because it would mean in most cases that we keep a GPIO line enabled during the time the HS is not in use, which burns power. In most cases the mute is enabled when the GPIO line is up.
If you are OK with the hassle I can add a patch to the series for include/linux/of.h to handle this.
Yes, please.
I'll wait for more comments and I'll include it to v2.
On Wed, Aug 08, 2012 at 05:31:19PM +0300, Peter Ujfalusi wrote:
On 08/08/2012 05:18 PM, Mark Brown wrote:
I'm struggling to see any reason not to just hold the external mute on for the entire time the headset is powered off?
I think because it would mean in most cases that we keep a GPIO line enabled during the time the HS is not in use, which burns power. In most cases the mute is enabled when the GPIO line is up.
That makes sense if the GPIO is actively driven, open drain should be better here, but it's still a generic thing which it'd be nice to extract.
Still, the main thing here was the fact that we should be able to drop the flag for the feature in the first place.
On 08/08/2012 05:49 PM, Mark Brown wrote:
That makes sense if the GPIO is actively driven, open drain should be better here, but it's still a generic thing which it'd be nice to extract.
Yes, the idea in it's core is generic, but right now I can not think of a generic implementation which would work. In case of twl4030 this is how it looks like:
1. if extmute is requested 1.1. if GPIO number is not valid Use the internal MUTE functionality (HS_POPN_SET register, EXTMUTE bit) 1.2. if GPIO number is valid set the GPIO to 1 2. Execute the sequence for the power up/down 3. if extmute is requested 3.1. if GPIO number is not valid Use the internal MUTE functionality (HS_POPN_SET register, EXTMUTE bit) 3.2. if GPIO number is valid set the GPIO to 0
For other devices it could be simpler: 1. Set GPIO for mute 2. Toggle a bit in the codec 3. Clear the GPIO to unmute
another device would need a sequence in step 2 instead of a single bit change.
To cover all of this in a generic way is not that straight forward IMHO.
Still, the main thing here was the fact that we should be able to drop the flag for the feature in the first place.
Which flag you are referring? I can (and have dropped) the hs_extmute_disable_level flag, but separate hs_extmute, hs_extmute_gpio is needed for this device.
hs_extmute tells that the board implemented some sort of HS extmute functionality. hs_extmute_gpio tells if the board is _not_ using the built in functionality, but it is using external GPIO line to enable/disable the mute.
Sure I could do this: hs_extmute: if only this is set we shall use the chip built in functionality hs_extmute_gpio: if this is set we use the extmute feature but with external GPIO.
But both need to be documented and supported.
On Thu, Aug 09, 2012 at 01:18:50PM +0300, Peter Ujfalusi wrote:
On 08/08/2012 05:49 PM, Mark Brown wrote:
That makes sense if the GPIO is actively driven, open drain should be better here, but it's still a generic thing which it'd be nice to extract.
To cover all of this in a generic way is not that straight forward IMHO.
The sequence is just:
1. Enable mutes (at _PRE time) 2. Do whatever the device needs 3. Disable the mutes (at _POST time)
I'm not sure there's any reason for you not to use the internal mute even if the external mute is present but if there is that's the only thing that's weird here. If there's no reason not to do it it just goes into step 2 and then it's fine, even if there is you can make it conditional in step 2.
Sure I could do this: hs_extmute: if only this is set we shall use the chip built in functionality hs_extmute_gpio: if this is set we use the extmute feature but with external GPIO.
But both need to be documented and supported.
Is there any actual case where an external mute is supplied via a mechanism other than a GPIO, and if there is would it not either need its own DT property or already need to interact with the driver from code, making the DT property redundant? My thinking here is that the flag should be redundant because we already need to specify how we do the mute, what I'd expect is that we activate the external mute functionality as a result of being given another way of doing it so we don't need to provide a flag.
On 08/09/2012 01:36 PM, Mark Brown wrote:
On Thu, Aug 09, 2012 at 01:18:50PM +0300, Peter Ujfalusi wrote:
On 08/08/2012 05:49 PM, Mark Brown wrote:
That makes sense if the GPIO is actively driven, open drain should be better here, but it's still a generic thing which it'd be nice to extract.
To cover all of this in a generic way is not that straight forward IMHO.
The sequence is just:
- Enable mutes (at _PRE time)
- Do whatever the device needs
- Disable the mutes (at _POST time)
I'm not sure there's any reason for you not to use the internal mute even if the external mute is present but if there is that's the only thing that's weird here. If there's no reason not to do it it just goes into step 2 and then it's fine, even if there is you can make it conditional in step 2.
Not sure, but it should not cause issues. The PIN is multiplexed between GPIO6/PWM0/MUTE functionality. For that matter probably I could just don't care about flags here and configure the extmute (the internal one) all the time. Not sure, it has been a long time I have dealt with the twl4030...
Sure I could do this: hs_extmute: if only this is set we shall use the chip built in functionality hs_extmute_gpio: if this is set we use the extmute feature but with external GPIO.
But both need to be documented and supported.
Is there any actual case where an external mute is supplied via a mechanism other than a GPIO, and if there is would it not either need its own DT property or already need to interact with the driver from code, making the DT property redundant?
Not with my knowledge. The only board using it is the zoom2 upstream. I know other boards (not in upstream) which either uses the internal mute or GPIO.
My thinking here is that the flag should be redundant because we already need to specify how we do the mute, what I'd expect is that we activate the external mute functionality as a result of being given another way of doing it so we don't need to provide a flag.
I perfectly understand your point. However how would you imagine this in the core? We should have something similar to DAPM_SUPPLY which we can attach to the widget which needs this sort of mute, but how big change we would need in the core to do this I'm not sure. I can take a look at this, but I would do it as a follow up series.
On Thu, Aug 09, 2012 at 04:53:26PM +0300, Peter Ujfalusi wrote:
On 08/09/2012 01:36 PM, Mark Brown wrote:
Is there any actual case where an external mute is supplied via a mechanism other than a GPIO, and if there is would it not either need its own DT property or already need to interact with the driver from code, making the DT property redundant?
Not with my knowledge. The only board using it is the zoom2 upstream. I know other boards (not in upstream) which either uses the internal mute or GPIO.
OK, so we can probably only support GPIO for now then worry about other external mutes when there's something to work with.
Add support when the kernel has been booted with DT blob. In this case the pdata is NULL, we need to reach up to the core node and check if the codec part has been enabled to determine if we need to coexist with the codec or not.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/input/misc/twl4030-vibra.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c index d1e1a7e..2cfc6c7 100644 --- a/drivers/input/misc/twl4030-vibra.c +++ b/drivers/input/misc/twl4030-vibra.c @@ -26,6 +26,7 @@ #include <linux/module.h> #include <linux/jiffies.h> #include <linux/platform_device.h> +#include <linux/of.h> #include <linux/workqueue.h> #include <linux/i2c/twl.h> #include <linux/mfd/twl4030-audio.h> @@ -194,13 +195,28 @@ static int twl4030_vibra_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(twl4030_vibra_pm_ops, twl4030_vibra_suspend, twl4030_vibra_resume);
+static bool twl4030_vibra_check_coexist(struct twl4030_vibra_data *pdata, + struct device_node *node) +{ + if (pdata && pdata->coexist) + return true; + +#ifdef CONFIG_OF + if (of_find_node_by_name(node, "codec")) + return true; +#endif + + return false; +} + static int __devinit twl4030_vibra_probe(struct platform_device *pdev) { struct twl4030_vibra_data *pdata = pdev->dev.platform_data; + struct device_node *twl4030_core_node = pdev->dev.parent->of_node; struct vibra_info *info; int ret;
- if (!pdata) { + if (!pdata && !twl4030_core_node) { dev_dbg(&pdev->dev, "platform_data not available\n"); return -EINVAL; } @@ -210,7 +226,7 @@ static int __devinit twl4030_vibra_probe(struct platform_device *pdev) return -ENOMEM;
info->dev = &pdev->dev; - info->coexist = pdata->coexist; + info->coexist = twl4030_vibra_check_coexist(pdata, twl4030_core_node); INIT_WORK(&info->play_work, vibra_play_work);
info->input_dev = input_allocate_device();
The external mute (if it is in use) is handled by a GPIO line. Prepare to remove the set_hs_extmute callback and replace it with two sets of platform data: hs_extmute_gpio: the GPIO number to use for external mute hs_extmute_disable_level: the GPIO level to use when the external mute is not enabled.
The codec driver can handle the external mute functionality based on these parameters and when the users of set_hs_extmute has been converted the callback can be removed.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- include/linux/i2c/twl.h | 6 +++++- sound/soc/codecs/twl4030.c | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h index 7ea898c..75ce6d1 100644 --- a/include/linux/i2c/twl.h +++ b/include/linux/i2c/twl.h @@ -666,7 +666,11 @@ struct twl4030_codec_data { unsigned int check_defaults:1; unsigned int reset_registers:1; unsigned int hs_extmute:1; - void (*set_hs_extmute)(int mute); + void (*set_hs_extmute)(int mute); /* Deprecated, use hs_extmute_gpio and + hs_extmute_disable_level */ + int hs_extmute_gpio; + unsigned int hs_extmute_disable_level:1; /* the GPIO state to disable + the extmute */ };
struct twl4030_vibra_data { diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 391fcfc..0630040 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -28,6 +28,7 @@ #include <linux/platform_device.h> #include <linux/i2c/twl.h> #include <linux/slab.h> +#include <linux/gpio.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -302,6 +303,24 @@ static void twl4030_init_chip(struct snd_soc_codec *codec) u8 reg, byte; int i = 0;
+ if (pdata && pdata->hs_extmute && + gpio_is_valid(pdata->hs_extmute_gpio)) { + int ret, level; + + if (!pdata->hs_extmute_gpio) + dev_warn(codec->dev, + "Extmute GPIO is 0 is this correct?\n"); + + level = pdata->hs_extmute_disable_level; + ret = gpio_request_one(pdata->hs_extmute_gpio, level ? + GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, + "hs_extmute"); + if (ret) { + dev_err(codec->dev, "Failed to get hs_extmute GPIO\n"); + pdata->hs_extmute_gpio = -1; + } + } + /* Check defaults, if instructed before anything else */ if (pdata && pdata->check_defaults) twl4030_check_defaults(codec); @@ -748,7 +767,11 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) /* Enable external mute control, this dramatically reduces * the pop-noise */ if (pdata && pdata->hs_extmute) { - if (pdata->set_hs_extmute) { + if (gpio_is_valid(pdata->hs_extmute_gpio)) { + gpio_set_value(pdata->hs_extmute_gpio, + !pdata->hs_extmute_disable_level); + } else if (pdata->set_hs_extmute) { + dev_warn(codec->dev, "set_hs_extmute is deprecated\n"); pdata->set_hs_extmute(1); } else { hs_pop |= TWL4030_EXTMUTE; @@ -786,7 +809,11 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp)
/* Disable external mute */ if (pdata && pdata->hs_extmute) { - if (pdata->set_hs_extmute) { + if (gpio_is_valid(pdata->hs_extmute_gpio)) { + gpio_set_value(pdata->hs_extmute_gpio, + pdata->hs_extmute_disable_level); + } else if (pdata->set_hs_extmute) { + dev_warn(codec->dev, "set_hs_extmute is deprecated\n"); pdata->set_hs_extmute(0); } else { hs_pop &= ~TWL4030_EXTMUTE; @@ -2230,12 +2257,17 @@ static int twl4030_soc_probe(struct snd_soc_codec *codec)
static int twl4030_soc_remove(struct snd_soc_codec *codec) { + struct twl4030_codec_data *pdata = dev_get_platdata(codec->dev); struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec);
/* Reset registers to their chip default before leaving */ twl4030_reset_registers(codec); twl4030_set_bias_level(codec, SND_SOC_BIAS_OFF); kfree(twl4030); + + if (pdata && pdata->hs_extmute && gpio_is_valid(pdata->hs_extmute_gpio)) + gpio_free(pdata->hs_extmute_gpio); + return 0; }
Remove the use of set_hs_extmute callback and let the codec driver to handle the extmute GPIO.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/board-zoom-peripherals.c | 9 ++------- arch/arm/mach-omap2/include/mach/board-zoom.h | 2 -- sound/soc/omap/zoom2.c | 4 ---- 3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-omap2/board-zoom-peripherals.c index b797cb2..a7d3b04 100644 --- a/arch/arm/mach-omap2/board-zoom-peripherals.c +++ b/arch/arm/mach-omap2/board-zoom-peripherals.c @@ -34,6 +34,7 @@ #include "common-board-devices.h"
#define OMAP_ZOOM_WLAN_PMENA_GPIO (101) +#define ZOOM2_HEADSET_EXTMUTE_GPIO (153) #define OMAP_ZOOM_WLAN_IRQ_GPIO (162)
#define LCD_PANEL_ENABLE_GPIO (7 + OMAP_MAX_GPIO_LINES) @@ -244,12 +245,6 @@ static int zoom_twl_gpio_setup(struct device *dev, return ret; }
-/* EXTMUTE callback function */ -static void zoom2_set_hs_extmute(int mute) -{ - gpio_set_value(ZOOM2_HEADSET_EXTMUTE_GPIO, mute); -} - static struct twl4030_gpio_platform_data zoom_gpio_data = { .gpio_base = OMAP_MAX_GPIO_LINES, .irq_base = TWL4030_GPIO_IRQ_BASE, @@ -279,7 +274,7 @@ static int __init omap_i2c_init(void)
codec_data->ramp_delay_value = 3; /* 161 ms */ codec_data->hs_extmute = 1; - codec_data->set_hs_extmute = zoom2_set_hs_extmute; + codec_data->hs_extmute_gpio = ZOOM2_HEADSET_EXTMUTE_GPIO; } omap_pmic_init(1, 2400, "twl5030", INT_34XX_SYS_NIRQ, &zoom_twldata); omap_register_i2c_bus(2, 400, NULL, 0); diff --git a/arch/arm/mach-omap2/include/mach/board-zoom.h b/arch/arm/mach-omap2/include/mach/board-zoom.h index 775fdc3..2e94869 100644 --- a/arch/arm/mach-omap2/include/mach/board-zoom.h +++ b/arch/arm/mach-omap2/include/mach/board-zoom.h @@ -8,5 +8,3 @@ extern int __init zoom_debugboard_init(void); extern void __init zoom_peripherals_init(void); extern void __init zoom_display_init(void); - -#define ZOOM2_HEADSET_EXTMUTE_GPIO 153 diff --git a/sound/soc/omap/zoom2.c b/sound/soc/omap/zoom2.c index 920e0d9..df97a41 100644 --- a/sound/soc/omap/zoom2.c +++ b/sound/soc/omap/zoom2.c @@ -191,9 +191,6 @@ static int __init zoom2_soc_init(void) BUG_ON(gpio_request(ZOOM2_HEADSET_MUX_GPIO, "hs_mux") < 0); gpio_direction_output(ZOOM2_HEADSET_MUX_GPIO, 0);
- BUG_ON(gpio_request(ZOOM2_HEADSET_EXTMUTE_GPIO, "ext_mute") < 0); - gpio_direction_output(ZOOM2_HEADSET_EXTMUTE_GPIO, 0); - return 0;
err1: @@ -207,7 +204,6 @@ module_init(zoom2_soc_init); static void __exit zoom2_soc_exit(void) { gpio_free(ZOOM2_HEADSET_MUX_GPIO); - gpio_free(ZOOM2_HEADSET_EXTMUTE_GPIO);
platform_device_unregister(zoom2_snd_device); }
* Peter Ujfalusi peter.ujfalusi@ti.com [120808 02:42]:
Remove the use of set_hs_extmute callback and let the codec driver to handle the extmute GPIO.
Acked-by: Tony Lindgren tony@atomide.com
We no longer have users for the set_hs_extmute callback which has been replaced by hs_extmute_gpio/hs_extmute_disable_level parameters so the codec driver can handle the external mute if it is needed by the board.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- include/linux/i2c/twl.h | 2 -- sound/soc/codecs/twl4030.c | 6 ------ 2 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h index 75ce6d1..c9c1fd2 100644 --- a/include/linux/i2c/twl.h +++ b/include/linux/i2c/twl.h @@ -666,8 +666,6 @@ struct twl4030_codec_data { unsigned int check_defaults:1; unsigned int reset_registers:1; unsigned int hs_extmute:1; - void (*set_hs_extmute)(int mute); /* Deprecated, use hs_extmute_gpio and - hs_extmute_disable_level */ int hs_extmute_gpio; unsigned int hs_extmute_disable_level:1; /* the GPIO state to disable the extmute */ diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 0630040..1247c4a 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -770,9 +770,6 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) if (gpio_is_valid(pdata->hs_extmute_gpio)) { gpio_set_value(pdata->hs_extmute_gpio, !pdata->hs_extmute_disable_level); - } else if (pdata->set_hs_extmute) { - dev_warn(codec->dev, "set_hs_extmute is deprecated\n"); - pdata->set_hs_extmute(1); } else { hs_pop |= TWL4030_EXTMUTE; twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); @@ -812,9 +809,6 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) if (gpio_is_valid(pdata->hs_extmute_gpio)) { gpio_set_value(pdata->hs_extmute_gpio, pdata->hs_extmute_disable_level); - } else if (pdata->set_hs_extmute) { - dev_warn(codec->dev, "set_hs_extmute is deprecated\n"); - pdata->set_hs_extmute(0); } else { hs_pop &= ~TWL4030_EXTMUTE; twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
Allocate the private data with devm_kzalloc.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl4030.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 1247c4a..6a00c01 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -2235,7 +2235,8 @@ static int twl4030_soc_probe(struct snd_soc_codec *codec) { struct twl4030_priv *twl4030;
- twl4030 = kzalloc(sizeof(struct twl4030_priv), GFP_KERNEL); + twl4030 = devm_kzalloc(codec->dev, sizeof(struct twl4030_priv), + GFP_KERNEL); if (twl4030 == NULL) { dev_err(codec->dev, "Can not allocate memory\n"); return -ENOMEM; @@ -2257,7 +2258,6 @@ static int twl4030_soc_remove(struct snd_soc_codec *codec) /* Reset registers to their chip default before leaving */ twl4030_reset_registers(codec); twl4030_set_bias_level(codec, SND_SOC_BIAS_OFF); - kfree(twl4030);
if (pdata && pdata->hs_extmute && gpio_is_valid(pdata->hs_extmute_gpio)) gpio_free(pdata->hs_extmute_gpio);
Access the pdata via a pointer within the twl4030_priv structure. In preparation for DeviceTree support.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl4030.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 6a00c01..bf0240d 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -153,8 +153,7 @@ struct twl4030_priv { u8 predrivel_enabled, predriver_enabled; u8 carkitl_enabled, carkitr_enabled;
- /* Delay needed after enabling the digimic interface */ - unsigned int digimic_delay; + struct twl4030_codec_data *pdata; };
/* @@ -350,7 +349,7 @@ static void twl4030_init_chip(struct snd_soc_codec *codec) if (!pdata) return;
- twl4030->digimic_delay = pdata->digimic_delay; + twl4030->pdata = pdata;
reg = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); reg &= ~TWL4030_RAMP_DELAY; @@ -751,9 +750,9 @@ static int aif_event(struct snd_soc_dapm_widget *w,
static void headset_ramp(struct snd_soc_codec *codec, int ramp) { - struct twl4030_codec_data *pdata = codec->dev->platform_data; unsigned char hs_gain, hs_pop; struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); + struct twl4030_codec_data *pdata = twl4030->pdata; /* Base values for ramp delay calculation: 2^19 - 2^26 */ unsigned int ramp_base[] = {524288, 1048576, 2097152, 4194304, 8388608, 16777216, 33554432, 67108864}; @@ -868,9 +867,10 @@ static int digimic_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(w->codec); + struct twl4030_codec_data *pdata = twl4030->pdata;
- if (twl4030->digimic_delay) - twl4030_wait_ms(twl4030->digimic_delay); + if (pdata && pdata->digimic_delay) + twl4030_wait_ms(pdata->digimic_delay); return 0; }
@@ -2252,8 +2252,8 @@ static int twl4030_soc_probe(struct snd_soc_codec *codec)
static int twl4030_soc_remove(struct snd_soc_codec *codec) { - struct twl4030_codec_data *pdata = dev_get_platdata(codec->dev); struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); + struct twl4030_codec_data *pdata = twl4030->pdata;
/* Reset registers to their chip default before leaving */ twl4030_reset_registers(codec);
When the kernel has been booted with DT blob the platform data is NULL for the driver. We need to construct the pdata based on the DT information for runtime use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl4030.c | 67 ++++++++++++++++++++++++++++++++++++++----- 1 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index bf0240d..68495a2 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -26,6 +26,7 @@ #include <linux/pm.h> #include <linux/i2c.h> #include <linux/platform_device.h> +#include <linux/of.h> #include <linux/i2c/twl.h> #include <linux/slab.h> #include <linux/gpio.h> @@ -295,13 +296,70 @@ static inline void twl4030_reset_registers(struct snd_soc_codec *codec)
}
-static void twl4030_init_chip(struct snd_soc_codec *codec) +static void twl4030_setup_pdata_of(struct twl4030_codec_data *pdata, + struct device_node *node) +{ + int value; + + of_property_read_u32(node, "ti,digimic_delay", + &pdata->digimic_delay); + of_property_read_u32(node, "ti,ramp_delay_value", + &pdata->ramp_delay_value); + of_property_read_u32(node, "ti,offset_cncl_path", + &pdata->offset_cncl_path); + if (!of_property_read_u32(node, "ti,hs_extmute", &value)) + pdata->hs_extmute = value; + else + pdata->hs_extmute = 0; + + if (pdata->hs_extmute) { + int ret = of_property_read_u32(node, "ti,hs_extmute_gpio", + &pdata->hs_extmute_gpio); + if (!ret) { + if (!of_property_read_u32(node, + "ti,hs_extmute_disable_level", + &value)) { + pdata->hs_extmute_disable_level = value; + } + } else { + pdata->hs_extmute_gpio = -1; + } + } +} + +static struct twl4030_codec_data *twl4030_get_pdata(struct snd_soc_codec *codec) { struct twl4030_codec_data *pdata = dev_get_platdata(codec->dev); + struct device_node *twl4030_codec_node = NULL; + +#ifdef CONFIG_OF + twl4030_codec_node = of_find_node_by_name(codec->dev->parent->of_node, + "codec"); +#endif + + if (!pdata && twl4030_codec_node) { + pdata = devm_kzalloc(codec->dev, + sizeof(struct twl4030_codec_data), + GFP_KERNEL); + if (!pdata) { + dev_err(codec->dev, "Can not allocate memory\n"); + return NULL; + } + twl4030_setup_pdata_of(pdata, twl4030_codec_node); + } + + return pdata; +} + +static void twl4030_init_chip(struct snd_soc_codec *codec) +{ + struct twl4030_codec_data *pdata; struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); u8 reg, byte; int i = 0;
+ pdata = twl4030_get_pdata(codec); + if (pdata && pdata->hs_extmute && gpio_is_valid(pdata->hs_extmute_gpio)) { int ret, level; @@ -2288,13 +2346,6 @@ static struct snd_soc_codec_driver soc_codec_dev_twl4030 = {
static int __devinit twl4030_codec_probe(struct platform_device *pdev) { - struct twl4030_codec_data *pdata = pdev->dev.platform_data; - - if (!pdata) { - dev_err(&pdev->dev, "platform_data is missing\n"); - return -EINVAL; - } - return snd_soc_register_codec(&pdev->dev, &soc_codec_dev_twl4030, twl4030_dai, ARRAY_SIZE(twl4030_dai)); }
participants (4)
-
Benoit Cousson -
Mark Brown -
Peter Ujfalusi -
Tony Lindgren