From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since thermal_of_should_bind() terminates the loop after processing
the first child found in cooling-maps, it will never match more than
one cdev to a given trip point which is incorrect, as there may be
cooling-maps associating one trip point with multiple cooling devices.

Address this by letting the loop continue until either all
children have been processed or a matching one has been found.

To avoid adding conditionals or goto statements, put the loop in
question into a separate function and make that function return
right away after finding a matching cooling-maps entry.

Fixes: 94c6110b0b13 ("thermal/of: Use the .should_bind() thermal zone callback")
Link: https://lore.kernel.org/linux-pm/20250219-fix-thermal-of-v1-1-de36e7a590c4@chromium.org/
Reported-by: Yu-Che Cheng <giver@chromium.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_of.c |   50 ++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -274,6 +274,34 @@
 	return true;
 }
 
+static bool thermal_of_cm_lookup(struct device_node *cm_np,
+				 const struct thermal_trip *trip,
+				 struct thermal_cooling_device *cdev,
+				 struct cooling_spec *c)
+{
+	for_each_child_of_node_scoped(cm_np, child) {
+		struct device_node *tr_np;
+		int count, i;
+
+		tr_np = of_parse_phandle(child, "trip", 0);
+		if (tr_np != trip->priv)
+			continue;
+
+		/* The trip has been found, look up the cdev. */
+		count = of_count_phandle_with_args(child, "cooling-device",
+						   "#cooling-cells");
+		if (count <= 0)
+			pr_err("Add a cooling_device property with at least one device\n");
+
+		for (i = 0; i < count; i++) {
+			if (thermal_of_get_cooling_spec(child, i, cdev, c))
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static bool thermal_of_should_bind(struct thermal_zone_device *tz,
 				   const struct thermal_trip *trip,
 				   struct thermal_cooling_device *cdev,
@@ -293,27 +321,7 @@
 		goto out;
 
 	/* Look up the trip and the cdev in the cooling maps. */
-	for_each_child_of_node_scoped(cm_np, child) {
-		struct device_node *tr_np;
-		int count, i;
-
-		tr_np = of_parse_phandle(child, "trip", 0);
-		if (tr_np != trip->priv)
-			continue;
-
-		/* The trip has been found, look up the cdev. */
-		count = of_count_phandle_with_args(child, "cooling-device", "#cooling-cells");
-		if (count <= 0)
-			pr_err("Add a cooling_device property with at least one device\n");
-
-		for (i = 0; i < count; i++) {
-			result = thermal_of_get_cooling_spec(child, i, cdev, c);
-			if (result)
-				break;
-		}
-
-		break;
-	}
+	result = thermal_of_cm_lookup(cm_np, trip, cdev, c);
 
 	of_node_put(cm_np);
 out:

Reviewed-by: Yu-Che Cheng <giver@chromium.org>
Tested-by: Yu-Che Cheng <giver@chromium.org>

On Sat, Feb 22, 2025 at 12:57 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since thermal_of_should_bind() terminates the loop after processing
> the first child found in cooling-maps, it will never match more than
> one cdev to a given trip point which is incorrect, as there may be
> cooling-maps associating one trip point with multiple cooling devices.
>
> Address this by letting the loop continue until either all
> children have been processed or a matching one has been found.
>
> To avoid adding conditionals or goto statements, put the loop in
> question into a separate function and make that function return
> right away after finding a matching cooling-maps entry.
>
> Fixes: 94c6110b0b13 ("thermal/of: Use the .should_bind() thermal zone callback")
> Link: https://lore.kernel.org/linux-pm/20250219-fix-thermal-of-v1-1-de36e7a590c4@chromium.org/
> Reported-by: Yu-Che Cheng <giver@chromium.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/thermal/thermal_of.c |   50 ++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
>
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -274,6 +274,34 @@
>         return true;
>  }
>
> +static bool thermal_of_cm_lookup(struct device_node *cm_np,
> +                                const struct thermal_trip *trip,
> +                                struct thermal_cooling_device *cdev,
> +                                struct cooling_spec *c)
> +{
> +       for_each_child_of_node_scoped(cm_np, child) {
> +               struct device_node *tr_np;
> +               int count, i;
> +
> +               tr_np = of_parse_phandle(child, "trip", 0);
> +               if (tr_np != trip->priv)
> +                       continue;
> +
> +               /* The trip has been found, look up the cdev. */
> +               count = of_count_phandle_with_args(child, "cooling-device",
> +                                                  "#cooling-cells");
> +               if (count <= 0)
> +                       pr_err("Add a cooling_device property with at least one device\n");
> +
> +               for (i = 0; i < count; i++) {
> +                       if (thermal_of_get_cooling_spec(child, i, cdev, c))
> +                               return true;
> +               }
> +       }
> +
> +       return false;
> +}
> +
>  static bool thermal_of_should_bind(struct thermal_zone_device *tz,
>                                    const struct thermal_trip *trip,
>                                    struct thermal_cooling_device *cdev,
> @@ -293,27 +321,7 @@
>                 goto out;
>
>         /* Look up the trip and the cdev in the cooling maps. */
> -       for_each_child_of_node_scoped(cm_np, child) {
> -               struct device_node *tr_np;
> -               int count, i;
> -
> -               tr_np = of_parse_phandle(child, "trip", 0);
> -               if (tr_np != trip->priv)
> -                       continue;
> -
> -               /* The trip has been found, look up the cdev. */
> -               count = of_count_phandle_with_args(child, "cooling-device", "#cooling-cells");
> -               if (count <= 0)
> -                       pr_err("Add a cooling_device property with at least one device\n");
> -
> -               for (i = 0; i < count; i++) {
> -                       result = thermal_of_get_cooling_spec(child, i, cdev, c);
> -                       if (result)
> -                               break;
> -               }
> -
> -               break;
> -       }
> +       result = thermal_of_cm_lookup(cm_np, trip, cdev, c);
>
>         of_node_put(cm_np);
>  out:
>
>
>