* [PATCH v21 0/6] Add PWM support for IPQ chipsets
@ 2026-04-06 20:24 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
` (5 more replies)
0 siblings, 6 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,
Dmitry Baryshkov, Bjorn Andersson, Krzysztof Kozlowski
Add PWM driver and binding support for IPQ chipsets.
Also, add nodes to add support for pwm in ipq6018, ipq5018, ipq5332, and
ipq9574.
I've picked up work based on Devi's last submission (v15) which dates
back to 05 October 2023 as below SoCs are still active.
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
Changes in v21:
- Added macro definition for minimum supported period and lower bound check to ensure a valid period
- Added code comment to explain why pre_div won't overflow
- Link to v20: https://patch.msgid.link/20260204-ipq-pwm-v20-0-91733011a3d1@outlook.com
Changes in v20:
- Updated IPQ_PWM_MAX_DIV macro definition to use FIELD_MAX
- Removed clk struct from ipq_pwm struct and added clk_rate field
instead which is set during probe.
- Consolidated config_div_and_duty into ipq_pwm_apply
- Fixed arithmetic overflows in apply and get_state
- Fixed off-by-one in divider calculation
- Enabled 100% relative duty cycle support
- Aligned continuation on next lines relative to opening parentheses
- Return 0 instead of ret in probe
- Link to v19: https://lore.kernel.org/r/20251128-ipq-pwm-v19-0-13bc704cc6a5@outlook.com
Changes in v19:
- Changed pwm-cells property in dt bindings from 2 to 3 as per Uwe's
recommendation
- Added hardware notes and limitations based on own findings as
requested. NOTE: there's no publically available datasheet though.
- Expanded comment on REG1_UPDATE to indicate that when this bit is set,
values for div and pre-div take effect. The hardware automatically
unsets it when the change is completed.
- Added newline between MACRO definition and next comment
- In config_div_and_duty, used mul_u64_u64_div_u64 to avoid overflow
- Removed unncessary restriction of pwm_div to MAX_DIV - 1 after testing
- Constrain pre_div to MAX_DIV is pre_div calculated is > MAX_DIV
- Use of mul_u64_u64_div_u64 in .apply
- Skip calculation of period and duty cycle when PWM_ENABLE REG is unset
- Set duty cycle to period value when calculated duty cycle > period to
return a valid config
- Removed .npwm as it's taken care of in devm_pwmchip_alloc
- Added call to devm_clk_rate_exclusive_get to lock the clock rate
- Start all kernel messages with a capital letter and end with \n.
- Changed pwm-cells property in all dtsi from 2->3 for in scope IPQ SOCs
- Link to v18: https://lore.kernel.org/r/20251029-ipq-pwm-v18-0-edbef8efbb8e@outlook.com
Changes in v18:
- Updated maintainer info in binding
- Squashed dt bindings patches into the first for adding compatibles for
IPQ5018, IPQ5332, and IPQ9574
- Link to v17: https://lore.kernel.org/r/20251008-ipq-pwm-v17-0-9bd43edfc7f7@outlook.com
Changes in v17:
- Picked up RB tags from Dmitry and Rob
- Removed unnecessary code comments
- Corrected reg property in PWM node in ipq6018 DTS in line with
expected nr of bytes for address and size cells
- Link to v16: https://lore.kernel.org/r/20251001-ipq-pwm-v16-0-300f237e0e68@outlook.com
Changes in v16:
- Removed reg description in bindings as the offset is not relative to
the TCSR region anymore since simple-mfd support was dropped and PWM
nodes defined as their own nodes, not child nodes. Updated the example
too.
- Dropped patch to add simple-mfd support to the qcom,tcsr bindings
- Simplified code to calculate divs and duty cycle as per Uwe's comments
- Removed unused pwm_chip struct from ipq_pwm_chip struct
- Removed unnecessary cast as per Uwe's comment
- Replaced devm_clk_get & clk_prepare_enable by devm_clk_get_enabled
- Replaced pwmchip_add by devm_pwmchip_add and removed .remove function
- Removed .owner from driver struct
- Added compatibles to the bindings and nodes to the device trees to add
PWM support in the IPQ5018, IPQ5332, and IPQ9574 SoCs
- Link to v15: https://lore.kernel.org/r/20231005160550.2423075-1-quic_devipriy@quicinc.com
Changes in v15:
- No change
- Link to v14: https://lore.kernel.org/r/20231005033053.2626465-1-quic_devipriy@quicinc.com
Changes in v14:
- Picked up the R-b tag
- Link to v13: https://lore.kernel.org/r/20231004090449.256229-1-quic_devipriy@quicinc.com
Changes in v13:
- Updated the file name to match the compatible
- Sorted the properties and updated the order in the required field
- Dropped the syscon node from examples
- Link to v12: https://lore.kernel.org/r/20230925065915.3467964-1-quic_devipriy@quicinc.com
Changes in v12:
- Picked up the R-b tag
Changes in v11:
- No change
Changes in v10:
- No change
Changes in v9:
- Add 'ranges' property to example (Rob)
- Drop label in example (Rob)
Changes in v8:
- Add size cell to 'reg' (Rob)
Changes in v7:
- Use 'reg' instead of 'offset' (Rob)
- Drop 'clock-names' and 'assigned-clock*' (Bjorn)
- Use single cell address/size in example node (Bjorn)
- Move '#pwm-cells' lower in example node (Bjorn)
- List 'reg' as required
Changes in v6:
- Device node is child of TCSR; remove phandle (Rob Herring)
- Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
Changes in v5:
- Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
Andersson, Kathiravan T)
Changes in v4:
- Update the binding example node as well (Rob Herring's bot)
Changes in v3:
- s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
Changes in v2:
- Make #pwm-cells const (Rob Herring)
---
---
Devi Priya (3):
dt-bindings: pwm: add IPQ6018 binding
pwm: driver for qualcomm ipq6018 pwm block
arm64: dts: qcom: ipq6018: add pwm node
George Moussalem (3):
arm64: dts: qcom: ipq5018: add pwm node
arm64: dts: qcom: ipq5332: add pwm node
arm64: dts: qcom: ipq9574: add pwm node
.../devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml | 51 ++++
arch/arm64/boot/dts/qcom/ipq5018.dtsi | 10 +
arch/arm64/boot/dts/qcom/ipq5332.dtsi | 10 +
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 10 +
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 10 +
drivers/pwm/Kconfig | 12 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-ipq.c | 259 +++++++++++++++++++++
8 files changed, 363 insertions(+)
---
base-commit: 2febe6e6ee6e34c7754eff3c4d81aa7b0dcb7979
change-id: 20250922-ipq-pwm-c8c75c147d52
Best regards,
--
George Moussalem <george.moussalem@outlook.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [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
* [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
* 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
end of thread, other threads:[~2026-04-13 20:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-13 9:35 ` Uwe Kleine-König
2026-04-13 19:17 ` George Moussalem
2026-04-13 20:27 ` 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
2026-04-06 20:24 ` [PATCH v21 4/6] arm64: dts: qcom: ipq5018: " 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox