[PATCH v2 1/2] drivers: pwm: pwm-atmel: switch to atomic PWM
From: Boris Brezillon <hidden>
Date: 2017-03-16 11:14:21
Also in:
linux-devicetree, linux-pwm, lkml
Hi Claudiu, On Thu, 2 Mar 2017 17:28:44 +0200 Claudiu Beznea [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The currently Atmel PWM controllers supported by this driver could change period or duty factor without channel disable, for regular channels (sama5d3 support this by using period or duty factor update registers, sam9rl support this by writing channel update register and select the corresponding update: period or duty factor). The chip doesn't support run time changings of signal polarity. To take advantage of atomic PWM framework and let controller works without glitches, in this patch only the duty factor could be changed without disabling PWM channel. For period and signal polarity the atomic PWM is simulated by disabling + enabling the right PWM channel. Signed-off-by: Claudiu Beznea <redacted> --- drivers/pwm/pwm-atmel.c | 271 ++++++++++++++++++++++++------------------------ 1 file changed, 138 insertions(+), 133 deletions(-)diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index 67a7023..3db82c3 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c@@ -58,17 +58,30 @@ #define PWM_MAX_PRD 0xFFFF #define PRD_MAX_PRES 10 +struct atmel_pwm_reg_data { + u8 period; + u8 period_upd; + u8 duty; + u8 duty_upd; +}; + +struct atmel_pwm_data { + void (*update_cdty)(struct pwm_chip *chip, struct pwm_device *pwm, + unsigned long cdty); + void (*set_cprd_cdty)(struct pwm_chip *chip, struct pwm_device *pwm, + unsigned long cprd, unsigned long cdty); + const struct atmel_pwm_reg_data regs;
If you go for this per-IP reg layout definition approach, you don't need the ->update_cdty()/->set_cprd_cdty() hooks anymore.
+}; +
[...]
+static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
+ struct pwm_state cstate;
+ unsigned long cprd, cdty;
+ u32 pres, val;
+ bool pwm_reset = false;
+ int ret;
+
+ pwm_get_state(pwm, &cstate);
+
+ if (cstate.enabled &&
+ (cstate.polarity != state->polarity ||
+ cstate.period != state->period))
+ pwm_reset = true;You can move that in the 'if (state->enabled)' block, but I'm not even sure this is really needed (see below).
+
+ if (state->enabled) {
+ if (cstate.enabled && !pwm_reset) {
if (cstate.enabled &&
(cstate.polarity != state->polarity ||
cstate.period != state->period)) {
+ cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
+ atmel_pwm->data->regs.period);
+ atmel_pwm_calculate_cdty(state, cprd, &cdty);
+ atmel_pwm->data->update_cdty(chip, pwm, cdty);
+ return 0;
+ }
+
+ ret = atmel_pwm_calculate_cprd_and_pres(chip, state, &cprd,
+ &pres);
+ if (ret) {
+ dev_err(chip->dev,
+ "failed to calculate cprd and prescaler\n");
+ return ret;
+ }
+
+ atmel_pwm_calculate_cdty(state, cprd, &cdty);
+
+ if (pwm_reset) {
if (cstate.enabled) {
+ atmel_pwm_disable(chip, pwm, false);
+ } else {
+ ret = clk_enable(atmel_pwm->clk);
+ if (ret) {
+ dev_err(chip->dev, "failed to enable clock\n");
+ return ret;
+ }
+ }
+
+ /* It is necessary to preserve CPOL, inside CMR */
+ val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
+ val = (val & ~PWM_CMR_CPRE_MSK) | (pres & PWM_CMR_CPRE_MSK);
+ if (state->polarity == PWM_POLARITY_NORMAL)
+ val &= ~PWM_CMR_CPOL;
+ else
+ val |= PWM_CMR_CPOL;
+ atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
+ atmel_pwm->data->set_cprd_cdty(chip, pwm, cprd, cdty);
+ mutex_lock(&atmel_pwm->isr_lock);
+ atmel_pwm->updated_pwms |= atmel_pwm_readl(atmel_pwm, PWM_ISR);
+ atmel_pwm->updated_pwms &= ~(1 << pwm->hwpwm);
+ mutex_unlock(&atmel_pwm->isr_lock);
+ atmel_pwm_writel(atmel_pwm, PWM_ENA, 1 << pwm->hwpwm);
+ } else if (cstate.enabled) {
+ atmel_pwm_disable(chip, pwm, true);
+ }
+
+ return 0;
}