Add RP1 PWM controller support
Subsystems (4)
Andrea della Porta
maintainerv11 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 will be exported by this driver via a function. A subsequent patch will add the function to read the speed to this driver and 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
CHANGES in V11:
- capped duty_cycle in rp1_pwm_round_waveform_fromhw()
- fixed a conditional in probe function dealing with an unitialized local variable (base instead of rp1->base). As a result, drop the unused local.
Link to V10:
https://lore.kernel.org/all/[email protected]/
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: rpi-5: Add RP1 PWM node
.../bindings/pwm/raspberrypi,rp1-pwm.yaml | 66 +++ .../boot/dts/broadcom/bcm2712-rpi-5-b.dts | 12 + arch/arm64/boot/dts/broadcom/rp1-common.dtsi | 9 + drivers/pwm/Kconfig | 7 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-rp1.c | 417 ++++++++++++++++++ 6 files changed, 512 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
Andrea della Porta
maintainerPATCH v10 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.
--- .../bindings/pwm/raspberrypi,rp1-pwm.yaml | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml
Documentation/devicetree/bindings/pwm/raspberrypi,rp1-pwm.yaml new file +66 -0
@@ -0,0 +1,66 @@ +# 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]> + - Andrea della Porta <[email protected]> + - Stanimir Varbanov <[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 + Serves as a fan speed provider to other nodes for a PWM-connected + fan. + +properties: + compatible: + const: raspberrypi,rp1-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + "#pwm-cells": + const: 3 + + dmas: + maxItems: 1 + description: | + Phandle to the DMA controller and the channel specifier for + the duty-cycle FIFO. + + dma-names: + items: + - const: tx + +required: + - compatible + - reg + - clocks + +unevaluatedProperties: false + +examples: + - | + pwm@98000 { + compatible = "raspberrypi,rp1-pwm"; + reg = <0x98000 0x100>; + clocks = <&rp1_clocks 17>; + #pwm-cells = <3>; + };
Signature · 1 line
2.35.3
Andrea della Porta
maintainerPATCH v10 2/3 pwm: rp1: Add RP1 PWM controller driver
In reply to Andrea della PortaFrom: 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.
--- drivers/pwm/Kconfig | 7 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-rp1.c | 417 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 425 insertions(+) create mode 100644 drivers/pwm/pwm-rp1.c
drivers/pwm/Kconfig +7 -0
@@ -637,6 +637,13 @@ config PWM_ROCKCHIP Generic PWM framework driver for the PWM controller found on Rockchip SoCs. +config PWM_RASPBERRYPI_RP1 + tristate "RP1 PWM support" + depends on MISC_RP1 || COMPILE_TEST + depends on HAS_IOMEM + help + PWM framework driver for Raspberry Pi RP1 controller. + config PWM_SAMSUNG tristate "Samsung PWM support" depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
drivers/pwm/Makefile +1 -0
@@ -57,6 +57,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_RASPBERRYPI_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 +417 -0
417 lines not on this page · raw message
Signature · 1 line
2.35.3
[email protected]
AI reviewRe: [PATCH v10 2/3] pwm: rp1: Add RP1 PWM controller driver
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] pwm: rp1: channel control register state loss across suspend --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pwm: rp1: channel control register state loss across suspend
--
commit b77b6f077f5aa3ca8ba16d313a2e9952c97e47f7
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. This patch adds the supporting driver.
Quoted 5 lines
drivers/pwm/pwm-rp1.c new file +0
-0
[ ... ]
drivers/pwm/pwm-rp1.c line 115
Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10
-0+static int rp1_pwm_request (struct pwm_chip *chip,struct pwm_device *pwm) +{ +struct rp1_pwm *rp1 =pwmchip_get_drvdata (chip); +u32 value, polarity; + +/* init channel to reset defaults, preserving the polarity bit */ + value =readl (rp1->base +RP1_PWM_CHAN_CTRL (pwm->hwpwm)); + polarity =FIELD_GET (RP1_PWM_CHAN_CTRL_POLARITY , value); + value =u32_replace_bits (RP1_PWM_CHAN_CTRL_DEFAULT , polarity,RP1_PWM_CHAN_CTRL_POLARITY ); +writel (value, rp1->base +RP1_PWM_CHAN_CTRL (pwm->hwpwm));
[Severity: Medium]
Will this channel control mode configuration be lost if the hardware loses power or is reset during system suspend?
The required MODE_TE_MS and FIFO_POP_MASK settings are initialized here, but they do not appear to be restored anywhere on resume.
Quoted 3 lines
drivers/pwm/pwm-rp1.c new file +3
-0+ +return 0 ; +}
[ ... ]
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 224
Quoted 10 lines
drivers/pwm/pwm-rp1.c new file +10
-0+const struct rp1_pwm_waveform *wfhw = _wfhw; +u32 value, ctrl; + +/* set polarity */ + value =readl (rp1->base +RP1_PWM_CHAN_CTRL (pwm->hwpwm)); +if (!wfhw->inverted_polarity) + value &= ~RP1_PWM_CHAN_CTRL_POLARITY ; +else + value |=RP1_PWM_CHAN_CTRL_POLARITY ; +writel (value, rp1->base +RP1_PWM_CHAN_CTRL (pwm->hwpwm));
[Severity: Medium]
If the channel control register loses its state across suspend, could this read-modify-write operation preserve the corrupted state?
If a PWM consumer calls pwm_apply_state() upon resume, it would invoke rp1_pwm_write_waveform() and read the potentially zeroed state, modify only the polarity bit, and write the corrupted state back. This would cause the PWM channel to fail to output correctly since the proper mode settings are missing.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/ee0aa89faa14e0073f1271693bae28678652998b.1790606188.git.andrea.porta@suse.com?part=2
Andrea della Porta
maintainerPATCH v10 3/3 arm64: dts: broadcom: rpi-5: Add RP1 PWM node
In reply to Andrea della PortaFrom: 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.
--- arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts | 12 ++++++++++++ arch/arm64/boot/dts/broadcom/rp1-common.dtsi | 9 +++++++++ 2 files changed, 21 insertions(+)
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +12 -0
@@ -64,6 +64,12 @@ 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";
@@ -94,6 +100,12 @@ &rp1_i2c6 { pinctrl-names = "default"; }; +&rp1_pwm1 { + 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 +9 -0
@@ -99,7 +99,16 @@ rp1_i2c6: i2c@40088000 { clocks = <&rp1_clocks RP1_CLK_SYS>; i2c-scl-rising-time-ns = <65>; i2c-scl-falling-time-ns = <100>; + status = "disabled"; + }; + rp1_pwm1: 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"; };
Signature · 1 line
2.35.3
[email protected]
AI reviewRe: [PATCH v10 3/3] arm64: dts: broadcom: rpi-5: Add RP1 PWM node
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] pwm: rp1: missing spinlock for shared RP1_PWM_GLB_CTRL register --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pwm: rp1: missing spinlock for shared RP1_PWM_GLB_CTRL register
--
commit 0c157fdfbde9f74961d4b31ce35f1c71872efa00
Author: Stanimir Varbanov <[email protected]>
arm64: dts: broadcom: rpi-5: Add RP1 PWM node
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.
Quoted 4 lines
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +0
-0
[ ... ]
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts &rp1_i2c6 { · line 107
Quoted 9 lines
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +5
-0@@ -94,6 +100,12 @@ &rp1_i2c6 { pinctrl-names = "default"; }; +&rp1_pwm1 { + pinctrl-0 = <&fan_pwm_default_state>; + pinctrl-names = "default"; + status = "okay"; +};
[Severity: High]
Can concurrent PWM channel configuration cause lost updates when this node is enabled?
Looking at drivers/pwm/pwm-rp1.c, there is a read-modify-write race on the shared hardware register RP1_PWM_GLB_CTRL:
drivers/pwm/pwm-rp1.c:rp1_pwm_write_waveform() {
...
/* enable the channel */
ctrl |= RP1_PWM_GLB_CTRL_CHANNEL_ENABLE(pwm->hwpwm);
writel(ctrl, rp1->base + RP1_PWM_GLB_CTRL);
...
}and also in:
drivers/pwm/pwm-rp1.c:rp1_pwm_apply_config() {
...
value = readl(rp1->base + RP1_PWM_GLB_CTRL);
value |= RP1_PWM_GLB_CTRL_SET_UPDATE;
writel(value, rp1->base + RP1_PWM_GLB_CTRL);
...
}If two threads call pwm_apply_state() on different PWM channels concurrently, the PWM core locks at the channel level, not the chip level.
One thread's enable/disable action could overwrite the other's, leading to channels unexpectedly disabling or failing to enable.
Since this PWM drives the cooling fan, could a lost update disabling the fan lead to a thermal shutdown?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/ee0aa89faa14e0073f1271693bae28678652998b.1790606188.git.andrea.porta@suse.com?part=3
Re: [PATCH v10 3/3] arm64: dts: broadcom: rpi-5: Add RP1 PWM node
In reply to Andrea della PortaQuoted 10 lines · 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]>
Applied to devicetre-arm64/next, thanks!
Signature · 1 line
Florian
Rob Herring
maintainerRe: [PATCH v10 3/3] arm64: dts: broadcom: rpi-5: Add RP1 PWM node
Quoted 7 lines · Florian Fainelli
Quoted 5 lines · 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.
Quoted 7 lines
Quoted 5 lines
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]>Applied to devicetre-arm64/next, thanks!
And that introduces warnings because the PWM binding is not applied. Either wait for the binding to be applied or apply the binding with the .dts change. IOW, Broadcom is warning free (you're welcome), don't introduce warnings.
Rob