Re: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
On Mon, Jul 14, 2014 at 03:10:45PM -0500, Dan Murphy wrote:
There's a few smallish issues below but this is basically good so I've
applied it, please send incremental fixed for the things below.
> + /* Turn on Class D amplifier */
> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
> + TAS2552_CLASSD_EN);
> +
Why is this being done in hw_params() and not using DAPM?
> +static int tas2552_runtime_suspend(struct device *dev)
> +{
> + struct tas2552_data *tas2552 = dev_get_drvdata(dev);
> +
> + tas2552_sw_shutdown(tas2552, 0);
> +
> + if (tas2552->enable_gpio)
> + gpiod_set_value(tas2552->enable_gpio, 0);
> +
> + regcache_cache_only(tas2552->regmap, true);
> + regcache_mark_dirty(tas2552->regmap);
It's better to do the GPIO set after making the device cache only in
order to be sure nothing can come in and try to use the register map
between the two.
> +static void tas2552_shutdown(struct snd_pcm_substream *substream,
> +struct snd_soc_dai *dai)
> +{
> + struct snd_soc_codec *codec = dai->codec;
> +
> + snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
> +}
I'd also expect the PLL power to be managed via DAPM.
> + ret = pm_runtime_get_sync(codec->dev);
> + if (ret < 0) {
> + dev_err(codec->dev, "Enabling device failed: %d\n",
> + ret);
> + goto probe_fail;
> + }
There's no matching put for this in remove().
> + snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN |
> + TAS2552_BOOST_EN | TAS2552_APT_EN |
> + TAS2552_LIM_EN);
> + return 0;
The class D is still being enabled here.
signature.asc
Description: Digital signature
Re: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
Mark
On 07/17/2014 11:58 AM, Mark Brown wrote:
> On Mon, Jul 14, 2014 at 03:10:45PM -0500, Dan Murphy wrote:
>
> There's a few smallish issues below but this is basically good so I've
> applied it, please send incremental fixed for the things below.
>
>> +/* Turn on Class D amplifier */
>> +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
>> +TAS2552_CLASSD_EN);
>> +
> Why is this being done in hw_params() and not using DAPM?
>
>> +static int tas2552_runtime_suspend(struct device *dev)
>> +{
>> +struct tas2552_data *tas2552 = dev_get_drvdata(dev);
>> +
>> +tas2552_sw_shutdown(tas2552, 0);
>> +
>> +if (tas2552->enable_gpio)
>> +gpiod_set_value(tas2552->enable_gpio, 0);
>> +
>> +regcache_cache_only(tas2552->regmap, true);
>> +regcache_mark_dirty(tas2552->regmap);
> It's better to do the GPIO set after making the device cache only in
> order to be sure nothing can come in and try to use the register map
> between the two.
>
>> +static void tas2552_shutdown(struct snd_pcm_substream *substream,
>> + struct snd_soc_dai *dai)
>> +{
>> +struct snd_soc_codec *codec = dai->codec;
>> +
>> +snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
>> +}
> I'd also expect the PLL power to be managed via DAPM.
>
>> +ret = pm_runtime_get_sync(codec->dev);
>> +if (ret < 0) {
>> +dev_err(codec->dev, "Enabling device failed: %d\n",
>> +ret);
>> +goto probe_fail;
>> +}
> There's no matching put for this in remove().
>
>> +snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN |
>> + TAS2552_BOOST_EN | TAS2552_APT_EN |
>> + TAS2552_LIM_EN);
>> +return 0;
> The class D is still being enabled here.
Thanks will send updates in a day or two.
--
--
Dan Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
Mark
On 07/17/2014 11:58 AM, Mark Brown wrote:
On Mon, Jul 14, 2014 at 03:10:45PM -0500, Dan Murphy wrote:
There's a few smallish issues below but this is basically good so I've
applied it, please send incremental fixed for the things below.
+/* Turn on Class D amplifier */
+snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
+TAS2552_CLASSD_EN);
+
Why is this being done in hw_params() and not using DAPM?
+static int tas2552_runtime_suspend(struct device *dev)
+{
+struct tas2552_data *tas2552 = dev_get_drvdata(dev);
+
+tas2552_sw_shutdown(tas2552, 0);
+
+if (tas2552-enable_gpio)
+gpiod_set_value(tas2552-enable_gpio, 0);
+
+regcache_cache_only(tas2552-regmap, true);
+regcache_mark_dirty(tas2552-regmap);
It's better to do the GPIO set after making the device cache only in
order to be sure nothing can come in and try to use the register map
between the two.
+static void tas2552_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+struct snd_soc_codec *codec = dai-codec;
+
+snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
+}
I'd also expect the PLL power to be managed via DAPM.
+ret = pm_runtime_get_sync(codec-dev);
+if (ret 0) {
+dev_err(codec-dev, Enabling device failed: %d\n,
+ret);
+goto probe_fail;
+}
There's no matching put for this in remove().
+snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN |
+ TAS2552_BOOST_EN | TAS2552_APT_EN |
+ TAS2552_LIM_EN);
+return 0;
The class D is still being enabled here.
Thanks will send updates in a day or two.
--
--
Dan Murphy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
On Mon, Jul 14, 2014 at 03:10:45PM -0500, Dan Murphy wrote:
There's a few smallish issues below but this is basically good so I've
applied it, please send incremental fixed for the things below.
+ /* Turn on Class D amplifier */
+ snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK,
+ TAS2552_CLASSD_EN);
+
Why is this being done in hw_params() and not using DAPM?
+static int tas2552_runtime_suspend(struct device *dev)
+{
+ struct tas2552_data *tas2552 = dev_get_drvdata(dev);
+
+ tas2552_sw_shutdown(tas2552, 0);
+
+ if (tas2552-enable_gpio)
+ gpiod_set_value(tas2552-enable_gpio, 0);
+
+ regcache_cache_only(tas2552-regmap, true);
+ regcache_mark_dirty(tas2552-regmap);
It's better to do the GPIO set after making the device cache only in
order to be sure nothing can come in and try to use the register map
between the two.
+static void tas2552_shutdown(struct snd_pcm_substream *substream,
+struct snd_soc_dai *dai)
+{
+ struct snd_soc_codec *codec = dai-codec;
+
+ snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0);
+}
I'd also expect the PLL power to be managed via DAPM.
+ ret = pm_runtime_get_sync(codec-dev);
+ if (ret 0) {
+ dev_err(codec-dev, Enabling device failed: %d\n,
+ ret);
+ goto probe_fail;
+ }
There's no matching put for this in remove().
+ snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN |
+ TAS2552_BOOST_EN | TAS2552_APT_EN |
+ TAS2552_LIM_EN);
+ return 0;
The class D is still being enabled here.
signature.asc
Description: Digital signature
[PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
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
---
v7 - Addressed pm_runtime comments, sorted and alphatized the Makefile
and Kconfg, removed "-codec", disable PLL at probe -
https://patchwork.kernel.org/patch/4535501/
v6 - Addressed comments from v5 also added PLL algorithim instead of
hard coding PLL registers - https://patchwork.kernel.org/patch/4476001/
v5 - Consolidated dai_fmt call back to write serial control register
once - https://patchwork.kernel.org/patch/4474531/
v4 - Added pm_runtime support, removed magical numbers, added regulator
support, changed from legacy gpio to new gpiod calls, fixed default register
values, flipped on battery tracking support and
added suspend/resume support - https://patchwork.kernel.org/patch/4465611/
v3 - Updated bindings doc per comments, rearranged probe pdata vs
np check - https://patchwork.kernel.org/patch/4453481/
v2 - Address RFC comments- Added regmap, and snd_soc calls
removed debug code, address checkpatch errors
-https://patchwork.kernel.org/patch/4378281/
.../devicetree/bindings/sound/tas2552.txt | 26 +
include/sound/tas2552-plat.h | 25 +
sound/soc/codecs/Kconfig |5 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/tas2552.c | 540
sound/soc/codecs/tas2552.h | 129 +
6 files changed, 727 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..55e2a0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas2552.txt
@@ -0,0 +1,26 @@
+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
+ - supply-*: Required supply regulators are:
+ "vbat" battery voltage
+ "iovdd" I/O Voltage
+ "avdd" Analog DAC Voltage
+
+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..fbaa329 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -91,6 +91,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_STA350 if I2C
select SND_SOC_STA529 if I2C
select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
+ select SND_SOC_TAS2552 if I2C
select SND_SOC_TAS5086 if I2C
select SND_SOC_TLV320AIC23_I2C if I2C
select SND_SOC_TLV320AIC23_SPI if SPI_MASTER
@@ -521,6 +522,10 @@ config SND_SOC_STA529
config SND_SOC_STAC9766
tristate
+config SND_SOC_TAS2552
+ tristate "Texas Instruments TAS2552 Mono Audio amplifier"
+ depends on I2C
+
config SND_SOC_TAS5086
tristate "Texas Instruments TAS5086 speaker amplifier"
depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 1bd6e1c..3f20ecd 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
[PATCH v7] ASoC: tas2552: Support TI TAS2552 Amplifier
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
---
v7 - Addressed pm_runtime comments, sorted and alphatized the Makefile
and Kconfg, removed -codec, disable PLL at probe -
https://patchwork.kernel.org/patch/4535501/
v6 - Addressed comments from v5 also added PLL algorithim instead of
hard coding PLL registers - https://patchwork.kernel.org/patch/4476001/
v5 - Consolidated dai_fmt call back to write serial control register
once - https://patchwork.kernel.org/patch/4474531/
v4 - Added pm_runtime support, removed magical numbers, added regulator
support, changed from legacy gpio to new gpiod calls, fixed default register
values, flipped on battery tracking support and
added suspend/resume support - https://patchwork.kernel.org/patch/4465611/
v3 - Updated bindings doc per comments, rearranged probe pdata vs
np check - https://patchwork.kernel.org/patch/4453481/
v2 - Address RFC comments- Added regmap, and snd_soc calls
removed debug code, address checkpatch errors
-https://patchwork.kernel.org/patch/4378281/
.../devicetree/bindings/sound/tas2552.txt | 26 +
include/sound/tas2552-plat.h | 25 +
sound/soc/codecs/Kconfig |5 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/tas2552.c | 540
sound/soc/codecs/tas2552.h | 129 +
6 files changed, 727 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..55e2a0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas2552.txt
@@ -0,0 +1,26 @@
+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
+ - supply-*: Required supply regulators are:
+ vbat battery voltage
+ iovdd I/O Voltage
+ avdd Analog DAC Voltage
+
+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..fbaa329 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -91,6 +91,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_STA350 if I2C
select SND_SOC_STA529 if I2C
select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
+ select SND_SOC_TAS2552 if I2C
select SND_SOC_TAS5086 if I2C
select SND_SOC_TLV320AIC23_I2C if I2C
select SND_SOC_TLV320AIC23_SPI if SPI_MASTER
@@ -521,6 +522,10 @@ config SND_SOC_STA529
config SND_SOC_STAC9766
tristate
+config SND_SOC_TAS2552
+ tristate Texas Instruments TAS2552 Mono Audio amplifier
+ depends on I2C
+
config SND_SOC_TAS5086
tristate Texas Instruments TAS5086 speaker amplifier
depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 1bd6e1c..3f20ecd 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
