| Message ID | 20250219-fix-thermal-of-v1-1-de36e7a590c4@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | thermal: of: Fix logic in thermal_of_should_bind | expand |
On Wed, Feb 19, 2025 at 8:06 AM Yu-Che Cheng <giver@chromium.org> wrote: > > The current thermal_of_should_bind will stop iterating cooling-maps on > the first matched trip point, leading to subsequent cooling devices > binding to the same trip point failing to find the cooling spec. > > The iteration should continue enumerating subsequent cooling-maps if the > target cooling device is not found. > > Fix the logic to break only when a matched cooling device is found. OK, but -> > Fixes: 94c6110b0b13 ("thermal/of: Use the .should_bind() thermal zone callback") > Signed-off-by: Yu-Che Cheng <giver@chromium.org> > --- > drivers/thermal/thermal_of.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > index 5ab4ce4daaeb..69c530e38574 100644 > --- a/drivers/thermal/thermal_of.c > +++ b/drivers/thermal/thermal_of.c > @@ -312,7 +312,8 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, > break; -> I'd prefer to do a jump from here, that is - break; + goto put_cm_np; > } > > - break; and remove the break statement above altogether. > + if (result) > + break; > } > And of course the label needs to be added too: +put_cm_np: > of_node_put(cm_np); > > ---
On Wed, Feb 19, 2025 at 10:40 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Feb 19, 2025 at 8:06 AM Yu-Che Cheng <giver@chromium.org> wrote: > > > > The current thermal_of_should_bind will stop iterating cooling-maps on > > the first matched trip point, leading to subsequent cooling devices > > binding to the same trip point failing to find the cooling spec. > > > > The iteration should continue enumerating subsequent cooling-maps if the > > target cooling device is not found. > > > > Fix the logic to break only when a matched cooling device is found. > > OK, but -> > > > Fixes: 94c6110b0b13 ("thermal/of: Use the .should_bind() thermal zone callback") > > Signed-off-by: Yu-Che Cheng <giver@chromium.org> > > --- > > drivers/thermal/thermal_of.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > > index 5ab4ce4daaeb..69c530e38574 100644 > > --- a/drivers/thermal/thermal_of.c > > +++ b/drivers/thermal/thermal_of.c > > @@ -312,7 +312,8 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, > > break; > > -> I'd prefer to do a jump from here, that is > > - break; > + goto put_cm_np; > > } > > > > - break; > > and remove the break statement above altogether. > > > + if (result) > > + break; > > } > > > > And of course the label needs to be added too: > > +put_cm_np: > > of_node_put(cm_np); > > > > --- Or even, to avoid adding a new label, move the loop from thermal_of_should_bind() into a new function that will be called by it do carry out the cooling-maps lookup, like in the attached patch. Can you check if it works for you, please?
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 5ab4ce4daaeb..69c530e38574 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -312,7 +312,8 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, break; } - break; + if (result) + break; } of_node_put(cm_np);
The current thermal_of_should_bind will stop iterating cooling-maps on the first matched trip point, leading to subsequent cooling devices binding to the same trip point failing to find the cooling spec. The iteration should continue enumerating subsequent cooling-maps if the target cooling device is not found. Fix the logic to break only when a matched cooling device is found. Fixes: 94c6110b0b13 ("thermal/of: Use the .should_bind() thermal zone callback") Signed-off-by: Yu-Che Cheng <giver@chromium.org> --- drivers/thermal/thermal_of.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- base-commit: 2408a807bfc3f738850ef5ad5e3fd59d66168996 change-id: 20250218-fix-thermal-of-247b71be0faa Best regards,