Re: [PATCH v1 13/14] thermal/of: Process cooling device index in cooling-spec

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Brittle logic uses an exact `args_count == 3` check to identify the new binding format, leading to silent misparsing of bindings with more than 3 cells.
--

commit 90bb7215d764688a8fd719c59ad60bede0aee71e
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

thermal/of: Process cooling device index in cooling-spec

This commit updates thermal OF binding parsing to support a new format that
includes a cooling device index alongside mitigation limits. It checks the
argument count from the device tree specifier to differentiate between the
legacy and new formats.

> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index d9bd7dc01e19a..e9b39d625b0ce 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
[ ... ]
> @@ -264,11 +264,29 @@ static bool thermal_of_get_cooling_spec(struct device_node *map_np, int index,
>  		return false;
>  	}
>  
> +	/*
> +	 * There are two formats:
> +	 * - Legacy format :	<&cdev lower upper>
> +	 * - New format    :	<&cdev of_index lower upper>
> +	 *
> +	 * With the new format, along with the device node pointer,
> +	 * the of_index must match with the cooling device of_index in
> +	 * order to bind
> +	 */
> +	if (cooling_spec.args_count == 3 &&
> +	    cooling_spec.args[0] != cdev->of_index)
> +		return false;

Does this code bypass the index validation for malformed bindings or future
extensions? If a device tree specifies a cell count greater than 3, this
condition evaluates to false and skips the check entirely.

> +
>  	if (cooling_spec.np != cdev->np)
>  		return false;
>  
> -	c->lower = cooling_spec.args[0];
> -	c->upper = cooling_spec.args[1];
> +	if (cooling_spec.args_count != 3) {
> +		c->lower = cooling_spec.args[0];
> +		c->upper = cooling_spec.args[1];

Will this incorrectly parse the limits if the argument count is greater than 3?

Since a count greater than 3 falls into this legacy branch, it would assign
the device index from args[0] to the lower limit, instead of args[1].

Should this logic either explicitly reject counts greater than 3, or use a
greater-than-or-equal condition to identify the new format?

> +	} else {
> +		c->lower = cooling_spec.args[1];
> +		c->upper = cooling_spec.args[2];
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260419182203.4083985-1-daniel.lezcano@oss.qualcomm.com?part=13