* [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context [not found] <cover.1697193646.git.sean@mess.org> @ 2023-10-13 10:46 ` Sean Young 2023-10-13 11:04 ` Stefan Wahren 0 siblings, 1 reply; 7+ messages in thread From: Sean Young @ 2023-10-13 10:46 UTC (permalink / raw) To: linux-media, Ivaylo Dimitrov, Thierry Reding, Uwe Kleine-König, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list Cc: Sean Young, linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel clk_get_rate() may do a mutex lock. Since the clock rate cannot change on an rpi, simply fetch it once. Signed-off-by: Sean Young <sean@mess.org> --- 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; -- 2.42.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-13 10:46 ` [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young @ 2023-10-13 11:04 ` Stefan Wahren 2023-10-13 11:13 ` Alexander Stein 0 siblings, 1 reply; 7+ messages in thread From: Stefan Wahren @ 2023-10-13 11:04 UTC (permalink / raw) To: Sean Young, linux-media, Ivaylo Dimitrov, Thierry Reding, Uwe Kleine-König, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list Cc: linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel 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@mess.org> > --- > 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; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-13 11:04 ` Stefan Wahren @ 2023-10-13 11:13 ` Alexander Stein 2023-10-13 11:44 ` Stefan Wahren 2023-10-13 17:51 ` Uwe Kleine-König 0 siblings, 2 replies; 7+ messages in thread From: Alexander Stein @ 2023-10-13 11:13 UTC (permalink / raw) To: Sean Young, linux-media, Ivaylo Dimitrov, Thierry Reding, Uwe Kleine-König, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list, Stefan Wahren Cc: linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel Hi, Am Freitag, 13. Oktober 2023, 13:04:48 CEST schrieb Stefan Wahren: > 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? Assuming the clock can change, which I would, then a clock notifier seems appropriate. See [1] for an example. Best regards, Alexander [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ commit/?id=90ad2cbe88c22d0215225ab9594eeead0eb24fde > Best regards > > > Signed-off-by: Sean Young <sean@mess.org> > > --- > > > > 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; -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider http://www.tq-group.com/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-13 11:13 ` Alexander Stein @ 2023-10-13 11:44 ` Stefan Wahren 2023-10-13 17:51 ` Uwe Kleine-König 1 sibling, 0 replies; 7+ messages in thread From: Stefan Wahren @ 2023-10-13 11:44 UTC (permalink / raw) To: Alexander Stein, Sean Young, linux-media, Ivaylo Dimitrov, Thierry Reding, Uwe Kleine-König, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list Cc: linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel Hi Alexander, Am 13.10.23 um 13:13 schrieb Alexander Stein: > Hi, > > Am Freitag, 13. Oktober 2023, 13:04:48 CEST schrieb Stefan Wahren: >> 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? > Assuming the clock can change, which I would, then a clock notifier seems > appropriate. See [1] for an example. i remember a similiar approach for the CPU frequency for the RPi. At end the we decided to let the firmware handle it and don't use clock notifier, see [2] and the related links for more background. The fact that the VideoCore has the real control makes it hard. I don't want to say that's impossible. [2] - https://patchwork.kernel.org/project/linux-arm-kernel/cover/20190520104708.11980-1-nsaenzjulienne@suse.de/ > > Best regards, > Alexander > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ > commit/?id=90ad2cbe88c22d0215225ab9594eeead0eb24fde > >> Best regards >> >>> Signed-off-by: Sean Young <sean@mess.org> >>> --- >>> >>> 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; > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-13 11:13 ` Alexander Stein 2023-10-13 11:44 ` Stefan Wahren @ 2023-10-13 17:51 ` Uwe Kleine-König 2023-10-14 6:51 ` Ivaylo Dimitrov 1 sibling, 1 reply; 7+ messages in thread From: Uwe Kleine-König @ 2023-10-13 17:51 UTC (permalink / raw) To: Alexander Stein Cc: Sean Young, linux-media, Ivaylo Dimitrov, Thierry Reding, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list, Stefan Wahren, linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 1164 bytes --] Hello, On Fri, Oct 13, 2023 at 01:13:50PM +0200, Alexander Stein wrote: > Am Freitag, 13. Oktober 2023, 13:04:48 CEST schrieb Stefan Wahren: > > 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? > > Assuming the clock can change, which I would, then a clock notifier seems > appropriate. See [1] for an example. I'm not a fan. If the clock changes, the output also changes. With a clock notifier you can soften the issue and reconfigure to something similar as the original wave form, but a glitch happens for sure. I already toyed with the thought to add clk_rate_exclusive_get() to all PWM drivers, but didn't come around it yet. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-13 17:51 ` Uwe Kleine-König @ 2023-10-14 6:51 ` Ivaylo Dimitrov 2023-10-14 8:47 ` Uwe Kleine-König 0 siblings, 1 reply; 7+ messages in thread From: Ivaylo Dimitrov @ 2023-10-14 6:51 UTC (permalink / raw) To: Uwe Kleine-König, Alexander Stein Cc: Sean Young, linux-media, Thierry Reding, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list, Stefan Wahren, linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel On 13.10.23 г. 20:51 ч., Uwe Kleine-König wrote: > Hello, > > On Fri, Oct 13, 2023 at 01:13:50PM +0200, Alexander Stein wrote: >> Am Freitag, 13. Oktober 2023, 13:04:48 CEST schrieb Stefan Wahren: >>> 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? >> >> Assuming the clock can change, which I would, then a clock notifier seems >> appropriate. See [1] for an example. > > I'm not a fan. If the clock changes, the output also changes. With a > clock notifier you can soften the issue and reconfigure to something > similar as the original wave form, but a glitch happens for sure. > Right, but without notifier, everything rate related after the change will be wrong Ivo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context 2023-10-14 6:51 ` Ivaylo Dimitrov @ 2023-10-14 8:47 ` Uwe Kleine-König 0 siblings, 0 replies; 7+ messages in thread From: Uwe Kleine-König @ 2023-10-14 8:47 UTC (permalink / raw) To: Ivaylo Dimitrov Cc: Alexander Stein, Sean Young, linux-media, Thierry Reding, Florian Fainelli, Ray Jui, Scott Branden, Broadcom internal kernel review list, Stefan Wahren, linux-pwm, linux-rpi-kernel, linux-arm-kernel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 1475 bytes --] Hello Ivaylo, On Sat, Oct 14, 2023 at 09:51:12AM +0300, Ivaylo Dimitrov wrote: > On 13.10.23 г. 20:51 ч., Uwe Kleine-König wrote: > > Hello, > > > > On Fri, Oct 13, 2023 at 01:13:50PM +0200, Alexander Stein wrote: > > > Am Freitag, 13. Oktober 2023, 13:04:48 CEST schrieb Stefan Wahren: > > > > 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? > > > > > > Assuming the clock can change, which I would, then a clock notifier seems > > > appropriate. See [1] for an example. > > > > I'm not a fan. If the clock changes, the output also changes. With a > > clock notifier you can soften the issue and reconfigure to something > > similar as the original wave form, but a glitch happens for sure. > > > > Right, but without notifier, everything rate related after the change will > be wrong So we agree clk_rate_exclusive_get() is the way to go?! It's simple, no need for a notifier and no glitches. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-14 8:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1697193646.git.sean@mess.org>
2023-10-13 10:46 ` [PATCH v2 2/3] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young
2023-10-13 11:04 ` Stefan Wahren
2023-10-13 11:13 ` Alexander Stein
2023-10-13 11:44 ` Stefan Wahren
2023-10-13 17:51 ` Uwe Kleine-König
2023-10-14 6:51 ` Ivaylo Dimitrov
2023-10-14 8:47 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox