[PATCH 0/3] Add RP1 PWM controller support

[PATCH 0/3] Add RP1 PWM controller support

[Andrea della Porta]

This patchset adds support for the PWM controller found on the
Raspberry Pi RP1 southbridge. This is necessary to operate the
cooling fan connected to one of the PWM channels.

The tachometer pin for the fan speed is managed by the firmware
running on the RP1's M-core. It uses the PHASE2 register
to report the RPM, which is then exported by this driver as a
hwmon device.

Subsequent patches will add the CPU thermal zone, which acts as
a consumer of the PWM device.

Best regards,
Andrea

Naushir Patuck (2):
  dt-bindings: pwm: Add Raspberry Pi RP1 PWM controller
  pwm: rp1: Add RP1 PWM controller driver

Stanimir Varbanov (1):
  arm64: dts: broadcom: rp1: Add PWM node

 .../bindings/pwm/raspberrypi,rp1-pwm.yaml     |  52 ++++
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |  12 +
 arch/arm64/boot/dts/broadcom/rp1-common.dtsi  |  10 +
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-rp1.c                         | 244 ++++++++++++++++++
 6 files changed, 329 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml
 create mode 100644 drivers/pwm/pwm-rp1.c

-- 
2.35.3

[PATCH 1/3] dt-bindings: pwm: Add Raspberry Pi RP1 PWM controller

[Andrea della Porta]

From: Naushir Patuck <naush@raspberrypi.com>

Add the devicetree binding documentation for the PWM
controller found in the Raspberry Pi RP1 chipset.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Co-developed-by: Stanimir Varbanov <svarbanov@suse.de>
Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 .../bindings/pwm/raspberrypi,rp1-pwm.yaml     | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml b/Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml
new file mode 100644
index 0000000000000..d6b3f52561636
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/raspberrypi,rp1-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi RP1 PWM controller
+
+maintainers:
+  - Naushir Patuck <naush@raspberrypi.com>
+
+allOf:
+  - $ref: pwm.yaml#
+
+description: |
+  The PWM peripheral is a flexible waveform generator with a
+  variety of operational modes. It has the following features:
+   - four independent output channels
+   - 32-bit counter widths
+   - Seven output generation modes
+   - Optional per-channel output inversion
+   - Optional duty-cycle data FIFO with DMA support
+   - Optional sigma-delta noise shaping engine
+
+properties:
+  compatible:
+    const: raspberrypi,rp1-pwm
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm@98000 {
+      compatible = "raspberrypi,rp1-pwm";
+      reg = <0x98000 0x100>;
+      clocks = <&rp1_clocks 17>;
+      #pwm-cells = <3>;
+    };
-- 
2.35.3

Re: [PATCH 1/3] dt-bindings: pwm: Add Raspberry Pi RP1 PWM controller

[Krzysztof Kozlowski]

On Fri, Apr 03, 2026 at 04:31:54PM +0200, Andrea della Porta wrote:
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +

Missing ref to pwm.yaml.

> +additionalProperties: false

and this should be unevaluatedProperties. See other files.

Best regards,
Krzysztof

[PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver

[Andrea della Porta]

From: Naushir Patuck <naush@raspberrypi.com>

The Raspberry Pi RP1 southbridge features an embedded PWM
controller with 4 output channels, alongside an RPM interface
to read the fan speed on the Raspberry Pi 5.

Add the supporting driver.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Co-developed-by: Stanimir Varbanov <svarbanov@suse.de>
Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 drivers/pwm/Kconfig   |  10 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-rp1.c | 244 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)
 create mode 100644 drivers/pwm/pwm-rp1.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 6f3147518376a..22e4fc6385da2 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -625,6 +625,16 @@ config PWM_ROCKCHIP
 	  Generic PWM framework driver for the PWM controller found on
 	  Rockchip SoCs.
 
+config PWM_RP1
+	tristate "RP1 PWM support"
+	depends on MISC_RP1 || COMPILE_TEST
+	depends on HWMON
+	help
+	  PWM framework driver for Raspberry Pi RP1 controller
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-rp1.
+
 config PWM_SAMSUNG
 	tristate "Samsung PWM support"
 	depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0dc0d2b69025d..895a7c42fe9c0 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_PWM_RENESAS_RZG2L_GPT)	+= pwm-rzg2l-gpt.o
 obj-$(CONFIG_PWM_RENESAS_RZ_MTU3)	+= pwm-rz-mtu3.o
 obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
 obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
+obj-$(CONFIG_PWM_RP1)		+= pwm-rp1.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
 obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
 obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
diff --git a/drivers/pwm/pwm-rp1.c b/drivers/pwm/pwm-rp1.c
new file mode 100644
index 0000000000000..0a1c1c1dd27e9
--- /dev/null
+++ b/drivers/pwm/pwm-rp1.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pwm-rp1.c
+ *
+ * Raspberry Pi RP1 PWM.
+ *
+ * Copyright © 2026 Raspberry Pi Ltd.
+ *
+ * Author: Naushir Patuck (naush@raspberrypi.com)
+ *
+ * Based on the pwm-bcm2835 driver by:
+ * Bart Tanghe <bart.tanghe@thomasmore.be>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#define PWM_GLOBAL_CTRL		0x000
+#define PWM_CHANNEL_CTRL(x)	(0x014 + ((x) * 0x10))
+#define PWM_RANGE(x)		(0x018 + ((x) * 0x10))
+#define PWM_PHASE(x)		(0x01C + ((x) * 0x10))
+#define PWM_DUTY(x)		(0x020 + ((x) * 0x10))
+
+/* 8:FIFO_POP_MASK + 0:Trailing edge M/S modulation */
+#define PWM_CHANNEL_DEFAULT	(BIT(8) + BIT(0))
+#define PWM_CHANNEL_ENABLE(x)	BIT(x)
+#define PWM_POLARITY		BIT(3)
+#define SET_UPDATE		BIT(31)
+#define PWM_MODE_MASK		GENMASK(1, 0)
+
+#define NUM_PWMS		4
+
+struct rp1_pwm {
+	void __iomem	*base;
+	struct clk	*clk;
+};
+
+static const struct hwmon_channel_info * const rp1_fan_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+	NULL
+};
+
+static umode_t rp1_fan_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	umode_t mode = 0;
+
+	if (type == hwmon_fan && attr == hwmon_fan_input)
+		mode = 0444;
+
+	return mode;
+}
+
+static int rp1_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
+
+	if (type != hwmon_fan || attr != hwmon_fan_input)
+		return -EOPNOTSUPP;
+
+	*val = readl(rp1->base + PWM_PHASE(2));
+
+	return 0;
+}
+
+static const struct hwmon_ops rp1_fan_hwmon_ops = {
+	.is_visible = rp1_fan_hwmon_is_visible,
+	.read = rp1_fan_hwmon_read,
+};
+
+static const struct hwmon_chip_info rp1_fan_hwmon_chip_info = {
+	.ops = &rp1_fan_hwmon_ops,
+	.info = rp1_fan_hwmon_info,
+};
+
+static void rp1_pwm_apply_config(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
+	u32 value;
+
+	value = readl(rp1->base + PWM_GLOBAL_CTRL);
+	value |= SET_UPDATE;
+	writel(value, rp1->base + PWM_GLOBAL_CTRL);
+}
+
+static int rp1_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
+
+	writel(PWM_CHANNEL_DEFAULT, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
+	return 0;
+}
+
+static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
+	u32 value;
+
+	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
+	value &= ~PWM_MODE_MASK;
+	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
+
+	rp1_pwm_apply_config(chip, pwm);
+}
+
+static int rp1_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			 const struct pwm_state *state)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
+	unsigned long clk_rate = clk_get_rate(rp1->clk);
+	unsigned long clk_period;
+	u32 value;
+
+	if (!clk_rate) {
+		dev_err(&chip->dev, "failed to get clock rate\n");
+		return -EINVAL;
+	}
+
+	/* set period and duty cycle */
+	clk_period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, clk_rate);
+
+	writel(DIV_ROUND_CLOSEST(state->duty_cycle, clk_period),
+	       rp1->base + PWM_DUTY(pwm->hwpwm));
+
+	writel(DIV_ROUND_CLOSEST(state->period, clk_period),
+	       rp1->base + PWM_RANGE(pwm->hwpwm));
+
+	/* set polarity */
+	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
+	if (state->polarity == PWM_POLARITY_NORMAL)
+		value &= ~PWM_POLARITY;
+	else
+		value |= PWM_POLARITY;
+	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
+
+	/* enable/disable */
+	value = readl(rp1->base + PWM_GLOBAL_CTRL);
+	if (state->enabled)
+		value |= PWM_CHANNEL_ENABLE(pwm->hwpwm);
+	else
+		value &= ~PWM_CHANNEL_ENABLE(pwm->hwpwm);
+	writel(value, rp1->base + PWM_GLOBAL_CTRL);
+
+	rp1_pwm_apply_config(chip, pwm);
+
+	return 0;
+}
+
+static const struct pwm_ops rp1_pwm_ops = {
+	.request = rp1_pwm_request,
+	.free = rp1_pwm_free,
+	.apply = rp1_pwm_apply,
+};
+
+static int rp1_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device *hwmon_dev;
+	struct pwm_chip *chip;
+	struct rp1_pwm *rp1;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, NUM_PWMS, sizeof(*rp1));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	rp1 = pwmchip_get_drvdata(chip);
+
+	rp1->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rp1->base))
+		return PTR_ERR(rp1->base);
+
+	rp1->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(rp1->clk))
+		return dev_err_probe(dev, PTR_ERR(rp1->clk), "clock not found\n");
+
+	ret = devm_clk_rate_exclusive_get(dev, rp1->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "fail to get exclusive rate\n");
+
+	chip->ops = &rp1_pwm_ops;
+
+	platform_set_drvdata(pdev, chip);
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register PWM chip\n");
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "rp1_fan_tach", rp1,
+							 &rp1_fan_hwmon_chip_info,
+							 NULL);
+
+	if (IS_ERR(hwmon_dev))
+		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
+				     "failed to register hwmon fan device\n");
+
+	return 0;
+}
+
+static int rp1_pwm_suspend(struct device *dev)
+{
+	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(rp1->clk);
+
+	return 0;
+}
+
+static int rp1_pwm_resume(struct device *dev)
+{
+	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(rp1->clk);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(rp1_pwm_pm_ops, rp1_pwm_suspend, rp1_pwm_resume);
+
+static const struct of_device_id rp1_pwm_of_match[] = {
+	{ .compatible = "raspberrypi,rp1-pwm" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rp1_pwm_of_match);
+
+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	.driver = {
+		.name = "rp1-pwm",
+		.of_match_table = rp1_pwm_of_match,
+		.pm = pm_ptr(&rp1_pwm_pm_ops),
+	},
+};
+module_platform_driver(rp1_pwm_driver);
+
+MODULE_DESCRIPTION("RP1 PWM driver");
+MODULE_AUTHOR("Naushir Patuck <naush@raspberrypi.com>");
+MODULE_LICENSE("GPL");
-- 
2.35.3

Re: [PATCH 2/3] pwm: rp1: Add RP1 PWM controller driver

[Uwe Kleine-König]

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

Hello Andrea,

On Fri, Apr 03, 2026 at 04:31:55PM +0200, Andrea della Porta wrote:
> From: Naushir Patuck <naush@raspberrypi.com>
> 
> The Raspberry Pi RP1 southbridge features an embedded PWM
> controller with 4 output channels, alongside an RPM interface
> to read the fan speed on the Raspberry Pi 5.
> 
> Add the supporting driver.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Co-developed-by: Stanimir Varbanov <svarbanov@suse.de>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> ---
>  drivers/pwm/Kconfig   |  10 ++
>  drivers/pwm/Makefile  |   1 +
>  drivers/pwm/pwm-rp1.c | 244 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 255 insertions(+)
>  create mode 100644 drivers/pwm/pwm-rp1.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 6f3147518376a..22e4fc6385da2 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -625,6 +625,16 @@ config PWM_ROCKCHIP
>  	  Generic PWM framework driver for the PWM controller found on
>  	  Rockchip SoCs.
>  
> +config PWM_RP1

I prefer PWM_RASPBERRYPI1, or PWM_RASPBERRYPI_RP1 here.

> +	tristate "RP1 PWM support"
> +	depends on MISC_RP1 || COMPILE_TEST
> +	depends on HWMON
> +	help
> +	  PWM framework driver for Raspberry Pi RP1 controller
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-rp1.
> +
>  config PWM_SAMSUNG
>  	tristate "Samsung PWM support"
>  	depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 0dc0d2b69025d..895a7c42fe9c0 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_PWM_RENESAS_RZG2L_GPT)	+= pwm-rzg2l-gpt.o
>  obj-$(CONFIG_PWM_RENESAS_RZ_MTU3)	+= pwm-rz-mtu3.o
>  obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
>  obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
> +obj-$(CONFIG_PWM_RP1)		+= pwm-rp1.o
>  obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
>  obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
>  obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
> diff --git a/drivers/pwm/pwm-rp1.c b/drivers/pwm/pwm-rp1.c
> new file mode 100644
> index 0000000000000..0a1c1c1dd27e9
> --- /dev/null
> +++ b/drivers/pwm/pwm-rp1.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pwm-rp1.c
> + *
> + * Raspberry Pi RP1 PWM.
> + *
> + * Copyright © 2026 Raspberry Pi Ltd.
> + *
> + * Author: Naushir Patuck (naush@raspberrypi.com)
> + *
> + * Based on the pwm-bcm2835 driver by:
> + * Bart Tanghe <bart.tanghe@thomasmore.be>
> + */

Please add a paragraph here named "Limitations" in the same format as
several other drivers describing how the driver behaves on disable and
configuration changes (can glitches occur? Is the currently running
period completed or aborted?)

> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +#define PWM_GLOBAL_CTRL		0x000
> +#define PWM_CHANNEL_CTRL(x)	(0x014 + ((x) * 0x10))
> +#define PWM_RANGE(x)		(0x018 + ((x) * 0x10))
> +#define PWM_PHASE(x)		(0x01C + ((x) * 0x10))
> +#define PWM_DUTY(x)		(0x020 + ((x) * 0x10))
> +
> +/* 8:FIFO_POP_MASK + 0:Trailing edge M/S modulation */
> +#define PWM_CHANNEL_DEFAULT	(BIT(8) + BIT(0))
> +#define PWM_CHANNEL_ENABLE(x)	BIT(x)
> +#define PWM_POLARITY		BIT(3)
> +#define SET_UPDATE		BIT(31)
> +#define PWM_MODE_MASK		GENMASK(1, 0)
> +
> +#define NUM_PWMS		4

Please prefix all #defines by something driver specific (e.g. RP1_PWM_).

> +
> +struct rp1_pwm {
> +	void __iomem	*base;
> +	struct clk	*clk;
> +};
> +
> +static const struct hwmon_channel_info * const rp1_fan_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> +	NULL
> +};
> +
> +static umode_t rp1_fan_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
> +					u32 attr, int channel)
> +{
> +	umode_t mode = 0;
> +
> +	if (type == hwmon_fan && attr == hwmon_fan_input)
> +		mode = 0444;
> +
> +	return mode;
> +}
> +
> +static int rp1_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			      u32 attr, int channel, long *val)
> +{
> +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> +
> +	if (type != hwmon_fan || attr != hwmon_fan_input)
> +		return -EOPNOTSUPP;
> +
> +	*val = readl(rp1->base + PWM_PHASE(2));
> +
> +	return 0;
> +}

I don't like having hwmon bits in pwm drivers. Is the PWM only usable
for a fan? I guess the hwmon parts should be dropped and a pwm-fan
defined in dt.

> +static const struct hwmon_ops rp1_fan_hwmon_ops = {
> +	.is_visible = rp1_fan_hwmon_is_visible,
> +	.read = rp1_fan_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info rp1_fan_hwmon_chip_info = {
> +	.ops = &rp1_fan_hwmon_ops,
> +	.info = rp1_fan_hwmon_info,
> +};
> +
> +static void rp1_pwm_apply_config(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> +	u32 value;
> +
> +	value = readl(rp1->base + PWM_GLOBAL_CTRL);
> +	value |= SET_UPDATE;
> +	writel(value, rp1->base + PWM_GLOBAL_CTRL);
> +}
> +
> +static int rp1_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> +
> +	writel(PWM_CHANNEL_DEFAULT, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));

Please add a comment about what this does.

> +	return 0;
> +}
> +
> +static void rp1_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> +	u32 value;
> +
> +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> +	value &= ~PWM_MODE_MASK;
> +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> +
> +	rp1_pwm_apply_config(chip, pwm);

What is the purpose of this call?

> +}
> +
> +static int rp1_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			 const struct pwm_state *state)
> +{
> +	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> +	unsigned long clk_rate = clk_get_rate(rp1->clk);
> +	unsigned long clk_period;
> +	u32 value;
> +
> +	if (!clk_rate) {
> +		dev_err(&chip->dev, "failed to get clock rate\n");
> +		return -EINVAL;
> +	}
> +
> +	/* set period and duty cycle */
> +	clk_period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, clk_rate);

DIV_ROUND_CLOSEST is wrong here. (I don't go into details as .apply()
should be dropped.)

> +	writel(DIV_ROUND_CLOSEST(state->duty_cycle, clk_period),

Dividing by the result of a division loses precision.

> +	       rp1->base + PWM_DUTY(pwm->hwpwm));
> +
> +	writel(DIV_ROUND_CLOSEST(state->period, clk_period),
> +	       rp1->base + PWM_RANGE(pwm->hwpwm));
> +
> +	/* set polarity */
> +	value = readl(rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> +	if (state->polarity == PWM_POLARITY_NORMAL)
> +		value &= ~PWM_POLARITY;
> +	else
> +		value |= PWM_POLARITY;
> +	writel(value, rp1->base + PWM_CHANNEL_CTRL(pwm->hwpwm));
> +
> +	/* enable/disable */
> +	value = readl(rp1->base + PWM_GLOBAL_CTRL);
> +	if (state->enabled)
> +		value |= PWM_CHANNEL_ENABLE(pwm->hwpwm);
> +	else
> +		value &= ~PWM_CHANNEL_ENABLE(pwm->hwpwm);
> +	writel(value, rp1->base + PWM_GLOBAL_CTRL);
> +
> +	rp1_pwm_apply_config(chip, pwm);
> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops rp1_pwm_ops = {
> +	.request = rp1_pwm_request,
> +	.free = rp1_pwm_free,
> +	.apply = rp1_pwm_apply,

Please implement the waveform callbacks instead of .apply().

> +};
> +
> +static int rp1_pwm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device *hwmon_dev;
> +	struct pwm_chip *chip;
> +	struct rp1_pwm *rp1;
> +	int ret;
> +
> +	chip = devm_pwmchip_alloc(dev, NUM_PWMS, sizeof(*rp1));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +
> +	rp1 = pwmchip_get_drvdata(chip);
> +
> +	rp1->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rp1->base))
> +		return PTR_ERR(rp1->base);
> +
> +	rp1->clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(rp1->clk))
> +		return dev_err_probe(dev, PTR_ERR(rp1->clk), "clock not found\n");

Please start error messages with a capital letter.

> +
> +	ret = devm_clk_rate_exclusive_get(dev, rp1->clk);

After this call you can determine the rate just once and fail if it's ==
0.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "fail to get exclusive rate\n");
> +
> +	chip->ops = &rp1_pwm_ops;
> +
> +	platform_set_drvdata(pdev, chip);
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register PWM chip\n");
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "rp1_fan_tach", rp1,
> +							 &rp1_fan_hwmon_chip_info,
> +							 NULL);
> +
> +	if (IS_ERR(hwmon_dev))
> +		return dev_err_probe(dev, PTR_ERR(hwmon_dev),
> +				     "failed to register hwmon fan device\n");
> +
> +	return 0;
> +}
> +
> +static int rp1_pwm_suspend(struct device *dev)
> +{
> +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(rp1->clk);
> +
> +	return 0;
> +}
> +
> +static int rp1_pwm_resume(struct device *dev)
> +{
> +	struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> +
> +	return clk_prepare_enable(rp1->clk);

Hmm, if this fails and then the driver is unbound, the clk operations
are not balanced.

> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(rp1_pwm_pm_ops, rp1_pwm_suspend, rp1_pwm_resume);
> +
> +static const struct of_device_id rp1_pwm_of_match[] = {
> +	{ .compatible = "raspberrypi,rp1-pwm" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rp1_pwm_of_match);
> +
> +static struct platform_driver rp1_pwm_driver = {
> +	.probe = rp1_pwm_probe,
> +	.driver = {
> +		.name = "rp1-pwm",
> +		.of_match_table = rp1_pwm_of_match,
> +		.pm = pm_ptr(&rp1_pwm_pm_ops),
> +	},
> +};
> +module_platform_driver(rp1_pwm_driver);
> +
> +MODULE_DESCRIPTION("RP1 PWM driver");
> +MODULE_AUTHOR("Naushir Patuck <naush@raspberrypi.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.35.3
> 

Best regards
Uwe

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

[PATCH 3/3] arm64: dts: broadcom: rp1: Add PWM node

[Andrea della Porta]

From: Stanimir Varbanov <svarbanov@suse.de>

The RP1 chipset used on the Raspberry Pi 5 features an integrated
PWM controller to drive the cooling fan.

Add the corresponding DT node for this PWM controller.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Co-developed-by: Andrea della Porta <andrea.porta@suse.com>
Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts | 12 ++++++++++++
 arch/arm64/boot/dts/broadcom/rp1-common.dtsi     | 10 ++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
index 2856082814462..a4e5ba23bf536 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
@@ -64,12 +64,24 @@ phy1: ethernet-phy@1 {
 };
 
 &rp1_gpio {
+	fan_pwm_default_state: fan-pwm-default-state {
+		function = "pwm1";
+		pins = "gpio45";
+		bias-pull-down;
+	};
+
 	usb_vbus_default_state: usb-vbus-default-state {
 		function = "vbus1";
 		groups = "vbus1";
 	};
 };
 
+&rp1_pwm {
+	pinctrl-0 = <&fan_pwm_default_state>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
 &rp1_usb0 {
 	pinctrl-0 = <&usb_vbus_default_state>;
 	pinctrl-names = "default";
diff --git a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
index 5a815c3797945..7e78501e62b0c 100644
--- a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
+++ b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
@@ -56,6 +56,16 @@ rp1_eth: ethernet@40100000 {
 		#size-cells = <0>;
 	};
 
+	rp1_pwm: pwm@4009c000 {
+		compatible = "raspberrypi,rp1-pwm";
+		reg = <0x00 0x4009c000  0x0 0x100>;
+		clocks = <&rp1_clocks RP1_CLK_PWM1>;
+		assigned-clocks = <&rp1_clocks RP1_CLK_PWM1>;
+		assigned-clock-rates = <50000000>;
+		#pwm-cells = <3>;
+		status = "disabled";
+	};
+
 	rp1_usb0: usb@40200000 {
 		compatible = "snps,dwc3";
 		reg = <0x00 0x40200000  0x0 0x100000>;
-- 
2.35.3

Re: [PATCH 3/3] arm64: dts: broadcom: rp1: Add PWM node

[Krzysztof Kozlowski]

On Fri, Apr 03, 2026 at 04:31:56PM +0200, Andrea della Porta wrote:
> +
>  &rp1_usb0 {
>  	pinctrl-0 = <&usb_vbus_default_state>;
>  	pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
> index 5a815c3797945..7e78501e62b0c 100644
> --- a/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/rp1-common.dtsi
> @@ -56,6 +56,16 @@ rp1_eth: ethernet@40100000 {
>  		#size-cells = <0>;
>  	};
>  
> +	rp1_pwm: pwm@4009c000 {

Don't break the order. Instead please read and follow DTS coding style.

Best regards,
Krzysztof

Re: [PATCH 3/3] arm64: dts: broadcom: rp1: Add PWM node

[Uwe Kleine-König]

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

Hello,

The "rp1" in the Subject is very irritating. Better make this:

	arm64: dts: broadcom: rpi-5: Add rp1 PWM node

or similar.
Best regards
Uwe

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