[PATCH] [GPIO]: Handle GPIO shadowing with generic GPIOs

Ben Dooks ben-linux at fluff.org
Sun Oct 12 12:44:25 CEST 2008


On Sun, Oct 12, 2008 at 11:19:53AM +0100, Andy Green wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi Ben -
> 
> Is this kind of change to
> 
> arch/arm/mach-s3c2410/include/mach/gpio.h
> 
> generally going to be acceptable upstream?  This is a workaround for an
> output GPIO which is loaded so heavily on some board revisions here the
> input register for it does not read back the true drive level.
> 
> At the moment we take care of it outside the s3c platform side; if we
> can hide it in there it's convenient, if not we can leave it like it is.

Hmm, this is an interesting problem, I did think that when a pin was
in output mode GPxDAT actually read back the value of the output latch
and not the actual pin. Is it possible to expand on the actual cause
of the problem?

I am actively interested in removing as many of the s3c24xx gpio calls
and replace them with the gpio library. As you may note, there are
several driver patches being pushed to move some of the s3c24xx drivers
to using these calls.

If we are using gpiolib, then moving the entirety of the gpio range to
have a shadow would be easier on the hacking of the code for specific
banks.
 
> - -Andy
> 
> 
> Jonas Bonn wrote:
> > This patch moves the shadowing of GPIO's in bank B to the generic GPIO
> > layer.  This makes the shadowing transparent and allows even these broken
> > GPIO's (on effected hardware) to be treated as normal GPIO's.
> > 
> > The function neo1973_gpb_setpin is no longer defined in neo1973.h.
> > All GPIO's should be set with gpio_set_value; for now,
> > neo1973_gpb_setpin has been redefined as such.  When all occurences of
> > neo1973_gpb_setpin have been removed, then the file
> > include/asm-arm/plat-s3c24xx/neo1973.h can be deleted.
> > 
> > Additionally, the shadowing is made specific to the GTA02 only.
> > 
> > Signed-off-by: Jonas Bonn <jonas.bonn at gmail.com>
> > ---
> >  arch/arm/mach-s3c2410/include/mach/gpio.h |   33 +++++++++++++++++++++++++++++
> >  arch/arm/plat-s3c24xx/Makefile            |    2 +-
> >  arch/arm/plat-s3c24xx/neo1973_shadow.c    |   29 +++++++++---------------
> >  drivers/leds/leds-neo1973-gta02.c         |    1 -
> >  include/asm-arm/plat-s3c24xx/neo1973.h    |    9 ++++++-
> >  5 files changed, 52 insertions(+), 22 deletions(-)
> > 
> > diff --git a/arch/arm/mach-s3c2410/include/mach/gpio.h b/arch/arm/mach-s3c2410/include/mach/gpio.h
> > index 7232f8c..6e31ac3 100644
> > --- a/arch/arm/mach-s3c2410/include/mach/gpio.h
> > +++ b/arch/arm/mach-s3c2410/include/mach/gpio.h
> > @@ -11,8 +11,39 @@
> >   * published by the Free Software Foundation.
> >  */
> >  
> > +#ifndef _ASM_ARM_GPIO_H
> > +#define _ASM_ARM_GPIO_H
> > +
> >  #define gpio_get_value	__gpio_get_value
> > +
> > +/*
> > + * The CONFIG_MACH_NEO1973_GTA02 case is to take care of borked hardware that 
> > + * can't be read back and thus needs to have its values shadowed.
> > + */
> > +
> > +#ifdef CONFIG_MACH_NEO1973_GTA02
> > +#include <asm/mach-types.h>
> > +#include <asm/arch/gta02.h>
> > +extern void neo1973_gpb_setpin(unsigned int pin, unsigned int to);
> > +extern void __gpio_set_value(unsigned gpio, int value);
> > +
> > +static inline void __gta02_gpio_set_value(unsigned gpio, int value) {
> > +	if (machine_is_neo1973_gta02() && system_rev == GTA02v5_SYSTEM_REV) {
> > +		if (gpio >= S3C2410_GPIO_BANKB && gpio < S3C2410_GPIO_BANKB+10) {
> > +			neo1973_gpb_setpin(gpio, value);
> > +		} else {
> > +			__gpio_set_value(gpio,value);
> > +		}
> > +	} else {
> > +                __gpio_set_value(gpio,value);
> > +        }                       
> > +}
> > +
> > +#define gpio_set_value	__gta02_gpio_set_value
> > +#else
> >  #define gpio_set_value	__gpio_set_value
> > +#endif
> > +
> >  #define gpio_cansleep	__gpio_cansleep
> >  
> >  /* These two defines should be removed as soon as the
> > @@ -23,3 +54,5 @@
> >  /* -- cut to here when generic irq makes it */
> >  
> >  #include <asm-generic/gpio.h>
> > +
> > +#endif
> > diff --git a/arch/arm/plat-s3c24xx/Makefile b/arch/arm/plat-s3c24xx/Makefile
> > index 49f7027..dcf7a2d 100644
> > --- a/arch/arm/plat-s3c24xx/Makefile
> > +++ b/arch/arm/plat-s3c24xx/Makefile
> > @@ -37,6 +37,6 @@ obj-$(CONFIG_MACH_NEO1973)	+= neo1973_version.o \
> >  				   neo1973_pm_gsm.o \
> >  				   neo1973_pm_gps.o \
> >  				   neo1973_pm_bt.o  \
> > -				   neo1973_shadow.o \
> >  				   neo1973_pm_resume_reason.o \
> >  				   neo1973_memconfig.o
> > +obj-$(CONFIG_MACH_NEO1973_GTA02) += neo1973_shadow.o
> > diff --git a/arch/arm/plat-s3c24xx/neo1973_shadow.c b/arch/arm/plat-s3c24xx/neo1973_shadow.c
> > index 0ff3b83..aaed189 100644
> > --- a/arch/arm/plat-s3c24xx/neo1973_shadow.c
> > +++ b/arch/arm/plat-s3c24xx/neo1973_shadow.c
> > @@ -1,7 +1,7 @@
> >  /*
> > - * include/asm-arm/plat-s3c24xx/neo1973.h
> > + * arch/arm/plat-s3c24xx/neo1973_shadow.h
> >   *
> > - * Common utility code for GTA01 and GTA02
> > + * Utility code for GTA02
> >   *
> >   * Copyright (C) 2008 by Openmoko, Inc.
> >   * Author: Holger Hans Peter Freyther <freyther at openmoko.org>
> > @@ -27,28 +27,21 @@
> >  #include <linux/io.h>
> >  #include <linux/irq.h>
> >  
> > -#include <asm/gpio.h>
> >  #include <mach/regs-gpio.h>
> > -#include <asm/plat-s3c24xx/neo1973.h>
> >  
> >  /**
> > - * Shadow GPIO bank B handling. For the LEDs we need to keep track of the state
> > - * in software. The s3c2410_gpio_setpin must not be used for GPIOs on bank B
> > + * Shadowing of bank B GPIO's: for the LEDS we keep track of their state in 
> > + * software.  This exists because of a hardware defect on the GTA02v5 which 
> > + * prevents bank B GPIO's 0,1, and 2 from being read back properly.
> > + * Since we know these GPIO's to be the only ones effected, we hardcode the 
> > + * mask; this file is for GTA02 only and not intended to be generic in any way.
> > + *
> > + * These functions are used by arch/arm/mach-s3c2410/include/mach/gpio.h only;
> > + * they should not be used else elsewhere.
> >   */
> > -static unsigned long gpb_mask;
> > +static unsigned long gpb_mask = 0x07;
> >  static unsigned long gpb_state;
> >  
> > -void neo1973_gpb_add_shadow_gpio(unsigned int gpio)
> > -{
> > -	unsigned long offset = S3C2410_GPIO_OFFSET(gpio);
> > -	unsigned long flags;
> > -
> > -	local_irq_save(flags);
> > -	gpb_mask |= 1L << offset;
> > -	local_irq_restore(flags);
> > -}
> > -EXPORT_SYMBOL(neo1973_gpb_add_shadow_gpio);
> > -
> >  static void set_shadow_gpio(unsigned long offset, unsigned int value)
> >  {
> >  	unsigned long state = value != 0;
> > diff --git a/drivers/leds/leds-neo1973-gta02.c b/drivers/leds/leds-neo1973-gta02.c
> > index 706f99f..9eb8cb4 100644
> > --- a/drivers/leds/leds-neo1973-gta02.c
> > +++ b/drivers/leds/leds-neo1973-gta02.c
> > @@ -117,7 +117,6 @@ static int __init gta02led_probe(struct platform_device *pdev)
> >  		case S3C2410_GPB1:
> >  		case S3C2410_GPB2:
> >  			s3c2410_gpio_cfgpin(lp->gpio, S3C2410_GPIO_OUTPUT);
> > -			neo1973_gpb_add_shadow_gpio(lp->gpio);
> >  			break;
> >  		default:
> >  			break;
> > diff --git a/include/asm-arm/plat-s3c24xx/neo1973.h b/include/asm-arm/plat-s3c24xx/neo1973.h
> > index 63297de..6a23975 100644
> > --- a/include/asm-arm/plat-s3c24xx/neo1973.h
> > +++ b/include/asm-arm/plat-s3c24xx/neo1973.h
> > @@ -27,7 +27,12 @@
> >  #ifndef NEO1973_H
> >  #define NEO1973_H
> >  
> > -void neo1973_gpb_add_shadow_gpio(unsigned int gpio);
> > -void neo1973_gpb_setpin(unsigned int pin, unsigned to);
> > +/*
> > + * This file can be deleted when neo1973_gpb_setpin has been replaced by
> > + * gpio_set_value throughout the rest of the code.
> > + */
> > +
> > +#include <linux/gpio.h>
> > +#define neo1973_gpb_setpin gpio_set_value
> >  
> >  #endif
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
> 
> iEYEARECAAYFAkjxz0kACgkQOjLpvpq7dMqhegCeNXR1CEoi5rmXtsgMFQ7lz/Cv
> mEYAnjGOfLskccd721USmOsv0UZ7VaKE
> =5PeY
> -----END PGP SIGNATURE-----

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.




More information about the openmoko-kernel mailing list