[PATCH v7 00/11] Stop monitoring disabled devices

[PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

A respin of v6 with these changes:

v6..v7:
- removed duplicate S-o-b line from patch 6/11

v5..v6:
- staticized thermal_zone_device_set_mode() (kbuild test robot)

v4..v5:

- EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL (Daniel)
- dropped unnecessary thermal_zone_device_enable() in int3400_thermal.c
and in thermal_of.c (Bartlomiej)

Andrzej Pietrasiewicz (11):
  acpi: thermal: Fix error handling in the register function
  thermal: Store thermal mode in a dedicated enum
  thermal: Add current mode to thermal zone device
  thermal: Store device mode in struct thermal_zone_device
  thermal: remove get_mode() operation of drivers
  thermal: Add mode helpers
  thermal: Use mode helpers in drivers
  thermal: Explicitly enable non-changing thermal zone devices
  thermal: core: Stop polling DISABLED thermal devices
  thermal: Simplify or eliminate unnecessary set_mode() methods
  thermal: Rename set_mode() to change_mode()

 drivers/acpi/thermal.c                        | 75 +++++----------
 .../ethernet/chelsio/cxgb4/cxgb4_thermal.c    |  8 ++
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 91 ++++---------------
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c   |  9 +-
 drivers/platform/x86/acerhdf.c                | 33 +++----
 drivers/platform/x86/intel_mid_thermal.c      |  6 ++
 drivers/power/supply/power_supply_core.c      |  9 +-
 drivers/thermal/armada_thermal.c              |  6 ++
 drivers/thermal/da9062-thermal.c              | 16 +---
 drivers/thermal/dove_thermal.c                |  6 ++
 drivers/thermal/hisi_thermal.c                |  6 +-
 drivers/thermal/imx_thermal.c                 | 57 ++++--------
 .../intel/int340x_thermal/int3400_thermal.c   | 38 ++------
 .../int340x_thermal/int340x_thermal_zone.c    |  5 +
 drivers/thermal/intel/intel_pch_thermal.c     |  5 +
 .../thermal/intel/intel_quark_dts_thermal.c   | 34 ++-----
 drivers/thermal/intel/intel_soc_dts_iosf.c    |  3 +
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  6 ++
 drivers/thermal/kirkwood_thermal.c            |  7 ++
 drivers/thermal/rcar_thermal.c                |  9 +-
 drivers/thermal/rockchip_thermal.c            |  6 +-
 drivers/thermal/spear_thermal.c               |  7 ++
 drivers/thermal/sprd_thermal.c                |  6 +-
 drivers/thermal/st/st_thermal.c               |  5 +
 drivers/thermal/thermal_core.c                | 76 ++++++++++++++--
 drivers/thermal/thermal_of.c                  | 41 +--------
 drivers/thermal/thermal_sysfs.c               | 37 +-------
 include/linux/thermal.h                       | 19 +++-
 28 files changed, 275 insertions(+), 351 deletions(-)


base-commit: 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
-- 
2.17.1

[PATCH v7 01/11] acpi: thermal: Fix error handling in the register function

[Andrzej Pietrasiewicz]

The acpi_thermal_register_thermal_zone() is missing any error handling.
This needs to be fixed.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 drivers/acpi/thermal.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 19067a5e5293..6de8066ca1e7 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -901,23 +901,35 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 	result = sysfs_create_link(&tz->device->dev.kobj,
 				   &tz->thermal_zone->device.kobj, "thermal_zone");
 	if (result)
-		return result;
+		goto unregister_tzd;
 
 	result = sysfs_create_link(&tz->thermal_zone->device.kobj,
 				   &tz->device->dev.kobj, "device");
 	if (result)
-		return result;
+		goto remove_tz_link;
 
 	status =  acpi_bus_attach_private_data(tz->device->handle,
 					       tz->thermal_zone);
-	if (ACPI_FAILURE(status))
-		return -ENODEV;
+	if (ACPI_FAILURE(status)) {
+		result = -ENODEV;
+		goto remove_dev_link;
+	}
 
 	tz->tz_enabled = 1;
 
 	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
 		 tz->thermal_zone->id);
+
 	return 0;
+
+remove_dev_link:
+	sysfs_remove_link(&tz->thermal_zone->device.kobj, "device");
+remove_tz_link:
+	sysfs_remove_link(&tz->device->dev.kobj, "thermal_zone");
+unregister_tzd:
+	thermal_zone_device_unregister(tz->thermal_zone);
+
+	return result;
 }
 
 static void acpi_thermal_unregister_thermal_zone(struct acpi_thermal *tz)
-- 
2.17.1

[PATCH v7 02/11] thermal: Store thermal mode in a dedicated enum

[Andrzej Pietrasiewicz]

Prepare for storing mode in struct thermal_zone_device.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 drivers/acpi/thermal.c                        | 27 +++++++++----------
 drivers/platform/x86/acerhdf.c                |  8 ++++--
 .../intel/int340x_thermal/int3400_thermal.c   | 18 +++++--------
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 6de8066ca1e7..fb46070c66d8 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -172,7 +172,7 @@ struct acpi_thermal {
 	struct acpi_thermal_trips trips;
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
-	int tz_enabled;
+	enum thermal_device_mode mode;
 	int kelvin_offset;	/* in millidegrees */
 	struct work_struct thermal_check_work;
 };
@@ -500,7 +500,7 @@ static void acpi_thermal_check(void *data)
 {
 	struct acpi_thermal *tz = data;
 
-	if (!tz->tz_enabled)
+	if (tz->mode != THERMAL_DEVICE_ENABLED)
 		return;
 
 	thermal_zone_device_update(tz->thermal_zone,
@@ -534,8 +534,7 @@ static int thermal_get_mode(struct thermal_zone_device *thermal,
 	if (!tz)
 		return -EINVAL;
 
-	*mode = tz->tz_enabled ? THERMAL_DEVICE_ENABLED :
-		THERMAL_DEVICE_DISABLED;
+	*mode = tz->mode;
 
 	return 0;
 }
@@ -544,27 +543,25 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode mode)
 {
 	struct acpi_thermal *tz = thermal->devdata;
-	int enable;
 
 	if (!tz)
 		return -EINVAL;
 
+	if (mode != THERMAL_DEVICE_DISABLED &&
+	    mode != THERMAL_DEVICE_ENABLED)
+		return -EINVAL;
 	/*
 	 * enable/disable thermal management from ACPI thermal driver
 	 */
-	if (mode == THERMAL_DEVICE_ENABLED)
-		enable = 1;
-	else if (mode == THERMAL_DEVICE_DISABLED) {
-		enable = 0;
+	if (mode == THERMAL_DEVICE_DISABLED)
 		pr_warn("thermal zone will be disabled\n");
-	} else
-		return -EINVAL;
 
-	if (enable != tz->tz_enabled) {
-		tz->tz_enabled = enable;
+	if (mode != tz->mode) {
+		tz->mode = mode;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			"%s kernel ACPI thermal control\n",
-			tz->tz_enabled ? "Enable" : "Disable"));
+			tz->mode == THERMAL_DEVICE_ENABLED ?
+			"Enable" : "Disable"));
 		acpi_thermal_check(tz);
 	}
 	return 0;
@@ -915,7 +912,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 		goto remove_dev_link;
 	}
 
-	tz->tz_enabled = 1;
+	tz->mode = THERMAL_DEVICE_ENABLED;
 
 	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
 		 tz->thermal_zone->id);
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 4df7609b4aa9..9d1030b1a4f4 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -68,6 +68,7 @@ static int kernelmode = 1;
 #else
 static int kernelmode;
 #endif
+static enum thermal_device_mode thermal_mode;
 
 static unsigned int interval = 10;
 static unsigned int fanon = 60000;
@@ -397,6 +398,7 @@ static inline void acerhdf_revert_to_bios_mode(void)
 {
 	acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
 	kernelmode = 0;
+	thermal_mode = THERMAL_DEVICE_DISABLED;
 	if (thz_dev)
 		thz_dev->polling_delay = 0;
 	pr_notice("kernel mode fan control OFF\n");
@@ -404,6 +406,7 @@ static inline void acerhdf_revert_to_bios_mode(void)
 static inline void acerhdf_enable_kernelmode(void)
 {
 	kernelmode = 1;
+	thermal_mode = THERMAL_DEVICE_ENABLED;
 
 	thz_dev->polling_delay = interval*1000;
 	thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
@@ -416,8 +419,7 @@ static int acerhdf_get_mode(struct thermal_zone_device *thermal,
 	if (verbose)
 		pr_notice("kernel mode fan control %d\n", kernelmode);
 
-	*mode = (kernelmode) ? THERMAL_DEVICE_ENABLED
-			     : THERMAL_DEVICE_DISABLED;
+	*mode = thermal_mode;
 
 	return 0;
 }
@@ -739,6 +741,8 @@ static int __init acerhdf_register_thermal(void)
 	if (IS_ERR(cl_dev))
 		return -EINVAL;
 
+	thermal_mode = kernelmode ?
+		THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
 	thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
 					      &acerhdf_dev_ops,
 					      &acerhdf_zone_params, 0,
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 0b3a62655843..e84faaadff87 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -48,7 +48,7 @@ struct int3400_thermal_priv {
 	struct acpi_device *adev;
 	struct platform_device *pdev;
 	struct thermal_zone_device *thermal;
-	int mode;
+	enum thermal_device_mode mode;
 	int art_count;
 	struct art *arts;
 	int trt_count;
@@ -395,24 +395,20 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode mode)
 {
 	struct int3400_thermal_priv *priv = thermal->devdata;
-	bool enable;
 	int result = 0;
 
 	if (!priv)
 		return -EINVAL;
 
-	if (mode == THERMAL_DEVICE_ENABLED)
-		enable = true;
-	else if (mode == THERMAL_DEVICE_DISABLED)
-		enable = false;
-	else
+	if (mode != THERMAL_DEVICE_ENABLED &&
+	    mode != THERMAL_DEVICE_DISABLED)
 		return -EINVAL;
 
-	if (enable != priv->mode) {
-		priv->mode = enable;
+	if (mode != priv->mode) {
+		priv->mode = mode;
 		result = int3400_thermal_run_osc(priv->adev->handle,
-						 priv->current_uuid_index,
-						 enable);
+						priv->current_uuid_index,
+						mode == THERMAL_DEVICE_ENABLED);
 	}
 
 	evaluate_odvp(priv);
-- 
2.17.1

[PATCH v7 03/11] thermal: Add current mode to thermal zone device

[Andrzej Pietrasiewicz]

Prepare for changing the place where the mode is stored: now it is in
drivers, which might or might not implement get_mode()/set_mode() methods.
A lot of cleanup can be done thanks to storing it in struct tzd. The
get_mode() methods will become redundant.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 include/linux/thermal.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 216185bb3014..5f91d7f04512 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -128,6 +128,7 @@ struct thermal_cooling_device {
  * @trip_temp_attrs:	attributes for trip points for sysfs: trip temperature
  * @trip_type_attrs:	attributes for trip points for sysfs: trip type
  * @trip_hyst_attrs:	attributes for trip points for sysfs: trip hysteresis
+ * @mode:		current mode of this thermal zone
  * @devdata:	private pointer for device private data
  * @trips:	number of trip points the thermal zone supports
  * @trips_disabled;	bitmap for disabled trips
@@ -170,6 +171,7 @@ struct thermal_zone_device {
 	struct thermal_attr *trip_temp_attrs;
 	struct thermal_attr *trip_type_attrs;
 	struct thermal_attr *trip_hyst_attrs;
+	enum thermal_device_mode mode;
 	void *devdata;
 	int trips;
 	unsigned long trips_disabled;	/* bitmap for disabled trips */
-- 
2.17.1

[PATCH v7 04/11] thermal: Store device mode in struct thermal_zone_device

[Andrzej Pietrasiewicz]

Prepare for eliminating get_mode().

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 drivers/acpi/thermal.c                        | 18 ++++++----------
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 21 +++++++------------
 drivers/platform/x86/acerhdf.c                | 15 ++++++-------
 drivers/thermal/da9062-thermal.c              |  6 ++----
 drivers/thermal/imx_thermal.c                 | 17 +++++++--------
 .../intel/int340x_thermal/int3400_thermal.c   | 12 +++--------
 .../thermal/intel/intel_quark_dts_thermal.c   | 16 +++++++-------
 drivers/thermal/thermal_of.c                  | 10 +++------
 8 files changed, 44 insertions(+), 71 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index fb46070c66d8..4ba273f49d87 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -172,7 +172,6 @@ struct acpi_thermal {
 	struct acpi_thermal_trips trips;
 	struct acpi_handle_list devices;
 	struct thermal_zone_device *thermal_zone;
-	enum thermal_device_mode mode;
 	int kelvin_offset;	/* in millidegrees */
 	struct work_struct thermal_check_work;
 };
@@ -500,7 +499,7 @@ static void acpi_thermal_check(void *data)
 {
 	struct acpi_thermal *tz = data;
 
-	if (tz->mode != THERMAL_DEVICE_ENABLED)
+	if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
 		return;
 
 	thermal_zone_device_update(tz->thermal_zone,
@@ -529,12 +528,7 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 static int thermal_get_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode *mode)
 {
-	struct acpi_thermal *tz = thermal->devdata;
-
-	if (!tz)
-		return -EINVAL;
-
-	*mode = tz->mode;
+	*mode = thermal->mode;
 
 	return 0;
 }
@@ -556,11 +550,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
 	if (mode == THERMAL_DEVICE_DISABLED)
 		pr_warn("thermal zone will be disabled\n");
 
-	if (mode != tz->mode) {
-		tz->mode = mode;
+	if (mode != tz->thermal_zone->mode) {
+		tz->thermal_zone->mode = mode;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			"%s kernel ACPI thermal control\n",
-			tz->mode == THERMAL_DEVICE_ENABLED ?
+			tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
 			"Enable" : "Disable"));
 		acpi_thermal_check(tz);
 	}
@@ -912,7 +906,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 		goto remove_dev_link;
 	}
 
-	tz->mode = THERMAL_DEVICE_ENABLED;
+	tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
 
 	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
 		 tz->thermal_zone->id);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 05f8d5a92862..51667ed99c21 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -98,7 +98,6 @@ struct mlxsw_thermal_module {
 	struct mlxsw_thermal *parent;
 	struct thermal_zone_device *tzdev;
 	struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
-	enum thermal_device_mode mode;
 	int module; /* Module or gearbox number */
 };
 
@@ -110,7 +109,6 @@ struct mlxsw_thermal {
 	struct thermal_cooling_device *cdevs[MLXSW_MFCR_PWMS_MAX];
 	u8 cooling_levels[MLXSW_THERMAL_MAX_STATE + 1];
 	struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
-	enum thermal_device_mode mode;
 	struct mlxsw_thermal_module *tz_module_arr;
 	u8 tz_module_num;
 	struct mlxsw_thermal_module *tz_gearbox_arr;
@@ -280,9 +278,7 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
 				  enum thermal_device_mode *mode)
 {
-	struct mlxsw_thermal *thermal = tzdev->devdata;
-
-	*mode = thermal->mode;
+	*mode = tzdev->mode;
 
 	return 0;
 }
@@ -299,9 +295,9 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
 	else
 		tzdev->polling_delay = 0;
 
+	tzdev->mode = mode;
 	mutex_unlock(&tzdev->lock);
 
-	thermal->mode = mode;
 	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
 
 	return 0;
@@ -468,9 +464,7 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
 					 enum thermal_device_mode *mode)
 {
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
-
-	*mode = tz->mode;
+	*mode = tzdev->mode;
 
 	return 0;
 }
@@ -488,9 +482,10 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
 	else
 		tzdev->polling_delay = 0;
 
+	tzdev->mode = mode;
+
 	mutex_unlock(&tzdev->lock);
 
-	tz->mode = mode;
 	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
 
 	return 0;
@@ -780,7 +775,7 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
 		return err;
 	}
 
-	module_tz->mode = THERMAL_DEVICE_ENABLED;
+	module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
 	return 0;
 }
 
@@ -896,7 +891,7 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
 	if (IS_ERR(gearbox_tz->tzdev))
 		return PTR_ERR(gearbox_tz->tzdev);
 
-	gearbox_tz->mode = THERMAL_DEVICE_ENABLED;
+	gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
 	return 0;
 }
 
@@ -1065,7 +1060,7 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
 	if (err)
 		goto err_unreg_modules_tzdev;
 
-	thermal->mode = THERMAL_DEVICE_ENABLED;
+	thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
 	*p_thermal = thermal;
 	return 0;
 
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 9d1030b1a4f4..6f21015e5fd9 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -68,7 +68,6 @@ static int kernelmode = 1;
 #else
 static int kernelmode;
 #endif
-static enum thermal_device_mode thermal_mode;
 
 static unsigned int interval = 10;
 static unsigned int fanon = 60000;
@@ -398,15 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
 {
 	acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
 	kernelmode = 0;
-	thermal_mode = THERMAL_DEVICE_DISABLED;
-	if (thz_dev)
+	if (thz_dev) {
+		thz_dev->mode = THERMAL_DEVICE_DISABLED;
 		thz_dev->polling_delay = 0;
+	}
 	pr_notice("kernel mode fan control OFF\n");
 }
 static inline void acerhdf_enable_kernelmode(void)
 {
 	kernelmode = 1;
-	thermal_mode = THERMAL_DEVICE_ENABLED;
+	thz_dev->mode = THERMAL_DEVICE_ENABLED;
 
 	thz_dev->polling_delay = interval*1000;
 	thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
@@ -419,7 +419,7 @@ static int acerhdf_get_mode(struct thermal_zone_device *thermal,
 	if (verbose)
 		pr_notice("kernel mode fan control %d\n", kernelmode);
 
-	*mode = thermal_mode;
+	*mode = thermal->mode;
 
 	return 0;
 }
@@ -741,8 +741,6 @@ static int __init acerhdf_register_thermal(void)
 	if (IS_ERR(cl_dev))
 		return -EINVAL;
 
-	thermal_mode = kernelmode ?
-		THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
 	thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
 					      &acerhdf_dev_ops,
 					      &acerhdf_zone_params, 0,
@@ -750,6 +748,9 @@ static int __init acerhdf_register_thermal(void)
 	if (IS_ERR(thz_dev))
 		return -EINVAL;
 
+	thz_dev->mode = kernelmode ?
+		THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
+
 	if (strcmp(thz_dev->governor->name,
 				acerhdf_zone_params.governor_name)) {
 		pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
index c32709badeda..a14c7981c7c7 100644
--- a/drivers/thermal/da9062-thermal.c
+++ b/drivers/thermal/da9062-thermal.c
@@ -49,7 +49,6 @@ struct da9062_thermal {
 	struct da9062 *hw;
 	struct delayed_work work;
 	struct thermal_zone_device *zone;
-	enum thermal_device_mode mode;
 	struct mutex lock; /* protection for da9062_thermal temperature */
 	int temperature;
 	int irq;
@@ -124,8 +123,7 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
 static int da9062_thermal_get_mode(struct thermal_zone_device *z,
 				   enum thermal_device_mode *mode)
 {
-	struct da9062_thermal *thermal = z->devdata;
-	*mode = thermal->mode;
+	*mode = z->mode;
 	return 0;
 }
 
@@ -233,7 +231,6 @@ static int da9062_thermal_probe(struct platform_device *pdev)
 
 	thermal->config = match->data;
 	thermal->hw = chip;
-	thermal->mode = THERMAL_DEVICE_ENABLED;
 	thermal->dev = &pdev->dev;
 
 	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
@@ -248,6 +245,7 @@ static int da9062_thermal_probe(struct platform_device *pdev)
 		ret = PTR_ERR(thermal->zone);
 		goto err;
 	}
+	thermal->zone->mode = THERMAL_DEVICE_ENABLED;
 
 	dev_dbg(&pdev->dev,
 		"TJUNC temperature polling period set at %d ms\n",
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index e761c9b42217..9a1114d721b6 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -197,7 +197,6 @@ struct imx_thermal_data {
 	struct cpufreq_policy *policy;
 	struct thermal_zone_device *tz;
 	struct thermal_cooling_device *cdev;
-	enum thermal_device_mode mode;
 	struct regmap *tempmon;
 	u32 c1, c2; /* See formula in imx_init_calib() */
 	int temp_passive;
@@ -256,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 	bool wait;
 	u32 val;
 
-	if (data->mode == THERMAL_DEVICE_ENABLED) {
+	if (tz->mode == THERMAL_DEVICE_ENABLED) {
 		/* Check if a measurement is currently in progress */
 		regmap_read(map, soc_data->temp_data, &val);
 		wait = !(val & soc_data->temp_valid_mask);
@@ -283,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	regmap_read(map, soc_data->temp_data, &val);
 
-	if (data->mode != THERMAL_DEVICE_ENABLED) {
+	if (tz->mode != THERMAL_DEVICE_ENABLED) {
 		regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
 			     soc_data->measure_temp_mask);
 		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
@@ -334,9 +333,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 static int imx_get_mode(struct thermal_zone_device *tz,
 			enum thermal_device_mode *mode)
 {
-	struct imx_thermal_data *data = tz->devdata;
-
-	*mode = data->mode;
+	*mode = tz->mode;
 
 	return 0;
 }
@@ -376,7 +373,7 @@ static int imx_set_mode(struct thermal_zone_device *tz,
 		}
 	}
 
-	data->mode = mode;
+	tz->mode = mode;
 	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	return 0;
@@ -831,7 +828,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
 		     data->socdata->measure_temp_mask);
 
 	data->irq_enabled = true;
-	data->mode = THERMAL_DEVICE_ENABLED;
+	data->tz->mode = THERMAL_DEVICE_ENABLED;
 
 	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
 			imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
@@ -885,7 +882,7 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
 		     data->socdata->measure_temp_mask);
 	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
 		     data->socdata->power_down_mask);
-	data->mode = THERMAL_DEVICE_DISABLED;
+	data->tz->mode = THERMAL_DEVICE_DISABLED;
 	clk_disable_unprepare(data->thermal_clk);
 
 	return 0;
@@ -905,7 +902,7 @@ static int __maybe_unused imx_thermal_resume(struct device *dev)
 		     data->socdata->power_down_mask);
 	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
 		     data->socdata->measure_temp_mask);
-	data->mode = THERMAL_DEVICE_ENABLED;
+	data->tz->mode = THERMAL_DEVICE_ENABLED;
 
 	return 0;
 }
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index e84faaadff87..f65b2fc09198 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -48,7 +48,6 @@ struct int3400_thermal_priv {
 	struct acpi_device *adev;
 	struct platform_device *pdev;
 	struct thermal_zone_device *thermal;
-	enum thermal_device_mode mode;
 	int art_count;
 	struct art *arts;
 	int trt_count;
@@ -381,12 +380,7 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
 static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode *mode)
 {
-	struct int3400_thermal_priv *priv = thermal->devdata;
-
-	if (!priv)
-		return -EINVAL;
-
-	*mode = priv->mode;
+	*mode = thermal->mode;
 
 	return 0;
 }
@@ -404,8 +398,8 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 	    mode != THERMAL_DEVICE_DISABLED)
 		return -EINVAL;
 
-	if (mode != priv->mode) {
-		priv->mode = mode;
+	if (mode != thermal->mode) {
+		thermal->mode = mode;
 		result = int3400_thermal_run_osc(priv->adev->handle,
 						priv->current_uuid_index,
 						mode == THERMAL_DEVICE_ENABLED);
diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
index d704fc104cfd..d77cb3df5ade 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -103,7 +103,6 @@ struct soc_sensor_entry {
 	bool locked;
 	u32 store_ptps;
 	u32 store_dts_enable;
-	enum thermal_device_mode mode;
 	struct thermal_zone_device *tzone;
 };
 
@@ -128,7 +127,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
 		return ret;
 
 	if (out & QRK_DTS_ENABLE_BIT) {
-		aux_entry->mode = THERMAL_DEVICE_ENABLED;
+		tzd->mode = THERMAL_DEVICE_ENABLED;
 		return 0;
 	}
 
@@ -139,9 +138,9 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
 		if (ret)
 			return ret;
 
-		aux_entry->mode = THERMAL_DEVICE_ENABLED;
+		tzd->mode = THERMAL_DEVICE_ENABLED;
 	} else {
-		aux_entry->mode = THERMAL_DEVICE_DISABLED;
+		tzd->mode = THERMAL_DEVICE_DISABLED;
 		pr_info("DTS is locked. Cannot enable DTS\n");
 		ret = -EPERM;
 	}
@@ -161,7 +160,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
 		return ret;
 
 	if (!(out & QRK_DTS_ENABLE_BIT)) {
-		aux_entry->mode = THERMAL_DEVICE_DISABLED;
+		tzd->mode = THERMAL_DEVICE_DISABLED;
 		return 0;
 	}
 
@@ -173,9 +172,9 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
 		if (ret)
 			return ret;
 
-		aux_entry->mode = THERMAL_DEVICE_DISABLED;
+		tzd->mode = THERMAL_DEVICE_DISABLED;
 	} else {
-		aux_entry->mode = THERMAL_DEVICE_ENABLED;
+		tzd->mode = THERMAL_DEVICE_ENABLED;
 		pr_info("DTS is locked. Cannot disable DTS\n");
 		ret = -EPERM;
 	}
@@ -312,8 +311,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
 static int sys_get_mode(struct thermal_zone_device *tzd,
 				enum thermal_device_mode *mode)
 {
-	struct soc_sensor_entry *aux_entry = tzd->devdata;
-	*mode = aux_entry->mode;
+	*mode = tzd->mode;
 	return 0;
 }
 
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index ddf88dbe7ba2..c495b1e48ef2 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -51,7 +51,6 @@ struct __thermal_bind_params {
 
 /**
  * struct __thermal_zone - internal representation of a thermal zone
- * @mode: current thermal zone device mode (enabled/disabled)
  * @passive_delay: polling interval while passive cooling is activated
  * @polling_delay: zone polling interval
  * @slope: slope of the temperature adjustment curve
@@ -65,7 +64,6 @@ struct __thermal_bind_params {
  */
 
 struct __thermal_zone {
-	enum thermal_device_mode mode;
 	int passive_delay;
 	int polling_delay;
 	int slope;
@@ -272,9 +270,7 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 static int of_thermal_get_mode(struct thermal_zone_device *tz,
 			       enum thermal_device_mode *mode)
 {
-	struct __thermal_zone *data = tz->devdata;
-
-	*mode = data->mode;
+	*mode = tz->mode;
 
 	return 0;
 }
@@ -296,7 +292,7 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
 
 	mutex_unlock(&tz->lock);
 
-	data->mode = mode;
+	tz->mode = mode;
 	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	return 0;
@@ -979,7 +975,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 
 finish:
 	of_node_put(child);
-	tz->mode = THERMAL_DEVICE_DISABLED;
 
 	return tz;
 
@@ -1134,6 +1129,7 @@ int __init of_parse_thermal_zones(void)
 			of_thermal_free_zone(tz);
 			/* attempting to build remaining zones still */
 		}
+		zone->mode = THERMAL_DEVICE_DISABLED;
 	}
 	of_node_put(np);
 
-- 
2.17.1

[PATCH v7 05/11] thermal: remove get_mode() operation of drivers

[Andrzej Pietrasiewicz]

get_mode() is now redundant, as the state is stored in struct
thermal_zone_device.

Consequently the "mode" attribute in sysfs can always be visible, because
it is always possible to get the mode from struct tzd.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 drivers/acpi/thermal.c                        |  9 ------
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 19 ------------
 drivers/platform/x86/acerhdf.c                | 12 --------
 drivers/thermal/da9062-thermal.c              |  8 -----
 drivers/thermal/imx_thermal.c                 |  9 ------
 .../intel/int340x_thermal/int3400_thermal.c   |  9 ------
 .../thermal/intel/intel_quark_dts_thermal.c   |  8 -----
 drivers/thermal/thermal_core.c                |  7 +----
 drivers/thermal/thermal_of.c                  |  9 ------
 drivers/thermal/thermal_sysfs.c               | 30 ++-----------------
 include/linux/thermal.h                       |  2 --
 11 files changed, 3 insertions(+), 119 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 4ba273f49d87..592be97c4456 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -525,14 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 	return 0;
 }
 
-static int thermal_get_mode(struct thermal_zone_device *thermal,
-				enum thermal_device_mode *mode)
-{
-	*mode = thermal->mode;
-
-	return 0;
-}
-
 static int thermal_set_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode mode)
 {
@@ -847,7 +839,6 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
 	.bind = acpi_thermal_bind_cooling_device,
 	.unbind	= acpi_thermal_unbind_cooling_device,
 	.get_temp = thermal_get_temp,
-	.get_mode = thermal_get_mode,
 	.set_mode = thermal_set_mode,
 	.get_trip_type = thermal_get_trip_type,
 	.get_trip_temp = thermal_get_trip_temp,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 51667ed99c21..ad61b2db30b8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -275,14 +275,6 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
 	return 0;
 }
 
-static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
-				  enum thermal_device_mode *mode)
-{
-	*mode = tzdev->mode;
-
-	return 0;
-}
-
 static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
 				  enum thermal_device_mode mode)
 {
@@ -402,7 +394,6 @@ static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 	.bind = mlxsw_thermal_bind,
 	.unbind = mlxsw_thermal_unbind,
-	.get_mode = mlxsw_thermal_get_mode,
 	.set_mode = mlxsw_thermal_set_mode,
 	.get_temp = mlxsw_thermal_get_temp,
 	.get_trip_type	= mlxsw_thermal_get_trip_type,
@@ -461,14 +452,6 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
 	return err;
 }
 
-static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
-					 enum thermal_device_mode *mode)
-{
-	*mode = tzdev->mode;
-
-	return 0;
-}
-
 static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
 					 enum thermal_device_mode mode)
 {
@@ -606,7 +589,6 @@ static int mlxsw_thermal_module_trend_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
-	.get_mode	= mlxsw_thermal_module_mode_get,
 	.set_mode	= mlxsw_thermal_module_mode_set,
 	.get_temp	= mlxsw_thermal_module_temp_get,
 	.get_trip_type	= mlxsw_thermal_module_trip_type_get,
@@ -645,7 +627,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
-	.get_mode	= mlxsw_thermal_module_mode_get,
 	.set_mode	= mlxsw_thermal_module_mode_set,
 	.get_temp	= mlxsw_thermal_gearbox_temp_get,
 	.get_trip_type	= mlxsw_thermal_module_trip_type_get,
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 6f21015e5fd9..58c4e1caaa09 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -413,17 +413,6 @@ static inline void acerhdf_enable_kernelmode(void)
 	pr_notice("kernel mode fan control ON\n");
 }
 
-static int acerhdf_get_mode(struct thermal_zone_device *thermal,
-			    enum thermal_device_mode *mode)
-{
-	if (verbose)
-		pr_notice("kernel mode fan control %d\n", kernelmode);
-
-	*mode = thermal->mode;
-
-	return 0;
-}
-
 /*
  * set operation mode;
  * enabled: the thermal layer of the kernel takes care about
@@ -490,7 +479,6 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
 	.bind = acerhdf_bind,
 	.unbind = acerhdf_unbind,
 	.get_temp = acerhdf_get_ec_temp,
-	.get_mode = acerhdf_get_mode,
 	.set_mode = acerhdf_set_mode,
 	.get_trip_type = acerhdf_get_trip_type,
 	.get_trip_hyst = acerhdf_get_trip_hyst,
diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
index a14c7981c7c7..a7ac8afb063e 100644
--- a/drivers/thermal/da9062-thermal.c
+++ b/drivers/thermal/da9062-thermal.c
@@ -120,13 +120,6 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int da9062_thermal_get_mode(struct thermal_zone_device *z,
-				   enum thermal_device_mode *mode)
-{
-	*mode = z->mode;
-	return 0;
-}
-
 static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
 					int trip,
 					enum thermal_trip_type *type)
@@ -179,7 +172,6 @@ static int da9062_thermal_get_temp(struct thermal_zone_device *z,
 
 static struct thermal_zone_device_ops da9062_thermal_ops = {
 	.get_temp	= da9062_thermal_get_temp,
-	.get_mode	= da9062_thermal_get_mode,
 	.get_trip_type	= da9062_thermal_get_trip_type,
 	.get_trip_temp	= da9062_thermal_get_trip_temp,
 };
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 9a1114d721b6..2c7ee5da608a 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -330,14 +330,6 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 	return 0;
 }
 
-static int imx_get_mode(struct thermal_zone_device *tz,
-			enum thermal_device_mode *mode)
-{
-	*mode = tz->mode;
-
-	return 0;
-}
-
 static int imx_set_mode(struct thermal_zone_device *tz,
 			enum thermal_device_mode mode)
 {
@@ -464,7 +456,6 @@ static struct thermal_zone_device_ops imx_tz_ops = {
 	.bind = imx_bind,
 	.unbind = imx_unbind,
 	.get_temp = imx_get_temp,
-	.get_mode = imx_get_mode,
 	.set_mode = imx_set_mode,
 	.get_trip_type = imx_get_trip_type,
 	.get_trip_temp = imx_get_trip_temp,
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index f65b2fc09198..9a622aaf29dd 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -377,14 +377,6 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
-				enum thermal_device_mode *mode)
-{
-	*mode = thermal->mode;
-
-	return 0;
-}
-
 static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 				enum thermal_device_mode mode)
 {
@@ -412,7 +404,6 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 
 static struct thermal_zone_device_ops int3400_thermal_ops = {
 	.get_temp = int3400_thermal_get_temp,
-	.get_mode = int3400_thermal_get_mode,
 	.set_mode = int3400_thermal_set_mode,
 };
 
diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
index d77cb3df5ade..c4879b4bfbf1 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -308,13 +308,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
 	return 0;
 }
 
-static int sys_get_mode(struct thermal_zone_device *tzd,
-				enum thermal_device_mode *mode)
-{
-	*mode = tzd->mode;
-	return 0;
-}
-
 static int sys_set_mode(struct thermal_zone_device *tzd,
 				enum thermal_device_mode mode)
 {
@@ -336,7 +329,6 @@ static struct thermal_zone_device_ops tzone_ops = {
 	.get_trip_type = sys_get_trip_type,
 	.set_trip_temp = sys_set_trip_temp,
 	.get_crit_temp = sys_get_crit_temp,
-	.get_mode = sys_get_mode,
 	.set_mode = sys_set_mode,
 };
 
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index b71196eaf90e..14d3b1b94c4f 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1456,7 +1456,6 @@ static int thermal_pm_notify(struct notifier_block *nb,
 			     unsigned long mode, void *_unused)
 {
 	struct thermal_zone_device *tz;
-	enum thermal_device_mode tz_mode;
 
 	switch (mode) {
 	case PM_HIBERNATION_PREPARE:
@@ -1469,11 +1468,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
 	case PM_POST_SUSPEND:
 		atomic_set(&in_suspend, 0);
 		list_for_each_entry(tz, &thermal_tz_list, node) {
-			tz_mode = THERMAL_DEVICE_ENABLED;
-			if (tz->ops->get_mode)
-				tz->ops->get_mode(tz, &tz_mode);
-
-			if (tz_mode == THERMAL_DEVICE_DISABLED)
+			if (tz->mode == THERMAL_DEVICE_DISABLED)
 				continue;
 
 			thermal_zone_device_init(tz);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index c495b1e48ef2..ba65d48a48cb 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -267,14 +267,6 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int of_thermal_get_mode(struct thermal_zone_device *tz,
-			       enum thermal_device_mode *mode)
-{
-	*mode = tz->mode;
-
-	return 0;
-}
-
 static int of_thermal_set_mode(struct thermal_zone_device *tz,
 			       enum thermal_device_mode mode)
 {
@@ -389,7 +381,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 }
 
 static struct thermal_zone_device_ops of_thermal_ops = {
-	.get_mode = of_thermal_get_mode,
 	.set_mode = of_thermal_set_mode,
 
 	.get_trip_type = of_thermal_get_trip_type,
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index aa99edb4dff7..096370977068 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -49,18 +49,9 @@ static ssize_t
 mode_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	enum thermal_device_mode mode;
-	int result;
-
-	if (!tz->ops->get_mode)
-		return -EPERM;
 
-	result = tz->ops->get_mode(tz, &mode);
-	if (result)
-		return result;
-
-	return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
-		       : "disabled");
+	return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
+		       "enabled" : "disabled");
 }
 
 static ssize_t
@@ -428,30 +419,13 @@ static struct attribute_group thermal_zone_attribute_group = {
 	.attrs = thermal_zone_dev_attrs,
 };
 
-/* We expose mode only if .get_mode is present */
 static struct attribute *thermal_zone_mode_attrs[] = {
 	&dev_attr_mode.attr,
 	NULL,
 };
 
-static umode_t thermal_zone_mode_is_visible(struct kobject *kobj,
-					    struct attribute *attr,
-					    int attrno)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct thermal_zone_device *tz;
-
-	tz = container_of(dev, struct thermal_zone_device, device);
-
-	if (tz->ops->get_mode)
-		return attr->mode;
-
-	return 0;
-}
-
 static struct attribute_group thermal_zone_mode_attribute_group = {
 	.attrs = thermal_zone_mode_attrs,
-	.is_visible = thermal_zone_mode_is_visible,
 };
 
 /* We expose passive only if passive trips are present */
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f91d7f04512..a808f6fa2777 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -76,8 +76,6 @@ struct thermal_zone_device_ops {
 		       struct thermal_cooling_device *);
 	int (*get_temp) (struct thermal_zone_device *, int *);
 	int (*set_trips) (struct thermal_zone_device *, int, int);
-	int (*get_mode) (struct thermal_zone_device *,
-			 enum thermal_device_mode *);
 	int (*set_mode) (struct thermal_zone_device *,
 		enum thermal_device_mode);
 	int (*get_trip_type) (struct thermal_zone_device *, int,
-- 
2.17.1

[PATCH v7 06/11] thermal: Add mode helpers

[Andrzej Pietrasiewicz]

Prepare for making the drivers not access tzd's private members.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
[staticize thermal_zone_device_set_mode()]
Signed-off-by: kernel test robot <lkp@intel.com>
---
 drivers/thermal/thermal_core.c | 53 ++++++++++++++++++++++++++++++++++
 include/linux/thermal.h        | 13 +++++++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 14d3b1b94c4f..f02c57c986f0 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -459,6 +459,59 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz)
 	thermal_zone_device_init(tz);
 }
 
+static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
+					enum thermal_device_mode mode)
+{
+	int ret = 0;
+
+	mutex_lock(&tz->lock);
+
+	/* do nothing if mode isn't changing */
+	if (mode == tz->mode) {
+		mutex_unlock(&tz->lock);
+
+		return ret;
+	}
+
+	if (tz->ops->set_mode)
+		ret = tz->ops->set_mode(tz, mode);
+
+	if (!ret)
+		tz->mode = mode;
+
+	mutex_unlock(&tz->lock);
+
+	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+
+	return ret;
+}
+
+int thermal_zone_device_enable(struct thermal_zone_device *tz)
+{
+	return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_ENABLED);
+}
+EXPORT_SYMBOL_GPL(thermal_zone_device_enable);
+
+int thermal_zone_device_disable(struct thermal_zone_device *tz)
+{
+	return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_DISABLED);
+}
+EXPORT_SYMBOL_GPL(thermal_zone_device_disable);
+
+int thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
+{
+	enum thermal_device_mode mode;
+
+	mutex_lock(&tz->lock);
+
+	mode = tz->mode;
+
+	mutex_unlock(&tz->lock);
+
+	return mode == THERMAL_DEVICE_ENABLED;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_device_is_enabled);
+
 void thermal_zone_device_update(struct thermal_zone_device *tz,
 				enum thermal_notify_event event)
 {
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a808f6fa2777..df013c39ba9b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -416,6 +416,9 @@ int thermal_zone_get_offset(struct thermal_zone_device *tz);
 
 void thermal_cdev_update(struct thermal_cooling_device *);
 void thermal_notify_framework(struct thermal_zone_device *, int);
+int thermal_zone_device_enable(struct thermal_zone_device *tz);
+int thermal_zone_device_disable(struct thermal_zone_device *tz);
+int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
 #else
 static inline struct thermal_zone_device *thermal_zone_device_register(
 	const char *type, int trips, int mask, void *devdata,
@@ -463,6 +466,16 @@ static inline void thermal_cdev_update(struct thermal_cooling_device *cdev)
 static inline void thermal_notify_framework(struct thermal_zone_device *tz,
 	int trip)
 { }
+
+static inline int thermal_zone_device_enable(struct thermal_zone_device *tz)
+{ return -ENODEV; }
+
+static inline int thermal_zone_device_disable(struct thermal_zone_device *tz)
+{ return -ENODEV; }
+
+static inline int
+thermal_zone_device_is_enabled(struct thermal_zone_device *tz)
+{ return -ENODEV; }
 #endif /* CONFIG_THERMAL */
 
 #endif /* __THERMAL_H__ */
-- 
2.17.1

[PATCH v7 07/11] thermal: Use mode helpers in drivers

[Andrzej Pietrasiewicz]

Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().

Consequently, all set_mode() implementations in drivers:

- can stop modifying tzd's "mode" member,
- shall stop taking tzd's lock, as it is taken in the helpers
- shall stop calling thermal_zone_device_update() as it is called in the
helpers
- can assume they are called when the mode truly changes, so checks to
verify that can be dropped

Not providing set_mode() by a driver no longer prevents the core from
being able to set tzd's mode, so the relevant check in mode_store() is
removed.

Other comments:

- acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
return from set_mode(), so use function parameter in thermal_set_mode()
instead, no need to call acpi_thermal_check() in set_mode()
- thermal/imx_thermal.c: regmap writes and mode assignment are done in
thermal_zone_device_{en|dis}able() and set_mode() callback
- thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
part of set_mode() callback, so they don't need to modify tzd->mode, and
don't need to fall back to the opposite mode if unsuccessful, as the return
value will be propagated to thermal_zone_device_{en|dis}able() and
ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
- thermal/of-thermal.c: no need to set zone->mode to DISABLED in
of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
---
 drivers/acpi/thermal.c                        | 21 ++++++-----
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 37 +++++++++----------
 drivers/platform/x86/acerhdf.c                | 17 +++++----
 drivers/thermal/da9062-thermal.c              |  6 ++-
 drivers/thermal/hisi_thermal.c                |  6 ++-
 drivers/thermal/imx_thermal.c                 | 33 +++++++----------
 .../intel/int340x_thermal/int3400_thermal.c   |  5 +--
 .../thermal/intel/intel_quark_dts_thermal.c   | 18 ++-------
 drivers/thermal/rockchip_thermal.c            |  6 ++-
 drivers/thermal/sprd_thermal.c                |  6 ++-
 drivers/thermal/thermal_core.c                |  2 +-
 drivers/thermal/thermal_of.c                  | 10 +----
 drivers/thermal/thermal_sysfs.c               | 11 ++----
 13 files changed, 80 insertions(+), 98 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 592be97c4456..52b6cda1bcc3 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -499,7 +499,7 @@ static void acpi_thermal_check(void *data)
 {
 	struct acpi_thermal *tz = data;
 
-	if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
+	if (!thermal_zone_device_is_enabled(tz->thermal_zone))
 		return;
 
 	thermal_zone_device_update(tz->thermal_zone,
@@ -542,14 +542,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
 	if (mode == THERMAL_DEVICE_DISABLED)
 		pr_warn("thermal zone will be disabled\n");
 
-	if (mode != tz->thermal_zone->mode) {
-		tz->thermal_zone->mode = mode;
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			"%s kernel ACPI thermal control\n",
-			tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
-			"Enable" : "Disable"));
-		acpi_thermal_check(tz);
-	}
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+		"%s kernel ACPI thermal control\n",
+		mode == THERMAL_DEVICE_ENABLED ?
+		"Enable" : "Disable"));
+
 	return 0;
 }
 
@@ -897,13 +894,17 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 		goto remove_dev_link;
 	}
 
-	tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
+	result = thermal_zone_device_enable(tz->thermal_zone);
+	if (result)
+		goto acpi_bus_detach;
 
 	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
 		 tz->thermal_zone->id);
 
 	return 0;
 
+acpi_bus_detach:
+	acpi_bus_detach_private_data(tz->device->handle);
 remove_dev_link:
 	sysfs_remove_link(&tz->thermal_zone->device.kobj, "device");
 remove_tz_link:
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index ad61b2db30b8..4fb73d0fd167 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -280,18 +280,11 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
 {
 	struct mlxsw_thermal *thermal = tzdev->devdata;
 
-	mutex_lock(&tzdev->lock);
-
 	if (mode == THERMAL_DEVICE_ENABLED)
 		tzdev->polling_delay = thermal->polling_delay;
 	else
 		tzdev->polling_delay = 0;
 
-	tzdev->mode = mode;
-	mutex_unlock(&tzdev->lock);
-
-	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
-
 	return 0;
 }
 
@@ -458,19 +451,11 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
 	struct mlxsw_thermal_module *tz = tzdev->devdata;
 	struct mlxsw_thermal *thermal = tz->parent;
 
-	mutex_lock(&tzdev->lock);
-
 	if (mode == THERMAL_DEVICE_ENABLED)
 		tzdev->polling_delay = thermal->polling_delay;
 	else
 		tzdev->polling_delay = 0;
 
-	tzdev->mode = mode;
-
-	mutex_unlock(&tzdev->lock);
-
-	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
-
 	return 0;
 }
 
@@ -756,8 +741,11 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
 		return err;
 	}
 
-	module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
-	return 0;
+	err = thermal_zone_device_enable(module_tz->tzdev);
+	if (err)
+		thermal_zone_device_unregister(module_tz->tzdev);
+
+	return err;
 }
 
 static void mlxsw_thermal_module_tz_fini(struct thermal_zone_device *tzdev)
@@ -860,6 +848,7 @@ static int
 mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
 {
 	char tz_name[MLXSW_THERMAL_ZONE_MAX_NAME];
+	int ret;
 
 	snprintf(tz_name, sizeof(tz_name), "mlxsw-gearbox%d",
 		 gearbox_tz->module + 1);
@@ -872,8 +861,11 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
 	if (IS_ERR(gearbox_tz->tzdev))
 		return PTR_ERR(gearbox_tz->tzdev);
 
-	gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
-	return 0;
+	ret = thermal_zone_device_enable(gearbox_tz->tzdev);
+	if (ret)
+		thermal_zone_device_unregister(gearbox_tz->tzdev);
+
+	return ret;
 }
 
 static void
@@ -1041,10 +1033,15 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
 	if (err)
 		goto err_unreg_modules_tzdev;
 
-	thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
+	err = thermal_zone_device_enable(thermal->tzdev);
+	if (err)
+		goto err_unreg_gearboxes;
+
 	*p_thermal = thermal;
 	return 0;
 
+err_unreg_gearboxes:
+	mlxsw_thermal_gearboxes_fini(thermal);
 err_unreg_modules_tzdev:
 	mlxsw_thermal_modules_fini(thermal);
 err_unreg_tzdev:
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 58c4e1caaa09..8fe0ecb6a626 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -397,19 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
 {
 	acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
 	kernelmode = 0;
-	if (thz_dev) {
-		thz_dev->mode = THERMAL_DEVICE_DISABLED;
+	if (thz_dev)
 		thz_dev->polling_delay = 0;
-	}
+
 	pr_notice("kernel mode fan control OFF\n");
 }
 static inline void acerhdf_enable_kernelmode(void)
 {
 	kernelmode = 1;
-	thz_dev->mode = THERMAL_DEVICE_ENABLED;
 
 	thz_dev->polling_delay = interval*1000;
-	thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
 	pr_notice("kernel mode fan control ON\n");
 }
 
@@ -723,6 +720,8 @@ static void acerhdf_unregister_platform(void)
 
 static int __init acerhdf_register_thermal(void)
 {
+	int ret;
+
 	cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
 						 &acerhdf_cooling_ops);
 
@@ -736,8 +735,12 @@ static int __init acerhdf_register_thermal(void)
 	if (IS_ERR(thz_dev))
 		return -EINVAL;
 
-	thz_dev->mode = kernelmode ?
-		THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
+	if (kernelmode)
+		ret = thermal_zone_device_enable(thz_dev);
+	else
+		ret = thermal_zone_device_disable(thz_dev);
+	if (ret)
+		return ret;
 
 	if (strcmp(thz_dev->governor->name,
 				acerhdf_zone_params.governor_name)) {
diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
index a7ac8afb063e..4d74994f160a 100644
--- a/drivers/thermal/da9062-thermal.c
+++ b/drivers/thermal/da9062-thermal.c
@@ -237,7 +237,11 @@ static int da9062_thermal_probe(struct platform_device *pdev)
 		ret = PTR_ERR(thermal->zone);
 		goto err;
 	}
-	thermal->zone->mode = THERMAL_DEVICE_ENABLED;
+	ret = thermal_zone_device_enable(thermal->zone);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot enable thermal zone device\n");
+		goto err_zone;
+	}
 
 	dev_dbg(&pdev->dev,
 		"TJUNC temperature polling period set at %d ms\n",
diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index 2d26ae80e202..ee05950afd2f 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -549,8 +549,10 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
 {
 	struct thermal_zone_device *tzd = sensor->tzd;
 
-	tzd->ops->set_mode(tzd,
-		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
+	if (on)
+		thermal_zone_device_enable(tzd);
+	else
+		thermal_zone_device_disable(tzd);
 }
 
 static int hisi_thermal_probe(struct platform_device *pdev)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2c7ee5da608a..53abb1be1cba 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -255,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 	bool wait;
 	u32 val;
 
-	if (tz->mode == THERMAL_DEVICE_ENABLED) {
+	if (thermal_zone_device_is_enabled(tz)) {
 		/* Check if a measurement is currently in progress */
 		regmap_read(map, soc_data->temp_data, &val);
 		wait = !(val & soc_data->temp_valid_mask);
@@ -282,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	regmap_read(map, soc_data->temp_data, &val);
 
-	if (tz->mode != THERMAL_DEVICE_ENABLED) {
+	if (!thermal_zone_device_is_enabled(tz)) {
 		regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
 			     soc_data->measure_temp_mask);
 		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
@@ -365,9 +365,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
 		}
 	}
 
-	tz->mode = mode;
-	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
 	return 0;
 }
 
@@ -819,7 +816,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
 		     data->socdata->measure_temp_mask);
 
 	data->irq_enabled = true;
-	data->tz->mode = THERMAL_DEVICE_ENABLED;
+	ret = thermal_zone_device_enable(data->tz);
+	if (ret)
+		goto thermal_zone_unregister;
 
 	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
 			imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
@@ -861,19 +860,18 @@ static int imx_thermal_remove(struct platform_device *pdev)
 static int __maybe_unused imx_thermal_suspend(struct device *dev)
 {
 	struct imx_thermal_data *data = dev_get_drvdata(dev);
-	struct regmap *map = data->tempmon;
+	int ret;
 
 	/*
 	 * Need to disable thermal sensor, otherwise, when thermal core
 	 * try to get temperature before thermal sensor resume, a wrong
 	 * temperature will be read as the thermal sensor is powered
-	 * down.
+	 * down. This is done in set_mode() operation called from
+	 * thermal_zone_device_disable()
 	 */
-	regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
-		     data->socdata->measure_temp_mask);
-	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
-		     data->socdata->power_down_mask);
-	data->tz->mode = THERMAL_DEVICE_DISABLED;
+	ret = thermal_zone_device_disable(data->tz);
+	if (ret)
+		return ret;
 	clk_disable_unprepare(data->thermal_clk);
 
 	return 0;
@@ -882,18 +880,15 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
 static int __maybe_unused imx_thermal_resume(struct device *dev)
 {
 	struct imx_thermal_data *data = dev_get_drvdata(dev);
-	struct regmap *map = data->tempmon;
 	int ret;
 
 	ret = clk_prepare_enable(data->thermal_clk);
 	if (ret)
 		return ret;
 	/* Enabled thermal sensor after resume */
-	regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
-		     data->socdata->power_down_mask);
-	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
-		     data->socdata->measure_temp_mask);
-	data->tz->mode = THERMAL_DEVICE_ENABLED;
+	ret = thermal_zone_device_enable(data->tz);
+	if (ret)
+		return ret;
 
 	return 0;
 }
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 9a622aaf29dd..3c0397a29b8c 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -390,12 +390,11 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 	    mode != THERMAL_DEVICE_DISABLED)
 		return -EINVAL;
 
