[PATCH v19 0/6] Add PWM support for IPQ chipsets

[PATCH v19 0/6] Add PWM support for IPQ chipsets

[George Moussalem via B4 Relay]

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 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)

---
George Moussalem (6):
      dt-bindings: pwm: qcom,ipq6018-pwm: Add compatible for ipq5018
      dt-bindings: pwm: qcom,ipq6018-pwm: Add compatible for ipq5332
      dt-bindings: pwm: qcom,ipq6018-pwm: Add compatible for ipq9574
      arm64: dts: qcom: ipq5018: add pwm node
      arm64: dts: qcom: ipq5332: add pwm node
      arm64: dts: qcom: ipq9574: add pwm node

Devi Priya (3):
      dt-bindings: pwm: add IPQ6018 binding
      pwm: driver for qualcomm ipq6018 pwm block
      arm64: dts: qcom: ipq6018: 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                              | 212 +++++++++++++++++++++
 8 files changed, 316 insertions(+)
---

---
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                              | 239 +++++++++++++++++++++
 8 files changed, 343 insertions(+)
---
base-commit: e15625dc543c0fbb8db84589678613efd6fd4f27
change-id: 20250922-ipq-pwm-c8c75c147d52

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>

[PATCH v19 1/6] dt-bindings: pwm: add IPQ6018 binding

[George Moussalem via B4 Relay]

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 0000000000000000000000000000000000000000..f9f1f652e7527bc8fb3d5fad51b0057ea53b3766
--- /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.52.0

[PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[George Moussalem via B4 Relay]

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>
---
v18:

  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.

v17:

  Removed unnecessary code comments

v16:

  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

v15:

  No change

v14:

  Picked up the R-b tag

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

v12:

  Picked up the R-b tag

v11:

  No change

v10:

  No change

v9:

  Add 'ranges' property to example (Rob)

  Drop label in example (Rob)

v8:

  Add size cell to 'reg' (Rob)

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

v6:

  Device node is child of TCSR; remove phandle (Rob Herring)

  Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)

v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
    Andersson, Kathiravan T)

v4: Update the binding example node as well (Rob Herring's bot)

v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)

v2: Make #pwm-cells const (Rob Herring)
---
 drivers/pwm/Kconfig   |  12 +++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-ipq.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 252 insertions(+)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index bf2d101f67a1e0ae12a58b5630abd5cfd77ccc99..6393f4e91697ae471b1aba72e7ef3f94c5e18383 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 0dc0d2b69025dbd27013285cd335d3cb1ca2ab3f..5630a521a7cffeb83ff8c8960e15eb23ddb1c9f8 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 0000000000000000000000000000000000000000..9955b185bc60f27d770f3833d5acd7f587595324
--- /dev/null
+++ b/drivers/pwm/pwm-ipq.c
@@ -0,0 +1,239 @@
+// 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 clock rate */
+#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)
+
+/*
+ * The max value specified for each field is based on the number of bits
+ * in the pwm control register for that field
+ */
+#define IPQ_PWM_MAX_DIV		0xFFFF
+
+/*
+ * 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)
+
+/*
+ * 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 {
+	struct clk *clk;
+	void __iomem *mem;
+};
+
+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 void config_div_and_duty(struct pwm_device *pwm, unsigned int pre_div,
+				unsigned int pwm_div, unsigned long rate, u64 duty_ns,
+				bool enable)
+{
+	unsigned long hi_dur;
+	unsigned long val = 0;
+
+	/*
+	 * high duration = pwm duty * (pwm div + 1)
+	 * pwm duty = duty_ns / period_ns
+	 */
+	hi_dur = mul_u64_u64_div_u64(duty_ns, rate, (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 (enable)
+		val |= IPQ_PWM_REG1_ENABLE;
+	ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
+}
+
+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 long rate = clk_get_rate(ipq_chip->clk);
+	unsigned int pre_div, pwm_div;
+	u64 period_ns, duty_ns;
+
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		return -EINVAL;
+
+	if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
+		return -ERANGE;
+
+	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
+	duty_ns = min(state->duty_cycle, period_ns);
+
+	pwm_div = IPQ_PWM_MAX_DIV;
+	pre_div = mul_u64_u64_div_u64(period_ns, rate, (u64)NSEC_PER_SEC * (pwm_div + 1));
+
+	if (pre_div > IPQ_PWM_MAX_DIV)
+		pre_div = IPQ_PWM_MAX_DIV;
+
+	config_div_and_duty(pwm, pre_div, pwm_div, rate, duty_ns, state->enabled);
+
+	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 long rate = clk_get_rate(ipq_chip->clk);
+	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);
+
+	/* No overflow here, both pre_div and pwm_div <= 0xffff */
+	effective_div = (pre_div + 1) * (pwm_div + 1);
+	state->period = DIV64_U64_ROUND_UP(effective_div * NSEC_PER_SEC, rate);
+
+	hi_div = hi_dur * (pre_div + 1);
+	state->duty_cycle = DIV64_U64_ROUND_UP(hi_div * NSEC_PER_SEC, 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;
+	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");
+
+	pwm->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(pwm->clk))
+		return dev_err_probe(dev, PTR_ERR(pwm->clk),
+				"Failed to get clock\n");
+
+	ret = devm_clk_rate_exclusive_get(dev, pwm->clk);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				"Failed to lock clock rate\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 ret;
+}
+
+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.52.0

[PATCH v19 3/6] arm64: dts: qcom: ipq6018: add pwm node

[George Moussalem via B4 Relay]

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>
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 40f1c262126eff3761430a47472b52d27f961040..7866844cc09fd2c2c2f512ce2c8fa7826fabc7aa 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.52.0

[PATCH v19 4/6] arm64: dts: qcom: ipq5018: add pwm node

[George Moussalem via B4 Relay]

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>
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 f024b3cba33f6100ac3f4d45598ff2356e026dcf..562e81c2670ce95e306401f4f4646fd7f9950fe6 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.52.0

[PATCH v19 5/6] arm64: dts: qcom: ipq5332: add pwm node

[George Moussalem via B4 Relay]

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>
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 45fc512a3bab221c0d99f819294abf63369987da..e58051f6c1c4fd8ef85cbcc9b98433f599377c7a 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.52.0

[PATCH v19 6/6] arm64: dts: qcom: ipq9574: add pwm node

[George Moussalem via B4 Relay]

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>
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 86c9cb9fffc98fdd1b0b08e81428ce5e7bb87e17..baf165dcb6b1be823332cdfac631eef2633867b4 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -449,6 +449,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.52.0

Re: [PATCH v19 1/6] dt-bindings: pwm: add IPQ6018 binding

[Rob Herring (Arm)]

