* [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding @ 2019-09-16 2:52 Anson Huang 2019-09-16 2:52 ` [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support Anson Huang ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Anson Huang @ 2019-09-16 2:52 UTC (permalink / raw) To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, dmitry.torokhov, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input Cc: Linux-imx NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system controller, the system controller is in charge of system power, clock and scu key event etc. management, Linux kernel has to communicate with system controller via MU (message unit) IPC to get scu key event, add binding doc for i.MX system controller key driver. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- No changes. --- .../devicetree/bindings/arm/freescale/fsl,scu.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt index c149fad..5eab7d0 100644 --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt @@ -157,6 +157,15 @@ Required properties: Optional properties: - timeout-sec: contains the watchdog timeout in seconds. +SCU key bindings based on SCU Message Protocol +------------------------------------------------------------ + +Required properties: +- compatible: should be: + "fsl,imx8qxp-sc-key" + followed by "fsl,imx-sc-key"; +- linux,keycodes: See Documentation/devicetree/bindings/input/keys.txt + Example (imx8qxp): ------------- aliases { @@ -220,6 +229,11 @@ firmware { compatible = "fsl,imx8qxp-sc-rtc"; }; + scu_key: scu-key { + compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; + linux,keycode = <KEY_POWER>; + }; + watchdog { compatible = "fsl,imx8qxp-sc-wdt", "fsl,imx-sc-wdt"; timeout-sec = <60>; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support 2019-09-16 2:52 [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding Anson Huang @ 2019-09-16 2:52 ` Anson Huang 2019-09-16 23:53 ` Dmitry Torokhov 2019-09-16 2:52 ` [PATCH V4 3/5] arm64: dts: imx8qxp: Add scu key node Anson Huang ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Anson Huang @ 2019-09-16 2:52 UTC (permalink / raw) To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, dmitry.torokhov, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input Cc: Linux-imx i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller inside, the system controller is in charge of controlling power, clock and scu key etc.. Adds i.MX system controller key driver support, Linux kernel has to communicate with system controller via MU (message unit) IPC to get scu key's status. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- Changes since V3: - switch the debounce and repeat interval time for delay work schdule; - add .remove to handle group irq and notify etc.. --- drivers/input/keyboard/Kconfig | 7 ++ drivers/input/keyboard/Makefile | 1 + drivers/input/keyboard/imx_sc_key.c | 190 ++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 drivers/input/keyboard/imx_sc_key.c diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig index 8911bc2..00f8428 100644 --- a/drivers/input/keyboard/Kconfig +++ b/drivers/input/keyboard/Kconfig @@ -469,6 +469,13 @@ config KEYBOARD_IMX To compile this driver as a module, choose M here: the module will be called imx_keypad. +config KEYBOARD_IMX_SC_KEY + tristate "IMX SCU Key Driver" + depends on IMX_SCU + help + This is the system controller key driver for NXP i.MX SoCs with + system controller inside. + config KEYBOARD_NEWTON tristate "Newton keyboard" select SERIO diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile index 9510325..f5b1752 100644 --- a/drivers/input/keyboard/Makefile +++ b/drivers/input/keyboard/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) += hil_kbd.o obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o +obj-$(CONFIG_KEYBOARD_IMX_SC_KEY) += imx_sc_key.o obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o diff --git a/drivers/input/keyboard/imx_sc_key.c b/drivers/input/keyboard/imx_sc_key.c new file mode 100644 index 0000000..59c68fa --- /dev/null +++ b/drivers/input/keyboard/imx_sc_key.c @@ -0,0 +1,190 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2019 NXP. + */ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/firmware/imx/sci.h> +#include <linux/init.h> +#include <linux/input.h> +#include <linux/interrupt.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 DEBOUNCE_TIME 100 +#define REPEAT_INTERVAL 60 + +#define SC_IRQ_BUTTON 1 +#define SC_IRQ_GROUP_WAKE 3 +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18 + +struct imx_key_drv_data { + int keycode; + bool keystate; /* 1: pressed, 0: release */ + bool delay_check; + struct delayed_work check_work; + struct input_dev *input; + struct imx_sc_ipc *key_ipc_handle; + struct notifier_block key_notifier; +}; + +struct imx_sc_msg_key { + struct imx_sc_rpc_msg hdr; + u8 state; +}; + +static int imx_sc_key_notify(struct notifier_block *nb, + unsigned long event, void *group) +{ + struct imx_key_drv_data *priv = + container_of(nb, + struct imx_key_drv_data, + key_notifier); + + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group == SC_IRQ_GROUP_WAKE) + && !priv->delay_check) { + priv->delay_check = 1; + schedule_delayed_work(&priv->check_work, + msecs_to_jiffies(DEBOUNCE_TIME)); + } + + return 0; +} + +static void imx_sc_check_for_events(struct work_struct *work) +{ + struct imx_key_drv_data *priv = + container_of(work, + struct imx_key_drv_data, + check_work.work); + struct input_dev *input = priv->input; + struct imx_sc_msg_key msg; + struct imx_sc_rpc_msg *hdr = &msg.hdr; + bool state; + int ret; + + hdr->ver = IMX_SC_RPC_VERSION; + hdr->svc = IMX_SC_RPC_SVC_MISC; + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS; + hdr->size = 1; + + ret = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true); + if (ret) { + dev_err(&input->dev, "read imx sc key failed, ret %d\n", ret); + return; + } + + state = (bool)msg.state; + + if (!state && !priv->keystate) + state = true; + + if (state ^ priv->keystate) { + pm_wakeup_event(input->dev.parent, 0); + priv->keystate = state; + input_event(input, EV_KEY, priv->keycode, state); + input_sync(input); + if (!state) + priv->delay_check = 0; + pm_relax(priv->input->dev.parent); + } + + if (state) + schedule_delayed_work(&priv->check_work, + msecs_to_jiffies(REPEAT_INTERVAL)); +} + +static int imx_sc_key_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + static struct imx_key_drv_data *priv; + struct input_dev *input; + int ret; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + ret = imx_scu_get_handle(&priv->key_ipc_handle); + if (ret) + return ret; + + if (of_property_read_u32(np, "linux,keycode", &priv->keycode)) { + dev_err(&pdev->dev, "missing KEY_POWER in DT\n"); + return -EINVAL; + } + + INIT_DELAYED_WORK(&priv->check_work, imx_sc_check_for_events); + + 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 = "imx-sc-key/input0"; + input->id.bustype = BUS_HOST; + + input_set_capability(input, EV_KEY, priv->keycode); + + ret = input_register_device(input); + if (ret) { + dev_err(&pdev->dev, "failed to register input device\n"); + return ret; + } + + priv->input = input; + platform_set_drvdata(pdev, priv); + + ret = imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, true); + if (ret) { + dev_warn(&pdev->dev, "enable scu group irq failed\n"); + return ret; + } + + priv->key_notifier.notifier_call = imx_sc_key_notify; + ret = imx_scu_irq_register_notifier(&priv->key_notifier); + if (ret) { + imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, false); + dev_warn(&pdev->dev, "register scu notifier failed\n"); + } + + return ret; +} + +static int imx_sc_key_remove(struct platform_device *pdev) +{ + struct imx_key_drv_data *priv = platform_get_drvdata(pdev); + + imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, false); + imx_scu_irq_unregister_notifier(&priv->key_notifier); + cancel_delayed_work_sync(&priv->check_work); + + return 0; +} + +static const struct of_device_id imx_sc_key_ids[] = { + { .compatible = "fsl,imx-sc-key" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, imx_sc_key_ids); + +static struct platform_driver imx_sc_key_driver = { + .driver = { + .name = "imx-sc-key", + .of_match_table = imx_sc_key_ids, + }, + .probe = imx_sc_key_probe, + .remove = imx_sc_key_remove, +}; +module_platform_driver(imx_sc_key_driver); + +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>"); +MODULE_DESCRIPTION("i.MX System Controller Key Driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support 2019-09-16 2:52 ` [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support Anson Huang @ 2019-09-16 23:53 ` Dmitry Torokhov 2019-09-17 2:42 ` Anson Huang 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2019-09-16 23:53 UTC (permalink / raw) To: Anson Huang Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input, Linux-imx On Mon, Sep 16, 2019 at 10:52:50AM +0800, Anson Huang wrote: > i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller > inside, the system controller is in charge of controlling power, > clock and scu key etc.. > > Adds i.MX system controller key driver support, Linux kernel has > to communicate with system controller via MU (message unit) IPC > to get scu key's status. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > Changes since V3: > - switch the debounce and repeat interval time for delay work schdule; > - add .remove to handle group irq and notify etc.. > --- > drivers/input/keyboard/Kconfig | 7 ++ > drivers/input/keyboard/Makefile | 1 + > drivers/input/keyboard/imx_sc_key.c | 190 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 198 insertions(+) > create mode 100644 drivers/input/keyboard/imx_sc_key.c > > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > index 8911bc2..00f8428 100644 > --- a/drivers/input/keyboard/Kconfig > +++ b/drivers/input/keyboard/Kconfig > @@ -469,6 +469,13 @@ config KEYBOARD_IMX > To compile this driver as a module, choose M here: the > module will be called imx_keypad. > > +config KEYBOARD_IMX_SC_KEY > + tristate "IMX SCU Key Driver" > + depends on IMX_SCU > + help > + This is the system controller key driver for NXP i.MX SoCs with > + system controller inside. > + > config KEYBOARD_NEWTON > tristate "Newton keyboard" > select SERIO > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > index 9510325..f5b1752 100644 > --- a/drivers/input/keyboard/Makefile > +++ b/drivers/input/keyboard/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) += hil_kbd.o > obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o > obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o > obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o > +obj-$(CONFIG_KEYBOARD_IMX_SC_KEY) += imx_sc_key.o > obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o > obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o > obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o > diff --git a/drivers/input/keyboard/imx_sc_key.c b/drivers/input/keyboard/imx_sc_key.c > new file mode 100644 > index 0000000..59c68fa > --- /dev/null > +++ b/drivers/input/keyboard/imx_sc_key.c > @@ -0,0 +1,190 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2019 NXP. > + */ > + > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/firmware/imx/sci.h> > +#include <linux/init.h> > +#include <linux/input.h> > +#include <linux/interrupt.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 DEBOUNCE_TIME 100 > +#define REPEAT_INTERVAL 60 > + > +#define SC_IRQ_BUTTON 1 > +#define SC_IRQ_GROUP_WAKE 3 > +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18 > + > +struct imx_key_drv_data { > + int keycode; > + bool keystate; /* 1: pressed, 0: release */ > + bool delay_check; > + struct delayed_work check_work; > + struct input_dev *input; > + struct imx_sc_ipc *key_ipc_handle; > + struct notifier_block key_notifier; > +}; > + > +struct imx_sc_msg_key { > + struct imx_sc_rpc_msg hdr; > + u8 state; > +}; > + > +static int imx_sc_key_notify(struct notifier_block *nb, > + unsigned long event, void *group) > +{ > + struct imx_key_drv_data *priv = > + container_of(nb, > + struct imx_key_drv_data, > + key_notifier); > + > + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group == SC_IRQ_GROUP_WAKE) > + && !priv->delay_check) { > + priv->delay_check = 1; > + schedule_delayed_work(&priv->check_work, > + msecs_to_jiffies(DEBOUNCE_TIME)); If I am reading this right, you are trying to avoid scheduling the work again if it is already scheduled. You do not need to do that, as schedule_delayed_work() will take care of that (if you want to make sure the work is re-scheduled with updated expiration, you need to use mod_delayed_work). > + } > + > + return 0; > +} > + > +static void imx_sc_check_for_events(struct work_struct *work) > +{ > + struct imx_key_drv_data *priv = > + container_of(work, > + struct imx_key_drv_data, > + check_work.work); > + struct input_dev *input = priv->input; > + struct imx_sc_msg_key msg; > + struct imx_sc_rpc_msg *hdr = &msg.hdr; > + bool state; > + int ret; > + > + hdr->ver = IMX_SC_RPC_VERSION; > + hdr->svc = IMX_SC_RPC_SVC_MISC; > + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS; > + hdr->size = 1; > + > + ret = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true); > + if (ret) { > + dev_err(&input->dev, "read imx sc key failed, ret %d\n", ret); > + return; > + } > + > + state = (bool)msg.state; > + > + if (!state && !priv->keystate) > + state = true; This needs an explanation please. > + > + if (state ^ priv->keystate) { > + pm_wakeup_event(input->dev.parent, 0); I'd expect this happening in imx_sc_key_notify() so that the device would have a change to run this work. > + priv->keystate = state; > + input_event(input, EV_KEY, priv->keycode, state); > + input_sync(input); > + if (!state) > + priv->delay_check = 0; > + pm_relax(priv->input->dev.parent); Are you sure you want to call pm_relax() unconditionally, and not when state == false (i.e. button released)? > + } > + > + if (state) > + schedule_delayed_work(&priv->check_work, > + msecs_to_jiffies(REPEAT_INTERVAL)); > +} > + > +static int imx_sc_key_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + static struct imx_key_drv_data *priv; > + struct input_dev *input; > + int ret; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + ret = imx_scu_get_handle(&priv->key_ipc_handle); > + if (ret) > + return ret; > + > + if (of_property_read_u32(np, "linux,keycode", &priv->keycode)) { > + dev_err(&pdev->dev, "missing KEY_POWER in DT\n"); > + return -EINVAL; > + } > + > + INIT_DELAYED_WORK(&priv->check_work, imx_sc_check_for_events); > + > + 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 = "imx-sc-key/input0"; > + input->id.bustype = BUS_HOST; > + > + input_set_capability(input, EV_KEY, priv->keycode); > + > + ret = input_register_device(input); > + if (ret) { Could you please rename this (and elsewhere) from 'ret' to 'error'? > + dev_err(&pdev->dev, "failed to register input device\n"); > + return ret; > + } > + > + priv->input = input; > + platform_set_drvdata(pdev, priv); > + > + ret = imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, true); > + if (ret) { > + dev_warn(&pdev->dev, "enable scu group irq failed\n"); > + return ret; > + } > + > + priv->key_notifier.notifier_call = imx_sc_key_notify; > + ret = imx_scu_irq_register_notifier(&priv->key_notifier); > + if (ret) { > + imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, false); > + dev_warn(&pdev->dev, "register scu notifier failed\n"); return error; > + } > + > + return ret; return 0; > +} > + > +static int imx_sc_key_remove(struct platform_device *pdev) > +{ > + struct imx_key_drv_data *priv = platform_get_drvdata(pdev); > + > + imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, SC_IRQ_BUTTON, false); > + imx_scu_irq_unregister_notifier(&priv->key_notifier); > + cancel_delayed_work_sync(&priv->check_work); > + > + return 0; > +} > + > +static const struct of_device_id imx_sc_key_ids[] = { > + { .compatible = "fsl,imx-sc-key" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imx_sc_key_ids); > + > +static struct platform_driver imx_sc_key_driver = { > + .driver = { > + .name = "imx-sc-key", > + .of_match_table = imx_sc_key_ids, > + }, > + .probe = imx_sc_key_probe, > + .remove = imx_sc_key_remove, > +}; > +module_platform_driver(imx_sc_key_driver); > + > +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>"); > +MODULE_DESCRIPTION("i.MX System Controller Key Driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support 2019-09-16 23:53 ` Dmitry Torokhov @ 2019-09-17 2:42 ` Anson Huang 2019-09-22 16:14 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Anson Huang @ 2019-09-17 2:42 UTC (permalink / raw) To: Dmitry Torokhov Cc: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, Aisheng Dong, ulf.hansson, Andy Duan, Peng Fan, Leonard Crestez, Daniel Baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, Jacky Bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input, dl-linux-imx Hi, Dmitry > On Mon, Sep 16, 2019 at 10:52:50AM +0800, Anson Huang wrote: > > i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller > > inside, the system controller is in charge of controlling power, clock > > and scu key etc.. > > > > Adds i.MX system controller key driver support, Linux kernel has to > > communicate with system controller via MU (message unit) IPC to get > > scu key's status. > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > --- > > Changes since V3: > > - switch the debounce and repeat interval time for delay work > schdule; > > - add .remove to handle group irq and notify etc.. > > --- > > drivers/input/keyboard/Kconfig | 7 ++ > > drivers/input/keyboard/Makefile | 1 + > > drivers/input/keyboard/imx_sc_key.c | 190 > > ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 198 insertions(+) > > create mode 100644 drivers/input/keyboard/imx_sc_key.c > > > > diff --git a/drivers/input/keyboard/Kconfig > > b/drivers/input/keyboard/Kconfig index 8911bc2..00f8428 100644 > > --- a/drivers/input/keyboard/Kconfig > > +++ b/drivers/input/keyboard/Kconfig > > @@ -469,6 +469,13 @@ config KEYBOARD_IMX > > To compile this driver as a module, choose M here: the > > module will be called imx_keypad. > > > > +config KEYBOARD_IMX_SC_KEY > > + tristate "IMX SCU Key Driver" > > + depends on IMX_SCU > > + help > > + This is the system controller key driver for NXP i.MX SoCs with > > + system controller inside. > > + > > config KEYBOARD_NEWTON > > tristate "Newton keyboard" > > select SERIO > > diff --git a/drivers/input/keyboard/Makefile > > b/drivers/input/keyboard/Makefile index 9510325..f5b1752 100644 > > --- a/drivers/input/keyboard/Makefile > > +++ b/drivers/input/keyboard/Makefile > > @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) += hil_kbd.o > > obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o > > obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o > > obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o > > +obj-$(CONFIG_KEYBOARD_IMX_SC_KEY) += imx_sc_key.o > > obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o > > obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o > > obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o > > diff --git a/drivers/input/keyboard/imx_sc_key.c > > b/drivers/input/keyboard/imx_sc_key.c > > new file mode 100644 > > index 0000000..59c68fa > > --- /dev/null > > +++ b/drivers/input/keyboard/imx_sc_key.c > > @@ -0,0 +1,190 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright 2019 NXP. > > + */ > > + > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/firmware/imx/sci.h> > > +#include <linux/init.h> > > +#include <linux/input.h> > > +#include <linux/interrupt.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 DEBOUNCE_TIME 100 > > +#define REPEAT_INTERVAL 60 > > + > > +#define SC_IRQ_BUTTON 1 > > +#define SC_IRQ_GROUP_WAKE 3 > > +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18 > > + > > +struct imx_key_drv_data { > > + int keycode; > > + bool keystate; /* 1: pressed, 0: release */ > > + bool delay_check; > > + struct delayed_work check_work; > > + struct input_dev *input; > > + struct imx_sc_ipc *key_ipc_handle; > > + struct notifier_block key_notifier; > > +}; > > + > > +struct imx_sc_msg_key { > > + struct imx_sc_rpc_msg hdr; > > + u8 state; > > +}; > > + > > +static int imx_sc_key_notify(struct notifier_block *nb, > > + unsigned long event, void *group) { > > + struct imx_key_drv_data *priv = > > + container_of(nb, > > + struct imx_key_drv_data, > > + key_notifier); > > + > > + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group == > SC_IRQ_GROUP_WAKE) > > + && !priv->delay_check) { > > + priv->delay_check = 1; > > + schedule_delayed_work(&priv->check_work, > > + msecs_to_jiffies(DEBOUNCE_TIME)); > > If I am reading this right, you are trying to avoid scheduling the work again if > it is already scheduled. You do not need to do that, as > schedule_delayed_work() will take care of that (if you want to make sure the > work is re-scheduled with updated expiration, you need to use > mod_delayed_work). The original code here is to skip the event of button release, try to ONLY handle the press interrupt, the release event is handled by busy loop check, but I double check the SCU FW's behavior, it does NOT send out interrupt for button release event at all, so it is NOT needed any more, I will remove the priv->delay_check completely in this driver. > > > + } > > + > > + return 0; > > +} > > + > > +static void imx_sc_check_for_events(struct work_struct *work) { > > + struct imx_key_drv_data *priv = > > + container_of(work, > > + struct imx_key_drv_data, > > + check_work.work); > > + struct input_dev *input = priv->input; > > + struct imx_sc_msg_key msg; > > + struct imx_sc_rpc_msg *hdr = &msg.hdr; > > + bool state; > > + int ret; > > + > > + hdr->ver = IMX_SC_RPC_VERSION; > > + hdr->svc = IMX_SC_RPC_SVC_MISC; > > + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS; > > + hdr->size = 1; > > + > > + ret = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true); > > + if (ret) { > > + dev_err(&input->dev, "read imx sc key failed, ret %d\n", ret); > > + return; > > + } > > + > > + state = (bool)msg.state; > > + > > + if (!state && !priv->keystate) > > + state = true; > > This needs an explanation please. This is to handle the quick press of button, e.g., when button is pressed and released very quickly, when the delay work is scheduled, the button state read from SCU FW is actually a release state (0), the press state is (1), so the quick press/release will be ignored. However, after double check and test, I think this should be handled by debounce time, if the button is pressed/release very quickly, the event should be ignored, I will remove it and reduce the debounce time to 30mS, previous 100mS is too long, using 30mS as debounce time, I did NOT see similar issue no matter how quick I press/release the button. > > > + > > + if (state ^ priv->keystate) { > > + pm_wakeup_event(input->dev.parent, 0); > > I'd expect this happening in imx_sc_key_notify() so that the device would > have a change to run this work. Agreed, will move it to the imx_sc_key_notify(). > > > + priv->keystate = state; > > + input_event(input, EV_KEY, priv->keycode, state); > > + input_sync(input); > > + if (!state) > > + priv->delay_check = 0; > > + pm_relax(priv->input->dev.parent); > > Are you sure you want to call pm_relax() unconditionally, and not when state > == false (i.e. button released)? Agreed, will make sure ONLY call it when button is released which indicates the event is finished. > > > + } > > + > > + if (state) > > + schedule_delayed_work(&priv->check_work, > > + msecs_to_jiffies(REPEAT_INTERVAL)); > > +} > > + > > +static int imx_sc_key_probe(struct platform_device *pdev) { > > + struct device_node *np = pdev->dev.of_node; > > + static struct imx_key_drv_data *priv; > > + struct input_dev *input; > > + int ret; > > + > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + ret = imx_scu_get_handle(&priv->key_ipc_handle); > > + if (ret) > > + return ret; > > + > > + if (of_property_read_u32(np, "linux,keycode", &priv->keycode)) { > > + dev_err(&pdev->dev, "missing KEY_POWER in DT\n"); > > + return -EINVAL; > > + } > > + > > + INIT_DELAYED_WORK(&priv->check_work, > imx_sc_check_for_events); > > + > > + 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 = "imx-sc-key/input0"; > > + input->id.bustype = BUS_HOST; > > + > > + input_set_capability(input, EV_KEY, priv->keycode); > > + > > + ret = input_register_device(input); > > + if (ret) { > > Could you please rename this (and elsewhere) from 'ret' to 'error'? OK. > > > + dev_err(&pdev->dev, "failed to register input device\n"); > > + return ret; > > + } > > + > > + priv->input = input; > > + platform_set_drvdata(pdev, priv); > > + > > + ret = imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, > SC_IRQ_BUTTON, true); > > + if (ret) { > > + dev_warn(&pdev->dev, "enable scu group irq failed\n"); > > + return ret; > > + } > > + > > + priv->key_notifier.notifier_call = imx_sc_key_notify; > > + ret = imx_scu_irq_register_notifier(&priv->key_notifier); > > + if (ret) { > > + imx_scu_irq_group_enable(SC_IRQ_GROUP_WAKE, > SC_IRQ_BUTTON, false); > > + dev_warn(&pdev->dev, "register scu notifier failed\n"); > > return error; > > + } > > + > > + return ret; > > return 0; OK. Thanks, Anson. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support 2019-09-17 2:42 ` Anson Huang @ 2019-09-22 16:14 ` Pavel Machek 2019-09-23 2:43 ` Anson Huang 0 siblings, 1 reply; 9+ messages in thread From: Pavel Machek @ 2019-09-22 16:14 UTC (permalink / raw) To: Anson Huang Cc: Dmitry Torokhov, robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, Aisheng Dong, ulf.hansson, Andy Duan, Peng Fan, Leonard Crestez, Daniel Baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, Jacky Bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input, dl-linux-imx Hi! > > > + ret = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true); > > > + if (ret) { > > > + dev_err(&input->dev, "read imx sc key failed, ret %d\n", ret); > > > + return; > > > + } > > > + > > > + state = (bool)msg.state; > > > + > > > + if (!state && !priv->keystate) > > > + state = true; > > > > This needs an explanation please. > > This is to handle the quick press of button, e.g., when button is pressed and released very quickly, > when the delay work is scheduled, the button state read from SCU FW is actually a release state (0), > the press state is (1), so the quick press/release will be ignored. > > However, after double check and test, I think this should be handled by debounce time, if the button is pressed/release > very quickly, the event should be ignored, I will remove it and reduce the debounce time to 30mS, previous 100mS > is too long, using 30mS as debounce time, I did NOT see similar issue no matter how quick I press/release the button. Are you sure this is expected behaviour? AFAIK microswitches can bounce when the button is pressed and released, but will not generate glitches when the button was not pressed, so even short presses are real and should be propagated... Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support 2019-09-22 16:14 ` Pavel Machek @ 2019-09-23 2:43 ` Anson Huang 0 siblings, 0 replies; 9+ messages in thread From: Anson Huang @ 2019-09-23 2:43 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, Aisheng Dong, ulf.hansson, Andy Duan, Peng Fan, Leonard Crestez, Daniel Baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, Jacky Bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input, dl-linux-imx Hi, Pavel > Subject: Re: [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system > controller key support > > Hi! > > > > > + ret = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true); > > > > + if (ret) { > > > > + dev_err(&input->dev, "read imx sc key failed, ret %d\n", ret); > > > > + return; > > > > + } > > > > + > > > > + state = (bool)msg.state; > > > > + > > > > + if (!state && !priv->keystate) > > > > + state = true; > > > > > > This needs an explanation please. > > > > This is to handle the quick press of button, e.g., when button is > > pressed and released very quickly, when the delay work is scheduled, > > the button state read from SCU FW is actually a release state (0), the press > state is (1), so the quick press/release will be ignored. > > > > However, after double check and test, I think this should be handled > > by debounce time, if the button is pressed/release very quickly, the > > event should be ignored, I will remove it and reduce the debounce time to > 30mS, previous 100mS is too long, using 30mS as debounce time, I did NOT > see similar issue no matter how quick I press/release the button. > > Are you sure this is expected behaviour? > > AFAIK microswitches can bounce when the button is pressed and released, > but will not generate glitches when the button was not pressed, so even > short presses are real and should be propagated... Latest version of patch handle the quick press as below: - When the button is pressed, the delayed work will be scheduled; - In the delayed work, the button state will be read, once the state is different as previous state, it will be propagated; - If the state is a press state, another delayed work will be scheduled for handling release event. So there could be a small window of DEBOUNCE_TIME (30mS) between button press and the delayed work scheduled for reading the button state, if the button is released in less than 30ms, then the state read could be a release state which is same as previous state and it will be ignored. Do you think in this case, we should also propagated the press event? If yes, then what about the release event? Although from what I tested, no matter how quick I press the button, the driver can always receive both the press and release event, as the 30mS is very short. Thanks, Anson ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V4 3/5] arm64: dts: imx8qxp: Add scu key node 2019-09-16 2:52 [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding Anson Huang 2019-09-16 2:52 ` [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support Anson Huang @ 2019-09-16 2:52 ` Anson Huang 2019-09-16 2:52 ` [PATCH V4 4/5] arm64: dts: imx8qxp-mek: Enable scu key Anson Huang 2019-09-16 2:52 ` [PATCH V4 5/5] arm64: defconfig: Enable CONFIG_KEYBOARD_IMX_SC_KEY as module Anson Huang 3 siblings, 0 replies; 9+ messages in thread From: Anson Huang @ 2019-09-16 2:52 UTC (permalink / raw) To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, dmitry.torokhov, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input Cc: Linux-imx Add scu key node for i.MX8QXP, disabled by default as it depends on board design. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- No changes. --- arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi index 1133b41..71e38c1 100644 --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi @@ -8,6 +8,7 @@ #include <dt-bindings/clock/imx8-clock.h> #include <dt-bindings/firmware/imx/rsrc.h> #include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/pinctrl/pads-imx8qxp.h> @@ -174,6 +175,12 @@ #power-domain-cells = <1>; }; + scu_key: scu-key { + compatible = "fsl,imx8qxp-sc-key", "fsl,imx-sc-key"; + linux,keycode = <KEY_POWER>; + status = "disabled"; + }; + rtc: rtc { compatible = "fsl,imx8qxp-sc-rtc"; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V4 4/5] arm64: dts: imx8qxp-mek: Enable scu key 2019-09-16 2:52 [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding Anson Huang 2019-09-16 2:52 ` [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support Anson Huang 2019-09-16 2:52 ` [PATCH V4 3/5] arm64: dts: imx8qxp: Add scu key node Anson Huang @ 2019-09-16 2:52 ` Anson Huang 2019-09-16 2:52 ` [PATCH V4 5/5] arm64: defconfig: Enable CONFIG_KEYBOARD_IMX_SC_KEY as module Anson Huang 3 siblings, 0 replies; 9+ messages in thread From: Anson Huang @ 2019-09-16 2:52 UTC (permalink / raw) To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, dmitry.torokhov, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input Cc: Linux-imx Enable scu key for i.MX8QXP MEK board. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- No changes. --- arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts index 1946805..88dd9132 100644 --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts @@ -234,3 +234,7 @@ &adma_dsp { status = "okay"; }; + +&scu_key { + status = "okay"; +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V4 5/5] arm64: defconfig: Enable CONFIG_KEYBOARD_IMX_SC_KEY as module 2019-09-16 2:52 [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding Anson Huang ` (2 preceding siblings ...) 2019-09-16 2:52 ` [PATCH V4 4/5] arm64: dts: imx8qxp-mek: Enable scu key Anson Huang @ 2019-09-16 2:52 ` Anson Huang 3 siblings, 0 replies; 9+ messages in thread From: Anson Huang @ 2019-09-16 2:52 UTC (permalink / raw) To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam, catalin.marinas, will, dmitry.torokhov, aisheng.dong, ulf.hansson, fugang.duan, peng.fan, leonard.crestez, daniel.baluta, olof, mripard, arnd, jagan, dinguyen, bjorn.andersson, marcin.juszkiewicz, andriy.shevchenko, yuehaibing, cw00.choi, enric.balletbo, m.felsch, ping.bai, ronald, stefan, devicetree, linux-kernel, linux-arm-kernel, linux-input Cc: Linux-imx Select CONFIG_KEYBOARD_IMX_SC_KEY as module by default to support i.MX8QXP scu key driver. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- No changes. --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 0a70e10..9c83014 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -314,6 +314,7 @@ CONFIG_INPUT_EVDEV=y CONFIG_KEYBOARD_ADC=m CONFIG_KEYBOARD_GPIO=y CONFIG_KEYBOARD_SNVS_PWRKEY=m +CONFIG_KEYBOARD_IMX_SC_KEY=m CONFIG_KEYBOARD_CROS_EC=y CONFIG_INPUT_TOUCHSCREEN=y CONFIG_TOUCHSCREEN_ATMEL_MXT=m -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-09-23 2:43 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-16 2:52 [PATCH V4 1/5] dt-bindings: fsl: scu: add scu key binding Anson Huang 2019-09-16 2:52 ` [PATCH V4 2/5] input: keyboard: imx_sc: Add i.MX system controller key support Anson Huang 2019-09-16 23:53 ` Dmitry Torokhov 2019-09-17 2:42 ` Anson Huang 2019-09-22 16:14 ` Pavel Machek 2019-09-23 2:43 ` Anson Huang 2019-09-16 2:52 ` [PATCH V4 3/5] arm64: dts: imx8qxp: Add scu key node Anson Huang 2019-09-16 2:52 ` [PATCH V4 4/5] arm64: dts: imx8qxp-mek: Enable scu key Anson Huang 2019-09-16 2:52 ` [PATCH V4 5/5] arm64: defconfig: Enable CONFIG_KEYBOARD_IMX_SC_KEY as module Anson Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).