2023-02-23 22:49:23

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH v3 00/17] Self-encapsulate the thermal zone device structure

The exported thermal headers expose the thermal core structure while those
should be private to the framework. The initial idea was the thermal sensor
drivers use the thermal zone device structure pointer to pass it around from
the ops to the thermal framework API like a handler.

Unfortunately, different drivers are using and abusing the internals of this
structure to hook the associated struct device, read the internals values, take
the lock, etc ...

rn order to fix this situation, let's encapsulate the structure leaking the
more in the different drivers: the thermal_zone_device structure.

This series revisit the existing drivers using the thermal zone private
structure internals to change the access to something else. For instance, the
get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
is not helpful, we can check the return value for the get_temp() ops in the
call site and show the message in this place.

With this set of changes, the thermal_zone_device is almost self-encapsulated.
As usual, the acpi driver needs a more complex changes, so that will come in a
separate series along with the structure moved the private core headers.

Changelog:
- V3:
- Collected more tags
- Added missing changes for ->devdata in some drivers
- Added a 'type' accessor
- Replaced the 'type' to 'id' changes by the 'type' accessor
- Used the 'type' accessor in the drivers
- V2:
- Collected tags
- Added missing changes for ->devdata for the tsens driver
- Renamed thermal_zone_device_get_data() to thermal_zone_priv()
- Added stubs when CONFIG_THERMAL is not set
- Dropped hwmon change where we remove the tz->lock usage

Thank you all for your comments


Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Zhang Rui <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Damien Le Moal <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: Jean Delvare <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Ido Schimmel <[email protected]>
Cc: Petr Machata <[email protected]>
Cc: Gregory Greenman <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Miquel Raynal <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Amit Kucheria <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Broadcom internal kernel review list <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Markus Mayer <[email protected]>
Cc: Support Opensource <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Thara Gopinath <[email protected]>
Cc: "Niklas Söderlund" <[email protected]>
Cc: Heiko Stuebner <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Alim Akhtar <[email protected]>
Cc: Orson Zhai <[email protected]>
Cc: Baolin Wang <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Cc: Vasily Khoruzhick <[email protected]>
Cc: Yangtao Li <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Jonathan Hunter <[email protected]>
Cc: Talel Shenhar <[email protected]>
Cc: Eduardo Valentin <[email protected]>
Cc: Keerthy <[email protected]>
Cc: Kunihiko Hayashi <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: AngeloGioacchino Del Regno <[email protected]>
Cc: Stefan Wahren <[email protected]>
Cc: Zheng Yongjun <[email protected]>
Cc: Yang Li <[email protected]>
Cc: Srinivas Pandruvada <[email protected]>
Cc: Daniel Golle <[email protected]>
Cc: Balsam CHIHI <[email protected]>
Cc: Mikko Perttunen <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Daniel Lezcano (17):
thermal/core: Add a thermal zone 'devdata' accessor
thermal/core: Show a debug message when get_temp() fails
thermal: Remove debug or error messages in get_temp() ops
thermal/hwmon: Do not set no_hwmon before calling
thermal_add_hwmon_sysfs()
thermal/hwmon: Use the right device for devm_thermal_add_hwmon_sysfs()
thermal: Don't use 'device' internal thermal zone structure field
thermal/core: Add 'type' accessor
thermal/drivers/spear: Don't use tz->device but pdev->dev
thermal: Add a thermal zone id accessor
thermal: Use thermal_zone_device_type() accessor
thermal/drivers/da9062: Don't access the thermal zone device fields
thermal/hwmon: Use the thermal_core.h header
thermal/drivers/tegra: Remove unneeded lock when setting a trip point
thermal/tegra: Do not enable the thermal zone, it is already enabled
thermal/drivers/acerhdf: Make interval setting only at module load
time
thermal/drivers/acerhdf: Remove pointless governor test
thermal/traces: Replace the thermal zone structure parameter with the
field value

drivers/acpi/thermal.c | 18 +++----
drivers/ata/ahci_imx.c | 2 +-
drivers/hwmon/hwmon.c | 4 +-
drivers/hwmon/pmbus/pmbus_core.c | 2 +-
drivers/hwmon/scmi-hwmon.c | 4 +-
drivers/hwmon/scpi-hwmon.c | 2 +-
drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
drivers/input/touchscreen/sun4i-ts.c | 2 +-
.../ethernet/chelsio/cxgb4/cxgb4_thermal.c | 2 +-
.../ethernet/mellanox/mlxsw/core_thermal.c | 16 +++----
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 4 +-
drivers/platform/x86/acerhdf.c | 19 ++------
drivers/power/supply/power_supply_core.c | 2 +-
drivers/regulator/max8973-regulator.c | 2 +-
drivers/thermal/amlogic_thermal.c | 4 +-
drivers/thermal/armada_thermal.c | 14 ++----
drivers/thermal/broadcom/bcm2711_thermal.c | 3 +-
drivers/thermal/broadcom/bcm2835_thermal.c | 3 +-
drivers/thermal/broadcom/brcmstb_thermal.c | 8 ++--
drivers/thermal/broadcom/ns-thermal.c | 2 +-
drivers/thermal/broadcom/sr-thermal.c | 2 +-
drivers/thermal/da9062-thermal.c | 13 +++--
drivers/thermal/db8500_thermal.c | 2 +-
drivers/thermal/dove_thermal.c | 7 +--
drivers/thermal/gov_fair_share.c | 4 +-
drivers/thermal/gov_power_allocator.c | 6 ++-
drivers/thermal/gov_step_wise.c | 4 +-
drivers/thermal/hisi_thermal.c | 5 +-
drivers/thermal/imx8mm_thermal.c | 4 +-
drivers/thermal/imx_sc_thermal.c | 9 ++--
drivers/thermal/imx_thermal.c | 47 +++++--------------
.../intel/int340x_thermal/int3400_thermal.c | 2 +-
.../int340x_thermal/int340x_thermal_zone.c | 4 +-
.../processor_thermal_device_pci.c | 4 +-
drivers/thermal/intel/intel_pch_thermal.c | 2 +-
.../thermal/intel/intel_quark_dts_thermal.c | 6 +--
drivers/thermal/intel/intel_soc_dts_iosf.c | 13 ++---
drivers/thermal/intel/x86_pkg_temp_thermal.c | 4 +-
drivers/thermal/k3_bandgap.c | 4 +-
drivers/thermal/k3_j72xx_bandgap.c | 2 +-
drivers/thermal/kirkwood_thermal.c | 7 +--
drivers/thermal/max77620_thermal.c | 6 +--
drivers/thermal/mediatek/auxadc_thermal.c | 4 +-
drivers/thermal/mediatek/lvts_thermal.c | 10 ++--
drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 6 +--
drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 6 +--
drivers/thermal/qcom/tsens.c | 6 +--
drivers/thermal/qoriq_thermal.c | 4 +-
drivers/thermal/rcar_gen3_thermal.c | 5 +-
drivers/thermal/rcar_thermal.c | 8 +---
drivers/thermal/rockchip_thermal.c | 8 +---
drivers/thermal/rzg2l_thermal.c | 3 +-
drivers/thermal/samsung/exynos_tmu.c | 4 +-
drivers/thermal/spear_thermal.c | 10 ++--
drivers/thermal/sprd_thermal.c | 2 +-
drivers/thermal/st/st_thermal.c | 4 +-
drivers/thermal/st/stm_thermal.c | 4 +-
drivers/thermal/sun8i_thermal.c | 4 +-
drivers/thermal/tegra/soctherm.c | 6 +--
drivers/thermal/tegra/tegra-bpmp-thermal.c | 6 ++-
drivers/thermal/tegra/tegra30-tsensor.c | 31 ++++++------
drivers/thermal/thermal-generic-adc.c | 7 ++-
drivers/thermal/thermal_core.c | 26 +++++++++-
drivers/thermal/thermal_helpers.c | 3 ++
drivers/thermal/thermal_hwmon.c | 9 ++--
drivers/thermal/thermal_hwmon.h | 4 +-
drivers/thermal/thermal_mmio.c | 2 +-
.../ti-soc-thermal/ti-thermal-common.c | 10 ++--
drivers/thermal/uniphier_thermal.c | 2 +-
include/linux/thermal.h | 19 ++++++++
include/trace/events/thermal.h | 24 +++++-----
.../trace/events/thermal_power_allocator.h | 12 ++---
72 files changed, 251 insertions(+), 270 deletions(-)

--
2.34.1



2023-02-23 22:56:59

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v3 00/17] Self-encapsulate the thermal zone device structure

On 23/02/2023 23:48, Daniel Lezcano wrote:
> The exported thermal headers expose the thermal core structure while those
> should be private to the framework. The initial idea was the thermal sensor
> drivers use the thermal zone device structure pointer to pass it around from
> the ops to the thermal framework API like a handler.
>
> Unfortunately, different drivers are using and abusing the internals of this
> structure to hook the associated struct device, read the internals values, take
> the lock, etc ...
>
> rn order to fix this situation, let's encapsulate the structure leaking the
> more in the different drivers: the thermal_zone_device structure.
>
> This series revisit the existing drivers using the thermal zone private
> structure internals to change the access to something else. For instance, the
> get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
> is not helpful, we can check the return value for the get_temp() ops in the
> call site and show the message in this place.
>
> With this set of changes, the thermal_zone_device is almost self-encapsulated.
> As usual, the acpi driver needs a more complex changes, so that will come in a
> separate series along with the structure moved the private core headers.
>
> Changelog:
> - V3:
> - Collected more tags
> - Added missing changes for ->devdata in some drivers
> - Added a 'type' accessor
> - Replaced the 'type' to 'id' changes by the 'type' accessor
> - Used the 'type' accessor in the drivers
> - V2:
> - Collected tags
> - Added missing changes for ->devdata for the tsens driver
> - Renamed thermal_zone_device_get_data() to thermal_zone_priv()
> - Added stubs when CONFIG_THERMAL is not set
> - Dropped hwmon change where we remove the tz->lock usage
>
> Thank you all for your comments

The series has been blocked by gsmtp because the next patch has too many
Cc. I'll sort out this and resend.


--
<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