ASoC: codecs: tas5720: add basic support for TAS5722 devices
Subsystems (3)
PATCH 1/4 ASoC: codecs: tas5720: add basic support for TAS5722 devices
From: Andreas Dannenberg <[email protected]>
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.
--- .../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(-)
Documentation/devicetree/bindings/sound/tas5720.txt +3 -1
@@ -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
sound/soc/codecs/tas5720.c +31 -7
@@ -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);
sound/soc/codecs/tas5720.h +1 -0
@@ -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)
Signature · 1 line
2.15.0
PATCH 2/4 ASoC: codecs: tas5720: add TAS5722 register support
From: Andreas Dannenberg <[email protected]>
Introduce a custom super-set register map and associated bit definitions to allow driver access to all TAS5722 device functionality.
--- sound/soc/codecs/tas5720.c | 23 ++++++++++++++++++++++- sound/soc/codecs/tas5720.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-)
sound/soc/codecs/tas5720.c +22 -1
@@ -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);
sound/soc/codecs/tas5720.h +30 -0
@@ -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__ */
Signature · 6 lines
2.15.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Brown
maintainerApplied "ASoC: tas5720: add TAS5722 register support" to the asoc tree
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
Quoted 1 line
From
d5eb436acc8104b5c789359aa8c923ff5fafcd62Mon Sep 17 00:00:00 2001
From: Andreas Dannenberg <[email protected]>
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.
--- sound/soc/codecs/tas5720.c | 23 ++++++++++++++++++++++- sound/soc/codecs/tas5720.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-)
sound/soc/codecs/tas5720.c +22 -1
@@ -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);
sound/soc/codecs/tas5720.h +30 -0
@@ -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__ */
Signature · 1 line
2.15.1
PATCH 3/4 ASoC: codecs: tas5720: add TAS5722 specific volume control
In reply to Andrew F. DavisFrom: Andreas Dannenberg <[email protected]>
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.
--- sound/soc/codecs/tas5720.c | 88 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 5 deletions(-)
sound/soc/codecs/tas5720.c +83 -5
@@ -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;
Signature · 1 line
2.15.0
Mark Brown
maintainerRe: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control
Quoted 3 lines · Andrew F. Davis
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.
sound/soc/codecs/tas5720.c static const DECLARE_TLV_DB_RANGE(dac_analog_tlv, · line 501
Quoted 6 lines
static const struct
snd_kcontrol_newtas5720_snd_controls[] = {sound/soc/codecs/tas5720.c +3
-1SOC_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).
Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control
Quoted 6 lines · Mark Brown
Quoted 3 lines · Andrew F. Davis
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.
Quoted 7 lines
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.
Quoted 10 lines
sound/soc/codecs/tas5720.c static const DECLARE_TLV_DB_RANGE(dac_analog_tlv, · line 501
Quoted 6 lines
static const struct
snd_kcontrol_newtas5720_snd_controls[] = {sound/soc/codecs/tas5720.c +3
-1SOC_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.
Mark Brown
maintainerRe: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control
Quoted 14 lines · Andrew F. Davis
Quoted 6 lines · Mark Brown
Quoted 3 lines · Andrew F. Davis
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.
Quoted 6 lines
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.
Quoted 3 lines
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.
Quoted 6 lines
Quoted 2 lines
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.
PATCH 4/4 ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support
In reply to Andrew F. DavisFrom: Andreas Dannenberg <[email protected]>
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.
--- sound/soc/codecs/tas5720.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
sound/soc/codecs/tas5720.c +11 -0
@@ -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:
Signature · 6 lines
2.15.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Brown
maintainerRe: [PATCH 4/4] ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support
sound/soc/codecs/tas5720.c static int tas5720_set_dai_tdm_slot(struct snd_soc_dai *dai, · line 197
Quoted 9 lines · Andrew F. Davis
sound/soc/codecs/tas5720.c +9
-0+/* 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.
Re: [PATCH 4/4] ASoC: codecs: tas5720: add TAS5722 TDM slot width setting support
Quoted 15 lines · Mark Brown
sound/soc/codecs/tas5720.c static int tas5720_set_dai_tdm_slot(struct snd_soc_dai *dai, · line 197
Quoted 9 lines · Andrew F. Davis
sound/soc/codecs/tas5720.c +9
-0+/* 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.
Mark Brown
maintainerApplied "ASoC: tas5720: add basic support for TAS5722 devices" to the asoc tree
In reply to Andrew F. DavisThe 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
Quoted 1 line
From
872bcad246e30f0ae8009a0f8c13874009601445Mon Sep 17 00:00:00 2001
From: Andreas Dannenberg <[email protected]>
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.
--- .../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(-)
Documentation/devicetree/bindings/sound/tas5720.txt +3 -1
@@ -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
sound/soc/codecs/tas5720.c +31 -7
@@ -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);
sound/soc/codecs/tas5720.h +1 -0
@@ -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)
Signature · 1 line
2.15.1