[PATCH] leds: New PCEngines Alix LED driver using gpio interface

[PATCH] leds: New PCEngines Alix LED driver using gpio interface

[kernel]

From: Ed Wildgoose <kernel@wildgooses.com>

This new driver replaces the old PCEngines Alix 2/3 LED driver with
a new driver that controls the LEDs through the leds-gpio driver.
The old driver accessed GPIOs directly, which created a conflict 
and prevented also loading the cs5535-gio driver to read other
GPIOs on the Alix board. With this new driver, both are loaded
and therefore it's possible to access both the LEDs and onboard GPIOs 

This driver is based on leds-net5501.c 
by: Alessandro Zummo <a.zummo@towertech.it>
Additionally it relies on parts of the patch: 7f131cf3ed
by: Daniel Mack <daniel@caiaq.de> to perform detection of the Alix board

Signed-off-by: Ed Wildgoose <kernel@wildgooses.com>
---
:100644 100644 6f190f4... 25f0285... M	drivers/leds/Kconfig
:100644 100644 f59ffad... 62c8fff... M	drivers/leds/leds-alix2.c
 drivers/leds/Kconfig      |    2 +-
 drivers/leds/leds-alix2.c |  196 ++++++++++----------------------------------
 2 files changed, 46 insertions(+), 152 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6f190f4..25f0285 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -100,7 +100,7 @@ config LEDS_WRAP
 config LEDS_ALIX2
 	tristate "LED Support for ALIX.2 and ALIX.3 series"
 	depends on LEDS_CLASS
-	depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
+	depends on X86 && LEDS_GPIO_PLATFORM && GPIO_CS5535
 	help
 	  This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
 	  You have to set leds-alix2.force=1 for boards with Award BIOS.
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
index f59ffad..62c8fff 100644
--- a/drivers/leds/leds-alix2.c
+++ b/drivers/leds/leds-alix2.c
@@ -1,119 +1,66 @@
 /*
  * LEDs driver for PCEngines ALIX.2 and ALIX.3
  *
- * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
+ * Copyright (C) 2011 Nippy Networks Ltd
+ * Written by Ed Wildgoose <kernel@wildgooses.com>
+ * Based on leds-net5501.c by Alessandro Zummo <a.zummo@towertech.it>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
  */
 
-#include <linux/err.h>
-#include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/string.h>
 #include <linux/leds.h>
-#include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/string.h>
-#include <linux/pci.h>
+#include <linux/gpio.h>
+
+#include <asm/geode.h>
 
 static int force = 0;
 module_param(force, bool, 0444);
 MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style LEDs");
 
-#define MSR_LBAR_GPIO		0x5140000C
-#define CS5535_GPIO_SIZE	256
-
-static u32 gpio_base;
-
-static struct pci_device_id divil_pci[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_NS,  PCI_DEVICE_ID_NS_CS5535_ISA) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
-	{ } /* NULL entry */
-};
-MODULE_DEVICE_TABLE(pci, divil_pci);
-
-struct alix_led {
-	struct led_classdev cdev;
-	unsigned short port;
-	unsigned int on_value;
-	unsigned int off_value;
-};
-
-static void alix_led_set(struct led_classdev *led_cdev,
-			 enum led_brightness brightness)
-{
-	struct alix_led *led_dev =
-		container_of(led_cdev, struct alix_led, cdev);
-
-	if (brightness)
-		outl(led_dev->on_value, gpio_base + led_dev->port);
-	else
-		outl(led_dev->off_value, gpio_base + led_dev->port);
-}
-
-static struct alix_led alix_leds[] = {
+static struct gpio_led alix_leds[] = {
 	{
-		.cdev = {
-			.name = "alix:1",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x00,
-		.on_value = 1 << 22,
-		.off_value = 1 << 6,
+		.name = "alix:1",
+		.gpio = 6,
+		.default_trigger = "default-on",
+		.active_low = 1,
 	},
 	{
-		.cdev = {
-			.name = "alix:2",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x80,
-		.on_value = 1 << 25,
-		.off_value = 1 << 9,
+		.name = "alix:2",
+		.gpio = 25,
+		.default_trigger = "default-off",
+		.active_low = 1,
 	},
 	{
-		.cdev = {
-			.name = "alix:3",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x80,
-		.on_value = 1 << 27,
-		.off_value = 1 << 11,
+		.name = "alix:3",
+		.gpio = 27,
+		.default_trigger = "default-off",
+		.active_low = 1,
 	},
 };
 
-static int __init alix_led_probe(struct platform_device *pdev)
-{
-	int i;
-	int ret;
-
-	for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
-		alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
-		ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev);
-		if (ret < 0)
-			goto fail;
-	}
-	return 0;
+static struct gpio_led_platform_data alix_leds_data = {
+	.num_leds = ARRAY_SIZE(alix_leds),
+	.leds = alix_leds,
+};
 
-fail:
-	while (--i >= 0)
-		led_classdev_unregister(&alix_leds[i].cdev);
-	return ret;
-}
+static struct platform_device alix_leds_dev = {
+	.name = "leds-gpio",
+	.id = -1,
+	.dev.platform_data = &alix_leds_data,
+};
 
-static int alix_led_remove(struct platform_device *pdev)
+static void __init register_alix(void)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
-		led_classdev_unregister(&alix_leds[i].cdev);
-	return 0;
+	platform_device_register(&alix_leds_dev);
 }
 
-static struct platform_driver alix_led_driver = {
-	.remove = alix_led_remove,
-	.driver = {
-		.name = KBUILD_MODNAME,
-		.owner = THIS_MODULE,
-	},
-};
-
 static int __init alix_present(unsigned long bios_phys,
 				const char *alix_sig,
 				size_t alix_sig_len)
@@ -164,76 +111,23 @@ static int __init alix_present(unsigned long bios_phys,
 	return 0;
 }
 
-static struct platform_device *pdev;
-
-static int __init alix_pci_led_init(void)
-{
-	u32 low, hi;
-
-	if (pci_dev_present(divil_pci) == 0) {
-		printk(KERN_WARNING KBUILD_MODNAME": DIVIL not found\n");
-		return -ENODEV;
-	}
-
-	/* Grab the GPIO I/O range */
-	rdmsr(MSR_LBAR_GPIO, low, hi);
-
-	/* Check the mask and whether GPIO is enabled (sanity check) */
-	if (hi != 0x0000f001) {
-		printk(KERN_WARNING KBUILD_MODNAME": GPIO not enabled\n");
-		return -ENODEV;
-	}
-
-	/* Mask off the IO base address */
-	gpio_base = low & 0x0000ff00;
-
-	if (!request_region(gpio_base, CS5535_GPIO_SIZE, KBUILD_MODNAME)) {
-		printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O for GPIO\n");
-		return -ENODEV;
-	}
-
-	/* Set GPIO function to output */
-	outl(1 << 6, gpio_base + 0x04);
-	outl(1 << 9, gpio_base + 0x84);
-	outl(1 << 11, gpio_base + 0x84);
-
-	return 0;
-}
-
-static int __init alix_led_init(void)
+static int __init alix_init(void)
 {
-	int ret = -ENODEV;
 	const char tinybios_sig[] = "PC Engines ALIX.";
 	const char coreboot_sig[] = "PC Engines\0ALIX.";
 
+	if (!is_geode())
+		return 0;
+
 	if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
 	    alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
-		ret = alix_pci_led_init();
+		register_alix();
 
-	if (ret < 0)
-		return ret;
-
-	pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
-	if (!IS_ERR(pdev)) {
-		ret = platform_driver_probe(&alix_led_driver, alix_led_probe);
-		if (ret)
-			platform_device_unregister(pdev);
-	} else
-		ret = PTR_ERR(pdev);
-
-	return ret;
-}
-
-static void __exit alix_led_exit(void)
-{
-	platform_device_unregister(pdev);
-	platform_driver_unregister(&alix_led_driver);
-	release_region(gpio_base, CS5535_GPIO_SIZE);
+	return 0;
 }
 
-module_init(alix_led_init);
-module_exit(alix_led_exit);
+arch_initcall(alix_init);
 
-MODULE_AUTHOR("Constantin Baranov <const@mimas.ru>");
+MODULE_AUTHOR("Ed Wildgoose <kernel@wildgooses.com>");
 MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
 MODULE_LICENSE("GPL");
-- 
1.7.4

[PATCH] leds: New PCEngines Alix LED driver using gpio interface

[kernel]

From: Ed Wildgoose <git@wildgooses.com>

This new driver replaces the old PCEngines Alix 2/3 LED driver with
a new driver that controls the LEDs through the leds-gpio driver.
The old driver accessed GPIOs directly, which created a conflict
and prevented also loading the cs5535-gpio driver to read other
GPIOs on the Alix board. With this new driver, we hook into leds-gpio
which in turn uses GPIO to control the LEDs and therefore it's
possible to control both the LEDs and access onboard GPIOs

Driver is moved to platform/geode and any other geode
initialisation modules should move here also.

This driver is inspired by leds-net5501.c
by: Alessandro Zummo <a.zummo@towertech.it>
Ideally, leds-net5501.c should also be moved to platform/geode.
Additionally the driver relies on parts of the patch: 7f131cf3ed
by: Daniel Mack <daniel@caiaq.de> to perform detection of the Alix board
Signed-off-by: Ed Wildgoose <git@wildgooses.com>
---
:100644 100644 d5ed94d... b16ab56... M	arch/x86/Kconfig
:100644 100644 021eee9... 8d87439... M	arch/x86/platform/Makefile
:000000 100644 0000000... d4e599f... A	arch/x86/platform/geode/Makefile
:000000 100644 0000000... bc1b3b3... A	arch/x86/platform/geode/alix_leds.c
:100644 100644 6f190f4... e4573d6... M	drivers/leds/Kconfig
:100644 100644 aae6989... bf5a00c... M	drivers/leds/Makefile
:100644 000000 f59ffad... 0000000... D	drivers/leds/leds-alix2.c
 arch/x86/Kconfig                    |    9 ++
 arch/x86/platform/Makefile          |    1 +
 arch/x86/platform/geode/Makefile    |    1 +
 arch/x86/platform/geode/alix_leds.c |  138 ++++++++++++++++++++
 drivers/leds/Kconfig                |    8 --
 drivers/leds/Makefile               |    1 -
 drivers/leds/leds-alix2.c           |  239 -----------------------------------
 7 files changed, 149 insertions(+), 248 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d5ed94d..b16ab56 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
 	default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
 	select OF_PROMTREE
 
+config ALIX_LEDS
+	bool "PCEngines ALIX.2/.3 LED Support"
+	select GPIOLIB
+	depends on LEDS_CLASS
+	depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
+	---help---
+	  This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
+	  You have to set alix-leds.force=1 for boards with Award BIOS.
+
 endif # X86_32
 
 config AMD_NB
diff --git a/arch/x86/platform/Makefile b/arch/x86/platform/Makefile
index 021eee9..8d87439 100644
--- a/arch/x86/platform/Makefile
+++ b/arch/x86/platform/Makefile
@@ -1,6 +1,7 @@
 # Platform specific code goes here
 obj-y	+= ce4100/
 obj-y	+= efi/
+obj-y	+= geode/
 obj-y	+= iris/
 obj-y	+= mrst/
 obj-y	+= olpc/
diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
new file mode 100644
index 0000000..d4e599f
--- /dev/null
+++ b/arch/x86/platform/geode/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ALIX_LEDS)		+= alix-leds.o
diff --git a/arch/x86/platform/geode/alix_leds.c b/arch/x86/platform/geode/alix_leds.c
new file mode 100644
index 0000000..bc1b3b3
--- /dev/null
+++ b/arch/x86/platform/geode/alix_leds.c
@@ -0,0 +1,138 @@
+/*
+ * Configure GPIO control of LEDs on Alix.2/3 from PCEngines
+ *
+ * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
+ * Copyright (C) 2011 Nippy Networks Ltd
+ * 
+ * TODO: There are large similarities with leds-net5501.c 
+ * by Alessandro Zummo <a.zummo@towertech.it>
+ * In the future leds-net5501.c should be migrated over to platform
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+
+#include <asm/geode.h>
+
+static int force = 0;
+module_param(force, bool, 0444);
+/* FIXME: Award bios is not automatically detected as Alix platform */
+MODULE_PARM_DESC(force, "Force detection as ALIX.2/ALIX.3 platform");
+
+static struct gpio_led alix_leds[] = {
+	{
+		.name = "alix:1",
+		.gpio = 6,
+		.default_trigger = "default-on",
+		.active_low = 1,
+	},
+	{
+		.name = "alix:2",
+		.gpio = 25,
+		.default_trigger = "default-off",
+		.active_low = 1,
+	},
+	{
+		.name = "alix:3",
+		.gpio = 27,
+		.default_trigger = "default-off",
+		.active_low = 1,
+	},
+};
+
+static struct gpio_led_platform_data alix_leds_data = {
+	.num_leds = ARRAY_SIZE(alix_leds),
+	.leds = alix_leds,
+};
+
+static struct platform_device alix_leds_dev = {
+	.name = "leds-gpio",
+	.id = -1,
+	.dev.platform_data = &alix_leds_data,
+};
+
+static void __init register_alix(void)
+{
+	/* Setup LED control through leds-gpio driver */
+	platform_device_register(&alix_leds_dev);
+}
+
+static int __init alix_present(unsigned long bios_phys,
+				const char *alix_sig,
+				size_t alix_sig_len)
+{
+	const size_t bios_len = 0x00010000;
+	const char *bios_virt;
+	const char *scan_end;
+	const char *p;
+	char name[64];
+
+	if (force) {
+		printk(KERN_NOTICE "%s: forced to skip BIOS test, "
+		       "assume system is ALIX.2/ALIX.3\n",
+		       KBUILD_MODNAME);
+		return 1;
+	}
+
+	bios_virt = phys_to_virt(bios_phys);
+	scan_end = bios_virt + bios_len - (alix_sig_len + 2);
+	for (p = bios_virt; p < scan_end; p++) {
+		const char *tail;
+		char *a;
+
+		if (memcmp(p, alix_sig, alix_sig_len) != 0)
+			continue;
+
+		memcpy(name, p, sizeof(name));
+
+		/* remove the first \0 character from string */
+		a = strchr(name, '\0');
+		if (a)
+			*a = ' ';
+
+		/* cut the string at a newline */
+		a = strchr(name, '\r');
+		if (a)
+			*a = '\0';
+
+		tail = p + alix_sig_len;
+		if ((tail[0] == '2' || tail[0] == '3')) {
+			printk(KERN_INFO
+			       "%s: system is recognized as \"%s\"\n",
+			       KBUILD_MODNAME, name);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static int __init alix_init(void)
+{
+	const char tinybios_sig[] = "PC Engines ALIX.";
+	const char coreboot_sig[] = "PC Engines\0ALIX.";
+
+	if (!is_geode())
+		return 0;
+
+	if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
+	    alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
+		register_alix();
+
+	return 0;
+}
+
+module_init(alix_init);
+
+MODULE_AUTHOR("Ed Wildgoose <kernel@wildgooses.com>");
+MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 System Setup");
+MODULE_LICENSE("GPL");
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6f190f4..e4573d6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -97,14 +97,6 @@ config LEDS_WRAP
 	help
 	  This option enables support for the PCEngines WRAP programmable LEDs.
 
-config LEDS_ALIX2
-	tristate "LED Support for ALIX.2 and ALIX.3 series"
-	depends on LEDS_CLASS
-	depends on X86 && !GPIO_CS5535 && !CS5535_GPIO
-	help
-	  This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
-	  You have to set leds-alix2.force=1 for boards with Award BIOS.
-
 config LEDS_H1940
 	tristate "LED Support for iPAQ H1940 device"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index aae6989..bf5a00c 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_LEDS_AMS_DELTA)		+= leds-ams-delta.o
 obj-$(CONFIG_LEDS_NET48XX)		+= leds-net48xx.o
 obj-$(CONFIG_LEDS_NET5501)		+= leds-net5501.o
 obj-$(CONFIG_LEDS_WRAP)			+= leds-wrap.o
-obj-$(CONFIG_LEDS_ALIX2)		+= leds-alix2.o
 obj-$(CONFIG_LEDS_H1940)		+= leds-h1940.o
 obj-$(CONFIG_LEDS_COBALT_QUBE)		+= leds-cobalt-qube.o
 obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
deleted file mode 100644
index f59ffad..0000000
--- a/drivers/leds/leds-alix2.c
+++ /dev/null
@@ -1,239 +0,0 @@
-/*
- * LEDs driver for PCEngines ALIX.2 and ALIX.3
- *
- * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/leds.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/string.h>
-#include <linux/pci.h>
-
-static int force = 0;
-module_param(force, bool, 0444);
-MODULE_PARM_DESC(force, "Assume system has ALIX.2/ALIX.3 style LEDs");
-
-#define MSR_LBAR_GPIO		0x5140000C
-#define CS5535_GPIO_SIZE	256
-
-static u32 gpio_base;
-
-static struct pci_device_id divil_pci[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_NS,  PCI_DEVICE_ID_NS_CS5535_ISA) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
-	{ } /* NULL entry */
-};
-MODULE_DEVICE_TABLE(pci, divil_pci);
-
-struct alix_led {
-	struct led_classdev cdev;
-	unsigned short port;
-	unsigned int on_value;
-	unsigned int off_value;
-};
-
-static void alix_led_set(struct led_classdev *led_cdev,
-			 enum led_brightness brightness)
-{
-	struct alix_led *led_dev =
-		container_of(led_cdev, struct alix_led, cdev);
-
-	if (brightness)
-		outl(led_dev->on_value, gpio_base + led_dev->port);
-	else
-		outl(led_dev->off_value, gpio_base + led_dev->port);
-}
-
-static struct alix_led alix_leds[] = {
-	{
-		.cdev = {
-			.name = "alix:1",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x00,
-		.on_value = 1 << 22,
-		.off_value = 1 << 6,
-	},
-	{
-		.cdev = {
-			.name = "alix:2",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x80,
-		.on_value = 1 << 25,
-		.off_value = 1 << 9,
-	},
-	{
-		.cdev = {
-			.name = "alix:3",
-			.brightness_set = alix_led_set,
-		},
-		.port = 0x80,
-		.on_value = 1 << 27,
-		.off_value = 1 << 11,
-	},
-};
-
-static int __init alix_led_probe(struct platform_device *pdev)
-{
-	int i;
-	int ret;
-
-	for (i = 0; i < ARRAY_SIZE(alix_leds); i++) {
-		alix_leds[i].cdev.flags |= LED_CORE_SUSPENDRESUME;
-		ret = led_classdev_register(&pdev->dev, &alix_leds[i].cdev);
-		if (ret < 0)
-			goto fail;
-	}
-	return 0;
-
-fail:
-	while (--i >= 0)
-		led_classdev_unregister(&alix_leds[i].cdev);
-	return ret;
-}
-
-static int alix_led_remove(struct platform_device *pdev)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
-		led_classdev_unregister(&alix_leds[i].cdev);
-	return 0;
-}
-
-static struct platform_driver alix_led_driver = {
-	.remove = alix_led_remove,
-	.driver = {
-		.name = KBUILD_MODNAME,
-		.owner = THIS_MODULE,
-	},
-};
-
-static int __init alix_present(unsigned long bios_phys,
-				const char *alix_sig,
-				size_t alix_sig_len)
-{
-	const size_t bios_len = 0x00010000;
-	const char *bios_virt;
-	const char *scan_end;
-	const char *p;
-	char name[64];
-
-	if (force) {
-		printk(KERN_NOTICE "%s: forced to skip BIOS test, "
-		       "assume system has ALIX.2 style LEDs\n",
-		       KBUILD_MODNAME);
-		return 1;
-	}
-
-	bios_virt = phys_to_virt(bios_phys);
-	scan_end = bios_virt + bios_len - (alix_sig_len + 2);
-	for (p = bios_virt; p < scan_end; p++) {
-		const char *tail;
-		char *a;
-
-		if (memcmp(p, alix_sig, alix_sig_len) != 0)
-			continue;
-
-		memcpy(name, p, sizeof(name));
-
-		/* remove the first \0 character from string */
-		a = strchr(name, '\0');
-		if (a)
-			*a = ' ';
-
-		/* cut the string at a newline */
-		a = strchr(name, '\r');
-		if (a)
-			*a = '\0';
-
-		tail = p + alix_sig_len;
-		if ((tail[0] == '2' || tail[0] == '3')) {
-			printk(KERN_INFO
-			       "%s: system is recognized as \"%s\"\n",
-			       KBUILD_MODNAME, name);
-			return 1;
-		}
-	}
-
-	return 0;
-}
-
-static struct platform_device *pdev;
-
-static int __init alix_pci_led_init(void)
-{
-	u32 low, hi;
-
-	if (pci_dev_present(divil_pci) == 0) {
-		printk(KERN_WARNING KBUILD_MODNAME": DIVIL not found\n");
-		return -ENODEV;
-	}
-
-	/* Grab the GPIO I/O range */
-	rdmsr(MSR_LBAR_GPIO, low, hi);
-
-	/* Check the mask and whether GPIO is enabled (sanity check) */
-	if (hi != 0x0000f001) {
-		printk(KERN_WARNING KBUILD_MODNAME": GPIO not enabled\n");
-		return -ENODEV;
-	}
-
-	/* Mask off the IO base address */
-	gpio_base = low & 0x0000ff00;
-
-	if (!request_region(gpio_base, CS5535_GPIO_SIZE, KBUILD_MODNAME)) {
-		printk(KERN_ERR KBUILD_MODNAME": can't allocate I/O for GPIO\n");
-		return -ENODEV;
-	}
-
-	/* Set GPIO function to output */
-	outl(1 << 6, gpio_base + 0x04);
-	outl(1 << 9, gpio_base + 0x84);
-	outl(1 << 11, gpio_base + 0x84);
-
-	return 0;
-}
-
-static int __init alix_led_init(void)
-{
-	int ret = -ENODEV;
-	const char tinybios_sig[] = "PC Engines ALIX.";
-	const char coreboot_sig[] = "PC Engines\0ALIX.";
-
-	if (alix_present(0xf0000, tinybios_sig, sizeof(tinybios_sig) - 1) ||
-	    alix_present(0x500, coreboot_sig, sizeof(coreboot_sig) - 1))
-		ret = alix_pci_led_init();
-
-	if (ret < 0)
-		return ret;
-
-	pdev = platform_device_register_simple(KBUILD_MODNAME, -1, NULL, 0);
-	if (!IS_ERR(pdev)) {
-		ret = platform_driver_probe(&alix_led_driver, alix_led_probe);
-		if (ret)
-			platform_device_unregister(pdev);
-	} else
-		ret = PTR_ERR(pdev);
-
-	return ret;
-}
-
-static void __exit alix_led_exit(void)
-{
-	platform_device_unregister(pdev);
-	platform_driver_unregister(&alix_led_driver);
-	release_region(gpio_base, CS5535_GPIO_SIZE);
-}
-
-module_init(alix_led_init);
-module_exit(alix_led_exit);
-
-MODULE_AUTHOR("Constantin Baranov <const@mimas.ru>");
-MODULE_DESCRIPTION("PCEngines ALIX.2 and ALIX.3 LED driver");
-MODULE_LICENSE("GPL");
-- 
1.7.4

Re: [PATCH] leds: New PCEngines Alix LED driver using gpio interface

[Ed W]

Hi, Thanks for "mentoring" me over this.  I appreciate that this
consumes your valuable time

I have reposted what I hope is close to the correct code for this new
driver.  I'm somewhat apprehensive, so to draw attention to the things I
have most likely "done wrong":


> :000000 100644 0000000... bc1b3b3... A	arch/x86/platform/geode/alix_leds.c

...

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d5ed94d..b16ab56 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
>  	default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
>  	select OF_PROMTREE
>  
> +config ALIX_LEDS
> +	bool "PCEngines ALIX.2/.3 LED Support"
> +	select GPIOLIB
> +	depends on LEDS_CLASS
> +	depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
> +	---help---
> +	  This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
> +	  You have to set alix-leds.force=1 for boards with Award BIOS.
> +
>  endif # X86_32


Driver and KConfig show this being specifically a driver for the Alix
LEDs rather than being a general Alix plaform initialisation module?

I was unsure if the trend was to have one module which initialised all
Alix platform stuff (whatever it needs), or to split by function?
Looking at other platform modules they seem to be somewhat fine grained
so I went with a specific "ALIX Led Module" approach?


Additionally I am unsure how strictly to set dependencies for my module?
 It clearly requires all the LED_GPIO_PLATFORM, GPIO_CS5535 dependencies
to do anything, but equally it doesn't seem to *break* anything if those
dependencies aren't compiled in?  Listing all the dependencies seems to
make it hard for users to find the option to enable it since it's not
even listed in menuconfig until your deps are met?  Please correct me as
to what level of deps should be listed?


On the topic of dependencies, Andres has changed cs5535-gpio.c to depend
on a new module cs5535-mfd - however, apart from the commit log message
this is not obvious to see?  OK, I'm an idiot, but it took me most of
this afternoon to understand why my GPIOs stopped working after moving
to 2.6.38?  Q: Should the new -mfd module be listed as a dep of -gpio?
Or perhaps -gpio should "select" -mfd? At least it would be helpful to
list the dependency in the Kconfig message?

Happy to submit a change - please advise on what (if anything) would be
the best solution?


Many thanks for your help and guidance so far

Ed W

Re: [PATCH] leds: New PCEngines Alix LED driver using gpio interface

[Grant Likely]

On Fri, Mar 18, 2011 at 12:32 PM, Ed W <lists@wildgooses.com> wrote:
> Hi, Thanks for "mentoring" me over this.  I appreciate that this
> consumes your valuable time
>
> I have reposted what I hope is close to the correct code for this new
> driver.  I'm somewhat apprehensive, so to draw attention to the things I
> have most likely "done wrong":
>
>
>> :000000 100644 0000000... bc1b3b3... A        arch/x86/platform/geode/alix_leds.c
>
> ...
>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index d5ed94d..b16ab56 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2094,6 +2094,15 @@ config OLPC_OPENFIRMWARE_DT
>>       default y if OLPC_OPENFIRMWARE && PROC_DEVICETREE
>>       select OF_PROMTREE
>>
>> +config ALIX_LEDS
>> +     bool "PCEngines ALIX.2/.3 LED Support"
>> +     select GPIOLIB
>> +     depends on LEDS_CLASS
>> +     depends on LEDS_GPIO_PLATFORM && GPIO_CS5535
>> +     ---help---
>> +       This option enables support for the PCEngines ALIX.2 and ALIX.3 LEDs.
>> +       You have to set alix-leds.force=1 for boards with Award BIOS.
>> +
>>  endif # X86_32
>
>
> Driver and KConfig show this being specifically a driver for the Alix
> LEDs rather than being a general Alix plaform initialisation module?
>
> I was unsure if the trend was to have one module which initialised all
> Alix platform stuff (whatever it needs), or to split by function?
> Looking at other platform modules they seem to be somewhat fine grained
> so I went with a specific "ALIX Led Module" approach?

That's very much up to the board maintainer.  Because it is just
device registrations, I would lump them all into one file.  If they
were actual drivers then I tend to split them up.

> Additionally I am unsure how strictly to set dependencies for my module?
>  It clearly requires all the LED_GPIO_PLATFORM, GPIO_CS5535 dependencies
> to do anything, but equally it doesn't seem to *break* anything if those
> dependencies aren't compiled in?  Listing all the dependencies seems to
> make it hard for users to find the option to enable it since it's not
> even listed in menuconfig until your deps are met?  Please correct me as
> to what level of deps should be listed?

Correct, you don't need to depend on the driver because this is just a
device registration.  Who cares if it never gets bound to the driver?
The device registration will happily sit in the device model
unregistered.

> On the topic of dependencies, Andres has changed cs5535-gpio.c to depend
> on a new module cs5535-mfd - however, apart from the commit log message
> this is not obvious to see?  OK, I'm an idiot, but it took me most of
> this afternoon to understand why my GPIOs stopped working after moving
> to 2.6.38?  Q: Should the new -mfd module be listed as a dep of -gpio?
> Or perhaps -gpio should "select" -mfd? At least it would be helpful to
> list the dependency in the Kconfig message?

select is a little touchy because selecting another symbol bypasses
and dependences the other symbol has.  In this case, it could skip
CONFIG_MFD_CORE which may be bad.  -gpio should be a dependancy of
-mfd in this case I think.

g.

Re: [PATCH] leds: New PCEngines Alix LED driver using gpio interface

[Andrew Morton]

On Sun,  6 Mar 2011 16:51:25 +0000
kernel@wildgooses.com wrote:

> From: Ed Wildgoose <kernel@wildgooses.com>
> 
> This new driver replaces the old PCEngines Alix 2/3 LED driver with
> a new driver that controls the LEDs through the leds-gpio driver.
> The old driver accessed GPIOs directly, which created a conflict 
> and prevented also loading the cs5535-gio driver to read other
> GPIOs on the Alix board. With this new driver, both are loaded
> and therefore it's possible to access both the LEDs and onboard GPIOs 
> 
> This driver is based on leds-net5501.c 
> by: Alessandro Zummo <a.zummo@towertech.it>
> Additionally it relies on parts of the patch: 7f131cf3ed
> by: Daniel Mack <daniel@caiaq.de> to perform detection of the Alix board

As far as I can tell from looking at it, this driver maintains all the
same interfaces and behaviour as the old version.  Am I right?

Re: [PATCH] leds: New PCEngines Alix LED driver using gpio interface

[Ed W]

On 07/04/2011 22:58, Andrew Morton wrote:
> On Sun,  6 Mar 2011 16:51:25 +0000
> kernel@wildgooses.com wrote:
>
>> From: Ed Wildgoose <kernel@wildgooses.com>
>>
>> This new driver replaces the old PCEngines Alix 2/3 LED driver with
>> a new driver that controls the LEDs through the leds-gpio driver.
>> The old driver accessed GPIOs directly, which created a conflict 
>> and prevented also loading the cs5535-gio driver to read other
>> GPIOs on the Alix board. With this new driver, both are loaded
>> and therefore it's possible to access both the LEDs and onboard GPIOs 
>>
>> This driver is based on leds-net5501.c 
>> by: Alessandro Zummo <a.zummo@towertech.it>
>> Additionally it relies on parts of the patch: 7f131cf3ed
>> by: Daniel Mack <daniel@caiaq.de> to perform detection of the Alix board
> As far as I can tell from looking at it, this driver maintains all the
> same interfaces and behaviour as the old version.  Am I right?
>

Yes, this is correct.  The substance of the change is simply to use the
leds-gpio interface rather than fiddling with gpio settings directly. 
However, please note that Grant and Andres suggested that the driver
should instead move to "arch" and out of leds? Therefore, I created an
updated version of this patch which you can find from message id:
"1300553466-23873-1-git-send-email-kernel@wildgooses.com", ie:
    http://lists.zerezo.com/linux-kernel/msg27455553.html

Additionally this updated patch has a slightly amended copyright -
several folks thought that the updated copyright statement was more
appropriate

My apologies that this updated patch was not more clearly marked as such
- Grant has taken the time to point out how I should submit patches more
clearly in the future

I would be happy to shuffle the code around further if anyone has an
opinion?

Thanks for picking this up

Ed W