From patchwork Thu Jul 19 05:55:05 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nishanth Menon X-Patchwork-Id: 1215231 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id D7CF03FD4F for ; Thu, 19 Jul 2012 05:59:33 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1SrjiI-0006li-G7; Thu, 19 Jul 2012 05:55:50 +0000 Received: from na3sys009aog136.obsmtp.com ([74.125.149.85]) by merlin.infradead.org with smtps (Exim 4.76 #1 (Red Hat Linux)) id 1Srji9-0006lU-3r for linux-arm-kernel@lists.infradead.org; Thu, 19 Jul 2012 05:55:43 +0000 Received: from mail-wg0-f47.google.com ([74.125.82.47]) (using TLSv1) by na3sys009aob136.postini.com ([74.125.148.12]) with SMTP ID DSNKUAehTncDmgQgfmx42EVyTJSjNWd1/8RD@postini.com; Wed, 18 Jul 2012 22:55:40 PDT Received: by wgbfa7 with SMTP id fa7so1488292wgb.28 for ; Wed, 18 Jul 2012 22:55:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:x-gm-message-state; bh=dEzjmzFzkNzr2OvSrFPnMNT+eowaJC0GbhzUJ8vglGI=; b=U95Tc8nk3el2ZECdhIH9u7q/AZD3+ax+pYwCwQ9x2fisz5BNDJOuFGhisAtgMCMZRO nYadapINqCUtIZHuzOMwdN/sTWpyXEDy6Pgko94X2xDnq9gyjF1E88zbwARnwEgiEKiv 4FWTPuWz4qX9dNfWf16V0RyOQpmMw0N7mGlNLiupI6ZuhD+fvRCt4TQlrWWblY4bBGt9 /fnmKa6mbg+N3NVBmARk5OIqmW/COrIZRl59yyyB/iiHDNRicAdnQrk5rNNajWLgp+YQ 3ax53nkYcZmceFo+FiM7m7t/qjwT3c3u5SEIHRguUEITXwzcbLf06MvsYxWdvUD5Th3s B5vw== Received: by 10.216.95.74 with SMTP id o52mr296295wef.123.1342677325230; Wed, 18 Jul 2012 22:55:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.216.136.199 with HTTP; Wed, 18 Jul 2012 22:55:05 -0700 (PDT) In-Reply-To: <1342602902.4672.91.camel@sokoban> References: <1339428387-4800-1-git-send-email-t-kristo@ti.com> <1339428387-4800-5-git-send-email-t-kristo@ti.com> <1342602902.4672.91.camel@sokoban> From: "Menon, Nishanth" Date: Thu, 19 Jul 2012 00:55:05 -0500 Message-ID: Subject: Re: [PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count To: t-kristo@ti.com X-Gm-Message-State: ALoCoQnabD+mGwS6/qIpF75kg+Vy9XNBcGqFjfZcZqK5PQMf9Ti2MZqgFjB4G7pxTlAjQkQlDLp1 X-Spam-Note: CRM114 invocation failed X-Spam-Score: -4.2 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [74.125.149.85 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: khilman@ti.com, paul@pwsan.com, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Rajendra Nayak X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Wed, Jul 18, 2012 at 4:15 AM, Tero Kristo wrote: > > On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote: > > Couple of minor comments: > > On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo wrote: > > [...] > > > /** > > > + * _omap4_update_context_lost - increment hwmod context loss counter > > > if > > > + * hwmod context was lost, and clear hardware context loss reg > > > + * @oh: hwmod to check for context loss > > > + * > > > + * If the PRCM indicates that the hwmod @oh lost context, increment > > > + * our in-memory context loss counter, and clear the RM_*_CONTEXT > > > + * bits. No return value. > > > + */ > > > +static void _omap4_update_context_lost(struct omap_hwmod *oh) > > > +{ > > > + u32 r; > > > + > > > + if (oh->prcm.omap4.context_offs == USHRT_MAX) > > > + return; > > would'nt it be better to return a dummy incremental counter instead of > > returning no context loss (count = 0)? > > I guess you are right, this way we may have some extra context restores > for modules which don't have context offs defined, rather than not > restoring them at all. Only thing I can think might prevent this is if > there are modules that never lose context but don't have context > register? How about omap5+? there has been an interesting debate ongoing with HWAUTO and context loss count handling -> since we update only on _enable, this might actually be interesting to consider: enable idle un_idle (lost context) read counter -> no update Now to handle modules that never loose context - they have to be in wakeup domain.. should we consider a flag for those? would'nt matter o5 or not, context is still the same.. this issue could be resolved if counter update is done even when a check is done. > > > > > + > > > + r = > > > omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition, > > > + > > > oh->clkdm->pwrdm.ptr->prcm_offs, > > > + oh->prcm.omap4.context_offs); > > > + > > > + if (!r) > > > + return; > > > + > > > + oh->prcm.omap4.context_lost_counter++; > > need to be careful about counter overflow. > > Well, this code can't do much for that even if it overflows... the type > for the context_lost_counter is unsigned though, maybe it should be > expanded if you are worried...? it can hit 0 with overflow(no context loss). that will not be good, right? How about something like: oh->clkdm->pwrdm.ptr->prcm_offs, Regards, Nishanth Menon diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index eac813a..5fb9572 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -1606,6 +1606,18 @@ static void _reconfigure_io_chain(void) spin_unlock_irqrestore(&io_chain_lock, flags); } +static inline void _omap4_inc_context_loss(unsigned int *v) +{ + + /* + * Context loss count has to be a non-negative value. + * Clear the sign bit to get a value range from 1 to + * INT_MAX. + */ + *v = (*v + 1) & INT_MAX; + *v = *v ? *v : 1; +} + /** * _omap4_update_context_lost - increment hwmod context loss counter if * hwmod context was lost, and clear hardware context loss reg @@ -1629,7 +1641,7 @@ static void _omap4_update_context_lost(struct omap_hwmod *oh) if (!r) return; - oh->prcm.omap4.context_lost_counter++; + _omap4_inc_context_loss(&oh->prcm.omap4.context_lost_counter); omap4_prminst_write_inst_reg(r, oh->clkdm->pwrdm.ptr->prcm_partition,