From: Chris Packham <judge.packham@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [Resend RFC PATCH v2] mips: Use unsigned int when reading c0 registers
Date: Wed, 15 Jul 2015 09:12:43 +1200 [thread overview]
Message-ID: <CAFOYHZCxKx1nRX5xgcPqDSyCH8qsLYgxDfUJH3Yoy-9atDAM2Q@mail.gmail.com> (raw)
In-Reply-To: <55A55C89.3030708@gmail.com>
On Wed, Jul 15, 2015 at 7:01 AM, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> Hi Chris,
>
> sorry for the delay.
No problem. It only just occurred to me that it's probably peak
holiday season for people in the northern hemisphere.
> Am 14.07.2015 um 12:54 schrieb Chris Packham:
>> In commit a18a477 (MIPS: use common code from lib/time.c) MIPS platforms
>> started using common the common timer functions which are based around
>> the fact that many platforms have a 32-bit free running counter register
>> that can be used see commit 8dfafdd (Introduce common timer functions).
>>
>> Even MIPS64 has such a 32-bit register (some have an additional 64-bit free
>> running counter, but that's something for another time).
>>
>> The problem is that in __read_32bit_c0_register() we read the value from
>> this register into an _signed_ int and as it's returned up the call
>> chain to timer_read_counter() it gets assigned to an unsigned long. On a
>> 32-bit system there is no problem. On a 64-bit system odd things happen,
>> sign extension seems to kick in and all of a sudden if the counter
>> register happens to have the MSb (i.e. the sign bit) set the negative
>> int gets sign extended into a very large unsigned long value. This in
>> turn throws out things from get_ticks() up.
>>
>> Update __read_32bit_c0_register() and __read_32bit_c0_ctrl_register() to
>> use "unsigned int res;" instead of "int res;". There seems to be little
>> reason to treat these register values as signed. They are either
>> counters (which by definition are unsigned) or are made up of various
>> bit fields to be interpreted as per the CPU datasheet.
>
> I agree that those macros should always use unsigned int's. Also some
> similar but newer macros use unsigned int's. But that header file is
> imported from Linux kernel and I'd like to keep it in sync. Could you
> post a similar patch to Linux MIPS mailing list? Maybe someone there
> know why signed int's are used and if a change would have side-effects.
> Thanks.
OK I'll go looking there, they may have already fixed it.
>
>>
>> Reported-by: Sachin Surendran <sachin.surendran@alliedtelesis.co.nz>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - Use Rob's current email address
>>
>> arch/mips/include/asm/mipsregs.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
>> index 3571e4f..c7a0849 100644
>> --- a/arch/mips/include/asm/mipsregs.h
>> +++ b/arch/mips/include/asm/mipsregs.h
>> @@ -594,7 +594,7 @@ do { \
>> */
>>
>> #define __read_32bit_c0_register(source, sel) \
>> -({ int __res; \
>> +({ unsigned int __res; \
>> if (sel == 0) \
>> __asm__ __volatile__( \
>> "mfc0\t%0, " #source "\n\t" \
>> @@ -676,7 +676,7 @@ do { \
>> * On RM7000/RM9000 these are uses to access cop0 set 1 registers
>> */
>> #define __read_32bit_c0_ctrl_register(source) \
>> -({ int __res; \
>> +({ unsigned int __res; \
>> __asm__ __volatile__( \
>> "cfc0\t%0, " #source "\n\t" \
>> : "=r" (__res)); \
>>
>
> --
> - Daniel
next prev parent reply other threads:[~2015-07-14 21:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-14 10:54 [U-Boot] [Resend RFC PATCH v2 0/1] MIPS64 timer bug Chris Packham
2015-07-14 10:54 ` [U-Boot] [Resend RFC PATCH v2] mips: Use unsigned int when reading c0 registers Chris Packham
2015-07-14 19:01 ` Daniel Schwierzeck
2015-07-14 21:12 ` Chris Packham [this message]
2015-07-16 0:14 ` Chris Packham
2015-07-18 9:29 ` Daniel Schwierzeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFOYHZCxKx1nRX5xgcPqDSyCH8qsLYgxDfUJH3Yoy-9atDAM2Q@mail.gmail.com \
--to=judge.packham@gmail.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.