From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: Re: [PATCH 2/2] x86 platform driver: intelligent power
 sharing driver
Date: Sat, 15 May 2010 09:29:08 -0700
Message-ID: <20100515092908.02cc71ba@blake>
References: <20100510142521.5a705dea@virtuousgeek.org>
	<20100510142652.12ca0824@virtuousgeek.org>
	<20100510220046.3f7cd9bf.akpm@linux-foundation.org>
	<20100515085456.GB7125@hexapodia.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from outbound-mail-158.bluehost.com (cpoproxy2-pub.bluehost.com
	[67.222.39.38])
	by gabe.freedesktop.org (Postfix) with SMTP id 23AB49E73D
	for <intel-gfx@lists.freedesktop.org>;
	Sat, 15 May 2010 09:29:35 -0700 (PDT)
In-Reply-To: <20100515085456.GB7125@hexapodia.org>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Andy Isaacson <adi@hexapodia.org>
Cc: Andrew Morton <akpm@linux-foundation.org>, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org
List-Id: intel-gfx@lists.freedesktop.org

On Sat, 15 May 2010 01:54:56 -0700
Andy Isaacson <adi@hexapodia.org> wrote:

> On Mon, May 10, 2010 at 10:00:46PM -0400, Andrew Morton wrote:
> > > +	int i;
> > > +	u16 avg;
> > > +
> > > +	for (i = 0; i < IPS_SAMPLE_COUNT; i++)
> > > +		total += (u64)(array[i] * 100);
> > 
> > Actually, that does work.  Somehow the compiler will promote
> > array[i] to u64 _before_ doing the multiplication.  I think.
> > Still, it looks like a deliberate attempt to trick the compiler
> > into doing a multiplicative overflow ;)
> 
> It seems to promote to int, probably due to the implicit type of
> "100".  Aind since array is u16, * 100 can't overflow int.  So yes,
> it's safe, but it does catch the eye as potentially unsafe.

Newest version uses do_div, I think I got it right there.

> 
> > > +		cur_seqno = (thm_readl(THM_ITV) &
> > > ITV_ME_SEQNO_MASK) >>
> > > +			ITV_ME_SEQNO_SHIFT;
> > > +		if (cur_seqno == last_seqno &&
> > > +		    time_after(jiffies, seqno_timestamp + HZ)) {
> > > +			dev_warn(&ips->dev->dev, "ME failed to
> > > update for more than 1s, likely hung\n");
> > > +		} else {
> > > +			seqno_timestamp = get_jiffies_64();
> > > +			last_seqno = cur_seqno;
> > > +		}
> > > +
> > > +		last_msecs = jiffies_to_msecs(jiffies);
> 
> Once it triggers, this will print the "likely hung" message every
> second until the end of time, won't it?

The ME should eventually reset itself, but in the interim we can't
trust its data.  So I should add some better handling for that case
(e.g. disable turbo); however this was more of a debug feature for
early MEs, I don't think it'll happen on production hardware.

Jesse