[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e8cf3273-0a2d-4749-a427-95111363d2c3@linaro.org>
Date: Fri, 16 Sep 2022 19:02:51 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>,
"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
<linux-arm-kernel@...ts.infradead.org>
Cc: rafael@...nel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH v3 22/30] thermal/drivers/imx: Use generic
thermal_zone_get_trip() function
Hi,
On 06/09/2022 18:47, 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.
Any comment on this patch?
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> drivers/thermal/imx_thermal.c | 72 +++++++++++++----------------------
> 1 file changed, 27 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 16663373b682..fb0d5cab70af 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -76,7 +76,6 @@
> enum imx_thermal_trip {
> IMX_TRIP_PASSIVE,
> IMX_TRIP_CRITICAL,
> - IMX_TRIP_NUM,
> };
>
> #define IMX_POLLING_DELAY 2000 /* millisecond */
> @@ -115,6 +114,11 @@ struct thermal_soc_data {
> u32 low_alarm_shift;
> };
>
> +static struct thermal_trip trips[] = {
> + [IMX_TRIP_PASSIVE] = { .type = THERMAL_TRIP_PASSIVE },
> + [IMX_TRIP_CRITICAL] = { .type = THERMAL_TRIP_CRITICAL },
> +};
> +
> static struct thermal_soc_data thermal_imx6q_data = {
> .version = TEMPMON_IMX6Q,
>
> @@ -201,8 +205,6 @@ struct imx_thermal_data {
> struct thermal_cooling_device *cdev;
> struct regmap *tempmon;
> u32 c1, c2; /* See formula in imx_init_calib() */
> - int temp_passive;
> - int temp_critical;
> int temp_max;
> int alarm_temp;
> int last_temp;
> @@ -279,12 +281,12 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>
> /* Update alarm value to next higher trip point for TEMPMON_IMX6Q */
> if (data->socdata->version == TEMPMON_IMX6Q) {
> - if (data->alarm_temp == data->temp_passive &&
> - *temp >= data->temp_passive)
> - imx_set_alarm_temp(data, data->temp_critical);
> - if (data->alarm_temp == data->temp_critical &&
> - *temp < data->temp_passive) {
> - imx_set_alarm_temp(data, data->temp_passive);
> + if (data->alarm_temp == trips[IMX_TRIP_PASSIVE].temperature &&
> + *temp >= trips[IMX_TRIP_PASSIVE].temperature)
> + imx_set_alarm_temp(data, trips[IMX_TRIP_CRITICAL].temperature);
> + if (data->alarm_temp == trips[IMX_TRIP_CRITICAL].temperature &&
> + *temp < trips[IMX_TRIP_PASSIVE].temperature) {
> + imx_set_alarm_temp(data, trips[IMX_TRIP_PASSIVE].temperature);
> dev_dbg(&tz->device, "thermal alarm off: T < %d\n",
> data->alarm_temp / 1000);
> }
> @@ -330,29 +332,10 @@ static int imx_change_mode(struct thermal_zone_device *tz,
> return 0;
> }
>
> -static int imx_get_trip_type(struct thermal_zone_device *tz, int trip,
> - enum thermal_trip_type *type)
> -{
> - *type = (trip == IMX_TRIP_PASSIVE) ? THERMAL_TRIP_PASSIVE :
> - THERMAL_TRIP_CRITICAL;
> - return 0;
> -}
> -
> static int imx_get_crit_temp(struct thermal_zone_device *tz, int *temp)
> {
> - struct imx_thermal_data *data = tz->devdata;
> -
> - *temp = data->temp_critical;
> - return 0;
> -}
> -
> -static int imx_get_trip_temp(struct thermal_zone_device *tz, int trip,
> - int *temp)
> -{
> - struct imx_thermal_data *data = tz->devdata;
> + *temp = trips[IMX_TRIP_CRITICAL].temperature;
>
> - *temp = (trip == IMX_TRIP_PASSIVE) ? data->temp_passive :
> - data->temp_critical;
> return 0;
> }
>
> @@ -371,10 +354,10 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> return -EPERM;
>
> /* do not allow passive to be set higher than critical */
> - if (temp < 0 || temp > data->temp_critical)
> + if (temp < 0 || temp > trips[IMX_TRIP_CRITICAL].temperature)
> return -EINVAL;
>
> - data->temp_passive = temp;
> + trips[IMX_TRIP_PASSIVE].temperature = temp;
>
> imx_set_alarm_temp(data, temp);
>
> @@ -423,8 +406,6 @@ static struct thermal_zone_device_ops imx_tz_ops = {
> .unbind = imx_unbind,
> .get_temp = imx_get_temp,
> .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,
> .set_trip_temp = imx_set_trip_temp,
> };
> @@ -507,8 +488,8 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
> * Set the critical trip point at 5 °C under max
> * Set the passive trip point at 10 °C under max (changeable via sysfs)
> */
> - data->temp_critical = data->temp_max - (1000 * 5);
> - data->temp_passive = data->temp_max - (1000 * 10);
> + trips[IMX_TRIP_PASSIVE].temperature = data->temp_max - (1000 * 10);
> + trips[IMX_TRIP_CRITICAL].temperature = data->temp_max - (1000 * 5);
> }
>
> static int imx_init_from_tempmon_data(struct platform_device *pdev)
> @@ -743,12 +724,13 @@ static int imx_thermal_probe(struct platform_device *pdev)
> goto legacy_cleanup;
> }
>
> - data->tz = thermal_zone_device_register("imx_thermal_zone",
> - IMX_TRIP_NUM,
> - BIT(IMX_TRIP_PASSIVE), data,
> - &imx_tz_ops, NULL,
> - IMX_PASSIVE_DELAY,
> - IMX_POLLING_DELAY);
> + data->tz = thermal_zone_device_register_with_trips("imx_thermal_zone",
> + trips,
> + ARRAY_SIZE(trips),
> + BIT(IMX_TRIP_PASSIVE), data,
> + &imx_tz_ops, NULL,
> + IMX_PASSIVE_DELAY,
> + IMX_POLLING_DELAY);
> if (IS_ERR(data->tz)) {
> ret = PTR_ERR(data->tz);
> dev_err(&pdev->dev,
> @@ -758,8 +740,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
>
> dev_info(&pdev->dev, "%s CPU temperature grade - max:%dC"
> " critical:%dC passive:%dC\n", data->temp_grade,
> - data->temp_max / 1000, data->temp_critical / 1000,
> - data->temp_passive / 1000);
> + data->temp_max / 1000, trips[IMX_TRIP_CRITICAL].temperature / 1000,
> + trips[IMX_TRIP_PASSIVE].temperature / 1000);
>
> /* Enable measurements at ~ 10 Hz */
> regmap_write(map, data->socdata->measure_freq_ctrl + REG_CLR,
> @@ -767,10 +749,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
> measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
> regmap_write(map, data->socdata->measure_freq_ctrl + REG_SET,
> measure_freq << data->socdata->measure_freq_shift);
> - imx_set_alarm_temp(data, data->temp_passive);
> + imx_set_alarm_temp(data, trips[IMX_TRIP_PASSIVE].temperature);
>
> if (data->socdata->version == TEMPMON_IMX6SX)
> - imx_set_panic_temp(data, data->temp_critical);
> + imx_set_panic_temp(data, trips[IMX_TRIP_CRITICAL].temperature);
>
> regmap_write(map, data->socdata->sensor_ctrl + REG_CLR,
> data->socdata->power_down_mask);
--
<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
Powered by blists - more mailing lists