All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kevin D. Kissell" <kevink@paralogos.com>
To: Sergei Shtylyov <sshtylyov@mvista.com>
Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>,
	ralf@linux-mips.org, mcdonald.shane@gmail.com,
	linux-mips@linux-mips.org
Subject: Re: [MIPS] FPU emulator: allow Cause bits of FCSR to be writeable by ctc1
Date: Thu, 06 May 2010 08:46:13 -0700	[thread overview]
Message-ID: <4BE2E445.5050809@paralogos.com> (raw)
In-Reply-To: <4BE2A6EE.80705@mvista.com>

Sergei Shtylyov wrote:
> Kevin D. Kissell wrote:
>
>> I'm cool with the patch as is, but in the general spirit of regarding
>> numeric constants other than 0 and 1 as instruments of Satan, it
>> would probably be even better if those reserved bits were defined
>> (FPU_CSR_RSVD, or whatever is compatible with existing convention for
>> such bits) along with the other FCSR bit masks in mipsregs.h, so that
>> the assigment looks like:
>>
>>          ctx->fcr31 = (value & ~(FPU_CSR_RSVD | 0x3)) |
>>                   ieee_rm[value & 0x3];
>
>   0x3 is still neither 0 nor 1, and so remains an instrument of Satan.
> How about #defining it also? :-)
In some software engineering cultures, that would be considered a good
idea or even mandatory.  This being the Linux kernel, I think it's OK,
because, as Shane remarked, it's a mask that's local to the module and
whose value is "obvious", and such things are pretty commonly handled as
numeric constants in the kernel.

There actually *is* a #define for that field, FPU_CSR_RD, which could be
used here in place of the 0x3, but I'm a little torn about its use.  On
one hand "value & ~(FPU_CSR_RSVD | FPU_CSR_RD)" is more clear about what
we're doing, but on the other hand, it's less obvious that "value &
FPU_CSR_RD" is generating an integer in the range 0-3 for an index.  So
I'm absolutely fine with the code as written, but wouldn't complain if
someone wanted to make it use the symbolic constant.

          Yours in excessive geekery,

          Kevin K.

  reply	other threads:[~2010-05-06 15:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-05  4:02 [MIPS] FPU emulator: allow Cause bits of FCSR to be writeable by ctc1 Shane McDonald
2010-05-05  7:48 ` Kevin D. Kissell
2010-05-05  9:11   ` Ralf Baechle
2010-05-05 15:43     ` Kevin D. Kissell
2010-05-05 16:22       ` Atsushi Nemoto
2010-05-05 19:20         ` Kevin D. Kissell
2010-05-06 11:24           ` Sergei Shtylyov
2010-05-06 15:46             ` Kevin D. Kissell [this message]
     [not found]               ` <o2yb2b2f2321005061142v431dbc78n2a21722676a72501@mail.gmail.com>
2010-05-06 18:47                 ` Ralf Baechle
2010-05-06 19:06                 ` Kevin D. Kissell
2010-05-08 16:46           ` Atsushi Nemoto
2010-05-06  9:01       ` Ralf Baechle

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=4BE2E445.5050809@paralogos.com \
    --to=kevink@paralogos.com \
    --cc=anemo@mba.ocn.ne.jp \
    --cc=linux-mips@linux-mips.org \
    --cc=mcdonald.shane@gmail.com \
    --cc=ralf@linux-mips.org \
    --cc=sshtylyov@mvista.com \
    /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.