For tac5572 tas2883#5675
Conversation
Add codec driver for tac5xx2 family of devices. Currently the driver is supporting the following devices. 1. tac5572 - DAC, PDM, UAJ and HID 2. tas2883 - Amplifier with DSP Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
Add TI amp utility for supporting the tac5xx2 family of devices. Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
Add machine driver changes to support tac5572, tas2883 on MTL machine. Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
|
Can one of the admins verify this patch?
|
|
test this please |
bardliao
left a comment
There was a problem hiding this comment.
Just a few comments. But I was wondering can we use the sdca class driver instead of creating a new codec driver? You probably just need to add the SDW_SLAVE_ENTRY into class_sdw_id[] in sound/soc/sdca/sdca_class.c maybe with a few change if something is missing in the sdca class driver. What do you think? @charleskeepax
| .cache_type = REGCACHE_MAPLE, | ||
| .volatile_reg = tac_volatile_reg, | ||
| .readable_reg = tac_readable_reg, | ||
| .writeable_reg = tac_writable_reg, |
There was a problem hiding this comment.
Do we need writeable_reg in this case?
| .use_single_write = true, | ||
| }; | ||
|
|
||
| static int tac_write_u16(struct tac5xx2_prv *tac_dev, unsigned int reg, |
There was a problem hiding this comment.
Can we use the regular regmap_sdw_mbq_cfg, like tas2783_mbq_cfg?
| } | ||
|
|
||
| /* Check if device has UAJ (Universal Audio Jack) support */ | ||
| static bool tac_has_uaj_support(struct tac5xx2_prv *tac_dev) |
There was a problem hiding this comment.
Can we get it by the DisCo table?
| static s32 tac5xx2_amp_getvol(struct snd_kcontrol *kcontrol, | ||
| struct snd_ctl_elem_value *ucontrol) | ||
| { | ||
| return snd_soc_get_volsw(kcontrol, ucontrol); |
There was a problem hiding this comment.
Why do we need the wrapper?
| u32 gain_value; | ||
|
|
||
| /* Default to 0 dB (144 in our -72dB to 6dB scale) */ | ||
| ucontrol->value.integer.value[0] = 144; |
There was a problem hiding this comment.
Shouldn't it be the cached value or just return error if you can't read the right value?
| ret = regmap_update_bits(priv->regmap, | ||
| TAC_INT_CFG, | ||
| TAC_INT_CFG_CLR_REG, | ||
| TAC_INT_CFG_CLR_REG); |
There was a problem hiding this comment.
nitpick: The indentation seems be incorrect.
| tac_dev->hid_func_data = function_data; | ||
| else | ||
| dev_warn(dev, "hid function parse failed:err%d, using defaults", ret); | ||
| } |
There was a problem hiding this comment.
Can we parse all 4 function data in one loop? Something like
if (peripheral->sdca_data.num_functions > 0) {
dev_dbg(dev, "SDCA functions found: %d", peripheral->sdca_data.num_functions);
for (i = 0; i < peripheral->sdca_data.num_functions; i++) {
if (peripheral->sdca_data.function[i].type ==
SDCA_FUNCTION_TYPE_SMART_AMP) {
dev_dbg(dev, "Found Smart Amp function at index %d", i);
function_data = devm_kzalloc(dev, sizeof(*function_data),
GFP_KERNEL);
if (!function_data)
return dev_err_probe(dev, -ENOMEM,
"failed to parse sdca functions");
ret = sdca_parse_function(dev, peripheral,
&peripheral->sdca_data.function[i],
function_data);
if (!ret)
tac_dev->sa_func_data = function_data;
else
dev_warn(dev,
"smartamp function parse failed:err%d, using defaults", ret);
break;
}
} else if (peripheral->sdca_data.function[i].type ==
SDCA_FUNCTION_TYPE_SMART_MIC) {
...
}
}
This is the initial commit to support family of devices tac5xx2 with SoundWire interface.
This commit patch includes the support for tac5572 and tas2883.
Support for additional devices in the family will be added in the coming days.