diff mbox series

[v4,3/4] pwm: bcm2835: allow pwm driver to be used in atomic context

Message ID 0b35ca65d6f4d53d3beb1411a64970ea5f969060.1698398004.git.sean@mess.org (mailing list archive)
State New, archived
Headers show
Series Improve pwm-ir-tx precision | expand

Commit Message

Sean Young Oct. 27, 2023, 9:20 a.m. UTC 
clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
rate changes using clk_rate_exclusive_get().

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/pwm/pwm-bcm2835.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

Comments

Sean Young Oct. 29, 2023, 8:39 a.m. UTC | #1 
Hello Uwe,

On Fri, Oct 27, 2023 at 03:38:18PM +0200, Uwe Kleine-König wrote:
> On Fri, Oct 27, 2023 at 10:20:46AM +0100, Sean Young wrote:
> > +	pc->rate = clk_get_rate(pc->clk);
> > +	if (!pc->rate) {
> > +		dev_err(pc->dev, "failed to get clock rate\n");
> > +		ret = -EINVAL;
> 
> Other error paths in this driver use dev_err_probe(). The most compact
> way here would be:
> 
> 	ret = dev_err_probe(pc->dev, -EINVAL, "....");
> 
> but maybe
> 
> 	ret = -EINVAL;
> 	dev_err_probe(pc->dev, ret, "...");
> 
> is a bit easier to parse for a human?!

Using the same dev_err_probe() function for all error paths is nice, so
I will change it for the next version. This change will print the EINVAL
error as well which does not really add anything, but no harm done there.

Thanks,

Sean
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index bdfc2a5ec0d6..bd636ac1c507 100644
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -28,6 +28,7 @@  struct bcm2835_pwm {
 	struct device *dev;
 	void __iomem *base;
 	struct clk *clk;
+	unsigned long rate;
 };
 
 static inline struct bcm2835_pwm *to_bcm2835_pwm(struct pwm_chip *chip)
@@ -63,17 +64,11 @@  static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 
 	struct bcm2835_pwm *pc = to_bcm2835_pwm(chip);
-	unsigned long rate = clk_get_rate(pc->clk);
 	unsigned long long period_cycles;
 	u64 max_period;
 
 	u32 val;
 
-	if (!rate) {
-		dev_err(pc->dev, "failed to get clock rate\n");
-		return -EINVAL;
-	}
-
 	/*
 	 * period_cycles must be a 32 bit value, so period * rate / NSEC_PER_SEC
 	 * must be <= U32_MAX. As U32_MAX * NSEC_PER_SEC < U64_MAX the
@@ -88,13 +83,13 @@  static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * <=> period < ((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate
 	 * <=> period <= ceil((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate) - 1
 	 */
-	max_period = DIV_ROUND_UP_ULL((u64)U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC / 2, rate) - 1;
+	max_period = DIV_ROUND_UP_ULL((u64)U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC / 2, pc->rate) - 1;
 
 	if (state->period > max_period)
 		return -EINVAL;
 
 	/* set period */
-	period_cycles = DIV_ROUND_CLOSEST_ULL(state->period * rate, NSEC_PER_SEC);
+	period_cycles = DIV_ROUND_CLOSEST_ULL(state->period * pc->rate, NSEC_PER_SEC);
 
 	/* don't accept a period that is too small */
 	if (period_cycles < PERIOD_MIN)
@@ -103,7 +98,7 @@  static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	writel(period_cycles, pc->base + PERIOD(pwm->hwpwm));
 
 	/* set duty cycle */
-	val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * rate, NSEC_PER_SEC);
+	val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * pc->rate, NSEC_PER_SEC);
 	writel(val, pc->base + DUTY(pwm->hwpwm));
 
 	/* set polarity */
@@ -156,18 +151,32 @@  static int bcm2835_pwm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = clk_rate_exclusive_get(pc->clk);
+	if (ret)
+		goto add_fail;
+
+	pc->rate = clk_get_rate(pc->clk);
+	if (!pc->rate) {
+		dev_err(pc->dev, "failed to get clock rate\n");
+		ret = -EINVAL;
+		goto rate_fail;
+	}
+
 	pc->chip.dev = &pdev->dev;
 	pc->chip.ops = &bcm2835_pwm_ops;
+	pc->chip.atomic = true;
 	pc->chip.npwm = 2;
 
 	platform_set_drvdata(pdev, pc);
 
 	ret = pwmchip_add(&pc->chip);
 	if (ret < 0)
-		goto add_fail;
+		goto rate_fail;
 
 	return 0;
 
+rate_fail:
+	clk_rate_exclusive_put(pc->clk);
 add_fail:
 	clk_disable_unprepare(pc->clk);
 	return ret;
@@ -179,6 +188,7 @@  static void bcm2835_pwm_remove(struct platform_device *pdev)
 
 	pwmchip_remove(&pc->chip);
 
+	clk_rate_exclusive_put(pc->clk);
 	clk_disable_unprepare(pc->clk);
 }