Re: [PATCH v3 2/3] Input: twl4030 - add TWL603x power button

[Dmitry Torokhov]

Hi Andreas,

On Wed, Nov 05, 2025 at 08:52:36PM +0100, akemnade@xxxxxxxxxx wrote:
> From: Andreas Kemnade <andreas@xxxxxxxxxxxx>
> 
> Like the TWL4030, these PMICs also have a power button feature, so extend
> the TWL4030 power button driver. As the irqchip of the TWL6030 mfd driver
> does not provide mask, unmask finctions, do it manually.
> 
> Signed-off-by: Andreas Kemnade <andreas@xxxxxxxxxxxx>
> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 61 +++++++++++++++++++++++++++++++---
>  1 file changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index f85cc289c053..b72fba9a1b2c 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -25,22 +25,40 @@
>  #include <linux/kernel.h>
>  #include <linux/errno.h>
>  #include <linux/input.h>
> +#include <linux/bits.h>

Move to the top of includes please.

>  #include <linux/interrupt.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/twl.h>
>  
> -#define PWR_PWRON_IRQ (1 << 0)
> +#define PWR_PWRON_IRQ BIT(0)
>  
> -#define STS_HW_CONDITIONS 0xf
> +#define STS_HW_CONDITIONS_4030 0xf
> +#define STS_HW_CONDITIONS_6030 0x2

Probably no need for these defines, just use numbers in structure
instances.

> +
> +struct twl_pwrbutton_chipdata {
> +	u8 status_reg;
> +	bool need_manual_irq;
> +};
> +
> +static const struct twl_pwrbutton_chipdata twl4030_chipdata = {
> +	STS_HW_CONDITIONS_4030,
> +	false,

I am a big fan of named initializers, so maybe

	.status_reg = 0x0f,
	.need_manual_irq = false,

?

Otherwise looks good.

Thanks.

-- 
Dmitry