pwm: atmel: Rework tracking updates pending in hardware
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 <u.kleine-koenig@pengutronix.de> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
This commit is contained in:
		
							parent
							
								
									2734d6c1b1
								
							
						
					
					
						commit
						52eaba4ced
					
				| @ -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); | ||||
| 	atmel_pwm_wait_nonpending(atmel_pwm, pwm->hwpwm); | ||||
| 
 | ||||
| 	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); | ||||
| 	} | ||||
| 
 | ||||
| 	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; | ||||
| } | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user