Re: [PATCH v10 3/3] arm64: dts: broadcom: rpi-5: Add RP1 PWM node

[sashiko-bot@kernel.org]

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 <svarbanov@suse.de>

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.

> 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 0fc57e72632ed..748be8f1ee9e2 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
[ ... ]
> @@ -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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ee0aa89faa14e0073f1271693bae28678652998b.1790606188.git.andrea.porta@suse.com?part=3