Re: [RFC PATCH 8/8] ASoC: codecs: add Qualcomm Tambora (WCD9378) SDCA codec

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

On 8/5/26 5:04 PM, Charles Keepax wrote:
> On Thu, Jul 23, 2026 at 12:42:18AM +0100, Srinivas Kandagatla wrote:
>> Add support for the Qualcomm Tambora (WCD9378) headset codec in SDCA
>> mode over SoundWire.
>>
>> WCD9378 ("Tambora") is a low-power stereo audio codec used across
>> Qualcomm mobile and compute platforms.  It integrates a headphone
>> amplifier, mono earpiece output, three analogue microphone inputs
>> with independent programmable mic-bias supplies, jack and button
>> detection, and SoundWire dataports for both playback and capture.
>>
>> Assisted-by: Claude:claude-opus-4-7
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>> ---
>> +	if (entity_id == WCD9378_PDE34_ENTITY_ID) {
>> +		unsigned int mic_bias = SDW_SDCA_CTL(function_id,
>> +						     WCD9378_IT33_ENTITY_ID,
>> +						     WCD9378_IT33_MICBIAS_SEL, 0);
>> +
>> +		regcache_drop_region(regmap, mic_bias, mic_bias);
>> +		ret = regmap_write(regmap, mic_bias, ctrl_it33_micbias_vals[0]);
>> +		if (ret)
>> +			dev_err(&slave->dev, "IT33 MIC_BIAS: %d\n", ret);
>> +	} else if (entity_id == WCD9378_PDE47_ENTITY_ID) {
>> +		/* Announce commit-group count for the FU42 writes in pde_post_pmu. */
>> +		ret = regmap_write(core->dev_regmap,
>> +				   WCD9378_ENT0_COMMIT_GROUP_COUNT_ADDR,
>> +				   WCD9378_ENT0_COMMIT_GROUP_COUNT_TWO);
>> +		if (ret)
>> +			dev_err(&slave->dev, "Commit_Group_Count: %d\n", ret);
>> +
>> +		/* Let the master's WR FIFO drain before PDE47_REQ_PS=0. */
>> +		usleep_range(2000, 2500);
> 
> Feels a bit odd using these callbacks for commit groups, really
> that is a thing that should be implemented more generically. That
> is a pretty serious task, so I can probably look past this as
> vendor specific thing for now. I guess my only points would be
> can you live without the commit groups and just use the current
> values? And if not, are you prepared that there is a good chance
> this gets missed when someone does implement commit groups and
> this breaks.
> 
Mos of this is gone now in v2.

>> +	/*
>> +	 * Unmute FU42 CH1/CH2 and stage 0 dB volume via the _CN (NEXT) alias
>> +	 * so all four staged writes activate together on the SCP_COMMIT
>> +	 * below.  Writes go to the vendor slave regmap because FU42
>> +	 * addresses are outside the SDCA class regmap's routing.
>> +	 */
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_MUTE_CH1_ADDR,
>> +			   WCD9378_FU42_MUTE_UNMUTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 MUTE_CH1: %d\n", ret);
>> +
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_MUTE_CH2_ADDR,
>> +			   WCD9378_FU42_MUTE_UNMUTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 MUTE_CH2: %d\n", ret);
>> +
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_CH_VOL_CH1_MSB_ADDR,
>> +			   WCD9378_FU42_CH_VOL_UNITY_BYTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 CH1_MSB: %d\n", ret);
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_CH_VOL_CH1_LSB_ADDR,
>> +			   WCD9378_FU42_CH_VOL_UNITY_BYTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 CH1_LSB: %d\n", ret);
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_CH_VOL_CH2_MSB_ADDR,
>> +			   WCD9378_FU42_CH_VOL_UNITY_BYTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 CH2_MSB: %d\n", ret);
>> +	ret = regmap_write(core->dev_regmap, WCD9378_FU42_CH_VOL_CH2_LSB_ADDR,
>> +			   WCD9378_FU42_CH_VOL_UNITY_BYTE);
>> +	if (ret)
>> +		dev_err(&slave->dev, "FU42 CH2_LSB: %d\n", ret);
> 
> These should all really be part of the function regmap they are
> function controls. Also would mean you have the MBQ so you don't
> need to do two writes per register. I assume, with the commit
> group stuff, these are writing the next value rather than the
> current? But nothing here updates the functions regmap for the
> new values, is it ok that those two get out of sync? Or do we
> not define these controls at the function level?

These are now part of the core in v2 which exposes them as proper mixer
controls.

These work fine as long as we apply the mixers after the PDE transition.
Which is not what user is expecting.

This behavior is because the codec seems to reset some registers to
default when we transition from PS3->PS0 state, volume, mute and micbias
are going back to default values, leaving the stream in mute.

I can try to re-apply them in the codec callback hook, but in general
how should we handle this in sdca core?


--srini


> 
> Thanks,
> Charles