Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
For the list archives: here's the latest version of this.
The signed-off-by discussion is offlist right now, so this
version has none; see what eventually merges.
From: Philipp Zabel <[EMAIL PROTECTED]>
Arch-neutral GPIO calls for PXA.
Index: pxa/include/asm-arm/arch-pxa/gpio.h
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ pxa/include/asm-arm/arch-pxa/gpio.h 2007-01-01 11:23:43.0 -0800
@@ -0,0 +1,97 @@
+/*
+ * linux/include/asm-arm/arch-pxa/gpio.h
+ *
+ * PXA GPIO wrappers for arch-neutral GPIO calls
+ *
+ * Written by Philipp Zabel <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_ARCH_PXA_GPIO_H
+#define __ASM_ARCH_PXA_GPIO_H
+
+#include
+#include
+#include
+
+#include
+
+static inline int gpio_get_value(unsigned gpio);
+static inline void gpio_set_value(unsigned gpio, int value);
+
+#include /* cansleep wrappers */
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ return;
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_IN);
+}
+
+static inline int gpio_direction_output(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_OUT);
+}
+
+static inline int __gpio_get_value(unsigned gpio)
+{
+ return GPLR(gpio) & GPIO_bit(gpio);
+}
+
+static inline int gpio_get_value(unsigned gpio)
+{
+ if (__builtin_constant_p(gpio))
+ return __gpio_get_value(gpio);
+ else
+ pxa_gpio_get_value(gpio);
+}
+
+static inline void __gpio_set_value(unsigned gpio, int value)
+{
+ if (value)
+ GPSR(gpio) = GPIO_bit(gpio);
+ else
+ GPCR(gpio) = GPIO_bit(gpio);
+}
+
+static inline void gpio_set_value(unsigned gpio, int value)
+{
+ if (__builtin_constant_p(gpio))
+ __gpio_set_value(gpio, value);
+ else
+ pxa_gpio_set_value(gpio, value);
+}
+
+static inline int gpio_to_irq(unsigned gpio)
+{
+ if (gpio <= PXA_LAST_GPIO)
+ return IRQ_GPIO(gpio);
+ return -EINVAL;
+}
+
+#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
+
+
+#endif
Index: pxa/arch/arm/mach-pxa/generic.c
===
--- pxa.orig/arch/arm/mach-pxa/generic.c2006-12-31 17:03:59.0
-0800
+++ pxa/arch/arm/mach-pxa/generic.c 2006-12-31 17:08:37.0 -0800
@@ -36,6 +36,7 @@
#include
#include
+#include
#include
#include
#include
@@ -104,13 +105,16 @@ unsigned long long sched_clock(void)
* Handy function to set GPIO alternate functions
*/
-void pxa_gpio_mode(int gpio_mode)
+int pxa_gpio_mode(int gpio_mode)
{
unsigned long flags;
int gpio = gpio_mode & GPIO_MD_MASK_NR;
int fn = (gpio_mode & GPIO_MD_MASK_FN) >> 8;
int gafr;
+ if (gpio > PXA_LAST_GPIO)
+ return -EINVAL;
+
local_irq_save(flags);
if (gpio_mode & GPIO_DFLT_LOW)
GPCR(gpio) = GPIO_bit(gpio);
@@ -123,11 +127,33 @@ void pxa_gpio_mode(int gpio_mode)
gafr = GAFR(gpio) & ~(0x3 << (((gpio) & 0xf)*2));
GAFR(gpio) = gafr | (fn << (((gpio) & 0xf)*2));
local_irq_restore(flags);
+
+ return 0;
}
EXPORT_SYMBOL(pxa_gpio_mode);
/*
+ * Return GPIO level
+ */
+int pxa_gpio_get_value(unsigned gpio)
+{
+ return __gpio_get_value(gpio);
+}
+
+EXPORT_SYMBOL(pxa_gpio_get_value);
+
+/*
+ * Set output GPIO level
+ */
+void pxa_gpio_set_value(unsigned gpio, int value)
+{
+ __gpio_set_value(gpio, value);
+}
+
+EXPORT_SYMBOL(pxa_gpio_set_value);
+
+/*
* Routine to safely enable or disable a clock in the CKEN
*/
void pxa_set_cken(int clock, int enable)
Index: pxa/include/asm-arm/arch-pxa/hardware.h
===
--- pxa.orig/include/asm-arm/arch-pxa/hardware.h2006-12-31
17:03:59.0 -0800
+++ pxa/include/asm-arm/arch-pxa/hardware.h 2006-12-31 17:08:37.0
-0800
@@ -65,7 +65,17 @@
/*
* Handy routine to set GPIO alternate functions
*/
-extern void pxa_gpio_mode( int gpio_mode );
+extern int pxa_gpio_mode( int
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Friday 29 December 2006 6:15 pm, Nicolas Pitre wrote: > On Thu, 28 Dec 2006, David Brownell wrote: > > > Phillip: is this the final version, then? It's missing > > a signed-off-by line, so I can't do anything appropriate. > > > > Nico, your signoff here would be a Good Thing too if it > > meets your technical review. (My only comment, ISTR, was > > that gpio_set_value macro should probably test for whether > > the value is a constant too, not just the gpio pin.) > > I don't think so. Expansion of GPIO_bit(x) is pretty simple even if x > is not constant. That probably makes it still less costly than a > function call. I was more concerned with the "value" ... that expands to a conditional, which is likely to cost a couple more instructions regardless, which in space terms competes with a function call. But I concluded much the same thing when I did that experimental conversion ... not because I was comparing the space for conditional vs funcall, but because the existing code already had the conditional. If more code savings can be had later, so be it ... no rush for now. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Thu, 28 Dec 2006, David Brownell wrote: > Phillip: is this the final version, then? It's missing > a signed-off-by line, so I can't do anything appropriate. > > Nico, your signoff here would be a Good Thing too if it > meets your technical review. (My only comment, ISTR, was > that gpio_set_value macro should probably test for whether > the value is a constant too, not just the gpio pin.) I don't think so. Expansion of GPIO_bit(x) is pretty simple even if x is not constant. That probably makes it still less costly than a function call. Nicolas - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Just FYI -- I updated your patch, fixed a compile bug, and switched
some code over to use this new API. The patch is appended.
I happen to think it's a lot easier to read this way. Maybe to some
people it's easy to remember what a GPLR, GPCR, and GPSR register is
supposed to do, after a long time away from PXA or StrongARM platform
code; I'm not one of them.
And on a side note, yes it would make sense for someone to update the
GPIO IRQ support to properly manage PWER, so there's less need for
board-specific PM glue code. :)
- Dave
This is an UNTESTED bunch of conversions of PXA code to use the new GPIO
interfaces, and other build/warning fixes. It's not complete, or even
fully reviewed; but it builds.
Note that the idioms in the API are, as with other architectures, a very
direct match for the existing code ... and so the conversions are easy
to do and to review.
arch/arm/mach-pxa/corgi.c | 13 -
arch/arm/mach-pxa/corgi_lcd.c |8 ++--
arch/arm/mach-pxa/corgi_pm.c| 25 ++---
arch/arm/mach-pxa/corgi_ssp.c | 18 ++
arch/arm/mach-pxa/sharpsl.h |6 --
arch/arm/mach-pxa/spitz_pm.c|6 +++---
drivers/usb/gadget/pxa2xx_udc.c | 20 ++--
drivers/usb/gadget/pxa2xx_udc.h | 21 ++---
drivers/video/backlight/corgi_bl.c |2 +-
drivers/video/backlight/locomolcd.c |3 ++-
10 files changed, 52 insertions(+), 70 deletions(-)
Index: pxa/arch/arm/mach-pxa/corgi.c
===
--- pxa.orig/arch/arm/mach-pxa/corgi.c 2006-12-10 01:30:42.0 -0800
+++ pxa/arch/arm/mach-pxa/corgi.c 2006-12-29 16:44:15.0 -0800
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
@@ -239,15 +240,12 @@ static void corgi_mci_setpower(struct de
{
struct pxamci_platform_data* p_d = dev->platform_data;
- if (( 1 << vdd) & p_d->ocr_mask)
- GPSR1 = GPIO_bit(CORGI_GPIO_SD_PWR);
- else
- GPCR1 = GPIO_bit(CORGI_GPIO_SD_PWR);
+ gpio_set_value(CORGI_GPIO_SD_PWR, (1 << vdd) & p_d->ocr_mask);
}
static int corgi_mci_get_ro(struct device *dev)
{
- return GPLR(CORGI_GPIO_nSD_WP) & GPIO_bit(CORGI_GPIO_nSD_WP);
+ return gpio_get_value(CORGI_GPIO_nSD_WP);
}
static void corgi_mci_exit(struct device *dev, void *data)
@@ -269,10 +267,7 @@ static struct pxamci_platform_data corgi
*/
static void corgi_irda_transceiver_mode(struct device *dev, int mode)
{
- if (mode & IR_OFF)
- GPSR(CORGI_GPIO_IR_ON) = GPIO_bit(CORGI_GPIO_IR_ON);
- else
- GPCR(CORGI_GPIO_IR_ON) = GPIO_bit(CORGI_GPIO_IR_ON);
+ gpio_set_value(CORGI_GPIO_IR_ON, mode & IR_OFF);
}
static struct pxaficp_platform_data corgi_ficp_platform_data = {
Index: pxa/drivers/usb/gadget/pxa2xx_udc.h
===
--- pxa.orig/drivers/usb/gadget/pxa2xx_udc.h2006-12-10 01:31:53.0
-0800
+++ pxa/drivers/usb/gadget/pxa2xx_udc.h 2006-12-29 16:32:41.0 -0800
@@ -139,6 +139,8 @@ struct pxa2xx_udc {
struct pxa2xx_epep [PXA_UDC_NUM_ENDPOINTS];
};
+static struct pxa2xx_udc *the_controller;
+
/*-*/
#ifdef CONFIG_ARCH_LUBBOCK
@@ -175,25 +177,6 @@ struct pxa2xx_udc {
/*-*/
-static struct pxa2xx_udc *the_controller;
-
-static inline int pxa_gpio_get(unsigned gpio)
-{
- return (GPLR(gpio) & GPIO_bit(gpio)) != 0;
-}
-
-static inline void pxa_gpio_set(unsigned gpio, int is_on)
-{
- int mask = GPIO_bit(gpio);
-
- if (is_on)
- GPSR(gpio) = mask;
- else
- GPCR(gpio) = mask;
-}
-
-/*-*/
-
/*
* Debugging support vanishes in non-debug builds. DBG_NORMAL should be
* mostly silent during normal use/testing, with no timing side-effects.
Index: pxa/arch/arm/mach-pxa/corgi_ssp.c
===
--- pxa.orig/arch/arm/mach-pxa/corgi_ssp.c 2006-12-10 01:30:42.0
-0800
+++ pxa/arch/arm/mach-pxa/corgi_ssp.c 2006-12-29 16:16:18.0 -0800
@@ -16,6 +16,8 @@
#include
#include
#include
+
+#include
#include
#include
@@ -52,13 +54,13 @@ unsigned long corgi_ssp_ads7846_putget(u
spin_lock_irqsave(&corgi_ssp_lock, flag);
if (ssp_machinfo->cs_ads7846 >= 0)
- GPCR(ssp_machinfo->cs_ads7846) =
GPIO_bit(ssp_machinfo->cs_ads7846);
+ gpio_set_value(ssp_machinfo->cs_ads7846, 0);
ssp_write_word(&corgi_ssp_dev,data);
ssp_read_word(&corgi_ssp_dev, &ret);
if (ssp_machinfo->cs_ads7846 >= 0)
- GPSR(ssp_machin
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Thu 2006-12-28 21:50:55, Pavel Machek wrote:
> Hi!
>
> > > > From: Philipp Zabel <[EMAIL PROTECTED]>
> > >
> > > Missing s-o-b?
> >
> > Yes, still ...
> >
> > > > +static inline int gpio_direction_input(unsigned gpio)
> > > > +{
> > > > + if (gpio > PXA_LAST_GPIO)
> > > > + return -EINVAL;
> > > > + pxa_gpio_mode(gpio | GPIO_IN);
> > > > +}
> > >
> > > Missing return 0?
> > >
> > > > +static inline int gpio_direction_output(unsigned gpio)
> > > > +{
> > > > + if (gpio > PXA_LAST_GPIO)
> > > > + return -EINVAL;
> > > > + pxa_gpio_mode(gpio | GPIO_OUT);
> > > > +}
> > > > +
> > >
> > > And here?
> >
> > You're looking at about the oldest version of that patch.
> > Admittedly there were too many floating around...
>
> I think I've looked at the newer ones, too, and this particular return
> was _not_ fixed.
Ok, I was wrong, the very newest one seems to be okay.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Hi!
> > > From: Philipp Zabel <[EMAIL PROTECTED]>
> >
> > Missing s-o-b?
>
> Yes, still ...
>
> > > +static inline int gpio_direction_input(unsigned gpio)
> > > +{
> > > + if (gpio > PXA_LAST_GPIO)
> > > + return -EINVAL;
> > > + pxa_gpio_mode(gpio | GPIO_IN);
> > > +}
> >
> > Missing return 0?
> >
> > > +static inline int gpio_direction_output(unsigned gpio)
> > > +{
> > > + if (gpio > PXA_LAST_GPIO)
> > > + return -EINVAL;
> > > + pxa_gpio_mode(gpio | GPIO_OUT);
> > > +}
> > > +
> >
> > And here?
>
> You're looking at about the oldest version of that patch.
> Admittedly there were too many floating around...
I think I've looked at the newer ones, too, and this particular return
was _not_ fixed.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Wednesday 27 December 2006 9:53 am, Pavel Machek wrote:
> Hi!
>
> > Arch-neutral GPIO calls for PXA.
> >
> > From: Philipp Zabel <[EMAIL PROTECTED]>
>
> Missing s-o-b?
Yes, still ...
> > +static inline int gpio_direction_input(unsigned gpio)
> > +{
> > + if (gpio > PXA_LAST_GPIO)
> > + return -EINVAL;
> > + pxa_gpio_mode(gpio | GPIO_IN);
> > +}
>
> Missing return 0?
>
> > +static inline int gpio_direction_output(unsigned gpio)
> > +{
> > + if (gpio > PXA_LAST_GPIO)
> > + return -EINVAL;
> > + pxa_gpio_mode(gpio | GPIO_OUT);
> > +}
> > +
>
> And here?
You're looking at about the oldest version of that patch.
Admittedly there were too many floating around...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Phillip: is this the final version, then? It's missing
a signed-off-by line, so I can't do anything appropriate.
Nico, your signoff here would be a Good Thing too if it
meets your technical review. (My only comment, ISTR, was
that gpio_set_value macro should probably test for whether
the value is a constant too, not just the gpio pin.)
- Dave
On Thursday 21 December 2006 10:53 pm, pHilipp Zabel wrote:
>
> Index: linux-2.6/include/asm-arm/arch-pxa/gpio.h
> ===
> --- /dev/null 1970-01-01 00:00:00.0 +
> +++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
> 20:07:48.0 +0100
> @@ -0,0 +1,82 @@
> +/*
> + * linux/include/asm-arm/arch-pxa/gpio.h
> + *
> + * PXA GPIO wrappers for arch-neutral GPIO calls
> + *
> + * Written by Philipp Zabel <[EMAIL PROTECTED]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#ifndef __ASM_ARCH_PXA_GPIO_H
> +#define __ASM_ARCH_PXA_GPIO_H
> +
> +#include
> +#include
> +#include
> +
> +#include
> +
> +static inline int gpio_request(unsigned gpio, const char *label)
> +{
> + return 0;
> +}
> +
> +static inline void gpio_free(unsigned gpio)
> +{
> + return;
> +}
> +
> +static inline int gpio_direction_input(unsigned gpio)
> +{
> + return pxa_gpio_mode(gpio | GPIO_IN);
> +}
> +
> +static inline int gpio_direction_output(unsigned gpio)
> +{
> + return pxa_gpio_mode(gpio | GPIO_OUT);
> +}
> +
> +static inline int __gpio_get_value(unsigned gpio)
> +{
> + return GPLR(gpio) & GPIO_bit(gpio);
> +}
> +
> +#define gpio_get_value(gpio) \
> + (__builtin_constant_p(gpio) ? \
> + __gpioe_get_value(gpio) : \
> + pxa_gpio_get_value(gpio))
> +
> +static inline void __gpio_set_value(unsigned gpio, int value)
> +{
> + if (value)
> + GPSR(gpio) = GPIO_bit(gpio);
> + else
> + GPCR(gpio) = GPIO_bit(gpio);
> +}
> +
> +#define gpio_set_value(gpio,value) \
> + (__builtin_constant_p(gpio) ? \
> + __gpio_set_value(gpio, value) :\
> + pxa_gpio_set_value(gpio, value))
> +
> +#include /* cansleep wrappers */
> +
> +#define gpio_to_irq(gpio)IRQ_GPIO(gpio)
> +#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
> +
> +
> +#endif
> Index: linux-2.6/arch/arm/mach-pxa/generic.c
> ===
> --- linux-2.6.orig/arch/arm/mach-pxa/generic.c2006-12-21
> 13:30:01.0 +0100
> +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-21 21:25:24.0
> +0100
> @@ -36,6 +36,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -105,13 +106,16 @@
> * Handy function to set GPIO alternate functions
> */
>
> -void pxa_gpio_mode(int gpio_mode)
> +int pxa_gpio_mode(int gpio_mode)
> {
> unsigned long flags;
> int gpio = gpio_mode & GPIO_MD_MASK_NR;
> int fn = (gpio_mode & GPIO_MD_MASK_FN) >> 8;
> int gafr;
>
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> +
> local_irq_save(flags);
> if (gpio_mode & GPIO_DFLT_LOW)
> GPCR(gpio) = GPIO_bit(gpio);
> @@ -124,11 +128,33 @@
> gafr = GAFR(gpio) & ~(0x3 << (((gpio) & 0xf)*2));
> GAFR(gpio) = gafr | (fn << (((gpio) & 0xf)*2));
> local_irq_restore(flags);
> +
> + return 0;
> }
>
> EXPORT_SYMBOL(pxa_gpio_mode);
>
> /*
> + * Return GPIO level
> + */
> +int pxa_gpio_get_value(unsigned gpio)
> +{
> + return __gpio_get_value(gpio);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_get_value);
> +
> +/*
> + * Set output GPIO level
> + */
> +void pxa_gpio_set_value(unsigned gpio, int value)
> +{
> + __gpio_set_value(gpio, value);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_set_value);
> +
> +/*
> * Routine to safely enable or disable a clock in the CKEN
> */
> void pxa_set_cken(int clock, int enable)
> Index: linux-2.6/include/asm-arm/arch-pxa/hardware.h
> ===
> --- linux-2.6.orig/include/asm-arm/arch-pxa/hardware.h2006-12-21
> 13:30:01.0 +0100
> +++ linux-2.6/include/asm-arm/arch-pxa/hardware.h 2006-12-21
> 20:03:55.0 +0100
> @@ -65,
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Hi!
> Arch-neutral GPIO calls for PXA.
>
> From: Philipp Zabel <[EMAIL PROTECTED]>
Missing s-o-b?
> +static inline int gpio_direction_input(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_IN);
> +}
Missing return 0?
> +static inline int gpio_direction_output(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_OUT);
> +}
> +
And here?
Pavel
--
Thanks for all the (sleeping) penguins.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On 12/21/06, Nicolas Pitre <[EMAIL PROTECTED]> wrote:
On Thu, 21 Dec 2006, pHilipp Zabel wrote:
> > > --- linux-2.6.orig/arch/arm/mach-pxa/generic.c2006-12-16
> > > +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16
> > > 16:47:45.0
> > > @@ -129,6 +129,29 @@
> > > EXPORT_SYMBOL(pxa_gpio_mode);
> > >
> > > /*
> > > + * Return GPIO level, nonzero means high, zero is low
> > > + */
> > > +int pxa_gpio_get_value(unsigned gpio)
> > > +{
> > > + return GPLR(gpio) & GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_get_value);
> > > +
> > > +/*
> > > + * Set output GPIO level
> > > + */
> > > +void pxa_gpio_set_value(unsigned gpio, int value)
> > > +{
> > > + if (value)
> > > + GPSR(gpio) = GPIO_bit(gpio);
> > > + else
> > > + GPCR(gpio) = GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_set_value);
> >
> > Instead of duplicating code here, you probably should just reuse
> > __gpio_set_value() and __gpio_get_value() inside those functions.
>
> Probably? What I am wondering is this: can the compiler
> optimize away the range check that is duplicated in GPSR/GPCR
> and GPIO_bit for __gpio_set/get_value? Or could we optimize
> this case by expanding the macros in place (which would mean
> duplicating code from pxa-regs.h)...
Sorry I don't quite follow you here. Why would you expand the macro in
place?
And that is no surprise because I seem to have problems to follow
myself here now after a good night's rest. Basically I was thinking
that after expanding the macros in place the code could be optimized.
Of course, this doesn't have any advantage for the inlined functions
(gpio is constant, so most of the code will be optimized away anyway),
and shaving one or two words off pxa_gpio_setval is hardly worthwile.
My suggestion is only about not duplicating the source code. The
generated assembly will be the same.
And your patch looks fine to me now, except for this:
+int pxa_gpio_get_value(unsigned gpio)
+{
+ __gpio_get_value(gpio);
+}
You certainly meant to add a "return" in there, right?
Oh yes. Sorry.
cheers
Philipp
Index: linux-2.6/include/asm-arm/arch-pxa/gpio.h
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
20:07:48.0 +0100
@@ -0,0 +1,82 @@
+/*
+ * linux/include/asm-arm/arch-pxa/gpio.h
+ *
+ * PXA GPIO wrappers for arch-neutral GPIO calls
+ *
+ * Written by Philipp Zabel <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_ARCH_PXA_GPIO_H
+#define __ASM_ARCH_PXA_GPIO_H
+
+#include
+#include
+#include
+
+#include
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ return;
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_IN);
+}
+
+static inline int gpio_direction_output(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_OUT);
+}
+
+static inline int __gpio_get_value(unsigned gpio)
+{
+ return GPLR(gpio) & GPIO_bit(gpio);
+}
+
+#define gpio_get_value(gpio) \
+ (__builtin_constant_p(gpio) ? \
+__gpioe_get_value(gpio) : \
+pxa_gpio_get_value(gpio))
+
+static inline void __gpio_set_value(unsigned gpio, int value)
+{
+ if (value)
+ GPSR(gpio) = GPIO_bit(gpio);
+ else
+ GPCR(gpio) = GPIO_bit(gpio);
+}
+
+#define gpio_set_value(gpio,value) \
+ (__builtin_constant_p(gpio) ? \
+__gpio_set_value(gpio, value) :\
+pxa_gpio_set_value(gpio, value))
+
+#include /* cansleep wrappers */
+
+#define gpio_to_irq(gpio) IRQ_GPIO(gpio)
+#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
+
+
+#endif
Index: linux-2.6/arch/arm/mach-pxa/generic.c
===
--- linux-2.6.orig/arch/arm/mach-pxa/generic.c 2006-12-21
13:30:01.0 +0100
+++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-21 21:25:24.0
+0100
@@ -36,6 +36,7 @@
#include
#include
+#include
#include
#include
#in
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Guys: Probably? What I am wondering is this: can the compiler optimize away the range check that is duplicated in GPSR/GPCR and GPIO_bit for __gpio_set/get_value? Or could we optimize this case by expanding the macros in place (which would mean duplicating code from pxa-regs.h)... Who cares? :) I don't think there's much point in optimizing here, since these functions won't be hot paths anyway. Yes, they'll be called in interrupt handlers and so we don't want them to be _too_ heavy, but compared to the overhead of an interrupt handler, a few extra instructions in the GPIO access will get lost in the noise. Inlines generally seem to be more maintainable, give you a symbol that you can disassemble and breakpoint, etc. I'll take them over the macro implementations any day, in this case even if there's a cost of a few instructions. All IMHO, of course. b.g. -- Bill Gatliff [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Thu, 21 Dec 2006, pHilipp Zabel wrote:
> > > --- linux-2.6.orig/arch/arm/mach-pxa/generic.c2006-12-16
> > > +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16
> > > 16:47:45.0
> > > @@ -129,6 +129,29 @@
> > > EXPORT_SYMBOL(pxa_gpio_mode);
> > >
> > > /*
> > > + * Return GPIO level, nonzero means high, zero is low
> > > + */
> > > +int pxa_gpio_get_value(unsigned gpio)
> > > +{
> > > + return GPLR(gpio) & GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_get_value);
> > > +
> > > +/*
> > > + * Set output GPIO level
> > > + */
> > > +void pxa_gpio_set_value(unsigned gpio, int value)
> > > +{
> > > + if (value)
> > > + GPSR(gpio) = GPIO_bit(gpio);
> > > + else
> > > + GPCR(gpio) = GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_set_value);
> >
> > Instead of duplicating code here, you probably should just reuse
> > __gpio_set_value() and __gpio_get_value() inside those functions.
>
> Probably? What I am wondering is this: can the compiler
> optimize away the range check that is duplicated in GPSR/GPCR
> and GPIO_bit for __gpio_set/get_value? Or could we optimize
> this case by expanding the macros in place (which would mean
> duplicating code from pxa-regs.h)...
Sorry I don't quite follow you here. Why would you expand the macro in
place?
My suggestion is only about not duplicating the source code. The
generated assembly will be the same.
And your patch looks fine to me now, except for this:
+int pxa_gpio_get_value(unsigned gpio)
+{
+ __gpio_get_value(gpio);
+}
You certainly meant to add a "return" in there, right?
Nicolas
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On 12/21/06, Nicolas Pitre <[EMAIL PROTECTED]> wrote:
On Thu, 21 Dec 2006, pHilipp Zabel wrote:
> David suggested to have both inline and non-inline functions depending
> on whether gpio is constant. How is this patch?
More comments below.
> --- /dev/null 1970-01-01 00:00:00.0 +
> +++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
> 07:57:12.0 +0100
> @@ -0,0 +1,86 @@
[...]
> +static inline int gpio_direction_input(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_IN);
> +}
> +
> +static inline int gpio_direction_output(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_OUT);
> +}
Please push this test against PXA_LAST_GPIO inside pxa_gpio_mode(). It
has no advantage to be inline if you're about to call a function anyway.
That would make pxa_gpio_mode() more reliable for those not calling it
through the generic API wrt that kind of error as well.
I see. Originally I didn't want to touch generic.c at all.
Now pxa_gpio_mode returns int instead of void.
> --- linux-2.6.orig/arch/arm/mach-pxa/generic.c2006-12-16
> +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16 16:47:45.0
> @@ -129,6 +129,29 @@
> EXPORT_SYMBOL(pxa_gpio_mode);
>
> /*
> + * Return GPIO level, nonzero means high, zero is low
> + */
> +int pxa_gpio_get_value(unsigned gpio)
> +{
> + return GPLR(gpio) & GPIO_bit(gpio);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_get_value);
> +
> +/*
> + * Set output GPIO level
> + */
> +void pxa_gpio_set_value(unsigned gpio, int value)
> +{
> + if (value)
> + GPSR(gpio) = GPIO_bit(gpio);
> + else
> + GPCR(gpio) = GPIO_bit(gpio);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_set_value);
Instead of duplicating code here, you probably should just reuse
__gpio_set_value() and __gpio_get_value() inside those functions.
Probably? What I am wondering is this: can the compiler
optimize away the range check that is duplicated in GPSR/GPCR
and GPIO_bit for __gpio_set/get_value? Or could we optimize
this case by expanding the macros in place (which would mean
duplicating code from pxa-regs.h)...
regards
Philipp
Index: linux-2.6/include/asm-arm/arch-pxa/gpio.h
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
20:07:48.0 +0100
@@ -0,0 +1,82 @@
+/*
+ * linux/include/asm-arm/arch-pxa/gpio.h
+ *
+ * PXA GPIO wrappers for arch-neutral GPIO calls
+ *
+ * Written by Philipp Zabel <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_ARCH_PXA_GPIO_H
+#define __ASM_ARCH_PXA_GPIO_H
+
+#include
+#include
+#include
+
+#include
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ return;
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_IN);
+}
+
+static inline int gpio_direction_output(unsigned gpio)
+{
+ return pxa_gpio_mode(gpio | GPIO_OUT);
+}
+
+static inline int __gpio_get_value(unsigned gpio)
+{
+ return GPLR(gpio) & GPIO_bit(gpio);
+}
+
+#define gpio_get_value(gpio) \
+ (__builtin_constant_p(gpio) ? \
+__gpioe_get_value(gpio) : \
+pxa_gpio_get_value(gpio))
+
+static inline void __gpio_set_value(unsigned gpio, int value)
+{
+ if (value)
+ GPSR(gpio) = GPIO_bit(gpio);
+ else
+ GPCR(gpio) = GPIO_bit(gpio);
+}
+
+#define gpio_set_value(gpio,value) \
+ (__builtin_constant_p(gpio) ? \
+__gpio_set_value(gpio, value) :\
+pxa_gpio_set_value(gpio, value))
+
+#include /* cansleep wrappers */
+
+#define gpio_to_irq(gpio) IRQ_GPIO(gpio)
+#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
+
+
+#endif
Index: linux-2.6/arch/arm/mach-pxa/generic.c
===
--- linux-2.6.orig/arch/arm/mach-pxa/generic.c 2006-12-21
13:30:01.0 +0100
+++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-2
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Thursday 21 December 2006 7:03 am, pHilipp Zabel wrote:
> On 12/21/06, Nicolas Pitre <[EMAIL PROTECTED]> wrote:
> +static inline void __gpio_set_value(unsigned gpio, int value)
> +{
> + if (value)
> + GPSR(gpio) = GPIO_bit(gpio);
> + else
> + GPCR(gpio) = GPIO_bit(gpio);
> +}
> +
> +#define gpio_set_value(gpio,value) \
> + (__builtin_constant_p(gpio) ? \
Should that be testing for _both_ gpio and value being constant?
I tend to think it should (assuming nonconstant 'variable' means
this costs more than a function call) ...
> + __gpio_set_value(gpio, value) :\
> + pxa_gpio_set_value(gpio, value))
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Thu, 21 Dec 2006, pHilipp Zabel wrote:
> David suggested to have both inline and non-inline functions depending
> on whether gpio is constant. How is this patch?
More comments below.
> --- /dev/null 1970-01-01 00:00:00.0 +
> +++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
> 07:57:12.0 +0100
> @@ -0,0 +1,86 @@
[...]
> +static inline int gpio_direction_input(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_IN);
> +}
> +
> +static inline int gpio_direction_output(unsigned gpio)
> +{
> + if (gpio > PXA_LAST_GPIO)
> + return -EINVAL;
> + pxa_gpio_mode(gpio | GPIO_OUT);
> +}
Please push this test against PXA_LAST_GPIO inside pxa_gpio_mode(). It
has no advantage to be inline if you're about to call a function anyway.
That would make pxa_gpio_mode() more reliable for those not calling it
through the generic API wrt that kind of error as well.
> --- linux-2.6.orig/arch/arm/mach-pxa/generic.c2006-12-16
> +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16 16:47:45.0
> @@ -129,6 +129,29 @@
> EXPORT_SYMBOL(pxa_gpio_mode);
>
> /*
> + * Return GPIO level, nonzero means high, zero is low
> + */
> +int pxa_gpio_get_value(unsigned gpio)
> +{
> + return GPLR(gpio) & GPIO_bit(gpio);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_get_value);
> +
> +/*
> + * Set output GPIO level
> + */
> +void pxa_gpio_set_value(unsigned gpio, int value)
> +{
> + if (value)
> + GPSR(gpio) = GPIO_bit(gpio);
> + else
> + GPCR(gpio) = GPIO_bit(gpio);
> +}
> +
> +EXPORT_SYMBOL(pxa_gpio_set_value);
Instead of duplicating code here, you probably should just reuse
__gpio_set_value() and __gpio_get_value() inside those functions.
Nicolas
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On 12/21/06, Nicolas Pitre <[EMAIL PROTECTED]> wrote:
On Wed, 20 Dec 2006, David Brownell wrote:
> On Wednesday 20 December 2006 10:12 pm, Andrew Morton wrote:
> > Why not implement them as inline functions?
>
> I just collected and forwarded the code from Philip...
> the better not to lose such stuff! :)
>
>
> > Or non-inline functions, come to that.
>
> In this case I think that'd be preferable; see what those macros
> expand to on pxa27x CPUs.
Only if the gpio argument is not constant please. When it is constant
this expands to a single word store.
Nicolas
David suggested to have both inline and non-inline functions depending
on whether gpio is constant. How is this patch?
regards
Philipp
Index: linux-2.6/include/asm-arm/arch-pxa/gpio.h
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21
07:57:12.0 +0100
@@ -0,0 +1,86 @@
+/*
+ * linux/include/asm-arm/arch-pxa/gpio.h
+ *
+ * PXA GPIO wrappers for arch-neutral GPIO calls
+ *
+ * Written by Philipp Zabel <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_ARCH_PXA_GPIO_H
+#define __ASM_ARCH_PXA_GPIO_H
+
+#include
+#include
+#include
+
+#include
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ return;
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ if (gpio > PXA_LAST_GPIO)
+ return -EINVAL;
+ pxa_gpio_mode(gpio | GPIO_IN);
+}
+
+static inline int gpio_direction_output(unsigned gpio)
+{
+ if (gpio > PXA_LAST_GPIO)
+ return -EINVAL;
+ pxa_gpio_mode(gpio | GPIO_OUT);
+}
+
+static inline int __gpio_get_value(unsigned gpio)
+{
+ return GPLR(gpio) & GPIO_bit(gpio);
+}
+
+#define gpio_get_value(gpio) \
+ (__builtin_constant_p(gpio) ? \
+__gpioe_get_value(gpio) : \
+pxa_gpio_get_value(gpio))
+
+static inline void __gpio_set_value(unsigned gpio, int value)
+{
+ if (value)
+ GPSR(gpio) = GPIO_bit(gpio);
+ else
+ GPCR(gpio) = GPIO_bit(gpio);
+}
+
+#define gpio_set_value(gpio,value) \
+ (__builtin_constant_p(gpio) ? \
+__gpio_set_value(gpio, value) :\
+pxa_gpio_set_value(gpio, value))
+
+#include /* cansleep wrappers */
+
+#define gpio_to_irq(gpio) IRQ_GPIO(gpio)
+#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
+
+
+#endif
Index: linux-2.6/arch/arm/mach-pxa/generic.c
===
--- linux-2.6.orig/arch/arm/mach-pxa/generic.c 2006-12-16
16:47:42.0 +0100
+++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16 16:47:45.0
+0100
@@ -129,6 +129,29 @@
EXPORT_SYMBOL(pxa_gpio_mode);
/*
+ * Return GPIO level, nonzero means high, zero is low
+ */
+int pxa_gpio_get_value(unsigned gpio)
+{
+ return GPLR(gpio) & GPIO_bit(gpio);
+}
+
+EXPORT_SYMBOL(pxa_gpio_get_value);
+
+/*
+ * Set output GPIO level
+ */
+void pxa_gpio_set_value(unsigned gpio, int value)
+{
+ if (value)
+ GPSR(gpio) = GPIO_bit(gpio);
+ else
+ GPCR(gpio) = GPIO_bit(gpio);
+}
+
+EXPORT_SYMBOL(pxa_gpio_set_value);
+
+/*
* Routine to safely enable or disable a clock in the CKEN
*/
void pxa_set_cken(int clock, int enable)
Index: linux-2.6/include/asm-arm/arch-pxa/hardware.h
===
--- linux-2.6.orig/include/asm-arm/arch-pxa/hardware.h 2006-12-17
15:42:36.0 +0100
+++ linux-2.6/include/asm-arm/arch-pxa/hardware.h 2006-12-17
15:43:26.0 +0100
@@ -68,6 +68,16 @@
extern void pxa_gpio_mode( int gpio_mode );
/*
+ * Return GPIO level, nonzero means high, zero is low
+ */
+extern int pxa_gpio_get_value(unsigned gpio);
+
+/*
+ * Set output GPIO level
+ */
+extern void pxa_gpio_set_value(unsigned gpio, int value);
+
+/*
* Routine to enable or disable CKEN
*/
extern void pxa_set_cken(int clock, int enable);
Index: linux-2.6/include/asm-arm/arch-pxa/gpio.h
=
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Wed, 20 Dec 2006, David Brownell wrote: > On Wednesday 20 December 2006 10:12 pm, Andrew Morton wrote: > > Why not implement them as inline functions? > > I just collected and forwarded the code from Philip... > the better not to lose such stuff! :) > > > > Or non-inline functions, come to that. > > In this case I think that'd be preferable; see what those macros > expand to on pxa27x CPUs. Only if the gpio argument is not constant please. When it is constant this expands to a single word store. Nicolas - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Wednesday 20 December 2006 10:12 pm, Andrew Morton wrote: > On Wed, 20 Dec 2006 13:12:35 -0800 > David Brownell <[EMAIL PROTECTED]> wrote: > > > +/* REVISIT these macros are correct, but suffer code explosion > > + * for non-constant parameters. Provide out-line versions too. > > + */ > > +#define gpio_get_value(gpio) \ > > + (GPLR(gpio) & GPIO_bit(gpio)) > > + > > +#define gpio_set_value(gpio,value) \ > > + ((value) ? (GPSR(gpio) = GPIO_bit(gpio)):(GPCR(gpio) = GPIO_bit(gpio))) > > Why not implement them as inline functions? I just collected and forwarded the code from Philip... the better not to lose such stuff! :) > Or non-inline functions, come to that. In this case I think that'd be preferable; see what those macros expand to on pxa27x CPUs. > Either way, programming in C is preferable to this ;) Hey, at least we've started using these new fangled "function prototypes"! I remember when we had to walk five miles through LINT every morning. Uphill. Then it was another five miles uphill on the way back, through a maze of twisty octal contants like "8" and "9". We didn't even have void pointers. UIDs were eight bits. This ANSI stuff you new kids are using ... you've got it easy!! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers
On Wed, 20 Dec 2006 13:12:35 -0800 David Brownell <[EMAIL PROTECTED]> wrote: > +/* REVISIT these macros are correct, but suffer code explosion > + * for non-constant parameters. Provide out-line versions too. > + */ > +#define gpio_get_value(gpio) \ > + (GPLR(gpio) & GPIO_bit(gpio)) > + > +#define gpio_set_value(gpio,value) \ > + ((value) ? (GPSR(gpio) = GPIO_bit(gpio)):(GPCR(gpio) = GPIO_bit(gpio))) Why not implement them as inline functions? Or non-inline functions, come to that. Either way, programming in C is preferable to this ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 2.6.20-rc1 4/6] PXA GPIO wrappers
Arch-neutral GPIO calls for PXA.
From: Philipp Zabel <[EMAIL PROTECTED]>
Index: at91/include/asm-arm/arch-pxa/gpio.h
===
--- /dev/null 1970-01-01 00:00:00.0 +
+++ at91/include/asm-arm/arch-pxa/gpio.h2006-12-19 02:06:39.0
-0800
@@ -0,0 +1,72 @@
+/*
+ * linux/include/asm-arm/arch-pxa/gpio.h
+ *
+ * PXA GPIO wrappers for arch-neutral GPIO calls
+ *
+ * Written by Philipp Zabel <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_ARCH_PXA_GPIO_H
+#define __ASM_ARCH_PXA_GPIO_H
+
+#include
+#include
+#include
+
+#include
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ return;
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ if (gpio > PXA_LAST_GPIO)
+ return -EINVAL;
+ pxa_gpio_mode(gpio | GPIO_IN);
+}
+
+static inline int gpio_direction_output(unsigned gpio)
+{
+ if (gpio > PXA_LAST_GPIO)
+ return -EINVAL;
+ pxa_gpio_mode(gpio | GPIO_OUT);
+}
+
+/* REVISIT these macros are correct, but suffer code explosion
+ * for non-constant parameters. Provide out-line versions too.
+ */
+#define gpio_get_value(gpio) \
+ (GPLR(gpio) & GPIO_bit(gpio))
+
+#define gpio_set_value(gpio,value) \
+ ((value) ? (GPSR(gpio) = GPIO_bit(gpio)):(GPCR(gpio) = GPIO_bit(gpio)))
+
+#include /* cansleep wrappers */
+
+#define gpio_to_irq(gpio) IRQ_GPIO(gpio)
+#define irq_to_gpio(irq) IRQ_TO_GPIO(irq)
+
+
+#endif
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
