linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] AMD Geode CS5535/5536 GPIO driver
@ 2009-02-05 15:10 Alessandro Zummo
  2009-02-06  0:12 ` David Brownell
  2009-02-14 20:27 ` Andres Salomon
  0 siblings, 2 replies; 8+ messages in thread
From: Alessandro Zummo @ 2009-02-05 15:10 UTC (permalink / raw)
  Cc: jordan, David Brownell, jordan, linux-geode, dilinger, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin



A GPIO driver for the AMD Geode CS5535/5536 Companion Devices.

In the release 2 I've addressed all the concerns expressed by the
various subsys maintainers involved.

---
 arch/x86/Kconfig           |   10 ++++
 arch/x86/kernel/geode_32.c |   91 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

--- linux-2.6.28.orig/arch/x86/Kconfig	2009-02-03 19:38:43.000000000 +0100
+++ linux-2.6.28/arch/x86/Kconfig	2009-02-03 20:15:25.000000000 +0100
@@ -1911,6 +1911,16 @@ config GEODE_MFGPT_TIMER
 	  MFGPTs have a better resolution and max interval than the
 	  generic PIT, and are suitable for use as high-res timers.
 
+config GEODE_GPIO
+	bool "Geode GPIO support"
+	depends on MGEODE_LX && !CS5535_GPIO
+	default n
+	help
+          Say yes here to provide support for the 28 GPIO pins on
+          the AMD CS5535 and CS5536 Geode Companion Devices.
+
+          If unsure, say N.
+
 config OLPC
 	bool "One Laptop Per Child support"
 	default n
--- linux-2.6.28.orig/arch/x86/kernel/geode_32.c	2008-12-25 00:26:37.000000000 +0100
+++ linux-2.6.28/arch/x86/kernel/geode_32.c	2009-02-03 20:22:32.000000000 +0100
@@ -2,6 +2,7 @@
  * AMD Geode southbridge support code
  * Copyright (C) 2006, Advanced Micro Devices, Inc.
  * Copyright (C) 2007, Andres Salomon <dilinger@debian.org>
+ * Copyright (C) 2009, Tower Technologies
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of version 2 of the GNU General Public License
@@ -12,6 +13,8 @@
 #include <linux/module.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
 #include <asm/msr.h>
 #include <asm/geode.h>
 
@@ -183,6 +186,89 @@ int geode_has_vsa2(void)
 }
 EXPORT_SYMBOL_GPL(geode_has_vsa2);
 
