For tac5572 tas2883#5675
Conversation
niranjanhyti
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
plbossart,
ranj063 and
ujfalusi
as code owners
|
Can one of the admins verify this patch?
|
|
test this please |
bardliao
left a comment
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
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| .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?
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| .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
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| 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?
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| 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?
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| 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.
sound/soc/codecs/tac5xx2-sdw.c
Outdated
| 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) {
...
}
}
niranjanhyti
force-pushed
the
for-tac5572_tas2883
branch
2 times, most recently
from
4c24815 to
5e26ae9
Compare
niranjanhyti
force-pushed
the
for-tac5572_tas2883
branch
from
5e26ae9 to
654b326
Compare
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>
niranjanhyti
force-pushed
the
for-tac5572_tas2883
branch
from
654b326 to
619545d
Compare
|
@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.
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>
niranjanhyti
force-pushed
the
for-tac5572_tas2883
branch
from
619545d to
8258520
Compare
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>
| .aux_num = 1, | ||
| }, | ||
| { | ||
| .part_id = 0xaaaa, /* generic codec mockup */ |
There was a problem hiding this comment.
@bardliao what should be the vendor_id IDs for mockup list ?
3 participants
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.