linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer
@ 2011-12-31  9:28 Philip Prindeville
  2012-01-27 22:59 ` Andrew Morton
  2012-01-28  5:02 ` [PATCH v2 " Philip Prindeville
  0 siblings, 2 replies; 5+ messages in thread
From: Philip Prindeville @ 2011-12-31  9:28 UTC (permalink / raw)
  To: linux-kernel; +Cc: Richard Purdie, Alessandro Zummo

From: "Philip A. Prindeville" <philipp@redfish-solutions.com>

Trivial platform driver for Soekris Engineering net5501 single-board
computer. Probes well-known locations in ROM for BIOS signature to
confirm correct platform. Registers 1 LED and 1 GPIO-based button
(typically used for soft reset).

Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Acked-by: Alessandro Zummo <a.zummo@towertech.it>
---
 arch/x86/Kconfig                  |    6 ++
 arch/x86/platform/geode/Makefile  |    1 +
 arch/x86/platform/geode/net5501.c |  147 +++++++++++++++++++++++++++++++++++++
 drivers/leds/leds-net5501.c       |   97 ------------------------
 4 files changed, 154 insertions(+), 97 deletions(-)
 create mode 100644 arch/x86/platform/geode/net5501.c
 delete mode 100644 drivers/leds/leds-net5501.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 90cab6f..b4d1dcf 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2164,6 +2164,12 @@ config ALIX
 
 	  Note: You have to set alix.force=1 for boards with Award BIOS.
 
+config NET5501
+	bool "Soekris Engineering net5501 System Support (LEDS, GPIO, etc)"
+	select GPIOLIB
+	---help---
+	  This option enables system support for the Soekris Engineering net5501.
+
 endif # X86_32
 
 config AMD_NB
diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
index 07c9cd0..246b788 100644
--- a/arch/x86/platform/geode/Makefile
+++ b/arch/x86/platform/geode/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ALIX)		+= alix.o
+obj-$(CONFIG_NET5501)		+= net5501.o
diff --git a/arch/x86/platform/geode/net5501.c b/arch/x86/platform/geode/net5501.c
new file mode 100644
index 0000000..40b4bd0
--- /dev/null
+++ b/arch/x86/platform/geode/net5501.c
@@ -0,0 +1,147 @@
+/*
+ * System Specific setup for Soekris net5501
+ * At the moment this means setup of GPIO control of LEDs and buttons
+ * on net5501 boards.
+ *
+ *
+ * Copyright (C) 2008-2009 Tower Technologies
+ * Written by Alessandro Zummo <a.zummo@towertech.it>
+ *
+ * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
+ * Copyright (C) 2011 Ed Wildgoose <kernel@wildgooses.com>
+ *                and Philip Prindeville <philipp@redfish-solutions.com>
+ *
+ * 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 <linux/input.h>
+#include <linux/gpio_keys.h>
+
+#include <asm/geode.h>
+
+#define BIOS_REGION_BASE		0xffff0000
+#define BIOS_REGION_SIZE		0x00010000
+
+static struct gpio_keys_button net5501_gpio_buttons[] = {
+	{
+		.code = KEY_RESTART,
+		.gpio = 24,
+		.active_low = 1,
+		.desc = "Reset button",
+		.type = EV_KEY,
+		.wakeup = 0,
+		.debounce_interval = 100,
+		.can_disable = 0,
+	}
+};
+static struct gpio_keys_platform_data net5501_buttons_data = {
+	.buttons = net5501_gpio_buttons,
+	.nbuttons = ARRAY_SIZE(net5501_gpio_buttons),
+	.poll_interval = 20,
+};
+
+static struct platform_device net5501_buttons_dev = {
+	.name = "gpio-keys-polled",
+	.id = 1,
+	.dev = {
+		.platform_data = &net5501_buttons_data,
+	}
+};
+
+static struct gpio_led net5501_leds[] = {
+	{
+		.name = "net5501:1",
+		.gpio = 6,
+		.default_trigger = "default-on",
+		.active_low = 1,
+	},
+};
+
+static struct gpio_led_platform_data net5501_leds_data = {
+	.num_leds = ARRAY_SIZE(net5501_leds),
+	.leds = net5501_leds,
+};
+
+static struct platform_device net5501_leds_dev = {
+	.name = "leds-gpio",
+	.id = -1,
+	.dev.platform_data = &net5501_leds_data,
+};
+
+static struct __initdata platform_device *net5501_devs[] = {
+	&net5501_buttons_dev,
+	&net5501_leds_dev,
+};
+
+static void __init register_net5501(void)
+{
+	/* Setup LED control through leds-gpio driver */
+	platform_add_devices(net5501_devs, ARRAY_SIZE(net5501_devs));
+}
+
+struct net5501_board {
+	u16	offset;
+	u16	len;
+	char	*sig;
+};
+
+static struct net5501_board __initdata boards[] = {
+	{ 0xb7b, 7, "net5501" },	/* net5501 v1.33/1.33c */
+	{ 0xb1f, 7, "net5501" },	/* net5501 v1.32i */
+};
+
+static int __init net5501_present(void)
+{
+	int i;
+	unsigned char *rombase, *bios;
+
+	rombase = ioremap(BIOS_REGION_BASE, BIOS_REGION_SIZE - 1);
+	if (!rombase)
+		printk(KERN_INFO "Soekris net5501 LED driver failed to get rombase");
+
+	bios = rombase + 0x20;	/* null terminated */
+
+	if (memcmp(bios, "comBIOS", 7))
+		goto unmap;
+
+	for (i = 0; i < ARRAY_SIZE(boards); i++) {
+		unsigned char *model = rombase + boards[i].offset;
+
+		if (memcmp(model, boards[i].sig, boards[i].len) == 0) {
+			printk(KERN_INFO "Soekris %s: %s\n", model, bios);
+
+			register_net5501();
+			break;
+		}
+	}
+
+unmap:
+	iounmap(rombase);
+	return 0;
+}
+
+static int __init net5501_init(void)
+{
+	if (!is_geode())
+		return 0;
+
+	if (net5501_present())
+		register_net5501();
+
+	return 0;
+}
+
+module_init(net5501_init);
+
+MODULE_AUTHOR("Philip Prindeville <philipp@redfish-solutions.com>");
+MODULE_DESCRIPTION("Soekris net5501 System Setup");
+MODULE_LICENSE("GPL");
diff --git a/drivers/leds/leds-net5501.c b/drivers/leds/leds-net5501.c
deleted file mode 100644
index 0555d47..0000000
--- a/drivers/leds/leds-net5501.c
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Soekris board support code
- *
- * Copyright (C) 2008-2009 Tower Technologies
- * Written 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/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 <linux/module.h>
-
-#include <asm/geode.h>
-
-static const struct gpio_led net5501_leds[] = {
-	{
-		.name = "error",
-		.gpio = 6,
-		.default_trigger = "default-on",
-	},
-};
-
-static struct gpio_led_platform_data net5501_leds_data = {
-	.num_leds = ARRAY_SIZE(net5501_leds),
-	.leds = net5501_leds,
-};
-
-static struct platform_device net5501_leds_dev = {
-	.name = "leds-gpio",
-	.id = -1,
-	.dev.platform_data = &net5501_leds_data,
-};
-
-static void __init init_net5501(void)
-{
-	platform_device_register(&net5501_leds_dev);
-}
-
-struct soekris_board {
-	u16	offset;
-	char	*sig;
-	u8	len;
-	void	(*init)(void);
-};
-
-static struct soekris_board __initdata boards[] = {
-	{ 0xb7b, "net5501", 7, init_net5501 },	/* net5501 v1.33/1.33c */
-	{ 0xb1f, "net5501", 7, init_net5501 },	/* net5501 v1.32i */
-};
-
-static int __init soekris_init(void)
-{
-	int i;
-	unsigned char *rombase, *bios;
-
-	if (!is_geode())
-		return 0;
-
-	rombase = ioremap(0xffff0000, 0xffff);
-	if (!rombase) {
-		printk(KERN_INFO "Soekris net5501 LED driver failed to get rombase");
-		return 0;
-	}
-
-	bios = rombase + 0x20;	/* null terminated */
-
-	if (strncmp(bios, "comBIOS", 7))
-		goto unmap;
-
-	for (i = 0; i < ARRAY_SIZE(boards); i++) {
-		unsigned char *model = rombase + boards[i].offset;
-
-		if (strncmp(model, boards[i].sig, boards[i].len) == 0) {
-			printk(KERN_INFO "Soekris %s: %s\n", model, bios);
-
-			if (boards[i].init)
-				boards[i].init();
-			break;
-		}
-	}
-
-unmap:
-	iounmap(rombase);
-	return 0;
-}
-
-arch_initcall(soekris_init);
-
-MODULE_LICENSE("GPL");
-- 
1.7.7.4


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

* Re: [PATCH 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer
  2011-12-31  9:28 [PATCH 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer Philip Prindeville
@ 2012-01-27 22:59 ` Andrew Morton
  2012-01-28  5:02 ` [PATCH v2 " Philip Prindeville
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2012-01-27 22:59 UTC (permalink / raw)
  To: Philip Prindeville; +Cc: linux-kernel, Richard Purdie, Alessandro Zummo

On Sat, 31 Dec 2011 02:28:42 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:

> From: "Philip A. Prindeville" <philipp@redfish-solutions.com>
> 
> Trivial platform driver for Soekris Engineering net5501 single-board
> computer. Probes well-known locations in ROM for BIOS signature to
> confirm correct platform. Registers 1 LED and 1 GPIO-based button
> (typically used for soft reset).
> 
>
> ...
>
> +static int __init net5501_present(void)
> +{
> +	int i;
> +	unsigned char *rombase, *bios;
> +
> +	rombase = ioremap(BIOS_REGION_BASE, BIOS_REGION_SIZE - 1);
> +	if (!rombase)
> +		printk(KERN_INFO "Soekris net5501 LED driver failed to get rombase");

It seems wrong to continue if ioremap() failed.  It should return
-ENOMEM here.  And perhaps boost the KERN_INFO to KERN_ERR?

> +	bios = rombase + 0x20;	/* null terminated */
> +
> +	if (memcmp(bios, "comBIOS", 7))
> +		goto unmap;
> +
> +	for (i = 0; i < ARRAY_SIZE(boards); i++) {
> +		unsigned char *model = rombase + boards[i].offset;
> +
> +		if (memcmp(model, boards[i].sig, boards[i].len) == 0) {
> +			printk(KERN_INFO "Soekris %s: %s\n", model, bios);
> +
> +			register_net5501();
> +			break;
> +		}
> +	}
> +
> +unmap:
> +	iounmap(rombase);
> +	return 0;
> +}
> +
> +static int __init net5501_init(void)
> +{
> +	if (!is_geode())
> +		return 0;
> +
> +	if (net5501_present())
> +		register_net5501();

This makes no sense.  net5501_present() always returns zero so
register_net5501() is never called.  net5501_present() itself calls
register_net5501(), so perhaps this is just dead code.

It seems cleaner to me to do the register_net5501() call from
net5501_init().  A function called "foo_present()" should test for the
presence of foo (and return a bool!) - we don't expect it to run off
and register things.

I assume this code is all runtime tested?

>
> ...
>


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

* [PATCH v2 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer
  2011-12-31  9:28 [PATCH 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer Philip Prindeville
  2012-01-27 22:59 ` Andrew Morton
@ 2012-01-28  5:02 ` Philip Prindeville
  2012-01-31 22:19   ` Andrew Morton
  1 sibling, 1 reply; 5+ messages in thread
From: Philip Prindeville @ 2012-01-28  5:02 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alessandro Zummo, Richard Purdie, linux-kernel

From: "Philip A. Prindeville" <philipp@redfish-solutions.com>

Andrew: this is from the console when booting 3.2.1:

[    0.308194] platform rtc_cmos: registered platform RTC device (no PNP device
found)
[    0.325158] net5501: system is recognized as "net5501"
[    0.375991] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    0.390948] JFFS2 version 2.2 (NAND) (SUMMARY) (LZMA) (RTIME) (CMODE_PRIORITY
) (c) 2001-2006 Red Hat, Inc.
[    0.400301] msgmni has been set to 500
[    0.412818] io scheduler noop registered
[    0.416325] io scheduler deadline registered (default)

so the driver seems to be working as expected.

---
Trivial platform driver for Soekris Engineering net5501 single-board
computer. Probes well-known locations in ROM for BIOS signature to
confirm correct platform. Registers 1 LED and 1 GPIO-based button
(typically used for soft reset).

Changes in v2 per Andrew:
Bug out early if ioremap() fails.
Elevate warning message for ioremap() failure to KERN_ERR.
Remove duplicated calls to is_geode() and net5501_register().

Other changes in v2:
Prefix printk() messages with KBUILD_MODNAME.
Add missing <linux/module.h> #include.
Add missing newline to ioremap() error message.

Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Andres Salomon <dilinger@queued.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Alessandro Zummo <a.zummo@towertech.it>
---
 arch/x86/Kconfig                  |    6 ++
 arch/x86/platform/geode/Makefile  |    1 +
 arch/x86/platform/geode/net5501.c |  154 +++++++++++++++++++++++++++++++++++++
 drivers/leds/leds-net5501.c       |   97 -----------------------
 4 files changed, 161 insertions(+), 97 deletions(-)
 create mode 100644 arch/x86/platform/geode/net5501.c
 delete mode 100644 drivers/leds/leds-net5501.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2f18a15..7a4f34b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2174,6 +2174,12 @@ config GEOS
 	---help---
 	  This option enables system support for the Traverse Technologies GEOS.
 
+config NET5501
+	bool "Soekris Engineering net5501 System Support (LEDS, GPIO, etc)"
+	select GPIOLIB
+	---help---
+	  This option enables system support for the Soekris Engineering net5501.
+
 endif # X86_32
 
 config AMD_NB
diff --git a/arch/x86/platform/geode/Makefile b/arch/x86/platform/geode/Makefile
index d8ba564..5b51194 100644
--- a/arch/x86/platform/geode/Makefile
+++ b/arch/x86/platform/geode/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_ALIX)		+= alix.o
+obj-$(CONFIG_NET5501)		+= net5501.o
 obj-$(CONFIG_GEOS)		+= geos.o
diff --git a/arch/x86/platform/geode/net5501.c b/arch/x86/platform/geode/net5501.c
new file mode 100644
index 0000000..66d377e
--- /dev/null
+++ b/arch/x86/platform/geode/net5501.c
@@ -0,0 +1,154 @@
+/*
+ * System Specific setup for Soekris net5501
+ * At the moment this means setup of GPIO control of LEDs and buttons
+ * on net5501 boards.
+ *
+ *
+ * Copyright (C) 2008-2009 Tower Technologies
+ * Written by Alessandro Zummo <a.zummo@towertech.it>
+ *
+ * Copyright (C) 2008 Constantin Baranov <const@mimas.ru>
+ * Copyright (C) 2011 Ed Wildgoose <kernel@wildgooses.com>
+ *                and Philip Prindeville <philipp@redfish-solutions.com>
+ *
+ * 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/module.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+
+#include <asm/geode.h>
+
+#define BIOS_REGION_BASE		0xffff0000
+#define BIOS_REGION_SIZE		0x00010000
+
+static struct gpio_keys_button net5501_gpio_buttons[] = {
+	{
+		.code = KEY_RESTART,
+		.gpio = 24,
+		.active_low = 1,
+		.desc = "Reset button",
+		.type = EV_KEY,
+		.wakeup = 0,
+		.debounce_interval = 100,
+		.can_disable = 0,
+	}
+};
+static struct gpio_keys_platform_data net5501_buttons_data = {
+	.buttons = net5501_gpio_buttons,
+	.nbuttons = ARRAY_SIZE(net5501_gpio_buttons),
+	.poll_interval = 20,
+};
+
+static struct platform_device net5501_buttons_dev = {
+	.name = "gpio-keys-polled",
+	.id = 1,
+	.dev = {
+		.platform_data = &net5501_buttons_data,
+	}
+};
+
+static struct gpio_led net5501_leds[] = {
+	{
+		.name = "net5501:1",
+		.gpio = 6,
+		.default_trigger = "default-on",
+		.active_low = 1,
+	},
+};
+
+static struct gpio_led_platform_data net5501_leds_data = {
+	.num_leds = ARRAY_SIZE(net5501_leds),
+	.leds = net5501_leds,
+};
+
+static struct platform_device net5501_leds_dev = {
+	.name = "leds-gpio",
+	.id = -1,
+	.dev.platform_data = &net5501_leds_data,
+};
+
+static struct __initdata platform_device *net5501_devs[] = {
+	&net5501_buttons_dev,
+	&net5501_leds_dev,
+};
+
+static void __init register_net5501(void)
+{
+	/* Setup LED control through leds-gpio driver */
+	platform_add_devices(net5501_devs, ARRAY_SIZE(net5501_devs));
+}
+
+struct net5501_board {
+	u16	offset;
+	u16	len;
+	char	*sig;
+};
+
+static struct net5501_board __initdata boards[] = {
+	{ 0xb7b, 7, "net5501" },	/* net5501 v1.33/1.33c */
+	{ 0xb1f, 7, "net5501" },	/* net5501 v1.32i */
+};
+
+static bool __init net5501_present(void)
+{
+	int i;
+	unsigned char *rombase, *bios;
+	bool found = false;
+
+	rombase = ioremap(BIOS_REGION_BASE, BIOS_REGION_SIZE - 1);
+	if (!rombase) {
+		printk(KERN_ERR "%s: failed to get rombase\n", KBUILD_MODNAME);
+		return found;
+	}
+
+	bios = rombase + 0x20;	/* null terminated */
+
+	if (memcmp(bios, "comBIOS", 7))
+		goto unmap;
+
+	for (i = 0; i < ARRAY_SIZE(boards); i++) {
+		unsigned char *model = rombase + boards[i].offset;
+
+		if (!memcmp(model, boards[i].sig, boards[i].len)) {
+			printk(KERN_INFO "%s: system is recognized as \"%s\"\n",
+			       KBUILD_MODNAME, model);
+
+			found = true;
+			break;
+		}
+	}
+
+unmap:
+	iounmap(rombase);
+	return found;
+}
+
+static int __init net5501_init(void)
+{
+	if (!is_geode())
+		return 0;
+
+	if (!net5501_present())
+		return 0;
+
+	register_net5501();
+
+	return 0;
+}
+
+module_init(net5501_init);
+
+MODULE_AUTHOR("Philip Prindeville <philipp@redfish-solutions.com>");
+MODULE_DESCRIPTION("Soekris net5501 System Setup");
+MODULE_LICENSE("GPL");
diff --git a/drivers/leds/leds-net5501.c b/drivers/leds/leds-net5501.c
deleted file mode 100644
index 0555d47..0000000
--- a/drivers/leds/leds-net5501.c
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Soekris board support code
- *
- * Copyright (C) 2008-2009 Tower Technologies
- * Written 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/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 <linux/module.h>
-
-#include <asm/geode.h>
-
-static const struct gpio_led net5501_leds[] = {
-	{
-		.name = "error",
-		.gpio = 6,
-		.default_trigger = "default-on",
-	},
-};
-
-static struct gpio_led_platform_data net5501_leds_data = {
-	.num_leds = ARRAY_SIZE(net5501_leds),
-	.leds = net5501_leds,
-};
-
-static struct platform_device net5501_leds_dev = {
-	.name = "leds-gpio",
-	.id = -1,
-	.dev.platform_data = &net5501_leds_data,
-};
-
-static void __init init_net5501(void)
-{
-	platform_device_register(&net5501_leds_dev);
-}
-
-struct soekris_board {
-	u16	offset;
-	char	*sig;
-	u8	len;
-	void	(*init)(void);
-};
-
-static struct soekris_board __initdata boards[] = {
-	{ 0xb7b, "net5501", 7, init_net5501 },	/* net5501 v1.33/1.33c */
-	{ 0xb1f, "net5501", 7, init_net5501 },	/* net5501 v1.32i */
-};
-
-static int __init soekris_init(void)
-{
-	int i;
-	unsigned char *rombase, *bios;
-
-	if (!is_geode())
-		return 0;
-
-	rombase = ioremap(0xffff0000, 0xffff);
-	if (!rombase) {
-		printk(KERN_INFO "Soekris net5501 LED driver failed to get rombase");
-		return 0;
-	}
-
-	bios = rombase + 0x20;	/* null terminated */
-
-	if (strncmp(bios, "comBIOS", 7))
-		goto unmap;
-
-	for (i = 0; i < ARRAY_SIZE(boards); i++) {
-		unsigned char *model = rombase + boards[i].offset;
-
-		if (strncmp(model, boards[i].sig, boards[i].len) == 0) {
-			printk(KERN_INFO "Soekris %s: %s\n", model, bios);
-
-			if (boards[i].init)
-				boards[i].init();
-			break;
-		}
-	}
-
-unmap:
-	iounmap(rombase);
-	return 0;
-}
-
-arch_initcall(soekris_init);
-
-MODULE_LICENSE("GPL");
-- 
1.7.7.5


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

* Re: [PATCH v2 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer
  2012-01-28  5:02 ` [PATCH v2 " Philip Prindeville
@ 2012-01-31 22:19   ` Andrew Morton
  2012-02-01  1:38     ` Philip Prindeville
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2012-01-31 22:19 UTC (permalink / raw)
  To: Philip Prindeville; +Cc: Alessandro Zummo, Richard Purdie, linux-kernel

On Fri, 27 Jan 2012 22:02:19 -0700
Philip Prindeville <philipp@redfish-solutions.com> wrote:

> ---
> Trivial platform driver for Soekris Engineering net5501 single-board
> computer. Probes well-known locations in ROM for BIOS signature to
> confirm correct platform. Registers 1 LED and 1 GPIO-based button
> (typically used for soft reset).
> 
> ...
>
>  arch/x86/Kconfig                  |    6 ++
>  arch/x86/platform/geode/Makefile  |    1 +
>  arch/x86/platform/geode/net5501.c |  154 +++++++++++++++++++++++++++++++++++++
>  drivers/leds/leds-net5501.c       |   97 -----------------------

Forgot to clean up the leftovers:

make[2]: *** No rule to make target `drivers/leds/leds-net5501.c', needed by `drivers/leds/leds-net5501.o'.  Stop.

Also, are you sure that the new Kconfig entry is sufficient?  The old
driver had more dependencies (GPIO_CS5535 and LEDS_TRIGGERS) and
selects LEDS_TRIGGER_DEFAULT_ON.  Should the new driver be doing such
things?


--- a/drivers/leds/Kconfig~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
+++ a/drivers/leds/Kconfig
@@ -89,16 +89,6 @@ config LEDS_NET48XX
 	  This option enables support for the Soekris net4801 and net4826 error
 	  LED.
 
-config LEDS_NET5501
-	tristate "LED Support for Soekris net5501 series Error LED"
-	depends on LEDS_TRIGGERS
-	depends on X86 && GPIO_CS5535
-	select LEDS_TRIGGER_DEFAULT_ON
-	default n
-	help
-	  Add support for the Soekris net5501 board (detection, error led
-	  and GPIO).
-
 config LEDS_FSG
 	tristate "LED Support for the Freecom FSG-3"
 	depends on LEDS_CLASS
--- a/drivers/leds/Makefile~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
+++ a/drivers/leds/Makefile
@@ -14,7 +14,6 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532)	+= led
 obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
 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_COBALT_QUBE)		+= leds-cobalt-qube.o
 obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
_




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

* Re: [PATCH v2 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer
  2012-01-31 22:19   ` Andrew Morton
@ 2012-02-01  1:38     ` Philip Prindeville
  0 siblings, 0 replies; 5+ messages in thread
From: Philip Prindeville @ 2012-02-01  1:38 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alessandro Zummo, Richard Purdie, linux-kernel

On 1/31/12 3:19 PM, Andrew Morton wrote:
> On Fri, 27 Jan 2012 22:02:19 -0700
> Philip Prindeville <philipp@redfish-solutions.com> wrote:
> 
>> ---
>> Trivial platform driver for Soekris Engineering net5501 single-board
>> computer. Probes well-known locations in ROM for BIOS signature to
>> confirm correct platform. Registers 1 LED and 1 GPIO-based button
>> (typically used for soft reset).
>>
>> ...
>>
>>  arch/x86/Kconfig                  |    6 ++
>>  arch/x86/platform/geode/Makefile  |    1 +
>>  arch/x86/platform/geode/net5501.c |  154 +++++++++++++++++++++++++++++++++++++
>>  drivers/leds/leds-net5501.c       |   97 -----------------------
> 
> Forgot to clean up the leftovers:
> 
> make[2]: *** No rule to make target `drivers/leds/leds-net5501.c', needed by `drivers/leds/leds-net5501.o'.  Stop.
> 
> Also, are you sure that the new Kconfig entry is sufficient?  The old
> driver had more dependencies (GPIO_CS5535 and LEDS_TRIGGERS) and
> selects LEDS_TRIGGER_DEFAULT_ON.  Should the new driver be doing such
> things?

Thanks for getting that.  Overlooked it.

The thing about the way the platform driver is now is it can wait until the subordinate modules get loaded and it will register the relevant devices then...

It's conceivable that one might want LEDs but not the reset button, or vice versa.

It might make sense to have the GPIO_CS5535 driver be selected, but also I'm good with the platform driver being statically linked in, and the LED or GPIO drivers being insmod'd.

I figure best to KISS and allow people to have as small a static kernel as they want, and then build up functionality with modules.

-Philip

> 
> 
> --- a/drivers/leds/Kconfig~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
> +++ a/drivers/leds/Kconfig
> @@ -89,16 +89,6 @@ config LEDS_NET48XX
>  	  This option enables support for the Soekris net4801 and net4826 error
>  	  LED.
>  
> -config LEDS_NET5501
> -	tristate "LED Support for Soekris net5501 series Error LED"
> -	depends on LEDS_TRIGGERS
> -	depends on X86 && GPIO_CS5535
> -	select LEDS_TRIGGER_DEFAULT_ON
> -	default n
> -	help
> -	  Add support for the Soekris net5501 board (detection, error led
> -	  and GPIO).
> -
>  config LEDS_FSG
>  	tristate "LED Support for the Freecom FSG-3"
>  	depends on LEDS_CLASS
> --- a/drivers/leds/Makefile~net5501-platform-driver-for-soekris-engineering-net5501-single-board-computer-fix
> +++ a/drivers/leds/Makefile
> @@ -14,7 +14,6 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532)	+= led
>  obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
>  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_COBALT_QUBE)		+= leds-cobalt-qube.o
>  obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
> _
> 
> 
> 


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

end of thread, other threads:[~2012-02-01  1:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-31  9:28 [PATCH 1/1] net5501: platform driver for Soekris Engineering net5501 single-board computer Philip Prindeville
2012-01-27 22:59 ` Andrew Morton
2012-01-28  5:02 ` [PATCH v2 " Philip Prindeville
2012-01-31 22:19   ` Andrew Morton
2012-02-01  1:38     ` Philip Prindeville

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).