From: Marco Felsch <m.felsch@pengutronix.de> To: thierry.reding@gmail.com, u.kleine-koenig@pengutronix.de, lee.jones@linaro.org, shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, Anson.Huang@nxp.com, michal.vokac@ysoft.com, l.majewski@majess.pl Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access Date: Fri, 25 Sep 2020 17:53:26 +0200 [thread overview] Message-ID: <20200925155330.32301-2-m.felsch@pengutronix.de> (raw) In-Reply-To: <20200925155330.32301-1-m.felsch@pengutronix.de> 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@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
WARNING: multiple messages have this Message-ID (diff)
From: Marco Felsch <m.felsch@pengutronix.de> To: thierry.reding@gmail.com, u.kleine-koenig@pengutronix.de, lee.jones@linaro.org, shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, Anson.Huang@nxp.com, michal.vokac@ysoft.com, l.majewski@majess.pl Cc: linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de Subject: [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access Date: Fri, 25 Sep 2020 17:53:26 +0200 [thread overview] Message-ID: <20200925155330.32301-2-m.felsch@pengutronix.de> (raw) In-Reply-To: <20200925155330.32301-1-m.felsch@pengutronix.de> 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@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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-25 15:53 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-25 15:53 [PATCH v2 0/5] PWM i.MX27 fix disabled state for inverted signals Marco Felsch 2020-09-25 15:53 ` Marco Felsch 2020-09-25 15:53 ` Marco Felsch [this message] 2020-09-25 15:53 ` [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access Marco Felsch 2020-09-26 13:28 ` Uwe Kleine-König 2020-09-26 13:28 ` Uwe Kleine-König 2020-09-26 13:48 ` Uwe Kleine-König 2020-09-26 13:48 ` Uwe Kleine-König 2020-09-28 5:52 ` Marco Felsch 2020-09-28 5:52 ` Marco Felsch 2020-09-25 15:53 ` [PATCH v2 2/5] pwm: imx27: move constant PWMCR register values into probe Marco Felsch 2020-09-25 15:53 ` Marco Felsch 2020-09-26 13:46 ` Uwe Kleine-König 2020-09-26 13:46 ` Uwe Kleine-König 2020-09-28 5:50 ` Marco Felsch 2020-09-28 5:50 ` Marco Felsch 2020-09-25 15:53 ` [PATCH v2 3/5] pwm: imx27: reset the PWM if it is not running Marco Felsch 2020-09-25 15:53 ` Marco Felsch 2020-09-28 7:30 ` Uwe Kleine-König 2020-09-28 7:30 ` Uwe Kleine-König 2020-09-28 9:29 ` Marco Felsch 2020-09-28 9:29 ` Marco Felsch 2020-09-25 15:53 ` [PATCH v2 4/5] pwm: imx27: fix disable state for inverted PWMs Marco Felsch 2020-09-25 15:53 ` Marco Felsch 2020-09-28 7:47 ` Uwe Kleine-König 2020-09-28 7:47 ` Uwe Kleine-König 2020-09-28 9:52 ` Marco Felsch 2020-09-28 9:52 ` Marco Felsch 2020-09-28 19:06 ` Uwe Kleine-König 2020-09-28 19:06 ` Uwe Kleine-König 2020-09-29 5:23 ` Marco Felsch 2020-09-29 5:23 ` Marco Felsch 2020-09-25 15:53 ` [PATCH v2 5/5] pwm: imx27: wait till the duty cycle is applied Marco Felsch 2020-09-25 15:53 ` Marco Felsch 2020-09-28 8:04 ` Uwe Kleine-König 2020-09-28 8:04 ` Uwe Kleine-König 2020-09-28 9:59 ` Marco Felsch 2020-09-28 9:59 ` Marco Felsch
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200925155330.32301-2-m.felsch@pengutronix.de \ --to=m.felsch@pengutronix.de \ --cc=Anson.Huang@nxp.com \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=l.majewski@majess.pl \ --cc=lee.jones@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-pwm@vger.kernel.org \ --cc=michal.vokac@ysoft.com \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=thierry.reding@gmail.com \ --cc=u.kleine-koenig@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.