Re: [PATCH V6 16/30] thermal: exynos: Make the zone handling use trip information

[Eduardo Valentin <eduardo.valentin@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 6283 bytes --]

On 17-06-2013 02:46, Amit Daniel Kachhap wrote:
> This code simplifies the zone handling to use the trip information passed
> by the TMU driver and not the hardcoded macros. This also helps in adding
> more zone support.
> 
> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
> Acked-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_thermal_common.c |   61 +++++++++++++----------
>  drivers/thermal/samsung/exynos_thermal_common.h |    3 +-
>  drivers/thermal/samsung/exynos_tmu.c            |    5 ++-
>  3 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
> index 86d39aa..2873ca3 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -78,17 +78,22 @@ 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;
> +	int trip_type;
> +
> +	if (trip < 0 || trip >= max_trip)
>  		return -EINVAL;
> -	}
> +
> +	trip_type = th_zone->sensor_conf->trip_data.trip_type[trip];
> +
> +	if (trip_type == SW_TRIP)
> +		*type = THERMAL_TRIP_CRITICAL;
> +	else if (trip_type == THROTTLE_ACTIVE)
> +		*type = THERMAL_TRIP_ACTIVE;
> +	else if (trip_type == THROTTLE_PASSIVE)
> +		*type = THERMAL_TRIP_PASSIVE;
> +
>  	return 0;
>  }
>  
> @@ -97,8 +102,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 +118,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 +346,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);

Did you mean th_zone->cool_dev[th_zone->cool_dev_size] ?

I think the logic behind the allocation of these cpufreq cooling devices
needs to be revisited. You always assigned to cool_dev[0], but you
always iterate the array until cool_dev_size. Remember that
cool_dev_size now is per th_zone, not global.

> +		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 1e9a326..dd0077e 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
> -
>  enum trigger_type {
>  	THROTTLE_ACTIVE = 1,
>  	THROTTLE_PASSIVE,
> @@ -68,6 +66,7 @@ struct freq_clip_table {
>  };
>  struct	thermal_trip_point_conf {
>  	int trip_val[MAX_TRIP_COUNT];
> +	int trip_type[MAX_TRIP_COUNT];
>  	int trip_count;
>  	unsigned char trigger_falling;
>  };
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 40e0cfd..acbd295 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -509,9 +509,12 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>  			pdata->trigger_enable[1] + pdata->trigger_enable[2]+
>  			pdata->trigger_enable[3];
>  
> -	for (i = 0; i < exynos_sensor_conf.trip_data.trip_count; i++)
> +	for (i = 0; i < exynos_sensor_conf.trip_data.trip_count; i++) {
>  		exynos_sensor_conf.trip_data.trip_val[i] =
>  			pdata->threshold + pdata->trigger_levels[i];
> +		exynos_sensor_conf.trip_data.trip_type[i] =
> +					pdata->trigger_type[i];
> +	}
>  
>  	exynos_sensor_conf.trip_data.trigger_falling = pdata->threshold_falling;
>  
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]