public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: max.schwarz@online.de (Max Schwarz)
To: linux-arm-kernel@lists.infradead.org
Subject: Rockchip RK3188 I2C driver
Date: Thu, 17 Apr 2014 02:04:20 +0200	[thread overview]
Message-ID: <5059282.MitP6iZ8n1@typ> (raw)
In-Reply-To: <20140415185025.GH12304@sirena.org.uk>

On Tuesday 15 April 2014 at 19:50:25, Mark Brown wrote:

> Perhaps I'm missing something about why this is helpful but it does seem
> like the hardware designers have good drugs here.

I assume that the point is to avoid read/write cycles for bit-level set/clear.
You can do a "change bit X, leave everything else intact" in a single write 
without reading the previous value, be it from the register itself or from 
some cache.

> For non-volatile registers this should work fine if the write enable
> bits are just latched on at probe time, we'll never actually look at the
> what the hardware reads back once things are in cache.  For ones that
> are volatile we'll need to teach the framework about it...  a write
> enable mask setting that we splat into the value perhaps?

The regmap is not volatile at the moment, but it should probably be. We don't 
have documentation on the registers (or do you know more, Heiko?), so we can't 
be sure how the bits we don't know about behave.

I see three possible solutions to our problem:

1) Ideally, regmap_update_bits(map, reg, mask, val) & co would directly boil 
   down to a single register write of (mask << 16) | val, even for volatile  
   registers. That's a rather big change to regmap though, isn't it?

2) A smaller change would be to always use 0xFFFF as the write enable mask 
   while writing. That would mean that write accesses to volatile registers 
   would always need a read access before to determine the previous value, 
   though.

3) Make the GRF/syscon device give direct register access to the drivers (i.e. 
   passing a pointer to the mapped registers) and bypass regmap entirely.

   That would make sense to me because we are AFAIK not using any of regmap's   
   features here:

   * Caching is not that useful because a) we are only doing a few init-time
     accesses and b) the write-mask provides similar functionality.
   * The bus for register access is always going to be MMIO.
   * All the drivers using the GRF are rockchip-specific and can know about 
     the write-mask thing.

>From the outcome, I would prefer (1). From the complexity, (3). Thoughts?

Cheers,
  Max

  reply	other threads:[~2014-04-17  0:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15  0:19 Rockchip RK3188 I2C driver Max Schwarz
2014-04-15  8:42 ` Heiko Stübner
2014-04-15 17:25   ` Heiko Stübner
2014-04-15 17:55     ` Mark Brown
2014-04-15 18:39       ` Heiko Stübner
2014-04-15 18:50         ` Mark Brown
2014-04-17  0:04           ` Max Schwarz [this message]
2014-04-17 13:27             ` Heiko Stübner
2014-04-17 23:10               ` Mark Brown
2014-04-17 18:38             ` Mark Brown
2014-04-17 23:06               ` Max Schwarz
2014-04-18  9:06                 ` Heiko Stübner
2014-04-18  9:30                   ` Max Schwarz
2014-04-18 10:03                     ` Heiko Stübner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5059282.MitP6iZ8n1@typ \
    --to=max.schwarz@online.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox