From 52eaba4cedbda728f78d7083a05725e537b3df91 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Wed, 21 Apr 2021 11:26:08 +0200 Subject: [PATCH] pwm: atmel: Rework tracking updates pending in hardware MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This improves the driver's behavior in several ways: - The lock is held for shorter periods and so a channel that is currently waited for doesn't block disabling another channel. - It's easier to understand because the procedure is split into more semantic units and documentation is improved - A channel is only set to pending when such an event is actually scheduled in hardware (by writing the CUPD register). - Also wait in .get_state() to report the last configured state instead of (maybe) the previous one. This fixes the read back duty cycle and so prevents a warning being emitted when PWM_DEBUG is on. Tested on an AriettaG25. Signed-off-by: Uwe Kleine-König Signed-off-by: Thierry Reding --- drivers/pwm/pwm-atmel.c | 102 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 23 deletions(-) diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c index a8162bae3e8a..e748604403cc 100644 --- a/drivers/pwm/pwm-atmel.c +++ b/drivers/pwm/pwm-atmel.c @@ -84,9 +84,19 @@ struct atmel_pwm_chip { void __iomem *base; const struct atmel_pwm_data *data; - unsigned int updated_pwms; - /* ISR is cleared when read, ensure only one thread does that */ - struct mutex isr_lock; + /* + * The hardware supports a mechanism to update a channel's duty cycle at + * the end of the currently running period. When such an update is + * pending we delay disabling the PWM until the new configuration is + * active because otherwise pmw_config(duty_cycle=0); pwm_disable(); + * might not result in an inactive output. + * This bitmask tracks for which channels an update is pending in + * hardware. + */ + u32 update_pending; + + /* Protects .update_pending */ + spinlock_t lock; }; static inline struct atmel_pwm_chip *to_atmel_pwm_chip(struct pwm_chip *chip) @@ -123,6 +133,64 @@ static inline void atmel_pwm_ch_writel(struct atmel_pwm_chip *chip, atmel_pwm_writel(chip, base + offset, val); } +static void atmel_pwm_update_pending(struct atmel_pwm_chip *chip) +{ + /* + * Each channel that has its bit in ISR set started a new period since + * ISR was cleared and so there is no more update pending. Note that + * reading ISR clears it, so this needs to handle all channels to not + * loose information. + */ + u32 isr = atmel_pwm_readl(chip, PWM_ISR); + + chip->update_pending &= ~isr; +} + +static void atmel_pwm_set_pending(struct atmel_pwm_chip *chip, unsigned int ch) +{ + spin_lock(&chip->lock); + + /* + * Clear pending flags in hardware because otherwise there might still + * be a stale flag in ISR. + */ + atmel_pwm_update_pending(chip); + + chip->update_pending |= (1 << ch); + + spin_unlock(&chip->lock); +} + +static int atmel_pwm_test_pending(struct atmel_pwm_chip *chip, unsigned int ch) +{ + int ret = 0; + + spin_lock(&chip->lock); + + if (chip->update_pending & (1 << ch)) { + atmel_pwm_update_pending(chip); + + if (chip->update_pending & (1 << ch)) + ret = 1; + } + + spin_unlock(&chip->lock); + + return ret; +} + +static int atmel_pwm_wait_nonpending(struct atmel_pwm_chip *chip, unsigned int ch) +{ + unsigned long timeout = jiffies + 2 * HZ; + int ret; + + while ((ret = atmel_pwm_test_pending(chip, ch)) && + time_before(jiffies, timeout)) + usleep_range(10, 100); + + return ret ? -ETIMEDOUT : 0; +} + static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip, unsigned long clkrate, const struct pwm_state *state, @@ -185,6 +253,7 @@ static void atmel_pwm_update_cdty(struct pwm_chip *chip, struct pwm_device *pwm, atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.duty_upd, cdty); + atmel_pwm_set_pending(atmel_pwm, pwm->hwpwm); } static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip, @@ -205,20 +274,8 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm, struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip); unsigned long timeout = jiffies + 2 * HZ; - /* - * Wait for at least a complete period to have passed before disabling a - * channel to be sure that CDTY has been updated - */ - mutex_lock(&atmel_pwm->isr_lock); - atmel_pwm->updated_pwms |= atmel_pwm_readl(atmel_pwm, PWM_ISR); - - while (!(atmel_pwm->updated_pwms & (1 << pwm->hwpwm)) && - time_before(jiffies, timeout)) { - usleep_range(10, 100); - atmel_pwm->updated_pwms |= atmel_pwm_readl(atmel_pwm, PWM_ISR); - } + atmel_pwm_wait_nonpending(atmel_pwm, pwm->hwpwm); - mutex_unlock(&atmel_pwm->isr_lock); atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm); /* @@ -292,10 +349,6 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, val |= PWM_CMR_CPOL; atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val); atmel_pwm_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); @@ -326,6 +379,9 @@ static void atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, tmp <<= pres; state->period = DIV64_U64_ROUND_UP(tmp, rate); + /* Wait for an updated duty_cycle queued in hardware */ + atmel_pwm_wait_nonpending(atmel_pwm, pwm->hwpwm); + cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, atmel_pwm->data->regs.duty); tmp = (u64)(cprd - cdty) * NSEC_PER_SEC; @@ -416,9 +472,10 @@ static int atmel_pwm_probe(struct platform_device *pdev) if (!atmel_pwm) return -ENOMEM; - mutex_init(&atmel_pwm->isr_lock); atmel_pwm->data = of_device_get_match_data(&pdev->dev); - atmel_pwm->updated_pwms = 0; + + atmel_pwm->update_pending = 0; + spin_lock_init(&atmel_pwm->lock); atmel_pwm->base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(atmel_pwm->base)) @@ -460,7 +517,6 @@ static int atmel_pwm_remove(struct platform_device *pdev) pwmchip_remove(&atmel_pwm->chip); clk_unprepare(atmel_pwm->clk); - mutex_destroy(&atmel_pwm->isr_lock); return 0; } -- 2.11.0