* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
0 siblings, 0 replies; 22+ messages in thread
From: Frank.Li @ 2015-05-13 14:47 UTC (permalink / raw)
To: lznuaa, shawn.guo, dmitry.torokhov
Cc: linux-arm-kernel, linux-input, Robin Gong, Frank Li
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>
---
Change from V1 to V2
- Remove SOC_IMX6SX depend
- Add "TO compile the deiver as module.. "
- Change license 2015
- BIT(x)
- use pdev->dev.of_node
- iounmap at remove
- use linux,keycode
- use linux,wakeup
- Remove irq >=0 check at devm_regust_irq
- Remove IRQF_NO_SUSPEND
- Register irq after allocate device to avoid race condition
- add devm_add_action to remove del_timer_sync
drivers/input/keyboard/Kconfig | 9 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
3 files changed, 249 insertions(+)
create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 106fbac..6c1c7de 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
+ help
+ This is the snvs powerkey driver for the Freescale i.MX application
+ processors that are newer than i.MX6 SX.
+
+ To compile this driver as a module, choose M here; the
+ module will be called snvs_pwrkey.
+
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..e4c2de3
--- /dev/null
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2015 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 BIT(6)
+#define SNVS_LPSR_SPO BIT(18)
+#define SNVS_LPCR_DEP_EN 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));
+
+ /* clear SPO status */
+ writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
+
+ return IRQ_HANDLED;
+}
+
+static void imx_snvs_pwrkey_act(void *pdata)
+{
+ struct pwrkey_drv_data *pd = pdata;
+
+ del_timer_sync(&pd->check_timer);
+}
+
+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 = pdev->dev.of_node;
+ 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, "linux,keycode", &pdata->keycode)) {
+ pdata->keycode = KEY_POWER;
+ dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
+ }
+
+ pdata->wakeup = !!of_get_property(np, "linux,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);
+
+ 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);
+
+ /* input customer action to cancel release timer */
+ ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register remove action\n");
+ return ret;
+ }
+
+ ret = devm_request_irq(&pdev->dev, pdata->irq,
+ imx_snvs_pwrkey_interrupt,
+ 0, pdev->name, pdev);
+
+ if (ret) {
+ dev_err(&pdev->dev, "interrupt not available.\n");
+ return ret;
+ }
+
+ 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);
+ iounmap(pdata->ioaddr);
+
+ 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
0 siblings, 0 replies; 22+ messages in thread
From: Frank.Li at freescale.com @ 2015-05-13 14:47 UTC (permalink / raw)
To: linux-arm-kernel
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>
---
Change from V1 to V2
- Remove SOC_IMX6SX depend
- Add "TO compile the deiver as module.. "
- Change license 2015
- BIT(x)
- use pdev->dev.of_node
- iounmap at remove
- use linux,keycode
- use linux,wakeup
- Remove irq >=0 check at devm_regust_irq
- Remove IRQF_NO_SUSPEND
- Register irq after allocate device to avoid race condition
- add devm_add_action to remove del_timer_sync
drivers/input/keyboard/Kconfig | 9 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
3 files changed, 249 insertions(+)
create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 106fbac..6c1c7de 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
+ help
+ This is the snvs powerkey driver for the Freescale i.MX application
+ processors that are newer than i.MX6 SX.
+
+ To compile this driver as a module, choose M here; the
+ module will be called snvs_pwrkey.
+
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..e4c2de3
--- /dev/null
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2015 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 BIT(6)
+#define SNVS_LPSR_SPO BIT(18)
+#define SNVS_LPCR_DEP_EN 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));
+
+ /* clear SPO status */
+ writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
+
+ return IRQ_HANDLED;
+}
+
+static void imx_snvs_pwrkey_act(void *pdata)
+{
+ struct pwrkey_drv_data *pd = pdata;
+
+ del_timer_sync(&pd->check_timer);
+}
+
+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 = pdev->dev.of_node;
+ 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, "linux,keycode", &pdata->keycode)) {
+ pdata->keycode = KEY_POWER;
+ dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
+ }
+
+ pdata->wakeup = !!of_get_property(np, "linux,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);
+
+ 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);
+
+ /* input customer action to cancel release timer */
+ ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register remove action\n");
+ return ret;
+ }
+
+ ret = devm_request_irq(&pdev->dev, pdata->irq,
+ imx_snvs_pwrkey_interrupt,
+ 0, pdev->name, pdev);
+
+ if (ret) {
+ dev_err(&pdev->dev, "interrupt not available.\n");
+ return ret;
+ }
+
+ 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);
+ iounmap(pdata->ioaddr);
+
+ 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] document: devicetree: input: imx: i.mx snvs power device tree bindings
2015-05-13 14:47 ` Frank.Li at freescale.com
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
-1 siblings, 0 replies; 22+ messages in thread
From: Frank.Li @ 2015-05-13 14:47 UTC (permalink / raw)
To: lznuaa, shawn.guo, dmitry.torokhov
Cc: linux-arm-kernel, linux-input, Frank Li, Robin Gong
From: Frank Li <Frank.Li@freescale.com>
The snvs-pwrkey is designed to enable POWER key function which controlled
by SNVS ONOFF. the driver can report the status of POWER key and wakeup
system if pressed after system suspend.
Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Robin Gong <b38343@freescale.com>
---
.../devicetree/bindings/input/snvs-pwrkey.txt | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/snvs-pwrkey.txt
diff --git a/Documentation/devicetree/bindings/input/snvs-pwrkey.txt b/Documentation/devicetree/bindings/input/snvs-pwrkey.txt
new file mode 100644
index 0000000..9b3b58d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/snvs-pwrkey.txt
@@ -0,0 +1,26 @@
+* Freescale i.MX SNVS powerkey device tree bindings
+
+The snvs-pwrkey is designed to enable POWER key function which controlled
+by SNVS ONOFF, the driver can report the status of POWER key and wakeup
+system if pressed after system suspend.
+
+Required SoC Specific Properties:
+- compatible: Should be "fsl,imx6sx-snvs-pwrkey".
+
+- reg: Physical base address of the SNVS and length of memory mapped
+ region.
+
+- interrupts: The SNVS interrupt number to the CPU(s).
+
+- linux,keycode: Keycode to emit, KEY_POWER by default.
+
+- linux,wakeup: Button can wake-up the system
+
+Example:
+snvs-pwrkey@0x020cc000 {
+ compatible = "fsl,imx6sx-snvs-pwrkey";
+ reg = <0x020cc000 0x4000>;
+ interrupts = <0 4 0x4>;
+ linux,keycode = <116>; /* KEY_POWER */
+ linux,wakeup;
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] document: devicetree: input: imx: i.mx snvs power device tree bindings
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
0 siblings, 0 replies; 22+ messages in thread
From: Frank.Li at freescale.com @ 2015-05-13 14:47 UTC (permalink / raw)
To: linux-arm-kernel
From: Frank Li <Frank.Li@freescale.com>
The snvs-pwrkey is designed to enable POWER key function which controlled
by SNVS ONOFF. the driver can report the status of POWER key and wakeup
system if pressed after system suspend.
Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Robin Gong <b38343@freescale.com>
---
.../devicetree/bindings/input/snvs-pwrkey.txt | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/snvs-pwrkey.txt
diff --git a/Documentation/devicetree/bindings/input/snvs-pwrkey.txt b/Documentation/devicetree/bindings/input/snvs-pwrkey.txt
new file mode 100644
index 0000000..9b3b58d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/snvs-pwrkey.txt
@@ -0,0 +1,26 @@
+* Freescale i.MX SNVS powerkey device tree bindings
+
+The snvs-pwrkey is designed to enable POWER key function which controlled
+by SNVS ONOFF, the driver can report the status of POWER key and wakeup
+system if pressed after system suspend.
+
+Required SoC Specific Properties:
+- compatible: Should be "fsl,imx6sx-snvs-pwrkey".
+
+- reg: Physical base address of the SNVS and length of memory mapped
+ region.
+
+- interrupts: The SNVS interrupt number to the CPU(s).
+
+- linux,keycode: Keycode to emit, KEY_POWER by default.
+
+- linux,wakeup: Button can wake-up the system
+
+Example:
+snvs-pwrkey at 0x020cc000 {
+ compatible = "fsl,imx6sx-snvs-pwrkey";
+ reg = <0x020cc000 0x4000>;
+ interrupts = <0 4 0x4>;
+ linux,keycode = <116>; /* KEY_POWER */
+ linux,wakeup;
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] arm: dts: imx6sx: enable snvs power key
2015-05-13 14:47 ` Frank.Li at freescale.com
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
-1 siblings, 0 replies; 22+ messages in thread
From: Frank.Li @ 2015-05-13 14:47 UTC (permalink / raw)
To: lznuaa, shawn.guo, dmitry.torokhov
Cc: linux-arm-kernel, linux-input, Frank Li
From: Frank Li <Frank.Li@freescale.com>
enable snvs ONOFF power key support
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
arch/arm/boot/dts/imx6sx.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 708175d..f531d18 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -680,6 +680,14 @@
};
};
+ snvs-pwrkey@0x020cc000 {
+ compatible = "fsl,imx6sx-snvs-pwrkey";
+ reg = <0x020cc000 0x4000>;
+ interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+ linux,keycode = <116>; /* KEY_POWER */
+ linux,wakeup;
+ };
+
epit1: epit@020d0000 {
reg = <0x020d0000 0x4000>;
interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] arm: dts: imx6sx: enable snvs power key
@ 2015-05-13 14:47 ` Frank.Li at freescale.com
0 siblings, 0 replies; 22+ messages in thread
From: Frank.Li at freescale.com @ 2015-05-13 14:47 UTC (permalink / raw)
To: linux-arm-kernel
From: Frank Li <Frank.Li@freescale.com>
enable snvs ONOFF power key support
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
arch/arm/boot/dts/imx6sx.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 708175d..f531d18 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -680,6 +680,14 @@
};
};
+ snvs-pwrkey at 0x020cc000 {
+ compatible = "fsl,imx6sx-snvs-pwrkey";
+ reg = <0x020cc000 0x4000>;
+ interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+ linux,keycode = <116>; /* KEY_POWER */
+ linux,wakeup;
+ };
+
epit1: epit at 020d0000 {
reg = <0x020d0000 0x4000>;
interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 14:47 ` Frank.Li at freescale.com
@ 2015-05-13 16:40 ` Dmitry Torokhov
-1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 16:40 UTC (permalink / raw)
To: Frank.Li; +Cc: lznuaa, shawn.guo, linux-arm-kernel, linux-input, Robin Gong
Hi Frank,
On Wed, May 13, 2015 at 10:47:40PM +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>
> ---
> Change from V1 to V2
> - Remove SOC_IMX6SX depend
No, I did not mean to remove SOC_IMX6SX completely. I was wondering if
we should also add "depends on OF". Even if we don't add OF dpeendency I
think we should have
depends on SOC_IMX6SX || COMPILE_TEST
> - Add "TO compile the deiver as module.. "
> - Change license 2015
> - BIT(x)
> - use pdev->dev.of_node
> - iounmap at remove
> - use linux,keycode
> - use linux,wakeup
> - Remove irq >=0 check at devm_regust_irq
> - Remove IRQF_NO_SUSPEND
> - Register irq after allocate device to avoid race condition
> - add devm_add_action to remove del_timer_sync
>
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
> 3 files changed, 249 insertions(+)
> create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 106fbac..6c1c7de 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
> + help
> + This is the snvs powerkey driver for the Freescale i.MX application
> + processors that are newer than i.MX6 SX.
> +
> + To compile this driver as a module, choose M here; the
> + module will be called snvs_pwrkey.
> +
> 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..e4c2de3
> --- /dev/null
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -0,0 +1,239 @@
> +/*
> + * Copyright (C) 2015 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 BIT(6)
> +#define SNVS_LPSR_SPO BIT(18)
> +#define SNVS_LPCR_DEP_EN 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));
#define for 60 please.
> + }
> +}
> +
> +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));
Hmm, 2 msec is kind of low for debouncing I think. In any case please
add a #define for it.
> +
> + /* clear SPO status */
> + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void imx_snvs_pwrkey_act(void *pdata)
> +{
> + struct pwrkey_drv_data *pd = pdata;
> +
> + del_timer_sync(&pd->check_timer);
> +}
> +
> +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;
Presonal preference - can we please call this variable "error"? And you
do not need to initialize it.
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + /* Get SNVS register Page */
> + np = pdev->dev.of_node;
> + if (!np)
> + return -ENODEV;
We should check it before trying to allocate memory.
> + pdata->ioaddr = of_iomap(np, 0);
> + if (IS_ERR(pdata->ioaddr))
> + return PTR_ERR(pdata->ioaddr);
Umm, you are still leaking it on error path. Can you try doing:
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(pdata->ioaddr))
return PTR_ERR(pdata->ioaddr);
> +
> + if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
> + pdata->keycode = KEY_POWER;
> + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
> + }
> +
> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
Just "wakeup", unlike "keycode" it is not linux-specific property, you
want wakeup on all OSes (or you don't).
> +
> + 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);
> +
> + 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);
No need to explicitly set EV_KEY bit, in newer kernels
input_set_capability() does this for you.
> +
> + input_set_capability(input, EV_KEY, pdata->keycode);
> +
> + /* input customer action to cancel release timer */
> + ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register remove action\n");
> + return ret;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, pdata->irq,
> + imx_snvs_pwrkey_interrupt,
> + 0, pdev->name, pdev);
> +
> + if (ret) {
> + dev_err(&pdev->dev, "interrupt not available.\n");
> + return ret;
> + }
> +
> + 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");
Drop this, input core will print message for the device.
> +
> + 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);
Not needed with devm-managed input device.
> + iounmap(pdata->ioaddr);
If you switch to devm_ioremap_resource you can remove
imx_snvs_pwrkey_remove() altogether.
> +
> + 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,
No need to set the owner, we do this automatically.
> + .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.
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 16:40 ` Dmitry Torokhov
0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Frank,
On Wed, May 13, 2015 at 10:47:40PM +0800, Frank.Li at 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>
> ---
> Change from V1 to V2
> - Remove SOC_IMX6SX depend
No, I did not mean to remove SOC_IMX6SX completely. I was wondering if
we should also add "depends on OF". Even if we don't add OF dpeendency I
think we should have
depends on SOC_IMX6SX || COMPILE_TEST
> - Add "TO compile the deiver as module.. "
> - Change license 2015
> - BIT(x)
> - use pdev->dev.of_node
> - iounmap at remove
> - use linux,keycode
> - use linux,wakeup
> - Remove irq >=0 check at devm_regust_irq
> - Remove IRQF_NO_SUSPEND
> - Register irq after allocate device to avoid race condition
> - add devm_add_action to remove del_timer_sync
>
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
> 3 files changed, 249 insertions(+)
> create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 106fbac..6c1c7de 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
> + help
> + This is the snvs powerkey driver for the Freescale i.MX application
> + processors that are newer than i.MX6 SX.
> +
> + To compile this driver as a module, choose M here; the
> + module will be called snvs_pwrkey.
> +
> 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..e4c2de3
> --- /dev/null
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -0,0 +1,239 @@
> +/*
> + * Copyright (C) 2015 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 BIT(6)
> +#define SNVS_LPSR_SPO BIT(18)
> +#define SNVS_LPCR_DEP_EN 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));
#define for 60 please.
> + }
> +}
> +
> +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));
Hmm, 2 msec is kind of low for debouncing I think. In any case please
add a #define for it.
> +
> + /* clear SPO status */
> + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void imx_snvs_pwrkey_act(void *pdata)
> +{
> + struct pwrkey_drv_data *pd = pdata;
> +
> + del_timer_sync(&pd->check_timer);
> +}
> +
> +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;
Presonal preference - can we please call this variable "error"? And you
do not need to initialize it.
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + /* Get SNVS register Page */
> + np = pdev->dev.of_node;
> + if (!np)
> + return -ENODEV;
We should check it before trying to allocate memory.
> + pdata->ioaddr = of_iomap(np, 0);
> + if (IS_ERR(pdata->ioaddr))
> + return PTR_ERR(pdata->ioaddr);
Umm, you are still leaking it on error path. Can you try doing:
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(pdata->ioaddr))
return PTR_ERR(pdata->ioaddr);
> +
> + if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
> + pdata->keycode = KEY_POWER;
> + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
> + }
> +
> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
Just "wakeup", unlike "keycode" it is not linux-specific property, you
want wakeup on all OSes (or you don't).
> +
> + 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);
> +
> + 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);
No need to explicitly set EV_KEY bit, in newer kernels
input_set_capability() does this for you.
> +
> + input_set_capability(input, EV_KEY, pdata->keycode);
> +
> + /* input customer action to cancel release timer */
> + ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register remove action\n");
> + return ret;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, pdata->irq,
> + imx_snvs_pwrkey_interrupt,
> + 0, pdev->name, pdev);
> +
> + if (ret) {
> + dev_err(&pdev->dev, "interrupt not available.\n");
> + return ret;
> + }
> +
> + 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");
Drop this, input core will print message for the device.
> +
> + 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);
Not needed with devm-managed input device.
> + iounmap(pdata->ioaddr);
If you switch to devm_ioremap_resource you can remove
imx_snvs_pwrkey_remove() altogether.
> +
> + 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,
No need to set the owner, we do this automatically.
> + .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.
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 14:47 ` Frank.Li at freescale.com
@ 2015-05-13 17:29 ` Stefan Wahren
-1 siblings, 0 replies; 22+ messages in thread
From: Stefan Wahren @ 2015-05-13 17:29 UTC (permalink / raw)
To: Frank.Li, lznuaa
Cc: linux-input, Robin Gong, linux-arm-kernel, shawn.guo, dmitry.torokhov
Hi Frank,
looks like a typo in your subject line. keyboard?
> Frank.Li@freescale.com hat am 13. Mai 2015 um 16:47 geschrieben:
>
> [...]
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c
> b/drivers/input/keyboard/snvs_pwrkey.c
> new file mode 100644
> index 0000000..e4c2de3
> --- /dev/null
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -0,0 +1,239 @@
> +/*
a small driver description here would be nice.
> + * Copyright (C) 2015 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
> + */
> +
> [...]
> +static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> +{
> [...]
> +
> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
of_property_read_bool() ?
> +
> [...]
> +
> + ret = input_register_device(input);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + input_free_device(input);
input_free_device() shouldn't be necessary since you are using
devm_input_allocate_device().
Thanks
Best regards
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 17:29 ` Stefan Wahren
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Wahren @ 2015-05-13 17:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Frank,
looks like a typo in your subject line. keyboard?
> Frank.Li at freescale.com hat am 13. Mai 2015 um 16:47 geschrieben:
>
> [...]
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c
> b/drivers/input/keyboard/snvs_pwrkey.c
> new file mode 100644
> index 0000000..e4c2de3
> --- /dev/null
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -0,0 +1,239 @@
> +/*
a small driver description here would be nice.
> + * Copyright (C) 2015 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
> + */
> +
> [...]
> +static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> +{
> [...]
> +
> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
of_property_read_bool() ?
> +
> [...]
> +
> + ret = input_register_device(input);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + input_free_device(input);
input_free_device() shouldn't be necessary since you are using
devm_input_allocate_device().
Thanks
Best regards
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 16:40 ` Dmitry Torokhov
@ 2015-05-13 18:27 ` Zhi Li
-1 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 18:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Frank.Li, Shawn Guo, linux-arm-kernel, linux-input, Robin Gong
On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Frank,
>
> On Wed, May 13, 2015 at 10:47:40PM +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>
>> ---
>> Change from V1 to V2
>> - Remove SOC_IMX6SX depend
>
> No, I did not mean to remove SOC_IMX6SX completely. I was wondering if
> we should also add "depends on OF". Even if we don't add OF dpeendency I
> think we should have
>
> depends on SOC_IMX6SX || COMPILE_TEST
>
>> - Add "TO compile the deiver as module.. "
>> - Change license 2015
>> - BIT(x)
>> - use pdev->dev.of_node
>> - iounmap at remove
>> - use linux,keycode
>> - use linux,wakeup
>> - Remove irq >=0 check at devm_regust_irq
>> - Remove IRQF_NO_SUSPEND
>> - Register irq after allocate device to avoid race condition
>> - add devm_add_action to remove del_timer_sync
>>
>> drivers/input/keyboard/Kconfig | 9 ++
>> drivers/input/keyboard/Makefile | 1 +
>> drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 249 insertions(+)
>> create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
>>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index 106fbac..6c1c7de 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
>> + help
>> + This is the snvs powerkey driver for the Freescale i.MX application
>> + processors that are newer than i.MX6 SX.
>> +
>> + To compile this driver as a module, choose M here; the
>> + module will be called snvs_pwrkey.
>> +
>> 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..e4c2de3
>> --- /dev/null
>> +++ b/drivers/input/keyboard/snvs_pwrkey.c
>> @@ -0,0 +1,239 @@
>> +/*
>> + * Copyright (C) 2015 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 BIT(6)
>> +#define SNVS_LPSR_SPO BIT(18)
>> +#define SNVS_LPCR_DEP_EN 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));
>
> #define for 60 please.
>
>> + }
>> +}
>> +
>> +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));
>
> Hmm, 2 msec is kind of low for debouncing I think. In any case please
> add a #define for it.
>
>> +
>> + /* clear SPO status */
>> + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void imx_snvs_pwrkey_act(void *pdata)
>> +{
>> + struct pwrkey_drv_data *pd = pdata;
>> +
>> + del_timer_sync(&pd->check_timer);
>> +}
>> +
>> +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;
>
> Presonal preference - can we please call this variable "error"? And you
> do not need to initialize it.
>
>> +
>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata)
>> + return -ENOMEM;
>> +
>> + /* Get SNVS register Page */
>> + np = pdev->dev.of_node;
>> + if (!np)
>> + return -ENODEV;
>
> We should check it before trying to allocate memory.
>
>> + pdata->ioaddr = of_iomap(np, 0);
>> + if (IS_ERR(pdata->ioaddr))
>> + return PTR_ERR(pdata->ioaddr);
>
> Umm, you are still leaking it on error path. Can you try doing:
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(pdata->ioaddr))
> return PTR_ERR(pdata->ioaddr);
>
Thank you for comments.
but I don't use of_iomap here because the resource is shared with the
other device.
SNVS included a rtc, a power off, ON/OFF key.
If I use platform_get_resource and devm_ioremap_resource, rtc driver
fail to probe.
best regards
Frank Li
>> +
>> + if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
>> + pdata->keycode = KEY_POWER;
>> + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
>> + }
>> +
>> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
>
> Just "wakeup", unlike "keycode" it is not linux-specific property, you
> want wakeup on all OSes (or you don't).
>
>> +
>> + 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);
>> +
>> + 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);
>
> No need to explicitly set EV_KEY bit, in newer kernels
> input_set_capability() does this for you.
>
>> +
>> + input_set_capability(input, EV_KEY, pdata->keycode);
>> +
>> + /* input customer action to cancel release timer */
>> + ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to register remove action\n");
>> + return ret;
>> + }
>> +
>> + ret = devm_request_irq(&pdev->dev, pdata->irq,
>> + imx_snvs_pwrkey_interrupt,
>> + 0, pdev->name, pdev);
>> +
>> + if (ret) {
>> + dev_err(&pdev->dev, "interrupt not available.\n");
>> + return ret;
>> + }
>> +
>> + 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");
>
> Drop this, input core will print message for the device.
>
>> +
>> + 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);
>
> Not needed with devm-managed input device.
>
>> + iounmap(pdata->ioaddr);
>
> If you switch to devm_ioremap_resource you can remove
> imx_snvs_pwrkey_remove() altogether.
>
>> +
>> + 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,
>
> No need to set the owner, we do this automatically.
>
>> + .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.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 18:27 ` Zhi Li
0 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 18:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Frank,
>
> On Wed, May 13, 2015 at 10:47:40PM +0800, Frank.Li at 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>
>> ---
>> Change from V1 to V2
>> - Remove SOC_IMX6SX depend
>
> No, I did not mean to remove SOC_IMX6SX completely. I was wondering if
> we should also add "depends on OF". Even if we don't add OF dpeendency I
> think we should have
>
> depends on SOC_IMX6SX || COMPILE_TEST
>
>> - Add "TO compile the deiver as module.. "
>> - Change license 2015
>> - BIT(x)
>> - use pdev->dev.of_node
>> - iounmap at remove
>> - use linux,keycode
>> - use linux,wakeup
>> - Remove irq >=0 check at devm_regust_irq
>> - Remove IRQF_NO_SUSPEND
>> - Register irq after allocate device to avoid race condition
>> - add devm_add_action to remove del_timer_sync
>>
>> drivers/input/keyboard/Kconfig | 9 ++
>> drivers/input/keyboard/Makefile | 1 +
>> drivers/input/keyboard/snvs_pwrkey.c | 239 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 249 insertions(+)
>> create mode 100644 drivers/input/keyboard/snvs_pwrkey.c
>>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index 106fbac..6c1c7de 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -400,6 +400,15 @@ 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 "IMX SNVS Power Key Driver"
>> + help
>> + This is the snvs powerkey driver for the Freescale i.MX application
>> + processors that are newer than i.MX6 SX.
>> +
>> + To compile this driver as a module, choose M here; the
>> + module will be called snvs_pwrkey.
>> +
>> 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..e4c2de3
>> --- /dev/null
>> +++ b/drivers/input/keyboard/snvs_pwrkey.c
>> @@ -0,0 +1,239 @@
>> +/*
>> + * Copyright (C) 2015 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 BIT(6)
>> +#define SNVS_LPSR_SPO BIT(18)
>> +#define SNVS_LPCR_DEP_EN 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));
>
> #define for 60 please.
>
>> + }
>> +}
>> +
>> +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));
>
> Hmm, 2 msec is kind of low for debouncing I think. In any case please
> add a #define for it.
>
>> +
>> + /* clear SPO status */
>> + writel_relaxed(lp_status, ioaddr + SNVS_LPSR_REG);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void imx_snvs_pwrkey_act(void *pdata)
>> +{
>> + struct pwrkey_drv_data *pd = pdata;
>> +
>> + del_timer_sync(&pd->check_timer);
>> +}
>> +
>> +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;
>
> Presonal preference - can we please call this variable "error"? And you
> do not need to initialize it.
>
>> +
>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata)
>> + return -ENOMEM;
>> +
>> + /* Get SNVS register Page */
>> + np = pdev->dev.of_node;
>> + if (!np)
>> + return -ENODEV;
>
> We should check it before trying to allocate memory.
>
>> + pdata->ioaddr = of_iomap(np, 0);
>> + if (IS_ERR(pdata->ioaddr))
>> + return PTR_ERR(pdata->ioaddr);
>
> Umm, you are still leaking it on error path. Can you try doing:
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(pdata->ioaddr))
> return PTR_ERR(pdata->ioaddr);
>
Thank you for comments.
but I don't use of_iomap here because the resource is shared with the
other device.
SNVS included a rtc, a power off, ON/OFF key.
If I use platform_get_resource and devm_ioremap_resource, rtc driver
fail to probe.
best regards
Frank Li
>> +
>> + if (of_property_read_u32(np, "linux,keycode", &pdata->keycode)) {
>> + pdata->keycode = KEY_POWER;
>> + dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
>> + }
>> +
>> + pdata->wakeup = !!of_get_property(np, "linux,wakeup", NULL);
>
> Just "wakeup", unlike "keycode" it is not linux-specific property, you
> want wakeup on all OSes (or you don't).
>
>> +
>> + 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);
>> +
>> + 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);
>
> No need to explicitly set EV_KEY bit, in newer kernels
> input_set_capability() does this for you.
>
>> +
>> + input_set_capability(input, EV_KEY, pdata->keycode);
>> +
>> + /* input customer action to cancel release timer */
>> + ret = devm_add_action(&pdev->dev, imx_snvs_pwrkey_act, pdata);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to register remove action\n");
>> + return ret;
>> + }
>> +
>> + ret = devm_request_irq(&pdev->dev, pdata->irq,
>> + imx_snvs_pwrkey_interrupt,
>> + 0, pdev->name, pdev);
>> +
>> + if (ret) {
>> + dev_err(&pdev->dev, "interrupt not available.\n");
>> + return ret;
>> + }
>> +
>> + 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");
>
> Drop this, input core will print message for the device.
>
>> +
>> + 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);
>
> Not needed with devm-managed input device.
>
>> + iounmap(pdata->ioaddr);
>
> If you switch to devm_ioremap_resource you can remove
> imx_snvs_pwrkey_remove() altogether.
>
>> +
>> + 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,
>
> No need to set the owner, we do this automatically.
>
>> + .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.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 18:27 ` Zhi Li
@ 2015-05-13 19:10 ` Joshua Clayton
-1 siblings, 0 replies; 22+ messages in thread
From: Joshua Clayton @ 2015-05-13 19:10 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Zhi Li, Dmitry Torokhov, Robin Gong, Shawn Guo, Frank.Li, linux-input
Hi Frank.
On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Frank,
> >
> >
> >> + pdata->ioaddr = of_iomap(np, 0);
> >> + if (IS_ERR(pdata->ioaddr))
> >> + return PTR_ERR(pdata->ioaddr);
> >
> > Umm, you are still leaking it on error path. Can you try doing:
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> > if (IS_ERR(pdata->ioaddr))
> >
> > return PTR_ERR(pdata->ioaddr);
>
> Thank you for comments.
> but I don't use of_iomap here because the resource is shared with the
> other device.
>
> SNVS included a rtc, a power off, ON/OFF key.
> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> fail to probe.
>
> best regards
> Frank Li
>
The fact that you cannot use of_iomap() seems a clear warning of unsafe
access.
Since it is a shared resource, which you are writing to as well as reading,
perhaps you should set it up for safe access using the regmap API like the
GPR registers.
/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
--
~Joshua Clayton
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 19:10 ` Joshua Clayton
0 siblings, 0 replies; 22+ messages in thread
From: Joshua Clayton @ 2015-05-13 19:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Frank.
On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Frank,
> >
> >
> >> + pdata->ioaddr = of_iomap(np, 0);
> >> + if (IS_ERR(pdata->ioaddr))
> >> + return PTR_ERR(pdata->ioaddr);
> >
> > Umm, you are still leaking it on error path. Can you try doing:
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> > if (IS_ERR(pdata->ioaddr))
> >
> > return PTR_ERR(pdata->ioaddr);
>
> Thank you for comments.
> but I don't use of_iomap here because the resource is shared with the
> other device.
>
> SNVS included a rtc, a power off, ON/OFF key.
> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> fail to probe.
>
> best regards
> Frank Li
>
The fact that you cannot use of_iomap() seems a clear warning of unsafe
access.
Since it is a shared resource, which you are writing to as well as reading,
perhaps you should set it up for safe access using the regmap API like the
GPR registers.
/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
--
~Joshua Clayton
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 19:10 ` Joshua Clayton
@ 2015-05-13 19:27 ` Zhi Li
-1 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 19:27 UTC (permalink / raw)
To: Joshua Clayton
Cc: linux-arm-kernel, Dmitry Torokhov, Robin Gong, Shawn Guo,
Frank.Li, linux-input
On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
<stillcompiling@gmail.com> wrote:
>
> Hi Frank.
>
> On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
>> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>>
>> <dmitry.torokhov@gmail.com> wrote:
>> > Hi Frank,
>> >
>
>> >
>> >> + pdata->ioaddr = of_iomap(np, 0);
>> >> + if (IS_ERR(pdata->ioaddr))
>> >> + return PTR_ERR(pdata->ioaddr);
>> >
>> > Umm, you are still leaking it on error path. Can you try doing:
>> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
>> > if (IS_ERR(pdata->ioaddr))
>> >
>> > return PTR_ERR(pdata->ioaddr);
>>
>> Thank you for comments.
>> but I don't use of_iomap here because the resource is shared with the
>> other device.
>>
>> SNVS included a rtc, a power off, ON/OFF key.
>> If I use platform_get_resource and devm_ioremap_resource, rtc driver
>> fail to probe.
>>
>> best regards
>> Frank Li
>>
>
> The fact that you cannot use of_iomap() seems a clear warning of unsafe
> access.
> Since it is a shared resource, which you are writing to as well as reading,
> perhaps you should set it up for safe access using the regmap API like the
> GPR registers.
> /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
I read driver again clearfully. It is safed shared with rtc driver directly.
Because pwr key driver just read a irq status register and other one
register, which used only in this driver.
Just w1c (write 1 clear) register. it is atomic access by hardware.
PowerKey use difference irq number with RTC part.
>
> --
> ~Joshua Clayton
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 19:27 ` Zhi Li
0 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 19:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
<stillcompiling@gmail.com> wrote:
>
> Hi Frank.
>
> On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
>> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>>
>> <dmitry.torokhov@gmail.com> wrote:
>> > Hi Frank,
>> >
>
>> >
>> >> + pdata->ioaddr = of_iomap(np, 0);
>> >> + if (IS_ERR(pdata->ioaddr))
>> >> + return PTR_ERR(pdata->ioaddr);
>> >
>> > Umm, you are still leaking it on error path. Can you try doing:
>> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
>> > if (IS_ERR(pdata->ioaddr))
>> >
>> > return PTR_ERR(pdata->ioaddr);
>>
>> Thank you for comments.
>> but I don't use of_iomap here because the resource is shared with the
>> other device.
>>
>> SNVS included a rtc, a power off, ON/OFF key.
>> If I use platform_get_resource and devm_ioremap_resource, rtc driver
>> fail to probe.
>>
>> best regards
>> Frank Li
>>
>
> The fact that you cannot use of_iomap() seems a clear warning of unsafe
> access.
> Since it is a shared resource, which you are writing to as well as reading,
> perhaps you should set it up for safe access using the regmap API like the
> GPR registers.
> /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
I read driver again clearfully. It is safed shared with rtc driver directly.
Because pwr key driver just read a irq status register and other one
register, which used only in this driver.
Just w1c (write 1 clear) register. it is atomic access by hardware.
PowerKey use difference irq number with RTC part.
>
> --
> ~Joshua Clayton
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 19:27 ` Zhi Li
@ 2015-05-13 19:44 ` Dmitry Torokhov
-1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 19:44 UTC (permalink / raw)
To: Zhi Li
Cc: Joshua Clayton, linux-arm-kernel, Robin Gong, Shawn Guo,
Frank.Li, linux-input
On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
> <stillcompiling@gmail.com> wrote:
> >
> > Hi Frank.
> >
> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
> >>
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > Hi Frank,
> >> >
> >
> >> >
> >> >> + pdata->ioaddr = of_iomap(np, 0);
> >> >> + if (IS_ERR(pdata->ioaddr))
> >> >> + return PTR_ERR(pdata->ioaddr);
> >> >
> >> > Umm, you are still leaking it on error path. Can you try doing:
> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> >> > if (IS_ERR(pdata->ioaddr))
> >> >
> >> > return PTR_ERR(pdata->ioaddr);
> >>
> >> Thank you for comments.
> >> but I don't use of_iomap here because the resource is shared with the
> >> other device.
> >>
> >> SNVS included a rtc, a power off, ON/OFF key.
> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> >> fail to probe.
> >>
> >> best regards
> >> Frank Li
> >>
> >
> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
> > access.
> > Since it is a shared resource, which you are writing to as well as reading,
> > perhaps you should set it up for safe access using the regmap API like the
> > GPR registers.
> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
>
> I read driver again clearfully. It is safed shared with rtc driver directly.
> Because pwr key driver just read a irq status register and other one
> register, which used only in this driver.
>
> Just w1c (write 1 clear) register. it is atomic access by hardware.
It does not really matter, driver has to claim resources it is using to
make sure other parties are not accessing its registers. If registers
are shared you need to have a 3rd party mediator.
>
> PowerKey use difference irq number with RTC part.
Yes and so they can run simultaneously and step on each other's feet,
right?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 19:44 ` Dmitry Torokhov
0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 19:44 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
> <stillcompiling@gmail.com> wrote:
> >
> > Hi Frank.
> >
> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
> >>
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > Hi Frank,
> >> >
> >
> >> >
> >> >> + pdata->ioaddr = of_iomap(np, 0);
> >> >> + if (IS_ERR(pdata->ioaddr))
> >> >> + return PTR_ERR(pdata->ioaddr);
> >> >
> >> > Umm, you are still leaking it on error path. Can you try doing:
> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> >> > if (IS_ERR(pdata->ioaddr))
> >> >
> >> > return PTR_ERR(pdata->ioaddr);
> >>
> >> Thank you for comments.
> >> but I don't use of_iomap here because the resource is shared with the
> >> other device.
> >>
> >> SNVS included a rtc, a power off, ON/OFF key.
> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> >> fail to probe.
> >>
> >> best regards
> >> Frank Li
> >>
> >
> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
> > access.
> > Since it is a shared resource, which you are writing to as well as reading,
> > perhaps you should set it up for safe access using the regmap API like the
> > GPR registers.
> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
>
> I read driver again clearfully. It is safed shared with rtc driver directly.
> Because pwr key driver just read a irq status register and other one
> register, which used only in this driver.
>
> Just w1c (write 1 clear) register. it is atomic access by hardware.
It does not really matter, driver has to claim resources it is using to
make sure other parties are not accessing its registers. If registers
are shared you need to have a 3rd party mediator.
>
> PowerKey use difference irq number with RTC part.
Yes and so they can run simultaneously and step on each other's feet,
right?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 19:44 ` Dmitry Torokhov
@ 2015-05-13 19:57 ` Zhi Li
-1 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 19:57 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Joshua Clayton, linux-arm-kernel, Robin Gong, Shawn Guo,
Frank.Li, linux-input
On Wed, May 13, 2015 at 2:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
>> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
>> <stillcompiling@gmail.com> wrote:
>> >
>> > Hi Frank.
>> >
>> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
>> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>> >>
>> >> <dmitry.torokhov@gmail.com> wrote:
>> >> > Hi Frank,
>> >> >
>> >
>> >> >
>> >> >> + pdata->ioaddr = of_iomap(np, 0);
>> >> >> + if (IS_ERR(pdata->ioaddr))
>> >> >> + return PTR_ERR(pdata->ioaddr);
>> >> >
>> >> > Umm, you are still leaking it on error path. Can you try doing:
>> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
>> >> > if (IS_ERR(pdata->ioaddr))
>> >> >
>> >> > return PTR_ERR(pdata->ioaddr);
>> >>
>> >> Thank you for comments.
>> >> but I don't use of_iomap here because the resource is shared with the
>> >> other device.
>> >>
>> >> SNVS included a rtc, a power off, ON/OFF key.
>> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
>> >> fail to probe.
>> >>
>> >> best regards
>> >> Frank Li
>> >>
>> >
>> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
>> > access.
>> > Since it is a shared resource, which you are writing to as well as reading,
>> > perhaps you should set it up for safe access using the regmap API like the
>> > GPR registers.
>> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
>>
>> I read driver again clearfully. It is safed shared with rtc driver directly.
>> Because pwr key driver just read a irq status register and other one
>> register, which used only in this driver.
>>
>> Just w1c (write 1 clear) register. it is atomic access by hardware.
>
> It does not really matter, driver has to claim resources it is using to
> make sure other parties are not accessing its registers. If registers
> are shared you need to have a 3rd party mediator.
I can change to use syscon.
but other two driver(rtc and power off) need change also.
>
>>
>> PowerKey use difference irq number with RTC part.
>
> Yes and so they can run simultaneously and step on each other's feet,
> right?
Yes.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 19:57 ` Zhi Li
0 siblings, 0 replies; 22+ messages in thread
From: Zhi Li @ 2015-05-13 19:57 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 13, 2015 at 2:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
>> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
>> <stillcompiling@gmail.com> wrote:
>> >
>> > Hi Frank.
>> >
>> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
>> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
>> >>
>> >> <dmitry.torokhov@gmail.com> wrote:
>> >> > Hi Frank,
>> >> >
>> >
>> >> >
>> >> >> + pdata->ioaddr = of_iomap(np, 0);
>> >> >> + if (IS_ERR(pdata->ioaddr))
>> >> >> + return PTR_ERR(pdata->ioaddr);
>> >> >
>> >> > Umm, you are still leaking it on error path. Can you try doing:
>> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
>> >> > if (IS_ERR(pdata->ioaddr))
>> >> >
>> >> > return PTR_ERR(pdata->ioaddr);
>> >>
>> >> Thank you for comments.
>> >> but I don't use of_iomap here because the resource is shared with the
>> >> other device.
>> >>
>> >> SNVS included a rtc, a power off, ON/OFF key.
>> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
>> >> fail to probe.
>> >>
>> >> best regards
>> >> Frank Li
>> >>
>> >
>> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
>> > access.
>> > Since it is a shared resource, which you are writing to as well as reading,
>> > perhaps you should set it up for safe access using the regmap API like the
>> > GPR registers.
>> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
>>
>> I read driver again clearfully. It is safed shared with rtc driver directly.
>> Because pwr key driver just read a irq status register and other one
>> register, which used only in this driver.
>>
>> Just w1c (write 1 clear) register. it is atomic access by hardware.
>
> It does not really matter, driver has to claim resources it is using to
> make sure other parties are not accessing its registers. If registers
> are shared you need to have a 3rd party mediator.
I can change to use syscon.
but other two driver(rtc and power off) need change also.
>
>>
>> PowerKey use difference irq number with RTC part.
>
> Yes and so they can run simultaneously and step on each other's feet,
> right?
Yes.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
2015-05-13 19:57 ` Zhi Li
@ 2015-05-13 20:14 ` Dmitry Torokhov
-1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 20:14 UTC (permalink / raw)
To: Zhi Li
Cc: Joshua Clayton, linux-arm-kernel, Robin Gong, Shawn Guo,
Frank.Li, linux-input
On Wed, May 13, 2015 at 02:57:00PM -0500, Zhi Li wrote:
> On Wed, May 13, 2015 at 2:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
> >> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
> >> <stillcompiling@gmail.com> wrote:
> >> >
> >> > Hi Frank.
> >> >
> >> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> >> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
> >> >>
> >> >> <dmitry.torokhov@gmail.com> wrote:
> >> >> > Hi Frank,
> >> >> >
> >> >
> >> >> >
> >> >> >> + pdata->ioaddr = of_iomap(np, 0);
> >> >> >> + if (IS_ERR(pdata->ioaddr))
> >> >> >> + return PTR_ERR(pdata->ioaddr);
> >> >> >
> >> >> > Umm, you are still leaking it on error path. Can you try doing:
> >> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> >> >> > if (IS_ERR(pdata->ioaddr))
> >> >> >
> >> >> > return PTR_ERR(pdata->ioaddr);
> >> >>
> >> >> Thank you for comments.
> >> >> but I don't use of_iomap here because the resource is shared with the
> >> >> other device.
> >> >>
> >> >> SNVS included a rtc, a power off, ON/OFF key.
> >> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> >> >> fail to probe.
> >> >>
> >> >> best regards
> >> >> Frank Li
> >> >>
> >> >
> >> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
> >> > access.
> >> > Since it is a shared resource, which you are writing to as well as reading,
> >> > perhaps you should set it up for safe access using the regmap API like the
> >> > GPR registers.
> >> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
> >>
> >> I read driver again clearfully. It is safed shared with rtc driver directly.
> >> Because pwr key driver just read a irq status register and other one
> >> register, which used only in this driver.
> >>
> >> Just w1c (write 1 clear) register. it is atomic access by hardware.
> >
> > It does not really matter, driver has to claim resources it is using to
> > make sure other parties are not accessing its registers. If registers
> > are shared you need to have a 3rd party mediator.
>
> I can change to use syscon.
> but other two driver(rtc and power off) need change also.
Makes sense.
>
> >
> >>
> >> PowerKey use difference irq number with RTC part.
> >
> > Yes and so they can run simultaneously and step on each other's feet,
> > right?
>
> Yes.
>
> >
> > Thanks.
> >
> > --
> > Dmitry
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver
@ 2015-05-13 20:14 ` Dmitry Torokhov
0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Torokhov @ 2015-05-13 20:14 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 13, 2015 at 02:57:00PM -0500, Zhi Li wrote:
> On Wed, May 13, 2015 at 2:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote:
> >> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton
> >> <stillcompiling@gmail.com> wrote:
> >> >
> >> > Hi Frank.
> >> >
> >> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote:
> >> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov
> >> >>
> >> >> <dmitry.torokhov@gmail.com> wrote:
> >> >> > Hi Frank,
> >> >> >
> >> >
> >> >> >
> >> >> >> + pdata->ioaddr = of_iomap(np, 0);
> >> >> >> + if (IS_ERR(pdata->ioaddr))
> >> >> >> + return PTR_ERR(pdata->ioaddr);
> >> >> >
> >> >> > Umm, you are still leaking it on error path. Can you try doing:
> >> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> >> >> > if (IS_ERR(pdata->ioaddr))
> >> >> >
> >> >> > return PTR_ERR(pdata->ioaddr);
> >> >>
> >> >> Thank you for comments.
> >> >> but I don't use of_iomap here because the resource is shared with the
> >> >> other device.
> >> >>
> >> >> SNVS included a rtc, a power off, ON/OFF key.
> >> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver
> >> >> fail to probe.
> >> >>
> >> >> best regards
> >> >> Frank Li
> >> >>
> >> >
> >> > The fact that you cannot use of_iomap() seems a clear warning of unsafe
> >> > access.
> >> > Since it is a shared resource, which you are writing to as well as reading,
> >> > perhaps you should set it up for safe access using the regmap API like the
> >> > GPR registers.
> >> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
> >>
> >> I read driver again clearfully. It is safed shared with rtc driver directly.
> >> Because pwr key driver just read a irq status register and other one
> >> register, which used only in this driver.
> >>
> >> Just w1c (write 1 clear) register. it is atomic access by hardware.
> >
> > It does not really matter, driver has to claim resources it is using to
> > make sure other parties are not accessing its registers. If registers
> > are shared you need to have a 3rd party mediator.
>
> I can change to use syscon.
> but other two driver(rtc and power off) need change also.
Makes sense.
>
> >
> >>
> >> PowerKey use difference irq number with RTC part.
> >
> > Yes and so they can run simultaneously and step on each other's feet,
> > right?
>
> Yes.
>
> >
> > Thanks.
> >
> > --
> > Dmitry
--
Dmitry
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-05-13 20:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13 14:47 [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver Frank.Li
2015-05-13 14:47 ` Frank.Li at freescale.com
2015-05-13 14:47 ` [PATCH v2 2/3] document: devicetree: input: imx: i.mx snvs power device tree bindings Frank.Li
2015-05-13 14:47 ` Frank.Li at freescale.com
2015-05-13 14:47 ` [PATCH v2 3/3] arm: dts: imx6sx: enable snvs power key Frank.Li
2015-05-13 14:47 ` Frank.Li at freescale.com
2015-05-13 16:40 ` [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver Dmitry Torokhov
2015-05-13 16:40 ` Dmitry Torokhov
2015-05-13 18:27 ` Zhi Li
2015-05-13 18:27 ` Zhi Li
2015-05-13 19:10 ` Joshua Clayton
2015-05-13 19:10 ` Joshua Clayton
2015-05-13 19:27 ` Zhi Li
2015-05-13 19:27 ` Zhi Li
2015-05-13 19:44 ` Dmitry Torokhov
2015-05-13 19:44 ` Dmitry Torokhov
2015-05-13 19:57 ` Zhi Li
2015-05-13 19:57 ` Zhi Li
2015-05-13 20:14 ` Dmitry Torokhov
2015-05-13 20:14 ` Dmitry Torokhov
2015-05-13 17:29 ` Stefan Wahren
2015-05-13 17:29 ` Stefan Wahren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.