Some drivers want to create a link from the thermal zone to the device
sysfs entry and vice versa. That is the case of the APCI driver.
Having a backpointer from the device to the thermal zone sounds akward
as we can have the same device instantiating multiple thermal zones so
there will be a conflict while creating the second link with the same
name. Moreover, the userspace has enough information to build the
dependency from the thermal zone device link without having this cyclic
link from the device to thermal zone.
Anyway, everything in its time.
This change allows to create a these cyclic links tz <-> device as
ACPI does and will allow to remove the code in the ACPI driver.
The limitation of this change is there can be only a 1:1 relationship
between the device and the thermal zone, otherwise the 'thermal_zone'
link name will conflict with the previous link with the same name.
Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/thermal_core.c | 16 ++++++++++++++++
include/linux/thermal.h | 7 +++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index cec72c6673a5..ca91189bc441 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1340,6 +1340,18 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
list_add_tail(&tz->node, &thermal_tz_list);
mutex_unlock(&thermal_list_lock);
+ if (tzp && tzp->linked_dev) {
+ result = sysfs_create_link(&tzp->linked_dev->kobj,
+ &tz->device.kobj, "thermal_zone");
+ if (result)
+ goto out_list_del;
+
+ result = sysfs_create_link(&tz->device.kobj,
+ &tzp->linked_dev->kobj, "device");
+ if (result)
+ goto out_del_link;
+ }
+
/* Bind cooling devices for this zone */
bind_tz(tz);
@@ -1354,6 +1366,10 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
return tz;
+out_del_link:
+ sysfs_remove_link(&tz->device.kobj, "thermal_zone");
+out_list_del:
+ list_del(&tz->node);
unregister:
device_del(&tz->device);
release_device:
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 8cdf94cdc5ff..f60d7edf1e5d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -256,6 +256,13 @@ struct thermal_zone_params {
int num_tbps; /* Number of tbp entries */
struct thermal_bind_params *tbp;
+ /*
+ * @linked_dev: Add a cross link from the device to the
+ * thermal zone and vice versa. They will be named
+ * respectively 'device' and 'thermal_zone'
+ */
+ struct device *linked_dev;
+
/*
* Sustainable power (heat) that this thermal zone can dissipate in
* mW
--
2.34.1
On Tue, Mar 7, 2023 at 2:38 PM Daniel Lezcano <[email protected]> wrote:
>
> Some drivers want to create a link from the thermal zone to the device
> sysfs entry and vice versa.
Which device is this, exactly?
> That is the case of the APCI driver.
>
> Having a backpointer from the device to the thermal zone sounds akward
> as we can have the same device instantiating multiple thermal zones so
> there will be a conflict while creating the second link with the same
> name. Moreover, the userspace has enough information to build the
> dependency from the thermal zone device link without having this cyclic
> link from the device to thermal zone.
>
> Anyway, everything in its time.
>
> This change allows to create a these cyclic links tz <-> device as
> ACPI does and will allow to remove the code in the ACPI driver.
Well, I'd rather have it in the driver than in the core TBH.
If ACPI is the only user of this, let it do the dirty thing by itself.
There are two cases which would justify making this change:
1. There will be more users of it going forward (seems unlikely from
the description).
2. It gets in the way of some other changes somehow.
I kind of expect 2. to be the case, so how does it get in the way?
On 27/03/2023 18:16, Rafael J. Wysocki wrote:
> On Tue, Mar 7, 2023 at 2:38 PM Daniel Lezcano <[email protected]> wrote:
>>
>> Some drivers want to create a link from the thermal zone to the device
>> sysfs entry and vice versa.
>
> Which device is this, exactly?
ls -alh /sys/bus/acpi/drivers/thermal/LNXTHERM:00/
[ ... ]
lrwxrwxrwx 1 0 thermal_zone ->
../../../virtual/thermal/thermal_zone0
[ ... ]
The ACPI driver is the only one doing this AFAICT.
>> That is the case of the APCI driver.
>>
>> Having a backpointer from the device to the thermal zone sounds akward
>> as we can have the same device instantiating multiple thermal zones so
>> there will be a conflict while creating the second link with the same
>> name. Moreover, the userspace has enough information to build the
>> dependency from the thermal zone device link without having this cyclic
>> link from the device to thermal zone.
>>
>> Anyway, everything in its time.
>>
>> This change allows to create a these cyclic links tz <-> device as
>> ACPI does and will allow to remove the code in the ACPI driver.
>
> Well, I'd rather have it in the driver than in the core TBH.
>
> If ACPI is the only user of this, let it do the dirty thing by itself.
>
> There are two cases which would justify making this change:
> 1. There will be more users of it going forward (seems unlikely from
> the description).
> 2. It gets in the way of some other changes somehow.
>
> I kind of expect 2. to be the case, so how does it get in the way?
Shall we do the same approach than 'Menlow' and add an option to remove
the thermal zone link in the acpi sysfs directory ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Tue, Apr 4, 2023 at 9:01 PM Daniel Lezcano <[email protected]> wrote:
>
> On 27/03/2023 18:16, Rafael J. Wysocki wrote:
> > On Tue, Mar 7, 2023 at 2:38 PM Daniel Lezcano <[email protected]> wrote:
> >>
> >> Some drivers want to create a link from the thermal zone to the device
> >> sysfs entry and vice versa.
> >
> > Which device is this, exactly?
>
> ls -alh /sys/bus/acpi/drivers/thermal/LNXTHERM:00/
>
> [ ... ]
>
> lrwxrwxrwx 1 0 thermal_zone ->
> ../../../virtual/thermal/thermal_zone0
>
> [ ... ]
>
> The ACPI driver is the only one doing this AFAICT.
>
>
> >> That is the case of the APCI driver.
> >>
> >> Having a backpointer from the device to the thermal zone sounds akward
> >> as we can have the same device instantiating multiple thermal zones so
> >> there will be a conflict while creating the second link with the same
> >> name. Moreover, the userspace has enough information to build the
> >> dependency from the thermal zone device link without having this cyclic
> >> link from the device to thermal zone.
> >>
> >> Anyway, everything in its time.
> >>
> >> This change allows to create a these cyclic links tz <-> device as
> >> ACPI does and will allow to remove the code in the ACPI driver.
> >
> > Well, I'd rather have it in the driver than in the core TBH.
> >
> > If ACPI is the only user of this, let it do the dirty thing by itself.
> >
> > There are two cases which would justify making this change:
> > 1. There will be more users of it going forward (seems unlikely from
> > the description).
> > 2. It gets in the way of some other changes somehow.
> >
> > I kind of expect 2. to be the case, so how does it get in the way?
>
> Shall we do the same approach than 'Menlow' and add an option to remove
> the thermal zone link in the acpi sysfs directory ?
That depends on the answer to the question above.