| Message ID | 1344418887-5262-5-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); > + } > 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), > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 38400000 > > Since 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: > > 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. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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),
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