After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org --- drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
- mask = GENMASK_ULL((ntrips) - 1, 0); + mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0;
tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, mask, data, of_ops, &tzp,
--- base-commit: a5df3a702b2cba82bcfb066fa9f5e4a349c33924 change-id: 20250707-trip-point-73dae9fd9c74
Best regards,
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH 6.6] thermal/of: Fix mask mismatch when no trips subnode Link: https://lore.kernel.org/stable/20250707-trip-point-v1-1-8f89d158eda0%40chrom...
Please ignore this mail if the patch is not relevant for upstream.
On Mon, Jul 7, 2025 at 12:27 PM Hsin-Te Yuan yuanhsinte@chromium.org wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
mask = GENMASK_ULL((ntrips) - 1, 0);
mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0; tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, mask, data, of_ops, &tzp,
If this issue is present in the mainline, it is not necessary to mention "stable" in the changelog.
Just post a patch against the mainline with an appropriate Fixes: tag.
Thanks!
On Tue, Jul 8, 2025 at 12:57 AM Rafael J. Wysocki rafael@kernel.org wrote:
On Mon, Jul 7, 2025 at 12:27 PM Hsin-Te Yuan yuanhsinte@chromium.org wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
mask = GENMASK_ULL((ntrips) - 1, 0);
mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0; tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, mask, data, of_ops, &tzp,
If this issue is present in the mainline, it is not necessary to mention "stable" in the changelog.
Just post a patch against the mainline with an appropriate Fixes: tag.
Thanks!
`mask` has been removed from the mainline, so this patch is only applicable on old branches.
On Tue, Jul 8, 2025 at 8:41 AM Hsin-Te Yuan yuanhsinte@chromium.org wrote:
On Tue, Jul 8, 2025 at 12:57 AM Rafael J. Wysocki rafael@kernel.org wrote:
On Mon, Jul 7, 2025 at 12:27 PM Hsin-Te Yuan yuanhsinte@chromium.org wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
mask = GENMASK_ULL((ntrips) - 1, 0);
mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0; tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, mask, data, of_ops, &tzp,
If this issue is present in the mainline, it is not necessary to mention "stable" in the changelog.
Just post a patch against the mainline with an appropriate Fixes: tag.
Thanks!
`mask` has been removed from the mainline, so this patch is only applicable on old branches.
So the way to go would be to follow the mainline in those branches.
On Mon, Jul 07, 2025 at 06:27:10PM +0800, Hsin-Te Yuan wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
- mask = GENMASK_ULL((ntrips) - 1, 0);
- mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0;
Meta-comment, I hate ? : lines in C, especially when they are not needed, like here. Spell this out, with a real if statement please, so that we can read and easily understand what is going on.
That being said, I agree with Rafael, let's do whatever is in mainline instead. Fix it the same way it was fixed there by backporting the relevant commits.
thanks,
greg k-h
On Thu, Jul 10, 2025 at 9:33 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jul 07, 2025 at 06:27:10PM +0800, Hsin-Te Yuan wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
mask = GENMASK_ULL((ntrips) - 1, 0);
mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0;
Meta-comment, I hate ? : lines in C, especially when they are not needed, like here. Spell this out, with a real if statement please, so that we can read and easily understand what is going on.
I will change this in v2 if we end up going with this solution.
That being said, I agree with Rafael, let's do whatever is in mainline instead. Fix it the same way it was fixed there by backporting the relevant commits.
thanks,
greg k-h
`mask` is removed in 83c2d444ed9d ("thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly"), which needs 5340f7647294 ("thermal: core: Add flags to struct thermal_trip"). I think it's beyond a fix to introduce this. Also, there were several conflicts when I tried to cherry-pick 5340f7647294. Compared to a simple solution like setting `mask` to 0, I don't think it's worthwhile and safe to cherry-pick all the dependencies.
Regards, Hsin-Te
On Mon, Jul 14, 2025 at 08:36:29PM +0800, Hsin-Te Yuan wrote:
On Thu, Jul 10, 2025 at 9:33 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jul 07, 2025 at 06:27:10PM +0800, Hsin-Te Yuan wrote:
After commit 725f31f300e3 ("thermal/of: support thermal zones w/o trips subnode") was backported on 6.6 stable branch as commit d3304dbc2d5f ("thermal/of: support thermal zones w/o trips subnode"), thermal zones w/o trips subnode still fail to register since `mask` argument is not set correctly. When number of trips subnode is 0, `mask` must be 0 to pass the check in `thermal_zone_device_register_with_trips()`.
Set `mask` to 0 when there's no trips subnode.
Signed-off-by: Hsin-Te Yuan yuanhsinte@chromium.org
drivers/thermal/thermal_of.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 0f520cf923a1e684411a3077ad283551395eec11..97aeb869abf5179dfa512dd744725121ec7fd0d9 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -514,7 +514,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * of_ops->bind = thermal_of_bind; of_ops->unbind = thermal_of_unbind;
mask = GENMASK_ULL((ntrips) - 1, 0);
mask = ntrips ? GENMASK_ULL((ntrips) - 1, 0) : 0;
Meta-comment, I hate ? : lines in C, especially when they are not needed, like here. Spell this out, with a real if statement please, so that we can read and easily understand what is going on.
I will change this in v2 if we end up going with this solution.
That being said, I agree with Rafael, let's do whatever is in mainline instead. Fix it the same way it was fixed there by backporting the relevant commits.
thanks,
greg k-h
`mask` is removed in 83c2d444ed9d ("thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly"), which needs 5340f7647294 ("thermal: core: Add flags to struct thermal_trip"). I think it's beyond a fix to introduce this. Also, there were several conflicts when I tried to cherry-pick 5340f7647294. Compared to a simple solution like setting `mask` to 0, I don't think it's worthwhile and safe to cherry-pick all the dependencies.
Remember, every patch you add to the tree that is NOT upstream, will almost always cause problems later on, if not immediately (we have a lousy track record of one-off-patches.) Also this prevents future upstream changes from being able to be applied to the tree.
And as you will now be responsible for maintaining this for the next 3-4 years, do whatever possible to make it easy to keep alive properly.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org