Hi,

For a couple of days I've been playing with wiring up the LEDs and
buttons (OK, there's only one of each) of a WAP54Gv2 device with the
newish generic Linux 2.6 LED class and input layer.

As it turns out, there's no problem with the LED, after registering
it as a platform device /sys/class/leds/gpio:green:wlan appears and
works with the available triggers without a hitch.

The reset button needs more tweaking, because on one hand the GPIO IRQ
interface is inadequate and on the other hand the gpio_keys kernel
module is way too rigid.  However, after fixing these the prototype
starts working.

I think this approach has technical merits: using the generic kernel
interfaces cuts down on home-grown code and provides free add-ons,
like the various LED trigger modules for example.  On the other hand,
it surely incurs some size increase, which may or may not be
acceptable for OpenWRT.  I can't provide hard numbers yet, but it
doesn't seem too bad.

The third part of broadcom-diag, namely, board identifiction, is
something which has to predate platform device registration, as
detecting GPIO wiring is impossible in general, and isn't described in
firmware either, AFAIK (how wonderful that would be!)  It could even
move into user space, if we don't want to use it earlier.

After this, /sbin/hotplug.failsafe could be replaced by a read from
/dev/event0.

Please comment on the idea in general or the implementation in
particular.  The latter is more a proof of concept than anything,
though, so be gentle...  All are platform/* patches.

Not for general consumption: hard wires WAP54Gv2 in the platform
setup code.  Board detection could be added here, if early
availability is desired.

Index: b/arch/mips/bcm47xx/setup.c
===================================================================
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -39,6 +39,9 @@
 #include <asm/time.h>
 #include <asm/fw/cfe/cfe_api.h>
 #include <linux/pm.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
 
 #include "include/nvram.h"
 
@@ -222,3 +225,73 @@ static int __init bcm47xx_register_gpiod
 	return 0;
 }
 device_initcall(bcm47xx_register_gpiodev);
+
+/* FIXME __initdata ? */
+struct gpio_led wap54gv2_leds[] = {
+	{
+	.name			= "gpio:green:wlan",
+	.default_trigger	= "default-on", /* provide diagnostics */
+	.gpio			= 5,
+	.active_low		= 1
+	}
+};
+
+struct gpio_led_platform_data wap54gv2_leds_pdata = {
+	.num_leds	= sizeof wap54gv2_leds / sizeof *wap54gv2_leds,
+	.leds		= wap54gv2_leds,
+};
+
+struct platform_device wap54gv2_gpio_led_device = {
+	.name	= "leds-gpio",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &wap54gv2_leds_pdata
+	}
+};
+
+static int __init wap54gv2_register_gpio_leds(void)
+{
+	if (platform_device_register(&wap54gv2_gpio_led_device)) {
+		printk(KERN_ERR "bcm47xx: registering wap54gv2 GPIO LED device failed\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+device_initcall(wap54gv2_register_gpio_leds);
+
+struct gpio_keys_button wap54gv2_buttons[] = {
+	{
+		.code		= KEY_R,
+        	.gpio		= 0,
+        	.active_low	= 0,
+        	.desc		= "reset",
+        	.type		= EV_KEY,
+        	.wakeup		= 0,
+        	.debounce_interval = 1000 /* msecs */
+	}
+};
+
+struct gpio_keys_platform_data wap54gv2_buttons_pdata = {
+        .buttons	= wap54gv2_buttons,
+        .nbuttons	= sizeof wap54gv2_buttons / sizeof *wap54gv2_buttons
+};
+
+struct platform_device wap54gv2_gpio_button_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &wap54gv2_buttons_pdata
+	}
+};
+
+static int __init wap54g_register_gpio_buttons(void)
+{
+	if (platform_device_register(&wap54gv2_gpio_button_device)) {
+		printk(KERN_ERR "bcm47xx: registering wap54gv2 GPIO button device failed\n");
+                return -ENODEV;
+        }
+
+	return 0;
+}
+device_initcall(wap54g_register_gpio_buttons);
We need a way to enable/disable GPIO interrupts.  Only handles
chipco, I don't know what to do with extif.

Index: b/include/linux/ssb/ssb_embedded.h
===================================================================
--- a/include/linux/ssb/ssb_embedded.h
+++ b/include/linux/ssb/ssb_embedded.h
@@ -14,5 +14,6 @@ u32 ssb_gpio_outen(struct ssb_bus *bus, 
 u32 ssb_gpio_control(struct ssb_bus *bus, u32 mask, u32 value);
 u32 ssb_gpio_intmask(struct ssb_bus *bus, u32 mask, u32 value);
 u32 ssb_gpio_polarity(struct ssb_bus *bus, u32 mask, u32 value);
+void ssb_gpio_irq_enable(struct ssb_bus *bus, int value);
 
 #endif /* LINUX_SSB_EMBEDDED_H_ */
Index: b/drivers/ssb/embedded.c
===================================================================
--- a/drivers/ssb/embedded.c
+++ b/drivers/ssb/embedded.c
@@ -135,6 +135,20 @@ u32 ssb_gpio_polarity(struct ssb_bus *bu
 }
 EXPORT_SYMBOL(ssb_gpio_polarity);
 
+void ssb_gpio_irq_enable(struct ssb_bus *bus, int enabled)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->gpio_lock, flags);
+	if (ssb_chipco_available(&bus->chipco))
+		ssb_chipco_irq_mask(&bus->chipco, SSB_CHIPCO_IRQ_GPIO,
+			enabled ? SSB_CHIPCO_IRQ_GPIO : 0);
+	else
+		SSB_WARN_ON(1);
+	spin_unlock_irqrestore(&bus->gpio_lock, flags);
+}
+EXPORT_SYMBOL(ssb_gpio_irq_enable);
+
 #ifdef CONFIG_SSB_DRIVER_GIGE
 static int gige_pci_init_callback(struct ssb_bus *bus, unsigned long data)
 {
Pass up the global interrupt enable/disable and control
functionality to the generic GPIO interface.

I again don't know what this "control functionality" is:
I use that based on broadcom-diag but it doesn't seem to
matter...

Index: b/arch/mips/include/asm/mach-bcm47xx/gpio.h
===================================================================
--- a/arch/mips/include/asm/mach-bcm47xx/gpio.h
+++ b/arch/mips/include/asm/mach-bcm47xx/gpio.h
@@ -41,6 +41,13 @@ static inline int gpio_direction_output(
 	return 0;
 }
 
+static inline int gpio_control(unsigned gpio, int value)
+{
+	ssb_gpio_control(&ssb_bcm47xx, 1 << gpio,
+			value ? 1 << gpio : 0);
+	return 0;
+}
+
 static inline int gpio_intmask(unsigned gpio, int value)
 {
 	ssb_gpio_intmask(&ssb_bcm47xx, 1 << gpio,
@@ -55,6 +62,10 @@ static inline int gpio_polarity(unsigned
 	return 0;
 }
 
+static inline void gpio_irq_enable(int value)
+{
+	ssb_gpio_irq_enable(&ssb_bcm47xx, value);
+}
 
 /* cansleep wrappers */
 #include <asm-generic/gpio.h>
This is very messy, and will need a general rework as
the comments imply.

Index: b/drivers/input/keyboard/gpio_keys.c
===================================================================
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -58,9 +58,15 @@ static irqreturn_t gpio_keys_isr(int irq
 {
 	struct gpio_button_data *bdata = dev_id;
 	struct gpio_keys_button *button = bdata->button;
+	int value = gpio_get_value(button->gpio);
 
 	BUG_ON(irq != gpio_to_irq(button->gpio));
 
+	/* FIXME: exit if this is not a GPIO button irq (as it's shared now) */
+
+	/* deassert (acknowledge) interrupt */
+	gpio_polarity(button->gpio, value);
+
 	if (button->debounce_interval)
 		mod_timer(&bdata->timer,
 			jiffies + msecs_to_jiffies(button->debounce_interval));
@@ -70,6 +76,9 @@ static irqreturn_t gpio_keys_isr(int irq
 	return IRQ_HANDLED;
 }
 
+static int gpio_keys_share_irqs = 1;
+module_param(gpio_keys_share_irqs, int, S_IRUGO|S_IWUSR);
+
 static int __devinit gpio_keys_probe(struct platform_device *pdev)
 {
 	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
@@ -77,6 +86,10 @@ static int __devinit gpio_keys_probe(str
 	struct input_dev *input;
 	int i, error;
 	int wakeup = 0;
+	unsigned long irqflags = IRQF_SAMPLE_RANDOM;
+
+	/* Leave triggering unspecified (level assumed) for better sharing */
+	irqflags |= gpio_keys_share_irqs ? IRQF_SHARED : IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
 	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
 			pdata->nbuttons * sizeof(struct gpio_button_data),
@@ -127,6 +140,14 @@ static int __devinit gpio_keys_probe(str
 			goto fail2;
 		}
 
+		error = gpio_control(button->gpio, 0);
+		if (error) {
+			pr_err("gpio-keys: Unable to disable control for GPIO %d, error %d\n",
+				button->gpio, error);
+			gpio_free(button->gpio);
+			goto fail2;
+		}
+
 		irq = gpio_to_irq(button->gpio);
 		if (irq < 0) {
 			error = irq;
@@ -137,14 +158,30 @@ static int __devinit gpio_keys_probe(str
 			goto fail2;
 		}
 
-		error = request_irq(irq, gpio_keys_isr,
-				    IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING,
+		/* FIXME: skip if already requested for another button */
+		/* FIXME: revert 57ffe9d539e0eb741bb9ca8f2834d210e70ee2e3 */
+		error = request_irq(irq, gpio_keys_isr, irqflags,
 				    button->desc ? button->desc : "gpio_keys",
 				    bdata);
 		if (error) {
-			pr_err("gpio-keys: Unable to claim irq %d; error %d\n",
-				irq, error);
+			pr_err("gpio-keys: Unable to claim %s irq %d; error %d\n",
+				gpio_keys_share_irqs ? "shared" : "private", irq, error);
+			gpio_free(button->gpio);
+			goto fail2;
+		}
+
+		error = gpio_polarity(button->gpio, gpio_get_value(button->gpio));
+		if (error) {
+			pr_err("gpio-keys: Unable to set polarity of GPIO %d, error %d\n",
+				button->gpio, error);
+			gpio_free(button->gpio);
+			goto fail2;
+		}
+
+		error = gpio_intmask(button->gpio, 1);
+		if (error) {
+			pr_err("gpio-keys: Unable to enable interrupts for GPIO %d, error %d\n",
+				button->gpio, error);
 			gpio_free(button->gpio);
 			goto fail2;
 		}
@@ -155,6 +192,9 @@ static int __devinit gpio_keys_probe(str
 		input_set_capability(input, type, button->code);
 	}
 
+	/* FIXME: once for each source */
+	gpio_irq_enable(1);
+
 	error = input_register_device(input);
 	if (error) {
 		pr_err("gpio-keys: Unable to register input device, "
@@ -191,12 +231,17 @@ static int __devexit gpio_keys_remove(st
 
 	device_init_wakeup(&pdev->dev, 0);
 
+	/* FIXME: once for each source */
+	gpio_irq_enable(0);
+
 	for (i = 0; i < pdata->nbuttons; i++) {
-		int irq = gpio_to_irq(pdata->buttons[i].gpio);
+		struct gpio_keys_button *button = &pdata->buttons[i];
+		int irq = gpio_to_irq(button->gpio);
+		gpio_intmask(button->gpio, 0);
 		free_irq(irq, &ddata->data[i]);
-		if (pdata->buttons[i].debounce_interval)
+		if (button->debounce_interval)
 			del_timer_sync(&ddata->data[i].timer);
-		gpio_free(pdata->buttons[i].gpio);
+		gpio_free(button->gpio);
 	}
 
 	input_unregister_device(input);
@@ -270,6 +315,7 @@ static void __exit gpio_keys_exit(void)
 module_init(gpio_keys_init);
 module_exit(gpio_keys_exit);
 
+MODULE_PARM_DESC(gpio_keys_share_irqs, "whether to request shared irqs (default=1 - yes)");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Phil Blundell <p...@handhelds.org>");
 MODULE_DESCRIPTION("Keyboard driver for CPU GPIOs");
-- 
Thanks,
Feri.
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to