lkml.org 
[lkml]   [2021]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH 1/2] ASoC: soc-component: add snd_soc_component_read/write_field()
From
Date


On 26/01/2021 13:36, Mark Brown wrote:
> 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?

Sure, that makes it even better to validate the mask aswell!

>
>> +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?

I agree! something like this should be good I guess:

unsigned int snd_soc_component_read_field(...)
{
unsigned int val;

val = snd_soc_component_read(component, reg);

val = (val & mask) >> __soc_component_field_shift(mask);

return val;
}

>
>> + 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.
True!, we could simplify this to :

int snd_soc_component_write_field(struct snd_soc_component *component,
unsigned int reg, unsigned int mask,
unsigned int val)
{
val = (val << __soc_component_field_shift(mask)) & mask;

return snd_soc_component_update_bits(component, reg, mask, val);
}

Does that look okay to you?


--srini
>

\
 
 \ /
  Last update: 2021-01-26 15:07    [W:0.065 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site