[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aFVgVTIe5oT0MkZy@finisterre.sirena.org.uk>
Date: Fri, 20 Jun 2025 14:21:25 +0100
From: Mark Brown <broonie@...nel.org>
To: wangweidong.a@...nic.com
Cc: lgirdwood@...il.com, perex@...ex.cz, tiwai@...e.com,
zhujun2@...s.chinamobile.com, colin.i.king@...il.com,
thorsten.blum@...ux.dev, u.kleine-koenig@...libre.com,
linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
yijiangtao@...nic.com
Subject: Re: [PATCH V2] ASoC: codecs: Add calibration function to aw88399 chip
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.
> @@ -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.
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists