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 ops content logic into generic trip points and register them with the
thermal zone.
Signed-off-by: Daniel Lezcano <[email protected]>
---
.../processor_thermal_device_pci.c | 53 ++++++++-----------
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
index bf1b1cdfade4..c7d50862bf56 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -144,34 +144,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
return 0;
}
-static int sys_get_trip_temp(struct thermal_zone_device *tzd,
- int trip, int *temp)
-{
- struct proc_thermal_pci *pci_info = tzd->devdata;
- u32 _temp;
-
- proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &_temp);
- if (!_temp) {
- *temp = THERMAL_TEMP_INVALID;
- } else {
- int tjmax;
-
- proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
- _temp = tjmax - _temp;
- *temp = (unsigned long)_temp * 1000;
- }
-
- return 0;
-}
-
-static int sys_get_trip_type(struct thermal_zone_device *tzd, int trip,
- enum thermal_trip_type *type)
-{
- *type = THERMAL_TRIP_PASSIVE;
-
- return 0;
-}
-
static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
{
struct proc_thermal_pci *pci_info = tzd->devdata;
@@ -200,10 +172,26 @@ static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp
return 0;
}
+static int get_trip_temp(struct proc_thermal_pci *pci_info)
+{
+ int temp, tjmax;
+
+ proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &temp);
+ if (!temp)
+ return THERMAL_TEMP_INVALID;
+
+ proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
+ temp = (tjmax - temp) * 1000;
+
+ return temp;
+}
+
+static struct thermal_trip psv_trip = {
+ .type = THERMAL_TRIP_PASSIVE,
+};
+
static struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
- .get_trip_temp = sys_get_trip_temp,
- .get_trip_type = sys_get_trip_type,
.set_trip_temp = sys_set_trip_temp,
};
@@ -251,7 +239,10 @@ static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
if (ret)
goto err_ret_thermal;
- pci_info->tzone = thermal_zone_device_register("TCPU_PCI", 1, 1, pci_info,
+ psv_trip.temperature = get_trip_temp(pci_info);
+
+ pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
+ 1, 1, pci_info,
&tzone_ops,
&tzone_params, 0, 0);
if (IS_ERR(pci_info->tzone)) {
--
2.34.1
On Wed, 2023-01-18 at 19:16 +0100, Daniel Lezcano wrote:
> 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 ops content logic into generic trip points and register them
> with the
> thermal zone.
>
In this scheme is the assumption is that trip point temperature never
changes? If firmware updated the trip temperature, what needs to be
done?
Thanks,
Srinivas
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> .../processor_thermal_device_pci.c | 53 ++++++++---------
> --
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> index bf1b1cdfade4..c7d50862bf56 100644
> ---
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> +++
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> @@ -144,34 +144,6 @@ static int sys_get_curr_temp(struct
> thermal_zone_device *tzd, int *temp)
> return 0;
> }
>
> -static int sys_get_trip_temp(struct thermal_zone_device *tzd,
> - int trip, int *temp)
> -{
> - struct proc_thermal_pci *pci_info = tzd->devdata;
> - u32 _temp;
> -
> - proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0,
> &_temp);
> - if (!_temp) {
> - *temp = THERMAL_TEMP_INVALID;
> - } else {
> - int tjmax;
> -
> - proc_thermal_mmio_read(pci_info,
> PROC_THERMAL_MMIO_TJMAX, &tjmax);
> - _temp = tjmax - _temp;
> - *temp = (unsigned long)_temp * 1000;
> - }
> -
> - return 0;
> -}
> -
> -static int sys_get_trip_type(struct thermal_zone_device *tzd, int
> trip,
> - enum thermal_trip_type *type)
> -{
> - *type = THERMAL_TRIP_PASSIVE;
> -
> - return 0;
> -}
> -
> static int sys_set_trip_temp(struct thermal_zone_device *tzd, int
> trip, int temp)
> {
> struct proc_thermal_pci *pci_info = tzd->devdata;
> @@ -200,10 +172,26 @@ static int sys_set_trip_temp(struct
> thermal_zone_device *tzd, int trip, int temp
> return 0;
> }
>
> +static int get_trip_temp(struct proc_thermal_pci *pci_info)
> +{
> + int temp, tjmax;
> +
> + proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0,
> &temp);
> + if (!temp)
> + return THERMAL_TEMP_INVALID;
> +
> + proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX,
> &tjmax);
> + temp = (tjmax - temp) * 1000;
> +
> + return temp;
> +}
> +
> +static struct thermal_trip psv_trip = {
> + .type = THERMAL_TRIP_PASSIVE,
> +};
> +
> static struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> - .get_trip_temp = sys_get_trip_temp,
> - .get_trip_type = sys_get_trip_type,
> .set_trip_temp = sys_set_trip_temp,
> };
>
> @@ -251,7 +239,10 @@ static int proc_thermal_pci_probe(struct pci_dev
> *pdev, const struct pci_device_
> if (ret)
> goto err_ret_thermal;
>
> - pci_info->tzone = thermal_zone_device_register("TCPU_PCI", 1,
> 1, pci_info,
> + psv_trip.temperature = get_trip_temp(pci_info);
> +
> + pci_info->tzone =
> thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
> + 1, 1,
> pci_info,
> &tzone_ops,
> &tzone_params
> , 0, 0);
> if (IS_ERR(pci_info->tzone)) {
Hi Srinivas,
On 18/01/2023 20:09, srinivas pandruvada wrote:
> On Wed, 2023-01-18 at 19:16 +0100, Daniel Lezcano wrote:
>> 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 ops content logic into generic trip points and register them
>> with the
>> thermal zone.
>>
> In this scheme is the assumption is that trip point temperature never
> changes? If firmware updated the trip temperature, what needs to be
> done?
I'm a bit confused about the situation where the firmware can change the
trip point in the back of the OSPM.
Does the firmware send a notification about the trip change? Or does it
assume the OSPM will be reading the trip point while monitoring/polling
the thermal zone ?
Is the question for this particular driver?
If the trip point is changed by the userspace (via sysfs),
thermal_zone_set_trip() is used which in turn changes the thermal trip
temperature directly in the generic structure and then calls the back
set_trip_temp.
--
<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
Hi Daniel,
On Mon, 2023-01-23 at 19:02 +0100, Daniel Lezcano wrote:
>
> Hi Srinivas,
>
>
> On 18/01/2023 20:09, srinivas pandruvada wrote:
> > On Wed, 2023-01-18 at 19:16 +0100, Daniel Lezcano wrote:
> > > 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 ops content logic into generic trip points and register
> > > them
> > > with the
> > > thermal zone.
> > >
> > In this scheme is the assumption is that trip point temperature
> > never
> > changes? If firmware updated the trip temperature, what needs to be
> > done?
>
> I'm a bit confused about the situation where the firmware can change
> the
> trip point in the back of the OSPM.
>
> Does the firmware send a notification about the trip change? Or does
> it
> assume the OSPM will be reading the trip point while
> monitoring/polling
> the thermal zone ?
Firmware sends an ACPI notification. For example INT3403.
https://elixir.bootlin.com/linux/latest/C/ident/INT3403_PERF_TRIP_POINT_CHANGED
>
> Is the question for this particular driver?
This PCH driver trips are not changed by firmware hence we don't have
to worry about here.
Thanks,
Srinivas
>
> If the trip point is changed by the userspace (via sysfs),
> thermal_zone_set_trip() is used which in turn changes the thermal
> trip
> temperature directly in the generic structure and then calls the back
> set_trip_temp.
>
On Wed, Jan 18, 2023 at 7:16 PM Daniel Lezcano
<[email protected]> wrote:
>
> 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 ops content logic into generic trip points and register them with the
> thermal zone.
>
> Signed-off-by: Daniel Lezcano <[email protected]>
> ---
> .../processor_thermal_device_pci.c | 53 ++++++++-----------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> index bf1b1cdfade4..c7d50862bf56 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> @@ -144,34 +144,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
> return 0;
> }
>
> -static int sys_get_trip_temp(struct thermal_zone_device *tzd,
> - int trip, int *temp)
> -{
> - struct proc_thermal_pci *pci_info = tzd->devdata;
> - u32 _temp;
> -
> - proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &_temp);
> - if (!_temp) {
> - *temp = THERMAL_TEMP_INVALID;
> - } else {
> - int tjmax;
> -
> - proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
> - _temp = tjmax - _temp;
> - *temp = (unsigned long)_temp * 1000;
> - }
> -
> - return 0;
> -}
> -
> -static int sys_get_trip_type(struct thermal_zone_device *tzd, int trip,
> - enum thermal_trip_type *type)
> -{
> - *type = THERMAL_TRIP_PASSIVE;
> -
> - return 0;
> -}
> -
> static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
> {
> struct proc_thermal_pci *pci_info = tzd->devdata;
> @@ -200,10 +172,26 @@ static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp
> return 0;
> }
>
> +static int get_trip_temp(struct proc_thermal_pci *pci_info)
> +{
> + int temp, tjmax;
> +
> + proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_THRES_0, &temp);
> + if (!temp)
> + return THERMAL_TEMP_INVALID;
> +
> + proc_thermal_mmio_read(pci_info, PROC_THERMAL_MMIO_TJMAX, &tjmax);
> + temp = (tjmax - temp) * 1000;
> +
> + return temp;
> +}
> +
> +static struct thermal_trip psv_trip = {
> + .type = THERMAL_TRIP_PASSIVE,
> +};
> +
> static struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> - .get_trip_temp = sys_get_trip_temp,
> - .get_trip_type = sys_get_trip_type,
> .set_trip_temp = sys_set_trip_temp,
> };
>
> @@ -251,7 +239,10 @@ static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
> if (ret)
> goto err_ret_thermal;
>
> - pci_info->tzone = thermal_zone_device_register("TCPU_PCI", 1, 1, pci_info,
> + psv_trip.temperature = get_trip_temp(pci_info);
> +
> + pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
> + 1, 1, pci_info,
> &tzone_ops,
> &tzone_params, 0, 0);
> if (IS_ERR(pci_info->tzone)) {
> --
Applied as 6.3 material with edits in the subject and changelog, thanks!