linux-kernel - Re: [PATCH v6 1/6] pwm: tiehrpwm: use GENMASK() and FIELD

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <wzb7dggrrbehqilrnj2kb2ngopxocf34cz6c3zp446jvbzuhv5@tf72gdpuslfz>
Date: Mon, 24 Nov 2025 11:36:20 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: "Rafael V. Volkmer" <rafael.v.volkmer@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v6 1/6] pwm: tiehrpwm: use GENMASK() and FIELD_PREP() for
 register fields

On Sun, Nov 23, 2025 at 08:25:21PM -0300, Rafael V. Volkmer wrote:
> Convert register field definitions in AQCTL, AQCSFRC and TBCTL to use
> GENMASK() and FIELD_PREP(). This clarifies the bit layout, reduces
> hand-rolled shift logic and aligns the driver with common kernel patterns.
> 
> No functional change.
> 
> Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@...il.com>
> ---
>  drivers/pwm/pwm-tiehrpwm.c | 91 ++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..d2151065083b 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -13,6 +13,7 @@
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/of.h>
> +#include <linux/bitfield.h>
>  
>  /* EHRPWM registers and bits definitions */
>  
> @@ -21,22 +22,24 @@
>  #define TBPRD			0x0A
>  
>  #define TBCTL_PRDLD_MASK	BIT(3)

I'd prefer s/TBCTL_PRDLD_MASK/TBCTL_PRDLD/ here. This way _MASK isn't
special compared to _SHOW (and others).

As you touch the register defines it would also be great to give them a
common prefix (e.g. TIEHRPWM_).

> -#define TBCTL_PRDLD_SHDW	0
> -#define TBCTL_PRDLD_IMDT	BIT(3)
> -#define TBCTL_CLKDIV_MASK	(BIT(12) | BIT(11) | BIT(10) | BIT(9) | \
> -				BIT(8) | BIT(7))
> -#define TBCTL_CTRMODE_MASK	(BIT(1) | BIT(0))
> -#define TBCTL_CTRMODE_UP	0
> -#define TBCTL_CTRMODE_DOWN	BIT(0)
> -#define TBCTL_CTRMODE_UPDOWN	BIT(1)
> -#define TBCTL_CTRMODE_FREEZE	(BIT(1) | BIT(0))
> +#define TBCTL_PRDLD_SHDW	FIELD_PREP(TBCTL_PRDLD_MASK, 0)
> +#define TBCTL_PRDLD_IMDT	FIELD_PREP(TBCTL_PRDLD_MASK, 1)
> +
> +#define TBCTL_CLKDIV_MASK	GENMASK(12, 7)
> +
> +#define TBCTL_CTRMODE_MASK	GENMASK(1, 0)
> +#define TBCTL_CTRMODE_UP	FIELD_PREP(TBCTL_CTRMODE_MASK, 0)
> +#define TBCTL_CTRMODE_DOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 1)
> +#define TBCTL_CTRMODE_UPDOWN	FIELD_PREP(TBCTL_CTRMODE_MASK, 2)
> +#define TBCTL_CTRMODE_FREEZE	FIELD_PREP(TBCTL_CTRMODE_MASK, 3)
>  
>  #define TBCTL_HSPCLKDIV_SHIFT	7
>  #define TBCTL_CLKDIV_SHIFT	10
>  
>  #define CLKDIV_MAX		7
>  #define HSPCLKDIV_MAX		7
> -#define PERIOD_MAX		0x10000
> +#define TBPRD_BITS		16
> +#define PERIOD_MAX		BIT(TBPRD_BITS)

If you do something like:

	#define TIEHRPWM_TBPRD		0xa
	#define TIEHRPWM_TBPRD_TBPRD		GENMASK(15, 0)

(which I admit looks a bit redundant, but still represents the
register layout properly) you can use

	FIELD_MAX(TIEHRPWM_TBPRD_TBPRD)

to define the maximal period.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)