Skip to content

Comments

For tac5572 tas2883#5675

Open
niranjanhyti wants to merge 3 commits intothesofproject:topic/sof-devfrom
niranjanhyti:for-tac5572_tas2883
Open

For tac5572 tas2883#5675
niranjanhyti wants to merge 3 commits intothesofproject:topic/sof-devfrom
niranjanhyti:for-tac5572_tas2883

Conversation

@niranjanhyti
Copy link

This is the initial commit to support family of devices tac5xx2 with SoundWire interface.
This commit patch includes the support for tac5572 and tas2883.

  • tac5572 is a codec with dac, mic, uaj
  • tas2883 is amplifier with dsp, mic
    Support for additional devices in the family will be added in the coming days.

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>
@sofci
Copy link
Collaborator

sofci commented Feb 20, 2026

Can one of the admins verify this patch?

reply test this please to run this test once

@bardliao
Copy link
Collaborator

test this please

Copy link
Collaborator

@bardliao bardliao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
				...
			}
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants