Hi Tony,
On Mon, Dec 31, 2012 at 11:01:02AM +1300, Tony Prisk wrote:
> This patch adds support for the Power Button keypad found on
> Wondermedia netbooks/tablets.
>
> A keymap property is exposed to allowing defining the key
> event to be generated when the power button is pressed.
>
> Signed-off-by: Tony Prisk <linux@xxxxxxxxxxxxxxx>
> ---
> .../bindings/input/vt8500-power-keypad.txt | 17 ++
> drivers/input/keyboard/Kconfig | 10 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/wmt_power_keypad.c | 174 ++++++++++++++++++++
> 4 files changed, 202 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/vt8500-power-keypad.txt
> create mode 100644 drivers/input/keyboard/wmt_power_keypad.c
>
> diff --git a/Documentation/devicetree/bindings/input/vt8500-power-keypad.txt b/Documentation/devicetree/bindings/input/vt8500-power-keypad.txt
> new file mode 100644
> index 0000000..bae6228
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/vt8500-power-keypad.txt
> @@ -0,0 +1,17 @@
> +* Wondermedia Power Keypad device tree bindings
> +
> +Wondermedia SoCs have a single irq-driven power button attached to
> +the power management controller.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> + - "wm,power-keypad": For all Wondermedia 8xxx-series SoCs.
> +- interrupts: should be the power management controller wakeup interrupt.
> +- keymap: linux keycode to generate when power button pressed.
> +
> +Example:
> + powerkey: pwrkey@0 {
> + compatible = "wm,power-keypad";
> + interrupts = <22>;
> + keymap = <116>; /* KEY_POWER */
Do we really need this in DT? I'd say just having it manageable from
userspace is enough.
> + };
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 5a240c6..c270f27 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -595,6 +595,16 @@ config KEYBOARD_TWL4030
> To compile this driver as a module, choose M here: the
> module will be called twl4030_keypad.
>
> +config KEYBOARD_WMT_POWER_KEYPAD
> + tristate "Wondermedia Power keypad support"
> + depends on ARCH_VT8500
I'd feel safer if we added "depends on OF" as well.
> + help
> + Say Y here to enable support for the power button keypad
> + on Wondermedia 8xxx-series SoCs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called wmt_power_keypad.
> +
> config KEYBOARD_XTKBD
> tristate "XT keyboard"
> select SERIO
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 44e7600..eea31af 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -52,5 +52,6 @@ obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> obj-$(CONFIG_KEYBOARD_TNETV107X) += tnetv107x-keypad.o
> obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o
> +obj-$(CONFIG_KEYBOARD_WMT_POWER_KEYPAD) += wmt_power_keypad.o
> obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o
> obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o
> diff --git a/drivers/input/keyboard/wmt_power_keypad.c b/drivers/input/keyboard/wmt_power_keypad.c
> new file mode 100644
> index 0000000..ce8aa9a
> --- /dev/null
> +++ b/drivers/input/keyboard/wmt_power_keypad.c
> @@ -0,0 +1,174 @@
> +/* Wondermedia Power Keypad
> + *
> + * Copyright (C) 2012 Tony Prisk <linux@xxxxxxxxxxxxxxx>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/bitops.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> +
> +static void __iomem *pmc_base;
> +static struct input_dev *kpad_power;
> +static spinlock_t kpad_power_lock;
> +static int power_button_pressed;
> +static struct timer_list kpad_power_timer;
> +static unsigned int kpad_power_code;
Maybe wrap it in a structure?
> +
> +static inline void kpad_power_timeout(unsigned long fcontext)
Why inline? It can't be inlined anyway since its address is used.
> +{
> + u32 status;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&kpad_power_lock, flags);
> +
> + status = readl(pmc_base + 0x14);
> +
> + if (power_button_pressed) {
This check is useless, you won't ever get here if button hasn't been
pressed.
> + input_report_key(kpad_power, kpad_power_code, 0);
> + input_sync(kpad_power);
> + power_button_pressed = 0;
> + }
> +
> + spin_unlock_irqrestore(&kpad_power_lock, flags);
> +}
> +
> +static irqreturn_t kpad_power_isr(int irq_num, void *data)
> +{
> + u32 status;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&kpad_power_lock, flags);
> +
> + status = readl(pmc_base + 0x14);
> + udelay(100);
> + writel(status, pmc_base + 0x14);
> +
> + if (status & BIT(14)) {
> + if (!power_button_pressed) {
No need to do this check.
> + input_report_key(kpad_power, kpad_power_code, 1);
> + input_sync(kpad_power);
> +
> + power_button_pressed = 1;
> +
> + mod_timer(&kpad_power_timer,
> + jiffies + msecs_to_jiffies(500));
> + }
> + }
> +
> + spin_unlock_irqrestore(&kpad_power_lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int vt8500_pwr_kpad_probe(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + u32 status;
> + int err;
> + int irq;
> +
> + np = of_find_compatible_node(NULL, NULL, "via,vt8500-pmc");
> + if (!np) {
> + dev_err(&pdev->dev, "pmc node not found\n");
> + return -EINVAL;
> + }
> +
> + pmc_base = of_iomap(np, 0);
> + if (!pmc_base) {
> + dev_err(&pdev->dev, "unable to map pmc memory\n");
> + return -ENOMEM;
> + }
> +
> + np = pdev->dev.of_node;
> + if (!np) {
> + dev_err(&pdev->dev, "devicenode not found\n");
> + return -ENODEV;
> + }
> +
> + err = of_property_read_u32(np, "keymap", &kpad_power_code);
> + if (err) {
> + dev_warn(&pdev->dev, "defaulting to KEY_POWER\n");
> + kpad_power_code = KEY_POWER;
> + }
> +
> + /* set power button to soft-power */
> + status = readl(pmc_base + 0x54);
> + writel(status | 1, pmc_base + 0x54);
> +
> + /* clear any pending interrupts */
> + status = readl(pmc_base + 0x14);
> + writel(status, pmc_base + 0x14);
> +
> + kpad_power = input_allocate_device();
> + if (!kpad_power) {
> + dev_err(&pdev->dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
> +
> + spin_lock_init(&kpad_power_lock);
> + setup_timer(&kpad_power_timer, kpad_power_timeout,
> + (unsigned long)kpad_power);
> +
> + irq = irq_of_parse_and_map(np, 0);
> + err = request_irq(irq, &kpad_power_isr, 0, "pwrbtn", NULL);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to request irq\n");
> + return err;
> + }
> +
> + kpad_power->evbit[0] = BIT_MASK(EV_KEY);
> + set_bit(kpad_power_code, kpad_power->keybit);
> +
> + kpad_power->name = "wmt_power_keypad";
> + kpad_power->phys = "wmt_power_keypad";
> + kpad_power->keycode = &kpad_power_code;
> + kpad_power->keycodesize = sizeof(unsigned int);
> + kpad_power->keycodemax = 1;
> +
> + err = input_register_device(kpad_power);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to register input device\n");
You either need to use managed resources or clean up after yourself;
leaking memory and other resources is not nice. Given that you are using
timer, which needs to be canceled, and I am not sure if your device
allows enabling/disabling generating interrupts while keeping the
interrupt handler registered, manual cleanup looks like the only option
for you.
BTW, where is your remove() method?
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static struct of_device_id vt8500_pwr_kpad_dt_ids[] = {
> + { .compatible = "wm,power-keypad" },
> + { /* Sentinel */ },
> +};
> +
> +static struct platform_driver vt8500_pwr_kpad_driver = {
> + .probe = vt8500_pwr_kpad_probe,
> + .driver = {
> + .name = "wmt-power-keypad",
> + .owner = THIS_MODULE,
> + .of_match_table = vt8500_pwr_kpad_dt_ids,
> + },
> +};
> +
> +module_platform_driver(vt8500_pwr_kpad_driver);
> +
> +MODULE_DESCRIPTION("Wondermedia Power Keypad Driver");
> +MODULE_AUTHOR("Tony Prisk <linux@xxxxxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DEVICE_TABLE(of, vt8500_pwr_kpad_dt_ids);
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
