[PATCH v2] pwm: imx-tpm: keep channel state instead of counting

[PATCH v2] pwm: imx-tpm: keep channel state instead of counting

[Viorel Suman (OSS)]

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 the problem by replacing counting logic with per-channel state
handling and by aligning IP and driver state at probe.

Signed-off-by: Viorel Suman (OSS) <viorel.suman@oss.nxp.com>
---
Changes since v1:

1. Moved device state check into the probe function. 
2. Dropped {} for one line blocks.

 drivers/pwm/pwm-imx-tpm.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 5b399de16d60..c87688dfd406 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;
 };
 
@@ -282,14 +282,16 @@ 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);
@@ -353,7 +355,7 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 	unsigned int npwm;
-	u32 val;
+	u32 val, i;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
@@ -382,6 +384,13 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
 
 	mutex_init(&tpm->lock);
 
+	/* get enabled state for each channel */
+	for (i = 0; i < npwm; i++) {
+		val = readl(base + PWM_IMX_TPM_CnSC(i));
+		if (FIELD_GET(PWM_IMX_TPM_CnSC_ELS, val))
+			tpm->enabled_channels |= BIT(i);
+	}
+
 	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
@@ -394,7 +403,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

Re: [PATCH v2] pwm: imx-tpm: keep channel state instead of counting

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

Hello,

On Mon, Feb 02, 2026 at 12:47:38PM +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 the problem by replacing counting logic with per-channel state
> handling and by aligning IP and driver state at probe.
> 
> Signed-off-by: Viorel Suman (OSS) <viorel.suman@oss.nxp.com>

I wonder if the following change would be enough:

diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index 5b399de16d60..36f873133f94 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -352,7 +352,7 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
 	struct clk *clk;
 	void __iomem *base;
 	int ret;
-	unsigned int npwm;
+	unsigned int i, npwm;
 	u32 val;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
@@ -382,6 +382,12 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
 
 	mutex_init(&tpm->lock);
 
+	for (i = 0; i < npwm; ++i) {
+		val = readl(base + PWM_IMX_TPM_CnSC(i));
+		if (FIELD_GET(PWM_IMX_TPM_CnSC_ELS, val))
+			++tpm->enable_count;
+	}
+
 	ret = devm_pwmchip_add(&pdev->dev, chip);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] pwm: imx-tpm: keep channel state instead of counting

[Viorel Suman (OSS)]

Hello,

On 26-03-05 10:29:21, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Feb 02, 2026 at 12:47:38PM +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 the problem by replacing counting logic with per-channel state
> > handling and by aligning IP and driver state at probe.
> > 
> > Signed-off-by: Viorel Suman (OSS) <viorel.suman@oss.nxp.com>
> 
> I wonder if the following change would be enough:
> 
> diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
> index 5b399de16d60..36f873133f94 100644
> --- a/drivers/pwm/pwm-imx-tpm.c
> +++ b/drivers/pwm/pwm-imx-tpm.c
> @@ -352,7 +352,7 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
>  	struct clk *clk;
>  	void __iomem *base;
>  	int ret;
> -	unsigned int npwm;
> +	unsigned int i, npwm;
>  	u32 val;
>  
>  	base = devm_platform_ioremap_resource(pdev, 0);
> @@ -382,6 +382,12 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
>  
>  	mutex_init(&tpm->lock);
>  
> +	for (i = 0; i < npwm; ++i) {
> +		val = readl(base + PWM_IMX_TPM_CnSC(i));
> +		if (FIELD_GET(PWM_IMX_TPM_CnSC_ELS, val))
> +			++tpm->enable_count;
> +	}
> +

Yes, the change above would be enough, will send v3.

Regards,
Viorel