cs5535-gpio: add AMD CS5535/CS5536 GPIO driver support

PATCH In review Andres Salomon 4 participants 10 messages Aug 18, 2009 – Aug 28, 2009

Subsystems (3)

x86 GPIO AMD geode processor/chipset support

lore

Share & tools

lore

Andres Salomon

author

2009-08-18 21:53 UTC #0 lore raw

PATCH 1/3 cs5535-gpio: add AMD CS5535/CS5536 GPIO driver support

This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).

Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.

Signed-off-by Andres Salomon <[email protected]>

---
 arch/x86/include/asm/geode.h |   28 +----
 drivers/gpio/Kconfig         |   10 ++
 drivers/gpio/Makefile        |    1 +
 drivers/gpio/cs5535-gpio.c   |  282 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/cs5535.h       |   58 +++++++++
 5 files changed, 352 insertions(+), 27 deletions(-)
 create mode 100644 drivers/gpio/cs5535-gpio.c
 create mode 100644 include/linux/cs5535.h

arch/x86/include/asm/geode.h +1 ~~-27~~

index ad3c2ed..5716214 100644
@@ -12,6 +12,7 @@
 
 #include <asm/processor.h>
 #include <linux/io.h>
+#include <linux/cs5535.h>
 
 /* Generic southbridge functions */

@@ -115,33 +116,6 @@ extern int geode_get_dev_base(unsigned int dev);
 #define VSA_VR_MEM_SIZE		0x0200
 #define AMD_VSA_SIG		0x4132	/* signature is ascii 'VSA2' */
 #define GSW_VSA_SIG		0x534d  /* General Software signature */
-/* GPIO */
-
-#define GPIO_OUTPUT_VAL		0x00
-#define GPIO_OUTPUT_ENABLE	0x04
-#define GPIO_OUTPUT_OPEN_DRAIN	0x08
-#define GPIO_OUTPUT_INVERT	0x0C
-#define GPIO_OUTPUT_AUX1	0x10
-#define GPIO_OUTPUT_AUX2	0x14
-#define GPIO_PULL_UP		0x18
-#define GPIO_PULL_DOWN		0x1C
-#define GPIO_INPUT_ENABLE	0x20
-#define GPIO_INPUT_INVERT	0x24
-#define GPIO_INPUT_FILTER	0x28
-#define GPIO_INPUT_EVENT_COUNT	0x2C
-#define GPIO_READ_BACK		0x30
-#define GPIO_INPUT_AUX1		0x34
-#define GPIO_EVENTS_ENABLE	0x38
-#define GPIO_LOCK_ENABLE	0x3C
-#define GPIO_POSITIVE_EDGE_EN	0x40
-#define GPIO_NEGATIVE_EDGE_EN	0x44
-#define GPIO_POSITIVE_EDGE_STS	0x48
-#define GPIO_NEGATIVE_EDGE_STS	0x4C
-
-#define GPIO_MAP_X		0xE0
-#define GPIO_MAP_Y		0xE4
-#define GPIO_MAP_Z		0xE8
-#define GPIO_MAP_W		0xEC
 
 static inline u32 geode_gpio(unsigned int nr)
 {

drivers/gpio/Kconfig +10 -0

index 96dda81..ff34b2c 100644
@@ -157,6 +157,16 @@ config GPIO_TWL4030
 
 comment "PCI GPIO expanders:"
 
+config GPIO_CS5535
+	tristate "AMD CS5535/CS5536 GPIO support"
+	depends on PCI && !CS5535_GPIO && !MGEODE_LX
+	help
+	  The AMD CS5535 and CS5536 southbridges support 28 GPIO pins that
+	  can be used for quite a number of things.  The CS5535/6 is found on
+	  AMD Geode and Lemote Yeeloong devices.
+
+	  If unsure, say N.
+
 config GPIO_BT8XX
 	tristate "BT8XX GPIO abuser"
 	depends on PCI && VIDEO_BT848=n

drivers/gpio/Makefile +1 -0

index 9244c6f..3ad6b40 100644
@@ -12,5 +12,6 @@ obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
 obj-$(CONFIG_GPIO_PL061)	+= pl061.o
 obj-$(CONFIG_GPIO_TWL4030)	+= twl4030-gpio.o
 obj-$(CONFIG_GPIO_XILINX)	+= xilinx_gpio.o
+obj-$(CONFIG_GPIO_CS5535)	+= cs5535-gpio.o
 obj-$(CONFIG_GPIO_BT8XX)	+= bt8xxgpio.o
 obj-$(CONFIG_GPIO_VR41XX)	+= vr41xx_giu.o

drivers/gpio/cs5535-gpio.c new file +282 -0

282 lines not on this page · raw message

include/linux/cs5535.h new file +58 -0

58 lines not on this page · raw message

Signature · 1 line

1.6.3.3

Andres Salomon

author

2009-08-19 19:13 UTC #1 lore raw

Hi Andrew,

BTW, this patch series includes fixes for symbol build errors (it should fix all build issues that I'm aware of), and Tobias's changes. When you get the chance, can you please include it in your tree so it can get some testing? Thanks!

Quoted 13 lines · On Tue, 18 Aug 2009 17:53:14 -0400 Andres Salomon <[email protected]>…

This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).
Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.
Signed-off-by Andres Salomon <[email protected]>

[...]

Takashi Iwai

2009-08-20 15:21 UTC #2 lore raw

In reply to Andres Salomon

At Tue, 18 Aug 2009 17:53:14 -0400,

Quoted 6 lines · Andres Salomon

This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).

