[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200529144821.GA93994@roeck-us.net>
Date: Fri, 29 May 2020 07:48:21 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
Cc: linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
platform-driver-x86@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-renesas-soc@...r.kernel.org,
linux-rockchip@...ts.infradead.org,
Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
Heiko Stuebner <heiko@...ech.de>,
"Rafael J . Wysocki" <rjw@...ysocki.net>,
Vishal Kulkarni <vishal@...lsio.com>,
Luca Coelho <luciano.coelho@...el.com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
kernel@...labora.com, Fabio Estevam <festevam@...il.com>,
Amit Kucheria <amit.kucheria@...durent.com>,
Chunyan Zhang <zhang.lyra@...il.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Allison Randal <allison@...utok.net>,
NXP Linux Team <linux-imx@....com>,
Darren Hart <dvhart@...radead.org>,
Zhang Rui <rui.zhang@...el.com>,
Gayatri Kammela <gayatri.kammela@...el.com>,
Len Brown <lenb@...nel.org>,
Johannes Berg <johannes.berg@...el.com>,
Intel Linux Wireless <linuxwifi@...el.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Ido Schimmel <idosch@...lanox.com>,
Baolin Wang <baolin.wang7@...il.com>,
Jiri Pirko <jiri@...lanox.com>,
Orson Zhai <orsonzhai@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
Kalle Valo <kvalo@...eaurora.org>,
Support Opensource <support.opensource@...semi.com>,
Enrico Weigelt <info@...ux.net>,
Peter Kaestle <peter@...e.net>,
Sebastian Reichel <sre@...nel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Niklas Söderlund
<niklas.soderlund@...natech.se>, Shawn Guo <shawnguo@...nel.org>,
"David S . Miller" <davem@...emloft.net>,
Andy Shevchenko <andy@...radead.org>
Subject: Re: [PATCH v4 02/11] thermal: Store thermal mode in a dedicated enum
On Thu, May 28, 2020 at 09:20:42PM +0200, Andrzej Pietrasiewicz wrote:
> Prepare for storing mode in struct thermal_zone_device.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
> ---
> 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;
Personally I find this check unnecessary: The enum has no other values,
and it is verifyable that the callers provide the enum and not some other
value.
> /*
> * 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 8cc86f4e3ac1..830a8b060e74 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;
Same as above.
>
> - 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);
Powered by blists - more mailing lists