diff mbox series

[v7,7/7] input: keyboard: support i.MX95 BBM module

Message ID 20240731-imx95-bbm-misc-v2-v7-7-a41394365602@nxp.com (mailing list archive)
State Superseded
Headers show
Series firmware: support i.MX95 SCMI BBM/MISC Extenstion | expand

Commit Message

Peng Fan (OSS) July 31, 2024, 12:56 p.m. UTC
From: Peng Fan <peng.fan@nxp.com>

The BBM module provides BUTTON feature. To i.MX95, this module
is managed by System Manager and exported using System Management
Control Interface(SCMI). Linux could use i.MX SCMI BBM Extension
protocol to use BUTTON feature.

This driver is to use SCMI interface to enable pwrkey.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/input/keyboard/Kconfig          |  11 ++
 drivers/input/keyboard/Makefile         |   1 +
 drivers/input/keyboard/imx-sm-bbm-key.c | 236 ++++++++++++++++++++++++++++++++
 3 files changed, 248 insertions(+)

Comments

Cristian Marussi July 31, 2024, 1:57 p.m. UTC | #1
On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The BBM module provides BUTTON feature. To i.MX95, this module
> is managed by System Manager and exported using System Management
> Control Interface(SCMI). Linux could use i.MX SCMI BBM Extension
> protocol to use BUTTON feature.
> 
> This driver is to use SCMI interface to enable pwrkey.
> 

Hi Peng,


> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/input/keyboard/Kconfig          |  11 ++
>  drivers/input/keyboard/Makefile         |   1 +
>  drivers/input/keyboard/imx-sm-bbm-key.c | 236 ++++++++++++++++++++++++++++++++
>  3 files changed, 248 insertions(+)
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 72f9552cb571..a3301239b9a6 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -454,6 +454,17 @@ config KEYBOARD_IMX
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called imx_keypad.
>  
> +config KEYBOARD_IMX_BBM_SCMI
> +	tristate "IMX BBM SCMI Key Driver"
> +	depends on IMX_SCMI_BBM_EXT || COMPILE_TEST
> +	default y if ARCH_MXC
> +	help
> +	  This is the BBM key driver for NXP i.MX SoCs managed through
> +	  SCMI protocol.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called scmi-imx-bbm-key.
> +
>  config KEYBOARD_IMX_SC_KEY
>  	tristate "IMX SCU Key Driver"
>  	depends on IMX_SCU
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index b8d12a0524e0..5915e52eac28 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_KEYBOARD_IPAQ_MICRO)	+= ipaq-micro-keys.o
>  obj-$(CONFIG_KEYBOARD_IQS62X)		+= iqs62x-keys.o
>  obj-$(CONFIG_KEYBOARD_IMX)		+= imx_keypad.o
>  obj-$(CONFIG_KEYBOARD_IMX_SC_KEY)	+= imx_sc_key.o
> +obj-$(CONFIG_KEYBOARD_IMX_BBM_SCMI)	+= imx-sm-bbm-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-sm-bbm-key.c b/drivers/input/keyboard/imx-sm-bbm-key.c
> new file mode 100644
> index 000000000000..ca430dbb61d0
> --- /dev/null
> +++ b/drivers/input/keyboard/imx-sm-bbm-key.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2024 NXP.
> + */
> +
> +#include <linux/input.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +#include <linux/suspend.h>
> +
> +#define DEBOUNCE_TIME		30
> +#define REPEAT_INTERVAL		60
> +
> +struct scmi_imx_bbm {
> +	struct scmi_protocol_handle *ph;
> +	const struct scmi_imx_bbm_proto_ops *ops;
> +	struct notifier_block nb;
> +	int keycode;
> +	int keystate;  /* 1:pressed */
> +	bool suspended;
> +	struct delayed_work check_work;
> +	struct input_dev *input;
> +};
> +
> +static void scmi_imx_bbm_pwrkey_check_for_events(struct work_struct *work)
> +{
> +	struct scmi_imx_bbm *bbnsm = container_of(to_delayed_work(work),
> +						  struct scmi_imx_bbm, check_work);
> +	struct scmi_protocol_handle *ph = bbnsm->ph;
> +	struct input_dev *input = bbnsm->input;
> +	u32 state = 0;
> +	int ret;
> +
> +	ret = bbnsm->ops->button_get(ph, &state);
> +	if (ret) {
> +		pr_err("%s: %d\n", __func__, ret);
> +		return;
> +	}
> +
> +	pr_debug("%s: state: %d, keystate %d\n", __func__, state, bbnsm->keystate);
> +
> +	/* only report new event if status changed */
> +	if (state ^ bbnsm->keystate) {
> +		bbnsm->keystate = state;
> +		input_event(input, EV_KEY, bbnsm->keycode, state);
> +		input_sync(input);
> +		pm_relax(bbnsm->input->dev.parent);
> +		pr_debug("EV_KEY: %x\n", bbnsm->keycode);
> +	}
> +
> +	/* repeat check if pressed long */
> +	if (state)
> +		schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(REPEAT_INTERVAL));
> +}
> +
> +static int scmi_imx_bbm_pwrkey_event(struct scmi_imx_bbm *bbnsm)
> +{
> +	struct input_dev *input = bbnsm->input;
> +
> +	pm_wakeup_event(input->dev.parent, 0);
> +
> +	/*
> +	 * Directly report key event after resume to make no key press
> +	 * event is missed.
> +	 */
> +	if (READ_ONCE(bbnsm->suspended)) {
> +		bbnsm->keystate = 1;
> +		input_event(input, EV_KEY, bbnsm->keycode, 1);
> +		input_sync(input);
> +		WRITE_ONCE(bbnsm->suspended, false);
> +	}
> +
> +	schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(DEBOUNCE_TIME));
> +
> +	return 0;
> +}
> +
> +static void scmi_imx_bbm_pwrkey_act(void *pdata)
> +{
> +	struct scmi_imx_bbm *bbnsm = pdata;
> +
> +	cancel_delayed_work_sync(&bbnsm->check_work);
> +}
> +
> +static int scmi_imx_bbm_key_notifier(struct notifier_block *nb, unsigned long event, void *data)
> +{
> +	struct scmi_imx_bbm *bbnsm = container_of(nb, struct scmi_imx_bbm, nb);
> +	struct scmi_imx_bbm_notif_report *r = data;
> +
> +	if (r->is_button) {
> +		pr_debug("BBM Button Power key pressed\n");
> +		scmi_imx_bbm_pwrkey_event(bbnsm);
> +	} else {
> +		/* Should never reach here */
> +		pr_err("Unexpected BBM event: %s\n", __func__);
> +	}
> +
> +	return 0;
> +}
> +
> +static int scmi_imx_bbm_pwrkey_init(struct scmi_device *sdev)
> +{
> +	const struct scmi_handle *handle = sdev->handle;
> +	struct device *dev = &sdev->dev;
> +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> +	struct input_dev *input;
> +	int ret;
> +
> +	if (device_property_read_u32(dev, "linux,code", &bbnsm->keycode)) {
> +		bbnsm->keycode = KEY_POWER;
> +		dev_warn(dev, "key code is not specified, using default KEY_POWER\n");
> +	}
> +
> +	INIT_DELAYED_WORK(&bbnsm->check_work, scmi_imx_bbm_pwrkey_check_for_events);
> +
> +	input = devm_input_allocate_device(dev);
> +	if (!input) {
> +		dev_err(dev, "failed to allocate the input device for SCMI IMX BBM\n");
> +		return -ENOMEM;
> +	}
> +
> +	input->name = dev_name(dev);
> +	input->phys = "bbnsm-pwrkey/input0";
> +	input->id.bustype = BUS_HOST;
> +
> +	input_set_capability(input, EV_KEY, bbnsm->keycode);
> +
> +	ret = devm_add_action_or_reset(dev, scmi_imx_bbm_pwrkey_act, bbnsm);
> +	if (ret) {
> +		dev_err(dev, "failed to register remove action\n");
> +		return ret;
> +	}
> +
> +	bbnsm->input = input;
> +
> +	bbnsm->nb.notifier_call = &scmi_imx_bbm_key_notifier;
> +	ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
> +							       SCMI_EVENT_IMX_BBM_BUTTON,
> +							       NULL, &bbnsm->nb);
> +
> +	if (ret)
> +		dev_err(dev, "Failed to register BBM Button Events %d:", ret);
> +
> +	ret = input_register_device(input);
> +	if (ret) {
> +		dev_err(dev, "failed to register input device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int scmi_imx_bbm_key_probe(struct scmi_device *sdev)
> +{
> +	const struct scmi_handle *handle = sdev->handle;
> +	struct device *dev = &sdev->dev;
> +	struct scmi_protocol_handle *ph;
> +	struct scmi_imx_bbm *bbnsm;
> +	int ret;
> +
> +	if (!handle)
> +		return -ENODEV;
> +
> +	bbnsm = devm_kzalloc(dev, sizeof(*bbnsm), GFP_KERNEL);
> +	if (!bbnsm)
> +		return -ENOMEM;
> +
> +	bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
> +	if (IS_ERR(bbnsm->ops))
> +		return PTR_ERR(bbnsm->ops);
> +
> +	bbnsm->ph = ph;
> +
> +	device_init_wakeup(dev, true);
> +
> +	dev_set_drvdata(dev, bbnsm);
> +
> +	ret = scmi_imx_bbm_pwrkey_init(sdev);
> +	if (ret)
> +		device_init_wakeup(dev, false);
> +
> +	return ret;
> +}
> +
> +static void scmi_imx_bbm_key_remove(struct scmi_device *sdev)
> +{
> +	struct device *dev = &sdev->dev;
> +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> +
> +	device_init_wakeup(dev, false);
> +
> +	cancel_delayed_work_sync(&bbnsm->check_work);
> +}
> +

..so in v6 I asked you to add a cancel_delayed_work_sync() on the
removal path, BUT I missed, my bad, that indeed above there was already
a call to cancel_delayed_work_sync() associated to a
devm_add_action_or_reset....so now we have 2....also you should try not
to mix devm_add_action_or_reset and plain .remove methods..use one or
the other.

Thanks,
Cristian
Peng Fan July 31, 2024, 3:37 p.m. UTC | #2
Hi Cristian,

> Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> module
> 
> On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The BBM module provides BUTTON feature. To i.MX95, this module is
> > managed by System Manager and exported using System
> Management Control
> > Interface(SCMI). Linux could use i.MX SCMI BBM Extension protocol
> to
> > use BUTTON feature.
> >
> > This driver is to use SCMI interface to enable pwrkey.
> >
> > +}
> > +
> > +static void scmi_imx_bbm_key_remove(struct scmi_device *sdev) {
> > +	struct device *dev = &sdev->dev;
> > +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > +
> > +	device_init_wakeup(dev, false);
> > +
> > +	cancel_delayed_work_sync(&bbnsm->check_work);
> > +}
> > +
> 
> ..so in v6 I asked you to add a cancel_delayed_work_sync() on the
> removal path, BUT I missed, my bad, that indeed above there was
> already a call to cancel_delayed_work_sync() associated to a
> devm_add_action_or_reset....so now we have 2....also you should try
> not to mix devm_add_action_or_reset and plain .remove methods..use
> one or the other.

Thanks for your detailed reviewing on this. I will wait to see if Sudeep
has any comments to patch 1-4. If no comments, I will not do a new
version to this patchset.

If v7 patch 1-4 are good for Sudeep to pick up, I will separate this patch
out as a standalone one for input subsystem maintainer.

Thanks,
Peng.

> 
> Thanks,
> Cristian
Dmitry Torokhov July 31, 2024, 5:28 p.m. UTC | #3
Hi Peng,

On Wed, Jul 31, 2024 at 03:37:18PM +0000, Peng Fan wrote:
> Hi Cristian,
> 
> > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> > module
> > 
> > On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The BBM module provides BUTTON feature. To i.MX95, this module is
> > > managed by System Manager and exported using System
> > Management Control
> > > Interface(SCMI). Linux could use i.MX SCMI BBM Extension protocol
> > to
> > > use BUTTON feature.
> > >
> > > This driver is to use SCMI interface to enable pwrkey.
> > >
> > > +}
> > > +
> > > +static void scmi_imx_bbm_key_remove(struct scmi_device *sdev) {
> > > +	struct device *dev = &sdev->dev;
> > > +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > > +
> > > +	device_init_wakeup(dev, false);

I do not believe you need to reset the wakeup flag on driver unbind, as
well as in the error handling path of probe(). If this is needed then
driver core should do this cleanup (maybe it already does?).

> > > +
> > > +	cancel_delayed_work_sync(&bbnsm->check_work);
> > > +}
> > > +
> > 
> > ..so in v6 I asked you to add a cancel_delayed_work_sync() on the
> > removal path, BUT I missed, my bad, that indeed above there was
> > already a call to cancel_delayed_work_sync() associated to a
> > devm_add_action_or_reset....so now we have 2....also you should try
> > not to mix devm_add_action_or_reset and plain .remove methods..use
> > one or the other.
> 
> Thanks for your detailed reviewing on this. I will wait to see if Sudeep
> has any comments to patch 1-4. If no comments, I will not do a new
> version to this patchset.
> 
> If v7 patch 1-4 are good for Sudeep to pick up, I will separate this patch
> out as a standalone one for input subsystem maintainer.

