Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | From | Leif Middelschulte <> | Subject | [PATCH] pwm: imx27: fix race condition .apply,.get_state | Date | Wed, 25 Jan 2023 17:01:42 +0100 |
| |
From: Leif Middelschulte <Leif.Middelschulte@klsmartin.com>
A race condition might occur, ultimately leading to switching off the PWM that is supposed to be turned on. The condition is more likely, if `CONFIG_PWM_DEBUG` is set and the PWM has been enabled before Linux is booted.
After writing some value to the register linked to the duty cycle (`MX3_PWMSAR`), the related debug function (`core.c:pwm_apply_state_debug`) reads back (`.get_state`) a wrong value (`0`) as the configured duty cycle. This value is stored as part of a temporary state variable that it subsequently reapplies to the PWM for testing purposes. Which, effectively, turns off the PWM.
This patch postpones the return of the `.apply` function until either the written value can be read back as expected or the operation times out.
Signed-off-by: Leif Middelschulte <Leif.Middelschulte@klsmartin.com> --- drivers/pwm/pwm-imx27.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index 29a3089c534c..9b473fe10cb9 100644 --- a/drivers/pwm/pwm-imx27.c +++ b/drivers/pwm/pwm-imx27.c @@ -15,6 +15,7 @@ #include <linux/delay.h> #include <linux/err.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> @@ -223,7 +224,7 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, unsigned long long c; unsigned long long clkrate; int ret; - u32 cr; + u32 cr, val; pwm_get_state(pwm, &cstate); @@ -290,6 +291,18 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (!state->enabled) pwm_imx27_clk_disable_unprepare(imx); + /* + * According to imx pwm RM the value can be: + * - written at any time + * - only be read, if the pwm is enabled + * Yet it returns a wrong value (i.e. within `pwm_imx27_get_state`) if it is subsequently read (while enabled). + * Apparently it takes the value some cycles to propagate. + * Wait a bit to make sure the right value can be read by other functions, before returning. + */ + ret = readl_relaxed_poll_timeout(imx->mmio_base + MX3_PWMSAR, val, val == duty_cycles, 20000, 300000); + if (ret) + return ret; + return 0; } -- 2.39.1
| ![\](/images/icornerr.gif) |