[PATCH v2 0/4] Add polling support for DA9063 onkey driver

[PATCH v2 0/4] Add polling support for DA9063 onkey driver

[Biju Das]

On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
populated by default. This patch series aims to add polling support
by parsing the polling interval from device tree and then detect
short and long key press.

v1->v2:
 * Add a new patch for dropping redundant prints in probe()
 * Fixed typo in commit description for patch#3.
 * Updated the print message for irq allocation failure.
 * Updated commit description for patch#4
 * Fixed the logical mistake for optional IRQ handling.

Biju Das (4):
  Input: da9063 - Simplify obtaining OF match data
  Input: da9063 - Drop redundant prints in probe()
  Input: da9063 - Use dev_err_probe()
  Input: da9063 - Add polling support

 drivers/input/misc/da9063_onkey.c | 146 +++++++++++++++++-------------
 1 file changed, 84 insertions(+), 62 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] Input: da9063 - Simplify obtaining OF match data

[Biju Das]

Simplify probe() by replacing of_match_node() for retrieving match data by
device_get_match_data().

Some minor cleanups:
 * Remove the trailing comma in the terminator entry for the OF
   table making code robust against (theoretical) misrebases or other
   similar things where the new entry goes _after_ the termination without
   the compiler noticing.
 * Move OF table near to the user.
 * Arrange variables in reverse xmas tree order in probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change.
---
 drivers/input/misc/da9063_onkey.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 74808bae326a..9351ce0bb405 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -74,13 +74,6 @@ static const struct da906x_chip_config da9062_regs = {
 	.name = "da9062-onkey",
 };
 
