lkml.org 
[lkml]   [2011]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] regmap: Support half writes and padding between register and value.
    On 09/08/11 17:27, Mark Brown wrote:
    > On Thu, Sep 08, 2011 at 03:09:23PM +0100, Jonathan Cameron wrote:
    >
    >> Note half writes currently assume address numbers are even only.
    >> That's a pain for caching so other suggestions welcome. I could set
    >> it as a 7 bit address and increase the padding to 9 bits. That makes
    >> the write bit a little strange though as it will be going into the
    >> padding.
    >
    > This needs more documentation somewhere explaining what a half write is
    > and/or a better name for half write but doesn't look too invasive so I
    > think this approach can work. If we come up with something better later
    > then we can always change things, there shouldn't be too many users to
    > update if that happens.
    Agreed. I'll document it thoroughly and explain the restricted use case.
    >
    >> - map->format.reg_bytes = config->reg_bits / 8;
    >> + map->format.buf_size = (config->reg_bits +
    >> + config->reg_pad_bits +
    >> + config->val_bits) / 8;
    >> + map->format.reg_bytes = (config->reg_bits + config->reg_pad_bits)/ 8;
    >
    > Please do a patch adding padding separately - that does seem like a
    > useful thing to have in general, with the option of having it both
    > before and after the register.
    Happy to do it separately, but not sure the padding before the register
    address is a real use case. You pad after to give the device time to
    respond, before doesn't make much sense to me on a register based device...
    I'll change the naming to make it explicit that it is after the register though
    so as to leave room for this to be added in future.

    >
    >> - if (val == map->work_buf + map->format.reg_bytes)
    >> - ret = map->bus->write(map->dev, map->work_buf,
    >> - map->format.reg_bytes + val_len);
    >> - else if (map->bus->gather_write)
    >> + if (val == map->work_buf + map->format.reg_bytes) {
    >> + if (map->format.half_write) {
    >> + ret = map->bus->write(map->dev, map->work_buf,
    >> + (map->format.reg_bytes +
    >> + val_len) >> 1);
    >> + if (ret >= 0)
    >> + ret = map->bus->write(map->dev,
    >> + map->work_buf +
    >> + ((map->format.reg_bytes +
    >> + val_len) >> 1),
    >> + (map->format.reg_bytes +
    >> + val_len) >> 1);
    >> + } else {
    >> + ret = map->bus->write(map->dev, map->work_buf,
    >> + map->format.reg_bytes + val_len);
    >> + }
    >
    > This code is getting very complicated... I think it'd be clearer to
    > have a special case at the head of the function that does the half write
    > stuff. It also feels like the half bit needs parameterisation, but I
    > can't immediately think of how to do that sensibly.
    Likewise. It's a very specific case. In theory there are two parameters that
    I can think of.

    write_length (bits per write)
    address_mangler (one for each write needed?)

    I'll clean it up and repost.
    >



    \
     
     \ /
      Last update: 2011-09-09 11:39    [W:0.028 / U:0.040 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site