Hi,
for a project I was working on, we used the J15 connector of the ALIX.2D
board (which is connected to a GPIO of the CS5536 Geode companion chip)
for an external button switch.
I thought it might be worth sharing this piece of code, so others can do
the same.
There is, however, a small hardware modification is necessary as the pin
is also shared as buzzer output. I noted that in the Kconfig entry.
The patch should go thru Richard's led-2.6.git as it depends on another
one that has been queued there already.
Thanks,
Daniel
>From fd5915cf86c659eac67a8c0ea4379a78efbe2231 Mon Sep 17 00:00:00 2001
From: Daniel Mack <[email protected]>
Date: Tue, 13 Oct 2009 12:42:52 +0800
Subject: [PATCH] leds-alix2: add support for button connected to J15
The ALIX2 boards have one GPIO pin which is reachable at connector J15.
One possible application for this feature is to connect a button which
closes the two pins.
This patch adds support to query these button and export its state via
an input device.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Richard Purdie <[email protected]>
Cc: [email protected]
Cc: Constantin Baranov <[email protected]>
---
drivers/leds/Kconfig | 14 ++++++++++++
drivers/leds/leds-alix2.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e4f599f..7e57bba 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -77,6 +77,20 @@ config LEDS_ALIX2
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_ALIX2_BUTTON
+ bool "Input device support for button on ALIX boards"
+ depends on LEDS_ALIX2 && INPUT
+ select INPUT_POLLDEV
+ help
+ This option enables support for a button connected to J15 of ALIX
+ boards.
+
+ Note that for this feature to work, there is need for a minor
+ modification to the hardware. R1 needs to be removed, and R4 needs
+ to be places as 100KOhms pull-up.
+
+ Only select that option if you modified your ALIX board like this.
+
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS && ARCH_H1940
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
index f59ffad..e41b252 100644
--- a/drivers/leds/leds-alix2.c
+++ b/drivers/leds/leds-alix2.c
@@ -12,6 +12,7 @@
#include <linux/platform_device.h>
#include <linux/string.h>
#include <linux/pci.h>
+#include <linux/input-polldev.h>
static int force = 0;
module_param(force, bool, 0444);
@@ -29,6 +30,12 @@ static struct pci_device_id divil_pci[] = {
};
MODULE_DEVICE_TABLE(pci, divil_pci);
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+static struct input_polled_dev *ipdev;
+static int alix_button_last;
+#define POLL_INTERVAL_DEFAULT 250
+#endif
+
struct alix_led {
struct led_classdev cdev;
unsigned short port;
@@ -78,6 +85,20 @@ static struct alix_led alix_leds[] = {
},
};
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+static void alix_button_poll(struct input_polled_dev *ipdev)
+{
+ unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
+
+ if (val == alix_button_last)
+ return;
+
+ input_report_key(ipdev->input, BTN_MISC, val);
+ input_sync(ipdev->input);
+ alix_button_last = val;
+}
+#endif
+
static int __init alix_led_probe(struct platform_device *pdev)
{
int i;
@@ -89,6 +110,35 @@ static int __init alix_led_probe(struct platform_device *pdev)
if (ret < 0)
goto fail;
}
+
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+ /* enable button input */
+ outl(1 << 1, gpio_base + 0x20);
+
+ /* enable pullup on input pin */
+ outl(1 << 1, gpio_base + 0x18);
+
+ alix_button_last = 0;
+ ipdev = input_allocate_polled_device();
+ if (!ipdev)
+ goto fail;
+
+ ipdev->poll = alix_button_poll;
+ ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
+ ipdev->input->name = "ALIX2 button";
+ ipdev->input->phys = "alix2/input0";
+ ipdev->input->id.bustype = BUS_HOST;
+
+ set_bit(EV_KEY, ipdev->input->evbit);
+ ipdev->input->keybit[BIT_WORD(BTN_MISC)] = BIT_MASK(BTN_MISC);
+
+ ret = input_register_polled_device(ipdev);
+ if (ret) {
+ input_free_polled_device(ipdev);
+ goto fail;
+ }
+#endif
+
return 0;
fail:
--
1.6.0.4
Hi Daniel,
On Fri, Oct 16, 2009 at 03:03:15PM +0200, Daniel Mack wrote:
>
> +#ifdef CONFIG_LEDS_ALIX2_BUTTON
> +static void alix_button_poll(struct input_polled_dev *ipdev)
> +{
> + unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
> +
> + if (val == alix_button_last)
> + return;
You can probably let input core filter out duplicate events...
> +
> + input_report_key(ipdev->input, BTN_MISC, val);
BTN_MISC is expected to be used by joysticks or poniting devices, I'd
recomment KEY_PROG1 for a key.
> + input_sync(ipdev->input);
> + alix_button_last = val;
> +}
> +#endif
> +
> static int __init alix_led_probe(struct platform_device *pdev)
> {
> int i;
> @@ -89,6 +110,35 @@ static int __init alix_led_probe(struct platform_device *pdev)
> if (ret < 0)
> goto fail;
> }
> +
> +#ifdef CONFIG_LEDS_ALIX2_BUTTON
> + /* enable button input */
> + outl(1 << 1, gpio_base + 0x20);
> +
> + /* enable pullup on input pin */
> + outl(1 << 1, gpio_base + 0x18);
> +
> + alix_button_last = 0;
> + ipdev = input_allocate_polled_device();
> + if (!ipdev)
> + goto fail;
> +
> + ipdev->poll = alix_button_poll;
> + ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
> + ipdev->input->name = "ALIX2 button";
> + ipdev->input->phys = "alix2/input0";
> + ipdev->input->id.bustype = BUS_HOST;
> +
> + set_bit(EV_KEY, ipdev->input->evbit);
> + ipdev->input->keybit[BIT_WORD(BTN_MISC)] = BIT_MASK(BTN_MISC);
> +
> + ret = input_register_polled_device(ipdev);
> + if (ret) {
> + input_free_polled_device(ipdev);
> + goto fail;
> + }
> +#endif
Fells like you want to make it a function (and a stub if not
configured)...
--
Dmitry
Hi,
On Fri, 2009-10-16 at 15:03 +0200, Daniel Mack wrote:
> for a project I was working on, we used the J15 connector of the ALIX.2D
> board (which is connected to a GPIO of the CS5536 Geode companion chip)
> for an external button switch.
>
> I thought it might be worth sharing this piece of code, so others can do
> the same.
>
> There is, however, a small hardware modification is necessary as the pin
> is also shared as buzzer output. I noted that in the Kconfig entry.
>
> The patch should go thru Richard's led-2.6.git as it depends on another
> one that has been queued there already.
How common is this modification? I'm just trying to get a feel for the
number of users out there of this feature...
Cheers,
Richard
--
Richard Purdie
Intel Open Source Technology Centre
On Sun, Oct 18, 2009 at 11:56:52AM +0100, Richard Purdie wrote:
> On Fri, 2009-10-16 at 15:03 +0200, Daniel Mack wrote:
> > for a project I was working on, we used the J15 connector of the ALIX.2D
> > board (which is connected to a GPIO of the CS5536 Geode companion chip)
> > for an external button switch.
> >
> > I thought it might be worth sharing this piece of code, so others can do
> > the same.
> >
> > There is, however, a small hardware modification is necessary as the pin
> > is also shared as buzzer output. I noted that in the Kconfig entry.
> >
> > The patch should go thru Richard's led-2.6.git as it depends on another
> > one that has been queued there already.
>
> How common is this modification? I'm just trying to get a feel for the
> number of users out there of this feature...
Hmm, hard to say. One can either use that IO as output to control a
buzzer (which isn't placed on the board, but there are solder pads for
it), as output for other purpose or as input.
Don't know what other people use it for, though, but we needed an
external button in our case and use this IO pin for that purpose.
Anyway - I don't insist in that patch to be taken. The idea was more to
share it so others won't have to hack the same thing.
I'll still submit a new version with Dmitry's suggestions addressed.
Thanks,
Daniel
On Sun, Oct 18, 2009 at 12:27:39AM -0700, Dmitry Torokhov wrote:
> On Fri, Oct 16, 2009 at 03:03:15PM +0200, Daniel Mack wrote:
> >
> > +#ifdef CONFIG_LEDS_ALIX2_BUTTON
> > +static void alix_button_poll(struct input_polled_dev *ipdev)
> > +{
> > + unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
> > +
> > + if (val == alix_button_last)
> > + return;
>
> You can probably let input core filter out duplicate events...
> > +
> > + input_report_key(ipdev->input, BTN_MISC, val);
>
> BTN_MISC is expected to be used by joysticks or poniting devices, I'd
> recomment KEY_PROG1 for a key.
>
> > + input_sync(ipdev->input);
> > + alix_button_last = val;
> > +}
> > +#endif
> > +
> > static int __init alix_led_probe(struct platform_device *pdev)
> > {
> > int i;
> > @@ -89,6 +110,35 @@ static int __init alix_led_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto fail;
> > }
> > +
> > +#ifdef CONFIG_LEDS_ALIX2_BUTTON
> > + /* enable button input */
> > + outl(1 << 1, gpio_base + 0x20);
> > +
> > + /* enable pullup on input pin */
> > + outl(1 << 1, gpio_base + 0x18);
> > +
> > + alix_button_last = 0;
> > + ipdev = input_allocate_polled_device();
> > + if (!ipdev)
> > + goto fail;
> > +
> > + ipdev->poll = alix_button_poll;
> > + ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
> > + ipdev->input->name = "ALIX2 button";
> > + ipdev->input->phys = "alix2/input0";
> > + ipdev->input->id.bustype = BUS_HOST;
> > +
> > + set_bit(EV_KEY, ipdev->input->evbit);
> > + ipdev->input->keybit[BIT_WORD(BTN_MISC)] = BIT_MASK(BTN_MISC);
> > +
> > + ret = input_register_polled_device(ipdev);
> > + if (ret) {
> > + input_free_polled_device(ipdev);
> > + goto fail;
> > + }
> > +#endif
>
> Fells like you want to make it a function (and a stub if not
> configured)...
Agreed in all points - new version below.
Thanks,
Daniel
>From 3095491476facc28f4cc639e7e44227c38ec0f8f Mon Sep 17 00:00:00 2001
From: Daniel Mack <[email protected]>
Date: Tue, 13 Oct 2009 12:42:52 +0800
Subject: [PATCH] leds-alix2: add support for button connected to J15
The ALIX2 boards have one GPIO pin which is reachable at connector J15.
One possible application for this feature is to connect a button which
closes the two pins.
This patch adds support to query these button and export its state via
an input device.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Richard Purdie <[email protected]>
Cc: [email protected]
Cc: Constantin Baranov <[email protected]>
---
drivers/leds/Kconfig | 14 ++++++++++
drivers/leds/leds-alix2.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 0 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e4f599f..2faba0a 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -77,6 +77,20 @@ config LEDS_ALIX2
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_ALIX2_BUTTON
+ bool "Input device support for button on ALIX boards"
+ depends on LEDS_ALIX2 && INPUT
+ select INPUT_POLLDEV
+ help
+ This option enables support for a button connected to J15 of ALIX
+ boards.
+
+ Note that for this feature to work, there is need for a minor
+ modification to the hardware: R1 needs to be removed, and R4 needs
+ to be placed as 100 KOhms pull-up.
+
+ Only select that option if you modified your ALIX board like this.
+
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS && ARCH_H1940
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
index f59ffad..5788767 100644
--- a/drivers/leds/leds-alix2.c
+++ b/drivers/leds/leds-alix2.c
@@ -12,6 +12,7 @@
#include <linux/platform_device.h>
#include <linux/string.h>
#include <linux/pci.h>
+#include <linux/input-polldev.h>
static int force = 0;
module_param(force, bool, 0444);
@@ -78,6 +79,55 @@ static struct alix_led alix_leds[] = {
},
};
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+
+#define POLL_INTERVAL_DEFAULT 250
+static struct input_polled_dev *ipdev;
+
+static void alix_button_poll(struct input_polled_dev *ipdev)
+{
+ unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
+
+ input_report_key(ipdev->input, KEY_PROG1, val);
+ input_sync(ipdev->input);
+}
+
+static int alix_button_register(void)
+{
+ int ret;
+
+ /* enable button input */
+ outl(1 << 1, gpio_base + 0x20);
+
+ /* enable pullup on input pin */
+ outl(1 << 1, gpio_base + 0x18);
+
+ ipdev = input_allocate_polled_device();
+ if (!ipdev)
+ return -ENOMEM;
+
+ ipdev->poll = alix_button_poll;
+ ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
+ ipdev->input->name = "ALIX2 button";
+ ipdev->input->phys = "alix2/input0";
+ ipdev->input->id.bustype = BUS_HOST;
+
+ set_bit(EV_KEY, ipdev->input->evbit);
+ ipdev->input->keybit[BIT_WORD(KEY_PROG1)] = BIT_MASK(KEY_PROG1);
+
+ ret = input_register_polled_device(ipdev);
+
+ if (ret) {
+ input_free_polled_device(ipdev);
+ ipdev = NULL;
+ }
+
+ return ret;
+}
+#else
+#define alix_button_register() (0)
+#endif
+
static int __init alix_led_probe(struct platform_device *pdev)
{
int i;
@@ -89,6 +139,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
if (ret < 0)
goto fail;
}
+
+ ret = alix_button_register();
+ if (ret)
+ goto fail;
+
return 0;
fail:
@@ -103,6 +158,10 @@ static int alix_led_remove(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
led_classdev_unregister(&alix_leds[i].cdev);
+
+ if (ipdev)
+ input_unregister_polled_device(ipdev);
+
return 0;
}
--
1.6.0.4
Hi Daniel,
On Mon, Oct 19, 2009 at 09:37:28AM +0200, Daniel Mack wrote:
> +#else
> +#define alix_button_register() (0)
> +#endif
> +
> static int __init alix_led_probe(struct platform_device *pdev)
> {
> int i;
> @@ -89,6 +139,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
> if (ret < 0)
> goto fail;
> }
> +
> + ret = alix_button_register();
> + if (ret)
> + goto fail;
> +
> return 0;
>
> fail:
> @@ -103,6 +158,10 @@ static int alix_led_remove(struct platform_device *pdev)
>
> for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
> led_classdev_unregister(&alix_leds[i].cdev);
> +
> + if (ipdev)
> + input_unregister_polled_device(ipdev);
> +
I think it will bomb on !CONFIG_LEDS_ALIX2_BUTTON... You need
alix_button_register() and a dummy wrapper for it as well.
--
Dmitry
Hi Dmitry,
On Mon, Oct 19, 2009 at 06:38:53PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 19, 2009 at 09:37:28AM +0200, Daniel Mack wrote:
> > +#else
> > +#define alix_button_register() (0)
> > +#endif
> > +
> > static int __init alix_led_probe(struct platform_device *pdev)
> > {
> > int i;
> > @@ -89,6 +139,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
> > if (ret < 0)
> > goto fail;
> > }
> > +
> > + ret = alix_button_register();
> > + if (ret)
> > + goto fail;
> > +
> > return 0;
> >
> > fail:
> > @@ -103,6 +158,10 @@ static int alix_led_remove(struct platform_device *pdev)
> >
> > for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
> > led_classdev_unregister(&alix_leds[i].cdev);
> > +
> > + if (ipdev)
> > + input_unregister_polled_device(ipdev);
> > +
>
>
> I think it will bomb on !CONFIG_LEDS_ALIX2_BUTTON... You need
> alix_button_register() and a dummy wrapper for it as well.
There is one actually - lazyly implemented as #define in this case. But
you're right, it does bomb, but due to an undefined reference to ipdev
in alix_led_remove(). Below is a new patch the finally works fine for
both configs.
Daniel
>From f3ed2eb2a123a2d2ff1fd587d1ff1076893ae148 Mon Sep 17 00:00:00 2001
From: Daniel Mack <[email protected]>
Date: Tue, 13 Oct 2009 12:42:52 +0800
Subject: [PATCH] leds-alix2: add support for button connected to J15
The ALIX2 boards have one GPIO pin which is reachable at connector J15.
One possible application for this feature is to connect a button which
closes the two pins.
This patch adds support to query these button and export its state via
an input device.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Richard Purdie <[email protected]>
Cc: [email protected]
Cc: Constantin Baranov <[email protected]>
---
drivers/leds/Kconfig | 14 ++++++++++
drivers/leds/leds-alix2.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+), 0 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e4f599f..2faba0a 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -77,6 +77,20 @@ config LEDS_ALIX2
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_ALIX2_BUTTON
+ bool "Input device support for button on ALIX boards"
+ depends on LEDS_ALIX2 && INPUT
+ select INPUT_POLLDEV
+ help
+ This option enables support for a button connected to J15 of ALIX
+ boards.
+
+ Note that for this feature to work, there is need for a minor
+ modification to the hardware: R1 needs to be removed, and R4 needs
+ to be placed as 100 KOhms pull-up.
+
+ Only select that option if you modified your ALIX board like this.
+
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS && ARCH_H1940
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
index f59ffad..6ef6362 100644
--- a/drivers/leds/leds-alix2.c
+++ b/drivers/leds/leds-alix2.c
@@ -12,6 +12,7 @@
#include <linux/platform_device.h>
#include <linux/string.h>
#include <linux/pci.h>
+#include <linux/input-polldev.h>
static int force = 0;
module_param(force, bool, 0444);
@@ -78,6 +79,55 @@ static struct alix_led alix_leds[] = {
},
};
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+
+#define POLL_INTERVAL_DEFAULT 250
+static struct input_polled_dev *ipdev;
+
+static void alix_button_poll(struct input_polled_dev *ipdev)
+{
+ unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
+
+ input_report_key(ipdev->input, KEY_PROG1, val);
+ input_sync(ipdev->input);
+}
+
+static int alix_button_register(void)
+{
+ int ret;
+
+ /* enable button input */
+ outl(1 << 1, gpio_base + 0x20);
+
+ /* enable pullup on input pin */
+ outl(1 << 1, gpio_base + 0x18);
+
+ ipdev = input_allocate_polled_device();
+ if (!ipdev)
+ return -ENOMEM;
+
+ ipdev->poll = alix_button_poll;
+ ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
+ ipdev->input->name = "ALIX2 button";
+ ipdev->input->phys = "alix2/input0";
+ ipdev->input->id.bustype = BUS_HOST;
+
+ set_bit(EV_KEY, ipdev->input->evbit);
+ ipdev->input->keybit[BIT_WORD(KEY_PROG1)] = BIT_MASK(KEY_PROG1);
+
+ ret = input_register_polled_device(ipdev);
+
+ if (ret) {
+ input_free_polled_device(ipdev);
+ ipdev = NULL;
+ }
+
+ return ret;
+}
+#else
+#define alix_button_register() (0)
+#endif
+
static int __init alix_led_probe(struct platform_device *pdev)
{
int i;
@@ -89,6 +139,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
if (ret < 0)
goto fail;
}
+
+ ret = alix_button_register();
+ if (ret)
+ goto fail;
+
return 0;
fail:
@@ -103,6 +158,12 @@ static int alix_led_remove(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
led_classdev_unregister(&alix_leds[i].cdev);
+
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+ if (ipdev)
+ input_unregister_polled_device(ipdev);
+#endif
+
return 0;
}
--
1.6.0.4
Hi Daniel,
On Tue, Oct 20, 2009 at 12:13:57PM +0200, Daniel Mack wrote:
> +
> + ret = input_register_polled_device(ipdev);
> +
> + if (ret) {
> + input_free_polled_device(ipdev);
> + ipdev = NULL;
> + }
> +
> + return ret;
> +}
Ah, don't be lazy ;)
static void alix_button_unregister(void)
{
if (ipdev) {
input_unregister_polled_device(ipdev);
/* Yes, polled devices need to be freed */
input_free_polled_device(ipdev);
ipdev = NULL;
}
}
> +#else
> +#define alix_button_register() (0)
static inline int alix_button_register(void) { return 0; }
static inline void alix_button_unregister(void) { }
So you get typechecking (if any).
> +#endif
> +
> static int __init alix_led_probe(struct platform_device *pdev)
> {
> int i;
> @@ -89,6 +139,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
> if (ret < 0)
> goto fail;
> }
> +
> + ret = alix_button_register();
> + if (ret)
> + goto fail;
> +
> return 0;
>
> fail:
> @@ -103,6 +158,12 @@ static int alix_led_remove(struct platform_device *pdev)
>
> for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
> led_classdev_unregister(&alix_leds[i].cdev);
> +
> +#ifdef CONFIG_LEDS_ALIX2_BUTTON
> + if (ipdev)
> + input_unregister_polled_device(ipdev);
> +#endif
alix_button_unregister();
> +
> return 0;
> }
>
--
Dmitry
Hi Dmitry,
On Tue, Oct 20, 2009 at 09:35:53PM -0700, Dmitry Torokhov wrote:
> On Tue, Oct 20, 2009 at 12:13:57PM +0200, Daniel Mack wrote:
> > +
> > + ret = input_register_polled_device(ipdev);
> > +
> > + if (ret) {
> > + input_free_polled_device(ipdev);
> > + ipdev = NULL;
> > + }
> > +
> > + return ret;
> > +}
>
> Ah, don't be lazy ;)
Ok ok, you're right, let's do it the clean way. Agreed :)
See the one below.
Thanks!
Daniel
>From 66953e5dda7bbbc22b92541ed3a6020773c62140 Mon Sep 17 00:00:00 2001
From: Daniel Mack <[email protected]>
Date: Tue, 13 Oct 2009 12:42:52 +0800
Subject: [PATCH] leds-alix2: add support for button connected to J15
The ALIX2 boards have one GPIO pin which is reachable at connector J15.
One possible application for this feature is to connect a button which
closes the two pins.
This patch adds support to query these button and export its state via
an input device.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Richard Purdie <[email protected]>
Cc: [email protected]
Cc: Constantin Baranov <[email protected]>
---
drivers/leds/Kconfig | 14 +++++++++
drivers/leds/leds-alix2.c | 71 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e4f599f..2faba0a 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -77,6 +77,20 @@ config LEDS_ALIX2
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_ALIX2_BUTTON
+ bool "Input device support for button on ALIX boards"
+ depends on LEDS_ALIX2 && INPUT
+ select INPUT_POLLDEV
+ help
+ This option enables support for a button connected to J15 of ALIX
+ boards.
+
+ Note that for this feature to work, there is need for a minor
+ modification to the hardware: R1 needs to be removed, and R4 needs
+ to be placed as 100 KOhms pull-up.
+
+ Only select that option if you modified your ALIX board like this.
+
config LEDS_H1940
tristate "LED Support for iPAQ H1940 device"
depends on LEDS_CLASS && ARCH_H1940
diff --git a/drivers/leds/leds-alix2.c b/drivers/leds/leds-alix2.c
index f59ffad..c981edf 100644
--- a/drivers/leds/leds-alix2.c
+++ b/drivers/leds/leds-alix2.c
@@ -12,6 +12,7 @@
#include <linux/platform_device.h>
#include <linux/string.h>
#include <linux/pci.h>
+#include <linux/input-polldev.h>
static int force = 0;
module_param(force, bool, 0444);
@@ -78,6 +79,68 @@ static struct alix_led alix_leds[] = {
},
};
+#ifdef CONFIG_LEDS_ALIX2_BUTTON
+
+#define POLL_INTERVAL_DEFAULT 250
+static struct input_polled_dev *ipdev;
+
+static void alix_button_poll(struct input_polled_dev *ipdev)
+{
+ unsigned int val = !(inl(gpio_base + 0x30) & (1 << 1));
+
+ input_report_key(ipdev->input, KEY_PROG1, val);
+ input_sync(ipdev->input);
+}
+
+static int alix_button_register(void)
+{
+ int ret;
+
+ /* enable button input */
+ outl(1 << 1, gpio_base + 0x20);
+
+ /* enable pullup on input pin */
+ outl(1 << 1, gpio_base + 0x18);
+
+ ipdev = input_allocate_polled_device();
+ if (!ipdev)
+ return -ENOMEM;
+
+ ipdev->poll = alix_button_poll;
+ ipdev->poll_interval = POLL_INTERVAL_DEFAULT;
+ ipdev->input->name = "ALIX2 button";
+ ipdev->input->phys = "alix2/input0";
+ ipdev->input->id.bustype = BUS_HOST;
+
+ set_bit(EV_KEY, ipdev->input->evbit);
+ ipdev->input->keybit[BIT_WORD(KEY_PROG1)] = BIT_MASK(KEY_PROG1);
+
+ ret = input_register_polled_device(ipdev);
+
+ if (ret) {
+ input_free_polled_device(ipdev);
+ ipdev = NULL;
+ }
+
+ return ret;
+}
+
+static void alix_button_unregister(void)
+{
+ if (!ipdev)
+ return;
+
+ input_unregister_polled_device(ipdev);
+ /* Yes, polled devices need to be freed */
+ input_free_polled_device(ipdev);
+ ipdev = NULL;
+}
+
+#else
+static inline int alix_button_register(void) { return 0; }
+static inline void alix_button_unregister(void) { }
+#endif
+
static int __init alix_led_probe(struct platform_device *pdev)
{
int i;
@@ -89,6 +152,11 @@ static int __init alix_led_probe(struct platform_device *pdev)
if (ret < 0)
goto fail;
}
+
+ ret = alix_button_register();
+ if (ret)
+ goto fail;
+
return 0;
fail:
@@ -103,6 +171,9 @@ static int alix_led_remove(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(alix_leds); i++)
led_classdev_unregister(&alix_leds[i].cdev);
+
+ alix_button_unregister();
+
return 0;
}
--
1.6.0.4
Hi, Daniel!
I wonder why do you integrate the input driver into the leds driver. They
should be separated. Or at least the complex driver should be moved to the
"X86 Platform Specific Device Drivers" and renamed to some like "PC Engines
ALIX Extras". I personally would prefer the separation way.
Also ALIX.2 documentation describes the "Mode switch" driven by GPIO which is a
small button on front side of a board. I guessed your driver is not for this
button. If so, would it be better to provide the mode switch as KEY_PROG1 and
the J15 connected button as KEY_PROG2 at once?
Cheers,
Constantin
> From 66953e5dda7bbbc22b92541ed3a6020773c62140 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <[email protected]>
> Date: Tue, 13 Oct 2009 12:42:52 +0800
> Subject: [PATCH] leds-alix2: add support for button connected to J15
>
> The ALIX2 boards have one GPIO pin which is reachable at connector J15.
> One possible application for this feature is to connect a button which
> closes the two pins.
>
> This patch adds support to query these button and export its state via
> an input device.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Richard Purdie <[email protected]>
> Cc: [email protected]
> Cc: Constantin Baranov <[email protected]>
On Thu, Oct 22, 2009 at 01:21:21AM +0500, Constantin Baranov wrote:
> I wonder why do you integrate the input driver into the leds driver. They
> should be separated. Or at least the complex driver should be moved to the
> "X86 Platform Specific Device Drivers" and renamed to some like "PC Engines
> ALIX Extras". I personally would prefer the separation way.
I would as well, yes. And I was of course considerating this when I
hacked these lines. However, the major part of the LED driver and what
it currently shares with the button implementation is the BIOS detection
code which is so ugly that I didn't want to duplicate it ;) That would,
however, be the only option if you wanted to split the drivers up. Or
do you have any better idea?
> Also ALIX.2 documentation describes the "Mode switch" driven by GPIO which is a
> small button on front side of a board. I guessed your driver is not for this
> button. If so, would it be better to provide the mode switch as KEY_PROG1 and
> the J15 connected button as KEY_PROG2 at once?
Correct, the button this code is for is not the one on the 'front' side
of the PCB. The other one I didn't try yet, but according the the
CS5536A datasheet, the alternate function for that pin is WORK_AUX which
can be used for power switching purposes. So that might need some extra
care probably.
However, if you can make any suggestion of how to split the code without
copying more than half of the lines for that, I'd be happy with that as
well, of course :)
Daniel
On Wed, 21 Oct 2009 22:41:58 +0200 Daniel Mack <[email protected]> wrote:
> On Thu, Oct 22, 2009 at 01:21:21AM +0500, Constantin Baranov wrote:
> > I wonder why do you integrate the input driver into the leds driver. They
> > should be separated. Or at least the complex driver should be moved to the
> > "X86 Platform Specific Device Drivers" and renamed to some like "PC Engines
> > ALIX Extras". I personally would prefer the separation way.
>
> I would as well, yes. And I was of course considerating this when I
> hacked these lines. However, the major part of the LED driver and what
> it currently shares with the button implementation is the BIOS detection
> code which is so ugly that I didn't want to duplicate it ;) That would,
> however, be the only option if you wanted to split the drivers up. Or
> do you have any better idea?
We may introduce new ALIX2 extended platform (module in arch/x86/kernel).
In the init function it shall perform detection. The module shall export
the is_alix2() function which shall be called from init functions of drivers.
Also the cs5535_gpio driver could be reused for gpio access (the previous
patch for leds-alix2 is copy&paste from cs5535_gpio).
> > Also ALIX.2 documentation describes the "Mode switch" driven by GPIO which is a
> > small button on front side of a board. I guessed your driver is not for this
> > button. If so, would it be better to provide the mode switch as KEY_PROG1 and
> > the J15 connected button as KEY_PROG2 at once?
>
> Correct, the button this code is for is not the one on the 'front' side
> of the PCB. The other one I didn't try yet, but according the the
> CS5536A datasheet, the alternate function for that pin is WORK_AUX which
> can be used for power switching purposes. So that might need some extra
> care probably.
Is there an ALIX board which uses that pin for something but button?
The driver ensures that it works with ALIX, not any CS5536.
Constantin
On Thu, 22 Oct 2009 02:39:28 +0500 Constantin Baranov <[email protected]> wrote:
> On Wed, 21 Oct 2009 22:41:58 +0200 Daniel Mack <[email protected]> wrote:
> > On Thu, Oct 22, 2009 at 01:21:21AM +0500, Constantin Baranov wrote:
> > > I wonder why do you integrate the input driver into the leds driver. They
> > > should be separated. Or at least the complex driver should be moved to the
> > > "X86 Platform Specific Device Drivers" and renamed to some like "PC Engines
> > > ALIX Extras". I personally would prefer the separation way.
> >
> > I would as well, yes. And I was of course considerating this when I
> > hacked these lines. However, the major part of the LED driver and what
> > it currently shares with the button implementation is the BIOS detection
> > code which is so ugly that I didn't want to duplicate it ;) That would,
> > however, be the only option if you wanted to split the drivers up. Or
> > do you have any better idea?
>
> We may introduce new ALIX2 extended platform (module in arch/x86/kernel).
> In the init function it shall perform detection. The module shall export
> the is_alix2() function which shall be called from init functions of drivers.
> Also the cs5535_gpio driver could be reused for gpio access (the previous
> patch for leds-alix2 is copy&paste from cs5535_gpio).
Moreover, the cs5535_gpio driver itself could be reworked and ported to
the gpiolib. This goes far from initial task (button support) indeed :)
Constantin
On Thu, Oct 22, 2009 at 03:09:09AM +0500, Constantin Baranov wrote:
> On Thu, 22 Oct 2009 02:39:28 +0500 Constantin Baranov <[email protected]> wrote:
> > On Wed, 21 Oct 2009 22:41:58 +0200 Daniel Mack <[email protected]> wrote:
> > > On Thu, Oct 22, 2009 at 01:21:21AM +0500, Constantin Baranov wrote:
> > > > I wonder why do you integrate the input driver into the leds driver. They
> > > > should be separated. Or at least the complex driver should be moved to the
> > > > "X86 Platform Specific Device Drivers" and renamed to some like "PC Engines
> > > > ALIX Extras". I personally would prefer the separation way.
> > >
> > > I would as well, yes. And I was of course considerating this when I
> > > hacked these lines. However, the major part of the LED driver and what
> > > it currently shares with the button implementation is the BIOS detection
> > > code which is so ugly that I didn't want to duplicate it ;) That would,
> > > however, be the only option if you wanted to split the drivers up. Or
> > > do you have any better idea?
> >
> > We may introduce new ALIX2 extended platform (module in arch/x86/kernel).
> > In the init function it shall perform detection. The module shall export
> > the is_alix2() function which shall be called from init functions of drivers.
> > Also the cs5535_gpio driver could be reused for gpio access (the previous
> > patch for leds-alix2 is copy&paste from cs5535_gpio).
>
> Moreover, the cs5535_gpio driver itself could be reworked and ported to
> the gpiolib. This goes far from initial task (button support) indeed :)
Indeed, agreed to all points, that would be the sane way to do it.
However, I won't have time to care for that soon. So the best thing to
do for now is drop the patch and leave it in the mail archives. And wait
untils eventually cares to do ir right :)
Daniel
Hi!
> > > for a project I was working on, we used the J15 connector of the ALIX.2D
> > > board (which is connected to a GPIO of the CS5536 Geode companion chip)
> > > for an external button switch.
...
> Hmm, hard to say. One can either use that IO as output to control a
> buzzer (which isn't placed on the board, but there are solder pads for
> it), as output for other purpose or as input.
>
> Don't know what other people use it for, though, but we needed an
> external button in our case and use this IO pin for that purpose.
>
> Anyway - I don't insist in that patch to be taken. The idea was more to
> share it so others won't have to hack the same thing.
So its gpio pin... if you teach gpio framework about it, you should be
able to just use gpio_button driver to handle it, no?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html