+/* GPIO subsystem functions */
+#ifdef CONFIG_GEODE_GPIO
+static void cs5535_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	if (value)
+		geode_gpio_set(geode_gpio(offset), GPIO_OUTPUT_VAL);
+	else
+		geode_gpio_clear(geode_gpio(offset), GPIO_OUTPUT_VAL);
+}
+
+static int cs5535_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	return geode_gpio_isset(geode_gpio(offset), GPIO_READ_BACK);
+}
+
+static int cs5535_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	geode_gpio_clear(geode_gpio(offset), GPIO_OUTPUT_ENABLE);
+	geode_gpio_set(geode_gpio(offset), GPIO_INPUT_ENABLE);
+
+	return 0;
+}
+
+static int cs5535_direction_output(struct gpio_chip *chip, unsigned offset,
+					int value)
+{
+	cs5535_gpio_set(chip, offset, value);
+
+	geode_gpio_set(geode_gpio(offset), GPIO_OUTPUT_ENABLE);
+	geode_gpio_clear(geode_gpio(offset), GPIO_INPUT_ENABLE);
+
+	return 0;
+}
+
+static struct gpio_chip cs5535 = {
+	.owner	= THIS_MODULE,
+	.label	= "cs5535-gpio",
+
+	.base	= 0,
+	.ngpio	= 28,
+
+	.get	= cs5535_gpio_get,
+	.set	= cs5535_gpio_set,
+
+	.direction_input = cs5535_direction_input,
+	.direction_output = cs5535_direction_output,
+};
+
+static int __init cs5535_gpio_probe(struct platform_device *pdev)
+{
+	int rc;
+
+	if (!request_region(geode_gpio_base(), LBAR_GPIO_SIZE, "cs5535-gpio")) {
+		dev_err(&pdev->dev, "cannot allocate I/O region at %x\n",
+				geode_gpio_base());
+		return -ENODEV;
+	}
+
+	rc = gpiochip_add(&cs5535);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "cannot add GPIO\n");
+		goto fail_release;
+	}
+
+	dev_info(&pdev->dev, "registered at 0x%x, GPIO base %d\n",
+			geode_gpio_base(), cs5535.base);
+
+	return 0;
+
+fail_release:
+	release_region(geode_gpio_base(), LBAR_GPIO_SIZE);
+
+	return rc;
+}
+
+static struct platform_driver cs5535_gpio_driver = {
+	.driver	 = {
+		.name   = "cs5535-gpio",
+		.owner  = THIS_MODULE,
+	},
+};
+#endif
+
 static int __init geode_southbridge_init(void)
 {
 	if (!is_geode())
@@ -190,6 +276,11 @@ static int __init geode_southbridge_init
 
 	init_lbars();
 	(void) mfgpt_timer_setup();
+
+#ifdef CONFIG_GEODE_GPIO
+	platform_device_register_simple("cs5535-gpio", 0, NULL, 0);
+	platform_driver_probe(&cs5535_gpio_driver, cs5535_gpio_probe);
+#endif
 	return 0;
 }
 

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-05 15:10 [PATCH] AMD Geode CS5535/5536 GPIO driver Alessandro Zummo
@ 2009-02-06  0:12 ` David Brownell
  2009-02-06  9:16   ` Alessandro Zummo
  2009-02-14 20:27 ` Andres Salomon
  1 sibling, 1 reply; 8+ messages in thread
From: David Brownell @ 2009-02-06  0:12 UTC (permalink / raw)
  To: Alessandro Zummo
  Cc: jordan, linux-geode, dilinger, dsaxena, Martin-Éric Racine,
	lkml, rpurdie, Ingo Molnar, Thomas Gleixner, H. Peter Anvin

On Thursday 05 February 2009, Alessandro Zummo wrote:
> --- linux-2.6.28.orig/arch/x86/Kconfig	2009-02-03 19:38:43.000000000 +0100
> +++ linux-2.6.28/arch/x86/Kconfig	2009-02-03 20:15:25.000000000 +0100
> @@ -1911,6 +1911,16 @@ config GEODE_MFGPT_TIMER
>  	  MFGPTs have a better resolution and max interval than the
>  	  generic PIT, and are suitable for use as high-res timers.
>  
> +config GEODE_GPIO
> +	bool "Geode GPIO support"
> +	depends on MGEODE_LX && !CS5535_GPIO

and also GPIOLIB ... maybe can you just "select GPIOLIB".


> +	default n
> +	help


> +static int cs5535_direction_output(struct gpio_chip *chip, unsigned offset,
> +					int value)
> +{
> +	cs5535_gpio_set(chip, offset, value);
> +
> +	geode_gpio_set(geode_gpio(offset), GPIO_OUTPUT_ENABLE);
> +	geode_gpio_clear(geode_gpio(offset), GPIO_INPUT_ENABLE);

Unless this hardware misbehaves when both output and input
modes are enabled (e.g. in the window between those two calls),
I'd suggest just not clearing INPUT_ENABLE.  It's legit to
ask for the *actual* value of an output line, e.g. for when
it uses open drain mode.  (And didn't this hardware have an
option for open drain GPIO signaling?)


Otherwise this has about the right shape for platform GPIOs.

- Dave

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-06  0:12 ` David Brownell
@ 2009-02-06  9:16   ` Alessandro Zummo
  2009-02-15 22:47     ` Jordan Crouse
  0 siblings, 1 reply; 8+ messages in thread
From: Alessandro Zummo @ 2009-02-06  9:16 UTC (permalink / raw)
  To: David Brownell
  Cc: jordan, linux-geode, dilinger, dsaxena, Martin-Éric Racine,
	lkml, rpurdie, Ingo Molnar, Thomas Gleixner, H. Peter Anvin

On Thu, 5 Feb 2009 16:12:20 -0800
David Brownell <david-b@pacbell.net> wrote:

> Unless this hardware misbehaves when both output and input
> modes are enabled (e.g. in the window between those two calls),
> I'd suggest just not clearing INPUT_ENABLE.  It's legit to
> ask for the *actual* value of an output line, e.g. for when
> it uses open drain mode.  (And didn't this hardware have an
> option for open drain GPIO signaling?)

 seems reasonable,
 I'll let the Geode people decide what to do here... 

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-05 15:10 [PATCH] AMD Geode CS5535/5536 GPIO driver Alessandro Zummo
  2009-02-06  0:12 ` David Brownell
@ 2009-02-14 20:27 ` Andres Salomon
  2009-02-14 22:02   ` Alessandro Zummo
  1 sibling, 1 reply; 8+ messages in thread
From: Andres Salomon @ 2009-02-14 20:27 UTC (permalink / raw)
  To: Alessandro Zummo
  Cc: jordan, David Brownell, linux-geode, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin

Sorry, I'm just now getting around to looking at this.  Why did we move it
from cs553x-gpio.c to geode_32.c?  I have a lemote (longsoon-based) which
uses cs5536:

Linux debian 2.6.27.1 #193 Mon Jan 5 11:14:50 CST 2009 mips64 GNU/Linux

system type             : lemote-notebook
processor               : 0
cpu model               : ICT Loongson-2 V0.3  FPU V0.1
BogoMIPS                : 132.09
wait instruction        : yes
microsecond timers      : yes
tlb_entries             : 64
extra interrupt vector  : no
hardware watchpoint     : no
ASEs implemented        :
shadow register sets    : 1
core                    : 0
VCED exceptions         : not available
VCEI exceptions         : not available


00:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 10)
00:08.0 VGA compatible controller: Silicon Motion, Inc. SM712 LynxEM+ (rev b0)
00:09.0 USB Controller: NEC Corporation USB (rev 44)
00:09.1 USB Controller: NEC Corporation USB 2.0 (rev 05)
00:0e.0 ISA bridge: Advanced Micro Devices [AMD] CS5536 [Geode companion] ISA (rev 03)
00:0e.2 IDE interface: Advanced Micro Devices [AMD] CS5536 [Geode companion] IDE (rev 01)
00:0e.3 Multimedia audio controller: Advanced Micro Devices [AMD] CS5536 [Geode companion] Audio (rev 01)
00:0e.4 USB Controller: Advanced Micro Devices [AMD] CS5536 [Geode companion] OHC (rev 02)
00:0e.5 USB Controller: Advanced Micro Devices [AMD] CS5536 [Geode companion] EHC (rev 02)
00:0e.6 USB Controller: Advanced Micro Devices [AMD] CS5536 [Geode companion] UDC (rev 02)
00:0e.7 USB Controller: Advanced Micro Devices [AMD] CS5536 [Geode companion] UOC (rev 02)

This leads me to believe that the gpio framework should not be
architecture-specific (nor geode-specific).


On Thu, 5 Feb 2009 16:10:07 +0100
Alessandro Zummo <azummo-lists@towertech.it> wrote:

> 
> 
> A GPIO driver for the AMD Geode CS5535/5536 Companion Devices.
> 
> In the release 2 I've addressed all the concerns expressed by the
> various subsys maintainers involved.
> 
> ---
[...]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-14 20:27 ` Andres Salomon
@ 2009-02-14 22:02   ` Alessandro Zummo
  2009-02-14 22:38     ` Andres Salomon
  0 siblings, 1 reply; 8+ messages in thread
From: Alessandro Zummo @ 2009-02-14 22:02 UTC (permalink / raw)
  To: Andres Salomon
  Cc: jordan, David Brownell, linux-geode, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin

On Sat, 14 Feb 2009 15:27:19 -0500
Andres Salomon <dilinger@queued.net> wrote:

> Sorry, I'm just now getting around to looking at this.  Why did we move it
> from cs553x-gpio.c to geode_32.c?  I have a lemote (longsoon-based) which
> uses cs5536:

 I've been suggested to move it within geode_32. the driver is probably
 pretty geode specific anyhow.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-14 22:02   ` Alessandro Zummo
@ 2009-02-14 22:38     ` Andres Salomon
  2009-02-14 22:54       ` Alessandro Zummo
  0 siblings, 1 reply; 8+ messages in thread
From: Andres Salomon @ 2009-02-14 22:38 UTC (permalink / raw)
  To: Alessandro Zummo
  Cc: jordan, David Brownell, linux-geode, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin

On Sat, 14 Feb 2009 23:02:35 +0100
Alessandro Zummo <azummo-lists@towertech.it> wrote:

> On Sat, 14 Feb 2009 15:27:19 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > Sorry, I'm just now getting around to looking at this.  Why did we
> > move it from cs553x-gpio.c to geode_32.c?  I have a lemote
> > (longsoon-based) which uses cs5536:
> 
>  I've been suggested to move it within geode_32. the driver is
> probably pretty geode specific anyhow.
> 


If the whole point is to make the cs553x GPIO stuff portable (and to use
the portable API), then what's the point if we know that it can/will
be used by non-x86 platforms?

We already have a cs553x GPIO char device, a geode GPIO lib that
works (but has flaws).. I'm all for creating The One True CS553x GPIO
API, but I'd consider something that's geode (and x86)-specific to be
a waste of time.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-14 22:38     ` Andres Salomon
@ 2009-02-14 22:54       ` Alessandro Zummo
  0 siblings, 0 replies; 8+ messages in thread
From: Alessandro Zummo @ 2009-02-14 22:54 UTC (permalink / raw)
  To: Andres Salomon
  Cc: jordan, David Brownell, linux-geode, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin

On Sat, 14 Feb 2009 17:38:11 -0500
Andres Salomon <dilinger@queued.net> wrote:

> >  I've been suggested to move it within geode_32. the driver is
> > probably pretty geode specific anyhow.
> > 
> 
> 
> If the whole point is to make the cs553x GPIO stuff portable (and to use
> the portable API), then what's the point if we know that it can/will
> be used by non-x86 platforms?
> 
> We already have a cs553x GPIO char device, a geode GPIO lib that
> works (but has flaws).. I'm all for creating The One True CS553x GPIO
> API, but I'd consider something that's geode (and x86)-specific to be
> a waste of time.

 GPIO lib is generic enough for most archs to use it and my driver uses it
 Anyway I did my homework.. I will submit a revision with the latest suggested
 changes... if accepted I will be happy to maintain it.
 


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] AMD Geode CS5535/5536 GPIO driver
  2009-02-06  9:16   ` Alessandro Zummo
@ 2009-02-15 22:47     ` Jordan Crouse
  0 siblings, 0 replies; 8+ messages in thread
From: Jordan Crouse @ 2009-02-15 22:47 UTC (permalink / raw)
  To: Alessandro Zummo
  Cc: David Brownell, linux-geode, dilinger, dsaxena,
	Martin-Éric Racine, lkml, rpurdie, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin

Alessandro Zummo wrote:
> On Thu, 5 Feb 2009 16:12:20 -0800
> David Brownell <david-b@pacbell.net> wrote:
> 
>> Unless this hardware misbehaves when both output and input
>> modes are enabled (e.g. in the window between those two calls),
>> I'd suggest just not clearing INPUT_ENABLE.  It's legit to
>> ask for the *actual* value of an output line, e.g. for when
>> it uses open drain mode.  (And didn't this hardware have an
>> option for open drain GPIO signaling?)
> 
>  seems reasonable,
>  I'll let the Geode people decide what to do here... 

Sorry, I'm just getting around to answering this. I don't recall any 
situation where the hardware would mis-behave, but you never know with 
this bit of silicon.

I say leave them both on, and revisit it if you get flamed.

Jordan





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-02-15 22:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-05 15:10 [PATCH] AMD Geode CS5535/5536 GPIO driver Alessandro Zummo
2009-02-06  0:12 ` David Brownell
2009-02-06  9:16   ` Alessandro Zummo
2009-02-15 22:47     ` Jordan Crouse
2009-02-14 20:27 ` Andres Salomon
2009-02-14 22:02   ` Alessandro Zummo
2009-02-14 22:38     ` Andres Salomon
2009-02-14 22:54       ` Alessandro Zummo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).