Re: [PATCH 4/4] ARM: dts: helios4: wire LM75 into a thermal zone with fan cooling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `pwm-fan` nodes (`fan1` and `fan2`) are missing the `cooling-levels` property, rendering the dynamic thermal fan cooling entirely non-functional.
- [Low] The `#thermal-sensor-cells` property is added to the LM75 node without documenting it in the `lm75.yaml` DT binding schema.
- [Low] The `board_alert` trip point incorrectly uses `type = "passive"` for active cooling fans.
--

commit 1aed98d24fa9eab58f80e2422b885b0e44e17d12
Author: Rosen Penev <rosenp@gmail.com>

ARM: dts: helios4: wire LM75 into a thermal zone with fan cooling

The LM75 temperature sensor on i2c0 creates a hwmon interface but was
not referenced by any thermal zone. This patch wires it into a thermal
zone with fan cooling maps.

> diff --git a/arch/arm/boot/dts/marvell/armada-388-helios4.dts b/arch/arm/boot/dts/marvell/armada-388-helios4.dts
> index 003b0708e9656..51675646ec397 100644
> --- a/arch/arm/boot/dts/marvell/armada-388-helios4.dts
> +++ b/arch/arm/boot/dts/marvell/armada-388-helios4.dts

[ ... ]

> @@ -68,6 +69,35 @@ reg_5p0v_usb: regulator-5v-usb {
>  		vin-supply = <&reg_12v>;
>  	};
>  
> +	thermal-zones {
> +		board-thermal {
> +			polling-delay-passive = <2000>;
> +			polling-delay = <10000>;
> +			thermal-sensors = <&temp_sensor>;
> +
> +			trips {
> +				board_alert: board-alert {
> +					temperature = <55000>;
> +					hysteresis = <5000>;
> +					type = "passive";

[Severity: Low]
Does this trip type match the intended use for active cooling fans?

The thermal-zones.yaml device tree binding explicitly designates type = active
for fans and type = passive for CPU throttling. By using passive, the thermal
framework will use polling-delay-passive intervals instead of standard active
cooling intervals.

> +				};

[ ... ]

> @@ -129,6 +159,7 @@ fan1: j10-pwm {
>  		pwms = <&gpio1 9 40000>;	/* Target freq:25 kHz */
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&helios_fan1_pins>;
> +		#cooling-cells = <2>;

[Severity: High]
Is the required cooling-levels property missing from this fan node?

Without cooling-levels defined in the pwm-fan nodes, the driver's
pwm_fan_get_cooling_data() in drivers/hwmon/pwm-fan.c returns early.

This leaves pwm_fan_max_state at 0, causing the driver to set the fans to a
100% duty cycle and register the cooling device with a max state of 0. When
the thermal governor attempts to change the state, it can only request state
0, leaving the fan permanently at full speed.

>  	};
>  
>  	fan2: j17-pwm {
> @@ -136,6 +167,7 @@ fan2: j17-pwm {
>  		pwms = <&gpio1 23 40000>;	/* Target freq:25 kHz */
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&helios_fan2_pins>;
> +		#cooling-cells = <2>;

[Severity: High]
Will this fan also need the cooling-levels property to be controlled by the
thermal zone?

Without the property, the same issue as above will prevent the thermal
governor from adjusting this fan's speed.

>  	};

[ ... ]

> @@ -201,6 +233,7 @@ temp_sensor: temp@4c {
>  					compatible = "ti,lm75";
>  					reg = <0x4c>;
>  					vcc-supply = <&reg_3p3v>;
> +					#thermal-sensor-cells = <0>;

[Severity: Low]
Should the LM75 device tree binding schema be updated to document this new
provider capability?

Adding an undocumented capability causes dtbs_check schema validation errors
or warnings. The Documentation/devicetree/bindings/hwmon/lm75.yaml file needs
to be updated to include #thermal-sensor-cells.

>  				};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628230042.1204293-1-rosenp@gmail.com?part=4