Re: [PATCH v3 1/6] riscv: dts: spacemit: k1-musepi-pro: add PMIC and power infrastructure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Regulator minimum voltage constraints are set too loosely on fixed board rails, risking hardware brownouts.
--

commit 823adf1d7b0f1286cdd0e228119b713b02c36d2e
Author: Andre Heider <a.heider@gmail.com>

riscv: dts: spacemit: k1-musepi-pro: add PMIC and power infrastructure

This commit enables i2c8 and adds the connected SpacemiT P1 PMIC with its
related regulators for the board's power infrastructure and voltage
regulation support.

> diff --git a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> index 29e333b670cf0..e0b585c50014b 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
[ ... ]
> @@ -72,6 +82,130 @@ &pdma {
>  	status = "okay";
>  };
>  
> +&i2c8 {
> +	pinctrl-0 = <&i2c8_cfg>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +
> +	pmic@41 {
> +		compatible = "spacemit,p1";
> +		reg = <0x41>;
> +		interrupts = <64>;
[ ... ]
> +		dldoin1-supply = <&buck5>;
> +		dldoin2-supply = <&buck5>;
> +
> +		regulators {
[ ... ]
> +			buck3_1v8: buck3 {
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1800000>;

Should these minimum bounds be restricted to match the exact hardware layout
of the board?

The labels buck3_1v8 and buck4_3v3 indicate these rails are wired for fixed
1.8V and 3.3V usage, but the regulator-min-microvolt allows them to scale
down to 0.5V. Could this loose constraint risk hardware brownouts if a driver
attempts dynamic voltage scaling on these rails?

> +				regulator-ramp-delay = <5000>;
> +				regulator-always-on;
> +			};
> +
> +			buck4_3v3: buck4 {
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-ramp-delay = <5000>;
> +				regulator-always-on;
> +			};
> +
> +			buck5: buck5 {
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <3450000>;

Since buck5 is used as the parent supply for the DLDOs (dldoin1-supply and
dldoin2-supply), could the wide 0.5V minimum constraint allow the OS to drop
its voltage below the necessary headroom needed to power the downstream
DLDOs?

> +				regulator-ramp-delay = <5000>;
> +				regulator-always-on;
> +			};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513071958.29574-1-a.heider@gmail.com?part=1