From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
To: Mark Brown <broonie@kernel.org>
Cc: Nariman Poushin <nariman@opensource.wolfsonmicro.com>,
gregkh@linuxfoundation.org, patches@opensource.wolfsonmicro.com,
linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH] regmap: Add support for sequences of writes with specified delays
Date: Tue, 26 May 2015 16:36:54 +0100 [thread overview]
Message-ID: <20150526153641.GA26432@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <20150526152100.GR21577@sirena.org.uk>
On Tue, May 26, 2015 at 04:21:00PM +0100, Mark Brown wrote:
> On Tue, May 26, 2015 at 01:39:21PM +0100, Nariman Poushin wrote:
>
> > Change-Id:Ie9e77aa48f258b353ffa7406d02e19c28d5f2a44
>
> Please don't include noise like this in upstream patches.
>
> > + if (regs[i].delay_us)
> > + udelay(regs[i].delay_us);
>
> This should be a usleep_range() at least (as checkpatch should have told
> you).
>
> > +int regmap_sequence_write(struct regmap *map, const struct reg_sequence *regs,
> > + int num_regs);
>
> It's a bit sad that this is a separate interface to the existing
> sequence writing interface (_multi_reg_write() and _patch()), and
> especially that it's a separate implementation. This means that if
> something needs a delay in the sequence it won't get to take advantage
> of any optimisations that the rest of the implementations get.
>
> Of course the fact that we used the same struct for both sequences and
> the register defaults makes this a bit annoying. We could either just
> add the extra field to the defaults and ignore it (we don't have *that*
> many defaults) or just update the existing users to use the new struct
> with the additional delay field (which is also fairly straightforward as
> we have few users right now).
If we're going to do something to avoid having another API, I prefer the
second option of updating the existing multi write to use the new structure.
The list of register default tables for the Arizona codecs is getting quite
large and adding a delay field to the defaults struct ends up with several
kBytes of wasted entries in the tables. In any case it makes some sense
in that a list of writes to be performed is not necessarily the same
conceptually as a list of register defaults.
> _______________________________________________
> patches mailing list
> patches@opensource.wolfsonmicro.com
> http://opensource.wolfsonmicro.com/cgi-bin/mailman/listinfo/patches
next prev parent reply other threads:[~2015-05-26 15:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-26 12:39 [RFC][PATCH] regmap: Add support for sequences of writes with specified delays Nariman Poushin
2015-05-26 15:21 ` Mark Brown
2015-05-26 15:36 ` Richard Fitzgerald [this message]
2015-05-27 8:27 ` Nariman Poushin
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=20150526153641.GA26432@opensource.wolfsonmicro.com \
--to=rf@opensource.wolfsonmicro.com \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nariman@opensource.wolfsonmicro.com \
--cc=patches@opensource.wolfsonmicro.com \
/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