-static const struct of_device_id da9063_compatible_reg_id_table[] = {
-	{ .compatible = "dlg,da9063-onkey", .data = &da9063_regs },
-	{ .compatible = "dlg,da9062-onkey", .data = &da9062_regs },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, da9063_compatible_reg_id_table);
-
 static void da9063_poll_on(struct work_struct *work)
 {
 	struct da9063_onkey *onkey = container_of(work,
@@ -187,14 +180,8 @@ static irqreturn_t da9063_onkey_irq_handler(int irq, void *data)
 static int da9063_onkey_probe(struct platform_device *pdev)
 {
 	struct da9063_onkey *onkey;
-	const struct of_device_id *match;
-	int irq;
 	int error;
-
-	match = of_match_node(da9063_compatible_reg_id_table,
-			      pdev->dev.of_node);
-	if (!match)
-		return -ENXIO;
+	int irq;
 
 	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
 			     GFP_KERNEL);
@@ -203,7 +190,10 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	onkey->config = match->data;
+	onkey->config = device_get_match_data(&pdev->dev);
+	if (!onkey->config)
+		return -ENXIO;
+
 	onkey->dev = &pdev->dev;
 
 	onkey->regmap = dev_get_regmap(pdev->dev.parent, NULL);
@@ -270,6 +260,13 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id da9063_compatible_reg_id_table[] = {
+	{ .compatible = "dlg,da9063-onkey", .data = &da9063_regs },
+	{ .compatible = "dlg,da9062-onkey", .data = &da9062_regs },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, da9063_compatible_reg_id_table);
+
 static struct platform_driver da9063_onkey_driver = {
 	.probe	= da9063_onkey_probe,
 	.driver	= {
-- 
2.25.1

Re: [PATCH v2 1/4] Input: da9063 - Simplify obtaining OF match data

[Dmitry Torokhov]

On Wed, Dec 13, 2023 at 09:48:00PM +0000, Biju Das wrote:
> Simplify probe() by replacing of_match_node() for retrieving match data by
> device_get_match_data().
> 
> Some minor cleanups:
>  * Remove the trailing comma in the terminator entry for the OF
>    table making code robust against (theoretical) misrebases or other
>    similar things where the new entry goes _after_ the termination without
>    the compiler noticing.
>  * Move OF table near to the user.
>  * Arrange variables in reverse xmas tree order in probe().
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Applied, thank you.

-- 
Dmitry

[PATCH v2 2/4] Input: da9063 - Drop redundant prints in probe()

[Biju Das]

The memory allocation core code already prints error message in case of
OOM. So, drop additional print messages for OOM cases.

While at it, input_register_device() is already printing error messages on
failure. Drop the redundant print.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2:
 * New patch.
---
 drivers/input/misc/da9063_onkey.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 9351ce0bb405..80878274204e 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -185,10 +185,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 
 	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
 			     GFP_KERNEL);
-	if (!onkey) {
-		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+	if (!onkey)
 		return -ENOMEM;
-	}
 
 	onkey->config = device_get_match_data(&pdev->dev);
 	if (!onkey->config)
@@ -206,10 +204,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 						  "dlg,disable-key-power");
 
 	onkey->input = devm_input_allocate_device(&pdev->dev);
-	if (!onkey->input) {
-		dev_err(&pdev->dev, "Failed to allocated input device.\n");
+	if (!onkey->input)
 		return -ENOMEM;
-	}
 
 	onkey->input->name = onkey->config->name;
 	snprintf(onkey->phys, sizeof(onkey->phys), "%s/input0",
@@ -221,12 +217,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 
 	error = devm_delayed_work_autocancel(&pdev->dev, &onkey->work,
 					     da9063_poll_on);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to add cancel poll action: %d\n",
-			error);
+	if (error)
 		return error;
-	}
 
 	irq = platform_get_irq_byname(pdev, "ONKEY");
 	if (irq < 0)
@@ -250,14 +242,7 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	else
 		device_init_wakeup(&pdev->dev, true);
 
-	error = input_register_device(onkey->input);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to register input device: %d\n", error);
-		return error;
-	}
-
-	return 0;
+	return input_register_device(onkey->input);
 }
 
 static const struct of_device_id da9063_compatible_reg_id_table[] = {
-- 
2.25.1

Re: [PATCH v2 2/4] Input: da9063 - Drop redundant prints in probe()

[Dmitry Torokhov]

On Wed, Dec 13, 2023 at 09:48:01PM +0000, Biju Das wrote:
> The memory allocation core code already prints error message in case of
> OOM. So, drop additional print messages for OOM cases.
> 
> While at it, input_register_device() is already printing error messages on
> failure. Drop the redundant print.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2:
>  * New patch.
> ---
>  drivers/input/misc/da9063_onkey.c | 23 ++++-------------------
>  1 file changed, 4 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
> index 9351ce0bb405..80878274204e 100644
> --- a/drivers/input/misc/da9063_onkey.c
> +++ b/drivers/input/misc/da9063_onkey.c
> @@ -185,10 +185,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
>  
>  	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
>  			     GFP_KERNEL);
> -	if (!onkey) {
> -		dev_err(&pdev->dev, "Failed to allocate memory.\n");
> +	if (!onkey)
>  		return -ENOMEM;
> -	}
>  
>  	onkey->config = device_get_match_data(&pdev->dev);
>  	if (!onkey->config)
> @@ -206,10 +204,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
>  						  "dlg,disable-key-power");
>  
>  	onkey->input = devm_input_allocate_device(&pdev->dev);
> -	if (!onkey->input) {
> -		dev_err(&pdev->dev, "Failed to allocated input device.\n");
> +	if (!onkey->input)
>  		return -ENOMEM;
> -	}
>  
>  	onkey->input->name = onkey->config->name;
>  	snprintf(onkey->phys, sizeof(onkey->phys), "%s/input0",
> @@ -221,12 +217,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
>  
>  	error = devm_delayed_work_autocancel(&pdev->dev, &onkey->work,
>  					     da9063_poll_on);
> -	if (error) {
> -		dev_err(&pdev->dev,
> -			"Failed to add cancel poll action: %d\n",
> -			error);
> +	if (error)
>  		return error;
> -	}
>  
>  	irq = platform_get_irq_byname(pdev, "ONKEY");
>  	if (irq < 0)
> @@ -250,14 +242,7 @@ static int da9063_onkey_probe(struct platform_device *pdev)
>  	else
>  		device_init_wakeup(&pdev->dev, true);
>  
> -	error = input_register_device(onkey->input);
> -	if (error) {
> -		dev_err(&pdev->dev,
> -			"Failed to register input device: %d\n", error);
> -		return error;
> -	}
> -
> -	return 0;
> +	return input_register_device(onkey->input);

When there are multiple exit points I prefer all of them to use form

	error = action(...);
	if (error)
		return error;

	...
	return 0;

Fixed up and applied, thank you.

Thanks.

-- 
Dmitry

[PATCH v2 3/4] Input: da9063 - Use dev_err_probe()

[Biju Das]

Replace dev_err()->dev_err_probe() to simplify probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Fixed typo in commit description
 * Updated the print message for irq allocation failure.
---
 drivers/input/misc/da9063_onkey.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 80878274204e..b18232e91844 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -195,10 +195,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	onkey->dev = &pdev->dev;
 
 	onkey->regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!onkey->regmap) {
-		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
-		return -ENXIO;
-	}
+	if (!onkey->regmap)
+		return dev_err_probe(&pdev->dev, -ENXIO,
+				     "Parent regmap unavailable.\n");
 
 	onkey->key_power = !of_property_read_bool(pdev->dev.of_node,
 						  "dlg,disable-key-power");
