On Tue, May 26, 2009 at 08:57:20PM +0200, Janusz Krzysztofik wrote:
> This trivial patch adds gpio-keys compatible platform device definition to
> ams-delta board, that supports hook switch found on this videophone. It is
> derived from similiar definitions found in other boards code. The patch is
> based on linux-2.6.30-rc5. Any comments are welcome.
I'm obviously too late as I've seen the "Applied" mail, but some
comments:
* I don't think SW_HEADPHONE_INSERT is appropriate. input guys, is it
not reasonable to have SW_PHONE_HOOK or similar? I can't find anything
else in tree I know to be a VOIP phone that has the concept of a hook
switch except perhaps the TuxScreen and it doesn't have anything set
up that I could find. I think you're correct that EV_SW is
appropriate; it may remain in the off-hook state for some time and
AFAICT the gpio-key driver would produce a repeating key press if you
used EV_KEY.
* We assume the bootloader does the appropriate GPIO pin setup for us,
so I don't think your omap_cfg_reg is required but it doesn't hurt in
the unlikely event we ever replace the Amstrad PBL.
* The commented out code to include the GPIO status in sysfs shouldn't
be included. Does the input layer not provide a way to obtain the
state of the switch?
> diff -Npru a/arch/arm/mach-omap1/board-ams-delta.c
> b/arch/arm/mach-omap1/board-ams-delta.c
> --- a/arch/arm/mach-omap1/board-ams-delta.c 2009-05-17 17:58:18.000000000
> +0200
> +++ b/arch/arm/mach-omap1/board-ams-delta.c 2009-05-17 16:22:59.000000000
> +0200
> @@ -15,6 +15,7 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/input.h>
> +#include <linux/gpio_keys.h>
> #include <linux/platform_device.h>
>
> #include <mach/hardware.h>
> @@ -213,10 +214,35 @@ static struct platform_device ams_delta_
> .id = -1
> };
>
> +static struct gpio_keys_button ams_delta_gpio_keys_buttons[] = {
> + [0] = {
> + .desc = "hook_switch",
> + .type = EV_SW, /* or EV_KEY ? */
> + .code = SW_HEADPHONE_INSERT, /* or a new one ? */
> + .active_low = 1,
> + .gpio = AMS_DELTA_GPIO_PIN_HOOK_SWITCH,
> + .debounce_interval = 10,
> + },
> +};
> +
> +static struct gpio_keys_platform_data ams_delta_gpio_keys = {
> + .buttons = ams_delta_gpio_keys_buttons,
> + .nbuttons = ARRAY_SIZE(ams_delta_gpio_keys_buttons),
> +};
> +
> +static struct platform_device ams_delta_gpio_keys_device = {
> + .name = "gpio-keys",
> + .id = -1,
> + .dev = {
> + .platform_data = &ams_delta_gpio_keys,
> + },
> +};
> +
> static struct platform_device *ams_delta_devices[] __initdata = {
> &ams_delta_kp_device,
> &ams_delta_lcd_device,
> &ams_delta_led_device,
> + &ams_delta_gpio_keys_device,
> };
>
> static void __init ams_delta_init(void)
> @@ -233,6 +259,13 @@ static void __init ams_delta_init_irq(vo
>
> omap_usb_init(&ams_delta_usb_config);
> platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
> +
> + omap_cfg_reg(P20_1610_GPIO4); /* is this required? */
> +
> + /* The hook switch state could be exposed over sysfs with gpio_export().
> + * This should be done after the gpio-keys driver calls gpio_request(),
> + * but I don't know how to do this from outside of the driver. */
> + /* gpio_export(AMS_DELTA_GPIO_PIN_HOOK_SWITCH, 0); */
> }
>
> static void __init ams_delta_map_io(void)
J.
--
Revd. Jonathan McDowell, ULC | I don't sleep, I dream.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
