On Sun, Jan 13, 2013 at 3:18 PM, Mark Brown
<broo...@opensource.wolfsonmicro.com> wrote:
> On Sat, Jan 12, 2013 at 12:54:14PM -0800, Andrey Smirnov wrote:
>
>> + bool cache_registers;
>> +
>
> I'm afraid I don't quite understand this...
>
>> - if (!map->cache_bypass && map->format.format_write) {
>> + if (!map->cache_bypass && map->cache_registers) {
>> ret = regcache_write(map, reg, val);
>
> ...I think it's mostly to service this check here, but looking at the
> code I can't quite think why the code is doing what it's doing - I
> suspect we should just remove the check for format_write() here. I
> think it was there to support potential bulk writes from the cache code
> (which we don't do yet) but we're not actually doing those and it's not
> clear that this should be doing that anyway.
>
>From my understanding of the code it is done because the caching is
handled differently for cases when format_write() and format_reg(),
format_val() are provided.
In case of 'format_write' the regcache_write() is called in
_regmap_write() directly whereas when format_reg(), format_val() are
there _regmap_write() calls _regmap_raw_write() which in turn calls
regcache_write(). If I remove that variable and corresponding check
then regcache_write() would be called twice in the case of
format_reg(), format_val(), when _regmap_write() is called, would it
not? I apologise if I miss something obvious and that is not a case(or
issue).
>> int ret;
>> + void *context = (map->bus) ? map : map->bus_context;
>> +
>
> Can you please make this a static inline regmap_map_get_context() or
> something? The same thing appears in quite a few places and the terery
> operator isn't great at the best of times.
>
Will do in the next version of this patch.
Completely unrelated off-topic question: the following code in
regmap_bulk_write()
...
if (map->use_single_rw) {
for (i = 0; i < val_count; i++) {
ret = regmap_raw_write(map,
reg + (i * map->reg_stride),
val + (i * val_bytes),
val_bytes);
if (ret != 0)
return ret;
}
}
....
Would it not deadlock, since both regmap_bulk_write() and
regmap_raw_write() call map->lock(...)?
Once again, sorry for off-topic, I just didn't want to start a new
thread because of what very well may by my poor understanding of the
code(I'll submit a patch to change regmap_raw_write() to
_regmap_raw_write() If I am correct).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
