[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4bbee009-3985-4679-a85e-76f4259ff8d6@denx.de>
Date: Sun, 22 Sep 2024 22:28:02 +0200
From: Marek Vasut <marex@...x.de>
To: Frank Li <Frank.Li@....com>
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org, festevam@...il.com,
francesco@...cini.it, imx@...ts.linux.dev, jun.li@....com,
kernel@...gutronix.de, krzk+dt@...nel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-pwm@...r.kernel.org, p.zabel@...gutronix.de, pratikmanvar09@...il.com,
robh@...nel.org, s.hauer@...gutronix.de, shawnguo@...nel.org,
ukleinek@...nel.org, xiaoning.wang@....com
Subject: Re: [PATCH v6 1/1] pwm: imx27: workaround of the pwm output bug when
decrease the duty cycle
Hi,
On 9/17/24 9:25 PM, Frank Li wrote:
[...]
> @@ -223,6 +224,8 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip);
> unsigned long long c;
> unsigned long long clkrate;
> + unsigned long flags;
> + int val;
> int ret;
> u32 cr;
>
> @@ -263,7 +266,69 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
[...]
> + c = clkrate * 1500;
> + do_div(c, NSEC_PER_SEC);
> +
> + local_irq_save(flags);
> + val = FIELD_GET(MX3_PWMSR_FIFOAV, readl_relaxed(imx->mmio_base + MX3_PWMSR));
I think the multi-write I mentioned in v5 for > 500 kHz case could
further improve the patch, let's see what others think:
if (state->period < 2000) { /* 2000ns = 500 kHz */
/* Best effort attempt to fix up >500 kHz case */
udelay(6); /* 2us per FIFO entry, 3 FIFO entries written => 6 us */
writel_relaxed(duty_cycles, imx->mmio_base + MX3_PWMSAR);
writel_relaxed(duty_cycles, imx->mmio_base + MX3_PWMSAR);
/* Last write is outside, after this conditional */
} else if (duty_cycles ...
> + if (duty_cycles < imx->duty_cycle && val < MX3_PWMSR_FIFOAV_2WORDS) {
> + val = readl_relaxed(imx->mmio_base + MX3_PWMCNR);
> + /*
> + * If counter is close to period, controller may roll over when
> + * next IO write.
> + */
c is only used in this if (duty_cycles ...) { } conditional, the
do_div() above can be moved here:
c = clkrate * 1500;
do_div(c, NSEC_PER_SEC);
> + if ((val + c >= duty_cycles && val < imx->duty_cycle) ||
> + val + c >= period_cycles)
> + writel_relaxed(imx->duty_cycle, imx->mmio_base + MX3_PWMSAR);
> + }
> + writel_relaxed(duty_cycles, imx->mmio_base + MX3_PWMSAR);
> + local_irq_restore(flags);
Powered by blists - more mailing lists