If you remove the duplicated cancel_delayed_work_sync() in remove() and
unneded device_init_wakeup(dev, false); then you can merge the input
patch with the rest of them with my:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.
Peng Fan Aug. 1, 2024, 1:36 a.m. UTC | #4
Hi Dmitry,

> Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> module
> 
> Hi Peng,
> 
> On Wed, Jul 31, 2024 at 03:37:18PM +0000, Peng Fan wrote:
> > Hi Cristian,
> >
> > > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> > > module
> > >
> > > On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > The BBM module provides BUTTON feature. To i.MX95, this
> module is
> > > > managed by System Manager and exported using System
> > > Management Control
> > > > Interface(SCMI). Linux could use i.MX SCMI BBM Extension
> protocol
> > > to
> > > > use BUTTON feature.
> > > >
> > > > This driver is to use SCMI interface to enable pwrkey.
> > > >
> > > > +}
> > > > +
> > > > +static void scmi_imx_bbm_key_remove(struct scmi_device
> *sdev) {
> > > > +	struct device *dev = &sdev->dev;
> > > > +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > > > +
> > > > +	device_init_wakeup(dev, false);
> 
> I do not believe you need to reset the wakeup flag on driver unbind, as
> well as in the error handling path of probe(). If this is needed then
> driver core should do this cleanup (maybe it already does?).

I just check the driver core code, you are right, there is
no need do this.

DevAttrError:
 device_pm_remove-> device_wakeup_disable(dev);
 dpm_sysfs_remove

> 
> > > > +
> > > > +	cancel_delayed_work_sync(&bbnsm->check_work);
> > > > +}
> > > > +
> > >
> > > ..so in v6 I asked you to add a cancel_delayed_work_sync() on the
> > > removal path, BUT I missed, my bad, that indeed above there was
> > > already a call to cancel_delayed_work_sync() associated to a
> > > devm_add_action_or_reset....so now we have 2....also you should
> try
> > > not to mix devm_add_action_or_reset and plain .remove
> methods..use
> > > one or the other.
> >
> > Thanks for your detailed reviewing on this. I will wait to see if
> > Sudeep has any comments to patch 1-4. If no comments, I will not do
> a
> > new version to this patchset.
> >
> > If v7 patch 1-4 are good for Sudeep to pick up, I will separate this
> > patch out as a standalone one for input subsystem maintainer.
> 
> If you remove the duplicated cancel_delayed_work_sync() in remove()
> and unneded device_init_wakeup(dev, false); then you can merge the
> input patch with the rest of them with my:
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks for your Ack. But I think patch 1-4 needs go to arm-scmi tree,
Patch 5 to arm imx tree, patch 6 to rtc tree, patch 7 to input tree.

