[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1377740630.1928.71.camel@joe-AO722>
Date: Wed, 28 Aug 2013 18:43:50 -0700
From: Joe Perches <joe@...ches.com>
To: Mischa Jonker <Mischa.Jonker@...opsys.com>
Cc: "Vineet.Gupta1@...opsys.com" <Vineet.Gupta1@...opsys.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARC: Fix __udelay parentheses
On Wed, 2013-08-28 at 19:12 +0000, Mischa Jonker wrote:
> Hello Joe,
>
> > I don't see the loops_per_jiffy initial shift << 32.
>
> loops_per_jiffy * HZ = loops_per_second
> loops_per_jiffy * HZ = 1,000,000 * loops_per_us
> loops_per_jiffy * HZ * 4295 = 4,295,000 * loops_per_us
>
> loops_per_jiffy * HZ * 4294.967296 = 2^32 * loops_per_us
>
> > > > > - loops = ((long long)(usecs * 4295 * HZ) *
> > > > > - (long long)(loops_per_jiffy)) >> 32;
> > > > > + loops = (((long long) usecs) * 4295 * HZ *
> > > > > + (long long) loops_per_jiffy) >> 32;
>
> >
> > I know that. It's the use of a signed long long vs the unsigned long long
> > that I think wrong.
>
> Yes that is wrong too.
>
> >
> > Why cast a unsigned to a signed?
>
> I don't know, this was in the original file. The issue that I was trying to solve, was that usleep didn't sleep long enough, and that is fixed by this patch.
>
> Wrt signed/unsigned: would you like me to update this patch or create a separate one?
I think the whole thing is odd and it should simply be
loops = loops_per_jiffy * usecs_to_jiffies(usecs);
and if it's really necessary to have a u64 delay
then __delay should be rewritten to take an one
as an argument and this calc should be:
u64 loops = (u64)loops_per_jiffy * usecs_to_jiffies(usecs);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists