diff mbox

[U-Boot,v8,2/4] gpio: Replace ARM gpio.h with the common API in include/asm-generic

Message ID 1320459901-30141-2-git-send-email-joe.hershberger@ni.com
State Superseded
Headers show

Commit Message

Joe Hershberger Nov. 5, 2011, 2:24 a.m. UTC
ARM boards should use the generic GPIO API
Update the existing gpio implementations to conform the the generic API

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Kim Phillips <kim.phillips@freescale.com>
---
Changes for v4:
 - Split out of patch 1/2
Changes for v5:
 - Moved asm/arch/gpio.h include to asm/gpio.h
Changes for v6:
Changes for v7:
Changes for v8:
 - Include omap-common/gpio.c in retrofit

 arch/arm/cpu/armv7/omap-common/gpio.c |   23 +++--
 arch/arm/include/asm/gpio.h           |   38 +--------
 drivers/gpio/da8xx_gpio.c             |   73 +++++++--------
 drivers/gpio/tegra2_gpio.c            |  160 ++++++++++++++++----------------
 4 files changed, 128 insertions(+), 166 deletions(-)

Comments

Stephen Warren Nov. 7, 2011, 4:35 p.m. UTC | #1
Joe Hershberger wrote at Friday, November 04, 2011 8:25 PM:
> ARM boards should use the generic GPIO API
> Update the existing gpio implementations to conform the the generic API

The Tegra changes look mostly OK, but:

a) They are almost entirely unrelated to this patch description; most of
the diff is variable renaming and nothing to do with conforming to the API.

b) I've CC'd the main Tegra maintainers to give final comment.

c) Some comments below.

> diff --git a/drivers/gpio/tegra2_gpio.c b/drivers/gpio/tegra2_gpio.c
...
> -void gpio_free(int gp)
> +int gpio_free(unsigned gpio)
>  {
> +	return 0;
>  }

If you're doing a cleanup pass on this driver, you may as well make
gpio_free() do something; it should probably clear gpio_names[gpio].name
and perhaps set the pin back to SFIO - in other words, undo gpio_reqeust().

> -void gpio_toggle_value(int gp)
> -{
> -	gpio_set_value(gp, !gpio_get_output_value(gp));

You should probably at least mention this function being removed in the
commit description. I hope nothing is using it.
Stephen Warren Nov. 7, 2011, 5:02 p.m. UTC | #2
Mike Frysinger wrote at Monday, November 07, 2011 10:40 AM:
> * PGP Signed by an unknown key
> 
> On Monday 07 November 2011 11:35:33 Stephen Warren wrote:
> > Joe Hershberger wrote at Friday, November 04, 2011 8:25 PM:
> > > -void gpio_free(int gp)
> > > +int gpio_free(unsigned gpio)
> > >  {
> > > +	return 0;
> > >  }
> >
> > If you're doing a cleanup pass on this driver, you may as well make
> > gpio_free() do something; it should probably clear gpio_names[gpio].name
> > and perhaps set the pin back to SFIO - in other words, undo gpio_reqeust().
> 
> i think the decision made in Linux was that freeing a GPIO should not cause it
> to change tristate or anything.  all it should do is mark it as "available" so
> something else can request it.
> -mike

OK, I'll buy that, but presumably gpio_names[gpio].name should still be cleared
to indicate the pin is free?
Mike Frysinger Nov. 7, 2011, 5:39 p.m. UTC | #3
On Monday 07 November 2011 11:35:33 Stephen Warren wrote:
> Joe Hershberger wrote at Friday, November 04, 2011 8:25 PM:
> > -void gpio_free(int gp)
> > +int gpio_free(unsigned gpio)
> >  {
> > +	return 0;
> >  }
> 
> If you're doing a cleanup pass on this driver, you may as well make
> gpio_free() do something; it should probably clear gpio_names[gpio].name
> and perhaps set the pin back to SFIO - in other words, undo gpio_reqeust().

i think the decision made in Linux was that freeing a GPIO should not cause it 
to change tristate or anything.  all it should do is mark it as "available" so 
something else can request it.
-mike
Joe Hershberger Nov. 7, 2011, 5:45 p.m. UTC | #4
On Mon, Nov 7, 2011 at 11:02 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Mike Frysinger wrote at Monday, November 07, 2011 10:40 AM:
>> * PGP Signed by an unknown key
>>
>> On Monday 07 November 2011 11:35:33 Stephen Warren wrote:
>> > Joe Hershberger wrote at Friday, November 04, 2011 8:25 PM:
>> > > -void gpio_free(int gp)
>> > > +int gpio_free(unsigned gpio)
>> > >  {
>> > > + return 0;
>> > >  }
>> >
>> > If you're doing a cleanup pass on this driver, you may as well make
>> > gpio_free() do something; it should probably clear gpio_names[gpio].name
>> > and perhaps set the pin back to SFIO - in other words, undo gpio_reqeust().
>>
>> i think the decision made in Linux was that freeing a GPIO should not cause it
>> to change tristate or anything.  all it should do is mark it as "available" so
>> something else can request it.
>> -mike
>
> OK, I'll buy that, but presumably gpio_names[gpio].name should still be cleared
> to indicate the pin is free?

Besides, that would directly undo this patch:
http://patchwork.ozlabs.org/patch/119277/

I'll clear the string.

Thanks,
-Joe
Mike Frysinger Nov. 7, 2011, 7:02 p.m. UTC | #5
On Monday 07 November 2011 12:02:16 Stephen Warren wrote:
> Mike Frysinger wrote at Monday, November 07, 2011 10:40 AM:
> > On Monday 07 November 2011 11:35:33 Stephen Warren wrote:
> > > Joe Hershberger wrote at Friday, November 04, 2011 8:25 PM:
> > > > -void gpio_free(int gp)
> > > > +int gpio_free(unsigned gpio)
> > > >  {
> > > > +	return 0;
> > > >  }
> > > 
> > > If you're doing a cleanup pass on this driver, you may as well make
> > > gpio_free() do something; it should probably clear
> > > gpio_names[gpio].name and perhaps set the pin back to SFIO - in other
> > > words, undo gpio_reqeust().
> > 
> > i think the decision made in Linux was that freeing a GPIO should not
> > cause it to change tristate or anything.  all it should do is mark it as
> > "available" so something else can request it.
> 
> OK, I'll buy that, but presumably gpio_names[gpio].name should still be
> cleared to indicate the pin is free?

if the tegra code is using the gpio_names[] array to determine whether a pin 
is allocated, then gpio_free() should take care of clearing it
-mike
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/omap-common/gpio.c b/arch/arm/cpu/armv7/omap-common/gpio.c
index 75a02da..fc62acc 100644
--- a/arch/arm/cpu/armv7/omap-common/gpio.c
+++ b/arch/arm/cpu/armv7/omap-common/gpio.c
@@ -36,7 +36,7 @@ 
  * published by the Free Software Foundation.
  */
 #include <common.h>
-#include <asm/arch/gpio.h>
+#include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/errno.h>
 
@@ -142,20 +142,22 @@  static void _set_gpio_dataout(const struct gpio_bank *bank, int gpio,
 /**
  * Set value of the specified gpio
  */
-void gpio_set_value(int gpio, int value)
+int gpio_set_value(unsigned gpio, int value)
 {
 	const struct gpio_bank *bank;
 
 	if (check_gpio(gpio) < 0)
-		return;
+		return -1;
 	bank = get_gpio_bank(gpio);
 	_set_gpio_dataout(bank, get_gpio_index(gpio), value);
+
+	return 0;
 }
 
 /**
  * Get value of the specified gpio
  */
-int gpio_get_value(int gpio)
+int gpio_get_value(unsigned gpio)
 {
 	const struct gpio_bank *bank;
 	void *reg;
@@ -176,11 +178,11 @@  int gpio_get_value(int gpio)
 			reg += OMAP_GPIO_DATAOUT;
 			break;
 		default:
-			return -EINVAL;
+			return -1;
 		}
 		break;
 	default:
-		return -EINVAL;
+		return -1;
 	}
 	return (__raw_readl(reg)
 			& (1 << get_gpio_index(gpio))) != 0;
@@ -194,7 +196,7 @@  int gpio_direction_input(unsigned gpio)
 	const struct gpio_bank *bank;
 
 	if (check_gpio(gpio) < 0)
-		return -EINVAL;
+		return -1;
 
 	bank = get_gpio_bank(gpio);
 	_set_gpio_direction(bank, get_gpio_index(gpio), 1);
@@ -210,7 +212,7 @@  int gpio_direction_output(unsigned gpio, int value)
 	const struct gpio_bank *bank;
 
 	if (check_gpio(gpio) < 0)
-		return -EINVAL;
+		return -1;
 
 	bank = get_gpio_bank(gpio);
 	_set_gpio_dataout(bank, get_gpio_index(gpio), value);
@@ -224,7 +226,7 @@  int gpio_direction_output(unsigned gpio, int value)
  *
  * NOTE: Argument 'label' is unused.
  */
-int gpio_request(int gpio, const char *label)
+int gpio_request(unsigned gpio, const char *label)
 {
 	if (check_gpio(gpio) < 0)
 		return -EINVAL;
@@ -235,6 +237,7 @@  int gpio_request(int gpio, const char *label)
 /**
  * Reset and free the gpio after using it.
  */
-void gpio_free(unsigned gpio)
+int gpio_free(unsigned gpio)
 {
+	return 0;
 }
diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index eb071d1..d49ad08 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -1,38 +1,2 @@ 
-/*
- * Copyright (c) 2011, NVIDIA Corp. All rights reserved.
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * 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 _GPIO_H_
-#define _GPIO_H_
-
 #include <asm/arch/gpio.h>
-/*
- * Generic GPIO API
- */
-
-int gpio_request(int gp, const char *label);
-void gpio_free(int gp);
-void gpio_toggle_value(int gp);
-int gpio_direction_input(int gp);
-int gpio_direction_output(int gp, int value);
-int gpio_get_value(int gp);
-void gpio_set_value(int gp, int value);
-
-#endif	/* _GPIO_H_ */
+#include <asm-generic/gpio.h>
diff --git a/drivers/gpio/da8xx_gpio.c b/drivers/gpio/da8xx_gpio.c
index 7a15614..9c2df44 100644
--- a/drivers/gpio/da8xx_gpio.c
+++ b/drivers/gpio/da8xx_gpio.c
@@ -181,90 +181,85 @@  static const struct pinmux_config gpio_pinmux[] = {
 	{ pinmux(18), 8, 2 },
 };
 
-int gpio_request(int gp, const char *label)
+int gpio_request(unsigned gpio, const char *label)
 {
-	if (gp >= MAX_NUM_GPIOS)
+	if (gpio >= MAX_NUM_GPIOS)
 		return -1;
 
-	if (gpio_registry[gp].is_registered)
+	if (gpio_registry[gpio].is_registered)
 		return -1;
 
-	gpio_registry[gp].is_registered = 1;
-	strncpy(gpio_registry[gp].name, label, GPIO_NAME_SIZE);
-	gpio_registry[gp].name[GPIO_NAME_SIZE - 1] = 0;
+	gpio_registry[gpio].is_registered = 1;
+	strncpy(gpio_registry[gpio].name, label, GPIO_NAME_SIZE);
+	gpio_registry[gpio].name[GPIO_NAME_SIZE - 1] = 0;
 
-	davinci_configure_pin_mux(&gpio_pinmux[gp], 1);
+	davinci_configure_pin_mux(&gpio_pinmux[gpio], 1);
 
 	return 0;
 }
 
-void gpio_free(int gp)
+int gpio_free(unsigned gpio)
 {
-	gpio_registry[gp].is_registered = 0;
-}
-
-void gpio_toggle_value(int gp)
-{
-	struct davinci_gpio *bank;
-
-	bank = GPIO_BANK(gp);
-	gpio_set_value(gp, !gpio_get_value(gp));
+	gpio_registry[gpio].is_registered = 0;
+	return 0;
 }
 
-int gpio_direction_input(int gp)
+int gpio_direction_input(unsigned gpio)
 {
 	struct davinci_gpio *bank;
 
-	bank = GPIO_BANK(gp);
-	setbits_le32(&bank->dir, 1U << GPIO_BIT(gp));
+	bank = GPIO_BANK(gpio);
+	setbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
 	return 0;
 }
 
-int gpio_direction_output(int gp, int value)
+int gpio_direction_output(unsigned gpio, int value)
 {
 	struct davinci_gpio *bank;
 
-	bank = GPIO_BANK(gp);
-	clrbits_le32(&bank->dir, 1U << GPIO_BIT(gp));
-	gpio_set_value(gp, value);
+	bank = GPIO_BANK(gpio);
+	clrbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
+	gpio_set_value(gpio, value);
 	return 0;
 }
 
-int gpio_get_value(int gp)
+int gpio_get_value(unsigned gpio)
 {
 	struct davinci_gpio *bank;
 	unsigned int ip;
 
-	bank = GPIO_BANK(gp);
-	ip = in_le32(&bank->in_data) & (1U << GPIO_BIT(gp));
+	bank = GPIO_BANK(gpio);
+	ip = in_le32(&bank->in_data) & (1U << GPIO_BIT(gpio));
 	return ip ? 1 : 0;
 }
 
-void gpio_set_value(int gp, int value)
+int gpio_set_value(unsigned gpio, int value)
 {
 	struct davinci_gpio *bank;
 
-	bank = GPIO_BANK(gp);
+	bank = GPIO_BANK(gpio);
 
 	if (value)
-		bank->set_data = 1U << GPIO_BIT(gp);
+		bank->set_data = 1U << GPIO_BIT(gpio);
 	else
-		bank->clr_data = 1U << GPIO_BIT(gp);
+		bank->clr_data = 1U << GPIO_BIT(gpio);
+
+	return 0;
 }
 
 void gpio_info(void)
 {
-	int gp, dir, val;
+	unsigned gpio, dir, val;
 	struct davinci_gpio *bank;
 
-	for (gp = 0; gp < MAX_NUM_GPIOS; ++gp) {
-		bank = GPIO_BANK(gp);
-		dir = in_le32(&bank->dir) & (1U << GPIO_BIT(gp));
-		val = gpio_get_value(gp);
+	for (gpio = 0; gpio < MAX_NUM_GPIOS; ++gpio) {
+		bank = GPIO_BANK(gpio);
+		dir = in_le32(&bank->dir) & (1U << GPIO_BIT(gpio));
+		val = gpio_get_value(gpio);
 
 		printf("% 4d: %s: %d [%c] %s\n",
-			gp, dir ? " in" : "out", val,
-			gpio_registry[gp].is_registered ? 'x' : ' ',
-			gpio_registry[gp].name);
+			gpio, dir ? " in" : "out", val,
+			gpio_registry[gpio].is_registered ? 'x' : ' ',
+			gpio_registry[gpio].name);
 	}
 }
diff --git a/drivers/gpio/tegra2_gpio.c b/drivers/gpio/tegra2_gpio.c
index f686e80..8f8db37 100644
--- a/drivers/gpio/tegra2_gpio.c
+++ b/drivers/gpio/tegra2_gpio.c
@@ -49,186 +49,185 @@  static char *get_name(int i)
 	return *gpio_names[i].name ? gpio_names[i].name : "UNKNOWN";
 }
 
-/* Return config of pin 'gp' as GPIO (1) or SFPIO (0) */
-static int get_config(int gp)
+/* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */
+static int get_config(unsigned gpio)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	u32 u;
 	int type;
 
-	u = readl(&bank->gpio_config[GPIO_PORT(gp)]);
-	type =  (u >> GPIO_BIT(gp)) & 1;
+	u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
+	type =  (u >> GPIO_BIT(gpio)) & 1;
 
 	debug("get_config: port = %d, bit = %d is %s\n",
-		GPIO_FULLPORT(gp), GPIO_BIT(gp), type ? "GPIO" : "SFPIO");
+		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
 
 	return type;
 }
 
-/* Config pin 'gp' as GPIO or SFPIO, based on 'type' */
-static void set_config(int gp, int type)
+/* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */
+static void set_config(unsigned gpio, int type)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	u32 u;
 
 	debug("set_config: port = %d, bit = %d, %s\n",
-		GPIO_FULLPORT(gp), GPIO_BIT(gp), type ? "GPIO" : "SFPIO");
+		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
 
-	u = readl(&bank->gpio_config[GPIO_PORT(gp)]);
+	u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
 	if (type)				/* GPIO */
-		u |= 1 << GPIO_BIT(gp);
+		u |= 1 << GPIO_BIT(gpio);
 	else
-		u &= ~(1 << GPIO_BIT(gp));
-	writel(u, &bank->gpio_config[GPIO_PORT(gp)]);
+		u &= ~(1 << GPIO_BIT(gpio));
+	writel(u, &bank->gpio_config[GPIO_PORT(gpio)]);
 }
 
-/* Return GPIO pin 'gp' direction - 0 = input or 1 = output */
-static int get_direction(int gp)
+/* Return GPIO pin 'gpio' direction - 0 = input or 1 = output */
+static int get_direction(unsigned gpio)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	u32 u;
 	int dir;
 
-	u = readl(&bank->gpio_dir_out[GPIO_PORT(gp)]);
-	dir =  (u >> GPIO_BIT(gp)) & 1;
+	u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]);
+	dir =  (u >> GPIO_BIT(gpio)) & 1;
 
 	debug("get_direction: port = %d, bit = %d, %s\n",
-		GPIO_FULLPORT(gp), GPIO_BIT(gp), dir ? "OUT" : "IN");
+		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN");
 
 	return dir;
 }
 
-/* Config GPIO pin 'gp' as input or output (OE) as per 'output' */
-static void set_direction(int gp, int output)
+/* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */
+static void set_direction(unsigned gpio, int output)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	u32 u;
 
 	debug("set_direction: port = %d, bit = %d, %s\n",
-		GPIO_FULLPORT(gp), GPIO_BIT(gp), output ? "OUT" : "IN");
+		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN");
 
-	u = readl(&bank->gpio_dir_out[GPIO_PORT(gp)]);
+	u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]);
 	if (output)
-		u |= 1 << GPIO_BIT(gp);
+		u |= 1 << GPIO_BIT(gpio);
 	else
-		u &= ~(1 << GPIO_BIT(gp));
-	writel(u, &bank->gpio_dir_out[GPIO_PORT(gp)]);
+		u &= ~(1 << GPIO_BIT(gpio));
+	writel(u, &bank->gpio_dir_out[GPIO_PORT(gpio)]);
 }
 
-/* set GPIO pin 'gp' output bit as 0 or 1 as per 'high' */
-static void set_level(int gp, int high)
+/* set GPIO pin 'gpio' output bit as 0 or 1 as per 'high' */
+static void set_level(unsigned gpio, int high)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	u32 u;
 
 	debug("set_level: port = %d, bit %d == %d\n",
-		GPIO_FULLPORT(gp), GPIO_BIT(gp), high);
+		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), high);
 
-	u = readl(&bank->gpio_out[GPIO_PORT(gp)]);
+	u = readl(&bank->gpio_out[GPIO_PORT(gpio)]);
 	if (high)
-		u |= 1 << GPIO_BIT(gp);
+		u |= 1 << GPIO_BIT(gpio);
 	else
-		u &= ~(1 << GPIO_BIT(gp));
-	writel(u, &bank->gpio_out[GPIO_PORT(gp)]);
+		u &= ~(1 << GPIO_BIT(gpio));
+	writel(u, &bank->gpio_out[GPIO_PORT(gpio)]);
 }
 
 /*
  * Generic_GPIO primitives.
  */
 
-int gpio_request(int gp, const char *label)
+int gpio_request(unsigned gpio, const char *label)
 {
-	if (gp >= MAX_NUM_GPIOS)
+	if (gpio >= MAX_NUM_GPIOS)
 		return -1;
 
-	strncpy(gpio_names[gp].name, label, GPIO_NAME_SIZE);
-	gpio_names[gp].name[GPIO_NAME_SIZE - 1] = '\0';
+	strncpy(gpio_names[gpio].name, label, GPIO_NAME_SIZE);
+	gpio_names[gpio].name[GPIO_NAME_SIZE - 1] = '\0';
 
 	/* Configure as a GPIO */
-	set_config(gp, 1);
+	set_config(gpio, 1);
 
 	return 0;
 }
 
-void gpio_free(int gp)
+int gpio_free(unsigned gpio)
 {
+	return 0;
 }
 
-/* read GPIO OUT value of pin 'gp' */
-static int gpio_get_output_value(int gp)
+/* read GPIO OUT value of pin 'gpio' */
+static int gpio_get_output_value(unsigned gpio)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	int val;
 
 	debug("gpio_get_output_value: pin = %d (port %d:bit %d)\n",
-		gp, GPIO_FULLPORT(gp), GPIO_BIT(gp));
-
-	val = readl(&bank->gpio_out[GPIO_PORT(gp)]);
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
 
-	return (val >> GPIO_BIT(gp)) & 1;
-}
+	val = readl(&bank->gpio_out[GPIO_PORT(gpio)]);
 
-void gpio_toggle_value(int gp)
-{
-	gpio_set_value(gp, !gpio_get_output_value(gp));
+	return (val >> GPIO_BIT(gpio)) & 1;
 }
 
-/* set GPIO pin 'gp' as an input */
-int gpio_direction_input(int gp)
+/* set GPIO pin 'gpio' as an input */
+int gpio_direction_input(unsigned gpio)
 {
 	debug("gpio_direction_input: pin = %d (port %d:bit %d)\n",
-		gp, GPIO_FULLPORT(gp), GPIO_BIT(gp));
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
 
 	/* Configure GPIO direction as input. */
-	set_direction(gp, 0);
+	set_direction(gpio, 0);
 
 	return 0;
 }
 
-/* set GPIO pin 'gp' as an output, with polarity 'value' */
-int gpio_direction_output(int gp, int value)
+/* set GPIO pin 'gpio' as an output, with polarity 'value' */
+int gpio_direction_output(unsigned gpio, int value)
 {
 	debug("gpio_direction_output: pin = %d (port %d:bit %d) = %s\n",
-		gp, GPIO_FULLPORT(gp), GPIO_BIT(gp), value ? "HIGH" : "LOW");
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio),
+		value ? "HIGH" : "LOW");
 
 	/* Configure GPIO output value. */
-	set_level(gp, value);
+	set_level(gpio, value);
 
 	/* Configure GPIO direction as output. */
-	set_direction(gp, 1);
+	set_direction(gpio, 1);
 
 	return 0;
 }
 
-/* read GPIO IN value of pin 'gp' */
-int gpio_get_value(int gp)
+/* read GPIO IN value of pin 'gpio' */
+int gpio_get_value(unsigned gpio)
 {
-	struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &gpio->gpio_bank[GPIO_BANK(gp)];
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
 	int val;
 
 	debug("gpio_get_value: pin = %d (port %d:bit %d)\n",
-		gp, GPIO_FULLPORT(gp), GPIO_BIT(gp));
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
 
-	val = readl(&bank->gpio_in[GPIO_PORT(gp)]);
+	val = readl(&bank->gpio_in[GPIO_PORT(gpio)]);
 
-	return (val >> GPIO_BIT(gp)) & 1;
+	return (val >> GPIO_BIT(gpio)) & 1;
 }
 
-/* write GPIO OUT value to pin 'gp' */
-void gpio_set_value(int gp, int value)
+/* write GPIO OUT value to pin 'gpio' */
+int gpio_set_value(unsigned gpio, int value)
 {
 	debug("gpio_set_value: pin = %d (port %d:bit %d), value = %d\n",
-		gp, GPIO_FULLPORT(gp), GPIO_BIT(gp), value);
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio), value);
 
 	/* Configure GPIO output value. */
-	set_level(gp, value);
+	set_level(gpio, value);
+
+	return 0;
 }
 
 /*
@@ -236,7 +235,8 @@  void gpio_set_value(int gp, int value)
  */
 void gpio_info(void)
 {
-	int c, type;
+	unsigned c;
+	int type;
 
 	for (c = 0; c < MAX_NUM_GPIOS; c++) {
 		type = get_config(c);		/* GPIO, not SFPIO */