| Message ID | 20231217131838.7569-5-karelb@gimli.ms.mff.cuni.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | support for Marvell 88PM886 PMIC | expand |
Hi Karel, On Sun, Dec 17, 2023 at 02:17:02PM +0100, Karel Balej wrote: > From: Karel Balej <balejk@matfyz.cz> > > The Marvell 88PM88X PMICs provide onkey among other things. Add client > driver to handle it. The driver currently only provides a basic support > omitting additional functions found in the vendor version, such as long > onkey and GPIO integration. > > Signed-off-by: Karel Balej <balejk@matfyz.cz> > --- > drivers/input/misc/88pm88x-onkey.c | 103 +++++++++++++++++++++++++++++ > drivers/input/misc/Kconfig | 10 +++ > drivers/input/misc/Makefile | 1 + > 3 files changed, 114 insertions(+) > create mode 100644 drivers/input/misc/88pm88x-onkey.c > > diff --git a/drivers/input/misc/88pm88x-onkey.c b/drivers/input/misc/88pm88x-onkey.c > new file mode 100644 > index 000000000000..0d6056a3cab2 > --- /dev/null > +++ b/drivers/input/misc/88pm88x-onkey.c > @@ -0,0 +1,103 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <linux/kernel.h> > +#include <linux/input.h> > +#include <linux/regmap.h> > +#include <linux/irq.h> > +#include <linux/interrupt.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> Please sort alphabetically. > + > +#include <linux/mfd/88pm88x.h> > + > +struct pm88x_onkey { > + struct input_dev *idev; > + struct pm88x_chip *chip; > + int irq; Since you are using devm to request interrupt I do not think you need to store it here. > +}; > + > +static irqreturn_t pm88x_onkey_irq_handler(int irq, void *data) > +{ > + struct pm88x_onkey *onkey = data; > + unsigned int val; > + int ret = 0; "error". Also no need to initialize it to 0. > + > + ret = regmap_read(onkey->chip->regmaps[PM88X_REGMAP_BASE], PM88X_REG_STATUS1, &val); I still prefer to keep withing 80 columns, unless longer lines are a clear win. > + if (ret) { > + dev_err(onkey->idev->dev.parent, "Failed to read status: %d\n", ret); Maybe have "dev" in onkey instead of poking through input? > + return IRQ_NONE; > + } > + val &= PM88X_ONKEY_STS1; > + > + input_report_key(onkey->idev, KEY_POWER, val); > + input_sync(onkey->idev); > + > + return IRQ_HANDLED; > +} > + > +static int pm88x_onkey_probe(struct platform_device *pdev) > +{ > + struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent); > + struct pm88x_onkey *onkey; > + int err; > + > + onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL); > + if (!onkey) > + return -ENOMEM; > + > + onkey->chip = chip; > + > + onkey->irq = platform_get_irq(pdev, 0); > + if (onkey->irq < 0) { > + dev_err(&pdev->dev, "Failed to get IRQ\n"); > + return -EINVAL; Do not clobber the return from platform_get_irq(). So "return onkey->irq" (or simply irq if you drop it from onkey and use temporary). > + } > + > + onkey->idev = devm_input_allocate_device(&pdev->dev); > + if (!onkey->idev) { > + dev_err(&pdev->dev, "Failed to allocate input device\n"); > + return -ENOMEM; > + } > + > + onkey->idev->name = "88pm88x-onkey"; > + onkey->idev->phys = "88pm88x-onkey/input0"; > + onkey->idev->id.bustype = BUS_I2C; > + onkey->idev->dev.parent = &pdev->dev; No need to do this since you are using devm_input_allocate_device(). > + onkey->idev->evbit[0] = BIT_MASK(EV_KEY); > + onkey->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER); Please use input_set_capability(onkey->idev, EV_KEY, KEY_POWER); > + > + err = devm_request_threaded_irq(&pdev->dev, onkey->irq, NULL, pm88x_onkey_irq_handler, > + IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey", onkey); Please align arguments. > + if (err) { > + dev_err(&pdev->dev, "Failed to request IRQ: %d\n", err); I was persuaded to stop my crusade against dev_err_probe() so you may use it here. > + return err; > + } > + > + err = input_register_device(onkey->idev); > + if (err) { > + dev_err(&pdev->dev, "Failed to register input device: %d\n", err); > + return err; > + } > + > + device_init_wakeup(&pdev->dev, 1); Are there cases where we woudl not want wakeup? Maybe use standard "wakeup-source" property? > + > + return 0; > +} > + > +static const struct of_device_id pm88x_onkey_of_match[] = { > + { .compatible = "marvell,88pm88x-onkey", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, pm88x_onkey_of_match); > + > +static struct platform_driver pm88x_onkey_driver = { > + .driver = { > + .name = "88pm88x-onkey", > + .of_match_table = of_match_ptr(pm88x_onkey_of_match), Given that you are not guarding pm88x_onkey_of_match definition with #ifdef CONFIG_OF you shoudl not use of_match_ptr(). Thanks.
diff --git a/drivers/input/misc/88pm88x-onkey.c b/drivers/input/misc/88pm88x-onkey.c new file mode 100644 index 000000000000..0d6056a3cab2 --- /dev/null +++ b/drivers/input/misc/88pm88x-onkey.c @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <linux/kernel.h> +#include <linux/input.h> +#include <linux/regmap.h> +#include <linux/irq.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/of.h> + +#include <linux/mfd/88pm88x.h> + +struct pm88x_onkey { + struct input_dev *idev; + struct pm88x_chip *chip; + int irq; +}; + +static irqreturn_t pm88x_onkey_irq_handler(int irq, void *data) +{ + struct pm88x_onkey *onkey = data; + unsigned int val; + int ret = 0; + + ret = regmap_read(onkey->chip->regmaps[PM88X_REGMAP_BASE], PM88X_REG_STATUS1, &val); + if (ret) { + dev_err(onkey->idev->dev.parent, "Failed to read status: %d\n", ret); + return IRQ_NONE; + } + val &= PM88X_ONKEY_STS1; + + input_report_key(onkey->idev, KEY_POWER, val); + input_sync(onkey->idev); + + return IRQ_HANDLED; +} + +static int pm88x_onkey_probe(struct platform_device *pdev) +{ + struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent); + struct pm88x_onkey *onkey; + int err; + + onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL); + if (!onkey) + return -ENOMEM; + + onkey->chip = chip; + + onkey->irq = platform_get_irq(pdev, 0); + if (onkey->irq < 0) { + dev_err(&pdev->dev, "Failed to get IRQ\n"); + return -EINVAL; + } + + onkey->idev = devm_input_allocate_device(&pdev->dev); + if (!onkey->idev) { + dev_err(&pdev->dev, "Failed to allocate input device\n"); + return -ENOMEM; + } + + onkey->idev->name = "88pm88x-onkey"; + onkey->idev->phys = "88pm88x-onkey/input0"; + onkey->idev->id.bustype = BUS_I2C; + onkey->idev->dev.parent = &pdev->dev; + onkey->idev->evbit[0] = BIT_MASK(EV_KEY); + onkey->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER); + + err = devm_request_threaded_irq(&pdev->dev, onkey->irq, NULL, pm88x_onkey_irq_handler, + IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey", onkey); + if (err) { + dev_err(&pdev->dev, "Failed to request IRQ: %d\n", err); + return err; + } + + err = input_register_device(onkey->idev); + if (err) { + dev_err(&pdev->dev, "Failed to register input device: %d\n", err); + return err; + } + + device_init_wakeup(&pdev->dev, 1); + + return 0; +} + +static const struct of_device_id pm88x_onkey_of_match[] = { + { .compatible = "marvell,88pm88x-onkey", }, + { }, +}; +MODULE_DEVICE_TABLE(of, pm88x_onkey_of_match); + +static struct platform_driver pm88x_onkey_driver = { + .driver = { + .name = "88pm88x-onkey", + .of_match_table = of_match_ptr(pm88x_onkey_of_match), + }, + .probe = pm88x_onkey_probe, +}; +module_platform_driver(pm88x_onkey_driver); + +MODULE_DESCRIPTION("Marvell 88PM88X onkey driver"); +MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>"); +MODULE_LICENSE("GPL"); diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 6ba984d7f0b1..fdfa3e23c3cf 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -33,6 +33,16 @@ config INPUT_88PM80X_ONKEY To compile this driver as a module, choose M here: the module will be called 88pm80x_onkey. +config INPUT_88PM88X_ONKEY + tristate "Marvell 88PM88X onkey support" + depends on MFD_88PM88X + help + Support the onkey of Marvell 88PM88X PMICs as an input device + reporting power button status. + + To compile this driver as a module, choose M here: the module + will be called 88pm88x-onkey. + config INPUT_AB8500_PONKEY tristate "AB8500 Pon (PowerOn) Key" depends on AB8500_CORE diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 04296a4abe8e..eab7a364188c 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_INPUT_88PM860X_ONKEY) += 88pm860x_onkey.o obj-$(CONFIG_INPUT_88PM80X_ONKEY) += 88pm80x_onkey.o +obj-$(CONFIG_INPUT_88PM88X_ONKEY) += 88pm88x-onkey.o obj-$(CONFIG_INPUT_AB8500_PONKEY) += ab8500-ponkey.o obj-$(CONFIG_INPUT_AD714X) += ad714x.o obj-$(CONFIG_INPUT_AD714X_I2C) += ad714x-i2c.o