On Wed, 2023-01-04 at 23:21 +0100, Daniel Lezcano wrote:
> From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>
> The thermal framework gives the possibility to register the trip
> points with the thermal zone. When that is done, no get_trip_* ops
> are
> needed and they can be removed.
>
> Convert the ops content logic into generic trip points and register
> them with the thermal zone.
>
> In order to consolidate the code, use the ACPI thermal framework API
> to fill the generic trip point from the ACPI tables.
>
> It has been tested on a Intel i7-8650U - x280 with the INT3400, the
> PCH, ACPITZ, and x86_pkg_temp. No regression observed so far.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> ---
> V3:
> - The driver Kconfig option selects CONFIG_THERMAL_ACPI
> ---
> drivers/thermal/intel/Kconfig | 1 +
> drivers/thermal/intel/intel_pch_thermal.c | 88 +++++--------------
> ----
> 2 files changed, 20 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/thermal/intel/Kconfig
> b/drivers/thermal/intel/Kconfig
> index f0c845679250..738b88b290f4 100644
> --- a/drivers/thermal/intel/Kconfig
> +++ b/drivers/thermal/intel/Kconfig
> @@ -75,6 +75,7 @@ config INTEL_BXT_PMIC_THERMAL
> config INTEL_PCH_THERMAL
> tristate "Intel PCH Thermal Reporting Driver"
> depends on X86 && PCI
> + select THERMAL_ACPI
THERMAL_ACPI depends on ACPI but the PCH thermal driver does not.
So we will run into "unmet dependencies" issue when CONFIG_ACPI is
cleared like below
WARNING: unmet direct dependencies detected for THERMAL_ACPI
Depends on [n]: THERMAL [=y] && ACPI [=n]
Selected by [m]:
- INTEL_PCH_THERMAL [=m] && THERMAL [=y] && (X86 [=y] ||
X86_INTEL_QUARK [=n] || COMPILE_TEST [=n]) && X86 [=y] && PCI [=y]
thanks,
rui
> help
> Enable this to support thermal reporting on certain intel
> PCHs.
> Thermal reporting device will provide temperature reading,
> diff --git a/drivers/thermal/intel/intel_pch_thermal.c
> b/drivers/thermal/intel/intel_pch_thermal.c
> index dabf11a687a1..530fe9b38381 100644
> --- a/drivers/thermal/intel/intel_pch_thermal.c
> +++ b/drivers/thermal/intel/intel_pch_thermal.c
> @@ -65,6 +65,8 @@
> #define WPT_TEMP_OFFSET (PCH_TEMP_OFFSET *
> MILLIDEGREE_PER_DEGREE)
> #define GET_PCH_TEMP(x) (((x) / 2) + PCH_TEMP_OFFSET)
>
> +#define PCH_MAX_TRIPS 3 /* critical, hot, passive */
> +
> /* Amount of time for each cooling delay, 100ms by default for now
> */
> static unsigned int delay_timeout = 100;
> module_param(delay_timeout, int, 0644);
> @@ -82,12 +84,7 @@ struct pch_thermal_device {
> const struct pch_dev_ops *ops;
> struct pci_dev *pdev;
> struct thermal_zone_device *tzd;
> - int crt_trip_id;
> - unsigned long crt_temp;
> - int hot_trip_id;
> - unsigned long hot_temp;
> - int psv_trip_id;
> - unsigned long psv_temp;
> + struct thermal_trip trips[PCH_MAX_TRIPS];
> bool bios_enabled;
> };
>
> @@ -102,33 +99,22 @@ static void pch_wpt_add_acpi_psv_trip(struct
> pch_thermal_device *ptd,
> int *nr_trips)
> {
> struct acpi_device *adev;
> -
> - ptd->psv_trip_id = -1;
> + int ret;
>
> adev = ACPI_COMPANION(&ptd->pdev->dev);
> - if (adev) {
> - unsigned long long r;
> - acpi_status status;
> -
> - status = acpi_evaluate_integer(adev->handle, "_PSV",
> NULL,
> - &r);
> - if (ACPI_SUCCESS(status)) {
> - unsigned long trip_temp;
> -
> - trip_temp = deci_kelvin_to_millicelsius(r);
> - if (trip_temp) {
> - ptd->psv_temp = trip_temp;
> - ptd->psv_trip_id = *nr_trips;
> - ++(*nr_trips);
> - }
> - }
> - }
> + if (!adev)
> + return;
> +
> + ret = thermal_acpi_trip_psv(adev, &ptd->trips[*nr_trips]);
> + if (ret)
> + return;
> +
> + ++(*nr_trips);
> }
> #else
> static void pch_wpt_add_acpi_psv_trip(struct pch_thermal_device
> *ptd,
> int *nr_trips)
> {
> - ptd->psv_trip_id = -1;
>
> }
> #endif
> @@ -163,21 +149,19 @@ static int pch_wpt_init(struct
> pch_thermal_device *ptd, int *nr_trips)
> }
>
> read_trips:
> - ptd->crt_trip_id = -1;
> trip_temp = readw(ptd->hw_base + WPT_CTT);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->crt_temp = GET_WPT_TEMP(trip_temp);
> - ptd->crt_trip_id = 0;
> + ptd->trips[*nr_trips].temperature =
> GET_WPT_TEMP(trip_temp);
> + ptd->trips[*nr_trips].type = THERMAL_TRIP_CRITICAL;
> ++(*nr_trips);
> }
>
> - ptd->hot_trip_id = -1;
> trip_temp = readw(ptd->hw_base + WPT_PHL);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->hot_temp = GET_WPT_TEMP(trip_temp);
> - ptd->hot_trip_id = *nr_trips;
> + ptd->trips[*nr_trips].temperature =
> GET_WPT_TEMP(trip_temp);
> + ptd->trips[*nr_trips].type = THERMAL_TRIP_HOT;
> ++(*nr_trips);
> }
>
> @@ -298,39 +282,6 @@ static int pch_thermal_get_temp(struct
> thermal_zone_device *tzd, int *temp)
> return ptd->ops->get_temp(ptd, temp);
> }
>
> -static int pch_get_trip_type(struct thermal_zone_device *tzd, int
> trip,
> - enum thermal_trip_type *type)
> -{
> - struct pch_thermal_device *ptd = tzd->devdata;
> -
> - if (ptd->crt_trip_id == trip)
> - *type = THERMAL_TRIP_CRITICAL;
> - else if (ptd->hot_trip_id == trip)
> - *type = THERMAL_TRIP_HOT;
> - else if (ptd->psv_trip_id == trip)
> - *type = THERMAL_TRIP_PASSIVE;
> - else
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> -static int pch_get_trip_temp(struct thermal_zone_device *tzd, int
> trip, int *temp)
> -{
> - struct pch_thermal_device *ptd = tzd->devdata;
> -
> - if (ptd->crt_trip_id == trip)
> - *temp = ptd->crt_temp;
> - else if (ptd->hot_trip_id == trip)
> - *temp = ptd->hot_temp;
> - else if (ptd->psv_trip_id == trip)
> - *temp = ptd->psv_temp;
> - else
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> static void pch_critical(struct thermal_zone_device *tzd)
> {
> dev_dbg(&tzd->device, "%s: critical temperature reached\n",
> tzd->type);
> @@ -338,8 +289,6 @@ static void pch_critical(struct
> thermal_zone_device *tzd)
>
> static struct thermal_zone_device_ops tzd_ops = {
> .get_temp = pch_thermal_get_temp,
> - .get_trip_type = pch_get_trip_type,
> - .get_trip_temp = pch_get_trip_temp,
> .critical = pch_critical,
> };
>
> @@ -423,8 +372,9 @@ static int intel_pch_thermal_probe(struct pci_dev
> *pdev,
> if (err)
> goto error_cleanup;
>
> - ptd->tzd = thermal_zone_device_register(bi->name, nr_trips, 0,
> ptd,
> - &tzd_ops, NULL, 0, 0);
> + ptd->tzd = thermal_zone_device_register_with_trips(bi->name,
> ptd->trips,
> + nr_trips, 0,
> ptd,
> + &tzd_ops,
> NULL, 0, 0);
> if (IS_ERR(ptd->tzd)) {
> dev_err(&pdev->dev, "Failed to register thermal zone
> %s\n",
> bi->name);
