Re: [PATCH v2] riscv: dts: spacemit: k3: Add pwm support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The driver ignores the `bus` clock, which will lead to a system hang when accessing registers.
- [Medium] DT binding violation due to undocumented compatible string and property constraints.
- [Low] Copy-paste errors in pinctrl sub-node names.
--

commit 4d46fb2ab333b7a733df38e29f0f7c7d23523cee
Author: Yixun Lan <dlan@kernel.org>

riscv: dts: spacemit: k3: Add pwm support
    
This patch populates all PWM device tree nodes for the SpacemiT K3 SoC. It
also adds the corresponding pinctrl configurations to easily enable them.

> diff --git a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> index 23899d3f308a0..06abca6d08832 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
[ ... ]
> +	/omit-if-no-ref/
> +	pwm1_1_cfg: pwm1-1-cfg {
> +		pwm0-0-pins {
> +			pinmux = <K3_PADCONF(43, 6)>;	/* pwm1 */
> +
> +			bias-pull-up = <0>;
> +			drive-strength = <25>;
> +		};
> +	};

This isn't a bug, but it looks like there might be a copy-paste error here.
Should this sub-node be named pwm1-1-pins instead of pwm0-0-pins? This
pattern also appears in other blocks like pwm1_2_cfg and pwm6_1_cfg.

[ ... ]
> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> index e6faf8d8759e1..e331adbcb11a9 100644
> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
[ ... ]
> +		pwm0: pwm@d401a000 {
> +			compatible = "spacemit,k3-pwm", "marvell,pxa910-pwm";
> +			reg = <0x0 0xd401a000 0x0 0x10>;
> +			clocks = <&syscon_apbc CLK_APBC_PWM0>,
> +				 <&syscon_apbc CLK_APBC_PWM0_BUS>;
> +			clock-names = "func", "bus";
> +			resets = <&syscon_apbc RESET_APBC_PWM0>;
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};

Will this cause make dtbs_check to fail?

The existing schema in marvell,pxa-pwm.yaml restricts clocks to a single
item, forbids clock-names entirely, and enforces #pwm-cells: 1 unless the
compatible is spacemit,k1-pwm. Are there missing schema updates for
spacemit,k3-pwm to allow these new properties?

Could this also lead to a system hang when accessing registers?

The device tree declares two clocks, but the driver in drivers/pwm/pwm-pxa.c
uses devm_clk_get(dev, NULL), which fetches only the first clock. 

Since the bus clock is never requested or enabled by the driver, won't it
be gated when the clock framework runs clk_disable_unused()? Accessing the
PWM controller's MMIO registers while the APB bus clock is gated usually
triggers a synchronous external abort.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-04-k3-pwm-dts-v2-1-c0ccba430abf@kernel.org?part=1