Will be any user of cs5535_gpio_*() expected? If not, it'd be better not to export stuff, IMO.

Quoted 4 lines

Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.

... or you can rewrite the old driver to use the functions above? Then the ugly Kconfig check can be dropped, too. (yeah that's an answer to my own question above.)

BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)

Quoted 1 line

Signed-off-by Andres Salomon <[email protected]>

I can happily merge this patch series to sound GIT tree if no one objects, since it's basically for OLPC sound stuff.

thanks,

Takashi

Andres Salomon

author

2009-08-20 16:40 UTC #3 lore raw

Quoted 4 lines · On Thu, 20 Aug 2009 17:21:20 +0200 Takashi Iwai <[email protected]> wrote:

At Tue, 18 Aug 2009 17:53:14 -0400,
Quoted 2 lines · Andres Salomon

Quoted 8 lines

Quoted 4 lines
This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).
Will be any user of cs5535_gpio_*() expected? If not, it'd be better not to export stuff, IMO.

The olpc-dcon driver, for one. If desired, I can remove the exports for now.

Quoted 9 lines

Quoted 5 lines
Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.
.... or you can rewrite the old driver to use the functions above? Then the ugly Kconfig check can be dropped, too. (yeah that's an answer to my own question above.)

I guess in theory I could..

Quoted 3 lines

BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)

Agreed. I'd really like CONFIG_CS5535_GPIO to go away, but first there needs to be a replacement. CONFIG_CS5535_GPIO will probably also need to be marked DEPRECATED for a while, too, since it's a userspace-visible change. I can follow up w/ a patch for that once people are happy w/ this current set of patches.

Quoted 9 lines

Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
I can happily merge this patch series to sound GIT tree if no one objects, since it's basically for OLPC sound stuff.
thanks,
Takashi

Martin-Éric Racine

2009-08-20 17:03 UTC #4 lore raw

Quoted 1 line · On Thu, 20 Aug 2009 17:21:20 +0200 Takashi Iwai <[email protected]> wrote:

BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)

That's because too many people who write drivers lack a bird's eye view on the whole Geode platform's history and end up inventing driver names that only further add to the confusion. Ages ago, on the Geode list (I cannot remember if was the AMD PCS maintained list or the X.org Geode list), we came to the following nomenclature:

gx1: driver that only affects the first-generation GX from Cyrix or NSC and the NSC SC system on chip's CPU+FPU components.
gx: driver that only affects the NSC/AMD Media GX (a.k.a. GX2 Red Cloud) CPU+FPU components.

lx:  driver that only affect the AMD LX CPU+FPU components.
sc1100:  driver that only affect the sc1100 system on chip's peripheral drivers.
scx200:  driver that affects the sc1200, sc2200 and sc3200 systems on
chip's peripheral drivers.
cs5510:  driver that only affect the cs5510 companion chip.
cs5520:  driver that only affect the cs5520 companion chip.
cs5530:  driver that affects the cs5530 (or cs5530a) companion chip alone.
cs5535:  driver that affects the cs5535 and cs5536 companion chips.
cs5536:  driver that affects the cs5536 specifically (same as cs5535,

plus USB 2.0 support and a different PATA implementation).
cs553x: driver that affects the WHOLE cs553x series. Should not be used since the cs5530 and cs553[5|6] are different beasts.
geode: driver that affects the whole Geode series, such as the SM bus.

