Skip to contentSkip to conversation

Add RP1 PWM controller support

PATCH Superseded Andrea della Porta 3 participants 13 messages Apr 3 – Apr 10

Subsystems (4)
Device tree bindings Broadcom BCM2711/BCM2835 Arm architecture PWM Raspberry pi RP1 PCI driver

v1 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Andrea della Porta

author maintainer
#0 lore raw

cover Add RP1 PWM controller support

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
Signature · 1 line
2.35.3
author maintainer
#1 lore raw

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

From: Naushir Patuck <[email protected]>

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

Signed-off-by Naushir Patuck <[email protected]>
Co-developed-by Stanimir Varbanov <[email protected]>
Signed-off-by Stanimir Varbanov <[email protected]>
Signed-off-by Andrea della Porta <[email protected]>
---
 .../bindings/pwm/raspberrypi,rp1-pwm.yaml     | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml
Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +52 -0
new file mode 100644
index 0000000000000..d6b3f52561636
@@ -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 <[email protected]>
+
+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>;
+    };
Signature · 1 line
2.35.3
#2 lore raw

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

Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml line 42

Quoted 5 lines · Andrea della Porta
Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +5 -0
+required:
+  - compatible
+  - reg
+  - clocks
+

Missing ref to pwm.yaml.

Quoted 1 line

+additionalProperties: false

and this should be unevaluatedProperties. See other files.

Best regards,
Krzysztof

author maintainer
#3 lore raw

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

Hi Krzysztof,

Quoted 8 lines · Krzysztof Kozlowski

Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml line 42

Quoted 5 lines · Andrea della Porta
Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +5 -0
+required:
+  - compatible
+  - reg
+  - clocks
+

Missing ref to pwm.yaml.

The reference to pwm.yaml is at line 13, as follows:

allOf:
- $ref: pwm.yaml#

currently right after the maintainers: block. Are you suggesting to move it after the required: block?

Quoted 4 lines
Quoted 2 lines

+additionalProperties: false

and this should be unevaluatedProperties. See other files.

Ack.

Many thanks,
Andrea

Quoted 4 lines

Best regards,
Krzysztof

author maintainer
#4 lore raw

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

In reply to Andrea della Porta

From: Naushir Patuck <[email protected]>

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 <[email protected]>
Co-developed-by Stanimir Varbanov <[email protected]>
Signed-off-by Stanimir Varbanov <[email protected]>
Signed-off-by Andrea della Porta <[email protected]>
---
 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
drivers/pwm/Kconfig +10 -0
index 6f3147518376a..22e4fc6385da2 100644
@@ -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
drivers/pwm/Makefile +1 -0
index 0dc0d2b69025d..895a7c42fe9c0 100644
@@ -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
drivers/pwm/pwm-rp1.c new file +244 -0

244 lines not on this page · raw message

Signature · 1 line
2.35.3
maintainer
#5 lore raw

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

Hello Andrea,

Quoted 24 lines · Andrea della Porta

From: Naushir Patuck <[email protected]>

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 <[email protected]>
Co-developed-by Stanimir Varbanov <[email protected]>
Signed-off-by Stanimir Varbanov <[email protected]>
Signed-off-by Andrea della Porta <[email protected]>
---
 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
drivers/pwm/Kconfig +0 -0
index 6f3147518376a..22e4fc6385da2 100644

drivers/pwm/Kconfig config PWM_ROCKCHIP · line 628

Quoted 4 lines
drivers/pwm/Kconfig +1 -0
@@ -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.

Quoted 33 lines
drivers/pwm/Kconfig +9 -0
+	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
drivers/pwm/Makefile +1 -0
index 0dc0d2b69025d..895a7c42fe9c0 100644
@@ -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
drivers/pwm/pwm-rp1.c new file +3 -0
new file mode 100644
index 0000000000000..0a1c1c1dd27e9
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pwm-rp1.c

drivers/pwm/pwm-rp1.c line 13

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+ *
+ * Raspberry Pi RP1 PWM.
+ *
+ * Copyright © 2026 Raspberry Pi Ltd.
+ *
+ * Author: Naushir Patuck ([email protected])
+ *
+ * Based on the pwm-bcm2835 driver by:
+ * Bart Tanghe <[email protected]>
+ */

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

Quoted 14 lines
drivers/pwm/pwm-rp1.c new file +14 -0
+#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))

drivers/pwm/pwm-rp1.c line 38

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+#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_).

Quoted 24 lines
drivers/pwm/pwm-rp1.c new file +24 -0
+
+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)

drivers/pwm/pwm-rp1.c line 72

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+{
+	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.

Quoted 15 lines
drivers/pwm/pwm-rp1.c new file +15 -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;
+

drivers/pwm/pwm-rp1.c line 98

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+	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.

drivers/pwm/pwm-rp1.c line 111

Quoted 13 lines
drivers/pwm/pwm-rp1.c new file +13 -0
+	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?

Quoted 7 lines
drivers/pwm/pwm-rp1.c new file +7 -0
+}
+
+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);

drivers/pwm/pwm-rp1.c line 128

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+	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.)

drivers/pwm/pwm-rp1.c line 130

Quoted 1 line
drivers/pwm/pwm-rp1.c new file +1 -0
+	writel(DIV_ROUND_CLOSEST(state->duty_cycle, clk_period),

Dividing by the result of a division loses precision.

Quoted 20 lines
drivers/pwm/pwm-rp1.c new file +20 -0
+	       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);

drivers/pwm/pwm-rp1.c line 160

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+
+	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().

Quoted 13 lines
drivers/pwm/pwm-rp1.c new file +13 -0
+};
+
+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);

drivers/pwm/pwm-rp1.c line 183

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+
+	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.

drivers/pwm/pwm-rp1.c line 182

Quoted 2 lines
drivers/pwm/pwm-rp1.c new file +2 -0
+
+	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.

Quoted 26 lines
drivers/pwm/pwm-rp1.c new file +26 -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);
+

drivers/pwm/pwm-rp1.c line 221

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -0
+	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.

Quoted 26 lines
drivers/pwm/pwm-rp1.c new file +23 -0
+}
+
+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 <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.35.3

Best regards
Uwe

author maintainer
#6 lore raw

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

Hi Uwe,

Quoted 30 lines · Uwe Kleine-König

Hello Andrea,

Quoted 27 lines · Andrea della Porta

From: Naushir Patuck <[email protected]>

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 <[email protected]>
Co-developed-by Stanimir Varbanov <[email protected]>
Signed-off-by Stanimir Varbanov <[email protected]>
Signed-off-by Andrea della Porta <[email protected]>
---
 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
drivers/pwm/Kconfig +0 -0
index 6f3147518376a..22e4fc6385da2 100644
@@ -625,6 +625,16 @@ config PWM_ROCKCHIP
 	  Generic PWM framework driver for the PWM controller found on
 	  Rockchip SoCs.
Quoted 3 lines

drivers/pwm/Kconfig config PWM_ROCKCHIP · line 628

Quoted 1 line
drivers/pwm/Kconfig +1 -0
+config PWM_RP1

I prefer PWM_RASPBERRYPI1, or PWM_RASPBERRYPI_RP1 here.

Ack.

Quoted 41 lines
Quoted 41 lines
drivers/pwm/Kconfig +9 -0
+	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
drivers/pwm/Makefile +1 -0
index 0dc0d2b69025d..895a7c42fe9c0 100644
@@ -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
drivers/pwm/pwm-rp1.c new file +10 -0
new file mode 100644
index 0000000000000..0a1c1c1dd27e9
@@ -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 ([email protected])
+ *
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 13

Quoted 3 lines
drivers/pwm/pwm-rp1.c new file +3 -0
+ * Based on the pwm-bcm2835 driver by:
+ * Bart Tanghe <[email protected]>
+ */

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

Ack.

Quoted 19 lines
Quoted 19 lines
drivers/pwm/pwm-rp1.c new file +18 -0
+#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))
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 38

Quoted 6 lines
drivers/pwm/pwm-rp1.c new file +6 -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_).

Ack.

Quoted 31 lines
Quoted 31 lines
drivers/pwm/pwm-rp1.c new file +30 -0
+
+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;
+
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 72

Quoted 4 lines
drivers/pwm/pwm-rp1.c new file +4 -0
+	*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.

The pwm-fan generic driver expects an interrupt to count the RPM, while on RP1 this data is passed via a register filled by the RP1 fw running on the internal core. Instead of changing the generic pwm-fan driver, I'll add a syscon to export this register which will be read by a new device/driver registering an hwmon device.

Quoted 20 lines
Quoted 20 lines
drivers/pwm/pwm-rp1.c new file +19 -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);
+}
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 98

Quoted 6 lines
drivers/pwm/pwm-rp1.c new file +6 -0
+
+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.

Ack.

Quoted 16 lines
Quoted 4 lines
drivers/pwm/pwm-rp1.c new file +3 -0
+	return 0;
+}
+

drivers/pwm/pwm-rp1.c line 111

Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10 -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?

To update the configuration on the next PWM strobe in order to avoid glitches. I'll add a short comment in the code.

Quoted 13 lines
Quoted 13 lines
drivers/pwm/pwm-rp1.c new file +12 -0
+}
+
+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");
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 128

Quoted 5 lines
drivers/pwm/pwm-rp1.c new file +5 -0
+		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.)

Ack.

Quoted 29 lines

drivers/pwm/pwm-rp1.c line 130

Quoted 2 lines
drivers/pwm/pwm-rp1.c new file +1 -0
+	writel(DIV_ROUND_CLOSEST(state->duty_cycle, clk_period),

Dividing by the result of a division loses precision.

Quoted 24 lines
drivers/pwm/pwm-rp1.c new file +24 -0
+	       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;
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 160

Quoted 6 lines
drivers/pwm/pwm-rp1.c new file +6 -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().

Ack.

Quoted 18 lines
Quoted 18 lines
drivers/pwm/pwm-rp1.c new file +17 -0
+};
+
+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);
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 183

Quoted 6 lines
drivers/pwm/pwm-rp1.c new file +6 -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.

Ack.

Quoted 6 lines

drivers/pwm/pwm-rp1.c line 182

Quoted 3 lines
drivers/pwm/pwm-rp1.c new file +2 -0
+
+	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.

Ack.

Quoted 32 lines
Quoted 32 lines
drivers/pwm/pwm-rp1.c new file +31 -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;
+}
+
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 221

Quoted 5 lines
drivers/pwm/pwm-rp1.c new file +5 -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.

I'll add some flags to check if the clock is really enabled or not.

Regards,
Andrea

Quoted 30 lines
Quoted 27 lines
drivers/pwm/pwm-rp1.c new file +23 -0
+}
+
+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 <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.35.3

Best regards
Uwe

maintainer
#7 lore raw

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

Hello Andrea,

Quoted 15 lines · Andrea della Porta
Quoted 13 lines · Uwe Kleine-König

drivers/pwm/pwm-rp1.c line 111

Quoted 10 lines · Andrea della Porta
drivers/pwm/pwm-rp1.c new file +10 -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?

Quoted 2 lines

To update the configuration on the next PWM strobe in order to avoid glitches. I'll add a short comment in the code.

.pwm_free() should not touch the hardware configuration. Changing the pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is somewhat a grey area. If that saves energy, that's okish. Otherwise not interfering with the operation of the PWM (e.g. to keep a display on during kexec or so) is preferred.

Quoted 10 lines
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 221

Quoted 5 lines
drivers/pwm/pwm-rp1.c new file +5 -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.

I'll add some flags to check if the clock is really enabled or not.

To be honest, I guess that is a problem of several drivers, not only in drivers/pwm. If this complicates the driver, I guess addressing this isn't very critical.

Best regards
Uwe

author maintainer
#8 lore raw

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

Hi Uwe,

Quoted 18 lines · Uwe Kleine-König

Hello Andrea,

Quoted 15 lines · Andrea della Porta
Quoted 13 lines · Uwe Kleine-König

drivers/pwm/pwm-rp1.c line 111

Quoted 10 lines · Andrea della Porta
drivers/pwm/pwm-rp1.c new file +10 -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?

Quoted 8 lines
Quoted 2 lines

To update the configuration on the next PWM strobe in order to avoid glitches. I'll add a short comment in the code.

.pwm_free() should not touch the hardware configuration. Changing the pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is somewhat a grey area. If that saves energy, that's okish. Otherwise not interfering with the operation of the PWM (e.g. to keep a display on during kexec or so) is preferred.

Sorry I should've been more clear on this. The pinmux/conf is not changed at all by this mask, only the PWM output mode is. The controller can output several type of waveforms and clearing PWM_MODE_MASK is just setting the controller to output a 0, which is the reset default i.e. the same value as just before exporting the channel.
I guess this is the expected behaviour in case of a fan, it should stop spinning in case you unexport the pwm channel, but I see it could be different with displays.
Honestly I don't have a strong opinion about that, please just let me know if I should drop that pwm_free entirely.

Quoted 10 lines
Quoted 10 lines
Quoted 9 lines

drivers/pwm/pwm-rp1.c line 221

Quoted 6 lines
drivers/pwm/pwm-rp1.c new file +5 -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.

Quoted 5 lines
Quoted 1 line

I'll add some flags to check if the clock is really enabled or not.

To be honest, I guess that is a problem of several drivers, not only in drivers/pwm. If this complicates the driver, I guess addressing this isn't very critical.

I'll come up with something, we can always drop this check if deemed too 'noisy'.

Many thanks,
Andrea

Quoted 3 lines

Best regards
Uwe

maintainer
#9 lore raw

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

Hello Andrea,

Quoted 26 lines · Andrea della Porta
Quoted 16 lines · Uwe Kleine-König
Quoted 15 lines · Andrea della Porta
Quoted 13 lines · Uwe Kleine-König

drivers/pwm/pwm-rp1.c line 111

Quoted 10 lines · Andrea della Porta
drivers/pwm/pwm-rp1.c new file +10 -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?

Quoted 8 lines
Quoted 2 lines

To update the configuration on the next PWM strobe in order to avoid glitches. I'll add a short comment in the code.

.pwm_free() should not touch the hardware configuration. Changing the pinmuxing (which I guess is the purpose of clearing PWM_MODE_MASK) is somewhat a grey area. If that saves energy, that's okish. Otherwise not interfering with the operation of the PWM (e.g. to keep a display on during kexec or so) is preferred.

Quoted 10 lines

Sorry I should've been more clear on this. The pinmux/conf is not changed at all by this mask, only the PWM output mode is. The controller can output several type of waveforms and clearing PWM_MODE_MASK is just setting the controller to output a 0, which is the reset default i.e. the same value as just before exporting the channel.
I guess this is the expected behaviour in case of a fan, it should stop spinning in case you unexport the pwm channel, but I see it could be different with displays.
Honestly I don't have a strong opinion about that, please just let me know if I should drop that pwm_free entirely.

Yes, in this case drop the function completely. It's the responsibility of the consumer to stop the PWM before releasing it.

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

drivers/pwm/pwm-rp1.c line 221

Quoted 5 lines
drivers/pwm/pwm-rp1.c new file +5 -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.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

I'll add some flags to check if the clock is really enabled or not.

To be honest, I guess that is a problem of several drivers, not only in drivers/pwm. If this complicates the driver, I guess addressing this isn't very critical.

I'll come up with something, we can always drop this check if deemed too 'noisy'.

Great, thanks
Uwe

author maintainer
#10 lore raw

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

In reply to Andrea della Porta

From: Stanimir Varbanov <[email protected]>

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 <[email protected]>
Co-developed-by Andrea della Porta <[email protected]>
Signed-off-by Andrea della Porta <[email protected]>
---
 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(+)
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +12 -0
index 2856082814462..a4e5ba23bf536 100644
@@ -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";
arch/arm64/boot/dts/broadcom/rp1-common.dtsi +10 -0
index 5a815c3797945..7e78501e62b0c 100644
@@ -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>;
Signature · 1 line
2.35.3
#11 lore raw

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

arch/arm64/boot/dts/broadcom/rp1-common.dtsi rp1_eth: ethernet@40100000 { · line 59

Quoted 13 lines · Andrea della Porta
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +1 -0
+
 &rp1_usb0 {
 	pinctrl-0 = <&usb_vbus_default_state>;
 	pinctrl-names = "default";
arch/arm64/boot/dts/broadcom/rp1-common.dtsi +1 -0
index 5a815c3797945..7e78501e62b0c 100644
@@ -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

maintainer
#12 lore raw

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

In reply to Andrea della Porta

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