[PATCH 1/4] ASoC: codecs: tas5720: add basic support for TAS5722 devices

[PATCH 1/4] ASoC: codecs: tas5720: add basic support for TAS5722 devices

[Andrew F. Davis]

From: Andreas Dannenberg <dannenberg@ti.com>

The TI TAS5722 digital amplifier is very similar to the TAS5720 from an
overall and register map perspective. Therefore the existing driver can be
extended easily to support this additional device. This commit allows
TAS5722 devices to be used in a "subset" type of fashion, without exposing
any of the additional features they offer.

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../devicetree/bindings/sound/tas5720.txt          |  4 ++-
 sound/soc/codecs/tas5720.c                         | 38 ++++++++++++++++++----
 sound/soc/codecs/tas5720.h                         |  1 +
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/tas5720.txt b/Documentation/devicetree/bindings/sound/tas5720.txt
index 40d94f82beb3..7481653fe8e3 100644
--- a/Documentation/devicetree/bindings/sound/tas5720.txt
+++ b/Documentation/devicetree/bindings/sound/tas5720.txt
@@ -6,10 +6,12 @@ audio playback. For more product information please see the links below:
 
 http://www.ti.com/product/TAS5720L
 http://www.ti.com/product/TAS5720M
+http://www.ti.com/product/TAS5722L
 
 Required properties:
 
-- compatible : "ti,tas5720"
+- compatible : "ti,tas5720",
+               "ti,tas5722"
 - reg : I2C slave address
 - dvdd-supply : phandle to a 3.3-V supply for the digital circuitry
 - pvdd-supply : phandle to a supply used for the Class-D amp and the analog
diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index a736a2a6976c..5def54d1336d 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -36,6 +36,11 @@
 /* Define how often to check (and clear) the fault status register (in ms) */
 #define TAS5720_FAULT_CHECK_INTERVAL		200
 
+enum tas572x_type {
+	TAS5720,
+	TAS5722,
+};
+
 static const char * const tas5720_supply_names[] = {
 	"dvdd",		/* Digital power supply. Connect to 3.3-V supply. */
 	"pvdd",		/* Class-D amp and analog power supply (connected). */
@@ -47,6 +52,7 @@ struct tas5720_data {
 	struct snd_soc_codec *codec;
 	struct regmap *regmap;
 	struct i2c_client *tas5720_client;
+	enum tas572x_type devtype;
 	struct regulator_bulk_data supplies[TAS5720_NUM_SUPPLIES];
 	struct delayed_work fault_check_work;
 	unsigned int last_fault;
@@ -264,7 +270,7 @@ static void tas5720_fault_check_work(struct work_struct *work)
 static int tas5720_codec_probe(struct snd_soc_codec *codec)
 {
 	struct tas5720_data *tas5720 = snd_soc_codec_get_drvdata(codec);
-	unsigned int device_id;
+	unsigned int device_id, expected_device_id;
 	int ret;
 
 	tas5720->codec = codec;
@@ -276,6 +282,11 @@ static int tas5720_codec_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
+	/*
+	 * Take a liberal approach to checking the device ID to allow the
+	 * driver to be used even if the device ID does not match, however
+	 * issue a warning if there is a mismatch.
+	 */
 	ret = regmap_read(tas5720->regmap, TAS5720_DEVICE_ID_REG, &device_id);
 	if (ret < 0) {
 		dev_err(codec->dev, "failed to read device ID register: %d\n",
@@ -283,13 +294,22 @@ static int tas5720_codec_probe(struct snd_soc_codec *codec)
 		goto probe_fail;
 	}
 
-	if (device_id != TAS5720_DEVICE_ID) {
-		dev_err(codec->dev, "wrong device ID. expected: %u read: %u\n",
-			TAS5720_DEVICE_ID, device_id);
-		ret = -ENODEV;
-		goto probe_fail;
+	switch (tas5720->devtype) {
+	case TAS5720:
+		expected_device_id = TAS5720_DEVICE_ID;
+		break;
+	case TAS5722:
+		expected_device_id = TAS5722_DEVICE_ID;
+		break;
+	default:
+		dev_err(codec->dev, "unexpected private driver data\n");
+		return -EINVAL;
 	}
 
+	if (device_id != expected_device_id)
+		dev_warn(codec->dev, "wrong device ID. expected: %u read: %u\n",
+			 expected_device_id, device_id);
+
 	/* Set device to mute */
 	ret = snd_soc_update_bits(codec, TAS5720_DIGITAL_CTRL2_REG,
 				  TAS5720_MUTE, TAS5720_MUTE);
@@ -552,6 +572,8 @@ static int tas5720_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	data->tas5720_client = client;
+	data->devtype = id->driver_data;
+
 	data->regmap = devm_regmap_init_i2c(client, &tas5720_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		ret = PTR_ERR(data->regmap);
@@ -592,7 +614,8 @@ static int tas5720_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id tas5720_id[] = {
-	{ "tas5720", 0 },
+	{ "tas5720", TAS5720 },
+	{ "tas5722", TAS5722 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tas5720_id);
@@ -600,6 +623,7 @@ MODULE_DEVICE_TABLE(i2c, tas5720_id);
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id tas5720_of_match[] = {
 	{ .compatible = "ti,tas5720", },
+	{ .compatible = "ti,tas5722", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, tas5720_of_match);
diff --git a/sound/soc/codecs/tas5720.h b/sound/soc/codecs/tas5720.h
index 3d077c779b12..bef802afcc69 100644
--- a/sound/soc/codecs/tas5720.h
+++ b/sound/soc/codecs/tas5720.h
@@ -32,6 +32,7 @@
 
 /* TAS5720_DEVICE_ID_REG */
 #define TAS5720_DEVICE_ID		0x01
+#define TAS5722_DEVICE_ID		0x12
 
 /* TAS5720_POWER_CTRL_REG */
 #define TAS5720_DIG_CLIP_MASK		GENMASK(7, 2)
-- 
2.15.0

[PATCH 2/4] ASoC: codecs: tas5720: add TAS5722 register support

[Andrew F. Davis]

From: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>

Introduce a custom super-set register map and associated bit definitions
to allow driver access to all TAS5722 device functionality.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
---
 sound/soc/codecs/tas5720.c | 23 ++++++++++++++++++++++-
 sound/soc/codecs/tas5720.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index 5def54d1336d..f3006f301fe8 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -466,6 +466,15 @@ static const struct regmap_config tas5720_regmap_config = {
 	.volatile_reg = tas5720_is_volatile_reg,
 };
 
+static const struct regmap_config tas5722_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = TAS5722_MAX_REG,
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_reg = tas5720_is_volatile_reg,
+};
+
 /*
  * DAC analog gain. There are four discrete values to select from, ranging
  * from 19.2 dB to 26.3dB.
@@ -564,6 +573,7 @@ static int tas5720_probe(struct i2c_client *client,
 {
 	struct device *dev = &client->dev;
 	struct tas5720_data *data;
+	const struct regmap_config *regmap_config;
 	int ret;
 	int i;
 
@@ -574,7 +584,18 @@ static int tas5720_probe(struct i2c_client *client,
 	data->tas5720_client = client;
 	data->devtype = id->driver_data;
 
-	data->regmap = devm_regmap_init_i2c(client, &tas5720_regmap_config);
+	switch (id->driver_data) {
+	case TAS5720:
+		regmap_config = &tas5720_regmap_config;
+		break;
+	case TAS5722:
+		regmap_config = &tas5722_regmap_config;
+		break;
+	default:
+		dev_err(dev, "unexpected private driver data\n");
+		return -EINVAL;
+	}
+	data->regmap = devm_regmap_init_i2c(client, regmap_config);
 	if (IS_ERR(data->regmap)) {
 		ret = PTR_ERR(data->regmap);
 		dev_err(dev, "failed to allocate register map: %d\n", ret);
diff --git a/sound/soc/codecs/tas5720.h b/sound/soc/codecs/tas5720.h
index bef802afcc69..1dda3095961d 100644
--- a/sound/soc/codecs/tas5720.h
+++ b/sound/soc/codecs/tas5720.h
@@ -30,6 +30,11 @@
 #define TAS5720_DIGITAL_CLIP1_REG	0x11
 #define TAS5720_MAX_REG			TAS5720_DIGITAL_CLIP1_REG
 
+/* Additional TAS5722-specific Registers */
+#define TAS5722_DIGITAL_CTRL2_REG	0x13
+#define TAS5722_ANALOG_CTRL2_REG	0x14
+#define TAS5722_MAX_REG			TAS5722_ANALOG_CTRL2_REG
+
 /* TAS5720_DEVICE_ID_REG */
 #define TAS5720_DEVICE_ID		0x01
 #define TAS5722_DEVICE_ID		0x12
@@ -52,6 +57,7 @@
 #define TAS5720_SAIF_FORMAT_MASK	GENMASK(2, 0)
 
 /* TAS5720_DIGITAL_CTRL2_REG */
+#define TAS5722_VOL_RAMP_RATE		BIT(6)
 #define TAS5720_MUTE			BIT(4)
 #define TAS5720_TDM_SLOT_SEL_MASK	GENMASK(2, 0)
 
@@ -88,4 +94,28 @@
 #define TAS5720_CLIP1_MASK		GENMASK(7, 2)
 #define TAS5720_CLIP1_SHIFT		(0x2)
 
+/* TAS5722_DIGITAL_CTRL2_REG */
+#define TAS5722_HPF_3_7HZ		(0x0 << 5)
+#define TAS5722_HPF_7_4HZ		(0x1 << 5)
+#define TAS5722_HPF_14_9HZ		(0x2 << 5)
+#define TAS5722_HPF_29_7HZ		(0x3 << 5)
+#define TAS5722_HPF_59_4HZ		(0x4 << 5)
+#define TAS5722_HPF_118_4HZ		(0x5 << 5)
+#define TAS5722_HPF_235_0HZ		(0x6 << 5)
+#define TAS5722_HPF_463_2HZ		(0x7 << 5)
+#define TAS5722_HPF_MASK		GENMASK(7, 5)
+#define TAS5722_AUTO_SLEEP_OFF		(0x0 << 3)
+#define TAS5722_AUTO_SLEEP_1024LR	(0x1 << 3)
+#define TAS5722_AUTO_SLEEP_65536LR	(0x2 << 3)
+#define TAS5722_AUTO_SLEEP_262144LR	(0x3 << 3)
+#define TAS5722_AUTO_SLEEP_MASK		GENMASK(4, 3)
+#define TAS5722_TDM_SLOT_16B		BIT(2)
+#define TAS5722_MCLK_PIN_CFG		BIT(1)
+#define TAS5722_VOL_CONTROL_LSB		BIT(0)
+
+/* TAS5722_ANALOG_CTRL2_REG */
+#define TAS5722_FAULTZ_PU		BIT(3)
+#define TAS5722_VREG_LVL		BIT(2)
+#define TAS5722_PWR_TUNE		BIT(0)
+
 #endif /* __TAS5720_H__ */
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control

[Andrew F. Davis]

From: Andreas Dannenberg <dannenberg@ti.com>

The TAS5722 supports modifying volume in 0.25dB steps (as opposed to 0.5dB
steps on the TAS5720). Introduce a custom mixer control that allows taking
advantage of this finer output volume granularity.

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 sound/soc/codecs/tas5720.c | 88 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index f3006f301fe8..042068964afb 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -491,11 +491,59 @@ static const DECLARE_TLV_DB_RANGE(dac_analog_tlv,
  * setting the gain below -100 dB (register value <0x7) is effectively a MUTE
  * as per device datasheet.
  */
-static DECLARE_TLV_DB_SCALE(dac_tlv, -10350, 50, 0);
+static DECLARE_TLV_DB_SCALE(tas5720_dac_tlv, -10350, 50, 0);
+
 
 static const struct snd_kcontrol_new tas5720_snd_controls[] = {
 	SOC_SINGLE_TLV("Speaker Driver Playback Volume",
-		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, dac_tlv),
+		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, tas5720_dac_tlv),
+	SOC_SINGLE_TLV("Speaker Driver Analog Gain", TAS5720_ANALOG_CTRL_REG,
+		       TAS5720_ANALOG_GAIN_SHIFT, 3, 0, dac_analog_tlv),
+};
+
+/*
+ * DAC digital volumes. From -103.5 to 24 dB in 0.25 dB steps. Note that
+ * setting the gain below -100 dB (register value <0x8)  is effectively a MUTE
+ * as per device datasheet.
+ *
+ * Note that for the TAS5722 the digital volume controls are actually split
+ * over two registers, so we need custom getters/setters for access.
+ */
+static DECLARE_TLV_DB_SCALE(tas5722_dac_tlv, -10350, 25, 0);
+
+static int tas5722_volume_get(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
+	unsigned int val;
+
+	val = snd_soc_read(codec, TAS5720_VOLUME_CTRL_REG);
+	ucontrol->value.integer.value[0] = val << 1;
+
+	val = snd_soc_read(codec, TAS5722_DIGITAL_CTRL2_REG);
+	ucontrol->value.integer.value[0] |= val & TAS5722_VOL_CONTROL_LSB;
+
+	return 0;
+}
+
+static int tas5722_volume_set(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
+	unsigned int sel = ucontrol->value.integer.value[0];
+
+	snd_soc_write(codec, TAS5720_VOLUME_CTRL_REG, sel >> 1);
+	snd_soc_update_bits(codec, TAS5722_DIGITAL_CTRL2_REG,
+			    TAS5722_VOL_CONTROL_LSB, sel);
+
+	return 0;
+}
+
+static const struct snd_kcontrol_new tas5722_snd_controls[] = {
+	SOC_SINGLE_EXT_TLV("Speaker Driver Playback Volume",
+			   0, 0, 511, 0,
+			   tas5722_volume_get, tas5722_volume_set,
+			   tas5722_dac_tlv),
 	SOC_SINGLE_TLV("Speaker Driver Analog Gain", TAS5720_ANALOG_CTRL_REG,
 		       TAS5720_ANALOG_GAIN_SHIFT, 3, 0, dac_analog_tlv),
 };
@@ -528,6 +576,22 @@ static const struct snd_soc_codec_driver soc_codec_dev_tas5720 = {
 	},
 };
 
+static struct snd_soc_codec_driver soc_codec_dev_tas5722 = {
+	.probe = tas5720_codec_probe,
+	.remove = tas5720_codec_remove,
+	.suspend = tas5720_suspend,
+	.resume = tas5720_resume,
+
+	.component_driver = {
+		.controls = tas5722_snd_controls,
+		.num_controls = ARRAY_SIZE(tas5722_snd_controls),
+		.dapm_widgets = tas5720_dapm_widgets,
+		.num_dapm_widgets = ARRAY_SIZE(tas5720_dapm_widgets),
+		.dapm_routes = tas5720_audio_map,
+		.num_dapm_routes = ARRAY_SIZE(tas5720_audio_map),
+	}
+};
+
 /* PCM rates supported by the TAS5720 driver */
 #define TAS5720_RATES	(SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |\
 			 SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
@@ -614,9 +678,23 @@ static int tas5720_probe(struct i2c_client *client,
 
 	dev_set_drvdata(dev, data);
 
-	ret = snd_soc_register_codec(&client->dev,
-				     &soc_codec_dev_tas5720,
-				     tas5720_dai, ARRAY_SIZE(tas5720_dai));
+	switch (id->driver_data) {
+	case TAS5720:
+		ret = snd_soc_register_codec(&client->dev,
+					     &soc_codec_dev_tas5720,
+					     tas5720_dai,
+					     ARRAY_SIZE(tas5720_dai));
+		break;
+	case TAS5722:
+		ret = snd_soc_register_codec(&client->dev,
+					     &soc_codec_dev_tas5722,
+					     tas5720_dai,
+					     ARRAY_SIZE(tas5720_dai));
+		break;
+	default:
+		dev_err(dev, "unexpected private driver data\n");
+		return -EINVAL;
+	}
 	if (ret < 0) {
 		dev_err(dev, "failed to register codec: %d\n", ret);
 		return ret;
-- 
2.15.0

[PATCH 4/4] ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support

[Andrew F. Davis]

From: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>

Unlike the TAS5720, the TAS5722 can be configured to utilize 16-bit wide
slots in TDM mode. This can help easing audio clocking/frequency
requirements.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
---
 sound/soc/codecs/tas5720.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index 042068964afb..08e0bfc9390c 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -152,6 +152,7 @@ static int tas5720_set_dai_tdm_slot(struct snd_soc_dai *dai,
 				    int slots, int slot_width)
 {
 	struct snd_soc_codec *codec = dai->codec;
+	struct tas5720_data *tas5720 = snd_soc_codec_get_drvdata(codec);
 	unsigned int first_slot;
 	int ret;
 
@@ -185,6 +186,16 @@ static int tas5720_set_dai_tdm_slot(struct snd_soc_dai *dai,
 	if (ret < 0)
 		goto error_snd_soc_update_bits;
 
+	/* Configure TDM slot width. This is only applicable to TAS5722. */
+	if (tas5720->devtype == TAS5722) {
+		ret = snd_soc_update_bits(codec, TAS5722_DIGITAL_CTRL2_REG,
+					  TAS5722_TDM_SLOT_16B,
+					  slot_width == 16 ?
+					  TAS5722_TDM_SLOT_16B : 0);
+		if (ret < 0)
+			goto error_snd_soc_update_bits;
+	}
+
 	return 0;
 
 error_snd_soc_update_bits:
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Mon, Dec 11, 2017 at 01:01:56PM -0600, Andrew F. Davis wrote:

> The TAS5722 supports modifying volume in 0.25dB steps (as opposed to 0.5dB
> steps on the TAS5720). Introduce a custom mixer control that allows taking
> advantage of this finer output volume granularity.

Don't do this, it's just making things more complicated.  Instead do
what other drivers do and register different sets of controls depending
on which part you're working with.  The normal thing is to have a big
table for all the shared controls that are the same on all variants then
register additional tables during probe with those that vary for the
individul devices.

>  static const struct snd_kcontrol_new tas5720_snd_controls[] = {
>  	SOC_SINGLE_TLV("Speaker Driver Playback Volume",
> -		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, dac_tlv),
> +		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, tas5720_dac_tlv),
> +	SOC_SINGLE_TLV("Speaker Driver Analog Gain", TAS5720_ANALOG_CTRL_REG,
> +		       TAS5720_ANALOG_GAIN_SHIFT, 3, 0, dac_analog_tlv),

As ever all volume controls should end in Volume (like the immediately
adjacent control does).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 486 bytes --]

On Mon, Dec 11, 2017 at 01:01:57PM -0600, Andrew F. Davis wrote:

> +	/* Configure TDM slot width. This is only applicable to TAS5722. */
> +	if (tas5720->devtype == TAS5722) {
> +		ret = snd_soc_update_bits(codec, TAS5722_DIGITAL_CTRL2_REG,
> +					  TAS5722_TDM_SLOT_16B,
> +					  slot_width == 16 ?
> +					  TAS5722_TDM_SLOT_16B : 0);
> +		if (ret < 0)
> +			goto error_snd_soc_update_bits;
> +	}

Use a switch statement, that way additional variants can be handled more
sensibly.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Applied "ASoC: tas5720: add TAS5722 register support" to the asoc tree

[Mark Brown]

The patch

   ASoC: tas5720: add TAS5722 register support

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d5eb436acc8104b5c789359aa8c923ff5fafcd62 Mon Sep 17 00:00:00 2001
From: Andreas Dannenberg <dannenberg@ti.com>
Date: Mon, 11 Dec 2017 13:01:55 -0600
Subject: [PATCH] ASoC: tas5720: add TAS5722 register support

Introduce a custom super-set register map and associated bit definitions
to allow driver access to all TAS5722 device functionality.

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/tas5720.c | 23 ++++++++++++++++++++++-
 sound/soc/codecs/tas5720.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index 5def54d1336d..f3006f301fe8 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -466,6 +466,15 @@ static const struct regmap_config tas5720_regmap_config = {
 	.volatile_reg = tas5720_is_volatile_reg,
 };
 
+static const struct regmap_config tas5722_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = TAS5722_MAX_REG,
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_reg = tas5720_is_volatile_reg,
+};
+
 /*
  * DAC analog gain. There are four discrete values to select from, ranging
  * from 19.2 dB to 26.3dB.
@@ -564,6 +573,7 @@ static int tas5720_probe(struct i2c_client *client,
 {
 	struct device *dev = &client->dev;
 	struct tas5720_data *data;
+	const struct regmap_config *regmap_config;
 	int ret;
 	int i;
 
@@ -574,7 +584,18 @@ static int tas5720_probe(struct i2c_client *client,
 	data->tas5720_client = client;
 	data->devtype = id->driver_data;
 
-	data->regmap = devm_regmap_init_i2c(client, &tas5720_regmap_config);
+	switch (id->driver_data) {
+	case TAS5720:
+		regmap_config = &tas5720_regmap_config;
+		break;
+	case TAS5722:
+		regmap_config = &tas5722_regmap_config;
+		break;
+	default:
+		dev_err(dev, "unexpected private driver data\n");
+		return -EINVAL;
+	}
+	data->regmap = devm_regmap_init_i2c(client, regmap_config);
 	if (IS_ERR(data->regmap)) {
 		ret = PTR_ERR(data->regmap);
 		dev_err(dev, "failed to allocate register map: %d\n", ret);
diff --git a/sound/soc/codecs/tas5720.h b/sound/soc/codecs/tas5720.h
index bef802afcc69..1dda3095961d 100644
--- a/sound/soc/codecs/tas5720.h
+++ b/sound/soc/codecs/tas5720.h
@@ -30,6 +30,11 @@
 #define TAS5720_DIGITAL_CLIP1_REG	0x11
 #define TAS5720_MAX_REG			TAS5720_DIGITAL_CLIP1_REG
 
+/* Additional TAS5722-specific Registers */
+#define TAS5722_DIGITAL_CTRL2_REG	0x13
+#define TAS5722_ANALOG_CTRL2_REG	0x14
+#define TAS5722_MAX_REG			TAS5722_ANALOG_CTRL2_REG
+
 /* TAS5720_DEVICE_ID_REG */
 #define TAS5720_DEVICE_ID		0x01
 #define TAS5722_DEVICE_ID		0x12
@@ -52,6 +57,7 @@
 #define TAS5720_SAIF_FORMAT_MASK	GENMASK(2, 0)
 
 /* TAS5720_DIGITAL_CTRL2_REG */
+#define TAS5722_VOL_RAMP_RATE		BIT(6)
 #define TAS5720_MUTE			BIT(4)
 #define TAS5720_TDM_SLOT_SEL_MASK	GENMASK(2, 0)
 
@@ -88,4 +94,28 @@
 #define TAS5720_CLIP1_MASK		GENMASK(7, 2)
 #define TAS5720_CLIP1_SHIFT		(0x2)
 
+/* TAS5722_DIGITAL_CTRL2_REG */
+#define TAS5722_HPF_3_7HZ		(0x0 << 5)
+#define TAS5722_HPF_7_4HZ		(0x1 << 5)
+#define TAS5722_HPF_14_9HZ		(0x2 << 5)
+#define TAS5722_HPF_29_7HZ		(0x3 << 5)
+#define TAS5722_HPF_59_4HZ		(0x4 << 5)
+#define TAS5722_HPF_118_4HZ		(0x5 << 5)
+#define TAS5722_HPF_235_0HZ		(0x6 << 5)
+#define TAS5722_HPF_463_2HZ		(0x7 << 5)
+#define TAS5722_HPF_MASK		GENMASK(7, 5)
+#define TAS5722_AUTO_SLEEP_OFF		(0x0 << 3)
+#define TAS5722_AUTO_SLEEP_1024LR	(0x1 << 3)
+#define TAS5722_AUTO_SLEEP_65536LR	(0x2 << 3)
+#define TAS5722_AUTO_SLEEP_262144LR	(0x3 << 3)
+#define TAS5722_AUTO_SLEEP_MASK		GENMASK(4, 3)
+#define TAS5722_TDM_SLOT_16B		BIT(2)
+#define TAS5722_MCLK_PIN_CFG		BIT(1)
+#define TAS5722_VOL_CONTROL_LSB		BIT(0)
+
+/* TAS5722_ANALOG_CTRL2_REG */
+#define TAS5722_FAULTZ_PU		BIT(3)
+#define TAS5722_VREG_LVL		BIT(2)
+#define TAS5722_PWR_TUNE		BIT(0)
+
 #endif /* __TAS5720_H__ */
-- 
2.15.1

Applied "ASoC: tas5720: add basic support for TAS5722 devices" to the asoc tree

[Mark Brown]

The patch

   ASoC: tas5720: add basic support for TAS5722 devices

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 872bcad246e30f0ae8009a0f8c13874009601445 Mon Sep 17 00:00:00 2001
From: Andreas Dannenberg <dannenberg@ti.com>
Date: Mon, 11 Dec 2017 13:01:54 -0600
Subject: [PATCH] ASoC: tas5720: add basic support for TAS5722 devices

The TI TAS5722 digital amplifier is very similar to the TAS5720 from an
overall and register map perspective. Therefore the existing driver can be
extended easily to support this additional device. This commit allows
TAS5722 devices to be used in a "subset" type of fashion, without exposing
any of the additional features they offer.

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../devicetree/bindings/sound/tas5720.txt          |  4 ++-
 sound/soc/codecs/tas5720.c                         | 38 ++++++++++++++++++----
 sound/soc/codecs/tas5720.h                         |  1 +
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/tas5720.txt b/Documentation/devicetree/bindings/sound/tas5720.txt
index 40d94f82beb3..7481653fe8e3 100644
--- a/Documentation/devicetree/bindings/sound/tas5720.txt
+++ b/Documentation/devicetree/bindings/sound/tas5720.txt
@@ -6,10 +6,12 @@ audio playback. For more product information please see the links below:
 
 http://www.ti.com/product/TAS5720L
 http://www.ti.com/product/TAS5720M
+http://www.ti.com/product/TAS5722L
 
 Required properties:
 
-- compatible : "ti,tas5720"
+- compatible : "ti,tas5720",
+               "ti,tas5722"
 - reg : I2C slave address
 - dvdd-supply : phandle to a 3.3-V supply for the digital circuitry
 - pvdd-supply : phandle to a supply used for the Class-D amp and the analog
diff --git a/sound/soc/codecs/tas5720.c b/sound/soc/codecs/tas5720.c
index a736a2a6976c..5def54d1336d 100644
--- a/sound/soc/codecs/tas5720.c
+++ b/sound/soc/codecs/tas5720.c
@@ -36,6 +36,11 @@
 /* Define how often to check (and clear) the fault status register (in ms) */
 #define TAS5720_FAULT_CHECK_INTERVAL		200
 
+enum tas572x_type {
+	TAS5720,
+	TAS5722,
+};
+
 static const char * const tas5720_supply_names[] = {
 	"dvdd",		/* Digital power supply. Connect to 3.3-V supply. */
 	"pvdd",		/* Class-D amp and analog power supply (connected). */
@@ -47,6 +52,7 @@ struct tas5720_data {
 	struct snd_soc_codec *codec;
 	struct regmap *regmap;
 	struct i2c_client *tas5720_client;
+	enum tas572x_type devtype;
 	struct regulator_bulk_data supplies[TAS5720_NUM_SUPPLIES];
 	struct delayed_work fault_check_work;
 	unsigned int last_fault;
@@ -264,7 +270,7 @@ static void tas5720_fault_check_work(struct work_struct *work)
 static int tas5720_codec_probe(struct snd_soc_codec *codec)
 {
 	struct tas5720_data *tas5720 = snd_soc_codec_get_drvdata(codec);
-	unsigned int device_id;
+	unsigned int device_id, expected_device_id;
 	int ret;
 
 	tas5720->codec = codec;
@@ -276,6 +282,11 @@ static int tas5720_codec_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
+	/*
+	 * Take a liberal approach to checking the device ID to allow the
+	 * driver to be used even if the device ID does not match, however
+	 * issue a warning if there is a mismatch.
+	 */
 	ret = regmap_read(tas5720->regmap, TAS5720_DEVICE_ID_REG, &device_id);
 	if (ret < 0) {
 		dev_err(codec->dev, "failed to read device ID register: %d\n",
@@ -283,13 +294,22 @@ static int tas5720_codec_probe(struct snd_soc_codec *codec)
 		goto probe_fail;
 	}
 
-	if (device_id != TAS5720_DEVICE_ID) {
-		dev_err(codec->dev, "wrong device ID. expected: %u read: %u\n",
-			TAS5720_DEVICE_ID, device_id);
-		ret = -ENODEV;
-		goto probe_fail;
+	switch (tas5720->devtype) {
+	case TAS5720:
+		expected_device_id = TAS5720_DEVICE_ID;
+		break;
+	case TAS5722:
+		expected_device_id = TAS5722_DEVICE_ID;
+		break;
+	default:
+		dev_err(codec->dev, "unexpected private driver data\n");
+		return -EINVAL;
 	}
 
+	if (device_id != expected_device_id)
+		dev_warn(codec->dev, "wrong device ID. expected: %u read: %u\n",
+			 expected_device_id, device_id);
+
 	/* Set device to mute */
 	ret = snd_soc_update_bits(codec, TAS5720_DIGITAL_CTRL2_REG,
 				  TAS5720_MUTE, TAS5720_MUTE);
@@ -552,6 +572,8 @@ static int tas5720_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	data->tas5720_client = client;
+	data->devtype = id->driver_data;
+
 	data->regmap = devm_regmap_init_i2c(client, &tas5720_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		ret = PTR_ERR(data->regmap);
@@ -592,7 +614,8 @@ static int tas5720_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id tas5720_id[] = {
-	{ "tas5720", 0 },
+	{ "tas5720", TAS5720 },
+	{ "tas5722", TAS5722 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tas5720_id);
@@ -600,6 +623,7 @@ MODULE_DEVICE_TABLE(i2c, tas5720_id);
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id tas5720_of_match[] = {
 	{ .compatible = "ti,tas5720", },
+	{ .compatible = "ti,tas5722", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, tas5720_of_match);
diff --git a/sound/soc/codecs/tas5720.h b/sound/soc/codecs/tas5720.h
index 3d077c779b12..bef802afcc69 100644
--- a/sound/soc/codecs/tas5720.h
+++ b/sound/soc/codecs/tas5720.h
@@ -32,6 +32,7 @@
 
 /* TAS5720_DEVICE_ID_REG */
 #define TAS5720_DEVICE_ID		0x01
+#define TAS5722_DEVICE_ID		0x12
 
 /* TAS5720_POWER_CTRL_REG */
 #define TAS5720_DIG_CLIP_MASK		GENMASK(7, 2)
-- 
2.15.1

Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control

[Andrew F. Davis]

On 12/12/2017 06:01 AM, Mark Brown wrote:
> On Mon, Dec 11, 2017 at 01:01:56PM -0600, Andrew F. Davis wrote:
> 
>> The TAS5722 supports modifying volume in 0.25dB steps (as opposed to 0.5dB
>> steps on the TAS5720). Introduce a custom mixer control that allows taking
>> advantage of this finer output volume granularity.
> 
> Don't do this, it's just making things more complicated.  Instead do
> what other drivers do and register different sets of controls depending
> on which part you're working with.  The normal thing is to have a big
> table for all the shared controls that are the same on all variants then
> register additional tables during probe with those that vary for the
> individul devices.
> 

That is what we are doing here, the reason for the custom mixer control
is that the controlled bits span two registers in a odd way that is not
supported by the standard handlers.

>>  static const struct snd_kcontrol_new tas5720_snd_controls[] = {
>>  	SOC_SINGLE_TLV("Speaker Driver Playback Volume",
>> -		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, dac_tlv),
>> +		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, tas5720_dac_tlv),
>> +	SOC_SINGLE_TLV("Speaker Driver Analog Gain", TAS5720_ANALOG_CTRL_REG,
>> +		       TAS5720_ANALOG_GAIN_SHIFT, 3, 0, dac_analog_tlv),
> 
> As ever all volume controls should end in Volume (like the immediately
> adjacent control does).
> 

This was done so this table exactly matches the existing table. If you
would like me to change this then I can, and can do it for the other
table as well, up to you.

Re: [PATCH 4/4] ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support

[Andrew F. Davis]

On 12/12/2017 06:02 AM, Mark Brown wrote:
> On Mon, Dec 11, 2017 at 01:01:57PM -0600, Andrew F. Davis wrote:
> 
>> +	/* Configure TDM slot width. This is only applicable to TAS5722. */
>> +	if (tas5720->devtype == TAS5722) {
>> +		ret = snd_soc_update_bits(codec, TAS5722_DIGITAL_CTRL2_REG,
>> +					  TAS5722_TDM_SLOT_16B,
>> +					  slot_width == 16 ?
>> +					  TAS5722_TDM_SLOT_16B : 0);
>> +		if (ret < 0)
>> +			goto error_snd_soc_update_bits;
>> +	}
> 
> Use a switch statement, that way additional variants can be handled more
> sensibly.
> 

Will fix.

Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

On Mon, Jan 15, 2018 at 08:50:09AM -0600, Andrew F. Davis wrote:
> On 12/12/2017 06:01 AM, Mark Brown wrote:
> > On Mon, Dec 11, 2017 at 01:01:56PM -0600, Andrew F. Davis wrote:

> >> The TAS5722 supports modifying volume in 0.25dB steps (as opposed to 0.5dB
> >> steps on the TAS5720). Introduce a custom mixer control that allows taking
> >> advantage of this finer output volume granularity.

> > Don't do this, it's just making things more complicated.  Instead do
> > what other drivers do and register different sets of controls depending
> > on which part you're working with.  The normal thing is to have a big
> > table for all the shared controls that are the same on all variants then
> > register additional tables during probe with those that vary for the
> > individul devices.

> That is what we are doing here, the reason for the custom mixer control
> is that the controlled bits span two registers in a odd way that is not
> supported by the standard handlers.

That's not clear from the commit message, it sounds like you're
introducing an an extra control rather than replacing the one that's
currently there.  

> > As ever all volume controls should end in Volume (like the immediately
> > adjacent control does).

> This was done so this table exactly matches the existing table. If you
> would like me to change this then I can, and can do it for the other
> table as well, up to you.

Of course fixes for bugs in existing code are welcome.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]