| 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
| ||
|
Message-ID: <sxdlmeurozq4laahoe2aonkw5jzpjxleafg7o7iji3cp6rlfea@6h4w2vcn55lh>
Date: Thu, 15 Jan 2026 10:36:24 +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 v7 6/9] pwm: tiehrpwm: factor out AQ force codes and
polarity presets
On Thu, Nov 27, 2025 at 09:36:31PM -0300, Rafael V. Volkmer wrote:
> Introduce named constants for the Action-Qualifier force action codes
> and use them to build the CAU/CAD/CBU/CBD/PRD/ZRO bitfield helpers
> instead of repeating hard-coded numeric values in each field.
>
> While at it, split the channel polarity presets into explicit up-count
> and down-count variants for both channels. This keeps the resulting
> AQCTL programming unchanged but makes the configuration easier to read
> and extend.
>
> No functional change intended.
>
> Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@...il.com>
> ---
> drivers/pwm/pwm-tiehrpwm.c | 85 +++++++++++++++++++++++++++++---------
> 1 file changed, 65 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 41af1bf74cbb..e8bcf1ffa770 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -52,33 +52,78 @@
> #define TIEHRPWM_AQSFRC 0x1A
> #define TIEHRPWM_AQCSFRC 0x1C
>
> +/* Action-Qualifier force action codes (per 2-bit field) */
> +#define TIEHRPWM_AQCTL_FRCLOW 0x1
> +#define TIEHRPWM_AQCTL_FRCHIGH 0x2
> +#define TIEHRPWM_AQCTL_FRCTOGGLE 0x3
while renaming the constants, and given that there are to AQCTL
registers (AQCTLA and AQCTLB), I suggest to use AQCTLx as register
identifier.
> +/* Action-Qualifier bitfields for compare/period/zero events */
> #define TIEHRPWM_AQCTL_CBU_MASK GENMASK(9, 8)
> -#define TIEHRPWM_AQCTL_CBU_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, 1)
> -#define TIEHRPWM_AQCTL_CBU_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, 2)
> -#define TIEHRPWM_AQCTL_CBU_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, 3)
> +#define TIEHRPWM_AQCTL_CBU_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
I think it's fine to skip the linebreak here and yield lines >80 char.
If you want to keep the linebreak, please align the continuing line to
the opening (.
> +#define TIEHRPWM_AQCTL_CBU_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CBU_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CBU_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
> +
> +#define TIEHRPWM_AQCTL_CBD_MASK GENMASK(11, 10)
> +#define TIEHRPWM_AQCTL_CBD_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CBD_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
> +#define TIEHRPWM_AQCTL_CBD_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CBD_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CBD_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CBD_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
>
> #define TIEHRPWM_AQCTL_CAU_MASK GENMASK(5, 4)
> -#define TIEHRPWM_AQCTL_CAU_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, 1)
> -#define TIEHRPWM_AQCTL_CAU_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, 2)
> -#define TIEHRPWM_AQCTL_CAU_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, 3)
> +#define TIEHRPWM_AQCTL_CAU_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
> +#define TIEHRPWM_AQCTL_CAU_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CAU_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CAU_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
> +
> +#define TIEHRPWM_AQCTL_CAD_MASK GENMASK(7, 6)
> +#define TIEHRPWM_AQCTL_CAD_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_CAD_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
> +#define TIEHRPWM_AQCTL_CAD_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_CAD_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CAD_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_CAD_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
>
> #define TIEHRPWM_AQCTL_PRD_MASK GENMASK(3, 2)
> -#define TIEHRPWM_AQCTL_PRD_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, 1)
> -#define TIEHRPWM_AQCTL_PRD_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, 2)
> -#define TIEHRPWM_AQCTL_PRD_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, 3)
> +#define TIEHRPWM_AQCTL_PRD_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
> +#define TIEHRPWM_AQCTL_PRD_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_PRD_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_PRD_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
>
> #define TIEHRPWM_AQCTL_ZRO_MASK GENMASK(1, 0)
> -#define TIEHRPWM_AQCTL_ZRO_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, 1)
> -#define TIEHRPWM_AQCTL_ZRO_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, 2)
> -#define TIEHRPWM_AQCTL_ZRO_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, 3)
> +#define TIEHRPWM_AQCTL_ZRO_FRCLOW FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, \
> + TIEHRPWM_AQCTL_FRCLOW)
> +#define TIEHRPWM_AQCTL_ZRO_FRCHIGH FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, \
> + TIEHRPWM_AQCTL_FRCHIGH)
> +#define TIEHRPWM_AQCTL_ZRO_FRCTOGGLE FIELD_PREP(TIEHRPWM_AQCTL_ZRO_MASK, \
> + TIEHRPWM_AQCTL_FRCTOGGLE)
> +
> +/* Action-Qualifier polarity presets for up-count mode */
> +#define TIEHRPWM_AQCTL_CHA_UP_POLNORMAL (TIEHRPWM_AQCTL_CAU_FRCLOW | \
> + TIEHRPWM_AQCTL_ZRO_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CHA_UP_POLINVERSE (TIEHRPWM_AQCTL_CAU_FRCHIGH | \
> + TIEHRPWM_AQCTL_ZRO_FRCLOW)
> +#define TIEHRPWM_AQCTL_CHB_UP_POLNORMAL (TIEHRPWM_AQCTL_CBU_FRCLOW | \
> + TIEHRPWM_AQCTL_ZRO_FRCHIGH)
> +#define TIEHRPWM_AQCTL_CHB_UP_POLINVERSE (TIEHRPWM_AQCTL_CBU_FRCHIGH | \
> + TIEHRPWM_AQCTL_ZRO_FRCLOW)
>
> -#define TIEHRPWM_AQCTL_CHANA_POLNORMAL (TIEHRPWM_AQCTL_CAU_FRCLOW | \
> +/* Action-Qualifier polarity presets for down-count mode */
> +#define TIEHRPWM_AQCTL_CHA_DN_POLNORMAL (TIEHRPWM_AQCTL_CAU_FRCLOW | \
> TIEHRPWM_AQCTL_ZRO_FRCHIGH)
> -#define TIEHRPWM_AQCTL_CHANA_POLINVERSED (TIEHRPWM_AQCTL_CAU_FRCHIGH | \
> +#define TIEHRPWM_AQCTL_CHA_DN_POLINVERSE (TIEHRPWM_AQCTL_CAU_FRCHIGH | \
> TIEHRPWM_AQCTL_ZRO_FRCLOW)
> -#define TIEHRPWM_AQCTL_CHANB_POLNORMAL (TIEHRPWM_AQCTL_CBU_FRCLOW | \
> +#define TIEHRPWM_AQCTL_CHB_DN_POLNORMAL (TIEHRPWM_AQCTL_CBU_FRCLOW | \
> TIEHRPWM_AQCTL_ZRO_FRCHIGH)
> -#define TIEHRPWM_AQCTL_CHANB_POLINVERSED (TIEHRPWM_AQCTL_CBU_FRCHIGH | \
> +#define TIEHRPWM_AQCTL_CHB_DN_POLINVERSE (TIEHRPWM_AQCTL_CBU_FRCHIGH | \
> TIEHRPWM_AQCTL_ZRO_FRCLOW)
>
> #define TIEHRPWM_AQSFRC_RLDCSF_MASK GENMASK(7, 6)
> @@ -262,9 +307,9 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> aqctl_mask = TIEHRPWM_AQCTL_CBU_MASK;
>
> if (polarity == PWM_POLARITY_INVERSED)
> - aqctl_val = TIEHRPWM_AQCTL_CHANB_POLINVERSED;
> + aqctl_val = TIEHRPWM_AQCTL_CHB_UP_POLINVERSE;
When I worked on the driver I wondered if these constants are really
helpful or if explicit spelling out would be more readable, i.e.
aqctl_val = TIEHRPWM_AQCTL_CBU_FRCHIGH | TIEHRPWM_AQCTL_ZRO_FRCLOW;
> else
> - aqctl_val = TIEHRPWM_AQCTL_CHANB_POLNORMAL;
> + aqctl_val = TIEHRPWM_AQCTL_CHB_UP_POLNORMAL;
>
> /* if duty_cycle is big, don't toggle on CBU */
> if (duty_cycles > period_cycles)
> @@ -278,9 +323,9 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> aqctl_mask = TIEHRPWM_AQCTL_CAU_MASK;
>
> if (polarity == PWM_POLARITY_INVERSED)
> - aqctl_val = TIEHRPWM_AQCTL_CHANA_POLINVERSED;
> + aqctl_val = TIEHRPWM_AQCTL_CHA_UP_POLINVERSE;
> else
> - aqctl_val = TIEHRPWM_AQCTL_CHANA_POLNORMAL;
> + aqctl_val = TIEHRPWM_AQCTL_CHA_UP_POLNORMAL;
>
> /* if duty_cycle is big, don't toggle on CAU */
> if (duty_cycles > period_cycles)
> --
> 2.43.0
Apart from the comments, I think the patch is an improvement.
Best regards
Uwe
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists