[PATCH] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration

Gokul Praveen posted 1 patch 1 month ago
There is a newer version of this series
drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
[PATCH] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration
Posted by Gokul Praveen 1 month ago
The period and duty cycle configurations does not get reflected
after setting them using sysfs nodes. This is because at the
end of ehrpwm_pwm_config function, the put_sync function is
called which resets the hardware.

Fix it by preventing the pwm controller from going into
low-power mode.

'Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement
.apply() callback")'

Signed-off-by: Gokul Praveen <g-praveen@ti.com>
---
 drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7a86cb090f76..408aed70be8c 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (period_cycles < 1)
 		period_cycles = 1;
 
-	pm_runtime_get_sync(pwmchip_parent(chip));
 
 	/* Update clock prescaler values */
 	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
@@ -290,12 +289,11 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (!(duty_cycles > period_cycles))
 		ehrpwm_write(pc->mmio_base, cmp_reg, duty_cycles);
 
-	pm_runtime_put_sync(pwmchip_parent(chip));
-
 	return 0;
 }
 
-static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
+				const struct pwm_state *state)
 {
 	struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
 	u16 aqcsfrc_val, aqcsfrc_mask;
@@ -304,6 +302,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	/* Leave clock enabled on enabling PWM */
 	pm_runtime_get_sync(pwmchip_parent(chip));
 
+	ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
+
+	if (ret) {
+		pm_runtime_put_sync(pwmchip_parent(chip));
+		return ret;
+	}
+
 	/* Disabling Action Qualifier on PWM output */
 	if (pwm->hwpwm) {
 		aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
@@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		return 0;
 	}
 
-	err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
-	if (err)
-		return err;
-
 	if (!enabled)
-		err = ehrpwm_pwm_enable(chip, pwm);
+		err = ehrpwm_pwm_enable(chip, pwm, state);
+	else
+		err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
+					state->period, state->polarity);
 
 	return err;
 }
-- 
2.34.1
Re: [PATCH] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration
Posted by Kumar, Udit 1 month ago
On 1/6/2026 4:24 PM, Gokul Praveen wrote:
> The period and duty cycle configurations does not get reflected
> after setting them using sysfs nodes. This is because at the
> end of ehrpwm_pwm_config function, the put_sync function is
> called which resets the hardware.
>
> Fix it by preventing the pwm controller from going into
> low-power mode.
>
> 'Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement
> .apply() callback")'
>
> Signed-off-by: Gokul Praveen <g-praveen@ti.com>

No new line,  between Fixes and Signed-off-by Please ,

You can drop ' before fixes.

rest LGTM.


> ---
>   drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
>   1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..408aed70be8c 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   	if (period_cycles < 1)
>   		period_cycles = 1;
>   
> -	pm_runtime_get_sync(pwmchip_parent(chip));
>   
>   	/* Update clock prescaler values */
>   	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
> @@ -290,12 +289,11 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   	if (!(duty_cycles > period_cycles))
>   		ehrpwm_write(pc->mmio_base, cmp_reg, duty_cycles);
>   
> -	pm_runtime_put_sync(pwmchip_parent(chip));
> -
>   	return 0;
>   }
>   
> -static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
> +				const struct pwm_state *state)
>   {
>   	struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
>   	u16 aqcsfrc_val, aqcsfrc_mask;
> @@ -304,6 +302,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>   	/* Leave clock enabled on enabling PWM */
>   	pm_runtime_get_sync(pwmchip_parent(chip));
>   
> +	ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
> +
> +	if (ret) {
> +		pm_runtime_put_sync(pwmchip_parent(chip));
> +		return ret;
> +	}
> +
>   	/* Disabling Action Qualifier on PWM output */
>   	if (pwm->hwpwm) {
>   		aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
> @@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>   		return 0;
>   	}
>   
> -	err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
> -	if (err)
> -		return err;
> -
>   	if (!enabled)
> -		err = ehrpwm_pwm_enable(chip, pwm);
> +		err = ehrpwm_pwm_enable(chip, pwm, state);
> +	else
> +		err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
> +					state->period, state->polarity);
>   
>   	return err;
>   }
Re: [PATCH] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration
Posted by Gokul Praveen 1 month ago
On 06/01/26 18:57, Kumar, Udit wrote:
> 
> On 1/6/2026 4:24 PM, Gokul Praveen wrote:
>> The period and duty cycle configurations does not get reflected
>> after setting them using sysfs nodes. This is because at the
>> end of ehrpwm_pwm_config function, the put_sync function is
>> called which resets the hardware.
>>
>> Fix it by preventing the pwm controller from going into
>> low-power mode.
>>
>> 'Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement
>> .apply() callback")'
>>
>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
> 
> No new line,  between Fixes and Signed-off-by Please ,
> 
> You can drop ' before fixes.
> 
> rest LGTM.

Sure, Udit. Thanks for that.

Best Regards
Gokul Praveen
> 
> 
>> ---
>>   drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
>>   1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
>> index 7a86cb090f76..408aed70be8c 100644
>> --- a/drivers/pwm/pwm-tiehrpwm.c
>> +++ b/drivers/pwm/pwm-tiehrpwm.c
>> @@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip 
>> *chip, struct pwm_device *pwm,
>>       if (period_cycles < 1)
>>           period_cycles = 1;
>> -    pm_runtime_get_sync(pwmchip_parent(chip));
>>       /* Update clock prescaler values */
>>       ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
>> @@ -290,12 +289,11 @@ static int ehrpwm_pwm_config(struct pwm_chip 
>> *chip, struct pwm_device *pwm,
>>       if (!(duty_cycles > period_cycles))
>>           ehrpwm_write(pc->mmio_base, cmp_reg, duty_cycles);
>> -    pm_runtime_put_sync(pwmchip_parent(chip));
>> -
>>       return 0;
>>   }
>> -static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device 
>> *pwm)
>> +static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device 
>> *pwm,
>> +                const struct pwm_state *state)
>>   {
>>       struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
>>       u16 aqcsfrc_val, aqcsfrc_mask;
>> @@ -304,6 +302,13 @@ static int ehrpwm_pwm_enable(struct pwm_chip 
>> *chip, struct pwm_device *pwm)
>>       /* Leave clock enabled on enabling PWM */
>>       pm_runtime_get_sync(pwmchip_parent(chip));
>> +    ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state- 
>> >period, state->polarity);
>> +
>> +    if (ret) {
>> +        pm_runtime_put_sync(pwmchip_parent(chip));
>> +        return ret;
>> +    }
>> +
>>       /* Disabling Action Qualifier on PWM output */
>>       if (pwm->hwpwm) {
>>           aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
>> @@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip 
>> *chip, struct pwm_device *pwm,
>>           return 0;
>>       }
>> -    err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state- 
>> >period, state->polarity);
>> -    if (err)
>> -        return err;
>> -
>>       if (!enabled)
>> -        err = ehrpwm_pwm_enable(chip, pwm);
>> +        err = ehrpwm_pwm_enable(chip, pwm, state);
>> +    else
>> +        err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
>> +                    state->period, state->polarity);
>>       return err;
>>   }