Messages in this thread | | | Date | Mon, 14 Jan 2013 09:08:27 -0800 | Subject | Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API | From | Andrey Smirnov <> |
| |
On Sun, Jan 13, 2013 at 3:18 PM, Mark Brown <broonie@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).
| |