linux-kernel - Re: [PATCH v2] pwm: imx-tpm: reset counter if CMOD is 0

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: <7wsbi7pcwa2otxqkon3zfat7faprhr3l7wc35fkzctmm3cv67m@ph6w7hzwt322>
Date: Mon, 21 Jul 2025 13:03:59 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Laurentiu Mihalcea <laurentiumihalcea111@...il.com>
Cc: Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	linux-pwm@...r.kernel.org, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] pwm: imx-tpm: reset counter if CMOD is 0

Hello Laurentiu,

On Mon, Jul 14, 2025 at 08:36:34AM -0400, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@....com>
> 
> As per the i.MX93 TRM, section 67.3.2.1 "MOD register update", the value
> of the TPM counter does NOT get updated when writing MOD.MOD unless
> SC.CMOD != 0. Therefore, with the current code, assuming the following
> sequence:
> 
> 	1) pwm_disable()
> 	2) pwm_apply_might_sleep() /* period is changed here */
> 	3) pwm_enable()
> 
> and assuming only one channel is active, if CNT.COUNT is higher than the
> MOD.MOD value written during the pwm_apply_might_sleep() call then, when
> re-enabling the PWM during pwm_enable(), the counter will end up resetting
> after UINT32_MAX - CNT.COUNT + MOD.MOD cycles instead of MOD.MOD cycles as
> normally expected.
> 
> Fix this problem by forcing a reset of the TPM counter before MOD.MOD is
> written.
> 
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@....com>

This needs backporting to stable, right? So we need a reference to the
commit that introduced the problem. I guess that's 738a1cfec2ed ("pwm:
Add i.MX TPM PWM driver support")? (Please add a matching Fixes: line in
your v3.)

> ---
> Changes in v2:
>   - dropped the "VERY IMPORTANT" bit as per Uwe's suggestion.
>   - Link to v1: https://lore.kernel.org/lkml/20250701220147.1007786-1-laurentiumihalcea111@gmail.com/
> 
>  drivers/pwm/pwm-imx-tpm.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
> index 7ee7b65b9b90..b15c22796ba9 100644
> --- a/drivers/pwm/pwm-imx-tpm.c
> +++ b/drivers/pwm/pwm-imx-tpm.c
> @@ -204,6 +204,19 @@ static int pwm_imx_tpm_apply_hw(struct pwm_chip *chip,
>  		val |= FIELD_PREP(PWM_IMX_TPM_SC_PS, p->prescale);
>  		writel(val, tpm->base + PWM_IMX_TPM_SC);
>  
> +		/*
> +		 * if CMOD is set to 0 then writing MOD will NOT reset the
> +		 * value of the TPM counter.
> +		 *
> +		 * Therefore, if CNT.COUNT > MOD.MOD, the counter will reset
> +		 * after UINT32_MAX - CNT.COUNT + MOD.MOD cycles, which is
> +		 * incorrect.
> +		 *
> +		 * To avoid this, we need to force a reset of the
> +		 * counter before writing the new MOD value.
> +		 */

I asked in reply to v1 about these register semantics. The idea was not
that you explain them by mail, but improve the comment accordingly that
someone reading the driver doesn't need to consult the reference manual
to understand it.

So maybe something like:

	/*
	 * If the counter is disabled (CMOD == 0), programming the new
	 * period length (MOD) doesn't reset the counter (CNT). If
	 * CNT.COUNT happens to be bigger than the new MOD value it will
	 * reset way to late. So reset it manually to zero.
	 */

?

> +		if (!cmod)
> +			writel(0x0, tpm->base + PWM_IMX_TPM_CNT);
>  		/*
>  		 * set period count:
>  		 * if the PWM is disabled (CMOD[1:0] = 2b00), then MOD register

Best regards
Uwe

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