On Fri, 28 Nov 2025 14:29:13 +0400, George Moussalem wrote:
> 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(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251128-ipq-pwm-v19-1-13bc704cc6a5@outlook.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[Konrad Dybcio]

On 11/28/25 11:29 AM, 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>
> ---

[...]

> +/* The frequency range supported is 1 Hz to clock rate */
> +#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)
> +
> +/*
> + * The max value specified for each field is based on the number of bits
> + * in the pwm control register for that field
> + */
> +#define IPQ_PWM_MAX_DIV		0xFFFF
> +
> +/*
> + * 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)

Sorry for coming in so late, you may consider this as material for a
follow-up patch (I *really* don't want to hold off your v19..)

I see that on ipq9574 the registers are named:

base = 0x1941010 = tcsr + 0xa010

0x0	CFG0_R0
0x4	CFG1_R0
0x8	CFG0_R1
0xc	CFG1_R1
0x10	CFG0_R2
0x14	CFG1_R2
0x18	CFG0_R3
0x1c	CFG1_R3

CFG0 and CFG1 are what you called REG0/REG1 and Rn is confusingly the
index of the controller/output

The other bits in CFG1 (29:16) are RESERVED so there's nothing you
missed in there

> +
> +/*
> + * 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 {
> +	struct clk *clk;
> +	void __iomem *mem;
> +};
> +
> +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;

This magic 8 could be #defined as IPQ6018_PWM_CONTROLLER_STRIDE or so

Konrad

Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[George Moussalem]

On 11/28/25 15:32, Konrad Dybcio wrote:
> On 11/28/25 11:29 AM, 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>
>> ---
> 
> [...]
> 
>> +/* The frequency range supported is 1 Hz to clock rate */
>> +#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)
>> +
>> +/*
>> + * The max value specified for each field is based on the number of bits
>> + * in the pwm control register for that field
>> + */
>> +#define IPQ_PWM_MAX_DIV		0xFFFF
>> +
>> +/*
>> + * 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)
> 
> Sorry for coming in so late, you may consider this as material for a
> follow-up patch (I *really* don't want to hold off your v19..)
> 
> I see that on ipq9574 the registers are named:
> 
> base = 0x1941010 = tcsr + 0xa010
> 
> 0x0	CFG0_R0
> 0x4	CFG1_R0
> 0x8	CFG0_R1
> 0xc	CFG1_R1
> 0x10	CFG0_R2
> 0x14	CFG1_R2
> 0x18	CFG0_R3
> 0x1c	CFG1_R3
> 
> CFG0 and CFG1 are what you called REG0/REG1 and Rn is confusingly the
> index of the controller/output
> 
> The other bits in CFG1 (29:16) are RESERVED so there's nothing you
> missed in there

thanks for your review and validation.

> 
>> +
>> +/*
>> + * 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 {
>> +	struct clk *clk;
>> +	void __iomem *mem;
>> +};
>> +
>> +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;
> 
> This magic 8 could be #defined as IPQ6018_PWM_CONTROLLER_STRIDE or so

good suggestion, will add it should another iteration be required.

> 
> Konrad

BR,
George

Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 14354 bytes --]

Hello,

On Fri, Nov 28, 2025 at 02:29:14PM +0400, 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>
> ---
> v18:
> 
>   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.
> 
> v17:
> 
>   Removed unnecessary code comments
> 
> v16:
> 
>   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
> 
> v15:
> 
>   No change
> 
> v14:
> 
>   Picked up the R-b tag
> 
> 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
> 
> v12:
> 
>   Picked up the R-b tag
> 
> v11:
> 
>   No change
> 
> v10:
> 
>   No change
> 
> v9:
> 
>   Add 'ranges' property to example (Rob)
> 
>   Drop label in example (Rob)
> 
> v8:
> 
>   Add size cell to 'reg' (Rob)
> 
> 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
> 
> v6:
> 
>   Device node is child of TCSR; remove phandle (Rob Herring)
> 
>   Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
> 
> v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
>     Andersson, Kathiravan T)
> 
> v4: Update the binding example node as well (Rob Herring's bot)
> 
> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> 
> v2: Make #pwm-cells const (Rob Herring)
> ---
>  drivers/pwm/Kconfig   |  12 +++
>  drivers/pwm/Makefile  |   1 +
>  drivers/pwm/pwm-ipq.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 252 insertions(+)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index bf2d101f67a1e0ae12a58b5630abd5cfd77ccc99..6393f4e91697ae471b1aba72e7ef3f94c5e18383 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 0dc0d2b69025dbd27013285cd335d3cb1ca2ab3f..5630a521a7cffeb83ff8c8960e15eb23ddb1c9f8 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 0000000000000000000000000000000000000000..9955b185bc60f27d770f3833d5acd7f587595324
> --- /dev/null
> +++ b/drivers/pwm/pwm-ipq.c
> @@ -0,0 +1,239 @@
> +// 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 clock rate */
> +#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)

This is used below with:
	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);

Where does this limitation come from?

> +/*
> + * The max value specified for each field is based on the number of bits
> + * in the pwm control register for that field
> + */
> +#define IPQ_PWM_MAX_DIV		0xFFFF

Maybe use (depending on context) FIELD_MAX(IPQ_PWM_REG0_PWM_DIV) or
FIELD_MAX(IPQ_PWM_REG1_PRE_DIV).

> +
> +/*
> + * 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)
> +
> +/*
> + * 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 {
> +	struct clk *clk;

you're using clk_rate_exclusive_get(), so it's enough to call
clk_get_rate() once. Then you can store the rate here instead of the
clk itself.

> +	void __iomem *mem;
> +};
> +
> +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 void config_div_and_duty(struct pwm_device *pwm, unsigned int pre_div,

Missing name prefix (i.e. rename to ipq_pwm_config_div_and_duty(), or
maybe even better: fold into ipq_pwm_apply()).

> +				unsigned int pwm_div, unsigned long rate, u64 duty_ns,
> +				bool enable)
> +{
> +	unsigned long hi_dur;
> +	unsigned long val = 0;
> +
> +	/*
> +	 * high duration = pwm duty * (pwm div + 1)
> +	 * pwm duty = duty_ns / period_ns
> +	 */
> +	hi_dur = mul_u64_u64_div_u64(duty_ns, rate, (pre_div + 1) * NSEC_PER_SEC);

With pre_div = 0xffff the multiplication in the 3rd parameter overflows.

> +	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 (enable)
> +		val |= IPQ_PWM_REG1_ENABLE;
> +	ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
> +}
> +
> +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 long rate = clk_get_rate(ipq_chip->clk);
> +	unsigned int pre_div, pwm_div;
> +	u64 period_ns, duty_ns;
> +
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;
> +
> +	if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
> +		return -ERANGE;
> +
> +	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
> +	duty_ns = min(state->duty_cycle, period_ns);
> +
> +	pwm_div = IPQ_PWM_MAX_DIV;

With pwm_div = 0xffff you cannot configure a 100% relative duty cycle,
right?

> +	pre_div = mul_u64_u64_div_u64(period_ns, rate, (u64)NSEC_PER_SEC * (pwm_div + 1));

according to the get_state() callback below we have:

            (PRE_DIV + 1) (PWM_DIV + 1)
	P = ---------------------------
	                RATE

The (first) +1 isn't accounted for in your formula.
	
> +
> +	if (pre_div > IPQ_PWM_MAX_DIV)
> +		pre_div = IPQ_PWM_MAX_DIV;
> +
> +	config_div_and_duty(pwm, pre_div, pwm_div, rate, duty_ns, state->enabled);
> +
> +	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 long rate = clk_get_rate(ipq_chip->clk);
> +	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);
> +
> +	/* No overflow here, both pre_div and pwm_div <= 0xffff */
> +	effective_div = (pre_div + 1) * (pwm_div + 1);

With pre_div = pwm_div = 0xffff this yields 0x100000000 which overflows
a (32 bit) unsigned int.

> +	state->period = DIV64_U64_ROUND_UP(effective_div * NSEC_PER_SEC, rate);
> +
> +	hi_div = hi_dur * (pre_div + 1);
> +	state->duty_cycle = DIV64_U64_ROUND_UP(hi_div * NSEC_PER_SEC, 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;
> +	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");

Please align continuation lines to the opening (.

> +
> +	pwm->clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(pwm->clk))
> +		return dev_err_probe(dev, PTR_ERR(pwm->clk),
> +				"Failed to get clock\n");
> +
> +	ret = devm_clk_rate_exclusive_get(dev, pwm->clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				"Failed to lock clock rate\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 ret;

You could return 0 here which is a tad clearer.

> +}
> +
> +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");

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v19 1/6] dt-bindings: pwm: add IPQ6018 binding

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

Hello,

On Fri, Nov 28, 2025 at 05:18:38AM -0600, Rob Herring (Arm) wrote:
> On Fri, 28 Nov 2025 14:29:13 +0400, George Moussalem wrote:
> > 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(+)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']

I admit I didn't try to reproduce that, but the patch in question
creates Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml and
isn't about temperature stuff at all. So I don't understand where
thermal-sensor and tsens comes into play here. Bot bug?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

Hello,

On Mon, Dec 08, 2025 at 07:40:21PM +0400, George Moussalem wrote:
> On 11/28/25 15:32, Konrad Dybcio wrote:
> > On 11/28/25 11:29 AM, George Moussalem via B4 Relay wrote:
> >> +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;
> > 
> > This magic 8 could be #defined as IPQ6018_PWM_CONTROLLER_STRIDE or so
> 
> good suggestion, will add it should another iteration be required.

