Re: [PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-07 Thread Dan Murphy
Mark
Thanks for the review

On 07/07/2014 03:08 AM, Mark Brown wrote:
> On Thu, Jul 03, 2014 at 11:24:35AM -0500, Dan Murphy wrote:
>
>> +static int tas2552_power(struct tas2552_data *data, u8 power)
>> +{
>> +int ret = 0;
>> +
>> +mutex_lock(>mutex);
>> +
>> +if (power) {
>> +if (data->enable_gpio)
>> +gpiod_set_value(data->enable_gpio, 1);
>> +
>> +data->power_state = 1;
>> +} else {
>> +if (data->enable_gpio)
>> +gpiod_set_value(data->enable_gpio, 0);
>> +
>> +data->power_state = 0;
>> +}
>> +
>> +mutex_unlock(>mutex);
>> +return ret;
>> +}
> I don't understand this function.  It appears to be the only place where
> either power_state or mutex is used so it's just adding some wrapping
> around setting the GPIO value which doesn't seem like it's doing much.
> Why are we tracking power_state?

This function and mutex are artifacts from the development and probably can be 
consolidated
into the runtime PM calls. 

>
>> +static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int 
>> sw_shutdown)
>> +{
>> +u8 cfg1_reg = 0x0;
>> +
>> +if (sw_shutdown)
>> +cfg1_reg |= TAS2552_SWS_MASK;
>> +else
>> +cfg1_reg &= ~TAS2552_SWS_MASK;
>> +
>> +snd_soc_update_bits(tas_data->codec, TAS2552_CFG_1,
>> + TAS2552_SWS_MASK, cfg1_reg);
> Given that you're using _update_bits() clearing the bits in a register
> that was just initialised to zero doesn't make a huge amount of sense.

This was an artifact from RFC in which I was not using the snd_soc functions.
I can remove the initialization of the variable.

>
>> +default:
>> +dev_vdbg(codec->dev, "Substream sample rate is not found\n");
>> +return -EINVAL;
>> +}
> Better to print the rate.

OK

>
>> +pm_runtime_get_sync(codec->dev);
>> +
>> +snd_soc_update_bits(codec, TAS2552_CFG_3, TAS2552_WCLK_MASK, wclk_reg);
>> +
>> +pm_runtime_put(codec->dev);
> This seems really strange - why is the device being powered up to just
> set a bit and then potentially powered down immediately?  I'd expect to
> just update the cache if the device is not active.  You're also not
> checking that the power up worked.

I wanted to make sure that the device was on.  I totally forgot that
the device was using regmap and the values are cached when the device
is not on.

I can remove the get/put around the update calls.

>
>> +
>> +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> +{
>> +u8 serial_format;
>> +u8 serial_control_mask = 0x00;
>> +if (fmt & SND_SOC_DAIFMT_FORMAT_MASK)
>> +serial_control_mask |= TAS2552_DATA_FORMAT_MASK;
>> +if (serial_control_mask) {
>> +pm_runtime_get_sync(codec->dev);
>> +
>> +snd_soc_update_bits(codec, TAS2552_SER_CTRL_1, 
>> serial_control_mask,
>> +serial_format);
>> +
>> +pm_runtime_put(codec->dev);
>> +}
> This seems broken - if the format mask ever gets set then it won't be
> cleared since we only do an update_bits() if the bit is being set.  Why
> isn't the driver just doing an _update_bits()?

I do not understand what this statement means.

Are you saying snd_soc_update_bits will not clear the bit if the bit mask
is set appropriately?

>
> The comments about runtime PM also apply, they applies throughout the
> driver.
>
>> +static int tas2552_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
>> +  unsigned int freq, int dir)
>> +{
>> +struct snd_soc_codec *codec = dai->codec;
>> +
>> +/* Fill in the PLL control registers for J & D
>> + * PLL_CLK = (.5 * freq * J.D) / 2^p
>> + * Need to fill in J and D here based on incoming freq
>> + */
>> +pm_runtime_get_sync(codec->dev);
>> +
>> +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
>> +
>> +snd_soc_write(codec, TAS2552_PLL_CTRL_1, 0x10);
>> +snd_soc_write(codec, TAS2552_PLL_CTRL_2, 0x00);
>> +snd_soc_write(codec, TAS2552_PLL_CTRL_3, 0x00);
>> +
>> +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE,
>> +TAS2552_PLL_ENABLE);
>> +
>> +pm_runtime_put(codec->dev);
> This makes no sense at all - please look at what other drivers are doing
> with set_sysclk().  It should be used to get information about how the
> device is clocked.
>
>> +static int tas2552_startup(struct snd_pcm_substream *substream,
>> +   struct snd_soc_dai *dai)
>> +{
>> +struct snd_soc_codec *codec = dai->codec;
>> +
>> +pm_runtime_get_sync(codec->dev);
>> +
>> +/* Turn on Class D amplifier */
>> +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
>> +TAS2552_CLASSD_EN);
> This should be done using DAPM.

You 

Re: [PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-07 Thread Mark Brown
On Thu, Jul 03, 2014 at 11:24:35AM -0500, Dan Murphy wrote:

> +static int tas2552_power(struct tas2552_data *data, u8 power)
> +{
> + int ret = 0;
> +
> + mutex_lock(>mutex);
> +
> + if (power) {
> + if (data->enable_gpio)
> + gpiod_set_value(data->enable_gpio, 1);
> +
> + data->power_state = 1;
> + } else {
> + if (data->enable_gpio)
> + gpiod_set_value(data->enable_gpio, 0);
> +
> + data->power_state = 0;
> + }
> +
> + mutex_unlock(>mutex);
> + return ret;
> +}

I don't understand this function.  It appears to be the only place where
either power_state or mutex is used so it's just adding some wrapping
around setting the GPIO value which doesn't seem like it's doing much.
Why are we tracking power_state?

> +static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int 
> sw_shutdown)
> +{
> + u8 cfg1_reg = 0x0;
> +
> + if (sw_shutdown)
> + cfg1_reg |= TAS2552_SWS_MASK;
> + else
> + cfg1_reg &= ~TAS2552_SWS_MASK;
> +
> + snd_soc_update_bits(tas_data->codec, TAS2552_CFG_1,
> +  TAS2552_SWS_MASK, cfg1_reg);

Given that you're using _update_bits() clearing the bits in a register
that was just initialised to zero doesn't make a huge amount of sense.

> + default:
> + dev_vdbg(codec->dev, "Substream sample rate is not found\n");
> + return -EINVAL;
> + }

Better to print the rate.

> + pm_runtime_get_sync(codec->dev);
> +
> + snd_soc_update_bits(codec, TAS2552_CFG_3, TAS2552_WCLK_MASK, wclk_reg);
> +
> + pm_runtime_put(codec->dev);

This seems really strange - why is the device being powered up to just
set a bit and then potentially powered down immediately?  I'd expect to
just update the cache if the device is not active.  You're also not
checking that the power up worked.

> +
> +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> + u8 serial_format;
> + u8 serial_control_mask = 0x00;

> + if (fmt & SND_SOC_DAIFMT_FORMAT_MASK)
> + serial_control_mask |= TAS2552_DATA_FORMAT_MASK;

> + if (serial_control_mask) {
> + pm_runtime_get_sync(codec->dev);
> +
> + snd_soc_update_bits(codec, TAS2552_SER_CTRL_1, 
> serial_control_mask,
> + serial_format);
> +
> + pm_runtime_put(codec->dev);
> + }

This seems broken - if the format mask ever gets set then it won't be
cleared since we only do an update_bits() if the bit is being set.  Why
isn't the driver just doing an _update_bits()?

The comments about runtime PM also apply, they applies throughout the
driver.

> +static int tas2552_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
> +   unsigned int freq, int dir)
> +{
> + struct snd_soc_codec *codec = dai->codec;
> +
> + /* Fill in the PLL control registers for J & D
> +  * PLL_CLK = (.5 * freq * J.D) / 2^p
> +  * Need to fill in J and D here based on incoming freq
> +  */
> + pm_runtime_get_sync(codec->dev);
> +
> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
> +
> + snd_soc_write(codec, TAS2552_PLL_CTRL_1, 0x10);
> + snd_soc_write(codec, TAS2552_PLL_CTRL_2, 0x00);
> + snd_soc_write(codec, TAS2552_PLL_CTRL_3, 0x00);
> +
> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE,
> + TAS2552_PLL_ENABLE);
> +
> + pm_runtime_put(codec->dev);

