On 10/5/24 5:57 PM, Uwe Kleine-König wrote:
On Sat, Oct 05, 2024 at 02:41:29AM +0200, Marek Vasut wrote:
On 10/4/24 10:58 PM, Uwe Kleine-König wrote:
[...]
Why not simply duplicate the ERRATA description for iMX8M Nano MX8MN_0N14Y
errata sheet ?
"
[...]
"
That is very clear to me.
Fine for me. Frank, do you want to try creating the right mix of the NXP
text, your and my description?
/*
* At each clock tick the hardware compares the SAR value with
* the current counter. If they are equal the output is changed
* to the inactive level.
I would skip this ^ part unless you can surely say the IP works exactly that
way because you checked the RTL.
That it works that way is clear from the errata text IMHO.
The errata description does not say anything about comparing SAR value
on each clock tick. Better stick to exactly what the errata does say.
[...]
+ 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));
+
+ if (duty_cycles < imx->duty_cycle) {
+ 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 */
I don't understand the motivation to wait here. Wouldn't it be better to
write the old value 3 - val times and not sleep?
No, because you would overflow the FIFO, see:
137fd45ffec1 ("pwm: imx: Avoid sample FIFO overflow for i.MX PWM version2")
val holds the number of uses FIFO entries, so writing (3 - val) new
items should be fine?!
Not necessarily, consider the case where:
- The PWM is very fast
- There are currently 3 entries in the FIFO according to driver state
- The driver determines 3-val is 1 and performs 1 single write to FIFO
=> If the PWM consumed the FIFO (FIFO is empty) before the 1 single
write arrives, then the aforementioned errata still occurs
I believe the better option is to wait until the FIFO is surely depleted
and then write three entries in short sequence -- OLD-OLD-NEW -- this
way the FIFO would get updated with old value first and then switched to
new value, hopefully mitigating the issue as best as possible even for
fast PWM settings.
btw. the two writes here should be writing the old value twice, now
there are three new value writes in this patch version.
Or busy loop until
MX3_PWMSR_FIFOAV becomes 0?
Do we really want a busy wait here if we can avoid it ?
udelay(6) is a busy loop, so we're already there.
We can do udelay(3 * state->period / 1000); so faster PWMs would wait
shorter.
state->period is the new value (and you want the old, right?), but
otherwise I agree
Right
