Adding Marek. On Mon, Jul 15, 2024 at 5:30 PM Frank Li <Frank.Li@xxxxxxx> wrote: > > From: Clark Wang <xiaoning.wang@xxxxxxx> > > Implement workaround for ERR051198 > (https://www.nxp.com/docs/en/errata/IMX8MN_0N14Y.pdf) > > PWM output may not function correctly if the FIFO is empty when a new SAR > value is programmed > > Description: > When the PWM FIFO is empty, a new value programmed to the PWM Sample > register (PWM_PWMSAR) will be directly applied even if the current timer > period has not expired. If the new SAMPLE value programmed in the > PWM_PWMSAR register is less than the previous value, and the PWM counter > register (PWM_PWMCNR) that contains the current COUNT value is greater > than the new programmed SAMPLE value, the current period will not flip > the level. This may result in an output pulse with a duty cycle of 100%. > > Workaround: > Program the current SAMPLE value in the PWM_PWMSAR register before > updating the new duty cycle to the SAMPLE value in the PWM_PWMSAR > register. This will ensure that the new SAMPLE value is modified during > a non-empty FIFO, and can be successfully updated after the period > expires. > > Write the old SAR value before updating the new duty cycle to SAR. This > avoids writing the new value into an empty FIFO. > > This only resolves the issue when the PWM period is longer than 2us > (or <500KHz) because write register is not quick enough when PWM period is > very short. > > Fixes: 166091b1894d ("[ARM] MXC: add pwm driver for i.MX SoCs") > Reviewed-by: Jun Li <jun.li@xxxxxxx> > Signed-off-by: Clark Wang <xiaoning.wang@xxxxxxx> > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > --- > Change from v1 to v2 > - address comments in https://lore.kernel.org/linux-pwm/20211221095053.uz4qbnhdqziftymw@xxxxxxxxxxxxxx/ > About disable/enable pwm instead of disable/enable irq: > Some pmw periphal may sensitive to period. Disable/enable pwm will > increase period, althouhg it is okay for most case, such as LED backlight > or FAN speed. But some device such servo may require strict period. > > - address comments in https://lore.kernel.org/linux-pwm/d72d1ae5-0378-4bac-8b77-0bb69f55accd@xxxxxxx/ > Using official errata number > fix typo 'filp' > add {} for else > > I supposed fixed all previous issues, let me know if I missed one. > --- > drivers/pwm/pwm-imx27.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c > index 253afe94c4776..e12eaaebe3499 100644 > --- a/drivers/pwm/pwm-imx27.c > +++ b/drivers/pwm/pwm-imx27.c > @@ -27,6 +27,7 @@ > #define MX3_PWMSR 0x04 /* PWM Status Register */ > #define MX3_PWMSAR 0x0C /* PWM Sample Register */ > #define MX3_PWMPR 0x10 /* PWM Period Register */ > +#define MX3_PWMCNR 0x14 /* PWM Counter Register */ > > #define MX3_PWMCR_FWM GENMASK(27, 26) > #define MX3_PWMCR_STOPEN BIT(25) > @@ -232,8 +233,11 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, > { > unsigned long period_cycles, duty_cycles, prescale; > struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip); > + void __iomem *reg_sar = imx->mmio_base + MX3_PWMSAR; > unsigned long long c; > unsigned long long clkrate; > + unsigned long flags; > + int val; > int ret; > u32 cr; > > @@ -274,7 +278,53 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm, > pwm_imx27_sw_reset(chip); > } > > - writel(duty_cycles, imx->mmio_base + MX3_PWMSAR); > + /* > + * This is a limited workaround. When the SAR FIFO is empty, the new > + * write value will be directly applied to SAR even the current period > + * is not over. > + * > + * If the new SAR value is less than the old one, and the counter is > + * greater than the new SAR value, the current period will not filp > + * the level. This will result in a pulse with a duty cycle of 100%. > + * So, writing the current value of the SAR to SAR here before updating > + * the new SAR value can avoid this issue. > + * > + * Add a spin lock and turn off the interrupt to ensure that the > + * real-time performance can be guaranteed as much as possible when > + * operating the following operations. > + * > + * 1. Add a threshold of 1.5us. If the time T between the read current > + * count value CNR and the end of the cycle is less than 1.5us, wait > + * for T to be longer than 1.5us before updating the SAR register. > + * This is to avoid the situation that when the first SAR is written, > + * the current cycle just ends and the SAR FIFO that just be written > + * is emptied again. > + * > + * 2. Use __raw_writel() to minimize the interval between two writes to > + * the SAR register to increase the fastest pwm frequency supported. > + * > + * When the PWM period is longer than 2us(or <500KHz), this workaround > + * can solve this problem. > + */ > + val = FIELD_GET(MX3_PWMSR_FIFOAV, readl_relaxed(imx->mmio_base + MX3_PWMSR)); > + if (duty_cycles < imx->duty_cycle && val < MX3_PWMSR_FIFOAV_2WORDS) { > + c = clkrate * 1500; > + do_div(c, NSEC_PER_SEC); > + > + local_irq_save(flags); > + if (state->period >= 2000) > + readl_poll_timeout_atomic(imx->mmio_base + MX3_PWMCNR, val, > + period_cycles - val >= c, 0, 10); > + > + val = FIELD_GET(MX3_PWMSR_FIFOAV, readl_relaxed(imx->mmio_base + MX3_PWMSR)); > + if (!val) > + writel_relaxed(imx->duty_cycle, reg_sar); > + writel_relaxed(duty_cycles, reg_sar); > + local_irq_restore(flags); > + } else { > + writel_relaxed(duty_cycles, reg_sar); > + } > + > writel(period_cycles, imx->mmio_base + MX3_PWMPR); > > /* > > -- > 2.34.1 >