This makes no sense at all - please look at what other drivers are doing
with set_sysclk().  It should be used to get information about how the
device is clocked.

> +static int tas2552_startup(struct snd_pcm_substream *substream,
> +struct snd_soc_dai *dai)
> +{
> + struct snd_soc_codec *codec = dai->codec;
> +
> + pm_runtime_get_sync(codec->dev);
> +
> + /* Turn on Class D amplifier */
> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
> + TAS2552_CLASSD_EN);

This should be done using DAPM.

> +static int tas2552_codec_probe(struct snd_soc_codec *codec)
> +{
> + struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec);
> + int ret;

> +
> + tas2552_power(tas2552, 1);
> + tas2552_sw_shutdown(tas2552, 0);
> +
> + pm_runtime_set_active(codec->dev);
> + pm_runtime_set_autosuspend_delay(codec->dev, 1000);
> + pm_runtime_use_autosuspend(codec->dev);
> + pm_runtime_enable(codec->dev);
> + pm_runtime_mark_last_busy(codec->dev);
> + pm_runtime_put_sync_autosuspend(codec->dev);

This should all be done at the device level probe.

> + /* 0dB gain */
> + snd_soc_write(codec, TAS2552_PGA_GAIN, 0x10);

Use the hardware default, your default might not be sensible for some
other user.

> + 

Re: [PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-07 Thread Mark Brown
On Thu, Jul 03, 2014 at 11:24:35AM -0500, Dan Murphy wrote:

 +static int tas2552_power(struct tas2552_data *data, u8 power)
 +{
 + int ret = 0;
 +
 + mutex_lock(data-mutex);
 +
 + if (power) {
 + if (data-enable_gpio)
 + gpiod_set_value(data-enable_gpio, 1);
 +
 + data-power_state = 1;
 + } else {
 + if (data-enable_gpio)
 + gpiod_set_value(data-enable_gpio, 0);
 +
 + data-power_state = 0;
 + }
 +
 + mutex_unlock(data-mutex);
 + return ret;
 +}

I don't understand this function.  It appears to be the only place where
either power_state or mutex is used so it's just adding some wrapping
around setting the GPIO value which doesn't seem like it's doing much.
Why are we tracking power_state?

 +static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int 
 sw_shutdown)
 +{
 + u8 cfg1_reg = 0x0;
 +
 + if (sw_shutdown)
 + cfg1_reg |= TAS2552_SWS_MASK;
 + else
 + cfg1_reg = ~TAS2552_SWS_MASK;
 +
 + snd_soc_update_bits(tas_data-codec, TAS2552_CFG_1,
 +  TAS2552_SWS_MASK, cfg1_reg);

Given that you're using _update_bits() clearing the bits in a register
that was just initialised to zero doesn't make a huge amount of sense.

 + default:
 + dev_vdbg(codec-dev, Substream sample rate is not found\n);
 + return -EINVAL;
 + }

Better to print the rate.

 + pm_runtime_get_sync(codec-dev);
 +
 + snd_soc_update_bits(codec, TAS2552_CFG_3, TAS2552_WCLK_MASK, wclk_reg);
 +
 + pm_runtime_put(codec-dev);

