Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context

From: Stefan Wahren
Date: Fri Oct 13 2023 - 07:05:10 EST


Hi Sean,

Am 13.10.23 um 12:46 schrieb Sean Young:
clk_get_rate() may do a mutex lock. Since the clock rate cannot change on
an rpi, simply fetch it once.
does it mean you checked all possible SoCs (BCM2835, BCM2836, BCM2837,
BCM2711, BCM2712) for this change?

Is it impossible that the real clock can never be influenced by turbo
mode like SPI?

Best regards

Signed-off-by: Sean Young <sean@xxxxxxxx>
---
drivers/pwm/pwm-bcm2835.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index bdfc2a5ec0d6..59ea154dd657 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 */
@@ -129,6 +124,7 @@ static const struct pwm_ops bcm2835_pwm_ops = {
.request = bcm2835_pwm_request,
.free = bcm2835_pwm_free,
.apply = bcm2835_pwm_apply,
+ .atomic = true,
.owner = THIS_MODULE,
};

@@ -156,6 +152,13 @@ static int bcm2835_pwm_probe(struct platform_device *pdev)
if (ret)
return ret;

+ pc->rate = clk_get_rate(pc->clk);
+ if (!pc->rate) {
+ dev_err(pc->dev, "failed to get clock rate\n");
+ ret = -EINVAL;
+ goto add_fail;
+ }
+
pc->chip.dev = &pdev->dev;
pc->chip.ops = &bcm2835_pwm_ops;
pc->chip.npwm = 2;