linux-kernel - Re: [PATCH V2] ASoC: codecs: Add calibration function to aw88399 chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250624065748.60509-1-wangweidong.a@awinic.com>
Date: Tue, 24 Jun 2025 14:57:48 +0800
From: wangweidong.a@...nic.com
To: broonie@...nel.org
Cc: colin.i.king@...il.com,
	lgirdwood@...il.com,
	linux-kernel@...r.kernel.org,
	linux-sound@...r.kernel.org,
	perex@...ex.cz,
	thorsten.blum@...ux.dev,
	tiwai@...e.com,
	u.kleine-koenig@...libre.com,
	wangweidong.a@...nic.com,
	yijiangtao@...nic.com,
	zhujun2@...s.chinamobile.com
Subject: Re: [PATCH V2] ASoC: codecs: Add calibration function to aw88399 chip

Thank you very much for your review

On Fri, Jun 20, 2025 at 14:21:25PM +0100, broonie@...nel.org wrote:
> On Fri, Jun 20, 2025 at 07:08:44PM +0800, wangweidong.a@...nic.com wrote:

>> +static int aw_cali_svc_dev_cali_re(struct aw88399 *aw88399)
>> +{
>> +	struct aw_device *aw_dev = aw88399->aw_pa;
>> +	int ret;
>> +
>> +	mutex_lock(&aw88399->lock);
>> +	aw_cali_svc_run_mute(aw_dev, CALI_RESULT_NORMAL);
>> +
>> +	ret = aw_cali_svc_cali_re_mode_enable(aw_dev, true);
>> +	if (ret) {
>> +		dev_err(aw_dev->dev, "start cali re failed\n");
>> +		goto re_mode_err;
>> +	}
>> +
>> +	msleep(3000);

> Callibration takes 3s which is a fairly long time.

This time is because the chip needs to wait for 
the data to stabilise during calibration 
in order to ensure more accurate calibration values.
I will reduce this time to 1s.

>> @@ -1588,6 +1968,13 @@ static int aw88399_re_get(struct snd_kcontrol *kcontrol,
>>  	struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
>>  	struct aw88399 *aw88399 = snd_soc_component_get_drvdata(codec);
>>  	struct aw_device *aw_dev = aw88399->aw_pa;
>> +	int ret;
>> +
>> +	if (aw_dev->status) {
>> +		ret = aw_cali_svc_dev_cali_re(aw88399);
>> +		if (ret)
>> +			return -EPERM;
>> +	}

> AFAICT it's triggered if the device is powered on and userspace reads
> the control that reports the callibration value.  That seems like it's a
> bit too easy to trigger - something like running amixer would read the
> control and lock the CODEC up for 3s, and I'm guessing that if the CODEC
> is powered due to audio playing that'd result in disruption to users
> listening to that audio.  I think it'd be better to have another write
> only control (or volatile one which reads 0 always) that triggers the
> calibration when userspace writes to it ("Calibrate Now Switch" or
> something).  Since the calibration is also directly writable from
> userspace we can't just use a write to this control.  In general it
> should always be possible to read controls without disrupting anything
> else that's going on.

> BTW since the calibration is dynamically done the control should be
> flagged as volatile.
I will make the following two changes:
1.Add an additional "Calib Switch" Kcontrol to 
  indicate whether calibration operations can be performed.
2.Add "Trigger Calib" to Kcontrol. 
  This Kcontrol is write-only. When calibration is allowed,
  perform the calibration operation.

Thank you again.

Best regards,
Weidong Wang