linux-kernel - Re: [PATCH RFC] ASoC: rt5663: use msleep() for uncritical delay

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]

Date:   Tue, 17 Jan 2017 19:33:02 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Nicholas Mc Guire <hofrat@...dl.org>
Cc:     Bard Liao <bardliao@...ltek.com>,
        Oder Chiou <oder_chiou@...ltek.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] ASoC: rt5663: use msleep() for uncritical delay

On Wed, Jan 11, 2017 at 04:03:51PM +0100, Nicholas Mc Guire wrote:
> The delay here does not seem to be critical with respect to longer
> delays than 10ms as this delay is to ensure that the write took
> effect before the next soc_update_bits/write call only, thus a 
> high resolution timer makes little sense here - msleep() should do.

No, that's not what the code is doing at all.

> +++ b/sound/soc/codecs/rt5663.c
> @@ -2764,7 +2764,7 @@ static int rt5663_set_bias_level(struct snd_soc_codec *codec,
>  			RT5663_PWR_FV1_MASK | RT5663_PWR_FV2_MASK |
>  			RT5663_PWR_MB_MASK, RT5663_PWR_VREF1 |
>  			RT5663_PWR_VREF2 | RT5663_PWR_MB);
> -		usleep_range(10000, 10005);
> +		msleep(10);

The write before is turning on a bunch of analogue power bits, the
enabled supplies will then take time to ramp up to their operating
state before we can proceed.  That's not just "make sure the change took
effect", there's a bit more to it than that, and power up sequences are
generally very latency sensitive as they tend to happen in response to
user input.  People are generally picking the minimum value they can
reliably get away with and a lot of effort goes into optimising the
power up procedures.

That doesn't mean that the change is bad but the analysis in the
changelog is and could cause confusion.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)