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

sashiko-bot
2026-04-19T18:51:58Z

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