Date: Tue, 26 May 2015 16:36:54 +0100
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
Message-ID: <20150526153641.GA26432@opensource.wolfsonmicro.com>
References: <20150526123921.GA19072@opensource.wolfsonmicro.com> <20150526152100.GR21577@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150526152100.GR21577@sirena.org.uk>
User-Agent: Mutt/1.5.17+20080114 (2008-01-14)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2178
Lines: 47

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/