linux-kernel - Re: [RFC PATCH 1/2] ASoC: soc-component: add snd_soc_component_read/write

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: <20210126133612.GB4839@sirena.org.uk>
Date:   Tue, 26 Jan 2021 13:36:12 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     lgirdwood@...il.com, alsa-devel@...a-project.org, perex@...ex.cz,
        tiwai@...e.com, linux-kernel@...r.kernel.org, vkoul@...nel.org
Subject: Re: [RFC PATCH 1/2] ASoC: soc-component: add
 snd_soc_component_read/write_field()

On Tue, Jan 26, 2021 at 12:20:19PM +0000, Srinivas Kandagatla wrote:

> +#define __soc_component_field_shift(x) (__builtin_ffs(x) - 1)

Why not have this be a static inline?

> +unsigned int snd_soc_component_read_field(struct snd_soc_component *component,
> +					  unsigned int reg, unsigned int mask)
> +{
> +	unsigned int val;
> +
> +	mutex_lock(&component->io_mutex);
> +	val = soc_component_read_no_lock(component, reg);
> +	if (mask)
> +		val = (val & mask) >> __soc_component_field_shift(mask);

I don't understand why this is open coding the locking when it's just a
simple read and then shift?

> +	mutex_lock(&component->io_mutex);
> +
> +	old = soc_component_read_no_lock(component, reg);
> +
> +	val = val << __soc_component_field_shift(mask);
> +
> +	new = (old & ~mask) | (val & mask);
> +
> +	change = old != new;
> +	if (change)
> +		ret = soc_component_write_no_lock(component, reg, new);
> +
> +	mutex_unlock(&component->io_mutex);

This needs the lock as it's a read/modify/write but could also be
implemented in terms of the existing update_bits() operation rather than
open coding it.

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