linux-kernel - Re: [PATCH v3 2/3] Input: twl4030

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <tyx4vvapd4pca6e236rcjkbxecor5kderzoinbwyuecdclzcix@jgksmvfioc4x>
Date: Wed, 5 Nov 2025 14:18:08 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: akemnade@...nel.org
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Andreas Kemnade <andreas@...nade.info>, Tony Lindgren <tony@...mide.com>, 
	Kevin Hilman <khilman@...nel.org>, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-input@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH v3 2/3] Input: twl4030 - add TWL603x power button

Hi Andreas,

On Wed, Nov 05, 2025 at 08:52:36PM +0100, akemnade@...nel.org wrote:
> From: Andreas Kemnade <andreas@...nade.info>
> 
> 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@...nade.info>
> ---
>  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