lkml.org 
[lkml]   [2015]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
>


\
 
 \ /
  Last update: 2015-05-27 11:01    [W:0.285 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site