Re: [PATCH v1 1/2] thermal: intel: intel_soc_dts_iosf: Always use 2 trips
From: srinivas pandruvada
Date: Thu Aug 10 2023 - 12:53:19 EST
On Wed, 2023-08-09 at 22:27 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Both the existing callers of intel_soc_dts_iosf_init() pass 2 as the
> trip
> count argument, so it can be replaced with SOC_MAX_DTS_TRIPS
> everywhere in
> the code and the trip_count argument of that function can be dropped.
>
> This also allows the trip_count field to be dropped from struct
> intel_soc_dts_sensor_entry, as it is always equal to 2, and some
> related code can be simplified.
>
> Make changes accordingly.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Reviewed-by: Srinivas Pandruvada<srinivas.pandruvada@xxxxxxxxxxxxxxx>
> ---
> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci_l
> egacy.c | 2
> drivers/thermal/intel/intel_soc_dts_iosf.c
> | 26 +++-------
> drivers/thermal/intel/intel_soc_dts_iosf.h
> | 9 +--
> drivers/thermal/intel/intel_soc_dts_thermal.c
> | 2
> 4 files changed, 15 insertions(+), 24 deletions(-)
>
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -37,9 +37,6 @@
> /* DTS encoding for TJ MAX temperature */
> #define SOC_DTS_TJMAX_ENCODING 0x7F
>
> -/* Only 2 out of 4 is allowed for OSPM */
> -#define SOC_MAX_DTS_TRIPS 2
> -
> /* Mask for two trips in status bits */
> #define SOC_DTS_TRIP_MASK 0x03
>
> @@ -253,12 +250,10 @@ static void remove_dts_thermal_zone(stru
> }
>
> static int add_dts_thermal_zone(int id, struct
> intel_soc_dts_sensor_entry *dts,
> - bool notification_support, int
> trip_cnt,
> - int read_only_trip_cnt)
> + bool notification_support, int
> read_only_trip_cnt)
> {
> char name[10];
> unsigned long trip;
> - int trip_count = 0;
> int trip_mask = 0;
> int writable_trip_cnt = 0;
> unsigned long ptps;
> @@ -274,8 +269,7 @@ static int add_dts_thermal_zone(int id,
>
> dts->id = id;
> if (notification_support) {
> - trip_count = min(SOC_MAX_DTS_TRIPS, trip_cnt);
> - writable_trip_cnt = trip_count - read_only_trip_cnt;
> + writable_trip_cnt = SOC_MAX_DTS_TRIPS -
> read_only_trip_cnt;
> trip_mask = GENMASK(writable_trip_cnt - 1, 0);
> }
>
> @@ -290,10 +284,9 @@ static int add_dts_thermal_zone(int id,
> trip_mask &= ~BIT(i / 8);
> }
> dts->trip_mask = trip_mask;
> - dts->trip_count = trip_count;
> snprintf(name, sizeof(name), "soc_dts%d", id);
> dts->tzone = thermal_zone_device_register(name,
> - trip_count,
> + SOC_MAX_DTS_TRIPS,
> trip_mask,
> dts, &tzone_ops,
> NULL, 0, 0);
> @@ -324,11 +317,10 @@ int intel_soc_dts_iosf_add_read_only_cri
> for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> struct intel_soc_dts_sensor_entry *entry = &sensors-
> >soc_dts[i];
> int temp = sensors->tj_max - critical_offset;
> - unsigned long count = entry->trip_count;
> unsigned long mask = entry->trip_mask;
>
> - j = find_first_zero_bit(&mask, count);
> - if (j < count)
> + j = find_first_zero_bit(&mask, SOC_MAX_DTS_TRIPS);
> + if (j < SOC_MAX_DTS_TRIPS)
> return update_trip_temp(entry, j, temp,
> THERMAL_TRIP_CRITICAL);
> }
>
> @@ -372,8 +364,7 @@ void intel_soc_dts_iosf_interrupt_handle
> EXPORT_SYMBOL_GPL(intel_soc_dts_iosf_interrupt_handler);
>
> struct intel_soc_dts_sensors *intel_soc_dts_iosf_init(
> - enum intel_soc_dts_interrupt_type intr_type, int trip_count,
> - int read_only_trip_count)
> + enum intel_soc_dts_interrupt_type intr_type, int
> read_only_trip_count)
> {
> struct intel_soc_dts_sensors *sensors;
> bool notification;
> @@ -384,7 +375,7 @@ struct intel_soc_dts_sensors *intel_soc_
> if (!iosf_mbi_available())
> return ERR_PTR(-ENODEV);
>
> - if (!trip_count || read_only_trip_count > trip_count)
> + if (read_only_trip_count > SOC_MAX_DTS_TRIPS)
> return ERR_PTR(-EINVAL);
>
> tj_max = intel_tcc_get_tjmax(-1);
> @@ -406,8 +397,7 @@ struct intel_soc_dts_sensors *intel_soc_
> for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> sensors->soc_dts[i].sensors = sensors;
> ret = add_dts_thermal_zone(i, &sensors->soc_dts[i],
> - notification, trip_count,
> - read_only_trip_count);
> + notification,
> read_only_trip_count);
> if (ret)
> goto err_free;
> }
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.h
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> @@ -12,6 +12,9 @@
> /* DTS0 and DTS 1 */
> #define SOC_MAX_DTS_SENSORS 2
>
> +/* Only 2 out of 4 is allowed for OSPM */
> +#define SOC_MAX_DTS_TRIPS 2
> +
> enum intel_soc_dts_interrupt_type {
> INTEL_SOC_DTS_INTERRUPT_NONE,
> INTEL_SOC_DTS_INTERRUPT_APIC,
> @@ -26,8 +29,7 @@ struct intel_soc_dts_sensor_entry {
> int id;
> u32 store_status;
> u32 trip_mask;
> - u32 trip_count;
> - enum thermal_trip_type trip_types[2];
> + enum thermal_trip_type trip_types[SOC_MAX_DTS_TRIPS];
> struct thermal_zone_device *tzone;
> struct intel_soc_dts_sensors *sensors;
> };
> @@ -41,8 +43,7 @@ struct intel_soc_dts_sensors {
> };
>
> struct intel_soc_dts_sensors *intel_soc_dts_iosf_init(
> - enum intel_soc_dts_interrupt_type intr_type, int trip_count,
> - int read_only_trip_count);
> + enum intel_soc_dts_interrupt_type intr_type, int
> read_only_trip_count);
> void intel_soc_dts_iosf_exit(struct intel_soc_dts_sensors *sensors);
> void intel_soc_dts_iosf_interrupt_handler(
> struct intel_soc_dts_sensors
> *sensors);
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_thermal.c
> @@ -51,7 +51,7 @@ static int __init intel_soc_thermal_init
> return -ENODEV;
>
> /* Create a zone with 2 trips with marked as read only */
> - soc_dts =
> intel_soc_dts_iosf_init(INTEL_SOC_DTS_INTERRUPT_APIC, 2, 1);
> + soc_dts =
> intel_soc_dts_iosf_init(INTEL_SOC_DTS_INTERRUPT_APIC, 1);
> if (IS_ERR(soc_dts)) {
> err = PTR_ERR(soc_dts);
> return err;
> Index: linux-
> pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci
> _legacy.c
> ===================================================================
> --- linux-
> pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_devic
> e_pci_legacy.c
> +++ linux-
> pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci
> _legacy.c
> @@ -59,7 +59,7 @@ static int proc_thermal_pci_probe(struct
> * ACPI/MSR. So we don't want to fail for auxiliary
> DTSs.
> */
> proc_priv->soc_dts = intel_soc_dts_iosf_init(
> - INTEL_SOC_DTS_INTERRUPT_MSI,
> 2, 0);
> + INTEL_SOC_DTS_INTERRUPT_MSI,
> 0);
>
> if (!IS_ERR(proc_priv->soc_dts) && pdev->irq) {
> ret = pci_enable_msi(pdev);
>
>
>