2023-09-29 21:12:54

by Andreas Kemnade

Subject: [RFC PATCH] Input: omap4-keypad: react on keypresses if device is runtime-suspended

According to SWPU235AB, table 26-6, fclk is required to generate events
at least on OMAP4460, so keep fclk enabled all the time the device
is opened.

Suggested-by: Tony Lindgren <[email protected]>
Signed-off-by: Andreas Kemnade <[email protected]>
---
Open questions:
- Should we rather (or also use) padconf irqs?
- It seems not to be required everywhere. Does it harm somewhere?

drivers/input/keyboard/omap4-keypad.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index 773e55eed88b1..a7585a09c48b4 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
+#include <linux/clk.h>
#include <linux/errno.h>
#include <linux/io.h>
#include <linux/of.h>
@@ -83,6 +84,7 @@ struct omap4_keypad {
bool no_autorepeat;
u64 keys;
unsigned short *keymap;
+ struct clk *fck;
};

static int kbd_readl(struct omap4_keypad *keypad_data, u32 offset)
@@ -211,6 +213,8 @@ static int omap4_keypad_open(struct input_dev *input)

disable_irq(keypad_data->irq);

+ clk_prepare_enable(keypad_data->fck);
+
kbd_writel(keypad_data, OMAP4_KBD_CTRL,
OMAP4_DEF_CTRL_NOSOFTMODE |
(OMAP4_KEYPAD_PTV_DIV_128 << OMAP4_DEF_CTRL_PTV_SHIFT));
@@ -258,6 +262,7 @@ static void omap4_keypad_close(struct input_dev *input)
disable_irq(keypad_data->irq);
omap4_keypad_stop(keypad_data);
enable_irq(keypad_data->irq);
+ clk_disable_unprepare(keypad_data->fck);

pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
@@ -356,6 +361,11 @@ static int omap4_keypad_probe(struct platform_device *pdev)
}

keypad_data->irq = irq;
+ keypad_data->fck = devm_clk_get(&pdev->dev, "fck");
+ if (IS_ERR(keypad_data->fck))
+ return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->fck),
+ "unable to get fck");
+
mutex_init(&keypad_data->lock);
platform_set_drvdata(pdev, keypad_data);

--
2.39.2

2023-10-11 10:09:07

by Tony Lindgren

Subject: Re: [RFC PATCH] Input: omap4-keypad: react on keypresses if device is runtime-suspended

Hi,

* Andreas Kemnade <[email protected]> [230929 23:01]:
> According to SWPU235AB, table 26-6, fclk is required to generate events
> at least on OMAP4460, so keep fclk enabled all the time the device
> is opened.

Sorry for the delay, the patch looks good to me:

Reviewed-by: Tony Lindgren <[email protected]>

> Open questions:
> - Should we rather (or also use) padconf irqs?

The keypad is in the wakeup domain and has SYSC_OMAP2_ENAWAKEUP set in
the dts, so padconf irqs should not be needed as long as the device is
clocked. As the source clock is sys_32k_ck, not sure if gating it would
save any power, so padconf interrupts are probably not needed.

> - It seems not to be required everywhere. Does it harm somewhere?

Sounds like on 4430 the gate bit does not do anything while on 4460 it
does. The source clock seems to be sys_32k_ck so it's enabled anyways
and should not affect power management.

Regards,

Tony

2023-10-11 15:46:19

by Andreas Kemnade

Subject: Re: [RFC PATCH] Input: omap4-keypad: react on keypresses if device is runtime-suspended

On Wed, 11 Oct 2023 13:07:26 +0300
Tony Lindgren <[email protected]> wrote:

> Hi,
>
> * Andreas Kemnade <[email protected]> [230929 23:01]:
> > According to SWPU235AB, table 26-6, fclk is required to generate events
> > at least on OMAP4460, so keep fclk enabled all the time the device
> > is opened.
>
> Sorry for the delay, the patch looks good to me:
>
> Reviewed-by: Tony Lindgren <[email protected]>
>
So with all open questions resolved, this can be treated as
s/RFC //

Regards,
Andreas