Re: [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pinmux configuration properties were erroneously omitted from `pwm1_pins` in `jh7100-common.dtsi`.
- [Medium] Removal of SoC-specific compatible strings violates Device Tree ABI best practices.
--

commit e05af83398a49d79a54da1bad8e64c7c1e5f98eb
Author: Hal Feng <hal.feng@starfivetech.com>

riscv: dts: starfive: Correct pwm nodes

This commit updates the PWM nodes for StarFive JH7100 and JH7110 SoCs,
splitting the original single PWM node into 8 individual nodes representing
the 8 OpenCores PTC IP cores.

> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi

[ ... ]

> +	pwm1_pins: pwm1-0 {
> +		pwm1-pins {
> +			pinmux =  <GPIOMUX(5,
>  				  GPO_PWM_PAD_OUT_BIT1,
>  				  GPO_PWM_PAD_OE_N_BIT1,
>  				  GPI_NONE)>;
>  		};
>  	};

In the original code, the single pwm_pins node applied electrical
properties like bias-disable, drive-strength = <35>, input-disable,
input-schmitt-disable, and slew-rate = <0> to both PWM0 and PWM1 pins. When
splitting the node, these properties were copied to pwm0_pins but omitted
here.

Will omitting these properties cause the pwm1 output to operate with
undefined or default hardware pin states instead of the intended parameters?

> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi

[ ... ]

> -		pwm: pwm@12490000 {
> -			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
> -			reg = <0x0 0x12490000 0x0 0x10000>;
> +		pwm0: pwm@12490000 {
> +			compatible = "opencores,pwm-v1";

Is it intentional to drop the SoC-specific "starfive,jh7100-pwm" compatible
string here, as well as "starfive,jh7110-pwm" in jh7110.dtsi?

Device Tree best practices generally recommend retaining SoC-specific
strings alongside generic IP strings. This ensures that if SoC-specific
integration quirks are discovered later, workarounds can be applied without
requiring users to update deployed device trees, and it maintains ABI
compatibility for older kernels that might match against the removed
strings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515054723.25024-1-hal.feng@starfivetech.com?part=2