Messages in this thread | | | Date | Tue, 26 Jan 2021 13:36:12 +0000 | From | Mark Brown <> | 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. [unhandled content-type:application/pgp-signature] | |