-	if (mode != thermal->mode) {
-		thermal->mode = mode;
+	if (mode != thermal->mode)
 		result = int3400_thermal_run_osc(priv->adev->handle,
 						priv->current_uuid_index,
 						mode == THERMAL_DEVICE_ENABLED);
-	}
+
 
 	evaluate_odvp(priv);
 
diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
index c4879b4bfbf1..e29c3e330b17 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -126,10 +126,8 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
 	if (ret)
 		return ret;
 
-	if (out & QRK_DTS_ENABLE_BIT) {
-		tzd->mode = THERMAL_DEVICE_ENABLED;
+	if (out & QRK_DTS_ENABLE_BIT)
 		return 0;
-	}
 
 	if (!aux_entry->locked) {
 		out |= QRK_DTS_ENABLE_BIT;
@@ -137,10 +135,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
 				     QRK_DTS_REG_OFFSET_ENABLE, out);
 		if (ret)
 			return ret;
-
-		tzd->mode = THERMAL_DEVICE_ENABLED;
 	} else {
-		tzd->mode = THERMAL_DEVICE_DISABLED;
 		pr_info("DTS is locked. Cannot enable DTS\n");
 		ret = -EPERM;
 	}
@@ -159,10 +154,8 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
 	if (ret)
 		return ret;
 
-	if (!(out & QRK_DTS_ENABLE_BIT)) {
-		tzd->mode = THERMAL_DEVICE_DISABLED;
+	if (!(out & QRK_DTS_ENABLE_BIT))
 		return 0;
-	}
 
 	if (!aux_entry->locked) {
 		out &= ~QRK_DTS_ENABLE_BIT;
@@ -171,10 +164,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
 
 		if (ret)
 			return ret;
-
-		tzd->mode = THERMAL_DEVICE_DISABLED;
 	} else {
-		tzd->mode = THERMAL_DEVICE_ENABLED;
 		pr_info("DTS is locked. Cannot disable DTS\n");
 		ret = -EPERM;
 	}
@@ -404,9 +394,7 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
 		goto err_ret;
 	}
 
-	mutex_lock(&dts_update_mutex);
-	err = soc_dts_enable(aux_entry->tzone);
-	mutex_unlock(&dts_update_mutex);
+	err = thermal_zone_device_enable(aux_entry->tzone);
 	if (err)
 		goto err_aux_status;
 
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 15a71ecc916c..aa9e0e31ef98 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1068,8 +1068,10 @@ rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
 {
 	struct thermal_zone_device *tzd = sensor->tzd;
 
-	tzd->ops->set_mode(tzd,
-		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
+	if (on)
+		thermal_zone_device_enable(tzd);
+	else
+		thermal_zone_device_disable(tzd);
 }
 
 static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
diff --git a/drivers/thermal/sprd_thermal.c b/drivers/thermal/sprd_thermal.c
index a340374e8c51..58f995b0f804 100644
--- a/drivers/thermal/sprd_thermal.c
+++ b/drivers/thermal/sprd_thermal.c
@@ -322,8 +322,10 @@ static void sprd_thm_toggle_sensor(struct sprd_thermal_sensor *sen, bool on)
 {
 	struct thermal_zone_device *tzd = sen->tzd;
 
-	tzd->ops->set_mode(tzd,
-		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
+	if (on)
+		thermal_zone_device_enable(tzd);
+	else
+		thermal_zone_device_disable(tzd);
 }
 
 static int sprd_thm_probe(struct platform_device *pdev)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index f02c57c986f0..52d136780577 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1521,7 +1521,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
 	case PM_POST_SUSPEND:
 		atomic_set(&in_suspend, 0);
 		list_for_each_entry(tz, &thermal_tz_list, node) {
-			if (tz->mode == THERMAL_DEVICE_DISABLED)
+			if (!thermal_zone_device_is_enabled(tz))
 				continue;
 
 			thermal_zone_device_init(tz);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index ba65d48a48cb..43a516a35d64 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -272,8 +272,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
 {
 	struct __thermal_zone *data = tz->devdata;
 
-	mutex_lock(&tz->lock);
-
 	if (mode == THERMAL_DEVICE_ENABLED) {
 		tz->polling_delay = data->polling_delay;
 		tz->passive_delay = data->passive_delay;
@@ -282,11 +280,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
 		tz->passive_delay = 0;
 	}
 
-	mutex_unlock(&tz->lock);
-
-	tz->mode = mode;
-	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
 	return 0;
 }
 
@@ -541,7 +534,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
 			tzd = thermal_zone_of_add_sensor(child, sensor_np,
 							 data, ops);
 			if (!IS_ERR(tzd))
-				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
+				thermal_zone_device_enable(tzd);
 
 			of_node_put(child);
 			goto exit;
@@ -1120,7 +1113,6 @@ int __init of_parse_thermal_zones(void)
 			of_thermal_free_zone(tz);
 			/* attempting to build remaining zones still */
 		}
-		zone->mode = THERMAL_DEVICE_DISABLED;
 	}
 	of_node_put(np);
 
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 096370977068..c23d67c4dc4e 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -49,9 +49,9 @@ static ssize_t
 mode_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	int enabled = thermal_zone_device_is_enabled(tz);
 
-	return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
-		       "enabled" : "disabled");
+	return sprintf(buf, "%s\n", enabled ? "enabled" : "disabled");
 }
 
 static ssize_t
@@ -61,13 +61,10 @@ mode_store(struct device *dev, struct device_attribute *attr,
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int result;
 
-	if (!tz->ops->set_mode)
-		return -EPERM;
-
 	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
+		result = thermal_zone_device_enable(tz);
 	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
+		result = thermal_zone_device_disable(tz);
 	else
 		result = -EINVAL;
 
-- 
2.17.1

[PATCH v7 08/11] thermal: Explicitly enable non-changing thermal zone devices

[Andrzej Pietrasiewicz]

Some thermal zone devices never change their state, so they should be
always enabled.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c       | 8 ++++++++
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c              | 9 ++++++++-
 drivers/platform/x86/intel_mid_thermal.c                 | 6 ++++++
 drivers/power/supply/power_supply_core.c                 | 9 +++++++--
 drivers/thermal/armada_thermal.c                         | 6 ++++++
 drivers/thermal/dove_thermal.c                           | 6 ++++++
 .../thermal/intel/int340x_thermal/int340x_thermal_zone.c | 5 +++++
 drivers/thermal/intel/intel_pch_thermal.c                | 5 +++++
 drivers/thermal/intel/intel_soc_dts_iosf.c               | 3 +++
 drivers/thermal/intel/x86_pkg_temp_thermal.c             | 6 ++++++
 drivers/thermal/kirkwood_thermal.c                       | 7 +++++++
 drivers/thermal/rcar_thermal.c                           | 9 ++++++++-
 drivers/thermal/spear_thermal.c                          | 7 +++++++
 drivers/thermal/st/st_thermal.c                          | 5 +++++
 14 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
index 3de8a5e83b6c..e3510e9b21f3 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
@@ -92,6 +92,14 @@ int cxgb4_thermal_init(struct adapter *adap)
 		ch_thermal->tzdev = NULL;
 		return ret;
 	}
+
+	ret = thermal_zone_device_enable(ch_thermal->tzdev);
+	if (ret) {
+		dev_err(adap->pdev_dev, "Failed to enable thermal zone\n");
+		thermal_zone_device_unregister(adap->ch_thermal.tzdev);
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 418e59b7c671..0c95663bf9ed 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -733,7 +733,7 @@ static  struct thermal_zone_device_ops tzone_ops = {
 
 static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
 {
-	int i;
+	int i, ret;
 	char name[16];
 	static atomic_t counter = ATOMIC_INIT(0);
 
@@ -759,6 +759,13 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
 		return;
 	}
 
+	ret = thermal_zone_device_enable(mvm->tz_device.tzone);
+	if (ret) {
+		IWL_DEBUG_TEMP(mvm, "Failed to enable thermal zone\n");
+		thermal_zone_device_unregister(mvm->tz_device.tzone);
+		return;
+	}
+
 	/* 0 is a valid temperature,
 	 * so initialize the array with S16_MIN which invalid temperature
 	 */
diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
index f402e2e74a38..f12f4e7bd971 100644
--- a/drivers/platform/x86/intel_mid_thermal.c
+++ b/drivers/platform/x86/intel_mid_thermal.c
@@ -493,6 +493,12 @@ static int mid_thermal_probe(struct platform_device *pdev)
 			ret = PTR_ERR(pinfo->tzd[i]);
 			goto err;
 		}
+		ret = thermal_zone_device_enable(pinfo->tzd[i]);
+		if (ret) {
+			kfree(td_info);
+			thermal_zone_device_unregister(pinfo->tzd[i]);
+			goto err;
+		}
 	}
 
 	pinfo->pdev = pdev;
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 02b37fe6061c..90e56736d479 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -939,7 +939,7 @@ static struct thermal_zone_device_ops psy_tzd_ops = {
 
 static int psy_register_thermal(struct power_supply *psy)
 {
-	int i;
+	int i, ret;
 
 	if (psy->desc->no_thermal)
 		return 0;
@@ -949,7 +949,12 @@ static int psy_register_thermal(struct power_supply *psy)
 		if (psy->desc->properties[i] == POWER_SUPPLY_PROP_TEMP) {
 			psy->tzd = thermal_zone_device_register(psy->desc->name,
 					0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
-			return PTR_ERR_OR_ZERO(psy->tzd);
+			if (IS_ERR(psy->tzd))
+				return PTR_ERR(psy->tzd);
+			ret = thermal_zone_device_enable(psy->tzd);
+			if (ret)
+				thermal_zone_device_unregister(psy->tzd);
+			return ret;
 		}
 	}
 	return 0;
diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
index 7c447cd149e7..c2ebfb5be4b3 100644
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -874,6 +874,12 @@ static int armada_thermal_probe(struct platform_device *pdev)
 			return PTR_ERR(tz);
 		}
 
+		ret = thermal_zone_device_enable(tz);
+		if (ret) {
+			thermal_zone_device_unregister(tz);
+			return ret;
+		}
+
 		drvdata->type = LEGACY;
 		drvdata->data.tz = tz;
 		platform_set_drvdata(pdev, drvdata);
diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
index 75901ced4a62..73182eb94bc0 100644
--- a/drivers/thermal/dove_thermal.c
+++ b/drivers/thermal/dove_thermal.c
@@ -153,6 +153,12 @@ static int dove_thermal_probe(struct platform_device *pdev)
 		return PTR_ERR(thermal);
 	}
 
+	ret = thermal_zone_device_enable(thermal);
+	if (ret) {
+		thermal_zone_device_unregister(thermal);
+		return ret;
+	}
+
 	platform_set_drvdata(pdev, thermal);
 
 	return 0;
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 432213272f1e..6e479deff76b 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -259,9 +259,14 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
 		ret = PTR_ERR(int34x_thermal_zone->zone);
 		goto err_thermal_zone;
 	}
+	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
+	if (ret)
+		goto err_enable;
 
 	return int34x_thermal_zone;
 
+err_enable:
+	thermal_zone_device_unregister(int34x_thermal_zone->zone);
 err_thermal_zone:
 	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
 	kfree(int34x_thermal_zone->aux_trips);
diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
index 56401fd4708d..65702094f3d3 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -352,9 +352,14 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
 		err = PTR_ERR(ptd->tzd);
 		goto error_cleanup;
 	}
+	err = thermal_zone_device_enable(ptd->tzd);
+	if (err)
+		goto err_unregister;
 
 	return 0;
 
+err_unregister:
+	thermal_zone_device_unregister(ptd->tzd);
 error_cleanup:
 	iounmap(ptd->hw_base);
 error_release:
diff --git a/drivers/thermal/intel/intel_soc_dts_iosf.c b/drivers/thermal/intel/intel_soc_dts_iosf.c
index f75271b669c6..4f1a2f7c016c 100644
--- a/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ b/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -329,6 +329,9 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
 		ret = PTR_ERR(dts->tzone);
 		goto err_ret;
 	}
+	ret = thermal_zone_device_enable(dts->tzone);
+	if (ret)
+		goto err_enable;
 
 	ret = soc_dts_enable(id);
 	if (ret)
diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
index a006b9fd1d72..b81c33202f41 100644
--- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -363,6 +363,12 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
 		kfree(zonedev);
 		return err;
 	}
+	err = thermal_zone_device_enable(zonedev->tzone);
+	if (err) {
+		thermal_zone_device_unregister(zonedev->tzone);
+		kfree(zonedev);
+		return err;
+	}
 	/* Store MSR value for package thermal interrupt, to restore at exit */
 	rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, zonedev->msr_pkg_therm_low,
 	      zonedev->msr_pkg_therm_high);
diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
index 189b675cf14d..7fb6e476c82a 100644
--- a/drivers/thermal/kirkwood_thermal.c
+++ b/drivers/thermal/kirkwood_thermal.c
@@ -65,6 +65,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
 	struct thermal_zone_device *thermal = NULL;
 	struct kirkwood_thermal_priv *priv;
 	struct resource *res;
+	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -82,6 +83,12 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
 			"Failed to register thermal zone device\n");
 		return PTR_ERR(thermal);
 	}
+	ret = thermal_zone_device_enable(thermal);
+	if (ret) {
+		thermal_zone_device_unregister(thermal);
+		dev_err(&pdev->dev, "Failed to enable thermal zone device\n");
+		return ret;
+	}
 
 	platform_set_drvdata(pdev, thermal);
 
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 46aeb28b4e90..787710bb88fe 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -550,12 +550,19 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 			priv->zone = devm_thermal_zone_of_sensor_register(
 						dev, i, priv,
 						&rcar_thermal_zone_of_ops);
-		else
+		else {
 			priv->zone = thermal_zone_device_register(
 						"rcar_thermal",
 						1, 0, priv,
 						&rcar_thermal_zone_ops, NULL, 0,
 						idle);
+
+			ret = thermal_zone_device_enable(priv->zone);
+			if (ret) {
+				thermal_zone_device_unregister(priv->zone);
+				priv->zone = ERR_PTR(ret);
+			}
+		}
 		if (IS_ERR(priv->zone)) {
 			dev_err(dev, "can't register thermal zone\n");
 			ret = PTR_ERR(priv->zone);
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index f68f581fd669..ee33ed692e4f 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -131,6 +131,11 @@ static int spear_thermal_probe(struct platform_device *pdev)
 		ret = PTR_ERR(spear_thermal);
 		goto disable_clk;
 	}
+	ret = thermal_zone_device_enable(spear_thermal);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot enable thermal zone\n");
+		goto unregister_tzd;
+	}
 
 	platform_set_drvdata(pdev, spear_thermal);
 
@@ -139,6 +144,8 @@ static int spear_thermal_probe(struct platform_device *pdev)
 
 	return 0;
 
+unregister_tzd:
+	thermal_zone_device_unregister(spear_thermal);
 disable_clk:
 	clk_disable(stdev->clk);
 
diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
index b928ca6a289b..1276b95604fe 100644
--- a/drivers/thermal/st/st_thermal.c
+++ b/drivers/thermal/st/st_thermal.c
@@ -246,11 +246,16 @@ int st_thermal_register(struct platform_device *pdev,
 		ret = PTR_ERR(sensor->thermal_dev);
 		goto sensor_off;
 	}
+	ret = thermal_zone_device_enable(sensor->thermal_dev);
+	if (ret)
+		goto tzd_unregister;
 
 	platform_set_drvdata(pdev, sensor);
 
 	return 0;
 
+tzd_unregister:
+	thermal_zone_device_unregister(sensor->thermal_dev);
 sensor_off:
 	st_thermal_sensor_off(sensor);
 
-- 
2.17.1

[PATCH v7 09/11] thermal: core: Stop polling DISABLED thermal devices

[Andrzej Pietrasiewicz]

Polling DISABLED devices is not desired, as all such "disabled" devices
are meant to be handled by userspace. This patch introduces and uses
should_stop_polling() to decide whether the device should be polled or not.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/thermal_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 52d136780577..e613f5c07bad 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -301,13 +301,22 @@ static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
 		cancel_delayed_work(&tz->poll_queue);
 }
 
+static inline bool should_stop_polling(struct thermal_zone_device *tz)
+{
+	return !thermal_zone_device_is_enabled(tz);
+}
+
 static void monitor_thermal_zone(struct thermal_zone_device *tz)
 {
+	bool stop;
+
+	stop = should_stop_polling(tz);
+
 	mutex_lock(&tz->lock);
 
-	if (tz->passive)
+	if (!stop && tz->passive)
 		thermal_zone_device_set_polling(tz, tz->passive_delay);
-	else if (tz->polling_delay)
+	else if (!stop && tz->polling_delay)
 		thermal_zone_device_set_polling(tz, tz->polling_delay);
 	else
 		thermal_zone_device_set_polling(tz, 0);
@@ -517,6 +526,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz,
 {
 	int count;
 
+	if (should_stop_polling(tz))
+		return;
+
 	if (atomic_read(&in_suspend))
 		return;
 
-- 
2.17.1

[PATCH v7 10/11] thermal: Simplify or eliminate unnecessary set_mode() methods

[Andrzej Pietrasiewicz]

Setting polling_delay is now done at thermal_core level (by not polling
DISABLED devices), so no need to repeat this code.

int340x: Checking for an impossible enum value is unnecessary.
acpi/thermal: It only prints debug messages.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/acpi/thermal.c                        | 26 ----------------
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 30 -------------------
 drivers/platform/x86/acerhdf.c                |  3 --
 drivers/thermal/imx_thermal.c                 |  6 ----
 .../intel/int340x_thermal/int3400_thermal.c   |  4 ---
 drivers/thermal/thermal_of.c                  | 18 -----------
 6 files changed, 87 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 52b6cda1bcc3..29a2b73fe035 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -525,31 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 	return 0;
 }
 
-static int thermal_set_mode(struct thermal_zone_device *thermal,
-				enum thermal_device_mode mode)
-{
-	struct acpi_thermal *tz = thermal->devdata;
-
-	if (!tz)
-		return -EINVAL;
-
-	if (mode != THERMAL_DEVICE_DISABLED &&
-	    mode != THERMAL_DEVICE_ENABLED)
-		return -EINVAL;
-	/*
-	 * enable/disable thermal management from ACPI thermal driver
-	 */
-	if (mode == THERMAL_DEVICE_DISABLED)
-		pr_warn("thermal zone will be disabled\n");
-
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-		"%s kernel ACPI thermal control\n",
-		mode == THERMAL_DEVICE_ENABLED ?
-		"Enable" : "Disable"));
-
-	return 0;
-}
-
 static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 				 int trip, enum thermal_trip_type *type)
 {
@@ -836,7 +811,6 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
 	.bind = acpi_thermal_bind_cooling_device,
 	.unbind	= acpi_thermal_unbind_cooling_device,
 	.get_temp = thermal_get_temp,
-	.set_mode = thermal_set_mode,
 	.get_trip_type = thermal_get_trip_type,
 	.get_trip_temp = thermal_get_trip_temp,
 	.get_crit_temp = thermal_get_crit_temp,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 4fb73d0fd167..8fa286ccdd6b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -275,19 +275,6 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
 	return 0;
 }
 
-static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
-				  enum thermal_device_mode mode)
-{
-	struct mlxsw_thermal *thermal = tzdev->devdata;
-
-	if (mode == THERMAL_DEVICE_ENABLED)
-		tzdev->polling_delay = thermal->polling_delay;
-	else
-		tzdev->polling_delay = 0;
-
-	return 0;
-}
-
 static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
 				  int *p_temp)
 {
@@ -387,7 +374,6 @@ static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 	.bind = mlxsw_thermal_bind,
 	.unbind = mlxsw_thermal_unbind,
-	.set_mode = mlxsw_thermal_set_mode,
 	.get_temp = mlxsw_thermal_get_temp,
 	.get_trip_type	= mlxsw_thermal_get_trip_type,
 	.get_trip_temp	= mlxsw_thermal_get_trip_temp,
@@ -445,20 +431,6 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
 	return err;
 }
 
-static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
-					 enum thermal_device_mode mode)
-{
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
-	struct mlxsw_thermal *thermal = tz->parent;
-
-	if (mode == THERMAL_DEVICE_ENABLED)
-		tzdev->polling_delay = thermal->polling_delay;
-	else
-		tzdev->polling_delay = 0;
-
-	return 0;
-}
-
 static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
 					 int *p_temp)
 {
@@ -574,7 +546,6 @@ static int mlxsw_thermal_module_trend_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
-	.set_mode	= mlxsw_thermal_module_mode_set,
 	.get_temp	= mlxsw_thermal_module_temp_get,
 	.get_trip_type	= mlxsw_thermal_module_trip_type_get,
 	.get_trip_temp	= mlxsw_thermal_module_trip_temp_get,
@@ -612,7 +583,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
 static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
-	.set_mode	= mlxsw_thermal_module_mode_set,
 	.get_temp	= mlxsw_thermal_gearbox_temp_get,
 	.get_trip_type	= mlxsw_thermal_module_trip_type_get,
 	.get_trip_temp	= mlxsw_thermal_module_trip_temp_get,
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 8fe0ecb6a626..76323855c80c 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -397,8 +397,6 @@ static inline void acerhdf_revert_to_bios_mode(void)
 {
 	acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
 	kernelmode = 0;
-	if (thz_dev)
-		thz_dev->polling_delay = 0;
 
 	pr_notice("kernel mode fan control OFF\n");
 }
@@ -406,7 +404,6 @@ static inline void acerhdf_enable_kernelmode(void)
 {
 	kernelmode = 1;
 
-	thz_dev->polling_delay = interval*1000;
 	pr_notice("kernel mode fan control ON\n");
 }
 
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 53abb1be1cba..a02398118d88 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -338,9 +338,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
 	const struct thermal_soc_data *soc_data = data->socdata;
 
 	if (mode == THERMAL_DEVICE_ENABLED) {
-		tz->polling_delay = IMX_POLLING_DELAY;
-		tz->passive_delay = IMX_PASSIVE_DELAY;
-
 		regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
 			     soc_data->power_down_mask);
 		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
@@ -356,9 +353,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
 		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
 			     soc_data->power_down_mask);
 
-		tz->polling_delay = 0;
-		tz->passive_delay = 0;
-
 		if (data->irq_enabled) {
 			disable_irq(data->irq);
 			data->irq_enabled = false;
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 3c0397a29b8c..ce49d3b100d5 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -386,10 +386,6 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 	if (!priv)
 		return -EINVAL;
 
-	if (mode != THERMAL_DEVICE_ENABLED &&
-	    mode != THERMAL_DEVICE_DISABLED)
-		return -EINVAL;
-
 	if (mode != thermal->mode)
 		result = int3400_thermal_run_osc(priv->adev->handle,
 						priv->current_uuid_index,
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 43a516a35d64..69ef12f852b7 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -267,22 +267,6 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int of_thermal_set_mode(struct thermal_zone_device *tz,
-			       enum thermal_device_mode mode)
-{
-	struct __thermal_zone *data = tz->devdata;
-
-	if (mode == THERMAL_DEVICE_ENABLED) {
-		tz->polling_delay = data->polling_delay;
-		tz->passive_delay = data->passive_delay;
-	} else {
-		tz->polling_delay = 0;
-		tz->passive_delay = 0;
-	}
-
-	return 0;
-}
-
 static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
 				    enum thermal_trip_type *type)
 {
@@ -374,8 +358,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 }
 
 static struct thermal_zone_device_ops of_thermal_ops = {
-	.set_mode = of_thermal_set_mode,
-
 	.get_trip_type = of_thermal_get_trip_type,
 	.get_trip_temp = of_thermal_get_trip_temp,
 	.set_trip_temp = of_thermal_set_trip_temp,
-- 
2.17.1

[PATCH v7 11/11] thermal: Rename set_mode() to change_mode()

[Andrzej Pietrasiewicz]

set_mode() is only called when tzd's mode is about to change. Actual
setting is performed in thermal_core, in thermal_zone_device_set_mode().
The meaning of set_mode() callback is actually to notify the driver about
the mode being changed and giving the driver a chance to oppose such
change.

To better reflect the purpose of the method rename it to change_mode()

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
[for acerhdf]
Acked-by: Peter Kaestle <peter@piie.net>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/platform/x86/acerhdf.c                          | 6 +++---
 drivers/thermal/imx_thermal.c                           | 8 ++++----
 drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
 drivers/thermal/intel/intel_quark_dts_thermal.c         | 6 +++---
 drivers/thermal/thermal_core.c                          | 4 ++--
 include/linux/thermal.h                                 | 2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 76323855c80c..f816a8a13039 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -413,8 +413,8 @@ static inline void acerhdf_enable_kernelmode(void)
  *          the temperature and the fan.
  * disabled: the BIOS takes control of the fan.
  */
-static int acerhdf_set_mode(struct thermal_zone_device *thermal,
-			    enum thermal_device_mode mode)
+static int acerhdf_change_mode(struct thermal_zone_device *thermal,
+			       enum thermal_device_mode mode)
 {
 	if (mode == THERMAL_DEVICE_DISABLED && kernelmode)
 		acerhdf_revert_to_bios_mode();
@@ -473,7 +473,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
 	.bind = acerhdf_bind,
 	.unbind = acerhdf_unbind,
 	.get_temp = acerhdf_get_ec_temp,
-	.set_mode = acerhdf_set_mode,
+	.change_mode = acerhdf_change_mode,
 	.get_trip_type = acerhdf_get_trip_type,
 	.get_trip_hyst = acerhdf_get_trip_hyst,
 	.get_trip_temp = acerhdf_get_trip_temp,
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index a02398118d88..9700ae39feb7 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -330,8 +330,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
 	return 0;
 }
 
-static int imx_set_mode(struct thermal_zone_device *tz,
-			enum thermal_device_mode mode)
+static int imx_change_mode(struct thermal_zone_device *tz,
+			   enum thermal_device_mode mode)
 {
 	struct imx_thermal_data *data = tz->devdata;
 	struct regmap *map = data->tempmon;
@@ -447,7 +447,7 @@ static struct thermal_zone_device_ops imx_tz_ops = {
 	.bind = imx_bind,
 	.unbind = imx_unbind,
 	.get_temp = imx_get_temp,
-	.set_mode = imx_set_mode,
+	.change_mode = imx_change_mode,
 	.get_trip_type = imx_get_trip_type,
 	.get_trip_temp = imx_get_trip_temp,
 	.get_crit_temp = imx_get_crit_temp,
@@ -860,7 +860,7 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
 	 * Need to disable thermal sensor, otherwise, when thermal core
 	 * try to get temperature before thermal sensor resume, a wrong
 	 * temperature will be read as the thermal sensor is powered
-	 * down. This is done in set_mode() operation called from
+	 * down. This is done in change_mode() operation called from
 	 * thermal_zone_device_disable()
 	 */
 	ret = thermal_zone_device_disable(data->tz);
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index ce49d3b100d5..d3732f624913 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -377,8 +377,8 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
 	return 0;
 }
 
-static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
-				enum thermal_device_mode mode)
+static int int3400_thermal_change_mode(struct thermal_zone_device *thermal,
+				       enum thermal_device_mode mode)
 {
 	struct int3400_thermal_priv *priv = thermal->devdata;
 	int result = 0;
@@ -399,7 +399,7 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
 
 static struct thermal_zone_device_ops int3400_thermal_ops = {
 	.get_temp = int3400_thermal_get_temp,
-	.set_mode = int3400_thermal_set_mode,
+	.change_mode = int3400_thermal_change_mode,
 };
 
 static struct thermal_zone_params int3400_thermal_params = {
diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
index e29c3e330b17..3eafc6b0e6c3 100644
--- a/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -298,8 +298,8 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
 	return 0;
 }
 
-static int sys_set_mode(struct thermal_zone_device *tzd,
-				enum thermal_device_mode mode)
+static int sys_change_mode(struct thermal_zone_device *tzd,
+			   enum thermal_device_mode mode)
 {
 	int ret;
 
@@ -319,7 +319,7 @@ static struct thermal_zone_device_ops tzone_ops = {
 	.get_trip_type = sys_get_trip_type,
 	.set_trip_temp = sys_set_trip_temp,
 	.get_crit_temp = sys_get_crit_temp,
-	.set_mode = sys_set_mode,
+	.change_mode = sys_change_mode,
 };
 
 static void free_soc_dts(struct soc_sensor_entry *aux_entry)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index e613f5c07bad..a61e91513584 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -482,8 +482,8 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
 		return ret;
 	}
 
-	if (tz->ops->set_mode)
-		ret = tz->ops->set_mode(tz, mode);
+	if (tz->ops->change_mode)
+		ret = tz->ops->change_mode(tz, mode);
 
 	if (!ret)
 		tz->mode = mode;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index df013c39ba9b..b9efaa780d88 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -76,7 +76,7 @@ struct thermal_zone_device_ops {
 		       struct thermal_cooling_device *);
 	int (*get_temp) (struct thermal_zone_device *, int *);
 	int (*set_trips) (struct thermal_zone_device *, int, int);
-	int (*set_mode) (struct thermal_zone_device *,
+	int (*change_mode) (struct thermal_zone_device *,
 		enum thermal_device_mode);
 	int (*get_trip_type) (struct thermal_zone_device *, int,
 		enum thermal_trip_type *);
-- 
2.17.1

Re: [PATCH v7 07/11] thermal: Use mode helpers in drivers

[Amit Kucheria]

On Mon, Jun 29, 2020 at 5:59 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().
>
> Consequently, all set_mode() implementations in drivers:
>
> - can stop modifying tzd's "mode" member,
> - shall stop taking tzd's lock, as it is taken in the helpers
> - shall stop calling thermal_zone_device_update() as it is called in the
> helpers
> - can assume they are called when the mode truly changes, so checks to
> verify that can be dropped
>
> Not providing set_mode() by a driver no longer prevents the core from
> being able to set tzd's mode, so the relevant check in mode_store() is
> removed.
>
> Other comments:
>
> - acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
> return from set_mode(), so use function parameter in thermal_set_mode()
> instead, no need to call acpi_thermal_check() in set_mode()
> - thermal/imx_thermal.c: regmap writes and mode assignment are done in
> thermal_zone_device_{en|dis}able() and set_mode() callback
> - thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
> part of set_mode() callback, so they don't need to modify tzd->mode, and
> don't need to fall back to the opposite mode if unsuccessful, as the return
> value will be propagated to thermal_zone_device_{en|dis}able() and
> ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
> - thermal/of-thermal.c: no need to set zone->mode to DISABLED in
> of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> [for acerhdf]
> Acked-by: Peter Kaestle <peter@piie.net>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/acpi/thermal.c                        | 21 ++++++-----
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 37 +++++++++----------
>  drivers/platform/x86/acerhdf.c                | 17 +++++----
>  drivers/thermal/da9062-thermal.c              |  6 ++-
>  drivers/thermal/hisi_thermal.c                |  6 ++-
>  drivers/thermal/imx_thermal.c                 | 33 +++++++----------
>  .../intel/int340x_thermal/int3400_thermal.c   |  5 +--
>  .../thermal/intel/intel_quark_dts_thermal.c   | 18 ++-------
>  drivers/thermal/rockchip_thermal.c            |  6 ++-
>  drivers/thermal/sprd_thermal.c                |  6 ++-
>  drivers/thermal/thermal_core.c                |  2 +-
>  drivers/thermal/thermal_of.c                  | 10 +----
>  drivers/thermal/thermal_sysfs.c               | 11 ++----
>  13 files changed, 80 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 592be97c4456..52b6cda1bcc3 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -499,7 +499,7 @@ static void acpi_thermal_check(void *data)
>  {
>         struct acpi_thermal *tz = data;
>
> -       if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> +       if (!thermal_zone_device_is_enabled(tz->thermal_zone))
>                 return;
>
>         thermal_zone_device_update(tz->thermal_zone,
> @@ -542,14 +542,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
>         if (mode == THERMAL_DEVICE_DISABLED)
>                 pr_warn("thermal zone will be disabled\n");
>
> -       if (mode != tz->thermal_zone->mode) {
> -               tz->thermal_zone->mode = mode;
> -               ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> -                       "%s kernel ACPI thermal control\n",
> -                       tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> -                       "Enable" : "Disable"));
> -               acpi_thermal_check(tz);
> -       }
> +       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +               "%s kernel ACPI thermal control\n",
> +               mode == THERMAL_DEVICE_ENABLED ?
> +               "Enable" : "Disable"));
> +
>         return 0;
>  }
>
> @@ -897,13 +894,17 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>                 goto remove_dev_link;
>         }
>
> -       tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
> +       result = thermal_zone_device_enable(tz->thermal_zone);
> +       if (result)
> +               goto acpi_bus_detach;
>
>         dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>                  tz->thermal_zone->id);
>
>         return 0;
>
> +acpi_bus_detach:
> +       acpi_bus_detach_private_data(tz->device->handle);
>  remove_dev_link:
>         sysfs_remove_link(&tz->thermal_zone->device.kobj, "device");
>  remove_tz_link:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index ad61b2db30b8..4fb73d0fd167 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -280,18 +280,11 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
>  {
>         struct mlxsw_thermal *thermal = tzdev->devdata;
>
> -       mutex_lock(&tzdev->lock);
> -
>         if (mode == THERMAL_DEVICE_ENABLED)
>                 tzdev->polling_delay = thermal->polling_delay;
>         else
>                 tzdev->polling_delay = 0;
>
> -       tzdev->mode = mode;
> -       mutex_unlock(&tzdev->lock);
> -
> -       thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
> -
>         return 0;
>  }
>
> @@ -458,19 +451,11 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
>         struct mlxsw_thermal_module *tz = tzdev->devdata;
>         struct mlxsw_thermal *thermal = tz->parent;
>
> -       mutex_lock(&tzdev->lock);
> -
>         if (mode == THERMAL_DEVICE_ENABLED)
>                 tzdev->polling_delay = thermal->polling_delay;
>         else
>                 tzdev->polling_delay = 0;
>
> -       tzdev->mode = mode;
> -
> -       mutex_unlock(&tzdev->lock);
> -
> -       thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
> -
>         return 0;
>  }
>
> @@ -756,8 +741,11 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
>                 return err;
>         }
>
> -       module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> -       return 0;
> +       err = thermal_zone_device_enable(module_tz->tzdev);
> +       if (err)
> +               thermal_zone_device_unregister(module_tz->tzdev);
> +
> +       return err;
>  }
>
>  static void mlxsw_thermal_module_tz_fini(struct thermal_zone_device *tzdev)
> @@ -860,6 +848,7 @@ static int
>  mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
>  {
>         char tz_name[MLXSW_THERMAL_ZONE_MAX_NAME];
> +       int ret;
>
>         snprintf(tz_name, sizeof(tz_name), "mlxsw-gearbox%d",
>                  gearbox_tz->module + 1);
> @@ -872,8 +861,11 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
>         if (IS_ERR(gearbox_tz->tzdev))
>                 return PTR_ERR(gearbox_tz->tzdev);
>
> -       gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> -       return 0;
> +       ret = thermal_zone_device_enable(gearbox_tz->tzdev);
> +       if (ret)
> +               thermal_zone_device_unregister(gearbox_tz->tzdev);
> +
> +       return ret;
>  }
>
>  static void
> @@ -1041,10 +1033,15 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
>         if (err)
>                 goto err_unreg_modules_tzdev;
>
> -       thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
> +       err = thermal_zone_device_enable(thermal->tzdev);
> +       if (err)
> +               goto err_unreg_gearboxes;
> +
>         *p_thermal = thermal;
>         return 0;
>
> +err_unreg_gearboxes:
> +       mlxsw_thermal_gearboxes_fini(thermal);
>  err_unreg_modules_tzdev:
>         mlxsw_thermal_modules_fini(thermal);
>  err_unreg_tzdev:
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 58c4e1caaa09..8fe0ecb6a626 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -397,19 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
>  {
>         acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
>         kernelmode = 0;
> -       if (thz_dev) {
> -               thz_dev->mode = THERMAL_DEVICE_DISABLED;
> +       if (thz_dev)
>                 thz_dev->polling_delay = 0;
> -       }
> +
>         pr_notice("kernel mode fan control OFF\n");
>  }
>  static inline void acerhdf_enable_kernelmode(void)
>  {
>         kernelmode = 1;
> -       thz_dev->mode = THERMAL_DEVICE_ENABLED;
>
>         thz_dev->polling_delay = interval*1000;
> -       thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
>         pr_notice("kernel mode fan control ON\n");
>  }
>
> @@ -723,6 +720,8 @@ static void acerhdf_unregister_platform(void)
>
>  static int __init acerhdf_register_thermal(void)
>  {
> +       int ret;
> +
>         cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
>                                                  &acerhdf_cooling_ops);
>
> @@ -736,8 +735,12 @@ static int __init acerhdf_register_thermal(void)
>         if (IS_ERR(thz_dev))
>                 return -EINVAL;
>
> -       thz_dev->mode = kernelmode ?
> -               THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> +       if (kernelmode)
> +               ret = thermal_zone_device_enable(thz_dev);
> +       else
> +               ret = thermal_zone_device_disable(thz_dev);
> +       if (ret)
> +               return ret;
>
>         if (strcmp(thz_dev->governor->name,
>                                 acerhdf_zone_params.governor_name)) {
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index a7ac8afb063e..4d74994f160a 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -237,7 +237,11 @@ static int da9062_thermal_probe(struct platform_device *pdev)
>                 ret = PTR_ERR(thermal->zone);
>                 goto err;
>         }
> -       thermal->zone->mode = THERMAL_DEVICE_ENABLED;
> +       ret = thermal_zone_device_enable(thermal->zone);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Cannot enable thermal zone device\n");
> +               goto err_zone;
> +       }
>
>         dev_dbg(&pdev->dev,
>                 "TJUNC temperature polling period set at %d ms\n",
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index 2d26ae80e202..ee05950afd2f 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -549,8 +549,10 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>  {
>         struct thermal_zone_device *tzd = sensor->tzd;
>
> -       tzd->ops->set_mode(tzd,
> -               on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +       if (on)
> +               thermal_zone_device_enable(tzd);
> +       else
> +               thermal_zone_device_disable(tzd);
>  }
>
>  static int hisi_thermal_probe(struct platform_device *pdev)
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2c7ee5da608a..53abb1be1cba 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -255,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>         bool wait;
>         u32 val;
>
> -       if (tz->mode == THERMAL_DEVICE_ENABLED) {
> +       if (thermal_zone_device_is_enabled(tz)) {
>                 /* Check if a measurement is currently in progress */
>                 regmap_read(map, soc_data->temp_data, &val);
>                 wait = !(val & soc_data->temp_valid_mask);
> @@ -282,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>
>         regmap_read(map, soc_data->temp_data, &val);
>
> -       if (tz->mode != THERMAL_DEVICE_ENABLED) {
> +       if (!thermal_zone_device_is_enabled(tz)) {
>                 regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
>                              soc_data->measure_temp_mask);
>                 regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> @@ -365,9 +365,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
>                 }
>         }
>
> -       tz->mode = mode;
> -       thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
>         return 0;
>  }
>
> @@ -819,7 +816,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
>                      data->socdata->measure_temp_mask);
>
>         data->irq_enabled = true;
> -       data->tz->mode = THERMAL_DEVICE_ENABLED;
> +       ret = thermal_zone_device_enable(data->tz);
> +       if (ret)
> +               goto thermal_zone_unregister;
>
>         ret = devm_request_threaded_irq(&pdev->dev, data->irq,
>                         imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
> @@ -861,19 +860,18 @@ static int imx_thermal_remove(struct platform_device *pdev)
>  static int __maybe_unused imx_thermal_suspend(struct device *dev)
>  {
>         struct imx_thermal_data *data = dev_get_drvdata(dev);
> -       struct regmap *map = data->tempmon;
> +       int ret;
>
>         /*
>          * Need to disable thermal sensor, otherwise, when thermal core
>          * try to get temperature before thermal sensor resume, a wrong
>          * temperature will be read as the thermal sensor is powered
> -        * down.
> +        * down. This is done in set_mode() operation called from
> +        * thermal_zone_device_disable()
>          */
> -       regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
> -                    data->socdata->measure_temp_mask);
> -       regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> -                    data->socdata->power_down_mask);
> -       data->tz->mode = THERMAL_DEVICE_DISABLED;
> +       ret = thermal_zone_device_disable(data->tz);
> +       if (ret)
> +               return ret;
>         clk_disable_unprepare(data->thermal_clk);
>
>         return 0;
> @@ -882,18 +880,15 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
>  static int __maybe_unused imx_thermal_resume(struct device *dev)
>  {
>         struct imx_thermal_data *data = dev_get_drvdata(dev);
> -       struct regmap *map = data->tempmon;
>         int ret;
>
>         ret = clk_prepare_enable(data->thermal_clk);
>         if (ret)
>                 return ret;
>         /* Enabled thermal sensor after resume */
> -       regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
> -                    data->socdata->power_down_mask);
> -       regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> -                    data->socdata->measure_temp_mask);
> -       data->tz->mode = THERMAL_DEVICE_ENABLED;
> +       ret = thermal_zone_device_enable(data->tz);
> +       if (ret)
> +               return ret;
>
>         return 0;
>  }
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 9a622aaf29dd..3c0397a29b8c 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -390,12 +390,11 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>             mode != THERMAL_DEVICE_DISABLED)
>                 return -EINVAL;
>
> -       if (mode != thermal->mode) {
> -               thermal->mode = mode;
> +       if (mode != thermal->mode)
>                 result = int3400_thermal_run_osc(priv->adev->handle,
>                                                 priv->current_uuid_index,
>                                                 mode == THERMAL_DEVICE_ENABLED);
> -       }
> +
>
>         evaluate_odvp(priv);
>
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index c4879b4bfbf1..e29c3e330b17 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -126,10 +126,8 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
>         if (ret)
>                 return ret;
>
> -       if (out & QRK_DTS_ENABLE_BIT) {
> -               tzd->mode = THERMAL_DEVICE_ENABLED;
> +       if (out & QRK_DTS_ENABLE_BIT)
>                 return 0;
> -       }
>
>         if (!aux_entry->locked) {
>                 out |= QRK_DTS_ENABLE_BIT;
> @@ -137,10 +135,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
>                                      QRK_DTS_REG_OFFSET_ENABLE, out);
>                 if (ret)
>                         return ret;
> -
> -               tzd->mode = THERMAL_DEVICE_ENABLED;
>         } else {
> -               tzd->mode = THERMAL_DEVICE_DISABLED;
>                 pr_info("DTS is locked. Cannot enable DTS\n");
>                 ret = -EPERM;
>         }
> @@ -159,10 +154,8 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
>         if (ret)
>                 return ret;
>
> -       if (!(out & QRK_DTS_ENABLE_BIT)) {
> -               tzd->mode = THERMAL_DEVICE_DISABLED;
> +       if (!(out & QRK_DTS_ENABLE_BIT))
>                 return 0;
> -       }
>
>         if (!aux_entry->locked) {
>                 out &= ~QRK_DTS_ENABLE_BIT;
> @@ -171,10 +164,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
>
>                 if (ret)
>                         return ret;
> -
> -               tzd->mode = THERMAL_DEVICE_DISABLED;
>         } else {
> -               tzd->mode = THERMAL_DEVICE_ENABLED;
>                 pr_info("DTS is locked. Cannot disable DTS\n");
>                 ret = -EPERM;
>         }
> @@ -404,9 +394,7 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
>                 goto err_ret;
>         }
>
> -       mutex_lock(&dts_update_mutex);
> -       err = soc_dts_enable(aux_entry->tzone);
> -       mutex_unlock(&dts_update_mutex);
> +       err = thermal_zone_device_enable(aux_entry->tzone);
>         if (err)
>                 goto err_aux_status;
>
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 15a71ecc916c..aa9e0e31ef98 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -1068,8 +1068,10 @@ rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
>  {
>         struct thermal_zone_device *tzd = sensor->tzd;
>
> -       tzd->ops->set_mode(tzd,
> -               on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +       if (on)
> +               thermal_zone_device_enable(tzd);
> +       else
> +               thermal_zone_device_disable(tzd);
>  }
>
>  static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
> diff --git a/drivers/thermal/sprd_thermal.c b/drivers/thermal/sprd_thermal.c
> index a340374e8c51..58f995b0f804 100644
> --- a/drivers/thermal/sprd_thermal.c
> +++ b/drivers/thermal/sprd_thermal.c
> @@ -322,8 +322,10 @@ static void sprd_thm_toggle_sensor(struct sprd_thermal_sensor *sen, bool on)
>  {
>         struct thermal_zone_device *tzd = sen->tzd;
>
> -       tzd->ops->set_mode(tzd,
> -               on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +       if (on)
> +               thermal_zone_device_enable(tzd);
> +       else
> +               thermal_zone_device_disable(tzd);
>  }
>
>  static int sprd_thm_probe(struct platform_device *pdev)
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index f02c57c986f0..52d136780577 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1521,7 +1521,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
>         case PM_POST_SUSPEND:
>                 atomic_set(&in_suspend, 0);
>                 list_for_each_entry(tz, &thermal_tz_list, node) {
> -                       if (tz->mode == THERMAL_DEVICE_DISABLED)
> +                       if (!thermal_zone_device_is_enabled(tz))
>                                 continue;
>
>                         thermal_zone_device_init(tz);
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index ba65d48a48cb..43a516a35d64 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -272,8 +272,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>  {
>         struct __thermal_zone *data = tz->devdata;
>
> -       mutex_lock(&tz->lock);
> -
>         if (mode == THERMAL_DEVICE_ENABLED) {
>                 tz->polling_delay = data->polling_delay;
>                 tz->passive_delay = data->passive_delay;
> @@ -282,11 +280,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>                 tz->passive_delay = 0;
>         }
>
> -       mutex_unlock(&tz->lock);
> -
> -       tz->mode = mode;
> -       thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
>         return 0;
>  }
>
> @@ -541,7 +534,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>                         tzd = thermal_zone_of_add_sensor(child, sensor_np,
>                                                          data, ops);
>                         if (!IS_ERR(tzd))
> -                               tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +                               thermal_zone_device_enable(tzd);
>
>                         of_node_put(child);
>                         goto exit;
> @@ -1120,7 +1113,6 @@ int __init of_parse_thermal_zones(void)
>                         of_thermal_free_zone(tz);
>                         /* attempting to build remaining zones still */
>                 }
> -               zone->mode = THERMAL_DEVICE_DISABLED;
>         }
>         of_node_put(np);
>
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 096370977068..c23d67c4dc4e 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -49,9 +49,9 @@ static ssize_t
>  mode_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> +       int enabled = thermal_zone_device_is_enabled(tz);
>
> -       return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
> -                      "enabled" : "disabled");
> +       return sprintf(buf, "%s\n", enabled ? "enabled" : "disabled");
>  }
>
>  static ssize_t
> @@ -61,13 +61,10 @@ mode_store(struct device *dev, struct device_attribute *attr,
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
>         int result;
>
> -       if (!tz->ops->set_mode)
> -               return -EPERM;
> -
>         if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
> +               result = thermal_zone_device_enable(tz);
>         else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
> +               result = thermal_zone_device_disable(tz);
>         else
>                 result = -EINVAL;
>
> --
> 2.17.1
>

Re: [PATCH v7 09/11] thermal: core: Stop polling DISABLED thermal devices

[Amit Kucheria]

On Mon, Jun 29, 2020 at 6:00 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Polling DISABLED devices is not desired, as all such "disabled" devices
> are meant to be handled by userspace. This patch introduces and uses
> should_stop_polling() to decide whether the device should be polled or not.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/thermal/thermal_core.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 52d136780577..e613f5c07bad 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -301,13 +301,22 @@ static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
>                 cancel_delayed_work(&tz->poll_queue);
>  }
>
> +static inline bool should_stop_polling(struct thermal_zone_device *tz)
> +{
> +       return !thermal_zone_device_is_enabled(tz);
> +}
> +
>  static void monitor_thermal_zone(struct thermal_zone_device *tz)
>  {
> +       bool stop;
> +
> +       stop = should_stop_polling(tz);
> +
>         mutex_lock(&tz->lock);
>
> -       if (tz->passive)
> +       if (!stop && tz->passive)
>                 thermal_zone_device_set_polling(tz, tz->passive_delay);
> -       else if (tz->polling_delay)
> +       else if (!stop && tz->polling_delay)
>                 thermal_zone_device_set_polling(tz, tz->polling_delay);
>         else
>                 thermal_zone_device_set_polling(tz, 0);
> @@ -517,6 +526,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz,
>  {
>         int count;
>
> +       if (should_stop_polling(tz))
> +               return;
> +
>         if (atomic_read(&in_suspend))
>                 return;
>
> --
> 2.17.1
>

Re: [PATCH v7 10/11] thermal: Simplify or eliminate unnecessary set_mode() methods

[Amit Kucheria]

On Mon, Jun 29, 2020 at 6:00 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Setting polling_delay is now done at thermal_core level (by not polling
> DISABLED devices), so no need to repeat this code.
>
> int340x: Checking for an impossible enum value is unnecessary.
> acpi/thermal: It only prints debug messages.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> [for acerhdf]
> Acked-by: Peter Kaestle <peter@piie.net>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/acpi/thermal.c                        | 26 ----------------
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 30 -------------------
>  drivers/platform/x86/acerhdf.c                |  3 --
>  drivers/thermal/imx_thermal.c                 |  6 ----
>  .../intel/int340x_thermal/int3400_thermal.c   |  4 ---
>  drivers/thermal/thermal_of.c                  | 18 -----------
>  6 files changed, 87 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 52b6cda1bcc3..29a2b73fe035 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -525,31 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
>         return 0;
>  }
>
> -static int thermal_set_mode(struct thermal_zone_device *thermal,
> -                               enum thermal_device_mode mode)
> -{
> -       struct acpi_thermal *tz = thermal->devdata;
> -
> -       if (!tz)
> -               return -EINVAL;
> -
> -       if (mode != THERMAL_DEVICE_DISABLED &&
> -           mode != THERMAL_DEVICE_ENABLED)
> -               return -EINVAL;
> -       /*
> -        * enable/disable thermal management from ACPI thermal driver
> -        */
> -       if (mode == THERMAL_DEVICE_DISABLED)
> -               pr_warn("thermal zone will be disabled\n");
> -
> -       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> -               "%s kernel ACPI thermal control\n",
> -               mode == THERMAL_DEVICE_ENABLED ?
> -               "Enable" : "Disable"));
> -
> -       return 0;
> -}
> -
>  static int thermal_get_trip_type(struct thermal_zone_device *thermal,
>                                  int trip, enum thermal_trip_type *type)
>  {
> @@ -836,7 +811,6 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
>         .bind = acpi_thermal_bind_cooling_device,
>         .unbind = acpi_thermal_unbind_cooling_device,
>         .get_temp = thermal_get_temp,
> -       .set_mode = thermal_set_mode,
>         .get_trip_type = thermal_get_trip_type,
>         .get_trip_temp = thermal_get_trip_temp,
>         .get_crit_temp = thermal_get_crit_temp,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index 4fb73d0fd167..8fa286ccdd6b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -275,19 +275,6 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
>         return 0;
>  }
>
> -static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> -                                 enum thermal_device_mode mode)
> -{
> -       struct mlxsw_thermal *thermal = tzdev->devdata;
> -
> -       if (mode == THERMAL_DEVICE_ENABLED)
> -               tzdev->polling_delay = thermal->polling_delay;
> -       else
> -               tzdev->polling_delay = 0;
> -
> -       return 0;
> -}
> -
>  static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
>                                   int *p_temp)
>  {
> @@ -387,7 +374,6 @@ static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
>  static struct thermal_zone_device_ops mlxsw_thermal_ops = {
>         .bind = mlxsw_thermal_bind,
>         .unbind = mlxsw_thermal_unbind,
> -       .set_mode = mlxsw_thermal_set_mode,
>         .get_temp = mlxsw_thermal_get_temp,
>         .get_trip_type  = mlxsw_thermal_get_trip_type,
>         .get_trip_temp  = mlxsw_thermal_get_trip_temp,
> @@ -445,20 +431,6 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
>         return err;
>  }
>
> -static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> -                                        enum thermal_device_mode mode)
> -{
> -       struct mlxsw_thermal_module *tz = tzdev->devdata;
> -       struct mlxsw_thermal *thermal = tz->parent;
> -
> -       if (mode == THERMAL_DEVICE_ENABLED)
> -               tzdev->polling_delay = thermal->polling_delay;
> -       else
> -               tzdev->polling_delay = 0;
> -
> -       return 0;
> -}
> -
>  static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
>                                          int *p_temp)
>  {
> @@ -574,7 +546,6 @@ static int mlxsw_thermal_module_trend_get(struct thermal_zone_device *tzdev,
>  static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
>         .bind           = mlxsw_thermal_module_bind,
>         .unbind         = mlxsw_thermal_module_unbind,
> -       .set_mode       = mlxsw_thermal_module_mode_set,
>         .get_temp       = mlxsw_thermal_module_temp_get,
>         .get_trip_type  = mlxsw_thermal_module_trip_type_get,
>         .get_trip_temp  = mlxsw_thermal_module_trip_temp_get,
> @@ -612,7 +583,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
>  static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
>         .bind           = mlxsw_thermal_module_bind,
>         .unbind         = mlxsw_thermal_module_unbind,
> -       .set_mode       = mlxsw_thermal_module_mode_set,
>         .get_temp       = mlxsw_thermal_gearbox_temp_get,
>         .get_trip_type  = mlxsw_thermal_module_trip_type_get,
>         .get_trip_temp  = mlxsw_thermal_module_trip_temp_get,
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 8fe0ecb6a626..76323855c80c 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -397,8 +397,6 @@ static inline void acerhdf_revert_to_bios_mode(void)
>  {
>         acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
>         kernelmode = 0;
> -       if (thz_dev)
> -               thz_dev->polling_delay = 0;
>
>         pr_notice("kernel mode fan control OFF\n");
>  }
> @@ -406,7 +404,6 @@ static inline void acerhdf_enable_kernelmode(void)
>  {
>         kernelmode = 1;
>
> -       thz_dev->polling_delay = interval*1000;
>         pr_notice("kernel mode fan control ON\n");
>  }
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 53abb1be1cba..a02398118d88 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -338,9 +338,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
>         const struct thermal_soc_data *soc_data = data->socdata;
>
>         if (mode == THERMAL_DEVICE_ENABLED) {
> -               tz->polling_delay = IMX_POLLING_DELAY;
> -               tz->passive_delay = IMX_PASSIVE_DELAY;
> -
>                 regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
>                              soc_data->power_down_mask);
>                 regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> @@ -356,9 +353,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
>                 regmap_write(map, soc_data->sensor_ctrl + REG_SET,
>                              soc_data->power_down_mask);
>
> -               tz->polling_delay = 0;
> -               tz->passive_delay = 0;
> -
>                 if (data->irq_enabled) {
>                         disable_irq(data->irq);
>                         data->irq_enabled = false;
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 3c0397a29b8c..ce49d3b100d5 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -386,10 +386,6 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>         if (!priv)
>                 return -EINVAL;
>
> -       if (mode != THERMAL_DEVICE_ENABLED &&
> -           mode != THERMAL_DEVICE_DISABLED)
> -               return -EINVAL;
> -
>         if (mode != thermal->mode)
>                 result = int3400_thermal_run_osc(priv->adev->handle,
>                                                 priv->current_uuid_index,
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 43a516a35d64..69ef12f852b7 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -267,22 +267,6 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
>         return 0;
>  }
>
> -static int of_thermal_set_mode(struct thermal_zone_device *tz,
> -                              enum thermal_device_mode mode)
> -{
> -       struct __thermal_zone *data = tz->devdata;
> -
> -       if (mode == THERMAL_DEVICE_ENABLED) {
> -               tz->polling_delay = data->polling_delay;
> -               tz->passive_delay = data->passive_delay;
> -       } else {
> -               tz->polling_delay = 0;
> -               tz->passive_delay = 0;
> -       }
> -
> -       return 0;
> -}
> -
>  static int of_thermal_get_trip_type(struct thermal_zone_device *tz, int trip,
>                                     enum thermal_trip_type *type)
>  {
> @@ -374,8 +358,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>  }
>
>  static struct thermal_zone_device_ops of_thermal_ops = {
> -       .set_mode = of_thermal_set_mode,
> -
>         .get_trip_type = of_thermal_get_trip_type,
>         .get_trip_temp = of_thermal_get_trip_temp,
>         .set_trip_temp = of_thermal_set_trip_temp,
> --
> 2.17.1
>

Re: [PATCH v7 11/11] thermal: Rename set_mode() to change_mode()

[Amit Kucheria]

On Mon, Jun 29, 2020 at 6:00 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> set_mode() is only called when tzd's mode is about to change. Actual
> setting is performed in thermal_core, in thermal_zone_device_set_mode().
> The meaning of set_mode() callback is actually to notify the driver about
> the mode being changed and giving the driver a chance to oppose such
> change.
>
> To better reflect the purpose of the method rename it to change_mode()
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> [for acerhdf]
> Acked-by: Peter Kaestle <peter@piie.net>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/platform/x86/acerhdf.c                          | 6 +++---
>  drivers/thermal/imx_thermal.c                           | 8 ++++----
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
>  drivers/thermal/intel/intel_quark_dts_thermal.c         | 6 +++---
>  drivers/thermal/thermal_core.c                          | 4 ++--
>  include/linux/thermal.h                                 | 2 +-
>  6 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 76323855c80c..f816a8a13039 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -413,8 +413,8 @@ static inline void acerhdf_enable_kernelmode(void)
>   *          the temperature and the fan.
>   * disabled: the BIOS takes control of the fan.
>   */
> -static int acerhdf_set_mode(struct thermal_zone_device *thermal,
> -                           enum thermal_device_mode mode)
> +static int acerhdf_change_mode(struct thermal_zone_device *thermal,
> +                              enum thermal_device_mode mode)
>  {
>         if (mode == THERMAL_DEVICE_DISABLED && kernelmode)
>                 acerhdf_revert_to_bios_mode();
> @@ -473,7 +473,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
>         .bind = acerhdf_bind,
>         .unbind = acerhdf_unbind,
>         .get_temp = acerhdf_get_ec_temp,
> -       .set_mode = acerhdf_set_mode,
> +       .change_mode = acerhdf_change_mode,
>         .get_trip_type = acerhdf_get_trip_type,
>         .get_trip_hyst = acerhdf_get_trip_hyst,
>         .get_trip_temp = acerhdf_get_trip_temp,
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index a02398118d88..9700ae39feb7 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -330,8 +330,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>         return 0;
>  }
>
> -static int imx_set_mode(struct thermal_zone_device *tz,
> -                       enum thermal_device_mode mode)
> +static int imx_change_mode(struct thermal_zone_device *tz,
> +                          enum thermal_device_mode mode)
>  {
>         struct imx_thermal_data *data = tz->devdata;
>         struct regmap *map = data->tempmon;
> @@ -447,7 +447,7 @@ static struct thermal_zone_device_ops imx_tz_ops = {
>         .bind = imx_bind,
>         .unbind = imx_unbind,
>         .get_temp = imx_get_temp,
> -       .set_mode = imx_set_mode,
> +       .change_mode = imx_change_mode,
>         .get_trip_type = imx_get_trip_type,
>         .get_trip_temp = imx_get_trip_temp,
>         .get_crit_temp = imx_get_crit_temp,
> @@ -860,7 +860,7 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
>          * Need to disable thermal sensor, otherwise, when thermal core
>          * try to get temperature before thermal sensor resume, a wrong
>          * temperature will be read as the thermal sensor is powered
> -        * down. This is done in set_mode() operation called from
> +        * down. This is done in change_mode() operation called from
>          * thermal_zone_device_disable()
>          */
>         ret = thermal_zone_device_disable(data->tz);
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index ce49d3b100d5..d3732f624913 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -377,8 +377,8 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
>         return 0;
>  }
>
> -static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> -                               enum thermal_device_mode mode)
> +static int int3400_thermal_change_mode(struct thermal_zone_device *thermal,
> +                                      enum thermal_device_mode mode)
>  {
>         struct int3400_thermal_priv *priv = thermal->devdata;
>         int result = 0;
> @@ -399,7 +399,7 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>
>  static struct thermal_zone_device_ops int3400_thermal_ops = {
>         .get_temp = int3400_thermal_get_temp,
> -       .set_mode = int3400_thermal_set_mode,
> +       .change_mode = int3400_thermal_change_mode,
>  };
>
>  static struct thermal_zone_params int3400_thermal_params = {
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index e29c3e330b17..3eafc6b0e6c3 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -298,8 +298,8 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
>         return 0;
>  }
>
> -static int sys_set_mode(struct thermal_zone_device *tzd,
> -                               enum thermal_device_mode mode)
> +static int sys_change_mode(struct thermal_zone_device *tzd,
> +                          enum thermal_device_mode mode)
>  {
>         int ret;
>
> @@ -319,7 +319,7 @@ static struct thermal_zone_device_ops tzone_ops = {
>         .get_trip_type = sys_get_trip_type,
>         .set_trip_temp = sys_set_trip_temp,
>         .get_crit_temp = sys_get_crit_temp,
> -       .set_mode = sys_set_mode,
> +       .change_mode = sys_change_mode,
>  };
>
>  static void free_soc_dts(struct soc_sensor_entry *aux_entry)
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index e613f5c07bad..a61e91513584 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -482,8 +482,8 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
>                 return ret;
>         }
>
> -       if (tz->ops->set_mode)
> -               ret = tz->ops->set_mode(tz, mode);
> +       if (tz->ops->change_mode)
> +               ret = tz->ops->change_mode(tz, mode);
>
>         if (!ret)
>                 tz->mode = mode;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index df013c39ba9b..b9efaa780d88 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -76,7 +76,7 @@ struct thermal_zone_device_ops {
>                        struct thermal_cooling_device *);
>         int (*get_temp) (struct thermal_zone_device *, int *);
>         int (*set_trips) (struct thermal_zone_device *, int, int);
> -       int (*set_mode) (struct thermal_zone_device *,
> +       int (*change_mode) (struct thermal_zone_device *,
>                 enum thermal_device_mode);
>         int (*get_trip_type) (struct thermal_zone_device *, int,
>                 enum thermal_trip_type *);
> --
> 2.17.1
>

Re: [PATCH v7 07/11] thermal: Use mode helpers in drivers

[Bartlomiej Zolnierkiewicz]

On 6/29/20 2:29 PM, Andrzej Pietrasiewicz wrote:
> Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().
> 
> Consequently, all set_mode() implementations in drivers:
> 
> - can stop modifying tzd's "mode" member,
> - shall stop taking tzd's lock, as it is taken in the helpers
> - shall stop calling thermal_zone_device_update() as it is called in the
> helpers
> - can assume they are called when the mode truly changes, so checks to
> verify that can be dropped
> 
> Not providing set_mode() by a driver no longer prevents the core from
> being able to set tzd's mode, so the relevant check in mode_store() is
> removed.
> 
> Other comments:
> 
> - acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
> return from set_mode(), so use function parameter in thermal_set_mode()
> instead, no need to call acpi_thermal_check() in set_mode()
> - thermal/imx_thermal.c: regmap writes and mode assignment are done in
> thermal_zone_device_{en|dis}able() and set_mode() callback
> - thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
> part of set_mode() callback, so they don't need to modify tzd->mode, and
> don't need to fall back to the opposite mode if unsuccessful, as the return
> value will be propagated to thermal_zone_device_{en|dis}able() and
> ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
> - thermal/of-thermal.c: no need to set zone->mode to DISABLED in
> of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> [for acerhdf]
> Acked-by: Peter Kaestle <peter@piie.net>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> ---
>  drivers/acpi/thermal.c                        | 21 ++++++-----
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 37 +++++++++----------
>  drivers/platform/x86/acerhdf.c                | 17 +++++----
>  drivers/thermal/da9062-thermal.c              |  6 ++-
>  drivers/thermal/hisi_thermal.c                |  6 ++-
>  drivers/thermal/imx_thermal.c                 | 33 +++++++----------
>  .../intel/int340x_thermal/int3400_thermal.c   |  5 +--
>  .../thermal/intel/intel_quark_dts_thermal.c   | 18 ++-------
>  drivers/thermal/rockchip_thermal.c            |  6 ++-
>  drivers/thermal/sprd_thermal.c                |  6 ++-
>  drivers/thermal/thermal_core.c                |  2 +-
>  drivers/thermal/thermal_of.c                  | 10 +----
>  drivers/thermal/thermal_sysfs.c               | 11 ++----
>  13 files changed, 80 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 592be97c4456..52b6cda1bcc3 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -499,7 +499,7 @@ static void acpi_thermal_check(void *data)
>  {
>  	struct acpi_thermal *tz = data;
>  
> -	if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> +	if (!thermal_zone_device_is_enabled(tz->thermal_zone))
>  		return;
>  
>  	thermal_zone_device_update(tz->thermal_zone,
> @@ -542,14 +542,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
>  	if (mode == THERMAL_DEVICE_DISABLED)
>  		pr_warn("thermal zone will be disabled\n");
>  
> -	if (mode != tz->thermal_zone->mode) {
> -		tz->thermal_zone->mode = mode;
> -		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> -			"%s kernel ACPI thermal control\n",
> -			tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> -			"Enable" : "Disable"));
> -		acpi_thermal_check(tz);
> -	}
> +	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +		"%s kernel ACPI thermal control\n",
> +		mode == THERMAL_DEVICE_ENABLED ?
> +		"Enable" : "Disable"));
> +
>  	return 0;
>  }
>  
> @@ -897,13 +894,17 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>  		goto remove_dev_link;
>  	}
>  
> -	tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
> +	result = thermal_zone_device_enable(tz->thermal_zone);
> +	if (result)
> +		goto acpi_bus_detach;
>  
>  	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>  		 tz->thermal_zone->id);
>  
>  	return 0;
>  
> +acpi_bus_detach:
> +	acpi_bus_detach_private_data(tz->device->handle);
>  remove_dev_link:
>  	sysfs_remove_link(&tz->thermal_zone->device.kobj, "device");
>  remove_tz_link:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index ad61b2db30b8..4fb73d0fd167 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -280,18 +280,11 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
>  {
>  	struct mlxsw_thermal *thermal = tzdev->devdata;
>  
> -	mutex_lock(&tzdev->lock);
> -
>  	if (mode == THERMAL_DEVICE_ENABLED)
>  		tzdev->polling_delay = thermal->polling_delay;
>  	else
>  		tzdev->polling_delay = 0;
>  
> -	tzdev->mode = mode;
> -	mutex_unlock(&tzdev->lock);
> -
> -	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
> -
>  	return 0;
>  }
>  
> @@ -458,19 +451,11 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
>  	struct mlxsw_thermal_module *tz = tzdev->devdata;
>  	struct mlxsw_thermal *thermal = tz->parent;
>  
> -	mutex_lock(&tzdev->lock);
> -
>  	if (mode == THERMAL_DEVICE_ENABLED)
>  		tzdev->polling_delay = thermal->polling_delay;
>  	else
>  		tzdev->polling_delay = 0;
>  
> -	tzdev->mode = mode;
> -
> -	mutex_unlock(&tzdev->lock);
> -
> -	thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
> -
>  	return 0;
>  }
>  
> @@ -756,8 +741,11 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
>  		return err;
>  	}
>  
> -	module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> -	return 0;
> +	err = thermal_zone_device_enable(module_tz->tzdev);
> +	if (err)
> +		thermal_zone_device_unregister(module_tz->tzdev);
> +
> +	return err;
>  }
>  
>  static void mlxsw_thermal_module_tz_fini(struct thermal_zone_device *tzdev)
> @@ -860,6 +848,7 @@ static int
>  mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
>  {
>  	char tz_name[MLXSW_THERMAL_ZONE_MAX_NAME];
> +	int ret;
>  
>  	snprintf(tz_name, sizeof(tz_name), "mlxsw-gearbox%d",
>  		 gearbox_tz->module + 1);
> @@ -872,8 +861,11 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
>  	if (IS_ERR(gearbox_tz->tzdev))
>  		return PTR_ERR(gearbox_tz->tzdev);
>  
> -	gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> -	return 0;
> +	ret = thermal_zone_device_enable(gearbox_tz->tzdev);
> +	if (ret)
> +		thermal_zone_device_unregister(gearbox_tz->tzdev);
> +
> +	return ret;
>  }
>  
>  static void
> @@ -1041,10 +1033,15 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
>  	if (err)
>  		goto err_unreg_modules_tzdev;
>  
> -	thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
> +	err = thermal_zone_device_enable(thermal->tzdev);
> +	if (err)
> +		goto err_unreg_gearboxes;
> +
>  	*p_thermal = thermal;
>  	return 0;
>  
> +err_unreg_gearboxes:
> +	mlxsw_thermal_gearboxes_fini(thermal);
>  err_unreg_modules_tzdev:
>  	mlxsw_thermal_modules_fini(thermal);
>  err_unreg_tzdev:
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 58c4e1caaa09..8fe0ecb6a626 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -397,19 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
>  {
>  	acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
>  	kernelmode = 0;
> -	if (thz_dev) {
> -		thz_dev->mode = THERMAL_DEVICE_DISABLED;
> +	if (thz_dev)
>  		thz_dev->polling_delay = 0;
> -	}
> +
>  	pr_notice("kernel mode fan control OFF\n");
>  }
>  static inline void acerhdf_enable_kernelmode(void)
>  {
>  	kernelmode = 1;
> -	thz_dev->mode = THERMAL_DEVICE_ENABLED;
>  
>  	thz_dev->polling_delay = interval*1000;
> -	thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
>  	pr_notice("kernel mode fan control ON\n");
>  }
>  
> @@ -723,6 +720,8 @@ static void acerhdf_unregister_platform(void)
>  
>  static int __init acerhdf_register_thermal(void)
>  {
> +	int ret;
> +
>  	cl_dev = thermal_cooling_device_register("acerhdf-fan", NULL,
>  						 &acerhdf_cooling_ops);
>  
> @@ -736,8 +735,12 @@ static int __init acerhdf_register_thermal(void)
>  	if (IS_ERR(thz_dev))
>  		return -EINVAL;
>  
> -	thz_dev->mode = kernelmode ?
> -		THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> +	if (kernelmode)
> +		ret = thermal_zone_device_enable(thz_dev);
> +	else
> +		ret = thermal_zone_device_disable(thz_dev);
> +	if (ret)
> +		return ret;
>  
>  	if (strcmp(thz_dev->governor->name,
>  				acerhdf_zone_params.governor_name)) {
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index a7ac8afb063e..4d74994f160a 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -237,7 +237,11 @@ static int da9062_thermal_probe(struct platform_device *pdev)
>  		ret = PTR_ERR(thermal->zone);
>  		goto err;
>  	}
> -	thermal->zone->mode = THERMAL_DEVICE_ENABLED;
> +	ret = thermal_zone_device_enable(thermal->zone);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot enable thermal zone device\n");
> +		goto err_zone;
> +	}
>  
>  	dev_dbg(&pdev->dev,
>  		"TJUNC temperature polling period set at %d ms\n",
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index 2d26ae80e202..ee05950afd2f 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -549,8 +549,10 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>  {
>  	struct thermal_zone_device *tzd = sensor->tzd;
>  
> -	tzd->ops->set_mode(tzd,
> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +	if (on)
> +		thermal_zone_device_enable(tzd);
> +	else
> +		thermal_zone_device_disable(tzd);
>  }
>  
>  static int hisi_thermal_probe(struct platform_device *pdev)
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2c7ee5da608a..53abb1be1cba 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -255,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>  	bool wait;
>  	u32 val;
>  
> -	if (tz->mode == THERMAL_DEVICE_ENABLED) {
> +	if (thermal_zone_device_is_enabled(tz)) {
>  		/* Check if a measurement is currently in progress */
>  		regmap_read(map, soc_data->temp_data, &val);
>  		wait = !(val & soc_data->temp_valid_mask);
> @@ -282,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>  
>  	regmap_read(map, soc_data->temp_data, &val);
>  
> -	if (tz->mode != THERMAL_DEVICE_ENABLED) {
> +	if (!thermal_zone_device_is_enabled(tz)) {
>  		regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
>  			     soc_data->measure_temp_mask);
>  		regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> @@ -365,9 +365,6 @@ static int imx_set_mode(struct thermal_zone_device *tz,
>  		}
>  	}
>  
> -	tz->mode = mode;
> -	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
>  	return 0;
>  }
>  
> @@ -819,7 +816,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
>  		     data->socdata->measure_temp_mask);
>  
>  	data->irq_enabled = true;
> -	data->tz->mode = THERMAL_DEVICE_ENABLED;
> +	ret = thermal_zone_device_enable(data->tz);
> +	if (ret)
> +		goto thermal_zone_unregister;
>  
>  	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
>  			imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
> @@ -861,19 +860,18 @@ static int imx_thermal_remove(struct platform_device *pdev)
>  static int __maybe_unused imx_thermal_suspend(struct device *dev)
>  {
>  	struct imx_thermal_data *data = dev_get_drvdata(dev);
> -	struct regmap *map = data->tempmon;
> +	int ret;
>  
>  	/*
>  	 * Need to disable thermal sensor, otherwise, when thermal core
>  	 * try to get temperature before thermal sensor resume, a wrong
>  	 * temperature will be read as the thermal sensor is powered
> -	 * down.
> +	 * down. This is done in set_mode() operation called from
> +	 * thermal_zone_device_disable()
>  	 */
> -	regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
> -		     data->socdata->measure_temp_mask);
> -	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> -		     data->socdata->power_down_mask);
> -	data->tz->mode = THERMAL_DEVICE_DISABLED;
> +	ret = thermal_zone_device_disable(data->tz);
> +	if (ret)
> +		return ret;
>  	clk_disable_unprepare(data->thermal_clk);
>  
>  	return 0;
> @@ -882,18 +880,15 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
>  static int __maybe_unused imx_thermal_resume(struct device *dev)
>  {
>  	struct imx_thermal_data *data = dev_get_drvdata(dev);
> -	struct regmap *map = data->tempmon;
>  	int ret;
>  
>  	ret = clk_prepare_enable(data->thermal_clk);
>  	if (ret)
>  		return ret;
>  	/* Enabled thermal sensor after resume */
> -	regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
> -		     data->socdata->power_down_mask);
> -	regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> -		     data->socdata->measure_temp_mask);
> -	data->tz->mode = THERMAL_DEVICE_ENABLED;
> +	ret = thermal_zone_device_enable(data->tz);
> +	if (ret)
> +		return ret;
>  
>  	return 0;
>  }
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 9a622aaf29dd..3c0397a29b8c 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -390,12 +390,11 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>  	    mode != THERMAL_DEVICE_DISABLED)
>  		return -EINVAL;
>  
> -	if (mode != thermal->mode) {
> -		thermal->mode = mode;
> +	if (mode != thermal->mode)
>  		result = int3400_thermal_run_osc(priv->adev->handle,
>  						priv->current_uuid_index,
>  						mode == THERMAL_DEVICE_ENABLED);
> -	}
> +
>  
>  	evaluate_odvp(priv);
>  
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index c4879b4bfbf1..e29c3e330b17 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -126,10 +126,8 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
>  	if (ret)
>  		return ret;
>  
> -	if (out & QRK_DTS_ENABLE_BIT) {
> -		tzd->mode = THERMAL_DEVICE_ENABLED;
> +	if (out & QRK_DTS_ENABLE_BIT)
>  		return 0;
> -	}
>  
>  	if (!aux_entry->locked) {
>  		out |= QRK_DTS_ENABLE_BIT;
> @@ -137,10 +135,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
>  				     QRK_DTS_REG_OFFSET_ENABLE, out);
>  		if (ret)
>  			return ret;
> -
> -		tzd->mode = THERMAL_DEVICE_ENABLED;
>  	} else {
> -		tzd->mode = THERMAL_DEVICE_DISABLED;
>  		pr_info("DTS is locked. Cannot enable DTS\n");
>  		ret = -EPERM;
>  	}
> @@ -159,10 +154,8 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
>  	if (ret)
>  		return ret;
>  
> -	if (!(out & QRK_DTS_ENABLE_BIT)) {
> -		tzd->mode = THERMAL_DEVICE_DISABLED;
> +	if (!(out & QRK_DTS_ENABLE_BIT))
>  		return 0;
> -	}
>  
>  	if (!aux_entry->locked) {
>  		out &= ~QRK_DTS_ENABLE_BIT;
> @@ -171,10 +164,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
>  
>  		if (ret)
>  			return ret;
> -
> -		tzd->mode = THERMAL_DEVICE_DISABLED;
>  	} else {
> -		tzd->mode = THERMAL_DEVICE_ENABLED;
>  		pr_info("DTS is locked. Cannot disable DTS\n");
>  		ret = -EPERM;
>  	}
> @@ -404,9 +394,7 @@ static struct soc_sensor_entry *alloc_soc_dts(void)
>  		goto err_ret;
>  	}
>  
> -	mutex_lock(&dts_update_mutex);
> -	err = soc_dts_enable(aux_entry->tzone);
> -	mutex_unlock(&dts_update_mutex);
> +	err = thermal_zone_device_enable(aux_entry->tzone);
>  	if (err)
>  		goto err_aux_status;
>  
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 15a71ecc916c..aa9e0e31ef98 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -1068,8 +1068,10 @@ rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
>  {
>  	struct thermal_zone_device *tzd = sensor->tzd;
>  
> -	tzd->ops->set_mode(tzd,
> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +	if (on)
> +		thermal_zone_device_enable(tzd);
> +	else
> +		thermal_zone_device_disable(tzd);
>  }
>  
>  static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
> diff --git a/drivers/thermal/sprd_thermal.c b/drivers/thermal/sprd_thermal.c
> index a340374e8c51..58f995b0f804 100644
> --- a/drivers/thermal/sprd_thermal.c
> +++ b/drivers/thermal/sprd_thermal.c
> @@ -322,8 +322,10 @@ static void sprd_thm_toggle_sensor(struct sprd_thermal_sensor *sen, bool on)
>  {
>  	struct thermal_zone_device *tzd = sen->tzd;
>  
> -	tzd->ops->set_mode(tzd,
> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> +	if (on)
> +		thermal_zone_device_enable(tzd);
> +	else
> +		thermal_zone_device_disable(tzd);
>  }
>  
>  static int sprd_thm_probe(struct platform_device *pdev)
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index f02c57c986f0..52d136780577 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1521,7 +1521,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
>  	case PM_POST_SUSPEND:
>  		atomic_set(&in_suspend, 0);
>  		list_for_each_entry(tz, &thermal_tz_list, node) {
> -			if (tz->mode == THERMAL_DEVICE_DISABLED)
> +			if (!thermal_zone_device_is_enabled(tz))
>  				continue;
>  
>  			thermal_zone_device_init(tz);
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index ba65d48a48cb..43a516a35d64 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -272,8 +272,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>  {
>  	struct __thermal_zone *data = tz->devdata;
>  
> -	mutex_lock(&tz->lock);
> -
>  	if (mode == THERMAL_DEVICE_ENABLED) {
>  		tz->polling_delay = data->polling_delay;
>  		tz->passive_delay = data->passive_delay;
> @@ -282,11 +280,6 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>  		tz->passive_delay = 0;
>  	}
>  
> -	mutex_unlock(&tz->lock);
> -
> -	tz->mode = mode;
> -	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
>  	return 0;
>  }
>  
> @@ -541,7 +534,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
>  			if (!IS_ERR(tzd))
> -				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +				thermal_zone_device_enable(tzd);
>  
>  			of_node_put(child);
>  			goto exit;
> @@ -1120,7 +1113,6 @@ int __init of_parse_thermal_zones(void)
>  			of_thermal_free_zone(tz);
>  			/* attempting to build remaining zones still */
>  		}
> -		zone->mode = THERMAL_DEVICE_DISABLED;
>  	}
>  	of_node_put(np);
>  
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 096370977068..c23d67c4dc4e 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -49,9 +49,9 @@ static ssize_t
>  mode_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> +	int enabled = thermal_zone_device_is_enabled(tz);
>  
> -	return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
> -		       "enabled" : "disabled");
> +	return sprintf(buf, "%s\n", enabled ? "enabled" : "disabled");
>  }
>  
>  static ssize_t
> @@ -61,13 +61,10 @@ mode_store(struct device *dev, struct device_attribute *attr,
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>  	int result;
>  
> -	if (!tz->ops->set_mode)
> -		return -EPERM;
> -
>  	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
> +		result = thermal_zone_device_enable(tz);
>  	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
> +		result = thermal_zone_device_disable(tz);
>  	else
>  		result = -EINVAL;
>  
> 

Re: [PATCH v7 08/11] thermal: Explicitly enable non-changing thermal zone devices

[Bartlomiej Zolnierkiewicz]

On 6/29/20 2:29 PM, Andrzej Pietrasiewicz wrote:
> Some thermal zone devices never change their state, so they should be
> always enabled.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> ---
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c       | 8 ++++++++
>  drivers/net/wireless/intel/iwlwifi/mvm/tt.c              | 9 ++++++++-
>  drivers/platform/x86/intel_mid_thermal.c                 | 6 ++++++
>  drivers/power/supply/power_supply_core.c                 | 9 +++++++--
>  drivers/thermal/armada_thermal.c                         | 6 ++++++
>  drivers/thermal/dove_thermal.c                           | 6 ++++++
>  .../thermal/intel/int340x_thermal/int340x_thermal_zone.c | 5 +++++
>  drivers/thermal/intel/intel_pch_thermal.c                | 5 +++++
>  drivers/thermal/intel/intel_soc_dts_iosf.c               | 3 +++
>  drivers/thermal/intel/x86_pkg_temp_thermal.c             | 6 ++++++
>  drivers/thermal/kirkwood_thermal.c                       | 7 +++++++
>  drivers/thermal/rcar_thermal.c                           | 9 ++++++++-
>  drivers/thermal/spear_thermal.c                          | 7 +++++++
>  drivers/thermal/st/st_thermal.c                          | 5 +++++
>  14 files changed, 87 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> index 3de8a5e83b6c..e3510e9b21f3 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> @@ -92,6 +92,14 @@ int cxgb4_thermal_init(struct adapter *adap)
>  		ch_thermal->tzdev = NULL;
>  		return ret;
>  	}
> +
> +	ret = thermal_zone_device_enable(ch_thermal->tzdev);
> +	if (ret) {
> +		dev_err(adap->pdev_dev, "Failed to enable thermal zone\n");
> +		thermal_zone_device_unregister(adap->ch_thermal.tzdev);
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> index 418e59b7c671..0c95663bf9ed 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> @@ -733,7 +733,7 @@ static  struct thermal_zone_device_ops tzone_ops = {
>  
>  static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
>  {
> -	int i;
> +	int i, ret;
>  	char name[16];
>  	static atomic_t counter = ATOMIC_INIT(0);
>  
> @@ -759,6 +759,13 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
>  		return;
>  	}
>  
> +	ret = thermal_zone_device_enable(mvm->tz_device.tzone);
> +	if (ret) {
> +		IWL_DEBUG_TEMP(mvm, "Failed to enable thermal zone\n");
> +		thermal_zone_device_unregister(mvm->tz_device.tzone);
> +		return;
> +	}
> +
>  	/* 0 is a valid temperature,
>  	 * so initialize the array with S16_MIN which invalid temperature
>  	 */
> diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
> index f402e2e74a38..f12f4e7bd971 100644
> --- a/drivers/platform/x86/intel_mid_thermal.c
> +++ b/drivers/platform/x86/intel_mid_thermal.c
> @@ -493,6 +493,12 @@ static int mid_thermal_probe(struct platform_device *pdev)
>  			ret = PTR_ERR(pinfo->tzd[i]);
>  			goto err;
>  		}
> +		ret = thermal_zone_device_enable(pinfo->tzd[i]);
> +		if (ret) {
> +			kfree(td_info);
> +			thermal_zone_device_unregister(pinfo->tzd[i]);
> +			goto err;
> +		}
>  	}
>  
>  	pinfo->pdev = pdev;
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 02b37fe6061c..90e56736d479 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -939,7 +939,7 @@ static struct thermal_zone_device_ops psy_tzd_ops = {
>  
>  static int psy_register_thermal(struct power_supply *psy)
>  {
> -	int i;
> +	int i, ret;
>  
>  	if (psy->desc->no_thermal)
>  		return 0;
> @@ -949,7 +949,12 @@ static int psy_register_thermal(struct power_supply *psy)
>  		if (psy->desc->properties[i] == POWER_SUPPLY_PROP_TEMP) {
>  			psy->tzd = thermal_zone_device_register(psy->desc->name,
>  					0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
> -			return PTR_ERR_OR_ZERO(psy->tzd);
> +			if (IS_ERR(psy->tzd))
> +				return PTR_ERR(psy->tzd);
> +			ret = thermal_zone_device_enable(psy->tzd);
> +			if (ret)
> +				thermal_zone_device_unregister(psy->tzd);
> +			return ret;
>  		}
>  	}
>  	return 0;
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 7c447cd149e7..c2ebfb5be4b3 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -874,6 +874,12 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  			return PTR_ERR(tz);
>  		}
>  
> +		ret = thermal_zone_device_enable(tz);
> +		if (ret) {
> +			thermal_zone_device_unregister(tz);
> +			return ret;
> +		}
> +
>  		drvdata->type = LEGACY;
>  		drvdata->data.tz = tz;
>  		platform_set_drvdata(pdev, drvdata);
> diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
> index 75901ced4a62..73182eb94bc0 100644
> --- a/drivers/thermal/dove_thermal.c
> +++ b/drivers/thermal/dove_thermal.c
> @@ -153,6 +153,12 @@ static int dove_thermal_probe(struct platform_device *pdev)
>  		return PTR_ERR(thermal);
>  	}
>  
> +	ret = thermal_zone_device_enable(thermal);
> +	if (ret) {
> +		thermal_zone_device_unregister(thermal);
> +		return ret;
> +	}
> +
>  	platform_set_drvdata(pdev, thermal);
>  
>  	return 0;
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 432213272f1e..6e479deff76b 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -259,9 +259,14 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>  		ret = PTR_ERR(int34x_thermal_zone->zone);
>  		goto err_thermal_zone;
>  	}
> +	ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
> +	if (ret)
> +		goto err_enable;
>  
>  	return int34x_thermal_zone;
>  
> +err_enable:
> +	thermal_zone_device_unregister(int34x_thermal_zone->zone);
>  err_thermal_zone:
>  	acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
>  	kfree(int34x_thermal_zone->aux_trips);
> diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
> index 56401fd4708d..65702094f3d3 100644
> --- a/drivers/thermal/intel/intel_pch_thermal.c
> +++ b/drivers/thermal/intel/intel_pch_thermal.c
> @@ -352,9 +352,14 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>  		err = PTR_ERR(ptd->tzd);
>  		goto error_cleanup;
>  	}
> +	err = thermal_zone_device_enable(ptd->tzd);
> +	if (err)
> +		goto err_unregister;
>  
>  	return 0;
>  
> +err_unregister:
> +	thermal_zone_device_unregister(ptd->tzd);
>  error_cleanup:
>  	iounmap(ptd->hw_base);
>  error_release:
> diff --git a/drivers/thermal/intel/intel_soc_dts_iosf.c b/drivers/thermal/intel/intel_soc_dts_iosf.c
> index f75271b669c6..4f1a2f7c016c 100644
> --- a/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ b/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -329,6 +329,9 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
>  		ret = PTR_ERR(dts->tzone);
>  		goto err_ret;
>  	}
> +	ret = thermal_zone_device_enable(dts->tzone);
> +	if (ret)
> +		goto err_enable;
>  
>  	ret = soc_dts_enable(id);
>  	if (ret)
> diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> index a006b9fd1d72..b81c33202f41 100644
> --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -363,6 +363,12 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
>  		kfree(zonedev);
>  		return err;
>  	}
> +	err = thermal_zone_device_enable(zonedev->tzone);
> +	if (err) {
> +		thermal_zone_device_unregister(zonedev->tzone);
> +		kfree(zonedev);
> +		return err;
> +	}
>  	/* Store MSR value for package thermal interrupt, to restore at exit */
>  	rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, zonedev->msr_pkg_therm_low,
>  	      zonedev->msr_pkg_therm_high);
> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
> index 189b675cf14d..7fb6e476c82a 100644
> --- a/drivers/thermal/kirkwood_thermal.c
> +++ b/drivers/thermal/kirkwood_thermal.c
> @@ -65,6 +65,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>  	struct thermal_zone_device *thermal = NULL;
>  	struct kirkwood_thermal_priv *priv;
>  	struct resource *res;
> +	int ret;
>  
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -82,6 +83,12 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>  			"Failed to register thermal zone device\n");
>  		return PTR_ERR(thermal);
>  	}
> +	ret = thermal_zone_device_enable(thermal);
> +	if (ret) {
> +		thermal_zone_device_unregister(thermal);
> +		dev_err(&pdev->dev, "Failed to enable thermal zone device\n");
> +		return ret;
> +	}
>  
>  	platform_set_drvdata(pdev, thermal);
>  
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 46aeb28b4e90..787710bb88fe 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -550,12 +550,19 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  			priv->zone = devm_thermal_zone_of_sensor_register(
>  						dev, i, priv,
>  						&rcar_thermal_zone_of_ops);
> -		else
> +		else {
>  			priv->zone = thermal_zone_device_register(
>  						"rcar_thermal",
>  						1, 0, priv,
>  						&rcar_thermal_zone_ops, NULL, 0,
>  						idle);
> +
> +			ret = thermal_zone_device_enable(priv->zone);
> +			if (ret) {
> +				thermal_zone_device_unregister(priv->zone);
> +				priv->zone = ERR_PTR(ret);
> +			}
> +		}
>  		if (IS_ERR(priv->zone)) {
>  			dev_err(dev, "can't register thermal zone\n");
>  			ret = PTR_ERR(priv->zone);
> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
> index f68f581fd669..ee33ed692e4f 100644
> --- a/drivers/thermal/spear_thermal.c
> +++ b/drivers/thermal/spear_thermal.c
> @@ -131,6 +131,11 @@ static int spear_thermal_probe(struct platform_device *pdev)
>  		ret = PTR_ERR(spear_thermal);
>  		goto disable_clk;
>  	}
> +	ret = thermal_zone_device_enable(spear_thermal);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot enable thermal zone\n");
> +		goto unregister_tzd;
> +	}
>  
>  	platform_set_drvdata(pdev, spear_thermal);
>  
> @@ -139,6 +144,8 @@ static int spear_thermal_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +unregister_tzd:
> +	thermal_zone_device_unregister(spear_thermal);
>  disable_clk:
>  	clk_disable(stdev->clk);
>  
> diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
> index b928ca6a289b..1276b95604fe 100644
> --- a/drivers/thermal/st/st_thermal.c
> +++ b/drivers/thermal/st/st_thermal.c
> @@ -246,11 +246,16 @@ int st_thermal_register(struct platform_device *pdev,
>  		ret = PTR_ERR(sensor->thermal_dev);
>  		goto sensor_off;
>  	}
> +	ret = thermal_zone_device_enable(sensor->thermal_dev);
> +	if (ret)
> +		goto tzd_unregister;
>  
>  	platform_set_drvdata(pdev, sensor);
>  
>  	return 0;
>  
> +tzd_unregister:
> +	thermal_zone_device_unregister(sensor->thermal_dev);
>  sensor_off:
>  	st_thermal_sensor_off(sensor);
>  
> 

Re: [PATCH v7 08/11] thermal: Explicitly enable non-changing thermal zone devices

[Amit Kucheria]

On Mon, Jun 29, 2020 at 5:59 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Some thermal zone devices never change their state, so they should be
> always enabled.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c       | 8 ++++++++
>  drivers/net/wireless/intel/iwlwifi/mvm/tt.c              | 9 ++++++++-
>  drivers/platform/x86/intel_mid_thermal.c                 | 6 ++++++
>  drivers/power/supply/power_supply_core.c                 | 9 +++++++--
>  drivers/thermal/armada_thermal.c                         | 6 ++++++
>  drivers/thermal/dove_thermal.c                           | 6 ++++++
>  .../thermal/intel/int340x_thermal/int340x_thermal_zone.c | 5 +++++
>  drivers/thermal/intel/intel_pch_thermal.c                | 5 +++++
>  drivers/thermal/intel/intel_soc_dts_iosf.c               | 3 +++
>  drivers/thermal/intel/x86_pkg_temp_thermal.c             | 6 ++++++
>  drivers/thermal/kirkwood_thermal.c                       | 7 +++++++
>  drivers/thermal/rcar_thermal.c                           | 9 ++++++++-
>  drivers/thermal/spear_thermal.c                          | 7 +++++++
>  drivers/thermal/st/st_thermal.c                          | 5 +++++
>  14 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> index 3de8a5e83b6c..e3510e9b21f3 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
> @@ -92,6 +92,14 @@ int cxgb4_thermal_init(struct adapter *adap)
>                 ch_thermal->tzdev = NULL;
>                 return ret;
>         }
> +
> +       ret = thermal_zone_device_enable(ch_thermal->tzdev);
> +       if (ret) {
> +               dev_err(adap->pdev_dev, "Failed to enable thermal zone\n");
> +               thermal_zone_device_unregister(adap->ch_thermal.tzdev);
> +               return ret;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> index 418e59b7c671..0c95663bf9ed 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> @@ -733,7 +733,7 @@ static  struct thermal_zone_device_ops tzone_ops = {
>
>  static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
>  {
> -       int i;
> +       int i, ret;
>         char name[16];
>         static atomic_t counter = ATOMIC_INIT(0);
>
> @@ -759,6 +759,13 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
>                 return;
>         }
>
> +       ret = thermal_zone_device_enable(mvm->tz_device.tzone);
> +       if (ret) {
> +               IWL_DEBUG_TEMP(mvm, "Failed to enable thermal zone\n");
> +               thermal_zone_device_unregister(mvm->tz_device.tzone);
> +               return;
> +       }
> +
>         /* 0 is a valid temperature,
>          * so initialize the array with S16_MIN which invalid temperature
>          */
> diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
> index f402e2e74a38..f12f4e7bd971 100644
> --- a/drivers/platform/x86/intel_mid_thermal.c
> +++ b/drivers/platform/x86/intel_mid_thermal.c
> @@ -493,6 +493,12 @@ static int mid_thermal_probe(struct platform_device *pdev)
>                         ret = PTR_ERR(pinfo->tzd[i]);
>                         goto err;
>                 }
> +               ret = thermal_zone_device_enable(pinfo->tzd[i]);
> +               if (ret) {
> +                       kfree(td_info);
> +                       thermal_zone_device_unregister(pinfo->tzd[i]);
> +                       goto err;
> +               }
>         }
>
>         pinfo->pdev = pdev;
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 02b37fe6061c..90e56736d479 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -939,7 +939,7 @@ static struct thermal_zone_device_ops psy_tzd_ops = {
>
>  static int psy_register_thermal(struct power_supply *psy)
>  {
> -       int i;
> +       int i, ret;
>
>         if (psy->desc->no_thermal)
>                 return 0;
> @@ -949,7 +949,12 @@ static int psy_register_thermal(struct power_supply *psy)
>                 if (psy->desc->properties[i] == POWER_SUPPLY_PROP_TEMP) {
>                         psy->tzd = thermal_zone_device_register(psy->desc->name,
>                                         0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
> -                       return PTR_ERR_OR_ZERO(psy->tzd);
> +                       if (IS_ERR(psy->tzd))
> +                               return PTR_ERR(psy->tzd);
> +                       ret = thermal_zone_device_enable(psy->tzd);
> +                       if (ret)
> +                               thermal_zone_device_unregister(psy->tzd);
> +                       return ret;
>                 }
>         }
>         return 0;
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 7c447cd149e7..c2ebfb5be4b3 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -874,6 +874,12 @@ static int armada_thermal_probe(struct platform_device *pdev)
>                         return PTR_ERR(tz);
>                 }
>
> +               ret = thermal_zone_device_enable(tz);
> +               if (ret) {
> +                       thermal_zone_device_unregister(tz);
> +                       return ret;
> +               }
> +
>                 drvdata->type = LEGACY;
>                 drvdata->data.tz = tz;
>                 platform_set_drvdata(pdev, drvdata);
> diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
> index 75901ced4a62..73182eb94bc0 100644
> --- a/drivers/thermal/dove_thermal.c
> +++ b/drivers/thermal/dove_thermal.c
> @@ -153,6 +153,12 @@ static int dove_thermal_probe(struct platform_device *pdev)
>                 return PTR_ERR(thermal);
>         }
>
> +       ret = thermal_zone_device_enable(thermal);
> +       if (ret) {
> +               thermal_zone_device_unregister(thermal);
> +               return ret;
> +       }
> +
>         platform_set_drvdata(pdev, thermal);
>
>         return 0;
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 432213272f1e..6e479deff76b 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -259,9 +259,14 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
>                 ret = PTR_ERR(int34x_thermal_zone->zone);
>                 goto err_thermal_zone;
>         }
> +       ret = thermal_zone_device_enable(int34x_thermal_zone->zone);
> +       if (ret)
> +               goto err_enable;
>
>         return int34x_thermal_zone;
>
> +err_enable:
> +       thermal_zone_device_unregister(int34x_thermal_zone->zone);
>  err_thermal_zone:
>         acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
>         kfree(int34x_thermal_zone->aux_trips);
> diff --git a/drivers/thermal/intel/intel_pch_thermal.c b/drivers/thermal/intel/intel_pch_thermal.c
> index 56401fd4708d..65702094f3d3 100644
> --- a/drivers/thermal/intel/intel_pch_thermal.c
> +++ b/drivers/thermal/intel/intel_pch_thermal.c
> @@ -352,9 +352,14 @@ static int intel_pch_thermal_probe(struct pci_dev *pdev,
>                 err = PTR_ERR(ptd->tzd);
>                 goto error_cleanup;
>         }
> +       err = thermal_zone_device_enable(ptd->tzd);
> +       if (err)
> +               goto err_unregister;
>
>         return 0;
>
> +err_unregister:
> +       thermal_zone_device_unregister(ptd->tzd);
>  error_cleanup:
>         iounmap(ptd->hw_base);
>  error_release:
> diff --git a/drivers/thermal/intel/intel_soc_dts_iosf.c b/drivers/thermal/intel/intel_soc_dts_iosf.c
> index f75271b669c6..4f1a2f7c016c 100644
> --- a/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ b/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -329,6 +329,9 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
>                 ret = PTR_ERR(dts->tzone);
>                 goto err_ret;
>         }
> +       ret = thermal_zone_device_enable(dts->tzone);
> +       if (ret)
> +               goto err_enable;
>
>         ret = soc_dts_enable(id);
>         if (ret)
> diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> index a006b9fd1d72..b81c33202f41 100644
> --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -363,6 +363,12 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
>                 kfree(zonedev);
>                 return err;
>         }
> +       err = thermal_zone_device_enable(zonedev->tzone);
> +       if (err) {
> +               thermal_zone_device_unregister(zonedev->tzone);
> +               kfree(zonedev);
> +               return err;
> +       }
>         /* Store MSR value for package thermal interrupt, to restore at exit */
>         rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, zonedev->msr_pkg_therm_low,
>               zonedev->msr_pkg_therm_high);
> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
> index 189b675cf14d..7fb6e476c82a 100644
> --- a/drivers/thermal/kirkwood_thermal.c
> +++ b/drivers/thermal/kirkwood_thermal.c
> @@ -65,6 +65,7 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>         struct thermal_zone_device *thermal = NULL;
>         struct kirkwood_thermal_priv *priv;
>         struct resource *res;
> +       int ret;
>
>         priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
> @@ -82,6 +83,12 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>                         "Failed to register thermal zone device\n");
>                 return PTR_ERR(thermal);
>         }
> +       ret = thermal_zone_device_enable(thermal);
> +       if (ret) {
> +               thermal_zone_device_unregister(thermal);
> +               dev_err(&pdev->dev, "Failed to enable thermal zone device\n");
> +               return ret;
> +       }
>
>         platform_set_drvdata(pdev, thermal);
>
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 46aeb28b4e90..787710bb88fe 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -550,12 +550,19 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>                         priv->zone = devm_thermal_zone_of_sensor_register(
>                                                 dev, i, priv,
>                                                 &rcar_thermal_zone_of_ops);
> -               else
> +               else {
>                         priv->zone = thermal_zone_device_register(
>                                                 "rcar_thermal",
>                                                 1, 0, priv,
>                                                 &rcar_thermal_zone_ops, NULL, 0,
>                                                 idle);
> +
> +                       ret = thermal_zone_device_enable(priv->zone);
> +                       if (ret) {
> +                               thermal_zone_device_unregister(priv->zone);
> +                               priv->zone = ERR_PTR(ret);
> +                       }
> +               }
>                 if (IS_ERR(priv->zone)) {
>                         dev_err(dev, "can't register thermal zone\n");
>                         ret = PTR_ERR(priv->zone);
> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
> index f68f581fd669..ee33ed692e4f 100644
> --- a/drivers/thermal/spear_thermal.c
> +++ b/drivers/thermal/spear_thermal.c
> @@ -131,6 +131,11 @@ static int spear_thermal_probe(struct platform_device *pdev)
>                 ret = PTR_ERR(spear_thermal);
>                 goto disable_clk;
>         }
> +       ret = thermal_zone_device_enable(spear_thermal);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Cannot enable thermal zone\n");
> +               goto unregister_tzd;
> +       }
>
>         platform_set_drvdata(pdev, spear_thermal);
>
> @@ -139,6 +144,8 @@ static int spear_thermal_probe(struct platform_device *pdev)
>
>         return 0;
>
> +unregister_tzd:
> +       thermal_zone_device_unregister(spear_thermal);
>  disable_clk:
>         clk_disable(stdev->clk);
>
> diff --git a/drivers/thermal/st/st_thermal.c b/drivers/thermal/st/st_thermal.c
> index b928ca6a289b..1276b95604fe 100644
> --- a/drivers/thermal/st/st_thermal.c
> +++ b/drivers/thermal/st/st_thermal.c
> @@ -246,11 +246,16 @@ int st_thermal_register(struct platform_device *pdev,
>                 ret = PTR_ERR(sensor->thermal_dev);
>                 goto sensor_off;
>         }
> +       ret = thermal_zone_device_enable(sensor->thermal_dev);
> +       if (ret)
> +               goto tzd_unregister;
>
>         platform_set_drvdata(pdev, sensor);
>
>         return 0;
>
> +tzd_unregister:
> +       thermal_zone_device_unregister(sensor->thermal_dev);
>  sensor_off:
>         st_thermal_sensor_off(sensor);
>
> --
> 2.17.1
>

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

Hi Andrzej,

I've tested your series with kernelci and there are 3 regressions for
the imx6.

https://kernelci.org/test/job/thermal/branch/testing/kernel/v5.8-rc3-11-gf5e50bf4d3ef/plan/baseline/


On 29/06/2020 14:29, Andrzej Pietrasiewicz wrote:
> A respin of v6 with these changes:
> 
> v6..v7:
> - removed duplicate S-o-b line from patch 6/11
> 
> v5..v6:
> - staticized thermal_zone_device_set_mode() (kbuild test robot)
> 
> v4..v5:
> 
> - EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL (Daniel)
> - dropped unnecessary thermal_zone_device_enable() in int3400_thermal.c
> and in thermal_of.c (Bartlomiej)
> 
> Andrzej Pietrasiewicz (11):
>   acpi: thermal: Fix error handling in the register function
>   thermal: Store thermal mode in a dedicated enum
>   thermal: Add current mode to thermal zone device
>   thermal: Store device mode in struct thermal_zone_device
>   thermal: remove get_mode() operation of drivers
>   thermal: Add mode helpers
>   thermal: Use mode helpers in drivers
>   thermal: Explicitly enable non-changing thermal zone devices
>   thermal: core: Stop polling DISABLED thermal devices
>   thermal: Simplify or eliminate unnecessary set_mode() methods
>   thermal: Rename set_mode() to change_mode()
> 
>  drivers/acpi/thermal.c                        | 75 +++++----------
>  .../ethernet/chelsio/cxgb4/cxgb4_thermal.c    |  8 ++
>  .../ethernet/mellanox/mlxsw/core_thermal.c    | 91 ++++---------------
>  drivers/net/wireless/intel/iwlwifi/mvm/tt.c   |  9 +-
>  drivers/platform/x86/acerhdf.c                | 33 +++----
>  drivers/platform/x86/intel_mid_thermal.c      |  6 ++
>  drivers/power/supply/power_supply_core.c      |  9 +-
>  drivers/thermal/armada_thermal.c              |  6 ++
>  drivers/thermal/da9062-thermal.c              | 16 +---
>  drivers/thermal/dove_thermal.c                |  6 ++
>  drivers/thermal/hisi_thermal.c                |  6 +-
>  drivers/thermal/imx_thermal.c                 | 57 ++++--------
>  .../intel/int340x_thermal/int3400_thermal.c   | 38 ++------
>  .../int340x_thermal/int340x_thermal_zone.c    |  5 +
>  drivers/thermal/intel/intel_pch_thermal.c     |  5 +
>  .../thermal/intel/intel_quark_dts_thermal.c   | 34 ++-----
>  drivers/thermal/intel/intel_soc_dts_iosf.c    |  3 +
>  drivers/thermal/intel/x86_pkg_temp_thermal.c  |  6 ++
>  drivers/thermal/kirkwood_thermal.c            |  7 ++
>  drivers/thermal/rcar_thermal.c                |  9 +-
>  drivers/thermal/rockchip_thermal.c            |  6 +-
>  drivers/thermal/spear_thermal.c               |  7 ++
>  drivers/thermal/sprd_thermal.c                |  6 +-
>  drivers/thermal/st/st_thermal.c               |  5 +
>  drivers/thermal/thermal_core.c                | 76 ++++++++++++++--
>  drivers/thermal/thermal_of.c                  | 41 +--------
>  drivers/thermal/thermal_sysfs.c               | 37 +-------
>  include/linux/thermal.h                       | 19 +++-
>  28 files changed, 275 insertions(+), 351 deletions(-)
> 
> 
> base-commit: 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
> 


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi Daniel,

I am reading the logs and can't find anything specific to thermal.

What I can see is

"random: crng init done"

with large times (~200s) and then e.g.

'auto-login-action timed out after 283 seconds'

I'm looking at e.g. 
https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html

Is there anywhere else I can look at?

Andrzej

W dniu 30.06.2020 o 14:57, Daniel Lezcano pisze:
> 
> Hi Andrzej,
> 
> I've tested your series with kernelci and there are 3 regressions for
> the imx6.
> 
> https://kernelci.org/test/job/thermal/branch/testing/kernel/v5.8-rc3-11-gf5e50bf4d3ef/plan/baseline/
> 
> 
> On 29/06/2020 14:29, Andrzej Pietrasiewicz wrote:
>> A respin of v6 with these changes:
>>
>> v6..v7:
>> - removed duplicate S-o-b line from patch 6/11
>>
>> v5..v6:
>> - staticized thermal_zone_device_set_mode() (kbuild test robot)
>>
>> v4..v5:
>>
>> - EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL (Daniel)
>> - dropped unnecessary thermal_zone_device_enable() in int3400_thermal.c
>> and in thermal_of.c (Bartlomiej)
>>
>> Andrzej Pietrasiewicz (11):
>>    acpi: thermal: Fix error handling in the register function
>>    thermal: Store thermal mode in a dedicated enum
>>    thermal: Add current mode to thermal zone device
>>    thermal: Store device mode in struct thermal_zone_device
>>    thermal: remove get_mode() operation of drivers
>>    thermal: Add mode helpers
>>    thermal: Use mode helpers in drivers
>>    thermal: Explicitly enable non-changing thermal zone devices
>>    thermal: core: Stop polling DISABLED thermal devices
>>    thermal: Simplify or eliminate unnecessary set_mode() methods
>>    thermal: Rename set_mode() to change_mode()
>>
>>   drivers/acpi/thermal.c                        | 75 +++++----------
>>   .../ethernet/chelsio/cxgb4/cxgb4_thermal.c    |  8 ++
>>   .../ethernet/mellanox/mlxsw/core_thermal.c    | 91 ++++---------------
>>   drivers/net/wireless/intel/iwlwifi/mvm/tt.c   |  9 +-
>>   drivers/platform/x86/acerhdf.c                | 33 +++----
>>   drivers/platform/x86/intel_mid_thermal.c      |  6 ++
>>   drivers/power/supply/power_supply_core.c      |  9 +-
>>   drivers/thermal/armada_thermal.c              |  6 ++
>>   drivers/thermal/da9062-thermal.c              | 16 +---
>>   drivers/thermal/dove_thermal.c                |  6 ++
>>   drivers/thermal/hisi_thermal.c                |  6 +-
>>   drivers/thermal/imx_thermal.c                 | 57 ++++--------
>>   .../intel/int340x_thermal/int3400_thermal.c   | 38 ++------
>>   .../int340x_thermal/int340x_thermal_zone.c    |  5 +
>>   drivers/thermal/intel/intel_pch_thermal.c     |  5 +
>>   .../thermal/intel/intel_quark_dts_thermal.c   | 34 ++-----
>>   drivers/thermal/intel/intel_soc_dts_iosf.c    |  3 +
>>   drivers/thermal/intel/x86_pkg_temp_thermal.c  |  6 ++
>>   drivers/thermal/kirkwood_thermal.c            |  7 ++
>>   drivers/thermal/rcar_thermal.c                |  9 +-
>>   drivers/thermal/rockchip_thermal.c            |  6 +-
>>   drivers/thermal/spear_thermal.c               |  7 ++
>>   drivers/thermal/sprd_thermal.c                |  6 +-
>>   drivers/thermal/st/st_thermal.c               |  5 +
>>   drivers/thermal/thermal_core.c                | 76 ++++++++++++++--
>>   drivers/thermal/thermal_of.c                  | 41 +--------
>>   drivers/thermal/thermal_sysfs.c               | 37 +-------
>>   include/linux/thermal.h                       | 19 +++-
>>   28 files changed, 275 insertions(+), 351 deletions(-)
>>
>>
>> base-commit: 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
>>
> 
> 

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
> Hi Daniel,
> 
> I am reading the logs and can't find anything specific to thermal.
> 
> What I can see is
> 
> "random: crng init done"
> 
> with large times (~200s) and then e.g.
> 
> 'auto-login-action timed out after 283 seconds'
> 
> I'm looking at e.g.
> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
> 
> 
> Is there anywhere else I can look at?

No unfortunately :/

I have a Meerkat96 which uses the same sensor as the imx6q.

I'll give a try to see if I can reproduce and let you know.


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi Daniel,

W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>> Hi Daniel,
>>
>> I am reading the logs and can't find anything specific to thermal.
>>
>> What I can see is
>>
>> "random: crng init done"
>>
>> with large times (~200s) and then e.g.
>>
>> 'auto-login-action timed out after 283 seconds'
>>
>> I'm looking at e.g.
>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>

f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
PATCH 11/11 renames a method and the code compiles, so it seems
unlikely that this is causing problems. One should never say never,
though ;)

The reported failure is not due to some test failing but rather due
to timeout logging into the test system. Could it be that there is
some other problem?

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 30/06/2020 17:29, Andrzej Pietrasiewicz wrote:
> Hi Daniel,
> 
> W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
>> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>>> Hi Daniel,
>>>
>>> I am reading the logs and can't find anything specific to thermal.
>>>
>>> What I can see is
>>>
>>> "random: crng init done"
>>>
>>> with large times (~200s) and then e.g.
>>>
>>> 'auto-login-action timed out after 283 seconds'
>>>
>>> I'm looking at e.g.
>>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>>
>>>
> 
> f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
> PATCH 11/11 renames a method and the code compiles, so it seems
> unlikely that this is causing problems. One should never say never,
> though ;)

The sha1 is just the HEAD for the kernel reference. The regression
happens with your series, somewhere.

> The reported failure is not due to some test failing but rather due
> to timeout logging into the test system. Could it be that there is
> some other problem?

I did reproduce:

v5.8-rc3 + series => imx6 hang at boot time
v5.8-rc3 => imx6 boots correctly


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi,

W dniu 30.06.2020 o 17:53, Daniel Lezcano pisze:
> On 30/06/2020 17:29, Andrzej Pietrasiewicz wrote:
>> Hi Daniel,
>>
>> W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
>>> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>>>> Hi Daniel,
>>>>
>>>> I am reading the logs and can't find anything specific to thermal.
>>>>
>>>> What I can see is
>>>>
>>>> "random: crng init done"
>>>>
>>>> with large times (~200s) and then e.g.
>>>>
>>>> 'auto-login-action timed out after 283 seconds'
>>>>
>>>> I'm looking at e.g.
>>>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>>>
>>>>
>>
>> f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
>> PATCH 11/11 renames a method and the code compiles, so it seems
>> unlikely that this is causing problems. One should never say never,
>> though ;)
> 
> The sha1 is just the HEAD for the kernel reference. The regression
> happens with your series, somewhere.
> 
>> The reported failure is not due to some test failing but rather due
>> to timeout logging into the test system. Could it be that there is
>> some other problem?
> 
> I did reproduce:
> 
> v5.8-rc3 + series => imx6 hang at boot time
> v5.8-rc3 => imx6 boots correctly
> 

I kindly ask for a bisect.

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 30/06/2020 18:56, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 30.06.2020 o 17:53, Daniel Lezcano pisze:
>> On 30/06/2020 17:29, Andrzej Pietrasiewicz wrote:
>>> Hi Daniel,
>>>
>>> W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
>>>> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> I am reading the logs and can't find anything specific to thermal.
>>>>>
>>>>> What I can see is
>>>>>
>>>>> "random: crng init done"
>>>>>
>>>>> with large times (~200s) and then e.g.
>>>>>
>>>>> 'auto-login-action timed out after 283 seconds'
>>>>>
>>>>> I'm looking at e.g.
>>>>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>>>>
>>>>>
>>>>>
>>>
>>> f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
>>> PATCH 11/11 renames a method and the code compiles, so it seems
>>> unlikely that this is causing problems. One should never say never,
>>> though ;)
>>
>> The sha1 is just the HEAD for the kernel reference. The regression
>> happens with your series, somewhere.
>>
>>> The reported failure is not due to some test failing but rather due
>>> to timeout logging into the test system. Could it be that there is
>>> some other problem?
>>
>> I did reproduce:
>>
>> v5.8-rc3 + series => imx6 hang at boot time
>> v5.8-rc3 => imx6 boots correctly
>>
> 
> I kindly ask for a bisect.

I will give a try but it is a very long process as the board is running
on kernelci.

I was not able to reproduce it on imx7 despite it is the same sensor :/


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi,

W dniu 30.06.2020 o 20:33, Daniel Lezcano pisze:
> On 30/06/2020 18:56, Andrzej Pietrasiewicz wrote:
>> Hi,
>>
>> W dniu 30.06.2020 o 17:53, Daniel Lezcano pisze:
>>> On 30/06/2020 17:29, Andrzej Pietrasiewicz wrote:
>>>> Hi Daniel,
>>>>
>>>> W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
>>>>> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>> I am reading the logs and can't find anything specific to thermal.
>>>>>>
>>>>>> What I can see is
>>>>>>
>>>>>> "random: crng init done"
>>>>>>
>>>>>> with large times (~200s) and then e.g.
>>>>>>
>>>>>> 'auto-login-action timed out after 283 seconds'
>>>>>>
>>>>>> I'm looking at e.g.
>>>>>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>> f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
>>>> PATCH 11/11 renames a method and the code compiles, so it seems
>>>> unlikely that this is causing problems. One should never say never,
>>>> though ;)
>>>
>>> The sha1 is just the HEAD for the kernel reference. The regression
>>> happens with your series, somewhere.
>>>
>>>> The reported failure is not due to some test failing but rather due
>>>> to timeout logging into the test system. Could it be that there is
>>>> some other problem?
>>>
>>> I did reproduce:
>>>
>>> v5.8-rc3 + series => imx6 hang at boot time
>>> v5.8-rc3 => imx6 boots correctly
>>>

What did you reproduce? Timeout logging in to the test system or a "real" 
failure of a test?

>>
>> I kindly ask for a bisect.
> 
> I will give a try but it is a very long process as the board is running
> on kernelci.
> 
> I was not able to reproduce it on imx7 despite it is the same sensor :/
> 
> 

Could it be that the thermal sensors somehow contribute to entropy and after
the series is applied on some machines it takes more time to gather enough
entropy?

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 01/07/2020 12:23, Andrzej Pietrasiewicz wrote:
> Hi,
> 

[ ... ]

>>>>
>>>> I did reproduce:
>>>>
>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>> v5.8-rc3 => imx6 boots correctly
>>>>
> 
> What did you reproduce? Timeout logging in to the test system or a
> "real" failure of a test?

Timeout logging. Boot hangs.

>>> I kindly ask for a bisect.
>>
>> I will give a try but it is a very long process as the board is running
>> on kernelci.
>>
>> I was not able to reproduce it on imx7 despite it is the same sensor :/
>>
>>
> 
> Could it be that the thermal sensors somehow contribute to entropy and
> after
> the series is applied on some machines it takes more time to gather enough
> entropy?

I assume you are talking about the entropy for random?

It would be really surprising if it is the case. The message appears
asynchronously, I believe the boot flow is stuck in a mutex.


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 01/07/2020 12:23, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 30.06.2020 o 20:33, Daniel Lezcano pisze:
>> On 30/06/2020 18:56, Andrzej Pietrasiewicz wrote:
>>> Hi,
>>>
>>> W dniu 30.06.2020 o 17:53, Daniel Lezcano pisze:
>>>> On 30/06/2020 17:29, Andrzej Pietrasiewicz wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> W dniu 30.06.2020 o 16:53, Daniel Lezcano pisze:
>>>>>> On 30/06/2020 15:43, Andrzej Pietrasiewicz wrote:
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> I am reading the logs and can't find anything specific to thermal.
>>>>>>>
>>>>>>> What I can see is
>>>>>>>
>>>>>>> "random: crng init done"
>>>>>>>
>>>>>>> with large times (~200s) and then e.g.
>>>>>>>
>>>>>>> 'auto-login-action timed out after 283 seconds'
>>>>>>>
>>>>>>> I'm looking at e.g.
>>>>>>> https://storage.kernelci.org/thermal/testing/v5.8-rc3-11-gf5e50bf4d3ef/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-imx6q-sabrelite.html
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>> f5e50bf4d3ef is PATCH 11/11. Does the problem happen at PATCH 1-10/11?
>>>>> PATCH 11/11 renames a method and the code compiles, so it seems
>>>>> unlikely that this is causing problems. One should never say never,
>>>>> though ;)
>>>>
>>>> The sha1 is just the HEAD for the kernel reference. The regression
>>>> happens with your series, somewhere.
>>>>
>>>>> The reported failure is not due to some test failing but rather due
>>>>> to timeout logging into the test system. Could it be that there is
>>>>> some other problem?
>>>>
>>>> I did reproduce:
>>>>
>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>> v5.8-rc3 => imx6 boots correctly

So finally I succeeded to reproduce it on my imx7 locally. The sensor
was failing to initialize for another reason related to the legacy
cooling device, this is why it is not appearing on the imx7.

I can now git-bisect :)



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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi Daniel,

<snip>

>>>>>
>>>>> I did reproduce:
>>>>>
>>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>>> v5.8-rc3 => imx6 boots correctly
> 
> So finally I succeeded to reproduce it on my imx7 locally. The sensor
> was failing to initialize for another reason related to the legacy
> cooling device, this is why it is not appearing on the imx7.
> 
> I can now git-bisect :)
> 

That would be very kind of you, thank you!

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 02/07/2020 15:53, Andrzej Pietrasiewicz wrote:
> Hi Daniel,
> 
> <snip>
> 
>>>>>>
>>>>>> I did reproduce:
>>>>>>
>>>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>>>> v5.8-rc3 => imx6 boots correctly
>>
>> So finally I succeeded to reproduce it on my imx7 locally. The sensor
>> was failing to initialize for another reason related to the legacy
>> cooling device, this is why it is not appearing on the imx7.
>>
>> I can now git-bisect :)
>>
> 
> That would be very kind of you, thank you!

Author: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Date:   Mon Jun 29 14:29:21 2020 +0200

    thermal: Use mode helpers in drivers

    Use thermal_zone_device_{en|dis}able() and
thermal_zone_device_is_enabled().



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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 02/07/2020 15:53, Andrzej Pietrasiewicz wrote:
> Hi Daniel,
> 
> <snip>
> 
>>>>>>
>>>>>> I did reproduce:
>>>>>>
>>>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>>>> v5.8-rc3 => imx6 boots correctly
>>
>> So finally I succeeded to reproduce it on my imx7 locally. The sensor
>> was failing to initialize for another reason related to the legacy
>> cooling device, this is why it is not appearing on the imx7.
>>
>> I can now git-bisect :)
>>
> 
> That would be very kind of you, thank you!

With the lock correctness option enabled:

[    4.179223] imx_thermal tempmon: Extended Commercial CPU temperature
grade - max:105C critical:100C passive:95C
[    4.189557]
[    4.191060] ============================================
[    4.196378] WARNING: possible recursive locking detected
[    4.201699] 5.8.0-rc3-00011-gf5e50bf4d3ef #42 Not tainted
[    4.207102] --------------------------------------------
[    4.212421] kworker/0:3/54 is trying to acquire lock:
[    4.217480] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_device_is_enabled+0x18/0x34
[    4.225777]
[    4.225777] but task is already holding lock:
[    4.231615] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_get_temp+0x38/0x6c
[    4.239121]
[    4.239121] other info that might help us debug this:
[    4.245655]  Possible unsafe locking scenario:
[    4.245655]
[    4.251579]        CPU0
[    4.254031]        ----
[    4.256481]   lock(&tz->lock);
[    4.259544]   lock(&tz->lock);
[    4.262608]
[    4.262608]  *** DEADLOCK ***
[    4.262608]
[    4.268533]  May be due to missing lock nesting notation
[    4.268533]
[    4.275329] 4 locks held by kworker/0:3/54:
[    4.279517]  #0: cb0066a8 ((wq_completion)events){+.+.}-{0:0}, at:
process_one_work+0x224/0x808
[    4.288241]  #1: ca075f10 (deferred_probe_work){+.+.}-{0:0}, at:
process_one_work+0x224/0x808
[    4.296787]  #2: cb1a48d8 (&dev->mutex){....}-{3:3}, at:
__device_attach+0x30/0x140
[    4.304468]  #3: ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
thermal_zone_get_temp+0x38/0x6c
[    4.312408]
[    4.312408] stack backtrace:
[    4.316778] CPU: 0 PID: 54 Comm: kworker/0:3 Not tainted
5.8.0-rc3-00011-gf5e50bf4d3ef #42
[    4.325048] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    4.330809] Workqueue: events deferred_probe_work_func
[    4.335973] [<c0312384>] (unwind_backtrace) from [<c030c580>]
(show_stack+0x10/0x14)
[    4.343734] [<c030c580>] (show_stack) from [<c079d7d8>]
(dump_stack+0xe8/0x114)
[    4.351062] [<c079d7d8>] (dump_stack) from [<c03abf78>]
(__lock_acquire+0xbfc/0x2cb4)
[    4.358909] [<c03abf78>] (__lock_acquire) from [<c03ae9c4>]
(lock_acquire+0xf4/0x4e4)
[    4.366758] [<c03ae9c4>] (lock_acquire) from [<c10630fc>]
(__mutex_lock+0xb0/0xaa8)
[    4.374431] [<c10630fc>] (__mutex_lock) from [<c1063b10>]
(mutex_lock_nested+0x1c/0x24)
[    4.382452] [<c1063b10>] (mutex_lock_nested) from [<c0d932c0>]
(thermal_zone_device_is_enabled+0x18/0x34)
[    4.392036] [<c0d932c0>] (thermal_zone_device_is_enabled) from
[<c0d9da90>] (imx_get_temp+0x30/0x208)
[    4.401271] [<c0d9da90>] (imx_get_temp) from [<c0d97484>]
(thermal_zone_get_temp+0x4c/0x6c)
[    4.409640] [<c0d97484>] (thermal_zone_get_temp) from [<c0d93df0>]
(thermal_zone_device_update+0x8c/0x258)
[    4.419310] [<c0d93df0>] (thermal_zone_device_update) from
[<c0d9401c>] (thermal_zone_device_set_mode+0x60/0x88)
[    4.429500] [<c0d9401c>] (thermal_zone_device_set_mode) from
[<c0d9e1d4>] (imx_thermal_probe+0x3e4/0x578)
[    4.439082] [<c0d9e1d4>] (imx_thermal_probe) from [<c0a78388>]
(platform_drv_probe+0x48/0x98)
[    4.447622] [<c0a78388>] (platform_drv_probe) from [<c0a7603c>]
(really_probe+0x218/0x348)
[    4.455903] [<c0a7603c>] (really_probe) from [<c0a76278>]
(driver_probe_device+0x5c/0xb4)
[    4.464098] [<c0a76278>] (driver_probe_device) from [<c0a743bc>]
(bus_for_each_drv+0x58/0xb8)
[    4.472638] [<c0a743bc>] (bus_for_each_drv) from [<c0a75db0>]
(__device_attach+0xd4/0x140)
[    4.480919] [<c0a75db0>] (__device_attach) from [<c0a750b0>]
(bus_probe_device+0x88/0x90)
[    4.489112] [<c0a750b0>] (bus_probe_device) from [<c0a75564>]
(deferred_probe_work_func+0x68/0x98)
[    4.498088] [<c0a75564>] (deferred_probe_work_func) from [<c0369988>]
(process_one_work+0x2e0/0x808)
[    4.507237] [<c0369988>] (process_one_work) from [<c036a150>]
(worker_thread+0x2a0/0x59c)
[    4.515432] [<c036a150>] (worker_thread) from [<c0372208>]
(kthread+0x16c/0x178)
[    4.522843] [<c0372208>] (kthread) from [<c0300174>]
(ret_from_fork+0x14/0x20)
[    4.530074] Exception stack(0xca075fb0 to 0xca075ff8)
[    4.535138] 5fa0:                                     00000000
00000000 00000000 00000000
[    4.543328] 5fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    4.551516] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000



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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi,

W dniu 02.07.2020 o 19:01, Daniel Lezcano pisze:
> On 02/07/2020 15:53, Andrzej Pietrasiewicz wrote:
>> Hi Daniel,
>>
>> <snip>
>>
>>>>>>>
>>>>>>> I did reproduce:
>>>>>>>
>>>>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>>>>> v5.8-rc3 => imx6 boots correctly
>>>
>>> So finally I succeeded to reproduce it on my imx7 locally. The sensor
>>> was failing to initialize for another reason related to the legacy
>>> cooling device, this is why it is not appearing on the imx7.
>>>
>>> I can now git-bisect :)
>>>
>>
>> That would be very kind of you, thank you!
> 
> With the lock correctness option enabled:
> 
> [    4.179223] imx_thermal tempmon: Extended Commercial CPU temperature
> grade - max:105C critical:100C passive:95C
> [    4.189557]
> [    4.191060] ============================================
> [    4.196378] WARNING: possible recursive locking detected
> [    4.201699] 5.8.0-rc3-00011-gf5e50bf4d3ef #42 Not tainted
> [    4.207102] --------------------------------------------
> [    4.212421] kworker/0:3/54 is trying to acquire lock:
> [    4.217480] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> thermal_zone_device_is_enabled+0x18/0x34
> [    4.225777]
> [    4.225777] but task is already holding lock:
> [    4.231615] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> thermal_zone_get_temp+0x38/0x6c
> [    4.239121]
> [    4.239121] other info that might help us debug this:
> [    4.245655]  Possible unsafe locking scenario:
> [    4.245655]
> [    4.251579]        CPU0
> [    4.254031]        ----
> [    4.256481]   lock(&tz->lock);
> [    4.259544]   lock(&tz->lock);
> [    4.262608]
> [    4.262608]  *** DEADLOCK ***
> [    4.262608]
> [    4.268533]  May be due to missing lock nesting notation
> [    4.268533]
> [    4.275329] 4 locks held by kworker/0:3/54:
> [    4.279517]  #0: cb0066a8 ((wq_completion)events){+.+.}-{0:0}, at:
> process_one_work+0x224/0x808
> [    4.288241]  #1: ca075f10 (deferred_probe_work){+.+.}-{0:0}, at:
> process_one_work+0x224/0x808
> [    4.296787]  #2: cb1a48d8 (&dev->mutex){....}-{3:3}, at:
> __device_attach+0x30/0x140
> [    4.304468]  #3: ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> thermal_zone_get_temp+0x38/0x6c
> [    4.312408]
> [    4.312408] stack backtrace:
> [    4.316778] CPU: 0 PID: 54 Comm: kworker/0:3 Not tainted
> 5.8.0-rc3-00011-gf5e50bf4d3ef #42
> [    4.325048] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    4.330809] Workqueue: events deferred_probe_work_func
> [    4.335973] [<c0312384>] (unwind_backtrace) from [<c030c580>]
> (show_stack+0x10/0x14)
> [    4.343734] [<c030c580>] (show_stack) from [<c079d7d8>]
> (dump_stack+0xe8/0x114)
> [    4.351062] [<c079d7d8>] (dump_stack) from [<c03abf78>]
> (__lock_acquire+0xbfc/0x2cb4)
> [    4.358909] [<c03abf78>] (__lock_acquire) from [<c03ae9c4>]
> (lock_acquire+0xf4/0x4e4)
> [    4.366758] [<c03ae9c4>] (lock_acquire) from [<c10630fc>]
> (__mutex_lock+0xb0/0xaa8)
> [    4.374431] [<c10630fc>] (__mutex_lock) from [<c1063b10>]
> (mutex_lock_nested+0x1c/0x24)
> [    4.382452] [<c1063b10>] (mutex_lock_nested) from [<c0d932c0>]
> (thermal_zone_device_is_enabled+0x18/0x34)
> [    4.392036] [<c0d932c0>] (thermal_zone_device_is_enabled) from
> [<c0d9da90>] (imx_get_temp+0x30/0x208)
> [    4.401271] [<c0d9da90>] (imx_get_temp) from [<c0d97484>]
> (thermal_zone_get_temp+0x4c/0x6c)
> [    4.409640] [<c0d97484>] (thermal_zone_get_temp) from [<c0d93df0>]
> (thermal_zone_device_update+0x8c/0x258)
> [    4.419310] [<c0d93df0>] (thermal_zone_device_update) from
> [<c0d9401c>] (thermal_zone_device_set_mode+0x60/0x88)
> [    4.429500] [<c0d9401c>] (thermal_zone_device_set_mode) from
> [<c0d9e1d4>] (imx_thermal_probe+0x3e4/0x578)
> [    4.439082] [<c0d9e1d4>] (imx_thermal_probe) from [<c0a78388>]
> (platform_drv_probe+0x48/0x98)
> [    4.447622] [<c0a78388>] (platform_drv_probe) from [<c0a7603c>]
> (really_probe+0x218/0x348)
> [    4.455903] [<c0a7603c>] (really_probe) from [<c0a76278>]
> (driver_probe_device+0x5c/0xb4)
> [    4.464098] [<c0a76278>] (driver_probe_device) from [<c0a743bc>]
> (bus_for_each_drv+0x58/0xb8)
> [    4.472638] [<c0a743bc>] (bus_for_each_drv) from [<c0a75db0>]
> (__device_attach+0xd4/0x140)
> [    4.480919] [<c0a75db0>] (__device_attach) from [<c0a750b0>]
> (bus_probe_device+0x88/0x90)
> [    4.489112] [<c0a750b0>] (bus_probe_device) from [<c0a75564>]
> (deferred_probe_work_func+0x68/0x98)
> [    4.498088] [<c0a75564>] (deferred_probe_work_func) from [<c0369988>]
> (process_one_work+0x2e0/0x808)
> [    4.507237] [<c0369988>] (process_one_work) from [<c036a150>]
> (worker_thread+0x2a0/0x59c)
> [    4.515432] [<c036a150>] (worker_thread) from [<c0372208>]
> (kthread+0x16c/0x178)
> [    4.522843] [<c0372208>] (kthread) from [<c0300174>]
> (ret_from_fork+0x14/0x20)
> [    4.530074] Exception stack(0xca075fb0 to 0xca075ff8)
> [    4.535138] 5fa0:                                     00000000
> 00000000 00000000 00000000
> [    4.543328] 5fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [    4.551516] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 
> 
> 

Thanks!

That confirms your suspicions.

So the reason is that ->get_temp() is called while the mutex is held and
thermal_zone_device_is_enabled() wants to take the same mutex.

Is adding a comment to thermal_zone_device_is_enabled() to never call
it while the mutex is held and adding another version of it which does
not take the mutex ok?

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 02/07/2020 19:19, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 02.07.2020 o 19:01, Daniel Lezcano pisze:
>> On 02/07/2020 15:53, Andrzej Pietrasiewicz wrote:
>>> Hi Daniel,
>>>
>>> <snip>
>>>
>>>>>>>>
>>>>>>>> I did reproduce:
>>>>>>>>
>>>>>>>> v5.8-rc3 + series => imx6 hang at boot time
>>>>>>>> v5.8-rc3 => imx6 boots correctly
>>>>
>>>> So finally I succeeded to reproduce it on my imx7 locally. The sensor
>>>> was failing to initialize for another reason related to the legacy
>>>> cooling device, this is why it is not appearing on the imx7.
>>>>
>>>> I can now git-bisect :)
>>>>
>>>
>>> That would be very kind of you, thank you!
>>
>> With the lock correctness option enabled:
>>
>> [    4.179223] imx_thermal tempmon: Extended Commercial CPU temperature
>> grade - max:105C critical:100C passive:95C
>> [    4.189557]
>> [    4.191060] ============================================
>> [    4.196378] WARNING: possible recursive locking detected
>> [    4.201699] 5.8.0-rc3-00011-gf5e50bf4d3ef #42 Not tainted
>> [    4.207102] --------------------------------------------
>> [    4.212421] kworker/0:3/54 is trying to acquire lock:
>> [    4.217480] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
>> thermal_zone_device_is_enabled+0x18/0x34
>> [    4.225777]
>> [    4.225777] but task is already holding lock:
>> [    4.231615] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
>> thermal_zone_get_temp+0x38/0x6c
>> [    4.239121]
>> [    4.239121] other info that might help us debug this:
>> [    4.245655]  Possible unsafe locking scenario:
>> [    4.245655]
>> [    4.251579]        CPU0
>> [    4.254031]        ----
>> [    4.256481]   lock(&tz->lock);
>> [    4.259544]   lock(&tz->lock);
>> [    4.262608]
>> [    4.262608]  *** DEADLOCK ***
>> [    4.262608]
>> [    4.268533]  May be due to missing lock nesting notation
>> [    4.268533]
>> [    4.275329] 4 locks held by kworker/0:3/54:
>> [    4.279517]  #0: cb0066a8 ((wq_completion)events){+.+.}-{0:0}, at:
>> process_one_work+0x224/0x808
>> [    4.288241]  #1: ca075f10 (deferred_probe_work){+.+.}-{0:0}, at:
>> process_one_work+0x224/0x808
>> [    4.296787]  #2: cb1a48d8 (&dev->mutex){....}-{3:3}, at:
>> __device_attach+0x30/0x140
>> [    4.304468]  #3: ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
>> thermal_zone_get_temp+0x38/0x6c
>> [    4.312408]
>> [    4.312408] stack backtrace:
>> [    4.316778] CPU: 0 PID: 54 Comm: kworker/0:3 Not tainted
>> 5.8.0-rc3-00011-gf5e50bf4d3ef #42
>> [    4.325048] Hardware name: Freescale i.MX7 Dual (Device Tree)
>> [    4.330809] Workqueue: events deferred_probe_work_func
>> [    4.335973] [<c0312384>] (unwind_backtrace) from [<c030c580>]
>> (show_stack+0x10/0x14)
>> [    4.343734] [<c030c580>] (show_stack) from [<c079d7d8>]
>> (dump_stack+0xe8/0x114)
>> [    4.351062] [<c079d7d8>] (dump_stack) from [<c03abf78>]
>> (__lock_acquire+0xbfc/0x2cb4)
>> [    4.358909] [<c03abf78>] (__lock_acquire) from [<c03ae9c4>]
>> (lock_acquire+0xf4/0x4e4)
>> [    4.366758] [<c03ae9c4>] (lock_acquire) from [<c10630fc>]
>> (__mutex_lock+0xb0/0xaa8)
>> [    4.374431] [<c10630fc>] (__mutex_lock) from [<c1063b10>]
>> (mutex_lock_nested+0x1c/0x24)
>> [    4.382452] [<c1063b10>] (mutex_lock_nested) from [<c0d932c0>]
>> (thermal_zone_device_is_enabled+0x18/0x34)
>> [    4.392036] [<c0d932c0>] (thermal_zone_device_is_enabled) from
>> [<c0d9da90>] (imx_get_temp+0x30/0x208)
>> [    4.401271] [<c0d9da90>] (imx_get_temp) from [<c0d97484>]
>> (thermal_zone_get_temp+0x4c/0x6c)
>> [    4.409640] [<c0d97484>] (thermal_zone_get_temp) from [<c0d93df0>]
>> (thermal_zone_device_update+0x8c/0x258)
>> [    4.419310] [<c0d93df0>] (thermal_zone_device_update) from
>> [<c0d9401c>] (thermal_zone_device_set_mode+0x60/0x88)
>> [    4.429500] [<c0d9401c>] (thermal_zone_device_set_mode) from
>> [<c0d9e1d4>] (imx_thermal_probe+0x3e4/0x578)
>> [    4.439082] [<c0d9e1d4>] (imx_thermal_probe) from [<c0a78388>]
>> (platform_drv_probe+0x48/0x98)
>> [    4.447622] [<c0a78388>] (platform_drv_probe) from [<c0a7603c>]
>> (really_probe+0x218/0x348)
>> [    4.455903] [<c0a7603c>] (really_probe) from [<c0a76278>]
>> (driver_probe_device+0x5c/0xb4)
>> [    4.464098] [<c0a76278>] (driver_probe_device) from [<c0a743bc>]
>> (bus_for_each_drv+0x58/0xb8)
>> [    4.472638] [<c0a743bc>] (bus_for_each_drv) from [<c0a75db0>]
>> (__device_attach+0xd4/0x140)
>> [    4.480919] [<c0a75db0>] (__device_attach) from [<c0a750b0>]
>> (bus_probe_device+0x88/0x90)
>> [    4.489112] [<c0a750b0>] (bus_probe_device) from [<c0a75564>]
>> (deferred_probe_work_func+0x68/0x98)
>> [    4.498088] [<c0a75564>] (deferred_probe_work_func) from [<c0369988>]
>> (process_one_work+0x2e0/0x808)
>> [    4.507237] [<c0369988>] (process_one_work) from [<c036a150>]
>> (worker_thread+0x2a0/0x59c)
>> [    4.515432] [<c036a150>] (worker_thread) from [<c0372208>]
>> (kthread+0x16c/0x178)
>> [    4.522843] [<c0372208>] (kthread) from [<c0300174>]
>> (ret_from_fork+0x14/0x20)
>> [    4.530074] Exception stack(0xca075fb0 to 0xca075ff8)
>> [    4.535138] 5fa0:                                     00000000
>> 00000000 00000000 00000000
>> [    4.543328] 5fc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [    4.551516] 5fe0: 00000000 00000000 00000000 00000000 00000013
>> 00000000
>>
>>
>>
> 
> Thanks!
> 
> That confirms your suspicions.
> 
> So the reason is that ->get_temp() is called while the mutex is held and
> thermal_zone_device_is_enabled() wants to take the same mutex.

Yes, that's correct.

> Is adding a comment to thermal_zone_device_is_enabled() to never call
> it while the mutex is held and adding another version of it which does
> not take the mutex ok?

The thermal_zone_device_is_enabled() is only used in two places, acpi
and this imx driver, and given:

1. as soon as the mutex is released, there is no guarantee the thermal
zone won't be changed right after, the lock is pointless, thus the
information also.

2. from a design point of view, I don't see why a driver should know if
a thermal zone is disabled or not

It would make sense to end with this function and do not give the
different drivers an opportunity to access this information.

Why not add change_mode for the acpi in order to enable or disable the
events and for imx_thermal use irq_enabled flag instead of the thermal
zone mode? Moreover it is very unclear why this function is needed in
imx_get_temp(), and I suspect we should be able to get rid of it.


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 02/07/2020 19:49, Daniel Lezcano wrote:

[ ... ]

>> Thanks!
>>
>> That confirms your suspicions.
>>
>> So the reason is that ->get_temp() is called while the mutex is held and
>> thermal_zone_device_is_enabled() wants to take the same mutex.
> 
> Yes, that's correct.
> 
>> Is adding a comment to thermal_zone_device_is_enabled() to never call
>> it while the mutex is held and adding another version of it which does
>> not take the mutex ok?
> 
> The thermal_zone_device_is_enabled() is only used in two places, acpi
> and this imx driver, and given:
> 
> 1. as soon as the mutex is released, there is no guarantee the thermal
> zone won't be changed right after, the lock is pointless, thus the
> information also.
> 
> 2. from a design point of view, I don't see why a driver should know if
> a thermal zone is disabled or not
> 
> It would make sense to end with this function and do not give the
> different drivers an opportunity to access this information.
> 
> Why not add change_mode for the acpi in order to enable or disable the
> events and for imx_thermal use irq_enabled flag instead of the thermal
> zone mode? Moreover it is very unclear why this function is needed in
> imx_get_temp(), and I suspect we should be able to get rid of it.

If you agree with that you can send a patch on top of your series so I
can test it fixes the imx platform.


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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Zhang Rui]

On Thu, 2020-07-02 at 19:49 +0200, Daniel Lezcano wrote:
> On 02/07/2020 19:19, Andrzej Pietrasiewicz wrote:
> > Hi,
> > 
> > W dniu 02.07.2020 o 19:01, Daniel Lezcano pisze:
> > > On 02/07/2020 15:53, Andrzej Pietrasiewicz wrote:
> > > > Hi Daniel,
> > > > 
> > > > <snip>
> > > > 
> > > > > > > > > 
> > > > > > > > > I did reproduce:
> > > > > > > > > 
> > > > > > > > > v5.8-rc3 + series => imx6 hang at boot time
> > > > > > > > > v5.8-rc3 => imx6 boots correctly
> > > > > 
> > > > > So finally I succeeded to reproduce it on my imx7 locally.
> > > > > The sensor
> > > > > was failing to initialize for another reason related to the
> > > > > legacy
> > > > > cooling device, this is why it is not appearing on the imx7.
> > > > > 
> > > > > I can now git-bisect :)
> > > > > 
> > > > 
> > > > That would be very kind of you, thank you!
> > > 
> > > With the lock correctness option enabled:
> > > 
> > > [    4.179223] imx_thermal tempmon: Extended Commercial CPU
> > > temperature
> > > grade - max:105C critical:100C passive:95C
> > > [    4.189557]
> > > [    4.191060] ============================================
> > > [    4.196378] WARNING: possible recursive locking detected
> > > [    4.201699] 5.8.0-rc3-00011-gf5e50bf4d3ef #42 Not tainted
> > > [    4.207102] --------------------------------------------
> > > [    4.212421] kworker/0:3/54 is trying to acquire lock:
> > > [    4.217480] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> > > thermal_zone_device_is_enabled+0x18/0x34
> > > [    4.225777]
> > > [    4.225777] but task is already holding lock:
> > > [    4.231615] ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> > > thermal_zone_get_temp+0x38/0x6c
> > > [    4.239121]
> > > [    4.239121] other info that might help us debug this:
> > > [    4.245655]  Possible unsafe locking scenario:
> > > [    4.245655]
> > > [    4.251579]        CPU0
> > > [    4.254031]        ----
> > > [    4.256481]   lock(&tz->lock);
> > > [    4.259544]   lock(&tz->lock);
> > > [    4.262608]
> > > [    4.262608]  *** DEADLOCK ***
> > > [    4.262608]
> > > [    4.268533]  May be due to missing lock nesting notation
> > > [    4.268533]
> > > [    4.275329] 4 locks held by kworker/0:3/54:
> > > [    4.279517]  #0: cb0066a8 ((wq_completion)events){+.+.}-{0:0}, 
> > > at:
> > > process_one_work+0x224/0x808
> > > [    4.288241]  #1: ca075f10 (deferred_probe_work){+.+.}-{0:0},
> > > at:
> > > process_one_work+0x224/0x808
> > > [    4.296787]  #2: cb1a48d8 (&dev->mutex){....}-{3:3}, at:
> > > __device_attach+0x30/0x140
> > > [    4.304468]  #3: ca09a3e4 (&tz->lock){+.+.}-{3:3}, at:
> > > thermal_zone_get_temp+0x38/0x6c
> > > [    4.312408]
> > > [    4.312408] stack backtrace:
> > > [    4.316778] CPU: 0 PID: 54 Comm: kworker/0:3 Not tainted
> > > 5.8.0-rc3-00011-gf5e50bf4d3ef #42
> > > [    4.325048] Hardware name: Freescale i.MX7 Dual (Device Tree)
> > > [    4.330809] Workqueue: events deferred_probe_work_func
> > > [    4.335973] [<c0312384>] (unwind_backtrace) from [<c030c580>]
> > > (show_stack+0x10/0x14)
> > > [    4.343734] [<c030c580>] (show_stack) from [<c079d7d8>]
> > > (dump_stack+0xe8/0x114)
> > > [    4.351062] [<c079d7d8>] (dump_stack) from [<c03abf78>]
> > > (__lock_acquire+0xbfc/0x2cb4)
> > > [    4.358909] [<c03abf78>] (__lock_acquire) from [<c03ae9c4>]
> > > (lock_acquire+0xf4/0x4e4)
> > > [    4.366758] [<c03ae9c4>] (lock_acquire) from [<c10630fc>]
> > > (__mutex_lock+0xb0/0xaa8)
> > > [    4.374431] [<c10630fc>] (__mutex_lock) from [<c1063b10>]
> > > (mutex_lock_nested+0x1c/0x24)
> > > [    4.382452] [<c1063b10>] (mutex_lock_nested) from [<c0d932c0>]
> > > (thermal_zone_device_is_enabled+0x18/0x34)
> > > [    4.392036] [<c0d932c0>] (thermal_zone_device_is_enabled) from
> > > [<c0d9da90>] (imx_get_temp+0x30/0x208)
> > > [    4.401271] [<c0d9da90>] (imx_get_temp) from [<c0d97484>]
> > > (thermal_zone_get_temp+0x4c/0x6c)
> > > [    4.409640] [<c0d97484>] (thermal_zone_get_temp) from
> > > [<c0d93df0>]
> > > (thermal_zone_device_update+0x8c/0x258)
> > > [    4.419310] [<c0d93df0>] (thermal_zone_device_update) from
> > > [<c0d9401c>] (thermal_zone_device_set_mode+0x60/0x88)
> > > [    4.429500] [<c0d9401c>] (thermal_zone_device_set_mode) from
> > > [<c0d9e1d4>] (imx_thermal_probe+0x3e4/0x578)
> > > [    4.439082] [<c0d9e1d4>] (imx_thermal_probe) from [<c0a78388>]
> > > (platform_drv_probe+0x48/0x98)
> > > [    4.447622] [<c0a78388>] (platform_drv_probe) from
> > > [<c0a7603c>]
> > > (really_probe+0x218/0x348)
> > > [    4.455903] [<c0a7603c>] (really_probe) from [<c0a76278>]
> > > (driver_probe_device+0x5c/0xb4)
> > > [    4.464098] [<c0a76278>] (driver_probe_device) from
> > > [<c0a743bc>]
> > > (bus_for_each_drv+0x58/0xb8)
> > > [    4.472638] [<c0a743bc>] (bus_for_each_drv) from [<c0a75db0>]
> > > (__device_attach+0xd4/0x140)
> > > [    4.480919] [<c0a75db0>] (__device_attach) from [<c0a750b0>]
> > > (bus_probe_device+0x88/0x90)
> > > [    4.489112] [<c0a750b0>] (bus_probe_device) from [<c0a75564>]
> > > (deferred_probe_work_func+0x68/0x98)
> > > [    4.498088] [<c0a75564>] (deferred_probe_work_func) from
> > > [<c0369988>]
> > > (process_one_work+0x2e0/0x808)
> > > [    4.507237] [<c0369988>] (process_one_work) from [<c036a150>]
> > > (worker_thread+0x2a0/0x59c)
> > > [    4.515432] [<c036a150>] (worker_thread) from [<c0372208>]
> > > (kthread+0x16c/0x178)
> > > [    4.522843] [<c0372208>] (kthread) from [<c0300174>]
> > > (ret_from_fork+0x14/0x20)
> > > [    4.530074] Exception stack(0xca075fb0 to 0xca075ff8)
> > > [    4.535138] 5fa0:                                     00000000
> > > 00000000 00000000 00000000
> > > [    4.543328] 5fc0: 00000000 00000000 00000000 00000000 00000000
> > > 00000000 00000000 00000000
> > > [    4.551516] 5fe0: 00000000 00000000 00000000 00000000 00000013
> > > 00000000
> > > 
> > > 
> > > 
> > 
> > Thanks!
> > 
> > That confirms your suspicions.
> > 
> > So the reason is that ->get_temp() is called while the mutex is
> > held and
> > thermal_zone_device_is_enabled() wants to take the same mutex.
> 
> Yes, that's correct.
> 
> > Is adding a comment to thermal_zone_device_is_enabled() to never
> > call
> > it while the mutex is held and adding another version of it which
> > does
> > not take the mutex ok?
> 
> The thermal_zone_device_is_enabled() is only used in two places, acpi
> and this imx driver, and given:
> 
> 1. as soon as the mutex is released, there is no guarantee the
> thermal
> zone won't be changed right after, the lock is pointless, thus the
> information also.
> 
> 2. from a design point of view, I don't see why a driver should know
> if
> a thermal zone is disabled or not
> 
> It would make sense to end with this function and do not give the
> different drivers an opportunity to access this information.

I agree.
> 
> Why not add change_mode for the acpi in order to enable or disable
> the
> events

thermal_zone_device_is_enabled() is invoked in acpi thermal driver
because we only want to do thermal_zone_device_update() when the acpi
thermal zone is enabled.

As thermal_zone_device_update() can handle a disabled thermal zone now,
we can just remove the check.

thanks,
rui

>  and for imx_thermal use irq_enabled flag instead of the thermal
> zone mode? Moreover it is very unclear why this function is needed in
> imx_get_temp(), and I suspect we should be able to get rid of it.
> 
> 

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 03/07/2020 03:49, Zhang Rui wrote:
> On Thu, 2020-07-02 at 19:49 +0200, Daniel Lezcano wrote:

[ ... ]

>>> So the reason is that ->get_temp() is called while the mutex is
>>> held and
>>> thermal_zone_device_is_enabled() wants to take the same mutex.
>>
>> Yes, that's correct.
>>
>>> Is adding a comment to thermal_zone_device_is_enabled() to never
>>> call
>>> it while the mutex is held and adding another version of it which
>>> does
>>> not take the mutex ok?
>>
>> The thermal_zone_device_is_enabled() is only used in two places, acpi
>> and this imx driver, and given:
>>
>> 1. as soon as the mutex is released, there is no guarantee the
>> thermal
>> zone won't be changed right after, the lock is pointless, thus the
>> information also.
>>
>> 2. from a design point of view, I don't see why a driver should know
>> if
>> a thermal zone is disabled or not
>>
>> It would make sense to end with this function and do not give the
>> different drivers an opportunity to access this information.
> 
> I agree.
>>
>> Why not add change_mode for the acpi in order to enable or disable
>> the
>> events
> 
> thermal_zone_device_is_enabled() is invoked in acpi thermal driver
> because we only want to do thermal_zone_device_update() when the acpi
> thermal zone is enabled.
> 
> As thermal_zone_device_update() can handle a disabled thermal zone now,
> we can just remove the check.

Ah yes, good point!



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

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Andrzej Pietrasiewicz]

Hi,

W dniu 03.07.2020 o 08:38, Daniel Lezcano pisze:
> On 03/07/2020 03:49, Zhang Rui wrote:
>> On Thu, 2020-07-02 at 19:49 +0200, Daniel Lezcano wrote:
> 
> [ ... ]
> 
>>>> So the reason is that ->get_temp() is called while the mutex is
>>>> held and
>>>> thermal_zone_device_is_enabled() wants to take the same mutex.
>>>
>>> Yes, that's correct.
>>>
>>>> Is adding a comment to thermal_zone_device_is_enabled() to never
>>>> call
>>>> it while the mutex is held and adding another version of it which
>>>> does
>>>> not take the mutex ok?
>>>
>>> The thermal_zone_device_is_enabled() is only used in two places, acpi
>>> and this imx driver, and given:
>>>
>>> 1. as soon as the mutex is released, there is no guarantee the
>>> thermal
>>> zone won't be changed right after, the lock is pointless, thus the
>>> information also.
>>>
>>> 2. from a design point of view, I don't see why a driver should know
>>> if
>>> a thermal zone is disabled or not
>>>
>>> It would make sense to end with this function and do not give the
>>> different drivers an opportunity to access this information.
>>
>> I agree.
>>>
>>> Why not add change_mode for the acpi in order to enable or disable
>>> the
>>> events
>>
>> thermal_zone_device_is_enabled() is invoked in acpi thermal driver
>> because we only want to do thermal_zone_device_update() when the acpi
>> thermal zone is enabled.
>>
>> As thermal_zone_device_update() can handle a disabled thermal zone now,
>> we can just remove the check.
> 
> Ah yes, good point!
> 
> 
> 

I sent a short series with fixes. Daniel, can you kindly test it?

Regards,

Andrzej

Re: [PATCH v7 00/11] Stop monitoring disabled devices

[Daniel Lezcano]

On 03/07/2020 12:45, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 03.07.2020 o 08:38, Daniel Lezcano pisze:
>> On 03/07/2020 03:49, Zhang Rui wrote:
>>> On Thu, 2020-07-02 at 19:49 +0200, Daniel Lezcano wrote:
>>
>> [ ... ]
>>
>>>>> So the reason is that ->get_temp() is called while the mutex is
>>>>> held and
>>>>> thermal_zone_device_is_enabled() wants to take the same mutex.
>>>>
>>>> Yes, that's correct.
>>>>
>>>>> Is adding a comment to thermal_zone_device_is_enabled() to never
>>>>> call
>>>>> it while the mutex is held and adding another version of it which
>>>>> does
>>>>> not take the mutex ok?
>>>>
>>>> The thermal_zone_device_is_enabled() is only used in two places, acpi
>>>> and this imx driver, and given:
>>>>
>>>> 1. as soon as the mutex is released, there is no guarantee the
>>>> thermal
>>>> zone won't be changed right after, the lock is pointless, thus the
>>>> information also.
>>>>
>>>> 2. from a design point of view, I don't see why a driver should know
>>>> if
>>>> a thermal zone is disabled or not
>>>>
>>>> It would make sense to end with this function and do not give the
>>>> different drivers an opportunity to access this information.
>>>
>>> I agree.
>>>>
>>>> Why not add change_mode for the acpi in order to enable or disable
>>>> the
>>>> events
>>>
>>> thermal_zone_device_is_enabled() is invoked in acpi thermal driver
>>> because we only want to do thermal_zone_device_update() when the acpi
>>> thermal zone is enabled.
>>>
>>> As thermal_zone_device_update() can handle a disabled thermal zone now,
>>> we can just remove the check.
>>
>> Ah yes, good point!
>>
>>
>>
> 
> I sent a short series with fixes. Daniel, can you kindly test it?

I confirm the i.MX is now correctly booting with the thermal zone
temperature available.


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