[v3,4/6] input: pf1550: add onkey support

From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>

Add support for the onkey of the pf1550 PMIC.

Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v3:
- Address Dmitry's feedback
  - Drop compatible string
  - Remove dependency on OF
  - Use generic device properties
  - Drop unnecessary includes
  - Drop unnecessary initializations in probe
  - Always use the KEY_POWER property for onkey->keycode
  - Do mapping of irqs in MFD driver
  - Define onkey->input before interrupts are active
  - Drop unnecessary input_free_device since devm
  - Manage onkey irqs instead of the main interrupt line.
- Fix integer overflow when unmasking onkey irqs in onkey_resume.
v2:
- Add driver for onkey
---
 drivers/input/misc/Kconfig        |  11 +++
 drivers/input/misc/Makefile       |   1 +
 drivers/input/misc/pf1550-onkey.c | 202 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+)

Hi Samuel,

On Tue, May 27, 2025 at 06:25:36PM -0400, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> 
> Add support for the onkey of the pf1550 PMIC.
> 
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> ---
> v3:
> - Address Dmitry's feedback
>   - Drop compatible string
>   - Remove dependency on OF
>   - Use generic device properties
>   - Drop unnecessary includes
>   - Drop unnecessary initializations in probe
>   - Always use the KEY_POWER property for onkey->keycode
>   - Do mapping of irqs in MFD driver
>   - Define onkey->input before interrupts are active
>   - Drop unnecessary input_free_device since devm
>   - Manage onkey irqs instead of the main interrupt line.
> - Fix integer overflow when unmasking onkey irqs in onkey_resume.

Thank you for making changes, some more comments below.

> v2:
> - Add driver for onkey
> ---
>  drivers/input/misc/Kconfig        |  11 +++
>  drivers/input/misc/Makefile       |   1 +
>  drivers/input/misc/pf1550-onkey.c | 202 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 214 insertions(+)
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index f5496ca0c0d2bfcb7968503ccd1844ff43bbc1c0..50ae50628f4d03f54b5678dbd28e3b58f8d02f86 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -179,6 +179,17 @@ config INPUT_PCSPKR
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called pcspkr.
>  
> +config INPUT_PF1550_ONKEY
> +	tristate "PF1550 Onkey support"
> +	depends on MFD_PF1550
> +	help
> +	  Say Y here if you want support for PF1550 PMIC. Onkey can trigger
> +	  release and 1s(push hold), 2s, 3s, 4s, 8s interrupt for long press
> +	  detect.
> +
> +	  To compile this driver as a module, choose M here. The module will be
> +	  called pf1550-onkey.
> +
>  config INPUT_PM8941_PWRKEY
>  	tristate "Qualcomm PM8941 power key support"
>  	depends on MFD_SPMI_PMIC
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 6d91804d0a6f761a094e6c380f878f74c3054d63..c652337de464c1eeaf1515d0bc84d10de0cb3a74 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
>  obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
>  obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
>  obj-$(CONFIG_INPUT_PCSPKR)		+= pcspkr.o
> +obj-$(CONFIG_INPUT_PF1550_ONKEY)	+= pf1550-onkey.o
>  obj-$(CONFIG_INPUT_PM8941_PWRKEY)	+= pm8941-pwrkey.o
>  obj-$(CONFIG_INPUT_PM8XXX_VIBRATOR)	+= pm8xxx-vibrator.o
>  obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY)	+= pmic8xxx-pwrkey.o
> diff --git a/drivers/input/misc/pf1550-onkey.c b/drivers/input/misc/pf1550-onkey.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..7c10bc75708891a22d8b67b44e55f18c42f09749
> --- /dev/null
> +++ b/drivers/input/misc/pf1550-onkey.c
> @@ -0,0 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the PF1550 ON_KEY
> + * Copyright (C) 2016 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/pf1550.h>
> +#include <linux/platform_device.h>
> +
> +#define PF1550_ONKEY_IRQ_NR	6
> +
> +struct onkey_drv_data {
> +	struct device *dev;
> +	struct pf1550_dev *pf1550;
> +	unsigned int irq;

I do not think you need to store this (with the current code).

> +	int keycode;

If you always send KEY_POWER you do not need to store keycode here.

> +	int wakeup;

bool?

> +	struct input_dev *input;
> +};
> +
> +static irqreturn_t pf1550_onkey_irq_handler(int irq, void *data)
> +{
> +	struct onkey_drv_data *onkey = data;
> +	struct irq_domain *domain;
> +	int i, state, irq_type = -1;
> +	unsigned int virq;
> +
> +	domain = regmap_irq_get_domain(onkey->pf1550->irq_data_onkey);
> +	onkey->irq = irq;
> +
> +	for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
> +		virq = irq_find_mapping(domain, i);
> +		if (onkey->irq == virq)
> +			irq_type = i;
> +	}

