mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Jorge Ramirez-Ortiz <jramirez@baylibre.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC] regmap: allow volatile register writes with cached only read maps
Date: Thu, 17 May 2018 18:04:11 +0100	[thread overview]
Message-ID: <20180517170410.GU20254@sirena.org.uk> (raw)
In-Reply-To: <201167c6-304c-138a-358f-bee096767527@baylibre.com>

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On Thu, May 17, 2018 at 09:12:49AM +0200, Jorge Ramirez-Ortiz wrote:
> On 05/13/2018 04:22 AM, Mark Brown wrote:

> > > So I dont see where/how your recommendation fits; maybe you could clarify a
> > > bit more please?

> > As I've been saying if you explicitly need a write to happen don't use
> > regmap_update_bits(), do something that guarantees you'll get a write
> > like regmap_write() or regmap_write_bits().

> I do understand your point but Mark, that interface you mention sits above
> the user request (as a client the user does not call regmap_update_bits or
> regmap_write_bits or regmap_write(): none of those functions mean anything
> to the foo_regmap definition below - that is why we have an interface).

...

> and all this request means is that it expects foo_volatile_regs to be taken
> into consideration when accessing the reg_write callback: so whoever is
> calling the interface reg_write (be it regmap_update_bits or
> regmap_write_bits or whoever it is, I dont know) must make sure the volatile
> request applies.

Right, but your code can expect a lot of things and not have them happen
if they're not good expectations - that's explicitly not what we've been
meaning by volatile registers (they're just registers that can't be
cached).  I mean, you mentioned that you were doing this for an ALSA
control and since ALSA reports noop updates to userspace it'd be
entirely reasonable for an implementation change there were to suppress
the write before it gets to regmap (this is the sort of thing I was
thinking of when I said I was suspicious of what you're doing there,
this doesn't sound like a normal ALSA control).

I get what you're saying, it just doesn't feel like this is the right
place to solve whatever your end use case is, it feels like it's going
to be fragile one way or another and end up adding more complexity -
having a hand coded ALSA control for this feels more secure at that
level never mind anything that's going on in regmap. 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      reply	other threads:[~2018-05-17 17:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08 22:06 Jorge Ramirez-Ortiz
2018-05-09  8:39 ` Mark Brown
2018-05-09 11:49   ` Jorge Ramirez-Ortiz
2018-05-11  2:00     ` Mark Brown
2018-05-11 10:29       ` Jorge Ramirez-Ortiz
2018-05-13  2:22         ` Mark Brown
2018-05-17  7:12           ` Jorge Ramirez-Ortiz
2018-05-17 17:04             ` Mark Brown [this message]

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=20180517170410.GU20254@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=jramirez@baylibre.com \
    --cc=linux-kernel@vger.kernel.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

all inboxes | Powered by JetHome®