I put the patches together in a patchset is to let reviewers could
get a full picture how the whole stuff work.

For patch 7, I will send out it as a separate patch with fix and tag
after patch 1-4 is ready in arm-scmi tree.

Thanks,
Peng.

> 
> Thanks.
> 
> --
> Dmitry
Dmitry Torokhov Aug. 3, 2024, 6:13 a.m. UTC | #5
On Thu, Aug 01, 2024 at 01:36:10AM +0000, Peng Fan wrote:
> Hi Dmitry,
> 
> > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> > module
> > 
> > Hi Peng,
> > 
> > On Wed, Jul 31, 2024 at 03:37:18PM +0000, Peng Fan wrote:
> > > Hi Cristian,
> > >
> > > > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> > > > module
> > > >
> > > > On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > The BBM module provides BUTTON feature. To i.MX95, this
> > module is
> > > > > managed by System Manager and exported using System
> > > > Management Control
> > > > > Interface(SCMI). Linux could use i.MX SCMI BBM Extension
> > protocol
> > > > to
> > > > > use BUTTON feature.
> > > > >
> > > > > This driver is to use SCMI interface to enable pwrkey.
> > > > >
> > > > > +}
> > > > > +
> > > > > +static void scmi_imx_bbm_key_remove(struct scmi_device
> > *sdev) {
> > > > > +	struct device *dev = &sdev->dev;
> > > > > +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > > > > +
> > > > > +	device_init_wakeup(dev, false);
> > 
> > I do not believe you need to reset the wakeup flag on driver unbind, as
> > well as in the error handling path of probe(). If this is needed then
> > driver core should do this cleanup (maybe it already does?).
> 
> I just check the driver core code, you are right, there is
> no need do this.
> 
> DevAttrError:
>  device_pm_remove-> device_wakeup_disable(dev);
>  dpm_sysfs_remove
> 
> > 
> > > > > +
> > > > > +	cancel_delayed_work_sync(&bbnsm->check_work);
> > > > > +}
> > > > > +
> > > >
> > > > ..so in v6 I asked you to add a cancel_delayed_work_sync() on the
> > > > removal path, BUT I missed, my bad, that indeed above there was
> > > > already a call to cancel_delayed_work_sync() associated to a
> > > > devm_add_action_or_reset....so now we have 2....also you should
> > try
> > > > not to mix devm_add_action_or_reset and plain .remove
> > methods..use
> > > > one or the other.
> > >
> > > Thanks for your detailed reviewing on this. I will wait to see if
> > > Sudeep has any comments to patch 1-4. If no comments, I will not do
> > a
> > > new version to this patchset.
> > >
> > > If v7 patch 1-4 are good for Sudeep to pick up, I will separate this
> > > patch out as a standalone one for input subsystem maintainer.
> > 
> > If you remove the duplicated cancel_delayed_work_sync() in remove()
> > and unneded device_init_wakeup(dev, false); then you can merge the
> > input patch with the rest of them with my:
> > 
> > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Thanks for your Ack. But I think patch 1-4 needs go to arm-scmi tree,
> Patch 5 to arm imx tree, patch 6 to rtc tree, patch 7 to input tree.
> 
> I put the patches together in a patchset is to let reviewers could
> get a full picture how the whole stuff work.
> 
> For patch 7, I will send out it as a separate patch with fix and tag
> after patch 1-4 is ready in arm-scmi tree.

Right, but to accelerate getting support for your part into the mainline
I am OK with input piece not going through the input tree but together
with the rest of the patches through some other tree, probably through
arm-scmi. If they are not willing to take it then we will have to wait
till core support lands in mainline and then I can pick up the input
piece and move it through my tree.

Thanks.
Peng Fan Aug. 6, 2024, 2:11 p.m. UTC | #6
Hi Dmitry,

> Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> module
> 
> On Thu, Aug 01, 2024 at 01:36:10AM +0000, Peng Fan wrote:
> > Hi Dmitry,
> >
> > > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
> > > module
> > >
> > > Hi Peng,
> > >
> > > On Wed, Jul 31, 2024 at 03:37:18PM +0000, Peng Fan wrote:
> > > > Hi Cristian,
> > > >
> > > > > Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95
> BBM
> > > > > module
> > > > >
> > > > > On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS)
> wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > The BBM module provides BUTTON feature. To i.MX95, this
> > > module is
> > > > > > managed by System Manager and exported using System
> > > > > Management Control
> > > > > > Interface(SCMI). Linux could use i.MX SCMI BBM Extension
> > > protocol
> > > > > to
> > > > > > use BUTTON feature.
> > > > > >
> > > > > > This driver is to use SCMI interface to enable pwrkey.
> > > > > >
> > > > > > +}
> > > > > > +
> > > > > > +static void scmi_imx_bbm_key_remove(struct scmi_device
> > > *sdev) {
> > > > > > +	struct device *dev = &sdev->dev;
> > > > > > +	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > > > > > +
> > > > > > +	device_init_wakeup(dev, false);
> > >
> > > I do not believe you need to reset the wakeup flag on driver unbind,
> > > as well as in the error handling path of probe(). If this is needed
> > > then driver core should do this cleanup (maybe it already does?).
> >
> > I just check the driver core code, you are right, there is no need do
> > this.
> >
> > DevAttrError:
> >  device_pm_remove-> device_wakeup_disable(dev);
> dpm_sysfs_remove
> >
> > >
> > > > > > +
> > > > > > +	cancel_delayed_work_sync(&bbnsm->check_work);
> > > > > > +}
> > > > > > +
> > > > >
> > > > > ..so in v6 I asked you to add a cancel_delayed_work_sync() on
> > > > > the removal path, BUT I missed, my bad, that indeed above
> there
> > > > > was already a call to cancel_delayed_work_sync() associated to
> a
> > > > > devm_add_action_or_reset....so now we have 2....also you
> should
> > > try
> > > > > not to mix devm_add_action_or_reset and plain .remove
> > > methods..use
> > > > > one or the other.
> > > >
> > > > Thanks for your detailed reviewing on this. I will wait to see if
> > > > Sudeep has any comments to patch 1-4. If no comments, I will not
> > > > do
> > > a
> > > > new version to this patchset.
> > > >
> > > > If v7 patch 1-4 are good for Sudeep to pick up, I will separate
> > > > this patch out as a standalone one for input subsystem maintainer.
> > >
> > > If you remove the duplicated cancel_delayed_work_sync() in
> remove()
> > > and unneded device_init_wakeup(dev, false); then you can merge
> the
> > > input patch with the rest of them with my:
> > >
> > > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > Thanks for your Ack. But I think patch 1-4 needs go to arm-scmi tree,
> > Patch 5 to arm imx tree, patch 6 to rtc tree, patch 7 to input tree.
> >
> > I put the patches together in a patchset is to let reviewers could get
> > a full picture how the whole stuff work.
> >
> > For patch 7, I will send out it as a separate patch with fix and tag
> > after patch 1-4 is ready in arm-scmi tree.
> 
> Right, but to accelerate getting support for your part into the mainline I
> am OK with input piece not going through the input tree but together
> with the rest of the patches through some other tree, probably through
> arm-scmi.

Thanks for your supporting on this patchset. I appreciate! 

 If they are not willing to take it then we will have to wait till
> core support lands in mainline and then I can pick up the input piece
> and move it through my tree.

There is no rush here, I still need to wait Sudeep's comments on
the scmi parts. So this patch probably needs go through your tree in
the end.

Thanks,
Peng.

> 
> Thanks.
> 
> --
> Dmitry
diff mbox series

Patch

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 72f9552cb571..a3301239b9a6 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -454,6 +454,17 @@  config KEYBOARD_IMX
 	  To compile this driver as a module, choose M here: the
 	  module will be called imx_keypad.
 
+config KEYBOARD_IMX_BBM_SCMI
+	tristate "IMX BBM SCMI Key Driver"
+	depends on IMX_SCMI_BBM_EXT || COMPILE_TEST
+	default y if ARCH_MXC
+	help
+	  This is the BBM key driver for NXP i.MX SoCs managed through
+	  SCMI protocol.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called scmi-imx-bbm-key.
+
 config KEYBOARD_IMX_SC_KEY
 	tristate "IMX SCU Key Driver"
 	depends on IMX_SCU
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index b8d12a0524e0..5915e52eac28 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -31,6 +31,7 @@  obj-$(CONFIG_KEYBOARD_IPAQ_MICRO)	+= ipaq-micro-keys.o
 obj-$(CONFIG_KEYBOARD_IQS62X)		+= iqs62x-keys.o
 obj-$(CONFIG_KEYBOARD_IMX)		+= imx_keypad.o
 obj-$(CONFIG_KEYBOARD_IMX_SC_KEY)	+= imx_sc_key.o
