| Message ID | 20200909130739.26717-2-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PWM i.MX27 fix disabled state for inverted signals | expand |
Hello, On Wed, Sep 09, 2020 at 03:07:37PM +0200, Marco Felsch wrote: > Introduce a simple clock state so we can enable/disable the clock > without the need to check if we are running or not. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > drivers/pwm/pwm-imx27.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) This doesn't suggest to be more simple. > @@ -223,6 +234,10 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, > int ret; > u32 cr; > > + ret = pwm_imx27_clk_prepare_enable(imx); > + if (ret) > + return ret; > + > pwm_get_state(pwm, &cstate); > > clkrate = clk_get_rate(imx->clk_per); > @@ -254,10 +269,6 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, > if (cstate.enabled) { > pwm_imx27_wait_fifo_slot(chip, pwm); > } else { > - ret = pwm_imx27_clk_prepare_enable(imx); > - if (ret) > - return ret; > - > pwm_imx27_sw_reset(chip); > } There are two clocks, I assume one if for register access and the other to actually drive the PWM. With that what I would consider simple is to enable the register clock at the start of each function and disable it at the end. And for the hardware clock enable it when the hardware should be enabled and disable it when it should be disabled. Probably this doesn't reduce the line count, too, but it makes the function more efficient (if this is measurable at all). If you want to keep the pwm_imx27_clk_prepare_enable() function that handles both clocks, just calling pwm_imx27_clk_prepare_enable unconditionally at the function entry and pwm_imx27_clk_disable_unprepare at the end should be easier and not require the .on member in the driver struct. Best regards Uwe
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c index c50d453552bd..3cf9f1774244 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 clk_on; }; #define to_pwm_imx27_chip(chip) container_of(chip, struct pwm_imx27_chip, chip) @@ -99,6 +100,9 @@ static int pwm_imx27_clk_prepare_enable(struct pwm_imx27_chip *imx) { int ret; + if (imx->clk_on) + return 0; + ret = clk_prepare_enable(imx->clk_ipg); if (ret) return ret; @@ -109,13 +113,20 @@ static int pwm_imx27_clk_prepare_enable(struct pwm_imx27_chip *imx) return ret; } + imx->clk_on = true; + return 0; } static void pwm_imx27_clk_disable_unprepare(struct pwm_imx27_chip *imx) { + if (!imx->clk_on) + return; + clk_disable_unprepare(imx->clk_per); clk_disable_unprepare(imx->clk_ipg); + + imx->clk_on = false; } static void pwm_imx27_get_state(struct pwm_chip *chip, @@ -223,6 +234,10 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, int ret; u32 cr; + ret = pwm_imx27_clk_prepare_enable(imx); + if (ret) + return ret; + pwm_get_state(pwm, &cstate); clkrate = clk_get_rate(imx->clk_per); @@ -254,10 +269,6 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (cstate.enabled) { pwm_imx27_wait_fifo_slot(chip, pwm); } else { - ret = pwm_imx27_clk_prepare_enable(imx); - if (ret) - return ret; - pwm_imx27_sw_reset(chip); }
Introduce a simple clock state so we can enable/disable the clock without the need to check if we are running or not. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- drivers/pwm/pwm-imx27.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)