lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 17 May 2013 21:17:40 +0900
From:	jonghwa3.lee@...sung.com
To:	Amit Daniel Kachhap <amit.daniel@...sung.com>
Cc:	linux-pm@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	amit.kachhap@...il.com, Kukjin Kim <kgene.kim@...sung.com>,
	Eduardo Valentin <eduardo.valentin@...com>
Subject: Re: [PATCH V4 16/30] thermal: exynos: Make the zone handling dependent
 on trip count

On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:

> This code simplifies the zone handling to use the trip count passed
> by the TMU driver. This also helps in adding more zone support.
> 
> Acked-by: Kukjin Kim <kgene.kim@...sung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@...sung.com>
> ---
>  drivers/thermal/samsung/exynos_thermal_common.c |   55 ++++++++++++-----------
>  drivers/thermal/samsung/exynos_thermal_common.h |    2 -
>  2 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
> index 86d39aa..2369417 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -78,17 +78,16 @@ static int exynos_set_mode(struct thermal_zone_device *thermal,
>  static int exynos_get_trip_type(struct thermal_zone_device *thermal, int trip,
>  				 enum thermal_trip_type *type)
>  {
> -	switch (GET_ZONE(trip)) {
> -	case MONITOR_ZONE:
> -	case WARN_ZONE:
> -		*type = THERMAL_TRIP_ACTIVE;
> -		break;
> -	case PANIC_ZONE:
> -		*type = THERMAL_TRIP_CRITICAL;
> -		break;
> -	default:
> +	struct exynos_thermal_zone *th_zone = thermal->devdata;
> +	int max_trip = th_zone->sensor_conf->trip_data.trip_count;
> +
> +	if (trip < 0 || trip >= max_trip)
>  		return -EINVAL;
> -	}
> +	else if (trip == (max_trip - 1))
> +		*type = THERMAL_TRIP_CRITICAL;
> +	else
> +		*type = THERMAL_TRIP_ACTIVE;
> +


 In current exynos_thermal driver, it is hard to set various trip type for each
trip, especially passive type. (not impossible, but complicated)

What do you think we just keep trip information with trip temperature in private
data? I mean if we just make trip level information to driver's private data
(like exynos_thermal_zone), it might be helpful to control trip type and trip
temperature. Like this,

struct exynos_thermal_trip_info {
	int trip_temp;
	enum thermal_trip_type type;
};
struct exynos_thermal_device {	
	.
	struct exynos_thermal_trip_info *trips;
	int num_trips;
};

Thanks,
Jonghwa

>  	return 0;
>  }
>  
> @@ -97,8 +96,9 @@ static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>  				unsigned long *temp)
>  {
>  	struct exynos_thermal_zone *th_zone = thermal->devdata;
> +	int max_trip = th_zone->sensor_conf->trip_data.trip_count;
>  
> -	if (trip < GET_TRIP(MONITOR_ZONE) || trip > GET_TRIP(PANIC_ZONE))
> +	if (trip < 0 || trip >= max_trip)
>  		return -EINVAL;
>  
>  	*temp = th_zone->sensor_conf->trip_data.trip_val[trip];
> @@ -112,10 +112,10 @@ static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>  static int exynos_get_crit_temp(struct thermal_zone_device *thermal,
>  				unsigned long *temp)
>  {
> -	int ret;
> -	/* Panic zone */
> -	ret = exynos_get_trip_temp(thermal, GET_TRIP(PANIC_ZONE), temp);
> -	return ret;
> +	struct exynos_thermal_zone *th_zone = thermal->devdata;
> +	int max_trip = th_zone->sensor_conf->trip_data.trip_count;
> +	/* Get the temp of highest trip*/
> +	return exynos_get_trip_temp(thermal, max_trip - 1, temp);
>  }
>  
>  /* Bind callback functions for thermal zone */
> @@ -340,19 +340,22 @@ int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
>  		return -ENOMEM;
>  
>  	th_zone->sensor_conf = sensor_conf;
> -	cpumask_set_cpu(0, &mask_val);
> -	th_zone->cool_dev[0] = cpufreq_cooling_register(&mask_val);
> -	if (IS_ERR(th_zone->cool_dev[0])) {
> -		pr_err("Failed to register cpufreq cooling device\n");
> -		ret = -EINVAL;
> -		goto err_unregister;
> +	if (sensor_conf->cooling_data.freq_clip_count > 0) {
> +		cpumask_set_cpu(0, &mask_val);
> +		th_zone->cool_dev[0] = cpufreq_cooling_register(&mask_val);
> +		if (IS_ERR(th_zone->cool_dev[0])) {
> +			pr_err("Failed to register cpufreq cooling device\n");
> +			ret = -EINVAL;
> +			goto err_unregister;
> +		}
> +		th_zone->cool_dev_size++;
>  	}
> -	th_zone->cool_dev_size++;
>  
> -	th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
> -			EXYNOS_ZONE_COUNT, 0, th_zone, &exynos_dev_ops, NULL, 0,
> -			sensor_conf->trip_data.trigger_falling ?
> -			0 : IDLE_INTERVAL);
> +	th_zone->therm_dev = thermal_zone_device_register(
> +			sensor_conf->name, sensor_conf->trip_data.trip_count,
> +			0, th_zone, &exynos_dev_ops, NULL, 0,
> +			sensor_conf->trip_data.trigger_falling ? 0 :
> +			IDLE_INTERVAL);
>  
>  	if (IS_ERR(th_zone->therm_dev)) {
>  		pr_err("Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
> index 59138ae..7b938e1 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.h
> +++ b/drivers/thermal/samsung/exynos_thermal_common.h
> @@ -42,8 +42,6 @@
>  #define GET_ZONE(trip) (trip + 2)
>  #define GET_TRIP(zone) (zone - 2)
>  
> -#define EXYNOS_ZONE_COUNT	3
> -
>  /**
>   * struct freq_clip_table
>   * @freq_clip_max: maximum frequency allowed for this cooling state.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