Uh oh!
There was an error while loading. Please reload this page.
For tac5572 tas2883 - #5675
Conversation
sofci
commented
Feb 20, 2026
Can one of the admins verify this patch?
|
bardliao
commented
Feb 23, 2026
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?
There was a problem hiding this comment.
We know the part IDs which support UAJ. Do we really need this ?
There was a problem hiding this comment.
Hmm, I think the question is whether the UAJ function is optional? Like some monolithic codecs, the functions are optional and will not show up if the function is not described in the DisCo table. Also, we check the presentation in the machine driver.
is_sdca_endpoint_present
| static s32 tac5xx2_amp_getvol(struct snd_kcontrol *kcontrol, | ||
| struct snd_ctl_elem_value *ucontrol) | ||
| { | ||
| return snd_soc_get_volsw(kcontrol, ucontrol); |
| 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)
returndev_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;
elsedev_warn(dev,
"smartamp function parse failed:err%d, using defaults", ret);
break;
}
} elseif (peripheral->sdca_data.function[i].type==SDCA_FUNCTION_TYPE_SMART_MIC) {
...
}
}4c24815 to
5e26ae9Compare654b326 to
619545dCompareniranjanhyti
commented
Mar 11, 2026
@bardliao I have rebased and fixed some minor issues. Request you to review the latest changes. |
| .dai_num = 3, | ||
| }, | ||
| { | ||
| .part_id = 0x5682, |
There was a problem hiding this comment.
Realtek rt5682 uses the same part_id. We need to add vendor_id field in the asoc_sdw_codec_info struct.
There was a problem hiding this comment.
I will investigate and come back on this.
| .group_id = 1, | ||
| }; | ||
| static const struct snd_soc_acpi_endpoint tac5xx2_endpoints[] = { |
There was a problem hiding this comment.
In commit message, it is more like adding ACPI match table items not a machine driver change.
619545d to
8258520Compare| .aux_num = 1, | ||
| }, | ||
| { | ||
| .part_id = 0xaaaa, /* generic codec mockup */ |
db8f726 to
0264a6bComparebardliao
commented
Mar 13, 2026
@niranjanhyti The "ASoC: sdw_utils: add vendor_id to asoc_sdw_codec_info" commit should be in the front of "ASoC: sdw_utils: TI amp utility for tac5xx2 family". Please reorder the commits, thanks. |
struct asoc_sdw_codec_info has part_id which is not sufficient to uniquely identify devices. This change adds the vendor_id field and updates the codec_info list with the corresponding vendor id as per the Manufacturer's id in https://mid.mipi.org/ Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
Add codec driver for tac5xx2 family of devices. This includes the support for 1. tac5572 - DAC, PDM, UAJ and HID 2. tas2883 - Amplifier with DSP 3. tac5672 - Similar to tac5572 with feedback 4. tac5682 - Similar to tac5672 with Amplifier DSP. Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
Add TI amp utility for supporting the tac5xx2 family of devices to support tac5572, tac5672, tac5682 and tas2883 Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
Add acpi match entries to support TI's tac5572, tas2883, tac5672 and tac5682 on link 0 on MTL machine. Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
0264a6b to
f022887Compareniranjanhyti
commented
Mar 20, 2026
@bardliao - does the changes look okay. Any major concerns ? Can I start upstreaming process to the sound branch ? |
bardliao
commented
Mar 20, 2026
Sure, please go ahead and feel free to add my Reviewed-by tag |
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.