I wonder why the driver still needs to poke into the IRQ domain? Is it
possible to have the mapped IRQs described as resources in onkey MFD
cell so here we can use platform_get_irq() or platform_get_irq_byname()
and use them? You can specify that "pushi" should be the first platform
IRQ, or go by names...

> +
> +	switch (irq_type) {
> +	case PF1550_ONKEY_IRQ_PUSHI:
> +		state = 0;
> +		break;
> +	case PF1550_ONKEY_IRQ_1SI:
> +	case PF1550_ONKEY_IRQ_2SI:
> +	case PF1550_ONKEY_IRQ_3SI:
> +	case PF1550_ONKEY_IRQ_4SI:
> +	case PF1550_ONKEY_IRQ_8SI:
> +		state = 1;
> +		break;
> +	default:
> +		dev_err(onkey->dev, "onkey interrupt: irq %d occurred\n",
> +			irq_type);
> +		return IRQ_HANDLED;
> +	}
> +
> +	input_event(onkey->input, EV_KEY, onkey->keycode, state);
> +	input_sync(onkey->input);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int pf1550_onkey_probe(struct platform_device *pdev)
> +{
> +	struct onkey_drv_data *onkey;
> +	struct input_dev *input;
> +	struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);

Can this be const?

> +	struct irq_domain *domain;
> +	int i, error;
> +
> +	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
> +	if (!onkey)
> +		return -ENOMEM;
> +
> +	if (!pf1550->regmap)
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +				     "failed to get regmap\n");
> +
> +	onkey->wakeup = device_property_read_bool(pdev->dev.parent,
> +						  "wakeup-source");
> +
> +	input = devm_input_allocate_device(&pdev->dev);
> +	if (!input)
> +		return dev_err_probe(&pdev->dev, -ENOMEM,
> +				     "failed to allocate the input device\n");
> +
> +	onkey->input = input;
> +	onkey->keycode = KEY_POWER;
> +
> +	input->name = pdev->name;
> +	input->phys = "pf1550-onkey/input0";
> +	input->id.bustype = BUS_HOST;
> +
> +	input_set_capability(input, EV_KEY, onkey->keycode);
> +
> +	domain = regmap_irq_get_domain(pf1550->irq_data_onkey);
> +
> +	for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
> +		unsigned int virq = irq_find_mapping(domain, i);

As I mentioned, I wonder if we can change the core so we use:

		irq = platform_get_irq(pdev, i);

> +
> +		error = devm_request_threaded_irq(&pdev->dev, virq, NULL,
> +						  pf1550_onkey_irq_handler,
> +						  IRQF_NO_SUSPEND,
> +						  "pf1550-onkey", onkey);
> +		if (error)
> +			return dev_err_probe(&pdev->dev, error,
> +					     "failed: irq request (IRQ: %d)\n",
> +					     i);
> +	}
> +
> +	error = input_register_device(input);
> +	if (error < 0)

Just "if (error)"

> +		return dev_err_probe(&pdev->dev, error,
> +				     "failed to register input device\n");
> +
> +	onkey->pf1550 = pf1550;
> +	platform_set_drvdata(pdev, onkey);
> +
> +	device_init_wakeup(&pdev->dev, onkey->wakeup);
> +
> +	return 0;
> +}
> +
> +static int pf1550_onkey_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
> +	struct irq_domain *domain;
> +	unsigned int virq;
> +	int i;
> +
> +	domain = regmap_irq_get_domain(onkey->pf1550->irq_data_onkey);
> +
> +	if (!device_may_wakeup(&pdev->dev)) {
> +		regmap_write(onkey->pf1550->regmap,
> +			     PF1550_PMIC_REG_ONKEY_INT_MASK0,
> +			     ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI | ONKEY_IRQ_2SI |
> +			     ONKEY_IRQ_3SI | ONKEY_IRQ_4SI | ONKEY_IRQ_8SI);
> +	} else {
> +		for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
> +			virq = irq_find_mapping(domain, i);
> +
> +			if (virq)
> +				enable_irq_wake(virq);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int pf1550_onkey_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
> +	struct irq_domain *domain;
> +	unsigned int virq;
> +	int i;
> +
> +	domain = regmap_irq_get_domain(onkey->pf1550->irq_data_onkey);
> +
> +	if (!device_may_wakeup(&pdev->dev)) {
> +		regmap_write(onkey->pf1550->regmap,
> +			     PF1550_PMIC_REG_ONKEY_INT_MASK0,
> +			     ~((u8)(ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI |
> +			     ONKEY_IRQ_2SI | ONKEY_IRQ_3SI | ONKEY_IRQ_4SI |
> +			     ONKEY_IRQ_8SI)));
> +	} else {
> +		for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
> +			virq = irq_find_mapping(domain, i);
> +
> +			if (virq)
> +				disable_irq_wake(virq);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(pf1550_onkey_pm_ops, pf1550_onkey_suspend,
> +			 pf1550_onkey_resume);
> +
> +static const struct platform_device_id pf1550_onkey_id[] = {
> +	{ "pf1550-onkey", PF1550 },

Why do we need to set driver_data here? 

> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, pf1550_onkey_id);
> +
> +static struct platform_driver pf1550_onkey_driver = {
> +	.driver = {
> +		.name = "pf1550-onkey",
> +		.pm   = &pf1550_onkey_pm_ops,
> +	},
> +	.probe = pf1550_onkey_probe,
> +	.id_table = pf1550_onkey_id,
> +};
> +module_platform_driver(pf1550_onkey_driver);
> +
> +MODULE_AUTHOR("Freescale Semiconductor");
> +MODULE_DESCRIPTION("PF1550 onkey Driver");
> +MODULE_LICENSE("GPL");

Thanks.

Hi Samuel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0a4b866d08c6adaea2f4592d31edac6deeb4dcbd]

url:    https://github.com/intel-lab-lkp/linux/commits/Samuel-Kayode-via-B4-Relay/dt-bindings-mfd-add-pf1550/20250528-062840
base:   0a4b866d08c6adaea2f4592d31edac6deeb4dcbd
patch link:    https://lore.kernel.org/r/20250527-pf1550-v3-4-45f69453cd51%40savoirfairelinux.com
patch subject: [PATCH v3 4/6] input: pf1550: add onkey support
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20250529/202505290028.2lbquDkW-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505290028.2lbquDkW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505290028.2lbquDkW-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/input/misc/pf1550-onkey.c:153:12: warning: 'pf1550_onkey_resume' defined but not used [-Wunused-function]
     153 | static int pf1550_onkey_resume(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~
>> drivers/input/misc/pf1550-onkey.c:126:12: warning: 'pf1550_onkey_suspend' defined but not used [-Wunused-function]
     126 | static int pf1550_onkey_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~


vim +/pf1550_onkey_resume +153 drivers/input/misc/pf1550-onkey.c

   125	
 > 126	static int pf1550_onkey_suspend(struct device *dev)
   127	{
   128		struct platform_device *pdev = to_platform_device(dev);
   129		struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
   130		struct irq_domain *domain;
   131		unsigned int virq;
   132		int i;
   133	
   134		domain = regmap_irq_get_domain(onkey->pf1550->irq_data_onkey);
   135	
   136		if (!device_may_wakeup(&pdev->dev)) {
   137			regmap_write(onkey->pf1550->regmap,
   138				     PF1550_PMIC_REG_ONKEY_INT_MASK0,
   139				     ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI | ONKEY_IRQ_2SI |
   140				     ONKEY_IRQ_3SI | ONKEY_IRQ_4SI | ONKEY_IRQ_8SI);
   141		} else {
   142			for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
   143				virq = irq_find_mapping(domain, i);
   144	
   145				if (virq)
   146					enable_irq_wake(virq);
   147			}
   148		}
   149	
   150		return 0;
   151	}
   152	
 > 153	static int pf1550_onkey_resume(struct device *dev)
   154	{
   155		struct platform_device *pdev = to_platform_device(dev);
   156		struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
   157		struct irq_domain *domain;
   158		unsigned int virq;
   159		int i;
   160	
   161		domain = regmap_irq_get_domain(onkey->pf1550->irq_data_onkey);
   162	
   163		if (!device_may_wakeup(&pdev->dev)) {
   164			regmap_write(onkey->pf1550->regmap,
   165				     PF1550_PMIC_REG_ONKEY_INT_MASK0,
   166				     ~((u8)(ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI |
   167				     ONKEY_IRQ_2SI | ONKEY_IRQ_3SI | ONKEY_IRQ_4SI |
   168				     ONKEY_IRQ_8SI)));
   169		} else {
   170			for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
   171				virq = irq_find_mapping(domain, i);
   172	
   173				if (virq)
   174					disable_irq_wake(virq);
   175			}
   176		}
   177	
   178		return 0;
   179	}
   180