[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXztQkefafsgFJvN@lizhi-Precision-Tower-5810>
Date: Fri, 30 Jan 2026 12:41:22 -0500
From: Frank Li <Frank.li@....com>
To: "Viorel Suman (OSS)" <viorel.suman@....nxp.com>
Cc: Uwe Kleine-König <ukleinek@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, linux-pwm@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pwm: imx-tpm: keep channel state instead of counting
On Fri, Jan 30, 2026 at 04:37:20PM +0200, Viorel Suman (OSS) wrote:
> On a soft reset TPM PWM IP may preserve its internal state from
> previous runtime, therefore on a subsequent OS boot and driver
> probe "enable_count" value and TPM PWM IP internal channels
> "enabled" states may get unaligned. In consequence on a suspend/resume
> cycle the call "if (--tpm->enable_count == 0)" may lead to
> "enable_count" overflow the system being blocked from entering
> suspend due to:
>
> if (tpm->enable_count > 0)
> return -EBUSY;
>
> Fix this problem by replacing counting logic with per-channel state handling.
>
> Signed-off-by: Viorel Suman (OSS) <viorel.suman@....nxp.com>
> ---
> drivers/pwm/pwm-imx-tpm.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
> index 5b399de16d60..0f8643f4a70b 100644
> --- a/drivers/pwm/pwm-imx-tpm.c
> +++ b/drivers/pwm/pwm-imx-tpm.c
> @@ -62,7 +62,7 @@ struct imx_tpm_pwm_chip {
> void __iomem *base;
> struct mutex lock;
> u32 user_count;
> - u32 enable_count;
> + u32 enabled_channels;
> u32 real_period;
> };
>
> @@ -166,6 +166,10 @@ static int pwm_imx_tpm_get_state(struct pwm_chip *chip,
>
> /* get channel status */
> state->enabled = FIELD_GET(PWM_IMX_TPM_CnSC_ELS, val) ? true : false;
> + if (state->enabled)
> + tpm->enabled_channels |= BIT(pwm->hwpwm);
> + else
> + tpm->enabled_channels &= ~BIT(pwm->hwpwm);
Do you have lock for RMW? or you should atomic_or() and atomic_and()
Frank
>
> return 0;
> }
> @@ -282,15 +286,19 @@ static int pwm_imx_tpm_apply_hw(struct pwm_chip *chip,
> }
> writel(val, tpm->base + PWM_IMX_TPM_CnSC(pwm->hwpwm));
>
> - /* control the counter status */
> + /* control the channel state */
> if (state->enabled != c.enabled) {
> val = readl(tpm->base + PWM_IMX_TPM_SC);
> if (state->enabled) {
> - if (++tpm->enable_count == 1)
> + if (tpm->enabled_channels == 0) {
> val |= PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLK;
> + }
> + tpm->enabled_channels |= BIT(pwm->hwpwm);
> } else {
> - if (--tpm->enable_count == 0)
> + tpm->enabled_channels &= ~BIT(pwm->hwpwm);
> + if (tpm->enabled_channels == 0) {
> val &= ~PWM_IMX_TPM_SC_CMOD;
> + }
> }
> writel(val, tpm->base + PWM_IMX_TPM_SC);
> }
> @@ -394,7 +402,7 @@ static int pwm_imx_tpm_suspend(struct device *dev)
> struct imx_tpm_pwm_chip *tpm = dev_get_drvdata(dev);
> int ret;
>
> - if (tpm->enable_count > 0)
> + if (tpm->enabled_channels > 0)
> return -EBUSY;
>
> /*
> --
> 2.34.1
>
Powered by blists - more mailing lists