Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers

David Brownell
2006-12-30T01:14:08Z

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.000000000 -0800
+++ pxa/arch/arm/mach-pxa/corgi.c 2006-12-29 16:44:15.000000000 -0800
@@ -28,6 +28,7 @@
#include <asm/irq.h>
#include <asm/io.h>
#include <asm/system.h>
+#include <asm/gpio.h>

#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -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.h 2006-12-10 01:31:53.000000000 -0800
+++ pxa/drivers/usb/gadget/pxa2xx_udc.h 2006-12-29 16:32:41.000000000 -0800
@@ -139,6 +139,8 @@ struct pxa2xx_udc {
struct pxa2xx_ep ep [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.000000000 -0800
+++ pxa/arch/arm/mach-pxa/corgi_ssp.c 2006-12-29 16:16:18.000000000 -0800
@@ -16,6 +16,8 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
+
+#include <asm/gpio.h>
#include <asm/hardware.h>
#include <asm/mach-types.h>

@@ -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_machinfo->cs_ads7846) = GPIO_bit(ssp_machinfo->cs_ads7846);
+ gpio_set_value(ssp_machinfo->cs_ads7846, 1);
spin_unlock_irqrestore(&corgi_ssp_lock, flag);

return ret;
@@ -78,7 +80,7 @@ void corgi_ssp_ads7846_lock(void)
void corgi_ssp_ads7846_unlock(void)
{
if (ssp_machinfo->cs_ads7846 >= 0)
- GPSR(ssp_machinfo->cs_ads7846) = GPIO_bit(ssp_machinfo->cs_ads7846);
+ gpio_set_value(ssp_machinfo->cs_ads7846, 1);
spin_unlock(&corgi_ssp_lock);
}

@@ -124,7 +126,7 @@ unsigned long corgi_ssp_dac_put(ulong da
/* Read null data back from device to prevent SSP overflow */
ssp_read_word(&corgi_ssp_dev, &tmp);
if (ssp_machinfo->cs_lcdcon >= 0)
- GPSR(ssp_machinfo->cs_lcdcon) = GPIO_bit(ssp_machinfo->cs_lcdcon);
+ gpio_set_value(ssp_machinfo->cs_lcdcon, 1);

ssp_disable(&corgi_ssp_dev);
ssp_config(&corgi_ssp_dev, (SSCR0_National | (SSCR0_DSS & 0x0b )), 0, 0, SSCR0_SerClkDiv(ssp_machinfo->clk_ads7846));
@@ -181,7 +183,7 @@ int corgi_ssp_max1111_get(ulong data)
ssp_config(&corgi_ssp_dev, (SSCR0_National | (SSCR0_DSS & 0x0b )), 0, 0, SSCR0_SerClkDiv(ssp_machinfo->clk_ads7846));
ssp_enable(&corgi_ssp_dev);
if (ssp_machinfo->cs_max1111 >= 0)
- GPSR(ssp_machinfo->cs_max1111) = GPIO_bit(ssp_machinfo->cs_max1111);
+ gpio_set_value(ssp_machinfo->cs_max1111, 1);
spin_unlock_irqrestore(&corgi_ssp_lock, flag);

if (voltage1 & 0xc0 || voltage2 & 0x3f)
@@ -245,11 +247,11 @@ static int corgi_ssp_suspend(struct plat
static int corgi_ssp_resume(struct platform_device *dev)
{
if (ssp_machinfo->cs_lcdcon >= 0)
- GPSR(ssp_machinfo->cs_lcdcon) = GPIO_bit(ssp_machinfo->cs_lcdcon); /* High - Disable LCD Control/Timing Gen */
+ gpio_set_value(ssp_machinfo->cs_lcdcon, 1); /* High - Disable LCD Control/Timing Gen */
if (ssp_machinfo->cs_max1111 >= 0)
- GPSR(ssp_machinfo->cs_max1111) = GPIO_bit(ssp_machinfo->cs_max1111); /* High - Disable MAX1111*/
+ gpio_set_value(ssp_machinfo->cs_max1111, 1); /* High - Disable MAX1111*/
if (ssp_machinfo->cs_ads7846 >= 0)
- GPSR(ssp_machinfo->cs_ads7846) = GPIO_bit(ssp_machinfo->cs_ads7846); /* High - Disable ADS7846*/
+ gpio_set_value(ssp_machinfo->cs_ads7846, 1); /* High - Disable ADS7846*/
ssp_restore_state(&corgi_ssp_dev,&corgi_ssp_state);
ssp_enable(&corgi_ssp_dev);

Index: pxa/drivers/usb/gadget/pxa2xx_udc.c
===================================================================
--- pxa.orig/drivers/usb/gadget/pxa2xx_udc.c 2006-12-27 13:38:43.000000000 -0800
+++ pxa/drivers/usb/gadget/pxa2xx_udc.c 2006-12-29 16:45:58.000000000 -0800
@@ -47,6 +47,7 @@

#include <asm/byteorder.h>
#include <asm/dma.h>
+#include <asm/gpio.h>
#include <asm/io.h>
#include <asm/system.h>
#include <asm/mach-types.h>
@@ -156,7 +157,7 @@ static int is_vbus_present(void)
struct pxa2xx_udc_mach_info *mach = the_controller->mach;

if (mach->gpio_vbus)
- return pxa_gpio_get(mach->gpio_vbus);
+ return gpio_get_value(mach->gpio_vbus);
if (mach->udc_is_connected)
return mach->udc_is_connected();
return 1;
@@ -168,7 +169,7 @@ static void pullup_off(void)
struct pxa2xx_udc_mach_info *mach = the_controller->mach;

if (mach->gpio_pullup)
- pxa_gpio_set(mach->gpio_pullup, 0);
+ gpio_set_value(mach->gpio_pullup, 0);
else if (mach->udc_command)
mach->udc_command(PXA2XX_UDC_CMD_DISCONNECT);
}
@@ -178,7 +179,7 @@ static void pullup_on(void)
struct pxa2xx_udc_mach_info *mach = the_controller->mach;

if (mach->gpio_pullup)
- pxa_gpio_set(mach->gpio_pullup, 1);
+ gpio_set_value(mach->gpio_pullup, 1);
else if (mach->udc_command)
mach->udc_command(PXA2XX_UDC_CMD_CONNECT);
}
@@ -1636,7 +1637,14 @@ int usb_gadget_register_driver(struct us
dev->gadget.dev.driver = &driver->driver;
dev->pullup = 1;

- device_add (&dev->gadget.dev);
+ retval = device_add (&dev->gadget.dev);
+ if (retval < 0) {
+ DMSG("add gadget dev --> error %d\n", retval);
+
+ dev->driver = NULL;
+ dev->gadget.dev.driver = NULL;
+ return retval;
+ }
retval = driver->bind(&dev->gadget);
if (retval) {
DMSG("bind to driver %s --> error %d\n",
@@ -1647,7 +1655,7 @@ int usb_gadget_register_driver(struct us
dev->gadget.dev.driver = NULL;
return retval;
}
- device_create_file(dev->dev, &dev_attr_function);
+ retval = device_create_file(dev->dev, &dev_attr_function);

/* ... then enable host detection and ep0; and we're ready
* for set_configuration as well as eventual disconnect.
@@ -1756,7 +1764,7 @@ lubbock_vbus_irq(int irq, void *_dev)
static irqreturn_t udc_vbus_irq(int irq, void *_dev)
{
struct pxa2xx_udc *dev = _dev;
- int vbus = pxa_gpio_get(dev->mach->gpio_vbus);
+ int vbus = gpio_get_value(dev->mach->gpio_vbus);

pxa2xx_udc_vbus_session(&dev->gadget, vbus);
return IRQ_HANDLED;
Index: pxa/arch/arm/mach-pxa/corgi_pm.c
===================================================================
--- pxa.orig/arch/arm/mach-pxa/corgi_pm.c 2006-12-10 01:30:42.000000000 -0800
+++ pxa/arch/arm/mach-pxa/corgi_pm.c 2006-12-29 16:57:32.000000000 -0800
@@ -20,6 +20,7 @@
#include <asm/irq.h>
#include <asm/mach-types.h>
#include <asm/hardware.h>
+#include <asm/gpio.h>
#include <asm/hardware/scoop.h>

#include <asm/arch/sharpsl.h>
@@ -45,10 +46,7 @@ static void corgi_charger_init(void)

static void corgi_measure_temp(int on)
{
- if (on)
- GPSR(CORGI_GPIO_ADC_TEMP_ON) = GPIO_bit(CORGI_GPIO_ADC_TEMP_ON);
- else
- GPCR(CORGI_GPIO_ADC_TEMP_ON) = GPIO_bit(CORGI_GPIO_ADC_TEMP_ON);
+ gpio_set_value(CORGI_GPIO_ADC_TEMP_ON, on);
}

static void corgi_charge(int on)
@@ -69,10 +67,7 @@ static void corgi_charge(int on)

static void corgi_discharge(int on)
{
- if (on)
- GPSR(CORGI_GPIO_DISCHARGE_ON) = GPIO_bit(CORGI_GPIO_DISCHARGE_ON);
- else
- GPCR(CORGI_GPIO_DISCHARGE_ON) = GPIO_bit(CORGI_GPIO_DISCHARGE_ON);
+ gpio_set_value(CORGI_GPIO_DISCHARGE_ON, on);
}

static void corgi_presuspend(void)
@@ -81,17 +76,17 @@ static void corgi_presuspend(void)
unsigned long wakeup_mask;

/* charging , so CHARGE_ON bit is HIGH during OFF. */
- if (READ_GPIO_BIT(CORGI_GPIO_CHRG_ON))
+ if (gpio_get_value(CORGI_GPIO_CHRG_ON))
PGSR1 |= GPIO_bit(CORGI_GPIO_CHRG_ON);
else
PGSR1 &= ~GPIO_bit(CORGI_GPIO_CHRG_ON);

- if (READ_GPIO_BIT(CORGI_GPIO_LED_ORANGE))
+ if (gpio_get_value(CORGI_GPIO_LED_ORANGE))
PGSR0 |= GPIO_bit(CORGI_GPIO_LED_ORANGE);
else
PGSR0 &= ~GPIO_bit(CORGI_GPIO_LED_ORANGE);

- if (READ_GPIO_BIT(CORGI_GPIO_CHRG_UKN))
+ if (gpio_get_value(CORGI_GPIO_CHRG_UKN))
PGSR1 |= GPIO_bit(CORGI_GPIO_CHRG_UKN);
else
PGSR1 &= ~GPIO_bit(CORGI_GPIO_CHRG_UKN);
@@ -171,13 +166,13 @@ unsigned long corgipm_read_devdata(int t
{
switch(type) {
case SHARPSL_STATUS_ACIN:
- return ((GPLR(CORGI_GPIO_AC_IN) & GPIO_bit(CORGI_GPIO_AC_IN)) != 0);
+ return gpio_get_value(CORGI_GPIO_AC_IN) != 0;
case SHARPSL_STATUS_LOCK:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batlock);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_batlock);
case SHARPSL_STATUS_CHRGFULL:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batfull);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_batfull);
case SHARPSL_STATUS_FATAL:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_fatal);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_fatal);
case SHARPSL_ACIN_VOLT:
return sharpsl_pm_pxa_read_max1111(MAX1111_ACIN_VOLT);
case SHARPSL_BATT_TEMP:
Index: pxa/drivers/video/backlight/corgi_bl.c
===================================================================
--- pxa.orig/drivers/video/backlight/corgi_bl.c 2006-12-10 01:31:55.000000000 -0800
+++ pxa/drivers/video/backlight/corgi_bl.c 2006-12-29 16:34:41.000000000 -0800
@@ -121,7 +121,7 @@ static int corgibl_probe(struct platform
machinfo->limit_mask = -1;

corgi_backlight_device = backlight_device_register ("corgi-bl",
- NULL, &corgibl_data);
+ &pdev->dev, NULL, &corgibl_data);
if (IS_ERR (corgi_backlight_device))
return PTR_ERR (corgi_backlight_device);

Index: pxa/arch/arm/mach-pxa/corgi_lcd.c
===================================================================
--- pxa.orig/arch/arm/mach-pxa/corgi_lcd.c 2006-12-10 01:30:42.000000000 -0800
+++ pxa/arch/arm/mach-pxa/corgi_lcd.c 2006-12-29 16:45:42.000000000 -0800
@@ -20,6 +20,8 @@
#include <linux/platform_device.h>
#include <linux/module.h>
#include <linux/string.h>
+
+#include <asm/gpio.h>
#include <asm/arch/akita.h>
#include <asm/arch/corgi.h>
#include <asm/hardware.h>
@@ -449,8 +451,10 @@ static unsigned long (*get_hsync_time)(s

static void inline sharpsl_wait_sync(int gpio)
{
- while((GPLR(gpio) & GPIO_bit(gpio)) == 0);
- while((GPLR(gpio) & GPIO_bit(gpio)) != 0);
+ while (gpio_get_value(gpio) == 0)
+ continue;
+ while (gpio_get_value(gpio) != 0)
+ continue;
}

#ifdef CONFIG_PXA_SHARP_C7xx
Index: pxa/drivers/video/backlight/locomolcd.c
===================================================================
--- pxa.orig/drivers/video/backlight/locomolcd.c 2006-12-10 01:31:55.000000000 -0800
+++ pxa/drivers/video/backlight/locomolcd.c 2006-12-29 16:37:31.000000000 -0800
@@ -184,7 +184,8 @@ static int locomolcd_probe(struct locomo

local_irq_restore(flags);

- locomolcd_bl_device = backlight_device_register("locomo-bl", NULL, &locomobl_data);
+ locomolcd_bl_device = backlight_device_register("locomo-bl",
+ &ldev->dev, NULL, &locomobl_data);

if (IS_ERR (locomolcd_bl_device))
return PTR_ERR (locomolcd_bl_device);
Index: pxa/arch/arm/mach-pxa/spitz_pm.c
===================================================================
--- pxa.orig/arch/arm/mach-pxa/spitz_pm.c 2006-12-10 01:30:42.000000000 -0800
+++ pxa/arch/arm/mach-pxa/spitz_pm.c 2006-12-29 16:50:47.000000000 -0800
@@ -176,11 +176,11 @@ unsigned long spitzpm_read_devdata(int t
case SHARPSL_STATUS_ACIN:
return (((~GPLR(SPITZ_GPIO_AC_IN)) & GPIO_bit(SPITZ_GPIO_AC_IN)) != 0);
case SHARPSL_STATUS_LOCK:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batlock);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_batlock);
case SHARPSL_STATUS_CHRGFULL:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batfull);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_batfull);
case SHARPSL_STATUS_FATAL:
- return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_fatal);
+ return gpio_get_value(sharpsl_pm.machinfo->gpio_fatal);
case SHARPSL_ACIN_VOLT:
return sharpsl_pm_pxa_read_max1111(MAX1111_ACIN_VOLT);
case SHARPSL_BATT_TEMP:
Index: pxa/arch/arm/mach-pxa/sharpsl.h
===================================================================
--- pxa.orig/arch/arm/mach-pxa/sharpsl.h 2006-12-10 01:30:42.000000000 -0800
+++ pxa/arch/arm/mach-pxa/sharpsl.h 2006-12-29 16:51:14.000000000 -0800
@@ -44,12 +44,6 @@ void corgi_wait_hsync(void);
void spitz_wait_hsync(void);

-/*
- * SharpSL Battery/PM Driver
- */
-
-#define READ_GPIO_BIT(x) (GPLR(x) & GPIO_bit(x))
-
/* MAX1111 Channel Definitions */
#define MAX1111_BATT_VOLT 4u
#define MAX1111_BATT_TEMP 2u