* [PATCH v21 1/6] dt-bindings: pwm: add IPQ6018 binding
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block George Moussalem via B4 Relay
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Devi Priya, Baruch Siach, Bjorn Andersson,
Krzysztof Kozlowski
From: Devi Priya <quic_devipriy@quicinc.com>
DT binding for the PWM block in Qualcomm IPQ6018 SoC.
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
.../devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
new file mode 100644
index 000000000000..f9f1f652e752
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/qcom,ipq6018-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ6018 PWM controller
+
+maintainers:
+ - George Moussalem <george.moussalem@outlook.com>
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - qcom,ipq5018-pwm
+ - qcom,ipq5332-pwm
+ - qcom,ipq9574-pwm
+ - const: qcom,ipq6018-pwm
+ - const: qcom,ipq6018-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 3
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - "#pwm-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
+
+ pwm: pwm@1941010 {
+ compatible = "qcom,ipq6018-pwm";
+ reg = <0x01941010 0x20>;
+ clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clock-rates = <100000000>;
+ #pwm-cells = <3>;
+ };
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 1/6] dt-bindings: pwm: add IPQ6018 binding George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
2026-04-13 9:35 ` Uwe Kleine-König
2026-04-06 20:24 ` [PATCH v21 3/6] arm64: dts: qcom: ipq6018: add pwm node George Moussalem via B4 Relay
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Devi Priya, Baruch Siach
From: Devi Priya <quic_devipriy@quicinc.com>
Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
driver from downstream Codeaurora kernel tree. Removed support for older
(V1) variants because I have no access to that hardware.
Tested on IPQ5018 and IPQ6010 based hardware.
Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
drivers/pwm/Kconfig | 12 +++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-ipq.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 272 insertions(+)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 6f3147518376..e8886a9b64d9 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -347,6 +347,18 @@ config PWM_INTEL_LGM
To compile this driver as a module, choose M here: the module
will be called pwm-intel-lgm.
+config PWM_IPQ
+ tristate "IPQ PWM support"
+ depends on ARCH_QCOM || COMPILE_TEST
+ depends on HAVE_CLK && HAS_IOMEM
+ help
+ Generic PWM framework driver for IPQ PWM block which supports
+ 4 pwm channels. Each of the these channels can be configured
+ independent of each other.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-ipq.
+
config PWM_IQS620A
tristate "Azoteq IQS620A PWM support"
depends on MFD_IQS62X || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0dc0d2b69025..5630a521a7cf 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o
obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o
obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o
obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o
+obj-$(CONFIG_PWM_IPQ) += pwm-ipq.o
obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o
obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o
obj-$(CONFIG_PWM_KEEMBAY) += pwm-keembay.o
diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
new file mode 100644
index 000000000000..b79e5e457d1a
--- /dev/null
+++ b/drivers/pwm/pwm-ipq.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/*
+ * Copyright (c) 2016-2017, 2020 The Linux Foundation. All rights reserved.
+ *
+ * Hardware notes / Limitations:
+ * - The PWM controller has no publicly available datasheet.
+ * - Each of the four channels is programmed via two 32-bit registers
+ * (REG0 and REG1 at 8-byte stride).
+ * - Period and duty-cycle reconfiguration is fully atomic: new divider,
+ * pre-divider, and high-duration values are latched by setting the
+ * UPDATE bit (bit 30 in REG1). The hardware applies the new settings
+ * at the beginning of the next period without disabling the output,
+ * so the currently running period is always completed.
+ * - On disable (clearing the ENABLE bit 31 in REG1), the hardware
+ * finishes the current period before stopping the output. The pin
+ * is then driven to the inactive (low) level.
+ * - Upon disabling, the hardware resets the pre-divider (PRE_DIV) and divider
+ * fields (PWM_DIV) in REG0 and REG1 to 0x0000 and 0x0001 respectively.
+ * - Only normal polarity is supported.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/math64.h>
+#include <linux/of_device.h>
+#include <linux/bitfield.h>
+#include <linux/units.h>
+
+/* The frequency range supported is 1 Hz to 100 Mhz (clock rate) */
+#define IPQ_PWM_MAX_PERIOD_NS ((u64)NSEC_PER_SEC)
+#define IPQ_PWM_MIN_PERIOD_NS 10
+
+/*
+ * Two 32-bit registers for each PWM: REG0, and REG1.
+ * Base offset for PWM #i is at 8 * #i.
+ */
+#define IPQ_PWM_REG0 0
+#define IPQ_PWM_REG0_PWM_DIV GENMASK(15, 0)
+#define IPQ_PWM_REG0_HI_DURATION GENMASK(31, 16)
+
+#define IPQ_PWM_REG1 4
+#define IPQ_PWM_REG1_PRE_DIV GENMASK(15, 0)
+
+/*
+ * The max value specified for each field is based on the number of bits
+ * in the pwm control register for that field (16-bit)
+ */
+#define IPQ_PWM_MAX_DIV FIELD_MAX(IPQ_PWM_REG0_PWM_DIV)
+
+/*
+ * Enable bit is set to enable output toggling in pwm device.
+ * Update bit is set to trigger the change and is unset automatically
+ * to reflect the changed divider and high duration values in register.
+ */
+#define IPQ_PWM_REG1_UPDATE BIT(30)
+#define IPQ_PWM_REG1_ENABLE BIT(31)
+
+struct ipq_pwm_chip {
+ void __iomem *mem;
+ unsigned long clk_rate;
+};
+
+static struct ipq_pwm_chip *ipq_pwm_from_chip(struct pwm_chip *chip)
+{
+ return pwmchip_get_drvdata(chip);
+}
+
+static unsigned int ipq_pwm_reg_read(struct pwm_device *pwm, unsigned int reg)
+{
+ struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
+ unsigned int off = 8 * pwm->hwpwm + reg;
+
+ return readl(ipq_chip->mem + off);
+}
+
+static void ipq_pwm_reg_write(struct pwm_device *pwm, unsigned int reg,
+ unsigned int val)
+{
+ struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
+ unsigned int off = 8 * pwm->hwpwm + reg;
+
+ writel(val, ipq_chip->mem + off);
+}
+
+static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
+ unsigned int pre_div, pwm_div;
+ u64 period_ns, duty_ns;
+ unsigned long val = 0;
+ unsigned long hi_dur;
+
+ if (state->polarity != PWM_POLARITY_NORMAL)
+ return -EINVAL;
+
+ /*
+ * Check the upper and lower bounds for the period as per
+ * hardware limits
+ */
+ period_ns = max(state->period, IPQ_PWM_MIN_PERIOD_NS);
+ period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
+ duty_ns = min(state->duty_cycle, period_ns);
+
+ /*
+ * Pick the maximal value for PWM_DIV that still allows a
+ * 100% relative duty cycle. This allows a fine grained
+ * selection of duty cycles.
+ */
+ pwm_div = IPQ_PWM_MAX_DIV - 1;
+
+ /*
+ * although mul_u64_u64_div_u64 returns a u64, in practice it
+ * won't overflow due to above constraints. Take the max period
+ * of 10^9 (NSEC_PER_SEC) and the pwm_div + 1 (IPQ_PWM_MAX_DIV)
+ * 10^9 * 10^8
+ * ------------- => which fits well into a 32-bit unsigned int.
+ * 10^9 * 65,535
+ */
+ pre_div = mul_u64_u64_div_u64(period_ns, ipq_chip->clk_rate,
+ (u64)NSEC_PER_SEC * (pwm_div + 1));
+
+ if (!pre_div)
+ return -ERANGE;
+
+ pre_div -= 1;
+
+ if (pre_div > IPQ_PWM_MAX_DIV)
+ pre_div = IPQ_PWM_MAX_DIV;
+
+ /* pwm duty = HI_DUR * (PRE_DIV + 1) / clk_rate */
+ hi_dur = mul_u64_u64_div_u64(duty_ns, ipq_chip->clk_rate,
+ (u64)(pre_div + 1) * NSEC_PER_SEC);
+
+ val = FIELD_PREP(IPQ_PWM_REG0_HI_DURATION, hi_dur) |
+ FIELD_PREP(IPQ_PWM_REG0_PWM_DIV, pwm_div);
+ ipq_pwm_reg_write(pwm, IPQ_PWM_REG0, val);
+
+ val = FIELD_PREP(IPQ_PWM_REG1_PRE_DIV, pre_div);
+ ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
+
+ /* PWM enable toggle needs a separate write to REG1 */
+ val |= IPQ_PWM_REG1_UPDATE;
+ if (state->enabled)
+ val |= IPQ_PWM_REG1_ENABLE;
+ ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
+
+ return 0;
+}
+
+static int ipq_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
+ unsigned int pre_div, pwm_div, hi_dur;
+ u64 effective_div, hi_div;
+ u32 reg0, reg1;
+
+ reg1 = ipq_pwm_reg_read(pwm, IPQ_PWM_REG1);
+ state->enabled = reg1 & IPQ_PWM_REG1_ENABLE;
+
+ if (!state->enabled)
+ return 0;
+
+ reg0 = ipq_pwm_reg_read(pwm, IPQ_PWM_REG0);
+
+ state->polarity = PWM_POLARITY_NORMAL;
+
+ pwm_div = FIELD_GET(IPQ_PWM_REG0_PWM_DIV, reg0);
+ hi_dur = FIELD_GET(IPQ_PWM_REG0_HI_DURATION, reg0);
+ pre_div = FIELD_GET(IPQ_PWM_REG1_PRE_DIV, reg1);
+
+ effective_div = (u64)(pre_div + 1) * (pwm_div + 1);
+
+ /*
+ * effective_div <= 0x100000000, so the multiplication doesn't overflow.
+ */
+ state->period = DIV64_U64_ROUND_UP(effective_div * NSEC_PER_SEC,
+ ipq_chip->clk_rate);
+
+ hi_div = hi_dur * (pre_div + 1);
+ state->duty_cycle = DIV64_U64_ROUND_UP(hi_div * NSEC_PER_SEC,
+ ipq_chip->clk_rate);
+
+ /*
+ * ensure a valid config is passed back to PWM core in case duty_cycle
+ * is > period (>100%)
+ */
+ state->duty_cycle = min(state->duty_cycle, state->period);
+
+ return 0;
+}
+
+static const struct pwm_ops ipq_pwm_ops = {
+ .apply = ipq_pwm_apply,
+ .get_state = ipq_pwm_get_state,
+};
+
+static int ipq_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ipq_pwm_chip *pwm;
+ struct pwm_chip *chip;
+ struct clk *clk;
+ int ret;
+
+ chip = devm_pwmchip_alloc(dev, 4, sizeof(*pwm));
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+ pwm = ipq_pwm_from_chip(chip);
+
+ pwm->mem = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(pwm->mem))
+ return dev_err_probe(dev, PTR_ERR(pwm->mem),
+ "Failed to acquire resource\n");
+
+ clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "Failed to get clock\n");
+
+ ret = devm_clk_rate_exclusive_get(dev, clk);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to lock clock rate\n");
+
+ pwm->clk_rate = clk_get_rate(clk);
+ if (!pwm->clk_rate)
+ return dev_err_probe(dev, -EINVAL, "Failed due to clock rate being zero\n");
+
+ chip->ops = &ipq_pwm_ops;
+
+ ret = devm_pwmchip_add(dev, chip);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to add pwm chip\n");
+
+ return 0;
+}
+
+static const struct of_device_id pwm_ipq_dt_match[] = {
+ { .compatible = "qcom,ipq6018-pwm", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pwm_ipq_dt_match);
+
+static struct platform_driver ipq_pwm_driver = {
+ .driver = {
+ .name = "ipq-pwm",
+ .of_match_table = pwm_ipq_dt_match,
+ },
+ .probe = ipq_pwm_probe,
+};
+
+module_platform_driver(ipq_pwm_driver);
+
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block
2026-04-06 20:24 ` [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block George Moussalem via B4 Relay
@ 2026-04-13 9:35 ` Uwe Kleine-König
2026-04-13 19:17 ` George Moussalem
0 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2026-04-13 9:35 UTC (permalink / raw)
To: george.moussalem
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, linux-pwm, devicetree, linux-kernel,
Devi Priya
[-- Attachment #1: Type: text/plain, Size: 5061 bytes --]
Hello,
On Mon, Apr 06, 2026 at 10:24:39PM +0200, George Moussalem via B4 Relay wrote:
> From: Devi Priya <quic_devipriy@quicinc.com>
>
> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
> driver from downstream Codeaurora kernel tree. Removed support for older
> (V1) variants because I have no access to that hardware.
>
> Tested on IPQ5018 and IPQ6010 based hardware.
>
> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
I have a few remaining nitpicks. If you're ok I'll squash the following
diff into this patch and apply it:
diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
index b79e5e457d1a..65af19ded72c 100644
--- a/drivers/pwm/pwm-ipq.c
+++ b/drivers/pwm/pwm-ipq.c
@@ -2,7 +2,7 @@
/*
* Copyright (c) 2016-2017, 2020 The Linux Foundation. All rights reserved.
*
- * Hardware notes / Limitations:
+ * Limitations:
* - The PWM controller has no publicly available datasheet.
* - Each of the four channels is programmed via two 32-bit registers
* (REG0 and REG1 at 8-byte stride).
This is to make
sed -rn '/Limitations:/,/\*\/?$/p' drivers/pwm/*.c
do the right thing. I know "Limitations" isn't a good subject for this,
but until I come around to pick a better marker, doing the same in all
drivers is good.
@@ -44,13 +44,6 @@
#define IPQ_PWM_REG1 4
#define IPQ_PWM_REG1_PRE_DIV GENMASK(15, 0)
-
-/*
- * The max value specified for each field is based on the number of bits
- * in the pwm control register for that field (16-bit)
- */
-#define IPQ_PWM_MAX_DIV FIELD_MAX(IPQ_PWM_REG0_PWM_DIV)
-
/*
* Enable bit is set to enable output toggling in pwm device.
* Update bit is set to trigger the change and is unset automatically
@@ -59,6 +52,12 @@
#define IPQ_PWM_REG1_UPDATE BIT(30)
#define IPQ_PWM_REG1_ENABLE BIT(31)
+/*
+ * The max value specified for each field is based on the number of bits
+ * in the pwm control register for that field (16-bit)
+ */
+#define IPQ_PWM_MAX_DIV FIELD_MAX(IPQ_PWM_REG0_PWM_DIV)
+
struct ipq_pwm_chip {
void __iomem *mem;
unsigned long clk_rate;
This is just about ordering definitions taken 1:1 from the manual before
driver specific stuff.
@@ -95,6 +94,12 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
unsigned long val = 0;
unsigned long hi_dur;
+ if (!state->enabled) {
+ /* clear IPQ_PWM_REG1_ENABLE */
+ ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, IPQ_PWM_REG1_UPDATE);
+ return 0;
+ }
+
if (state->polarity != PWM_POLARITY_NORMAL)
return -EINVAL;
This ensures that the PWM can be disabled even if state->polarity is
bogus or period and duty_cycle are out of range.
@@ -102,7 +107,8 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* Check the upper and lower bounds for the period as per
* hardware limits
*/
- period_ns = max(state->period, IPQ_PWM_MIN_PERIOD_NS);
+ if (state->period < IPQ_PWM_MIN_PERIOD_NS)
+ return -ERANGE;
period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
duty_ns = min(state->duty_cycle, period_ns);
This is about correctness. A driver is expected to never configure a
higher value than requested. (And otherwise I would have converted that
to clamp().)
@@ -134,7 +140,7 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
/* pwm duty = HI_DUR * (PRE_DIV + 1) / clk_rate */
hi_dur = mul_u64_u64_div_u64(duty_ns, ipq_chip->clk_rate,
- (u64)(pre_div + 1) * NSEC_PER_SEC);
+ (u64)NSEC_PER_SEC * (pre_div + 1));
val = FIELD_PREP(IPQ_PWM_REG0_HI_DURATION, hi_dur) |
FIELD_PREP(IPQ_PWM_REG0_PWM_DIV, pwm_div);
Just consistency with the period calculation
@@ -144,9 +150,7 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
/* PWM enable toggle needs a separate write to REG1 */
- val |= IPQ_PWM_REG1_UPDATE;
- if (state->enabled)
- val |= IPQ_PWM_REG1_ENABLE;
+ val |= IPQ_PWM_REG1_UPDATE | IPQ_PWM_REG1_ENABLE;
ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
return 0;
Simplification that is possible after checking for state->enabled early.
@@ -174,7 +178,7 @@ static int ipq_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
hi_dur = FIELD_GET(IPQ_PWM_REG0_HI_DURATION, reg0);
pre_div = FIELD_GET(IPQ_PWM_REG1_PRE_DIV, reg1);
- effective_div = (u64)(pre_div + 1) * (pwm_div + 1);
+ effective_div = (u64)(pwm_div + 1) * (pre_div + 1)
/*
* effective_div <= 0x100000000, so the multiplication doesn't overflow.
Again consistency.
A nice followup for this patch would be the conversion to the waveform
API; just in case you're still motivated to work on this driver :-)
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block
2026-04-13 9:35 ` Uwe Kleine-König
@ 2026-04-13 19:17 ` George Moussalem
2026-04-13 20:27 ` Uwe Kleine-König
0 siblings, 1 reply; 10+ messages in thread
From: George Moussalem @ 2026-04-13 19:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, linux-pwm, devicetree, linux-kernel,
Devi Priya
Hello
On 4/13/2026 1:35 PM, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Apr 06, 2026 at 10:24:39PM +0200, George Moussalem via B4 Relay wrote:
>> From: Devi Priya <quic_devipriy@quicinc.com>
>>
>> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
>> driver from downstream Codeaurora kernel tree. Removed support for older
>> (V1) variants because I have no access to that hardware.
>>
>> Tested on IPQ5018 and IPQ6010 based hardware.
>>
>> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
>> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>
> I have a few remaining nitpicks. If you're ok I'll squash the following
> diff into this patch and apply it:
Just applied it to my own branch, all okay from my side. Thanks for your
guidance and support!
>
> diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
> index b79e5e457d1a..65af19ded72c 100644
> --- a/drivers/pwm/pwm-ipq.c
> +++ b/drivers/pwm/pwm-ipq.c
> @@ -2,7 +2,7 @@
> /*
> * Copyright (c) 2016-2017, 2020 The Linux Foundation. All rights reserved.
> *
> - * Hardware notes / Limitations:
> + * Limitations:
> * - The PWM controller has no publicly available datasheet.
> * - Each of the four channels is programmed via two 32-bit registers
> * (REG0 and REG1 at 8-byte stride).
>
> This is to make
>
> sed -rn '/Limitations:/,/\*\/?$/p' drivers/pwm/*.c
>
> do the right thing. I know "Limitations" isn't a good subject for this,
> but until I come around to pick a better marker, doing the same in all
> drivers is good.
>
> @@ -44,13 +44,6 @@
>
> #define IPQ_PWM_REG1 4
> #define IPQ_PWM_REG1_PRE_DIV GENMASK(15, 0)
> -
> -/*
> - * The max value specified for each field is based on the number of bits
> - * in the pwm control register for that field (16-bit)
> - */
> -#define IPQ_PWM_MAX_DIV FIELD_MAX(IPQ_PWM_REG0_PWM_DIV)
> -
> /*
> * Enable bit is set to enable output toggling in pwm device.
> * Update bit is set to trigger the change and is unset automatically
> @@ -59,6 +52,12 @@
> #define IPQ_PWM_REG1_UPDATE BIT(30)
> #define IPQ_PWM_REG1_ENABLE BIT(31)
>
> +/*
> + * The max value specified for each field is based on the number of bits
> + * in the pwm control register for that field (16-bit)
> + */
> +#define IPQ_PWM_MAX_DIV FIELD_MAX(IPQ_PWM_REG0_PWM_DIV)
> +
> struct ipq_pwm_chip {
> void __iomem *mem;
> unsigned long clk_rate;
>
> This is just about ordering definitions taken 1:1 from the manual before
> driver specific stuff.
>
> @@ -95,6 +94,12 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> unsigned long val = 0;
> unsigned long hi_dur;
>
> + if (!state->enabled) {
> + /* clear IPQ_PWM_REG1_ENABLE */
> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, IPQ_PWM_REG1_UPDATE);
> + return 0;
> + }
> +
> if (state->polarity != PWM_POLARITY_NORMAL)
> return -EINVAL;
>
> This ensures that the PWM can be disabled even if state->polarity is
> bogus or period and duty_cycle are out of range.
>
> @@ -102,7 +107,8 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> * Check the upper and lower bounds for the period as per
> * hardware limits
> */
> - period_ns = max(state->period, IPQ_PWM_MIN_PERIOD_NS);
> + if (state->period < IPQ_PWM_MIN_PERIOD_NS)
> + return -ERANGE;
> period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
> duty_ns = min(state->duty_cycle, period_ns);
>
> This is about correctness. A driver is expected to never configure a
> higher value than requested. (And otherwise I would have converted that
> to clamp().)
>
> @@ -134,7 +140,7 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>
> /* pwm duty = HI_DUR * (PRE_DIV + 1) / clk_rate */
> hi_dur = mul_u64_u64_div_u64(duty_ns, ipq_chip->clk_rate,
> - (u64)(pre_div + 1) * NSEC_PER_SEC);
> + (u64)NSEC_PER_SEC * (pre_div + 1));
>
> val = FIELD_PREP(IPQ_PWM_REG0_HI_DURATION, hi_dur) |
> FIELD_PREP(IPQ_PWM_REG0_PWM_DIV, pwm_div);
>
> Just consistency with the period calculation
>
> @@ -144,9 +150,7 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
>
> /* PWM enable toggle needs a separate write to REG1 */
> - val |= IPQ_PWM_REG1_UPDATE;
> - if (state->enabled)
> - val |= IPQ_PWM_REG1_ENABLE;
> + val |= IPQ_PWM_REG1_UPDATE | IPQ_PWM_REG1_ENABLE;
> ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
>
> return 0;
>
> Simplification that is possible after checking for state->enabled early.
>
> @@ -174,7 +178,7 @@ static int ipq_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> hi_dur = FIELD_GET(IPQ_PWM_REG0_HI_DURATION, reg0);
> pre_div = FIELD_GET(IPQ_PWM_REG1_PRE_DIV, reg1);
>
> - effective_div = (u64)(pre_div + 1) * (pwm_div + 1);
> + effective_div = (u64)(pwm_div + 1) * (pre_div + 1)
>
> /*
> * effective_div <= 0x100000000, so the multiplication doesn't overflow.
>
> Again consistency.
>
> A nice followup for this patch would be the conversion to the waveform
> API; just in case you're still motivated to work on this driver :-)
>
> Best regards
> Uwe
Best regards,
George
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block
2026-04-13 19:17 ` George Moussalem
@ 2026-04-13 20:27 ` Uwe Kleine-König
0 siblings, 0 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2026-04-13 20:27 UTC (permalink / raw)
To: George Moussalem
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, linux-pwm, devicetree, linux-kernel,
Devi Priya
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
Hello George,
On Mon, Apr 13, 2026 at 11:17:18PM +0400, George Moussalem wrote:
> On 4/13/2026 1:35 PM, Uwe Kleine-König wrote:
> > On Mon, Apr 06, 2026 at 10:24:39PM +0200, George Moussalem via B4 Relay wrote:
> >> From: Devi Priya <quic_devipriy@quicinc.com>
> >>
> >> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
> >> driver from downstream Codeaurora kernel tree. Removed support for older
> >> (V1) variants because I have no access to that hardware.
> >>
> >> Tested on IPQ5018 and IPQ6010 based hardware.
> >>
> >> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> >> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> >> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> >> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
> >> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> >
> > I have a few remaining nitpicks. If you're ok I'll squash the following
> > diff into this patch and apply it:
>
> Just applied it to my own branch, all okay from my side. Thanks for your
> guidance and support!
I did that and added a ; to make the compiler happy. It is now contained
at
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-nexxt
as 7.2-rc1 material. I'll push it into next after the merge window
closes in ~ two weeks.
Best regards and thanks for work
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v21 3/6] arm64: dts: qcom: ipq6018: add pwm node
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 1/6] dt-bindings: pwm: add IPQ6018 binding George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 2/6] pwm: driver for qualcomm ipq6018 pwm block George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 4/6] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Devi Priya, Baruch Siach, Konrad Dybcio,
Krzysztof Kozlowski
From: Devi Priya <quic_devipriy@quicinc.com>
Describe the PWM block on IPQ6018.
Although PWM is in the TCSR area, make pwm its own node as simple-mfd
has been removed from the bindings and as such hardware components
should have its own node.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 40f1c262126e..7866844cc09f 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -413,6 +413,16 @@ tcsr: syscon@1937000 {
reg = <0x0 0x01937000 0x0 0x21000>;
};
+ pwm: pwm@1941010 {
+ compatible = "qcom,ipq6018-pwm";
+ reg = <0x0 0x01941010 0x0 0x20>;
+ clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clock-rates = <100000000>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
usb2: usb@70f8800 {
compatible = "qcom,ipq6018-dwc3", "qcom,dwc3";
reg = <0x0 0x070f8800 0x0 0x400>;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v21 4/6] arm64: dts: qcom: ipq5018: add pwm node
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
` (2 preceding siblings ...)
2026-04-06 20:24 ` [PATCH v21 3/6] arm64: dts: qcom: ipq6018: add pwm node George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 5/6] arm64: dts: qcom: ipq5332: " George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 6/6] arm64: dts: qcom: ipq9574: " George Moussalem via B4 Relay
5 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Dmitry Baryshkov, Konrad Dybcio
From: George Moussalem <george.moussalem@outlook.com>
Describe the PWM block on IPQ5018.
Although PWM is in the TCSR area, make pwm its own node as simple-mfd
has been removed from the bindings and as such hardware components
should have its own node.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
arch/arm64/boot/dts/qcom/ipq5018.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 6f8004a22a1f..edff89257468 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -453,6 +453,16 @@ tcsr: syscon@1937000 {
reg = <0x01937000 0x21000>;
};
+ pwm: pwm@1941010 {
+ compatible = "qcom,ipq5018-pwm", "qcom,ipq6018-pwm";
+ reg = <0x01941010 0x20>;
+ clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clock-rates = <100000000>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
sdhc_1: mmc@7804000 {
compatible = "qcom,ipq5018-sdhci", "qcom,sdhci-msm-v5";
reg = <0x7804000 0x1000>;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v21 5/6] arm64: dts: qcom: ipq5332: add pwm node
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
` (3 preceding siblings ...)
2026-04-06 20:24 ` [PATCH v21 4/6] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
2026-04-06 20:24 ` [PATCH v21 6/6] arm64: dts: qcom: ipq9574: " George Moussalem via B4 Relay
5 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Dmitry Baryshkov, Konrad Dybcio
From: George Moussalem <george.moussalem@outlook.com>
Describe the PWM block on IPQ5332.
Although PWM is in the TCSR area, make pwm its own node as simple-mfd
has been removed from the bindings and as such hardware components
should have its own node.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
arch/arm64/boot/dts/qcom/ipq5332.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index e227730d99a6..27504b7cfe9e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -334,6 +334,16 @@ tcsr: syscon@1937000 {
reg = <0x01937000 0x21000>;
};
+ pwm: pwm@1941010 {
+ compatible = "qcom,ipq5332-pwm", "qcom,ipq6018-pwm";
+ reg = <0x01941010 0x20>;
+ clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clock-rates = <100000000>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
sdhc: mmc@7804000 {
compatible = "qcom,ipq5332-sdhci", "qcom,sdhci-msm-v5";
reg = <0x07804000 0x1000>, <0x07805000 0x1000>;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v21 6/6] arm64: dts: qcom: ipq9574: add pwm node
2026-04-06 20:24 [PATCH v21 0/6] Add PWM support for IPQ chipsets George Moussalem via B4 Relay
` (4 preceding siblings ...)
2026-04-06 20:24 ` [PATCH v21 5/6] arm64: dts: qcom: ipq5332: " George Moussalem via B4 Relay
@ 2026-04-06 20:24 ` George Moussalem via B4 Relay
5 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-04-06 20:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pwm, devicetree, linux-kernel,
George Moussalem, Dmitry Baryshkov, Konrad Dybcio
From: George Moussalem <george.moussalem@outlook.com>
Describe the PWM block on IPQ9574.
Although PWM is in the TCSR area, make pwm its own node as simple-mfd
has been removed from the bindings and as such hardware components
should have its own node.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 622cfa96ed2b..3f15c40f7841 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -445,6 +445,16 @@ tcsr: syscon@1937000 {
reg = <0x01937000 0x21000>;
};
+ pwm: pwm@1941010 {
+ compatible = "qcom,ipq9574-pwm", "qcom,ipq6018-pwm";
+ reg = <0x01941010 0x20>;
+ clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+ assigned-clock-rates = <100000000>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
sdhc_1: mmc@7804000 {
compatible = "qcom,ipq9574-sdhci", "qcom,sdhci-msm-v5";
reg = <0x07804000 0x1000>,
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread