linux-mips
[Top] [All Lists]

Re: [PATCH] provide functions for gpio configuration

To: Phil Sutter <n0-1@freewrt.org>
Subject: Re: [PATCH] provide functions for gpio configuration
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date: Mon, 03 Nov 2008 00:56:16 +0300
Cc: Ralf Baechle <ralf@linux-mips.org>, linux-mips@linux-mips.org
In-reply-to: <1225465088-29365-1-git-send-email-n0-1@freewrt.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1225412757-17894-1-git-send-email-n0-1@freewrt.org> <1225465088-29365-1-git-send-email-n0-1@freewrt.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Thunderbird 2.0.0.17 (Windows/20080914)
Hello.

Phil Sutter wrote:

As gpiolib doesn't support pin multiplexing, it provides no way to
access the GPIOFUNC register. Also there is no support for setting
interrupt status and level. These functions provide access to them and
are needed by the CompactFlash driver.

This is a combined version of the first two patches send in this thread
on the linux-mips mailinglist. It's also addressing the conflict
generated due to the change in arch/mips/include/asm/mach-rc32434/rb.h.
Though this patch depends on it, it has been moved to another
(yet uncommitted) patch changing the same part of rb.h.

Signed-off-by: Phil Sutter <n0-1@freewrt.org>

  NAK, some of the code is wrong.

diff --git a/arch/mips/rb532/gpio.c b/arch/mips/rb532/gpio.c
index 76a7fd9..de29aba 100644
--- a/arch/mips/rb532/gpio.c
+++ b/arch/mips/rb532/gpio.c
[...]
@@ -111,15 +107,47 @@ unsigned char get_latch_u5(void)
 }
 EXPORT_SYMBOL(get_latch_u5);
+/* rb532_set_bit - sanely set a bit + * + * bitval: new value for the bit
+ * offset: bit index in the 4 byte address range
+ * ioaddr: 4 byte aligned address being altered
+ */
+static inline void rb532_set_bit(unsigned bitval,
+               unsigned offset, void __iomem *ioaddr)
+{
+       unsigned long flags;
+       u32 val;
+
+       bitval = !!bitval;              /* map parameter to {0,1} */
+
+       local_irq_save(flags);
+
+       val = readl(ioaddr);
+       val &= ~( ~bitval << offset );   /* unset bit if bitval == 0 */

If bitval == 0, ~bitval evaluates to all ones, and the final AND mask ~(0xffffffff << offset) clears 32-offset MSBs. What you want is simply ~(1 << offset).

+       val |=  (  bitval << offset );   /* set bit if bitval == 1 */

And don't put spaces before ) and after (, checkpatch.pl wouldn't approve that. ;-)

@@ -176,117 +184,60 @@ static int rb532_gpio_direction_input(struct gpio_chip 
*chip, unsigned offset)
 static int rb532_gpio_direction_output(struct gpio_chip *chip,
                                        unsigned offset, int value)
 {
-       unsigned long           flags;
-       u32                     mask = 1 << offset;
-       u32                     tmp;
        struct rb532_gpio_chip  *gpch;
-       void __iomem            *gpdr;
gpch = container_of(chip, struct rb532_gpio_chip, chip);
-       writel(mask, gpch->regbase + GPIOD);

  Hm, the old code seems really borked here...

MBR, Sergei



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