@@ -228,11 +227,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 					  NULL, da9063_onkey_irq_handler,
 					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 					  "ONKEY", onkey);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to request IRQ %d: %d\n", irq, error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				     "Failed to allocate onkey IRQ\n");
 
 	error = dev_pm_set_wake_irq(&pdev->dev, irq);
 	if (error)
-- 
2.25.1

Re: [PATCH v2 3/4] Input: da9063 - Use dev_err_probe()

[Dmitry Torokhov]

On Wed, Dec 13, 2023 at 09:48:02PM +0000, Biju Das wrote:
> Replace dev_err()->dev_err_probe() to simplify probe().
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Applied, thank you.

-- 
Dmitry

[PATCH v2 4/4] Input: da9063 - Add polling support

[Biju Das]

On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no
onkey IRQ populated by default. Add polling support.

While at it, doing some cleanups in da9063_poll_on()
as regmap_read() and dev_err() can fit in single line.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated commit description
 * Fixed the logical mistake for optional IRQ handling.
---
 drivers/input/misc/da9063_onkey.c | 89 +++++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 23 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index b18232e91844..734cee9e9253 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -19,6 +19,8 @@
 #include <linux/mfd/da9062/core.h>
 #include <linux/mfd/da9062/registers.h>
 
+#define DA9062_KEY_THRESHOLD_MSEC	(200)
+
 struct da906x_chip_config {
 	/* REGS */
 	int onkey_status;
@@ -42,6 +44,8 @@ struct da9063_onkey {
 	const struct da906x_chip_config *config;
 	char phys[32];
 	bool key_power;
+	unsigned int poll_interval;
+	unsigned int key_threshold_release_time;
 };
 
 static const struct da906x_chip_config da9063_regs = {
@@ -86,15 +90,27 @@ static void da9063_poll_on(struct work_struct *work)
 	int error;
 
 	/* Poll to see when the pin is released */
-	error = regmap_read(onkey->regmap,
-			    config->onkey_status,
-			    &val);
+	error = regmap_read(onkey->regmap, config->onkey_status, &val);
 	if (error) {
-		dev_err(onkey->dev,
-			"Failed to read ON status: %d\n", error);
+		dev_err(onkey->dev, "Failed to read ON status: %d\n", error);
 		goto err_poll;
 	}
 
+	if (onkey->poll_interval &&
+	    onkey->key_threshold_release_time <= DA9062_KEY_THRESHOLD_MSEC) {
+		/* detect short or long key press */
+		if (!(val & config->onkey_nonkey_mask)) {
+			input_report_key(onkey->input, KEY_POWER, 0);
+			input_sync(onkey->input);
+			onkey->key_threshold_release_time = 0;
+			dev_dbg(onkey->dev, "KEY_POWER short press.\n");
+		} else {
+			schedule_delayed_work(&onkey->work, msecs_to_jiffies(50));
+			onkey->key_threshold_release_time += 50;
+		}
+		return;
+	}
+
 	if (!(val & config->onkey_nonkey_mask)) {
 		error = regmap_update_bits(onkey->regmap,
 					   config->onkey_pwr_signalling,
@@ -177,6 +193,21 @@ static irqreturn_t da9063_onkey_irq_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void da9063_onkey_polled_poll(struct input_dev *input)
+{
+	struct da9063_onkey *onkey = input_get_drvdata(input);
+	const struct da906x_chip_config *config = onkey->config;
+	unsigned int val;
+	int error;
+
+	error = regmap_read(onkey->regmap, config->onkey_status, &val);
+	if (onkey->key_power && !error && (val & config->onkey_nonkey_mask)) {
+		input_report_key(onkey->input, KEY_POWER, 1);
+		input_sync(onkey->input);
+		schedule_delayed_work(&onkey->work, 0);
+	}
+}
+
 static int da9063_onkey_probe(struct platform_device *pdev)
 {
 	struct da9063_onkey *onkey;
@@ -219,25 +250,37 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
-	irq = platform_get_irq_byname(pdev, "ONKEY");
-	if (irq < 0)
+	irq = platform_get_irq_byname_optional(pdev, "ONKEY");
+	if (irq >= 0) {
+		error = devm_request_threaded_irq(&pdev->dev, irq,
+						  NULL, da9063_onkey_irq_handler,
+						  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						  "ONKEY", onkey);
+		if (error)
+			return dev_err_probe(&pdev->dev, error,
+					     "Failed to allocate onkey irq\n");
+
+		error = dev_pm_set_wake_irq(&pdev->dev, irq);
+		if (error)
+			dev_warn(&pdev->dev,
+				 "Failed to set IRQ %d as a wake IRQ: %d\n",
+				 irq, error);
+		else
+			device_init_wakeup(&pdev->dev, true);
+	} else if (irq != -ENXIO) {
 		return irq;
-
-	error = devm_request_threaded_irq(&pdev->dev, irq,
-					  NULL, da9063_onkey_irq_handler,
-					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					  "ONKEY", onkey);
-	if (error)
-		return dev_err_probe(&pdev->dev, error,
-				     "Failed to allocate onkey IRQ\n");
-
-	error = dev_pm_set_wake_irq(&pdev->dev, irq);
-	if (error)
-		dev_warn(&pdev->dev,
-			 "Failed to set IRQ %d as a wake IRQ: %d\n",
-			 irq, error);
-	else
-		device_init_wakeup(&pdev->dev, true);
+	} else {
+		input_set_drvdata(onkey->input, onkey);
+		device_property_read_u32(&pdev->dev, "poll-interval",
+					 &onkey->poll_interval);
+		error = input_setup_polling(onkey->input,
+					    da9063_onkey_polled_poll);
+		if (error)
+			return dev_err_probe(&pdev->dev, error,
+					     "unable to set up polling\n");
+
+		input_set_poll_interval(onkey->input, onkey->poll_interval);
+	}
 
 	return input_register_device(onkey->input);
 }
-- 
2.25.1

Re: [PATCH v2 4/4] Input: da9063 - Add polling support

[Dmitry Torokhov]

On Wed, Dec 13, 2023 at 09:48:03PM +0000, Biju Das wrote:
> +static void da9063_onkey_polled_poll(struct input_dev *input)
> +{
> +	struct da9063_onkey *onkey = input_get_drvdata(input);
> +	const struct da906x_chip_config *config = onkey->config;
> +	unsigned int val;
> +	int error;
> +
> +	error = regmap_read(onkey->regmap, config->onkey_status, &val);
> +	if (onkey->key_power && !error && (val & config->onkey_nonkey_mask)) {
> +		input_report_key(onkey->input, KEY_POWER, 1);
> +		input_sync(onkey->input);
> +		schedule_delayed_work(&onkey->work, 0);

In the polling case you should not be scheduling any additional works as
the driver may get confused if you repeatedly open and close input
device.

Also I think in threaded case it might be cleaner to avoid scheduling
work and simply loop in the interrupt thread (since it can sleep).

Thanks.

-- 
Dmitry

Re: [PATCH v2 4/4] Input: da9063 - Add polling support

[Biju Das]

Hi Dmitry,

Thanks for the feedback.

On Tue, Dec 19, 2023 at 1:52 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Dec 13, 2023 at 09:48:03PM +0000, Biju Das wrote:
> > +static void da9063_onkey_polled_poll(struct input_dev *input)
> > +{
> > +     struct da9063_onkey *onkey = input_get_drvdata(input);
> > +     const struct da906x_chip_config *config = onkey->config;
> > +     unsigned int val;
> > +     int error;
> > +
> > +     error = regmap_read(onkey->regmap, config->onkey_status, &val);
> > +     if (onkey->key_power && !error && (val & config->onkey_nonkey_mask)) {
> > +             input_report_key(onkey->input, KEY_POWER, 1);
> > +             input_sync(onkey->input);
> > +             schedule_delayed_work(&onkey->work, 0);
>
> In the polling case you should not be scheduling any additional works as
> the driver may get confused if you repeatedly open and close input
> device.

OK.

>
> Also I think in threaded case it might be cleaner to avoid scheduling
> work and simply loop in the interrupt thread (since it can sleep).

Agreed. Will fix this in the next version.

Cheers,
Biju