+obj-$(CONFIG_KEYBOARD_IMX_BBM_SCMI)	+= imx-sm-bbm-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-sm-bbm-key.c b/drivers/input/keyboard/imx-sm-bbm-key.c
new file mode 100644
index 000000000000..ca430dbb61d0
--- /dev/null
+++ b/drivers/input/keyboard/imx-sm-bbm-key.c
@@ -0,0 +1,236 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+#include <linux/suspend.h>
+
+#define DEBOUNCE_TIME		30
+#define REPEAT_INTERVAL		60
+
+struct scmi_imx_bbm {
+	struct scmi_protocol_handle *ph;
+	const struct scmi_imx_bbm_proto_ops *ops;
+	struct notifier_block nb;
+	int keycode;
+	int keystate;  /* 1:pressed */
+	bool suspended;
+	struct delayed_work check_work;
+	struct input_dev *input;
+};
+
+static void scmi_imx_bbm_pwrkey_check_for_events(struct work_struct *work)
+{
+	struct scmi_imx_bbm *bbnsm = container_of(to_delayed_work(work),
+						  struct scmi_imx_bbm, check_work);
+	struct scmi_protocol_handle *ph = bbnsm->ph;
+	struct input_dev *input = bbnsm->input;
+	u32 state = 0;
+	int ret;
+
+	ret = bbnsm->ops->button_get(ph, &state);
+	if (ret) {
+		pr_err("%s: %d\n", __func__, ret);
+		return;
+	}
+
+	pr_debug("%s: state: %d, keystate %d\n", __func__, state, bbnsm->keystate);
+
+	/* only report new event if status changed */
+	if (state ^ bbnsm->keystate) {
+		bbnsm->keystate = state;
+		input_event(input, EV_KEY, bbnsm->keycode, state);
+		input_sync(input);
+		pm_relax(bbnsm->input->dev.parent);
+		pr_debug("EV_KEY: %x\n", bbnsm->keycode);
+	}
+
+	/* repeat check if pressed long */
+	if (state)
+		schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(REPEAT_INTERVAL));
+}
+
+static int scmi_imx_bbm_pwrkey_event(struct scmi_imx_bbm *bbnsm)
+{
+	struct input_dev *input = bbnsm->input;
+
+	pm_wakeup_event(input->dev.parent, 0);
+
+	/*
+	 * Directly report key event after resume to make no key press
+	 * event is missed.
+	 */
+	if (READ_ONCE(bbnsm->suspended)) {
+		bbnsm->keystate = 1;
+		input_event(input, EV_KEY, bbnsm->keycode, 1);
+		input_sync(input);
+		WRITE_ONCE(bbnsm->suspended, false);
+	}
+
+	schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(DEBOUNCE_TIME));
+
+	return 0;
+}
+
+static void scmi_imx_bbm_pwrkey_act(void *pdata)
+{
+	struct scmi_imx_bbm *bbnsm = pdata;
+
+	cancel_delayed_work_sync(&bbnsm->check_work);
+}
+
+static int scmi_imx_bbm_key_notifier(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct scmi_imx_bbm *bbnsm = container_of(nb, struct scmi_imx_bbm, nb);
+	struct scmi_imx_bbm_notif_report *r = data;
+
+	if (r->is_button) {
+		pr_debug("BBM Button Power key pressed\n");
+		scmi_imx_bbm_pwrkey_event(bbnsm);
+	} else {
+		/* Should never reach here */
+		pr_err("Unexpected BBM event: %s\n", __func__);
+	}
+
+	return 0;
+}
+
+static int scmi_imx_bbm_pwrkey_init(struct scmi_device *sdev)
+{
+	const struct scmi_handle *handle = sdev->handle;
+	struct device *dev = &sdev->dev;
+	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+	struct input_dev *input;
+	int ret;
+
+	if (device_property_read_u32(dev, "linux,code", &bbnsm->keycode)) {
+		bbnsm->keycode = KEY_POWER;
+		dev_warn(dev, "key code is not specified, using default KEY_POWER\n");
+	}
+
+	INIT_DELAYED_WORK(&bbnsm->check_work, scmi_imx_bbm_pwrkey_check_for_events);
+
+	input = devm_input_allocate_device(dev);
+	if (!input) {
+		dev_err(dev, "failed to allocate the input device for SCMI IMX BBM\n");
+		return -ENOMEM;
+	}
+
+	input->name = dev_name(dev);
+	input->phys = "bbnsm-pwrkey/input0";
+	input->id.bustype = BUS_HOST;
+
+	input_set_capability(input, EV_KEY, bbnsm->keycode);
+
+	ret = devm_add_action_or_reset(dev, scmi_imx_bbm_pwrkey_act, bbnsm);
+	if (ret) {
+		dev_err(dev, "failed to register remove action\n");
+		return ret;
+	}
+
+	bbnsm->input = input;
+
+	bbnsm->nb.notifier_call = &scmi_imx_bbm_key_notifier;
+	ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+							       SCMI_EVENT_IMX_BBM_BUTTON,
+							       NULL, &bbnsm->nb);
+
+	if (ret)
+		dev_err(dev, "Failed to register BBM Button Events %d:", ret);
+
+	ret = input_register_device(input);
+	if (ret) {
+		dev_err(dev, "failed to register input device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int scmi_imx_bbm_key_probe(struct scmi_device *sdev)
+{
+	const struct scmi_handle *handle = sdev->handle;
+	struct device *dev = &sdev->dev;
+	struct scmi_protocol_handle *ph;
+	struct scmi_imx_bbm *bbnsm;
+	int ret;
+
+	if (!handle)
+		return -ENODEV;
+
+	bbnsm = devm_kzalloc(dev, sizeof(*bbnsm), GFP_KERNEL);
+	if (!bbnsm)
+		return -ENOMEM;
+
+	bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
+	if (IS_ERR(bbnsm->ops))
+		return PTR_ERR(bbnsm->ops);
+
+	bbnsm->ph = ph;
+
+	device_init_wakeup(dev, true);
+
+	dev_set_drvdata(dev, bbnsm);
+
+	ret = scmi_imx_bbm_pwrkey_init(sdev);
+	if (ret)
+		device_init_wakeup(dev, false);
+
+	return ret;
+}
+
+static void scmi_imx_bbm_key_remove(struct scmi_device *sdev)
+{
+	struct device *dev = &sdev->dev;
+	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+	device_init_wakeup(dev, false);
+
+	cancel_delayed_work_sync(&bbnsm->check_work);
+}
+
+static int __maybe_unused scmi_imx_bbm_key_suspend(struct device *dev)
+{
+	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+	WRITE_ONCE(bbnsm->suspended, true);
+
+	return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_key_resume(struct device *dev)
+{
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(scmi_imx_bbm_pm_key_ops, scmi_imx_bbm_key_suspend,
+			 scmi_imx_bbm_key_resume);
+
+static const struct scmi_device_id scmi_id_table[] = {
+	{ SCMI_PROTOCOL_IMX_BBM, "imx-bbm-key" },
+	{ },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_bbm_key_driver = {
+	.driver = {
+		.pm = &scmi_imx_bbm_pm_key_ops,
+	},
+	.name = "scmi-imx-bbm-key",
+	.probe = scmi_imx_bbm_key_probe,
+	.remove = scmi_imx_bbm_key_remove,
+	.id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_bbm_key_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM BBM Key driver");
+MODULE_LICENSE("GPL");