| Message ID | 1431449438-19460-2-git-send-email-Frank.Li@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Robin, On Wed, May 13, 2015 at 12:50:37AM +0800, Frank.Li@freescale.com wrote: > From: Robin Gong <b38343@freescale.com> > > add snvs power key driver. > It work in imx chips after i.mx6sx > > Signed-off-by: Robin Gong <b38343@freescale.com> > Signed-off-by: Frank Li <Frank.Li@freescale.com> > --- > drivers/input/keyboard/Kconfig | 7 ++ > drivers/input/keyboard/Makefile | 1 + > drivers/input/keyboard/snvs_pwrkey.c | 226 +++++++++++++++++++++++++++++++++++ > 3 files changed, 234 insertions(+) > create mode 100644 drivers/input/keyboard/snvs_pwrkey.c > > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > index 106fbac..33c9bed 100644 > --- a/drivers/input/keyboard/Kconfig > +++ b/drivers/input/keyboard/Kconfig > @@ -400,6 +400,13 @@ config KEYBOARD_MPR121 > To compile this driver as a module, choose M here: the > module will be called mpr121_touchkey. > > +config KEYBOARD_SNVS_PWRKEY > + tristate "IMX6SX SNVS Power Key Driver" > + depends on SOC_IMX6SX Does SOC_IMX6SX depend on OF? Maybe have explicit dependency here? > + help > + This is the snvs powerkey driver for the Freescale i.MX6SX application > + processors. "To compile this driver as a module..." > + > config KEYBOARD_IMX > tristate "IMX keypad support" > depends on ARCH_MXC > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > index df28d55..1d416dd 100644 > --- a/drivers/input/keyboard/Makefile > +++ b/drivers/input/keyboard/Makefile > @@ -51,6 +51,7 @@ obj-$(CONFIG_KEYBOARD_QT1070) += qt1070.o > obj-$(CONFIG_KEYBOARD_QT2160) += qt2160.o > obj-$(CONFIG_KEYBOARD_SAMSUNG) += samsung-keypad.o > obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o > +obj-$(CONFIG_KEYBOARD_SNVS_PWRKEY) += snvs_pwrkey.o > obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o > obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o > obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o > diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c > new file mode 100644 > index 0000000..24abbb4 > --- /dev/null > +++ b/drivers/input/keyboard/snvs_pwrkey.c > @@ -0,0 +1,226 @@ > +/* > + * Copyright (C) 2011-2014 Freescale Semiconductor, Inc. All Rights Reserved. 2015? > + * > + * The code contained herein is licensed under the GNU General Public > + * License. You may obtain a copy of the GNU General Public License > + * Version 2 or later at the following locations: > + * > + * http://www.opensource.org/licenses/gpl-license.html > + * http://www.gnu.org/copyleft/gpl.html > + */ > + > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/input.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/jiffies.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > + > +#define SNVS_LPSR_REG 0x4C /* LP Status Register */ > +#define SNVS_LPCR_REG 0x38 /* LP Control Register */ > +#define SNVS_HPSR_REG 0x14 > +#define SNVS_HPSR_BTN (0x1 << 6) BIT(6)? > +#define SNVS_LPSR_SPO (0x1 << 18) BIT(18) > +#define SNVS_LPCR_DEP_EN (0x1 << 5) BIT(5) > + > +struct pwrkey_drv_data { > + void __iomem *ioaddr; > + int irq; > + int keycode; > + int keystate; /* 1:pressed */ > + int wakeup; > + struct timer_list check_timer; > + struct input_dev *input; > +}; > + > +static void imx_imx_snvs_check_for_events(unsigned long data) > +{ > + struct pwrkey_drv_data *pdata = (struct pwrkey_drv_data *) data; > + struct input_dev *input = pdata->input; > + void __iomem *ioaddr = pdata->ioaddr; > + u32 state; > + > + state = ((readl_relaxed(ioaddr + SNVS_HPSR_REG) & SNVS_HPSR_BTN) ? > + 1 : 0); > + > + /* only report new event if status changed */ > + if (state ^ pdata->keystate) { > + pdata->keystate = state; > + input_event(input, EV_KEY, pdata->keycode, state); > + input_sync(input); > + pm_relax(pdata->input->dev.parent); > + } > + > + /* repeat check if pressed long */ > + if (state) { > + mod_timer(&pdata->check_timer, > + jiffies + msecs_to_jiffies(60)); > + } > +} > + > +static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id) > +{ > + struct platform_device *pdev = dev_id; > + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); > + void __iomem *ioaddr = pdata->ioaddr; > + u32 lp_status; > + > + pm_wakeup_event(pdata->input->dev.parent, 0); > + lp_status = readl_relaxed(ioaddr + SNVS_LPSR_REG); > + if (lp_status & SNVS_LPSR_SPO) > + mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(2)); Why 2 and not 1 or 0? > + > + /* clear SPO status */ > + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG); > + > + return IRQ_HANDLED; > +} > + > +static int imx_snvs_pwrkey_probe(struct platform_device *pdev) > +{ > + struct pwrkey_drv_data *pdata = NULL; > + struct input_dev *input = NULL; > + struct device_node *np; > + void __iomem *ioaddr; > + u32 val; > + int ret = 0; > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return -ENOMEM; > + > + /* Get SNVS register Page */ > + np = of_find_compatible_node(NULL, NULL, "fsl,imx6sx-snvs-pwrkey"); Why such unorthodox way of getting node pointer? Why not use pdev->dev.of_node? > + if (!np) > + return -ENODEV; > + pdata->ioaddr = of_iomap(np, 0); I do not see you freeing this anywhere, not in error paths, nor in remove(). > + if (IS_ERR(pdata->ioaddr)) > + return PTR_ERR(pdata->ioaddr); > + > + if (of_property_read_u32(np, "fsl,keycode", &pdata->keycode)) { I think this should be "linux,keycode" > + pdata->keycode = KEY_POWER; > + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n"); > + } > + > + pdata->wakeup = !!of_get_property(np, "fsl,wakeup", NULL); Just use "wakeup" I guess. > + > + pdata->irq = platform_get_irq(pdev, 0); > + if (pdata->irq < 0) { > + dev_err(&pdev->dev, "no irq defined in platform data\n"); > + return -EINVAL; > + } > + > + ioaddr = pdata->ioaddr; > + val = readl_relaxed(ioaddr + SNVS_LPCR_REG); > + val |= SNVS_LPCR_DEP_EN, > + writel_relaxed(val, ioaddr + SNVS_LPCR_REG); > + /* clear the unexpected interrupt before driver ready */ > + val = readl_relaxed(ioaddr + SNVS_LPSR_REG); > + if (val & SNVS_LPSR_SPO) > + writel_relaxed(val | SNVS_LPSR_SPO, ioaddr + SNVS_LPSR_REG); > + > + setup_timer(&pdata->check_timer, > + imx_imx_snvs_check_for_events, (unsigned long) pdata); > + > + if (pdata->irq >= 0) { Why do we need this condition? You error out if irq < 0 above. > + ret = devm_request_irq(&pdev->dev, pdata->irq, > + imx_snvs_pwrkey_interrupt, > + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, pdev->name, pdev); Why IRQF_NO_SUSPEND? Also should we not get trigger type from OF data? > + if (ret) { > + dev_err(&pdev->dev, "interrupt not available.\n"); > + return ret; > + } > + } > + > + input = devm_input_allocate_device(&pdev->dev); > + if (!input) { > + dev_err(&pdev->dev, "failed to allocate the input device\n"); > + return -ENOMEM; > + } This is too late. Interrupt may come before you allocate input device and you will get kernel crash. > + > + input->name = pdev->name; > + input->phys = "snvs-pwrkey/input0"; > + input->id.bustype = BUS_HOST; > + input->evbit[0] = BIT_MASK(EV_KEY); > + > + input_set_capability(input, EV_KEY, pdata->keycode); > + > + ret = input_register_device(input); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to register input device\n"); > + input_free_device(input); > + return ret; > + } > + > + pdata->input = input; > + platform_set_drvdata(pdev, pdata); > + > + device_init_wakeup(&pdev->dev, pdata->wakeup); > + > + dev_info(&pdev->dev, "i.MX snvs powerkey probed\n"); > + > + return 0; > +} > + > +static int imx_snvs_pwrkey_remove(struct platform_device *pdev) > +{ > + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); > + > + input_unregister_device(pdata->input); > + del_timer_sync(&pdata->check_timer); This is racy. You unregister input device (which is likely to free it) and only then cancel timer and _then_ free IRQ. If timer or IRQ fire at wrong time you'll get crash. Consider using devm_input_allocate_device() and devm_add_action() to schedule del_timer_sync() after freeing interrupt and before input device is freed. Alternatively, is it possible to disable the device? Then implement open() and close() methods and call del_timer_sync() from close(). > + > + return 0; > +} > + > +static int imx_snvs_pwrkey_suspend(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); > + > + if (device_may_wakeup(&pdev->dev)) > + enable_irq_wake(pdata->irq); > + > + return 0; > +} > + > +static int imx_snvs_pwrkey_resume(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); > + > + if (device_may_wakeup(&pdev->dev)) > + disable_irq_wake(pdata->irq); > + > + return 0; > +} > + > +static const struct of_device_id imx_snvs_pwrkey_ids[] = { > + { .compatible = "fsl,imx6sx-snvs-pwrkey" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids); > + > +static SIMPLE_DEV_PM_OPS(imx_snvs_pwrkey_pm_ops, imx_snvs_pwrkey_suspend, > + imx_snvs_pwrkey_resume); > + > +static struct platform_driver imx_snvs_pwrkey_driver = { > + .driver = { > + .name = "snvs_pwrkey", > + .owner = THIS_MODULE, > + .pm = &imx_snvs_pwrkey_pm_ops, > + .of_match_table = imx_snvs_pwrkey_ids, > + }, > + .probe = imx_snvs_pwrkey_probe, > + .remove = imx_snvs_pwrkey_remove, > +}; > +module_platform_driver(imx_snvs_pwrkey_driver); > + > +MODULE_AUTHOR("Freescale Semiconductor"); > +MODULE_DESCRIPTION("i.MX snvs power key Driver"); > +MODULE_LICENSE("GPL"); > -- > 1.9.1 > Thanks.
>> + ret = devm_request_irq(&pdev->dev, pdata->irq, >> + imx_snvs_pwrkey_interrupt, >> + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, pdev->name, pdev); > > Why IRQF_NO_SUSPEND? Also should we not get trigger type from OF data? > wake up by PWRON key in freeze mode Do you have any example to get trigger type from OF data? best regards Frank Li
On Tue, May 12, 2015 at 03:37:41PM -0500, Zhi Li wrote: > >> + ret = devm_request_irq(&pdev->dev, pdata->irq, > >> + imx_snvs_pwrkey_interrupt, > >> + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, pdev->name, pdev); > > > > Why IRQF_NO_SUSPEND? Also should we not get trigger type from OF data? > > > wake up by PWRON key in freeze mode enable_irq_wake() should adjust the handler as needed, the driver should request flags that are needed for it's own operations. > > Do you have any example to get trigger type from OF data? You can retrieve the flags via irqd_get_trigger_type() for example, but you do not need to do that, because OF code will set up interrupt properly (based on DTS) when creating the corresponsing platform device (see of_irq_parse_and_map). so you just need to do: ret = devm_request_irq(&pdev->dev, pdata->irq, imx_snvs_pwrkey_interrupt, 0, pdev->name, pdev); Thanks.
>> +static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id) >> +{ >> + struct platform_device *pdev = dev_id; >> + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); >> + void __iomem *ioaddr = pdata->ioaddr; >> + u32 lp_status; >> + >> + pm_wakeup_event(pdata->input->dev.parent, 0); >> + lp_status = readl_relaxed(ioaddr + SNVS_LPSR_REG); >> + if (lp_status & SNVS_LPSR_SPO) >> + mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(2)); > > Why 2 and not 1 or 0? > I checked internal team about this. no special reason for that. Just do debounce. best regards Frank Li
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig index 106fbac..33c9bed 100644 --- a/drivers/input/keyboard/Kconfig +++ b/drivers/input/keyboard/Kconfig @@ -400,6 +400,13 @@ config KEYBOARD_MPR121 To compile this driver as a module, choose M here: the module will be called mpr121_touchkey. +config KEYBOARD_SNVS_PWRKEY + tristate "IMX6SX SNVS Power Key Driver" + depends on SOC_IMX6SX + help + This is the snvs powerkey driver for the Freescale i.MX6SX application + processors. + config KEYBOARD_IMX tristate "IMX keypad support" depends on ARCH_MXC diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile index df28d55..1d416dd 100644 --- a/drivers/input/keyboard/Makefile +++ b/drivers/input/keyboard/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_KEYBOARD_QT1070) += qt1070.o obj-$(CONFIG_KEYBOARD_QT2160) += qt2160.o obj-$(CONFIG_KEYBOARD_SAMSUNG) += samsung-keypad.o obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o +obj-$(CONFIG_KEYBOARD_SNVS_PWRKEY) += snvs_pwrkey.o obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c new file mode 100644 index 0000000..24abbb4 --- /dev/null +++ b/drivers/input/keyboard/snvs_pwrkey.c @@ -0,0 +1,226 @@ +/* + * Copyright (C) 2011-2014 Freescale Semiconductor, Inc. All Rights Reserved. + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/input.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/jiffies.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> + +#define SNVS_LPSR_REG 0x4C /* LP Status Register */ +#define SNVS_LPCR_REG 0x38 /* LP Control Register */ +#define SNVS_HPSR_REG 0x14 +#define SNVS_HPSR_BTN (0x1 << 6) +#define SNVS_LPSR_SPO (0x1 << 18) +#define SNVS_LPCR_DEP_EN (0x1 << 5) + +struct pwrkey_drv_data { + void __iomem *ioaddr; + int irq; + int keycode; + int keystate; /* 1:pressed */ + int wakeup; + struct timer_list check_timer; + struct input_dev *input; +}; + +static void imx_imx_snvs_check_for_events(unsigned long data) +{ + struct pwrkey_drv_data *pdata = (struct pwrkey_drv_data *) data; + struct input_dev *input = pdata->input; + void __iomem *ioaddr = pdata->ioaddr; + u32 state; + + state = ((readl_relaxed(ioaddr + SNVS_HPSR_REG) & SNVS_HPSR_BTN) ? + 1 : 0); + + /* only report new event if status changed */ + if (state ^ pdata->keystate) { + pdata->keystate = state; + input_event(input, EV_KEY, pdata->keycode, state); + input_sync(input); + pm_relax(pdata->input->dev.parent); + } + + /* repeat check if pressed long */ + if (state) { + mod_timer(&pdata->check_timer, + jiffies + msecs_to_jiffies(60)); + } +} + +static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id) +{ + struct platform_device *pdev = dev_id; + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); + void __iomem *ioaddr = pdata->ioaddr; + u32 lp_status; + + pm_wakeup_event(pdata->input->dev.parent, 0); + lp_status = readl_relaxed(ioaddr + SNVS_LPSR_REG); + if (lp_status & SNVS_LPSR_SPO) + mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(2)); + + /* clear SPO status */ + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG); + + return IRQ_HANDLED; +} + +static int imx_snvs_pwrkey_probe(struct platform_device *pdev) +{ + struct pwrkey_drv_data *pdata = NULL; + struct input_dev *input = NULL; + struct device_node *np; + void __iomem *ioaddr; + u32 val; + int ret = 0; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + /* Get SNVS register Page */ + np = of_find_compatible_node(NULL, NULL, "fsl,imx6sx-snvs-pwrkey"); + if (!np) + return -ENODEV; + pdata->ioaddr = of_iomap(np, 0); + if (IS_ERR(pdata->ioaddr)) + return PTR_ERR(pdata->ioaddr); + + if (of_property_read_u32(np, "fsl,keycode", &pdata->keycode)) { + pdata->keycode = KEY_POWER; + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n"); + } + + pdata->wakeup = !!of_get_property(np, "fsl,wakeup", NULL); + + pdata->irq = platform_get_irq(pdev, 0); + if (pdata->irq < 0) { + dev_err(&pdev->dev, "no irq defined in platform data\n"); + return -EINVAL; + } + + ioaddr = pdata->ioaddr; + val = readl_relaxed(ioaddr + SNVS_LPCR_REG); + val |= SNVS_LPCR_DEP_EN, + writel_relaxed(val, ioaddr + SNVS_LPCR_REG); + /* clear the unexpected interrupt before driver ready */ + val = readl_relaxed(ioaddr + SNVS_LPSR_REG); + if (val & SNVS_LPSR_SPO) + writel_relaxed(val | SNVS_LPSR_SPO, ioaddr + SNVS_LPSR_REG); + + setup_timer(&pdata->check_timer, + imx_imx_snvs_check_for_events, (unsigned long) pdata); + + if (pdata->irq >= 0) { + ret = devm_request_irq(&pdev->dev, pdata->irq, + imx_snvs_pwrkey_interrupt, + IRQF_TRIGGER_HIGH | IRQF_NO_SUSPEND, pdev->name, pdev); + if (ret) { + dev_err(&pdev->dev, "interrupt not available.\n"); + return ret; + } + } + + input = devm_input_allocate_device(&pdev->dev); + if (!input) { + dev_err(&pdev->dev, "failed to allocate the input device\n"); + return -ENOMEM; + } + + input->name = pdev->name; + input->phys = "snvs-pwrkey/input0"; + input->id.bustype = BUS_HOST; + input->evbit[0] = BIT_MASK(EV_KEY); + + input_set_capability(input, EV_KEY, pdata->keycode); + + ret = input_register_device(input); + if (ret < 0) { + dev_err(&pdev->dev, "failed to register input device\n"); + input_free_device(input); + return ret; + } + + pdata->input = input; + platform_set_drvdata(pdev, pdata); + + device_init_wakeup(&pdev->dev, pdata->wakeup); + + dev_info(&pdev->dev, "i.MX snvs powerkey probed\n"); + + return 0; +} + +static int imx_snvs_pwrkey_remove(struct platform_device *pdev) +{ + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); + + input_unregister_device(pdata->input); + del_timer_sync(&pdata->check_timer); + + return 0; +} + +static int imx_snvs_pwrkey_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); + + if (device_may_wakeup(&pdev->dev)) + enable_irq_wake(pdata->irq); + + return 0; +} + +static int imx_snvs_pwrkey_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev); + + if (device_may_wakeup(&pdev->dev)) + disable_irq_wake(pdata->irq); + + return 0; +} + +static const struct of_device_id imx_snvs_pwrkey_ids[] = { + { .compatible = "fsl,imx6sx-snvs-pwrkey" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids); + +static SIMPLE_DEV_PM_OPS(imx_snvs_pwrkey_pm_ops, imx_snvs_pwrkey_suspend, + imx_snvs_pwrkey_resume); + +static struct platform_driver imx_snvs_pwrkey_driver = { + .driver = { + .name = "snvs_pwrkey", + .owner = THIS_MODULE, + .pm = &imx_snvs_pwrkey_pm_ops, + .of_match_table = imx_snvs_pwrkey_ids, + }, + .probe = imx_snvs_pwrkey_probe, + .remove = imx_snvs_pwrkey_remove, +}; +module_platform_driver(imx_snvs_pwrkey_driver); + +MODULE_AUTHOR("Freescale Semiconductor"); +MODULE_DESCRIPTION("i.MX snvs power key Driver"); +MODULE_LICENSE("GPL");