[PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access

Marco Felsch m.felsch at pengutronix.de
Fri Sep 25 11:53:26 EDT 2020


The current implementation enables the clock if the current PWM state
is '!enabled' to ensure the register access and left the clock on if the
new state is 'enabled'. Further apply calls don't enable the clock since
they relying on the fact the the clock is already running. Change this
behaviour since it is not very intuitive.

This commit changes this behaviour. Now the clocks are unconditional
enabled/disabled before/after the register access. If the PWM should be
turned on (state.enabled) we enable the clock again and vice versa if
the PWM should be turned off (!state.enabled).

Therefore I added the enable member to the driver state struct since
the usage of cstate and pwm_get_state() is a layer violation. I removed
this violation while on it.

Signed-off-by: Marco Felsch <m.felsch at pengutronix.de>
---
v2:
- use enable var which can be shared later on
- remove cstate and pwm_get_state() layer violation
- explicite enable/disable the clock twice if the pwm should be
  enabled/disabled rather than tracking the clock usage within the
  pwm_imx27_clk_prepare_enable() state.
- rename commit message

 drivers/pwm/pwm-imx27.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index c50d453552bd..7edac4ac6395 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -91,6 +91,7 @@ struct pwm_imx27_chip {
 	 * value to return in that case.
 	 */
 	unsigned int duty_cycle;
+	bool enabled;
 };
 
 #define to_pwm_imx27_chip(chip)	container_of(chip, struct pwm_imx27_chip, chip)
@@ -217,13 +218,14 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	unsigned long period_cycles, duty_cycles, prescale;
 	struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip);
-	struct pwm_state cstate;
 	unsigned long long c;
 	unsigned long long clkrate;
 	int ret;
 	u32 cr;
 
-	pwm_get_state(pwm, &cstate);
+	ret = pwm_imx27_clk_prepare_enable(imx);
+	if (ret)
+		return ret;
 
 	clkrate = clk_get_rate(imx->clk_per);
 	c = clkrate * state->period;
@@ -251,15 +253,10 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * Wait for a free FIFO slot if the PWM is already enabled, and flush
 	 * the FIFO if the PWM was disabled and is about to be enabled.
 	 */
-	if (cstate.enabled) {
+	if (imx->enabled)
 		pwm_imx27_wait_fifo_slot(chip, pwm);
-	} else {
-		ret = pwm_imx27_clk_prepare_enable(imx);
-		if (ret)
-			return ret;
-
+	else
 		pwm_imx27_sw_reset(chip);
-	}
 
 	writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
 	writel(period_cycles, imx->mmio_base + MX3_PWMPR);
@@ -284,10 +281,21 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	writel(cr, imx->mmio_base + MX3_PWMCR);
 
-	if (!state->enabled)
-		pwm_imx27_clk_disable_unprepare(imx);
+	if (imx->enabled != state->enabled) {
+		if (state->enabled) {
+			ret = pwm_imx27_clk_prepare_enable(imx);
+			if (ret)
+				goto out;
+		} else {
+			pwm_imx27_clk_disable_unprepare(imx);
+		}
+		imx->enabled = state->enabled;
+	}
 
-	return 0;
+out:
+	pwm_imx27_clk_disable_unprepare(imx);
+
+	return ret;
 }
 
 static const struct pwm_ops pwm_imx27_ops = {
-- 
2.20.1




More information about the linux-arm-kernel mailing list