From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752647AbbE0I1c (ORCPT ); Wed, 27 May 2015 04:27:32 -0400 Received: from opensource.wolfsonmicro.com ([80.75.67.52]:35295 "EHLO opensource.wolfsonmicro.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751799AbbE0I13 (ORCPT ); Wed, 27 May 2015 04:27:29 -0400 Date: Wed, 27 May 2015 09:27:25 +0100 From: Nariman Poushin To: Richard Fitzgerald Cc: Mark Brown , 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: <20150527082725.GA28236@opensource.wolfsonmicro.com> References: <20150526123921.GA19072@opensource.wolfsonmicro.com> <20150526152100.GR21577@sirena.org.uk> <20150526153641.GA26432@opensource.wolfsonmicro.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150526153641.GA26432@opensource.wolfsonmicro.com> User-Agent: Mutt/1.5.17+20080114 (2008-01-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >