Message ID | 20250527-pf1550-v3-4-45f69453cd51@savoirfairelinux.com |
---|---|
State | New |
Headers | show |
Series | add support for pf1550 PMIC MFD-based drivers | expand |
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
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; + int keycode; + int wakeup; + 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; + } + + 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); + 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); + + 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) + 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 }, + { /* 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");