This seems really strange - why is the device being powered up to just
set a bit and then potentially powered down immediately?  I'd expect to
just update the cache if the device is not active.  You're also not
checking that the power up worked.

 +
 +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 +{
 + u8 serial_format;
 + u8 serial_control_mask = 0x00;

 + if (fmt  SND_SOC_DAIFMT_FORMAT_MASK)
 + serial_control_mask |= TAS2552_DATA_FORMAT_MASK;

 + if (serial_control_mask) {
 + pm_runtime_get_sync(codec-dev);
 +
 + snd_soc_update_bits(codec, TAS2552_SER_CTRL_1, 
 serial_control_mask,
 + serial_format);
 +
 + pm_runtime_put(codec-dev);
 + }

This seems broken - if the format mask ever gets set then it won't be
cleared since we only do an update_bits() if the bit is being set.  Why
isn't the driver just doing an _update_bits()?

The comments about runtime PM also apply, they applies throughout the
driver.

 +static int tas2552_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 +   unsigned int freq, int dir)
 +{
 + struct snd_soc_codec *codec = dai-codec;
 +
 + /* Fill in the PLL control registers for J  D
 +  * PLL_CLK = (.5 * freq * J.D) / 2^p
 +  * Need to fill in J and D here based on incoming freq
 +  */
 + pm_runtime_get_sync(codec-dev);
 +
 + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
 +
 + snd_soc_write(codec, TAS2552_PLL_CTRL_1, 0x10);
 + snd_soc_write(codec, TAS2552_PLL_CTRL_2, 0x00);
 + snd_soc_write(codec, TAS2552_PLL_CTRL_3, 0x00);
 +
 + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE,
 + TAS2552_PLL_ENABLE);
 +
 + pm_runtime_put(codec-dev);

This makes no sense at all - please look at what other drivers are doing
with set_sysclk().  It should be used to get information about how the
device is clocked.

 +static int tas2552_startup(struct snd_pcm_substream *substream,
 +struct snd_soc_dai *dai)
 +{
 + struct snd_soc_codec *codec = dai-codec;
 +
 + pm_runtime_get_sync(codec-dev);
 +
 + /* Turn on Class D amplifier */
 + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
 + TAS2552_CLASSD_EN);

This should be done using DAPM.

 +static int tas2552_codec_probe(struct snd_soc_codec *codec)
 +{
 + struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec);
 + int ret;

 +
 + tas2552_power(tas2552, 1);
 + tas2552_sw_shutdown(tas2552, 0);
 +
 + pm_runtime_set_active(codec-dev);
 + pm_runtime_set_autosuspend_delay(codec-dev, 1000);
 + pm_runtime_use_autosuspend(codec-dev);
 + pm_runtime_enable(codec-dev);
 + pm_runtime_mark_last_busy(codec-dev);
 + pm_runtime_put_sync_autosuspend(codec-dev);

This should all be done at the device level probe.

 + /* 0dB gain */
 + snd_soc_write(codec, TAS2552_PGA_GAIN, 0x10);

Use the hardware default, your default might not be sensible for some
other user.

 + /**
 +  * Data sheet indicates to write 0x0c to 0x0d during init but no
 +  * additional information is given to what 

Re: [PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-07 Thread Dan Murphy
Mark
Thanks for the review

On 07/07/2014 03:08 AM, Mark Brown wrote:
 On Thu, Jul 03, 2014 at 11:24:35AM -0500, Dan Murphy wrote:

 +static int tas2552_power(struct tas2552_data *data, u8 power)
 +{
 +int ret = 0;
 +
 +mutex_lock(data-mutex);
 +
 +if (power) {
 +if (data-enable_gpio)
 +gpiod_set_value(data-enable_gpio, 1);
 +
 +data-power_state = 1;
 +} else {
 +if (data-enable_gpio)
 +gpiod_set_value(data-enable_gpio, 0);
 +
 +data-power_state = 0;
 +}
 +
 +mutex_unlock(data-mutex);
 +return ret;
 +}
 I don't understand this function.  It appears to be the only place where
 either power_state or mutex is used so it's just adding some wrapping
 around setting the GPIO value which doesn't seem like it's doing much.
 Why are we tracking power_state?

This function and mutex are artifacts from the development and probably can be 
consolidated
into the runtime PM calls. 


 +static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int 
 sw_shutdown)
 +{
 +u8 cfg1_reg = 0x0;
 +
 +if (sw_shutdown)
 +cfg1_reg |= TAS2552_SWS_MASK;
 +else
 +cfg1_reg = ~TAS2552_SWS_MASK;
 +
 +snd_soc_update_bits(tas_data-codec, TAS2552_CFG_1,
 + TAS2552_SWS_MASK, cfg1_reg);
 Given that you're using _update_bits() clearing the bits in a register
 that was just initialised to zero doesn't make a huge amount of sense.

This was an artifact from RFC in which I was not using the snd_soc functions.
I can remove the initialization of the variable.


 +default:
 +dev_vdbg(codec-dev, Substream sample rate is not found\n);
 +return -EINVAL;
 +}
 Better to print the rate.

OK


 +pm_runtime_get_sync(codec-dev);
 +
 +snd_soc_update_bits(codec, TAS2552_CFG_3, TAS2552_WCLK_MASK, wclk_reg);
 +
 +pm_runtime_put(codec-dev);
 This seems really strange - why is the device being powered up to just
 set a bit and then potentially powered down immediately?  I'd expect to
 just update the cache if the device is not active.  You're also not
 checking that the power up worked.

I wanted to make sure that the device was on.  I totally forgot that
the device was using regmap and the values are cached when the device
is not on.

I can remove the get/put around the update calls.


 +
 +static int tas2552_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 +{
 +u8 serial_format;
 +u8 serial_control_mask = 0x00;
 +if (fmt  SND_SOC_DAIFMT_FORMAT_MASK)
 +serial_control_mask |= TAS2552_DATA_FORMAT_MASK;
 +if (serial_control_mask) {
 +pm_runtime_get_sync(codec-dev);
 +
 +snd_soc_update_bits(codec, TAS2552_SER_CTRL_1, 
 serial_control_mask,
 +serial_format);
 +
 +pm_runtime_put(codec-dev);
 +}
 This seems broken - if the format mask ever gets set then it won't be
 cleared since we only do an update_bits() if the bit is being set.  Why
 isn't the driver just doing an _update_bits()?

I do not understand what this statement means.

Are you saying snd_soc_update_bits will not clear the bit if the bit mask
is set appropriately?


 The comments about runtime PM also apply, they applies throughout the
 driver.

 +static int tas2552_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 +  unsigned int freq, int dir)
 +{
 +struct snd_soc_codec *codec = dai-codec;
 +
 +/* Fill in the PLL control registers for J  D
 + * PLL_CLK = (.5 * freq * J.D) / 2^p
 + * Need to fill in J and D here based on incoming freq
 + */
 +pm_runtime_get_sync(codec-dev);
 +
 +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
 +
 +snd_soc_write(codec, TAS2552_PLL_CTRL_1, 0x10);
 +snd_soc_write(codec, TAS2552_PLL_CTRL_2, 0x00);
 +snd_soc_write(codec, TAS2552_PLL_CTRL_3, 0x00);
 +
 +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE,
 +TAS2552_PLL_ENABLE);
 +
 +pm_runtime_put(codec-dev);
 This makes no sense at all - please look at what other drivers are doing
 with set_sysclk().  It should be used to get information about how the
 device is clocked.

 +static int tas2552_startup(struct snd_pcm_substream *substream,
 +   struct snd_soc_dai *dai)
 +{
 +struct snd_soc_codec *codec = dai-codec;
 +
 +pm_runtime_get_sync(codec-dev);
 +
 +/* Turn on Class D amplifier */
 +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
 +TAS2552_CLASSD_EN);
 This should be done using DAPM.

You mentioned this before.  Isn't the startup part of the DAPM calls?


 +static int tas2552_codec_probe(struct snd_soc_codec *codec)
 +{
 +struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec);
 +int ret;
 +
 

[PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-03 Thread Dan Murphy
Support the TI TAS2552 Class D amplifier.

The TAS2552 is a high efficiency Class-D audio
power amplifier with advanced battery current
management and an integrated Class-G boost
The device constantly measures the
current and voltage across the load and provides a
digital stream of this information.

Signed-off-by: Dan Murphy 
---

v5 - Consolidated dai_fmt call back to write serial control register
once - https://patchwork.kernel.org/patch/4474531/

 .../devicetree/bindings/sound/tas2552.txt  |   25 +
 include/sound/tas2552-plat.h   |   25 +
 sound/soc/codecs/Kconfig   |5 +
 sound/soc/codecs/Makefile  |2 +
 sound/soc/codecs/tas2552.c |  576 
 sound/soc/codecs/tas2552.h |  120 
 6 files changed, 753 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tas2552.txt
 create mode 100644 include/sound/tas2552-plat.h
 create mode 100644 sound/soc/codecs/tas2552.c
 create mode 100644 sound/soc/codecs/tas2552.h

diff --git a/Documentation/devicetree/bindings/sound/tas2552.txt 
b/Documentation/devicetree/bindings/sound/tas2552.txt
new file mode 100644
index 000..00223dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas2552.txt
@@ -0,0 +1,25 @@
+Texas Instruments - tas2552 Codec module
+
+The tas2552 serial control bus communicates through I2C protocols
+
+Required properties:
+
+- compatible - One of:
+"ti,tas2552" - TAS2552
+
+- reg -  I2C slave address
+
+Optional properties:
+
+- enable-gpio - gpio pin to enable/disable the device
+
+Example:
+
+tas2552: tas2552@41 {
+   compatible = "ti,tas2552";
+   reg = <0x41>;
+   enable-gpio = < 2 GPIO_ACTIVE_HIGH>;
+};
+
+For more product information please see the link below:
+http://www.ti.com/product/TAS2552
diff --git a/include/sound/tas2552-plat.h b/include/sound/tas2552-plat.h
new file mode 100644
index 000..65e7627
--- /dev/null
+++ b/include/sound/tas2552-plat.h
@@ -0,0 +1,25 @@
+/*
+ * TAS2552 driver platform header
+ *
+ * Copyright (C) 2014 Texas Instruments Inc.
+ *
+ * Author: Dan Murphy 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef TAS2552_PLAT_H
+#define TAS2552_PLAT_H
+
+struct tas2552_platform_data {
+   int enable_gpio;
+};
+
+#endif
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0b9571c..cc09261 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -99,6 +99,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_TLV320AIC32X4 if I2C
select SND_SOC_TLV320AIC3X if I2C
select SND_SOC_TPA6130A2 if I2C
+   select SND_SOC_TAS2552 if I2C
select SND_SOC_TLV320DAC33 if I2C
select SND_SOC_TWL4030 if TWL4030_CORE
select SND_SOC_TWL6040 if TWL6040_CORE
@@ -754,4 +755,8 @@ config SND_SOC_TPA6130A2
tristate "Texas Instruments TPA6130A2 headphone amplifier"
depends on I2C
 
+config SND_SOC_TAS2552
+   tristate "Texas Instruments TAS2552 Mono Audio amplifier"
+   depends on I2C
+
 endmenu
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 1bd6e1c..33bc7228 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -162,6 +162,7 @@ snd-soc-wm-hubs-objs := wm_hubs.o
 # Amp
 snd-soc-max9877-objs := max9877.o
 snd-soc-tpa6130a2-objs := tpa6130a2.o
+snd-soc-tas2552-objs := tas2552.o
 
 obj-$(CONFIG_SND_SOC_88PM860X) += snd-soc-88pm860x.o
 obj-$(CONFIG_SND_SOC_AB8500_CODEC) += snd-soc-ab8500-codec.o
@@ -325,3 +326,4 @@ obj-$(CONFIG_SND_SOC_WM_HUBS)   += snd-soc-wm-hubs.o
 # Amp
 obj-$(CONFIG_SND_SOC_MAX9877)  += snd-soc-max9877.o
 obj-$(CONFIG_SND_SOC_TPA6130A2)+= snd-soc-tpa6130a2.o
+obj-$(CONFIG_SND_SOC_TAS2552)  += snd-soc-tas2552.o
diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c
new file mode 100644
index 000..fe305d6
--- /dev/null
+++ b/sound/soc/codecs/tas2552.c
@@ -0,0 +1,576 @@
+/*
+ * tas2552.c - ALSA SoC Texas Instruments TAS2552 Mono Audio Amplifier
+ *
+ * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
+ *
+ * Author: Dan Murphy 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * 

[PATCH v5] ASoC: tas2552: Support TI TAS2552 Amplifier

2014-07-03 Thread Dan Murphy
Support the TI TAS2552 Class D amplifier.

The TAS2552 is a high efficiency Class-D audio
power amplifier with advanced battery current
management and an integrated Class-G boost
The device constantly measures the
current and voltage across the load and provides a
digital stream of this information.

Signed-off-by: Dan Murphy dmur...@ti.com
---

v5 - Consolidated dai_fmt call back to write serial control register
once - https://patchwork.kernel.org/patch/4474531/

 .../devicetree/bindings/sound/tas2552.txt  |   25 +
 include/sound/tas2552-plat.h   |   25 +
 sound/soc/codecs/Kconfig   |5 +
 sound/soc/codecs/Makefile  |2 +
 sound/soc/codecs/tas2552.c |  576 
 sound/soc/codecs/tas2552.h |  120 
 6 files changed, 753 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tas2552.txt
 create mode 100644 include/sound/tas2552-plat.h
 create mode 100644 sound/soc/codecs/tas2552.c
 create mode 100644 sound/soc/codecs/tas2552.h

diff --git a/Documentation/devicetree/bindings/sound/tas2552.txt 
b/Documentation/devicetree/bindings/sound/tas2552.txt
new file mode 100644
index 000..00223dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas2552.txt
@@ -0,0 +1,25 @@
+Texas Instruments - tas2552 Codec module
+
+The tas2552 serial control bus communicates through I2C protocols
+
+Required properties:
+
+- compatible - One of:
+ti,tas2552 - TAS2552
+
+- reg -  I2C slave address
+
+Optional properties:
+
+- enable-gpio - gpio pin to enable/disable the device
+
+Example:
+
+tas2552: tas2552@41 {
+   compatible = ti,tas2552;
+   reg = 0x41;
+   enable-gpio = gpio4 2 GPIO_ACTIVE_HIGH;
+};
+
+For more product information please see the link below:
+http://www.ti.com/product/TAS2552
diff --git a/include/sound/tas2552-plat.h b/include/sound/tas2552-plat.h
new file mode 100644
index 000..65e7627
--- /dev/null
+++ b/include/sound/tas2552-plat.h
@@ -0,0 +1,25 @@
+/*
+ * TAS2552 driver platform header
+ *
+ * Copyright (C) 2014 Texas Instruments Inc.
+ *
+ * Author: Dan Murphy dmur...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef TAS2552_PLAT_H
+#define TAS2552_PLAT_H
+
+struct tas2552_platform_data {
+   int enable_gpio;
+};
+
+#endif
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0b9571c..cc09261 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -99,6 +99,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_TLV320AIC32X4 if I2C
select SND_SOC_TLV320AIC3X if I2C
select SND_SOC_TPA6130A2 if I2C
+   select SND_SOC_TAS2552 if I2C
select SND_SOC_TLV320DAC33 if I2C
select SND_SOC_TWL4030 if TWL4030_CORE
select SND_SOC_TWL6040 if TWL6040_CORE
@@ -754,4 +755,8 @@ config SND_SOC_TPA6130A2
tristate Texas Instruments TPA6130A2 headphone amplifier
depends on I2C
 
+config SND_SOC_TAS2552
+   tristate Texas Instruments TAS2552 Mono Audio amplifier
+   depends on I2C
+
 endmenu
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 1bd6e1c..33bc7228 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -162,6 +162,7 @@ snd-soc-wm-hubs-objs := wm_hubs.o
 # Amp
 snd-soc-max9877-objs := max9877.o
 snd-soc-tpa6130a2-objs := tpa6130a2.o
+snd-soc-tas2552-objs := tas2552.o
 
 obj-$(CONFIG_SND_SOC_88PM860X) += snd-soc-88pm860x.o
 obj-$(CONFIG_SND_SOC_AB8500_CODEC) += snd-soc-ab8500-codec.o
@@ -325,3 +326,4 @@ obj-$(CONFIG_SND_SOC_WM_HUBS)   += snd-soc-wm-hubs.o
 # Amp
 obj-$(CONFIG_SND_SOC_MAX9877)  += snd-soc-max9877.o
 obj-$(CONFIG_SND_SOC_TPA6130A2)+= snd-soc-tpa6130a2.o
+obj-$(CONFIG_SND_SOC_TAS2552)  += snd-soc-tas2552.o
diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c
new file mode 100644
index 000..fe305d6
--- /dev/null
+++ b/sound/soc/codecs/tas2552.c
@@ -0,0 +1,576 @@
+/*
+ * tas2552.c - ALSA SoC Texas Instruments TAS2552 Mono Audio Amplifier
+ *
+ * Copyright (C) 2014 Texas Instruments Incorporated -  http://www.ti.com
+ *
+ * Author: Dan Murphy dmur...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A