linux-mips
[Top] [All Lists]

Re: [PATCH] MIPS: rb532: fix bit swapping in rb532_set_bit()

To: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Subject: Re: [PATCH] MIPS: rb532: fix bit swapping in rb532_set_bit()
From: Phil Sutter <n0-1@freewrt.org>
Date: Mon, 3 Nov 2008 16:05:42 +0100
Cc: ralf@linux-mips.org, sshtylyov@ru.mvista.com, linux-mips@linux-mips.org
In-reply-to: <20081103.234856.61509468.anemo@mba.ocn.ne.jp>
Mail-followup-to: Atsushi Nemoto <anemo@mba.ocn.ne.jp>, ralf@linux-mips.org, sshtylyov@ru.mvista.com, linux-mips@linux-mips.org
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20081103142942.GA13461@nuty> <1225722625-19750-1-git-send-email-n0-1@freewrt.org> <20081103.234856.61509468.anemo@mba.ocn.ne.jp>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.11
Hi,

On Mon, Nov 03, 2008 at 11:48:56PM +0900, Atsushi Nemoto wrote:
> Well, the linux gpio framework uses 0 for low, _nonzero_ for high.
> You should not assume the bitval is 0 or 1.

Good point. I was thinking about this potential problem, too. This is
why the mentioned function contains the following line (sadly too far
off to occur in the patch):

| bitval = !!bitval;              /* map parameter to {0,1} */

I put that separately to not make the below lines even more unreadable.

>       val &= ~(!bitval << offset);   /* unset bit if bitval == 0 */
>       val |= (!!bitval << offset);   /* set bit if bitval != 0 */

But as a boolean inverse has to be used anyway, your solution looks more
elegant than mine.

Greetings, Phil

<Prev in Thread] Current Thread [Next in Thread>