| Message ID | 20230410205305.1649678-1-daniel.lezcano@linaro.org |
|---|---|
| Headers | show |
| Series | Thermal zone device structure encapsulation | expand |
On 11/04/2023 20:26, Rafael J. Wysocki wrote: > On Mon, Apr 10, 2023 at 10:53 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> The ACPI thermal driver creates a link in the thermal zone device >> sysfs directory pointing to the device sysfs directory. At the same >> time, it creates a back pointer link from the device to the thermal >> zone device sysfs directory. >> >> From a generic perspective, having a device pointer in the sysfs >> thermal zone directory may make sense. But the opposite is not true as >> the same driver can be related to multiple thermal zones. >> >> The usage of these information is very specific to ACPI and it is >> questionable if they are really needed. >> >> Let's make the code optional and disable it by default. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/acpi/thermal.c | 62 ++++++++++++++++++++++++++++-------------- >> 1 file changed, 42 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c >> index 5763db4528b8..70f1d28810f2 100644 >> --- a/drivers/acpi/thermal.c >> +++ b/drivers/acpi/thermal.c >> @@ -787,9 +787,44 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = { >> .critical = acpi_thermal_zone_device_critical, >> }; >> >> +#ifdef CONFIG_THERMAL_SYSFS_OBSOLETE_SINGULARITY > > It is OK to move the code to the separate functions below, but it is > not OK to make it depend on the Kconfig option above. > > The extra sysfs things were added in different drivers for different > reasons. Making them all depend on one Kconfig option is just wrong. Ok, I'll do the changes accordingly. Thanks for reviewing the series [ ... ]
The thermal zone device structure is defined in the exported thermal header include/linux/thermal.h Given the definition being public, the structure is exposed to the external components other than the thermal framework core code. It results the drivers are tampering the structure internals like taking the lock or changing the field values. Obviously that is bad for several reasons as the drivers can hook the thermal framework behavior and makes very difficult the changes in the core code as external components depend on it directly. Moreover, the thermal trip points being reworked, we don't want the drivers to access the trips array directly in the thermal zone structure and doing assumptions on how they are organized. This series provides a second set of changes moving to the thermal zone device structure self-encapsulation. The ACPI and the Menlon drivers are using the thermal zone's device fields to create symlinks and new attributes in the sysfs thermal zone directory. These changes provide a hopefully temporary wrapper to access it in order to allow moving forward in the thermal zone device self-encapsulation and a Kconfig option to disable by default such a extra sysfs information. Changelog: v2: - Add the Kconfig option to remove specific attributes - Add a thermal_zone_device() wrapper to access tz->device Daniel Lezcano (7): thermal/drivers/intel_pch_thermal: Use thermal driver device to write a trace thermal/core: Encapsulate tz->device field thermal/drivers/acpi: Use thermal_zone_device() thermal/drivers/menlow: Use thermal_zone_device() thermal/core: Prepare sanitizing thermal class sysfs content thermal/drivers/acpi: Make cross dev link optional by configuration thermal/drivers/intel_menlow: Make additionnal sysfs information optional drivers/acpi/thermal.c | 57 +++++++++++++++++------ drivers/thermal/Kconfig | 12 +++++ drivers/thermal/intel/intel_menlow.c | 12 +++-- drivers/thermal/intel/intel_pch_thermal.c | 5 +- drivers/thermal/thermal_core.c | 6 +++ include/linux/thermal.h | 1 + 6 files changed, 74 insertions(+), 19 deletions(-)