| Message ID | 20241113085634.7657-1-lihuisong@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [v2] ACPI: thermal: Support for linking devices associated with the thermal zone | expand |
On Wed, Nov 13, 2024 at 10:07 AM Huisong Li <lihuisong@huawei.com> wrote: > > There are many 'cdevX' files which link cooling devices under > '/sys/class/thermal/thermal_zoneX/'. These devices contain active cooling > devices and passive cooling devices. And user cann't directly know which > devices temperature is represented by the thermal zone. > > However, ACPI spec provides a '_TZD' object which evaluates to a package > of device names. Each name corresponds to a device in the ACPI namespace > that is associated with the thermal zone. The temperature reported by the > thermal zone is roughly correspondent to that of each of the devices. > > User can get all devices a thermal zone measured by the 'measures' > directory under the thermal zone device. Well, that's kind of clear, but what exactly is the use case? Why does the user need to know that? > Signed-off-by: Huisong Li <lihuisong@huawei.com> > --- > v2: fix commitlog based on Rafael's comment. > --- > drivers/acpi/thermal.c | 114 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 113 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index 78db38c7076e..398195a5d42f 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -119,6 +119,9 @@ struct acpi_thermal { > struct work_struct thermal_check_work; > struct mutex thermal_check_lock; > refcount_t thermal_check_count; > + int num_domain_devices; > + struct acpi_device **domain_devices; > + struct kobject *holders_dir; > }; > > /* -------------------------------------------------------------------------- > @@ -589,6 +592,103 @@ static const struct thermal_zone_device_ops acpi_thermal_zone_ops = { > .critical = acpi_thermal_zone_device_critical, > }; > > +static void acpi_thermal_remove_domain_devices(struct acpi_thermal *tz) > +{ > + int i; > + > + if (!tz->num_domain_devices) > + return; > + > + for (i = 0; i < tz->num_domain_devices; i++) { > + struct acpi_device *obj = tz->domain_devices[i]; > + > + if (!obj) > + continue; > + > + sysfs_remove_link(tz->holders_dir, > + kobject_name(&obj->dev.kobj)); > + acpi_dev_put(obj); > + } > + > + kfree(tz->domain_devices); > + kobject_put(tz->holders_dir); > + tz->num_domain_devices = 0; > +} > + > +static int acpi_thermal_read_domain_devices(struct acpi_thermal *tz) > +{ > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *pss; > + acpi_status status; > + int ret = 0; > + int i; > + > + status = acpi_evaluate_object(tz->device->handle, "_TZD", NULL, > + &buffer); > + if (ACPI_FAILURE(status)) { > + acpi_evaluation_failure_warn(tz->device->handle, "_TZD", > + status); > + return -ENODEV; > + } > + > + pss = buffer.pointer; > + if (!pss || > + pss->type != ACPI_TYPE_PACKAGE) { > + dev_err(&tz->device->dev, "Thermal zone invalid _TZD data\n"); > + ret = -EFAULT; > + goto end; > + } > + > + if (!pss->package.count) > + goto end; > + > + tz->domain_devices = kcalloc(pss->package.count, > + sizeof(struct acpi_device *), GFP_KERNEL); > + if (!tz->domain_devices) { > + ret = -ENOMEM; > + goto end; > + } > + > + tz->holders_dir = kobject_create_and_add("measures", > + &tz->device->dev.kobj); > + if (!tz->holders_dir) { > + ret = -ENOMEM; > + goto exit_free; > + } > + > + tz->num_domain_devices = pss->package.count; > + for (i = 0; i < pss->package.count; i++) { > + struct acpi_device *obj; > + union acpi_object *element = &pss->package.elements[i]; > + > + /* Refuse non-references */ > + if (element->type != ACPI_TYPE_LOCAL_REFERENCE) > + continue; > + > + /* Create a symlink to domain objects */ > + obj = acpi_get_acpi_dev(element->reference.handle); > + tz->domain_devices[i] = obj; > + if (!obj) > + continue; > + > + ret = sysfs_create_link(tz->holders_dir, &obj->dev.kobj, > + kobject_name(&obj->dev.kobj)); > + if (ret) { > + acpi_dev_put(obj); > + tz->domain_devices[i] = NULL; > + } > + } > + > + ret = 0; > + goto end; > + > +exit_free: > + kfree(tz->domain_devices); > +end: > + kfree(buffer.pointer); > + return ret; > +} > + > static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) > { > struct device *tzdev = thermal_zone_device(tz->thermal_zone); > @@ -602,8 +702,19 @@ static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) > ret = sysfs_create_link(&tzdev->kobj, > &tz->device->dev.kobj, "device"); > if (ret) > - sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); > + goto remove_thermal_zone; > > + /* _TZD method is optional. */ > + ret = acpi_thermal_read_domain_devices(tz); > + if (ret != -ENODEV) > + goto remove_device; > + > + return 0; > + > +remove_device: > + sysfs_remove_link(&tz->device->dev.kobj, "device"); > +remove_thermal_zone: > + sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); > return ret; > } > > @@ -611,6 +722,7 @@ static void acpi_thermal_zone_sysfs_remove(struct acpi_thermal *tz) > { > struct device *tzdev = thermal_zone_device(tz->thermal_zone); > > + acpi_thermal_remove_domain_devices(tz); > sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); > sysfs_remove_link(&tzdev->kobj, "device"); > } > -- > 2.22.0 > >
Hi Rafael, 在 2024/11/13 17:26, Rafael J. Wysocki 写道: > On Wed, Nov 13, 2024 at 10:07 AM Huisong Li <lihuisong@huawei.com> wrote: >> There are many 'cdevX' files which link cooling devices under >> '/sys/class/thermal/thermal_zoneX/'. These devices contain active cooling >> devices and passive cooling devices. And user cann't directly know which >> devices temperature is represented by the thermal zone. >> >> However, ACPI spec provides a '_TZD' object which evaluates to a package >> of device names. Each name corresponds to a device in the ACPI namespace >> that is associated with the thermal zone. The temperature reported by the >> thermal zone is roughly correspondent to that of each of the devices. >> >> User can get all devices a thermal zone measured by the 'measures' >> directory under the thermal zone device. > Well, that's kind of clear, but what exactly is the use case? Why > does the user need to know that? IMO, this makes thermal zone information more friendly. For instance, user can directly know the temperature of CPUs or other devices is roughly represented by which thermal zone. This may offer the convenience for further usersapce application. BTW, the '_TZD' method is similar to the '_PMD' in acpi power meter. Since ACPI spec provides them, they should also have a role in their existence. > >> Signed-off-by: Huisong Li <lihuisong@huawei.com> >> --- >> v2: fix commitlog based on Rafael's comment. >> --- >> drivers/acpi/thermal.c | 114 ++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 113 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c >> index 78db38c7076e..398195a5d42f 100644 >> --- a/drivers/acpi/thermal.c >> +++ b/drivers/acpi/thermal.c >> @@ -119,6 +119,9 @@ struct acpi_thermal { >> struct work_struct thermal_check_work; >> struct mutex thermal_check_lock; >> refcount_t thermal_check_count; >> + int num_domain_devices; >> + struct acpi_device **domain_devices; >> + struct kobject *holders_dir; >> }; >> >> /* -------------------------------------------------------------------------- >> @@ -589,6 +592,103 @@ static const struct thermal_zone_device_ops acpi_thermal_zone_ops = { >> .critical = acpi_thermal_zone_device_critical, >> }; >> >> +static void acpi_thermal_remove_domain_devices(struct acpi_thermal *tz) >> +{ >> + int i; >> + >> + if (!tz->num_domain_devices) >> + return; >> + >> + for (i = 0; i < tz->num_domain_devices; i++) { >> + struct acpi_device *obj = tz->domain_devices[i]; >> + >> + if (!obj) >> + continue; >> + >> + sysfs_remove_link(tz->holders_dir, >> + kobject_name(&obj->dev.kobj)); >> + acpi_dev_put(obj); >> + } >> + >> + kfree(tz->domain_devices); >> + kobject_put(tz->holders_dir); >> + tz->num_domain_devices = 0; >> +} >> + >> +static int acpi_thermal_read_domain_devices(struct acpi_thermal *tz) >> +{ >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; >> + union acpi_object *pss; >> + acpi_status status; >> + int ret = 0; >> + int i; >> + >> + status = acpi_evaluate_object(tz->device->handle, "_TZD", NULL, >> + &buffer); >> + if (ACPI_FAILURE(status)) { >> + acpi_evaluation_failure_warn(tz->device->handle, "_TZD", >> + status); >> + return -ENODEV; >> + } >> + >> + pss = buffer.pointer; >> + if (!pss || >> + pss->type != ACPI_TYPE_PACKAGE) { >> + dev_err(&tz->device->dev, "Thermal zone invalid _TZD data\n"); >> + ret = -EFAULT; >> + goto end; >> + } >> + >> + if (!pss->package.count) >> + goto end; >> + >> + tz->domain_devices = kcalloc(pss->package.count, >> + sizeof(struct acpi_device *), GFP_KERNEL); >> + if (!tz->domain_devices) { >> + ret = -ENOMEM; >> + goto end; >> + } >> + >> + tz->holders_dir = kobject_create_and_add("measures", >> + &tz->device->dev.kobj); >> + if (!tz->holders_dir) { >> + ret = -ENOMEM; >> + goto exit_free; >> + } >> + >> + tz->num_domain_devices = pss->package.count; >> + for (i = 0; i < pss->package.count; i++) { >> + struct acpi_device *obj; >> + union acpi_object *element = &pss->package.elements[i]; >> + >> + /* Refuse non-references */ >> + if (element->type != ACPI_TYPE_LOCAL_REFERENCE) >> + continue; >> + >> + /* Create a symlink to domain objects */ >> + obj = acpi_get_acpi_dev(element->reference.handle); >> + tz->domain_devices[i] = obj; >> + if (!obj) >> + continue; >> + >> + ret = sysfs_create_link(tz->holders_dir, &obj->dev.kobj, >> + kobject_name(&obj->dev.kobj)); >> + if (ret) { >> + acpi_dev_put(obj); >> + tz->domain_devices[i] = NULL; >> + } >> + } >> + >> + ret = 0; >> + goto end; >> + >> +exit_free: >> + kfree(tz->domain_devices); >> +end: >> + kfree(buffer.pointer); >> + return ret; >> +} >> + >> static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) >> { >> struct device *tzdev = thermal_zone_device(tz->thermal_zone); >> @@ -602,8 +702,19 @@ static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) >> ret = sysfs_create_link(&tzdev->kobj, >> &tz->device->dev.kobj, "device"); >> if (ret) >> - sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); >> + goto remove_thermal_zone; >> >> + /* _TZD method is optional. */ >> + ret = acpi_thermal_read_domain_devices(tz); >> + if (ret != -ENODEV) >> + goto remove_device; >> + >> + return 0; >> + >> +remove_device: >> + sysfs_remove_link(&tz->device->dev.kobj, "device"); >> +remove_thermal_zone: >> + sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); >> return ret; >> } >> >> @@ -611,6 +722,7 @@ static void acpi_thermal_zone_sysfs_remove(struct acpi_thermal *tz) >> { >> struct device *tzdev = thermal_zone_device(tz->thermal_zone); >> >> + acpi_thermal_remove_domain_devices(tz); >> sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); >> sysfs_remove_link(&tzdev->kobj, "device"); >> } >> -- >> 2.22.0 >> >> > > .
Hi, On Thu, Nov 14, 2024 at 9:37 AM lihuisong (C) <lihuisong@huawei.com> wrote: > > Hi Rafael, > > 在 2024/11/13 17:26, Rafael J. Wysocki 写道: > > On Wed, Nov 13, 2024 at 10:07 AM Huisong Li <lihuisong@huawei.com> wrote: > >> There are many 'cdevX' files which link cooling devices under > >> '/sys/class/thermal/thermal_zoneX/'. These devices contain active cooling > >> devices and passive cooling devices. And user cann't directly know which > >> devices temperature is represented by the thermal zone. > >> > >> However, ACPI spec provides a '_TZD' object which evaluates to a package > >> of device names. Each name corresponds to a device in the ACPI namespace > >> that is associated with the thermal zone. The temperature reported by the > >> thermal zone is roughly correspondent to that of each of the devices. > >> > >> User can get all devices a thermal zone measured by the 'measures' > >> directory under the thermal zone device. > > Well, that's kind of clear, but what exactly is the use case? Why > > does the user need to know that? > IMO, this makes thermal zone information more friendly. > For instance, user can directly know the temperature of CPUs or other > devices is roughly represented by which thermal zone. > This may offer the convenience for further usersapce application. > > BTW, the '_TZD' method is similar to the '_PMD' in acpi power meter. > Since ACPI spec provides them, they should also have a role in their > existence. So there is no specific use case, but it is possible that somebody may want to use this information, IIUC. Well, let's defer making kernel changes until there is a user wanting/needing this information. Then we'll decide how to expose it. For one, I'm not convinced that exposing it under the ACPI representation of a thermal zone is going to be really useful.
在 2024/11/14 19:53, Rafael J. Wysocki 写道: > Hi, > > On Thu, Nov 14, 2024 at 9:37 AM lihuisong (C) <lihuisong@huawei.com> wrote: >> Hi Rafael, >> >> 在 2024/11/13 17:26, Rafael J. Wysocki 写道: >>> On Wed, Nov 13, 2024 at 10:07 AM Huisong Li <lihuisong@huawei.com> wrote: >>>> There are many 'cdevX' files which link cooling devices under >>>> '/sys/class/thermal/thermal_zoneX/'. These devices contain active cooling >>>> devices and passive cooling devices. And user cann't directly know which >>>> devices temperature is represented by the thermal zone. >>>> >>>> However, ACPI spec provides a '_TZD' object which evaluates to a package >>>> of device names. Each name corresponds to a device in the ACPI namespace >>>> that is associated with the thermal zone. The temperature reported by the >>>> thermal zone is roughly correspondent to that of each of the devices. >>>> >>>> User can get all devices a thermal zone measured by the 'measures' >>>> directory under the thermal zone device. >>> Well, that's kind of clear, but what exactly is the use case? Why >>> does the user need to know that? >> IMO, this makes thermal zone information more friendly. >> For instance, user can directly know the temperature of CPUs or other >> devices is roughly represented by which thermal zone. >> This may offer the convenience for further usersapce application. >> >> BTW, the '_TZD' method is similar to the '_PMD' in acpi power meter. >> Since ACPI spec provides them, they should also have a role in their >> existence. > So there is no specific use case, but it is possible that somebody may > want to use this information, IIUC. > > Well, let's defer making kernel changes until there is a user > wanting/needing this information. Then we'll decide how to expose it. > > For one, I'm not convinced that exposing it under the ACPI > representation of a thermal zone is going to be really useful. All right. Thanks for your review. > > .
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index 78db38c7076e..398195a5d42f 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -119,6 +119,9 @@ struct acpi_thermal { struct work_struct thermal_check_work; struct mutex thermal_check_lock; refcount_t thermal_check_count; + int num_domain_devices; + struct acpi_device **domain_devices; + struct kobject *holders_dir; }; /* -------------------------------------------------------------------------- @@ -589,6 +592,103 @@ static const struct thermal_zone_device_ops acpi_thermal_zone_ops = { .critical = acpi_thermal_zone_device_critical, }; +static void acpi_thermal_remove_domain_devices(struct acpi_thermal *tz) +{ + int i; + + if (!tz->num_domain_devices) + return; + + for (i = 0; i < tz->num_domain_devices; i++) { + struct acpi_device *obj = tz->domain_devices[i]; + + if (!obj) + continue; + + sysfs_remove_link(tz->holders_dir, + kobject_name(&obj->dev.kobj)); + acpi_dev_put(obj); + } + + kfree(tz->domain_devices); + kobject_put(tz->holders_dir); + tz->num_domain_devices = 0; +} + +static int acpi_thermal_read_domain_devices(struct acpi_thermal *tz) +{ + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *pss; + acpi_status status; + int ret = 0; + int i; + + status = acpi_evaluate_object(tz->device->handle, "_TZD", NULL, + &buffer); + if (ACPI_FAILURE(status)) { + acpi_evaluation_failure_warn(tz->device->handle, "_TZD", + status); + return -ENODEV; + } + + pss = buffer.pointer; + if (!pss || + pss->type != ACPI_TYPE_PACKAGE) { + dev_err(&tz->device->dev, "Thermal zone invalid _TZD data\n"); + ret = -EFAULT; + goto end; + } + + if (!pss->package.count) + goto end; + + tz->domain_devices = kcalloc(pss->package.count, + sizeof(struct acpi_device *), GFP_KERNEL); + if (!tz->domain_devices) { + ret = -ENOMEM; + goto end; + } + + tz->holders_dir = kobject_create_and_add("measures", + &tz->device->dev.kobj); + if (!tz->holders_dir) { + ret = -ENOMEM; + goto exit_free; + } + + tz->num_domain_devices = pss->package.count; + for (i = 0; i < pss->package.count; i++) { + struct acpi_device *obj; + union acpi_object *element = &pss->package.elements[i]; + + /* Refuse non-references */ + if (element->type != ACPI_TYPE_LOCAL_REFERENCE) + continue; + + /* Create a symlink to domain objects */ + obj = acpi_get_acpi_dev(element->reference.handle); + tz->domain_devices[i] = obj; + if (!obj) + continue; + + ret = sysfs_create_link(tz->holders_dir, &obj->dev.kobj, + kobject_name(&obj->dev.kobj)); + if (ret) { + acpi_dev_put(obj); + tz->domain_devices[i] = NULL; + } + } + + ret = 0; + goto end; + +exit_free: + kfree(tz->domain_devices); +end: + kfree(buffer.pointer); + return ret; +} + static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) { struct device *tzdev = thermal_zone_device(tz->thermal_zone); @@ -602,8 +702,19 @@ static int acpi_thermal_zone_sysfs_add(struct acpi_thermal *tz) ret = sysfs_create_link(&tzdev->kobj, &tz->device->dev.kobj, "device"); if (ret) - sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); + goto remove_thermal_zone; + /* _TZD method is optional. */ + ret = acpi_thermal_read_domain_devices(tz); + if (ret != -ENODEV) + goto remove_device; + + return 0; + +remove_device: + sysfs_remove_link(&tz->device->dev.kobj, "device"); +remove_thermal_zone: + sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); return ret; } @@ -611,6 +722,7 @@ static void acpi_thermal_zone_sysfs_remove(struct acpi_thermal *tz) { struct device *tzdev = thermal_zone_device(tz->thermal_zone); + acpi_thermal_remove_domain_devices(tz); sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone"); sysfs_remove_link(&tzdev->kobj, "device"); }
There are many 'cdevX' files which link cooling devices under '/sys/class/thermal/thermal_zoneX/'. These devices contain active cooling devices and passive cooling devices. And user cann't directly know which devices temperature is represented by the thermal zone. However, ACPI spec provides a '_TZD' object which evaluates to a package of device names. Each name corresponds to a device in the ACPI namespace that is associated with the thermal zone. The temperature reported by the thermal zone is roughly correspondent to that of each of the devices. User can get all devices a thermal zone measured by the 'measures' directory under the thermal zone device. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- v2: fix commitlog based on Rafael's comment. --- drivers/acpi/thermal.c | 114 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-)