lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOMZO5DNmHfHWbLoPj9P=_+JiLLQ4tiDd_90+UX+_psN2o+Knw@mail.gmail.com>
Date: Thu, 5 Sep 2024 14:12:33 -0300
From: Fabio Estevam <festevam@...il.com>
To: Frank Li <Frank.Li@....com>, Marek Vasut <marex@...x.de>
Cc: Uwe Kleine-König <ukleinek@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Philipp Zabel <p.zabel@...gutronix.de>, 
	linux-pwm@...r.kernel.org, devicetree@...r.kernel.org, imx@...ts.linux.dev, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	pratikmanvar09@...il.com, francesco@...cini.it, 
	Clark Wang <xiaoning.wang@....com>, Jun Li <jun.li@....com>
Subject: Re: [PATCH v2 3/3] pwm: imx27: workaround of the pwm output bug when
 decrease the duty cycle

Adding Marek.

On Mon, Jul 15, 2024 at 5:30 PM Frank Li <Frank.Li@....com> wrote:
>
> From: Clark Wang <xiaoning.wang@....com>
>
> 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@....com>
> Signed-off-by: Clark Wang <xiaoning.wang@....com>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
> Change from v1 to v2
> - address comments in https://lore.kernel.org/linux-pwm/20211221095053.uz4qbnhdqziftymw@pengutronix.de/
>   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@gmx.net/
>   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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