Sadly, some drivers don't respect this nomenclature, the most common mistake being to use cs553x for a driver that only works on cs5535 and cs5536. Another mistake was the naming of the ALSA driver, which came as snd-cs5535audio instead of just snd-cs5535.

Martin-Éric

Andres Salomon

author

2009-08-20 17:27 UTC #5 lore raw

Quoted 7 lines · On Thu, 20 Aug 2009 20:03:14 +0300 Martin-Éric Racine <[email protected]> wrote:

Quoted 1 line · On Thu, 20 Aug 2009 17:21:20 +0200 Takashi Iwai <[email protected]> wrote:
BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)
That's because too many people who write drivers lack a bird's eye view on the whole Geode platform's history and end up inventing driver names that only further add to the confusion. Ages ago, on the Geode

Uh, I'm assuming he meant the GPIO_CS* vs CS*_GPIO bit. They're both CS5535 in Kconfig.

Takashi Iwai

2009-08-20 19:35 UTC #6 lore raw

At Thu, 20 Aug 2009 13:27:34 -0400,

Quoted 6 lines · Andres Salomon

Quoted 2 lines · On Thu, 20 Aug 2009 20:03:14 +0300 Martin-Éric Racine <[email protected]> wrote:
On Thu, 20 Aug 2009 17:21:20 +0200 Takashi Iwai <[email protected]>
wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)
That's because too many people who write drivers lack a bird's eye view on the whole Geode platform's history and end up inventing driver names that only further add to the confusion. Ages ago, on the Geode
Uh, I'm assuming he meant the GPIO_CS* vs CS*_GPIO bit. They're both CS5535 in Kconfig.

Yes, sorry for typo.

Takashi

Takashi Iwai

2009-08-20 19:37 UTC #7 lore raw

In reply to Andres Salomon

At Thu, 20 Aug 2009 12:40:02 -0400,

Quoted 13 lines · Andres Salomon

Quoted 9 lines · On Thu, 20 Aug 2009 17:21:20 +0200 Takashi Iwai <[email protected]> wrote:
At Tue, 18 Aug 2009 17:53:14 -0400,
Quoted 6 lines · Andres Salomon
This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).

Quoted 6 lines

Quoted 3 lines
Will be any user of cs5535_gpio_*() expected? If not, it'd be better not to export stuff, IMO.
The olpc-dcon driver, for one. If desired, I can remove the exports for now.

Ah, OK. If this is going to be merged soonish, it should be fine to have the exports from the beginning.

Quoted 14 lines

Quoted 9 lines
Quoted 5 lines
Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.
.... or you can rewrite the old driver to use the functions above? Then the ugly Kconfig check can be dropped, too. (yeah that's an answer to my own question above.)
I guess in theory I could..
Quoted 1 line

Quoted 8 lines

Quoted 2 lines
BTW CONFIG_GPIO_CS5536 and CONFIG_CS5536_GPIO are so confusing... :)
Agreed. I'd really like CONFIG_CS5535_GPIO to go away, but first there needs to be a replacement. CONFIG_CS5535_GPIO will probably also need to be marked DEPRECATED for a while, too, since it's a userspace-visible change. I can follow up w/ a patch for that once people are happy w/ this current set of patches.

Agreed. Unless the compatible feature is given, some deprecation period would be safer.

thanks,

Takashi

Alessandro Zummo

Reviewed-by on v1 1/3

2009-08-23 09:22 UTC #8 lore raw

In reply to Andres Salomon

Quoted 6 lines · On Tue, 18 Aug 2009 17:53:14 -0400 Andres Salomon <[email protected]>…

This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).

Quoted 6 lines

Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.
Signed-off-by Andres Salomon <[email protected]>

fwiw,

Reviewed-by Alessandro Zummo <[email protected]>

drivers/gpio/cs5535-gpio.c line 194

Quoted 2 lines

drivers/gpio/cs5535-gpio.c new file +2 -0

+	dev_info(&pdev->dev, "allocated PCI BAR #%d: base 0x%llx\n", GPIO_BAR,
+			(unsigned long long) cs5535_gpio_chip.base);

shouldn't this be dev_dbg ?

drivers/gpio/cs5535-gpio.c line 199

Quoted 5 lines

drivers/gpio/cs5535-gpio.c new file +4 -0

+	/* finally, register with the generic GPIO API */
+	err = gpiochip_add(&cs5535_gpio_chip.chip);
+	if (err) {
+		dev_err(&pdev->dev, "failed to register gpio chip\n");

gpiochip_add already uses pr_err for error conditions, there's no need to report twice.

drivers/gpio/cs5535-gpio.c line 203

Quoted 4 lines

drivers/gpio/cs5535-gpio.c new file +4 -0

+		goto release_region;
+	}
+
+	printk(KERN_INFO DRV_NAME ": GPIO support successfully loaded.\n");

I'd use dev_xxx here, maybe it's worth to have a generic one in gpiochip_add .

Signature · 6 lines

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Andres Salomon

author

2009-08-28 19:52 UTC #9 lore raw

Quoted 15 lines · On Sun, 23 Aug 2009 11:22:14 +0200 Alessandro Zummo…

Quoted 12 lines · On Tue, 18 Aug 2009 17:53:14 -0400 Andres Salomon <[email protected]>…
This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend (allowing GPIO users to use the generic GPIO API if desired) while also allowing architecture-specific users directly (via the cs5535_gpio_* functions).
Tested on an OLPC machine. Some Leemotes also use CS5536 (with a mips cpu), which is why this is in drivers/gpio rather than arch/x86. Currently, it conflicts with older geode GPIO support; once MFGPT support is reworked to also be more generic, the older geode code will be removed.

Quoted 6 lines

Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
fwiw,
Reviewed-by Alessandro Zummo <[email protected]>

Thanks!

Quoted 7 lines

Quoted 4 lines
(quoted diff) +1 -0
+	dev_info(&pdev->dev, "allocated PCI BAR #%d: base
0x%llx\n", GPIO_BAR,
(quoted diff) +1 -0
+			(unsigned long long)
cs5535_gpio_chip.base);
shouldn't this be dev_dbg ?

I prefer to see it when the driver is loaded (as info).

Quoted 10 lines

Quoted 6 lines
(quoted diff) +4 -0
+	/* finally, register with the generic GPIO API */
+	err = gpiochip_add(&cs5535_gpio_chip.chip);
+	if (err) {
+		dev_err(&pdev->dev, "failed to register gpio
chip\n");
gpiochip_add already uses pr_err for error conditions, there's no
need to report twice.

Hm, you're right. Andrew, can you please add/merge the patch below?

Quoted 10 lines

Quoted 5 lines
(quoted diff) +4 -0
+		goto release_region;
+	}
+
+	printk(KERN_INFO DRV_NAME ": GPIO support successfully
loaded.\n");
I'd use dev_xxx here, maybe it's worth to have a generic one in gpiochip_add .

Noted and changed (along w/ a few other printks). Thanks!

From: Andres Salomon <[email protected]>
Subject: [PATCH] cs5535-gpio: make printk usage consistent

Clean up places where we were using printk()s instead of dev_*.

Signed-off-by Andres Salomon <[email protected]>

---
 drivers/gpio/cs5535-gpio.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

drivers/gpio/cs5535-gpio.c +5 -8

index ec1f0e8..0fdbe94 100644
@@ -134,8 +134,8 @@ static int chip_gpio_request(struct gpio_chip *c, unsigned offset)
 
 	/* check if this pin is available */
 	if ((mask & (1 << offset)) == 0) {
-		printk(KERN_INFO DRV_NAME
-			": pin %u is not available (check mask)\n", offset);
+		dev_info(&chip->pdev->dev,
+			"pin %u is not available (check mask)\n", offset);
 		spin_unlock_irqrestore(&chip->lock, flags);
 		return -EINVAL;
 	}

@@ -265,18 +265,15 @@ static int __init cs5535_gpio_probe(struct pci_dev *pdev,
 	mask &= ~(1 << 28);
 
 	if (mask_orig != mask)
-		printk(KERN_INFO DRV_NAME
-				": mask changed from 0x%08lX to 0x%08lX\n",
+		dev_info(&pdev->dev, "mask changed from 0x%08lX to 0x%08lX\n",
 				mask_orig, mask);
 
 	/* finally, register with the generic GPIO API */
 	err = gpiochip_add(&cs5535_gpio_chip.chip);
-	if (err) {
-		dev_err(&pdev->dev, "failed to register gpio chip\n");
+	if (err)
 		goto release_region;
-	}
 
-	printk(KERN_INFO DRV_NAME ": GPIO support successfully loaded.\n");
+	dev_info(&pdev->dev, DRV_NAME ": GPIO support successfully loaded.\n");
 	return 0;
 
 release_region:

Signature · 1 line

1.5.6.5