| 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
| ||
|
Message-Id: <20240711-pwm-v1-3-4d5766f99b8b@nxp.com>
Date: Thu, 11 Jul 2024 17:08:58 -0400
From: Frank Li <Frank.Li@....com>
To: 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>,
Fabio Estevam <festevam@...il.com>, Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Frank Li <Frank.Li@....com>,
Clark Wang <xiaoning.wang@....com>, Jun Li <jun.li@....com>
Subject: [PATCH 3/3] pwm: imx27: workaround of the pwm output bug when
decrease the duty cycle
From: Clark Wang <xiaoning.wang@....com>
When the SAR FIFO is empty, the write value is directly applied to SAR even
though 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 flip the level. This result in a pulse with a 100% duty
cycle.
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.
Reviewed-by: Jun Li <jun.li@....com>
Signed-off-by: Clark Wang <xiaoning.wang@....com>
Signed-off-by: Frank Li <Frank.Li@....com>
---
drivers/pwm/pwm-imx27.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 032bce7d1fdd3..b817d73effc93 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -22,11 +22,13 @@
#include <linux/platform_device.h>
#include <linux/pwm.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#define MX3_PWMCR 0x00 /* PWM Control Register */
#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)
@@ -92,6 +94,7 @@ struct pwm_imx27_chip {
* value to return in that case.
*/
unsigned int duty_cycle;
+ spinlock_t lock;
};
static inline struct pwm_imx27_chip *to_pwm_imx27_chip(struct pwm_chip *chip)
@@ -220,10 +223,10 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
sr = readl(imx->mmio_base + MX3_PWMSR);
fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr);
- if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
+ if (fifoav >= MX3_PWMSR_FIFOAV_3WORDS) {
period_ms = DIV_ROUND_UP_ULL(pwm->state.period,
NSEC_PER_MSEC);
- msleep(period_ms);
+ msleep(period_ms * 2);
sr = readl(imx->mmio_base + MX3_PWMSR);
if (fifoav == FIELD_GET(MX3_PWMSR_FIFOAV, sr))
@@ -236,8 +239,10 @@ 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;
@@ -279,7 +284,50 @@ 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.
+ */
+ if (duty_cycles < imx->duty_cycle) {
+ c = clkrate * 1500;
+ do_div(c, NSEC_PER_SEC);
+
+ spin_lock_irqsave(&imx->lock, flags);
+ if (state->period >= 2000)
+ readl_poll_timeout_atomic(imx->mmio_base + MX3_PWMCNR, val,
+ period_cycles - val >= c, 0, 10);
+
+ if (!(MX3_PWMSR_FIFOAV & readl_relaxed(imx->mmio_base + MX3_PWMSR)))
+ __raw_writel(imx->duty_cycle, reg_sar);
+ __raw_writel(duty_cycles, reg_sar);
+ spin_unlock_irqrestore(&imx->lock, flags);
+ } else
+ writel(duty_cycles, reg_sar);
+
writel(period_cycles, imx->mmio_base + MX3_PWMPR);
/*
@@ -348,6 +396,7 @@ static int pwm_imx27_probe(struct platform_device *pdev)
return PTR_ERR(imx->clk_32k);
}
+ spin_lock_init(&imx->lock);
chip->ops = &pwm_imx27_ops;
imx->mmio_base = devm_platform_ioremap_resource(pdev, 0);
--
2.34.1
Powered by blists - more mailing lists