From: Karel Balej <[email protected]>
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 <[email protected]>
---
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>
+
+#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 <[email protected]>");
+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
--
2.43.0
Hi Karel,
On Sun, Dec 17, 2023 at 02:17:02PM +0100, Karel Balej wrote:
> From: Karel Balej <[email protected]>
>
> 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 <[email protected]>
> ---
> 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.
--
Dmitry