Add RP1 PWM controller support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding description states the device acts as a syscon provider for shared registers, but the schema prohibits the use of the "syscon" compatible string. --

--- Patch [1]: [PATCH v7 1/3] dt-bindings: pwm: Add Raspberry Pi RP1 PWM controller --- commit 06ebdbe78362ab02ea5e88d503ad7c179855b48a
Author: Naushir Patuck <[email protected]>

dt-bindings: pwm: Add Raspberry Pi RP1 PWM controller

This commit adds the devicetree binding documentation for the PWM controller found in the Raspberry Pi RP1 chipset.

Quoted 5 lines

Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +0 -0

new file mode 100644
@@ -0,0 +1,66 @@

[ ... ]

Quoted 5 lines

Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +5 -0

+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

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

Quoted 10 lines

Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +10 -0

+   - Seven output generation modes
+   - Optional per-channel output inversion
+   - Optional duty-cycle data FIFO with DMA support
+   - Optional sigma-delta noise shaping engine
+  Serves as a fan speed provider to other nodes for a PWM-connected
+  fan using shared registers (syscon).
+
+properties:
+  compatible:
+    const: raspberrypi,rp1-pwm

[Severity: Medium]
The description states this device acts as a syscon provider for shared registers, but the compatible property is restricted to a single string.

Will device tree validation fail if a user includes the "syscon" string normally required for standard kernel syscon lookup APIs alongside the "raspberrypi,rp1-pwm" compatible string?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing .remove callback leads to Use-After-Free in the PWM and Syscon cores and leaks clock resources on driver unbind.
- [High] Register context loss on suspend/resume silently breaks PWM output.
- [High] Unclamped hardware duty ticks lead to integer underflow in the PWM core for normal polarity.
--

--- Patch [2]: [PATCH v7 2/3] pwm: rp1: Add RP1 PWM controller driver --- commit 0c98703d857577902e57d25b42a3d46dfbde5795
Author: Naushir Patuck <[email protected]>

pwm: rp1: Add RP1 PWM controller driver

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.

Quoted 4 lines

drivers/pwm/pwm-rp1.c new file +0 -0

new file mode 100644

[ ... ]

Quoted 12 lines

drivers/pwm/pwm-rp1.c new file +12 -0

+static int rp1_pwm_round_waveform_fromhw(struct pwm_chip *chip,
+					 struct pwm_device *pwm,
+					 const void *_wfhw,
+					 struct pwm_waveform *wf)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
+	const struct rp1_pwm_waveform *wfhw = _wfhw;
+	u64 clk_rate = rp1->clk_rate;
+	u64 ticks;
+
+	*wf = (struct pwm_waveform){ };
+

drivers/pwm/pwm-rp1.c line 198

Quoted 10 lines

drivers/pwm/pwm-rp1.c new file +10 -0

+	if (!wfhw->enabled)
+		return 0;
+
+	wf->period_length_ns = DIV_ROUND_UP_ULL(((u64)wfhw->period_ticks + 1) * NSEC_PER_SEC,
+						clk_rate);
+
+	if (!wfhw->inverted_polarity) {
+		wf->duty_length_ns = DIV_ROUND_UP_ULL((u64)wfhw->duty_ticks * NSEC_PER_SEC,
+						      (u32)clk_rate);
+	} else {

[Severity: High]
Does the normal polarity case need to clamp the duty ticks similar to the inverted polarity case?

If the bootloader configures a continuous-high state where the raw hardware duty ticks exceed the period ticks + 1, this path calculates a duty_length_ns that is strictly greater than the period_length_ns.

When the PWM core subsequently reads this initial state, pwm_wf2state() handles duty > period by treating it as an inverted polarity and calculating duty_cycle = period - duty_length_ns. This subtraction can silently underflow, causing the core to interpret the hardware state as having a badly corrupted duty cycle.

Quoted 5 lines

drivers/pwm/pwm-rp1.c new file +4 -0

+		if (wfhw->duty_ticks > (u64)wfhw->period_ticks + 1) {
+			/* 100% duty cycle case */
+			ticks = 0;
+		} else {

[ ... ]

Quoted 5 lines

drivers/pwm/pwm-rp1.c new file +5 -0

+static int rp1_pwm_write_waveform(struct pwm_chip *chip,
+				  struct pwm_device *pwm,
+				  const void *_wfhw)
+{
+	struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);

drivers/pwm/pwm-rp1.c line 227

Quoted 10 lines

drivers/pwm/pwm-rp1.c new file +10 -0

+	const struct rp1_pwm_waveform *wfhw = _wfhw;
+	u32 value, ctrl;
+
+	/* set polarity */
+	regmap_read(rp1->regmap, RP1_PWM_CHAN_CTRL(pwm->hwpwm), &value);
+	if (!wfhw->inverted_polarity)
+		value &= ~RP1_PWM_CHAN_CTRL_POLARITY;
+	else
+		value |= RP1_PWM_CHAN_CTRL_POLARITY;
+	regmap_write(rp1->regmap, RP1_PWM_CHAN_CTRL(pwm->hwpwm), value);

[Severity: High]
Could the PWM controller lose its hardware configuration after a system suspend and resume?

During rp1_pwm_request(), the channel control register is initialized with essential bits like RP1_PWM_CHAN_CTRL_DEFAULT, which includes the MODE_TE_MS and FIFO_POP_MASK configurations.

If the hardware loses power during suspend, the registers reset. On resume, when a consumer re-applies its state, this regmap_read() will read the wiped hardware register. The driver will then update only the polarity bit and write it back, failing to restore the required operational mode bits that were initialized during the request phase.

Quoted 3 lines

drivers/pwm/pwm-rp1.c new file +2 -0

+
+	/* early exit if disabled */

[ ... ]

drivers/pwm/pwm-rp1.c line 417

Quoted 8 lines

drivers/pwm/pwm-rp1.c new file +8 -0

+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	/*
+	 * .remove is not needed because this driver cannot be removed/unbound
+	 * cause it relies on syscon which cannot be removed
+	 */
+	.driver = {
+		.name = "rp1-pwm",

[Severity: High]
Is it safe to omit the remove callback here?

Even with suppress_bind_attrs set on the driver, if the parent RP1 PCIe device is removed, reset, or unbound via sysfs, it will forcibly unbind all child devices.

Without a remove callback to run cleanup, pwmchip_remove() is never called and the clock is left enabled. Because the backing memory is devm-managed, the devres subsystem will eventually free the pwm_chip and regmap memory. This can leave dangling pointers in the core pwm_chips IDR and the global syscon_list, which could result in a use-after-free.

Quoted 6 lines

drivers/pwm/pwm-rp1.c new file +5 -0

+		.of_match_table = rp1_pwm_of_match,
+		.pm = pm_ptr(&rp1_pwm_pm_ops),
+		.suppress_bind_attrs = true,
+	},
+};

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Hi,

Quoted 32 lines · On 10:02 Mon 20 Jul , [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing .remove callback leads to Use-After-Free in the PWM and Syscon cores and leaks clock resources on driver unbind.
- [High] Register context loss on suspend/resume silently breaks PWM output.
- [High] Unclamped hardware duty ticks lead to integer underflow in the PWM core for normal polarity.
--

--- Patch [2]: [PATCH v7 2/3] pwm: rp1: Add RP1 PWM controller driver --- commit 0c98703d857577902e57d25b42a3d46dfbde5795
Author: Naushir Patuck <[email protected]>

pwm: rp1: Add RP1 PWM controller driver

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.

drivers/pwm/pwm-rp1.c line 417

Quoted 8 lines

drivers/pwm/pwm-rp1.c new file +8 -0

+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	/*
+	 * .remove is not needed because this driver cannot be removed/unbound
+	 * cause it relies on syscon which cannot be removed
+	 */
+	.driver = {
+		.name = "rp1-pwm",

[Severity: High]
Is it safe to omit the remove callback here?

Even with suppress_bind_attrs set on the driver, if the parent RP1 PCIe device is removed, reset, or unbound via sysfs, it will forcibly unbind all child devices.

Quoted 5 lines

Without a remove callback to run cleanup, pwmchip_remove() is never called and the clock is left enabled. Because the backing memory is devm-managed, the devres subsystem will eventually free the pwm_chip and regmap memory. This can leave dangling pointers in the core pwm_chips IDR and the global syscon_list, which could result in a use-after-free.

This is true. Uwe, I guess I should reintroduce the .remove callback, what do you think?

Quoted 41 lines · Andrea della Porta

Hi,

Quoted 37 lines · On 10:02 Mon 20 Jul , [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing .remove callback leads to Use-After-Free in the PWM and Syscon cores and leaks clock resources on driver unbind.
- [High] Register context loss on suspend/resume silently breaks PWM output.
- [High] Unclamped hardware duty ticks lead to integer underflow in the PWM core for normal polarity.
--

--- Patch [2]: [PATCH v7 2/3] pwm: rp1: Add RP1 PWM controller driver --- commit 0c98703d857577902e57d25b42a3d46dfbde5795
Author: Naushir Patuck <[email protected]>

pwm: rp1: Add RP1 PWM controller driver

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.

drivers/pwm/pwm-rp1.c line 417

Quoted 8 lines

drivers/pwm/pwm-rp1.c new file +8 -0

+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	/*
+	 * .remove is not needed because this driver cannot be removed/unbound
+	 * cause it relies on syscon which cannot be removed
+	 */
+	.driver = {
+		.name = "rp1-pwm",

[Severity: High]
Is it safe to omit the remove callback here?

Even with suppress_bind_attrs set on the driver, if the parent RP1 PCIe device is removed, reset, or unbound via sysfs, it will forcibly unbind all child devices.

Without a remove callback to run cleanup, pwmchip_remove() is never called and the clock is left enabled. Because the backing memory is devm-managed, the devres subsystem will eventually free the pwm_chip and regmap memory. This can leave dangling pointers in the core pwm_chips IDR and the global syscon_list, which could result in a use-after-free.

Quoted 3 lines

This is true. Uwe, I guess I should reintroduce the .remove callback, what do you think?

How is remove going to work with syscon? The previous version seems to just ignore it completely.

Also, do you actually need syscon here? Given that the pwm fan device is going to depend on the PWM fan device anyway, you could just EXPORT_SYMBOL_NS an API from the rp1 driver that returns the tachometer reading when supplied with a rp1 pwm device pointer?

Best,
Gary

Hi Gary,

Quoted 42 lines · Gary Guo

Quoted 41 lines · Andrea della Porta

Hi,

Quoted 37 lines · On 10:02 Mon 20 Jul , [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing .remove callback leads to Use-After-Free in the PWM and Syscon cores and leaks clock resources on driver unbind.
- [High] Register context loss on suspend/resume silently breaks PWM output.
- [High] Unclamped hardware duty ticks lead to integer underflow in the PWM core for normal polarity.
--

--- Patch [2]: [PATCH v7 2/3] pwm: rp1: Add RP1 PWM controller driver --- commit 0c98703d857577902e57d25b42a3d46dfbde5795
Author: Naushir Patuck <[email protected]>

pwm: rp1: Add RP1 PWM controller driver

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.

drivers/pwm/pwm-rp1.c line 417

Quoted 8 lines

drivers/pwm/pwm-rp1.c new file +8 -0

+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	/*
+	 * .remove is not needed because this driver cannot be removed/unbound
+	 * cause it relies on syscon which cannot be removed
+	 */
+	.driver = {
+		.name = "rp1-pwm",

[Severity: High]
Is it safe to omit the remove callback here?

Even with suppress_bind_attrs set on the driver, if the parent RP1 PCIe device is removed, reset, or unbound via sysfs, it will forcibly unbind all child devices.

Without a remove callback to run cleanup, pwmchip_remove() is never called and the clock is left enabled. Because the backing memory is devm-managed, the devres subsystem will eventually free the pwm_chip and regmap memory. This can leave dangling pointers in the core pwm_chips IDR and the global syscon_list, which could result in a use-after-free.

Quoted 6 lines

Quoted 3 lines

This is true. Uwe, I guess I should reintroduce the .remove callback, what do you think?

How is remove going to work with syscon? The previous version seems to just ignore it completely.

True, unfortunately we have not much choice with syscon because it does not contemplate a corresponding remove function. The best well-behaved code could just deal with clocks and resources that can be released, while syscon is unfortunately not among them.

On the other hand, I'd like to emphasize the following:

- the driver could not be unbinded nor unloaded (directly) by the user, and...
- ...PWM driver removal as a consequence of the pci bus device being forcibly removed will almost surely leave the system unusable.

As a matter of fact, even though it's not impossible for the remove callback to be called, it seems highly unlikely. So maybe it's not that big deal.

Quoted 3 lines

Also, do you actually need syscon here? Given that the pwm fan device is going to depend on the PWM fan device anyway,

Quoted 3 lines

you could just EXPORT_SYMBOL_NS an API
from the rp1 driver that returns the tachometer reading when supplied with a rp1 pwm device pointer?

I'm not devoted to syscon so if everyone agrees on the EXPORT_SYMBOL_NS solution I'm perfectly fine. It seems to be used throughout several drivers and as long as it's not considered to couple the consumer and the PWM producer too much, I think it's an easy alternative. Uwe, are you fine with that?

Many thanks,
Andrea

Quoted 4 lines

Best,
Gary

Quoted 57 lines · Andrea della Porta

Hi Gary,

Quoted 42 lines · Gary Guo

Quoted 41 lines · Andrea della Porta

Hi,

Quoted 37 lines · On 10:02 Mon 20 Jul , [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing .remove callback leads to Use-After-Free in the PWM and Syscon cores and leaks clock resources on driver unbind.
- [High] Register context loss on suspend/resume silently breaks PWM output.
- [High] Unclamped hardware duty ticks lead to integer underflow in the PWM core for normal polarity.
--

--- Patch [2]: [PATCH v7 2/3] pwm: rp1: Add RP1 PWM controller driver --- commit 0c98703d857577902e57d25b42a3d46dfbde5795
Author: Naushir Patuck <[email protected]>

pwm: rp1: Add RP1 PWM controller driver

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.

drivers/pwm/pwm-rp1.c line 417

Quoted 8 lines

drivers/pwm/pwm-rp1.c new file +8 -0

+static struct platform_driver rp1_pwm_driver = {
+	.probe = rp1_pwm_probe,
+	/*
+	 * .remove is not needed because this driver cannot be removed/unbound
+	 * cause it relies on syscon which cannot be removed
+	 */
+	.driver = {
+		.name = "rp1-pwm",

[Severity: High]
Is it safe to omit the remove callback here?

Even with suppress_bind_attrs set on the driver, if the parent RP1 PCIe device is removed, reset, or unbound via sysfs, it will forcibly unbind all child devices.

Without a remove callback to run cleanup, pwmchip_remove() is never called and the clock is left enabled. Because the backing memory is devm-managed, the devres subsystem will eventually free the pwm_chip and regmap memory. This can leave dangling pointers in the core pwm_chips IDR and the global syscon_list, which could result in a use-after-free.

Quoted 6 lines

Quoted 3 lines

This is true. Uwe, I guess I should reintroduce the .remove callback, what do you think?

How is remove going to work with syscon? The previous version seems to just ignore it completely.

True, unfortunately we have not much choice with syscon because it does not contemplate a corresponding remove function. The best well-behaved code could just deal with clocks and resources that can be released, while syscon is unfortunately not among them.

Quoted 12 lines

On the other hand, I'd like to emphasize the following:

- the driver could not be unbinded nor unloaded (directly) by the user, and...
- ...PWM driver removal as a consequence of the pci bus device being forcibly removed will almost surely leave the system unusable.

As a matter of fact, even though it's not impossible for the remove callback to be called, it seems highly unlikely. So maybe it's not that big deal.

Quoted 3 lines

Also, do you actually need syscon here? Given that the pwm fan device is going to depend on the PWM fan device anyway,

Oops, it should have been "PWM fan device depend on PWM device". But you got it.

Quoted 8 lines

Quoted 3 lines

you could just EXPORT_SYMBOL_NS an API
from the rp1 driver that returns the tachometer reading when supplied with a rp1 pwm device pointer?

I'm not devoted to syscon so if everyone agrees on the EXPORT_SYMBOL_NS solution I'm perfectly fine. It seems to be used throughout several drivers and as long as it's not considered to couple the consumer and the PWM producer too much, I think it's an easy alternative. Uwe, are you fine with that?

I think that's pretty typical for platform devices, provided that the consumer and the producer are closely related.

I checked the RPi vendor kernel code and it looks like they're modifying the PWM fan driver code directly -- in which case the coupling would definitely be problematic. But I think that modification is problematic itself and not upstreamable. I suppose you'll add a separate, dedicated RP1 fan driver?

In which case I don't see the coupling being an issue.

Best,
Gary

Quoted 8 lines

Many thanks,
Andrea

Quoted 4 lines

Best,
Gary

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/pwm-rp1.c new file +15 -0

+static int rp1_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	unsigned long clk_rate;
+	struct pwm_chip *chip;
+	void __iomem	*base;
+	struct rp1_pwm *rp1;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, RP1_PWM_NUM_PWMS, sizeof(*rp1));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	rp1 = pwmchip_get_drvdata(chip);

drivers/pwm/pwm-rp1.c new file +20 -0

+	rp1->clk_enabled = true;
+
+	ret = devm_clk_rate_exclusive_get(dev, rp1->clk);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to get exclusive rate\n");
+		goto err_disable_clk;
+	}
+
+	clk_rate = clk_get_rate(rp1->clk);
+	if (!clk_rate) {
+		ret = dev_err_probe(dev, -EINVAL, "Failed to get clock rate\n");
+		goto err_disable_clk;
+	}
+	/*
+	 * To prevent u64 overflow in period calculations:
+	 * mul_u64_u64_div_u64(period_ns, clk_rate, NSEC_PER_SEC)
+	 * If clk_rate > 1 GHz, the result can overflow.
+	 */
+	if (clk_rate > HZ_PER_GHZ) {
+		ret = dev_err_probe(dev, -EINVAL, "Clock rate > 1 GHz is not supported\n");

drivers/pwm/pwm-rp1.c new file +10 -0

+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to register PWM chip\n");
+		goto err_disable_clk;
+	}
+
+	ret = of_syscon_register_regmap(np, rp1->regmap);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to register syscon\n");
+		goto err_remove_chip;
+	}

Le 20/07/2026 à 11:44, Andrea della Porta a écrit :

Quoted 6 lines

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.

Quoted 6 lines

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

Hi,

...

Quoted 20 lines

drivers/pwm/pwm-rp1.c new file +20 -0

+static int rp1_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	unsigned long clk_rate;
+	struct pwm_chip *chip;
+	void __iomem	*base;
+	struct rp1_pwm *rp1;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, RP1_PWM_NUM_PWMS, sizeof(*rp1));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	rp1 = pwmchip_get_drvdata(chip);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+

drivers/pwm/pwm-rp1.c line 314

Quoted 5 lines

drivers/pwm/pwm-rp1.c new file +5 -0

+	rp1->regmap = devm_regmap_init_mmio(dev, base, &rp1_pwm_regmap_config);
+	if (IS_ERR(rp1->regmap))
+		return dev_err_probe(dev, PTR_ERR(rp1->regmap), "Cannot initialize regmap\n");
+
+	rp1->clk = devm_clk_get(dev, NULL);

Could it be devm_clk_get_enabled() to simplify the error handling path as done above with other devm function?

...

drivers/pwm/pwm-rp1.c line 320

Quoted 6 lines

drivers/pwm/pwm-rp1.c new file +6 -0

+	if (IS_ERR(rp1->clk))
+		return dev_err_probe(dev, PTR_ERR(rp1->clk), "Clock not found\n");
+
+	ret = clk_prepare_enable(rp1->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable clock\n");

... this also saves these 3 lines.

Quoted 26 lines

drivers/pwm/pwm-rp1.c new file +25 -0

+	rp1->clk_enabled = true;
+
+	ret = devm_clk_rate_exclusive_get(dev, rp1->clk);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to get exclusive rate\n");
+		goto err_disable_clk;
+	}
+
+	clk_rate = clk_get_rate(rp1->clk);
+	if (!clk_rate) {
+		ret = dev_err_probe(dev, -EINVAL, "Failed to get clock rate\n");
+		goto err_disable_clk;
+	}
+	/*
+	 * To prevent u64 overflow in period calculations:
+	 * mul_u64_u64_div_u64(period_ns, clk_rate, NSEC_PER_SEC)
+	 * If clk_rate > 1 GHz, the result can overflow.
+	 */
+	if (clk_rate > HZ_PER_GHZ) {
+		ret = dev_err_probe(dev, -EINVAL, "Clock rate > 1 GHz is not supported\n");
+		goto err_disable_clk;
+	}
+	rp1->clk_rate = clk_rate;
+
+	chip->ops = &rp1_pwm_ops;

drivers/pwm/pwm-rp1.c line 350

Quoted 5 lines

drivers/pwm/pwm-rp1.c new file +5 -0

+	chip->atomic = true;
+
+	platform_set_drvdata(pdev, chip);
+
+	ret = pwmchip_add(chip);

Could it be devm_pwmchip_add() to simplify the error handling path as done above with other devm function?

Quoted 11 lines

drivers/pwm/pwm-rp1.c new file +10 -0

+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to register PWM chip\n");
+		goto err_disable_clk;
+	}
+
+	ret = of_syscon_register_regmap(np, rp1->regmap);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to register syscon\n");
+		goto err_remove_chip;
+	}

drivers/pwm/pwm-rp1.c line 370

Quoted 10 lines

drivers/pwm/pwm-rp1.c new file +10 -0

+
+	return 0;
+
+err_remove_chip:
+	pwmchip_remove(chip);
+err_disable_clk:
+	clk_disable_unprepare(rp1->clk);
+
+	return ret;
+}

...

CJ

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 via syscon registers. A subsequent patch will add a new device and driver to read the RPM and export this value via hwmon.

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

Best regards,
Andrea