Messages in this thread | | | Date | Wed, 27 May 2015 09:27:25 +0100 | From | Nariman Poushin <> | Subject | Re: [RFC][PATCH] regmap: Add support for sequences of writes with specified delays |
| |
On Tue, May 26, 2015 at 04:36:54PM +0100, Richard Fitzgerald wrote: > 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
My bad, should have removed it.
> > > > 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).
Checkpatch did not complain, but I take your point.
> > > > > +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. >
Yes, the initial discussion was that it was increasing the memory usage of the register defaults table (like Richard says some arizona tables have thousands of defaults).
I am happy to resend using an updated implementation of _multi_reg_write to use the reg_sequence struct, and update the current users.
Thanks Nariman
> > _______________________________________________ > > patches mailing list > > patches@opensource.wolfsonmicro.com > > http://opensource.wolfsonmicro.com/cgi-bin/mailman/listinfo/patches >
| |