I don't consider that an advantage. If I see a register dump and want to
interpret it, I think

	8 * pwm->hwpwm + reg

is much more helpful than

	IPQ_PWM_CONTROLLER_STRIDE * pwm->hwpwm + reg

where I have to check the value of the define. Yes, then the 8 appears
twice (i.e. in reg_read and reg_write), but IMHO that's ok.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v19 1/6] dt-bindings: pwm: add IPQ6018 binding

[Konrad Dybcio]

On 1/19/26 5:35 PM, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Nov 28, 2025 at 05:18:38AM -0600, Rob Herring (Arm) wrote:
>> On Fri, 28 Nov 2025 14:29:13 +0400, George Moussalem wrote:
>>> 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(+)
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
>> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
>> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
>> Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
> 
> I admit I didn't try to reproduce that, but the patch in question
> creates Documentation/devicetree/bindings/pwm/qcom,ipq6018-pwm.yaml and
> isn't about temperature stuff at all. So I don't understand where
> thermal-sensor and tsens comes into play here. Bot bug?

IIRC this was the cause

https://lore.kernel.org/lkml/20251203152437.3751325-1-robh@kernel.org/

Konrad

Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

[George Moussalem]

Hi Uwe,

On 1/19/26 20:30, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Nov 28, 2025 at 02:29:14PM +0400, 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>
>> ---
>> v18:
>>
>>   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.
>>
>> v17:
>>
>>   Removed unnecessary code comments
>>
>> v16:
>>
>>   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
>>
>> v15:
>>
>>   No change
>>
>> v14:
>>
>>   Picked up the R-b tag
>>
>> 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
>>
>> v12:
>>
>>   Picked up the R-b tag
>>
>> v11:
>>
>>   No change
>>
>> v10:
>>
>>   No change
>>
>> v9:
>>
>>   Add 'ranges' property to example (Rob)
>>
>>   Drop label in example (Rob)
>>
>> v8:
>>
>>   Add size cell to 'reg' (Rob)
>>
>> 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
>>
>> v6:
>>
>>   Device node is child of TCSR; remove phandle (Rob Herring)
>>
>>   Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
>>
>> v5: Use qcom,pwm-regs for phandle instead of direct regs (Bjorn
>>     Andersson, Kathiravan T)
>>
>> v4: Update the binding example node as well (Rob Herring's bot)
>>
>> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
>>
>> v2: Make #pwm-cells const (Rob Herring)
>> ---
>>  drivers/pwm/Kconfig   |  12 +++
>>  drivers/pwm/Makefile  |   1 +
>>  drivers/pwm/pwm-ipq.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 252 insertions(+)
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index bf2d101f67a1e0ae12a58b5630abd5cfd77ccc99..6393f4e91697ae471b1aba72e7ef3f94c5e18383 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 0dc0d2b69025dbd27013285cd335d3cb1ca2ab3f..5630a521a7cffeb83ff8c8960e15eb23ddb1c9f8 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 0000000000000000000000000000000000000000..9955b185bc60f27d770f3833d5acd7f587595324
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-ipq.c
>> @@ -0,0 +1,239 @@
>> +// 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 clock rate */
>> +#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)
> 
> This is used below with:
> 	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
> 
> Where does this limitation come from?

Tbh, I don't know, it was added by qcom, I took over from v16 onwards
and I don't have the technical datasheet / specs. Perhaps Devi or Baruch
can chime in?

I've got v20 ready and will submit as is if that's okay.

> 
>> +/*
>> + * The max value specified for each field is based on the number of bits
>> + * in the pwm control register for that field
>> + */
>> +#define IPQ_PWM_MAX_DIV		0xFFFF
> 
> Maybe use (depending on context) FIELD_MAX(IPQ_PWM_REG0_PWM_DIV) or
> FIELD_MAX(IPQ_PWM_REG1_PRE_DIV).

Changed to FIELD_MAX in next version (16-bit)

> 
>> +
>> +/*
>> + * 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)
>> +
>> +/*
>> + * 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 {
>> +	struct clk *clk;
> 
> you're using clk_rate_exclusive_get(), so it's enough to call
> clk_get_rate() once. Then you can store the rate here instead of the
> clk itself.

Removed clk from ipq_pwm struct and added unsigned long rate field which
is set during probe after enabling the clock.

> 
>> +	void __iomem *mem;
>> +};
>> +
>> +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 void config_div_and_duty(struct pwm_device *pwm, unsigned int pre_div,
> 
> Missing name prefix (i.e. rename to ipq_pwm_config_div_and_duty(), or
> maybe even better: fold into ipq_pwm_apply()).

Addressed and folded into .apply in v20.

> 
>> +				unsigned int pwm_div, unsigned long rate, u64 duty_ns,
>> +				bool enable)
>> +{
>> +	unsigned long hi_dur;
>> +	unsigned long val = 0;
>> +
>> +	/*
>> +	 * high duration = pwm duty * (pwm div + 1)
>> +	 * pwm duty = duty_ns / period_ns
>> +	 */
>> +	hi_dur = mul_u64_u64_div_u64(duty_ns, rate, (pre_div + 1) * NSEC_PER_SEC);
> 
> With pre_div = 0xffff the multiplication in the 3rd parameter overflows.

cast to u64 during calculation -> mul_u64_u64_div_u64(duty_ns, 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 (enable)
>> +		val |= IPQ_PWM_REG1_ENABLE;
>> +	ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
>> +}
>> +
>> +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 long rate = clk_get_rate(ipq_chip->clk);
>> +	unsigned int pre_div, pwm_div;
>> +	u64 period_ns, duty_ns;
>> +
>> +	if (state->polarity != PWM_POLARITY_NORMAL)
>> +		return -EINVAL;
>> +
>> +	if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
>> +		return -ERANGE;
>> +
>> +	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
>> +	duty_ns = min(state->duty_cycle, period_ns);
>> +
>> +	pwm_div = IPQ_PWM_MAX_DIV;
> 
> With pwm_div = 0xffff you cannot configure a 100% relative duty cycle,
> right?

Fixed in next version.

> 
>> +	pre_div = mul_u64_u64_div_u64(period_ns, rate, (u64)NSEC_PER_SEC * (pwm_div + 1));
> 
> according to the get_state() callback below we have:
> 
>             (PRE_DIV + 1) (PWM_DIV + 1)
> 	P = ---------------------------
> 	                RATE
> 
> The (first) +1 isn't accounted for in your formula.

thanks, fixed in v20.
> 	
>> +
>> +	if (pre_div > IPQ_PWM_MAX_DIV)
>> +		pre_div = IPQ_PWM_MAX_DIV;
>> +
>> +	config_div_and_duty(pwm, pre_div, pwm_div, rate, duty_ns, state->enabled);
>> +
>> +	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 long rate = clk_get_rate(ipq_chip->clk);
>> +	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);
>> +
>> +	/* No overflow here, both pre_div and pwm_div <= 0xffff */
>> +	effective_div = (pre_div + 1) * (pwm_div + 1);
> 
> With pre_div = pwm_div = 0xffff this yields 0x100000000 which overflows
> a (32 bit) unsigned int.
> 

fixed in v20.

>> +	state->period = DIV64_U64_ROUND_UP(effective_div * NSEC_PER_SEC, rate);
>> +
>> +	hi_div = hi_dur * (pre_div + 1);
>> +	state->duty_cycle = DIV64_U64_ROUND_UP(hi_div * NSEC_PER_SEC, 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;
>> +	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");
> 
> Please align continuation lines to the opening (.
> 
>> +
>> +	pwm->clk = devm_clk_get_enabled(dev, NULL);
>> +	if (IS_ERR(pwm->clk))
>> +		return dev_err_probe(dev, PTR_ERR(pwm->clk),
>> +				"Failed to get clock\n");
>> +
>> +	ret = devm_clk_rate_exclusive_get(dev, pwm->clk);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret,
>> +				"Failed to lock clock rate\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 ret;
> 
> You could return 0 here which is a tad clearer.

Done.

> 
>> +}
>> +
>> +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");
> 
> Best regards
> Uwe