lkml.org 
[lkml]   [2013]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API
    From
    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).


    \
     
     \ /
      Last update: 2013-01-14 19:01    [W:2.757 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site