Re: [PATCH v2 07/17] thermal: cpu_cooling: Modify exynos thermal code to use device tree for cpu cooling configuration

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Eduardo,

> On Wed, Dec 10, 2014 at 01:09:46PM +0100, Lukasz Majewski wrote:
> > Up till now exynos_tmu_data.c was used for storing CPU cooling
> > configuration data. Now the Exynos thermal core code uses device
> > tree to get this data. For this purpose generic thermal code for
> > configuring CPU cooling was used.
> 
> Title prefix also does not help here, I would use 'thermal:
> exynos: ....'

Ok.

> 
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > ---
> > Changes for v2:
> > - None
> > ---
> >  drivers/cpufreq/exynos-cpufreq.c                |  23 ++++-
> >  drivers/thermal/samsung/exynos_thermal_common.c | 122
> > ++++++++++++++----------
> > drivers/thermal/samsung/exynos_tmu.c            |   7 --
> > drivers/thermal/samsung/exynos_tmu.h            |   5 -
> > drivers/thermal/samsung/exynos_tmu_data.c       |  42 +------- 5
> > files changed, 94 insertions(+), 105 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/exynos-cpufreq.c
> > b/drivers/cpufreq/exynos-cpufreq.c index 1e0ec57..fdedb8d 100644
> > --- a/drivers/cpufreq/exynos-cpufreq.c
> > +++ b/drivers/cpufreq/exynos-cpufreq.c
> > @@ -18,10 +18,13 @@
> >  #include <linux/cpufreq.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/of.h>
> > +#include <linux/cpu_cooling.h>
> > +#include <linux/cpu.h>
> >  
> >  #include "exynos-cpufreq.h"
> >  
> >  static struct exynos_dvfs_info *exynos_info;
> > +static struct thermal_cooling_device *cdev;
> >  static struct regulator *arm_regulator;
> >  static unsigned int locking_frequency;
> >  
> > @@ -156,6 +159,7 @@ static struct cpufreq_driver exynos_driver = {
> >  
> >  static int exynos_cpufreq_probe(struct platform_device *pdev)
> >  {
> > +	struct device_node *np;
> >  	int ret = -EINVAL;
> >  
> >  	exynos_info = kzalloc(sizeof(*exynos_info), GFP_KERNEL);
> > @@ -198,9 +202,24 @@ static int exynos_cpufreq_probe(struct
> > platform_device *pdev) /* Done here as we want to capture boot
> > frequency */ locking_frequency =
> > clk_get_rate(exynos_info->cpu_clk) / 1000; 
> > -	if (!cpufreq_register_driver(&exynos_driver))
> > -		return 0;
> > +	if (cpufreq_register_driver(&exynos_driver))
> > +		goto err;
> >  
> > +	np = of_find_node_by_path("/cpus/cpu@0");
> > +	if (!np) {
> > +		pr_err("failed to find cpu0 node\n");
> > +		return -ENOENT;
> > +	}
> > +	if (of_find_property(np, "#cooling-cells", NULL)) {
> > +		cdev = of_cpufreq_cooling_register(np,
> > cpu_present_mask);
> > +		if (IS_ERR(cdev))
> > +			pr_err("running cpufreq without cooling
> > device: %ld\n",
> > +			       PTR_ERR(cdev));
> > +	}
> > +	of_node_put(np);
> > +
> > +	return 0;
> > + err:
> >  	dev_err(&pdev->dev, "failed to register cpufreq driver\n");
> >  	regulator_put(arm_regulator);
> >  err_vdd_arm:
> > diff --git a/drivers/thermal/samsung/exynos_thermal_common.c
> > b/drivers/thermal/samsung/exynos_thermal_common.c index
> > 6dc3815..00aa688 100644 ---
> > a/drivers/thermal/samsung/exynos_thermal_common.c +++
> > b/drivers/thermal/samsung/exynos_thermal_common.c @@ -133,47
> > +133,62 @@ static int exynos_get_crit_temp(struct
> > thermal_zone_device *thermal, static int exynos_bind(struct
> > thermal_zone_device *thermal, struct thermal_cooling_device *cdev) {
> > -	int ret = 0, i, tab_size, level;
> > -	struct freq_clip_table *tab_ptr, *clip_data;
> >  	struct exynos_thermal_zone *th_zone = thermal->devdata;
> >  	struct thermal_sensor_conf *data = th_zone->sensor_conf;
> > +	struct device_node *child, *gchild, *np;
> > +	struct of_phandle_args cooling_spec;
> > +	unsigned long max, state = 0;
> > +	int ret = 0, i = 0;
> >  
> > -	tab_ptr = (struct freq_clip_table
> > *)data->cooling_data.freq_data;
> > -	tab_size = data->cooling_data.freq_clip_count;
> > -
> > -	if (tab_ptr == NULL || tab_size == 0)
> > +	/*
> > +	 * Below code is necessary to skip binding when cpufreq's
> > +	 * frequency table is not yet initialized.
> > +	 */
> > +	cdev->ops->get_max_state(cdev, &state);
> > +	if (!state && !th_zone->cool_dev_size) {
> > +		th_zone->cool_dev_size = 1;
> > +		th_zone->cool_dev[0] = cdev;
> > +		th_zone->bind = false;
> >  		return 0;
> > +	}
> >  
> > -	/* find the cooling device registered*/
> > -	for (i = 0; i < th_zone->cool_dev_size; i++)
> > -		if (cdev == th_zone->cool_dev[i])
> > -			break;
> > +	np = of_find_node_by_path("/thermal-zones/cpu-thermal");
> > +	if (!np) {
> > +		pr_err("failed to find thmerla-zones/cpu-thermal
> > node\n");
> > +		return -ENOENT;
> > +	}
> >  
> > -	/* No matching cooling device */
> > -	if (i == th_zone->cool_dev_size)
> > -		return 0;
> > +	child = of_get_child_by_name(np, "cooling-maps");
> >  
> > -	/* Bind the thermal zone to the cpufreq cooling device */
> > -	for (i = 0; i < tab_size; i++) {
> > -		clip_data = (struct freq_clip_table
> > *)&(tab_ptr[i]);
> > -		level = cpufreq_cooling_get_level(0,
> > clip_data->freq_clip_max);
> > -		if (level == THERMAL_CSTATE_INVALID)
> > -			return 0;
> > -		switch (GET_ZONE(i)) {
> > -		case MONITOR_ZONE:
> > -		case WARN_ZONE:
> > -			if
> > (thermal_zone_bind_cooling_device(thermal, i, cdev,
> > -
> > level, 0)) {
> > -				dev_err(data->dev,
> > -					"error unbinding cdev
> > inst=%d\n", i);
> > -				ret = -EINVAL;
> > -			}
> > -			th_zone->bind = true;
> > -			break;
> > -		default:
> > +	for_each_child_of_node(child, gchild) {
> > +		ret = of_parse_phandle_with_args(gchild,
> > "cooling-device",
> > +						 "#cooling-cells",
> > +						 0, &cooling_spec);
> 
> 
> hmm.. Why do we need to duplicate the same parsing found at
> of-thermal? Can you please help me understand the reasoning?

This hack is to provide thermal functionality between commits.

Please note that this file - exynos_thermal_common.c - is removed in
[PATCH 14/17], so this code is not present after applying the whole
series.

> 
> > +		if (ret < 0) {
> > +			pr_err("missing cooling_device
> > property\n");
> > +			goto end;
> > +		}
> > +
> > +		if (cooling_spec.args_count < 2) {
> >  			ret = -EINVAL;
> > +			goto end;
> >  		}
> > +
> > +		max = cooling_spec.args[0];
> > +		if (thermal_zone_bind_cooling_device(thermal, i,
> > cdev,
> > +						     max, 0)) {
> > +			dev_err(data->dev,
> > +				"thermal error unbinding cdev
> > inst=%d\n", i); +
> > +			ret = -EINVAL;
> > +			goto end;
> > +		}
> > +		i++;
> >  	}
> > +	th_zone->bind = true;
> > +end:
> > +	of_node_put(child);
> > +	of_node_put(np);
> >  
> >  	return ret;
> >  }
> > @@ -182,16 +197,12 @@ static int exynos_bind(struct
> > thermal_zone_device *thermal, static int exynos_unbind(struct
> > thermal_zone_device *thermal, struct thermal_cooling_device *cdev)
> >  {
> > -	int ret = 0, i, tab_size;
> > +	int ret = 0, i;
> >  	struct exynos_thermal_zone *th_zone = thermal->devdata;
> >  	struct thermal_sensor_conf *data = th_zone->sensor_conf;
> > +	struct device_node *child, *gchild, *np;
> >  
> > -	if (th_zone->bind == false)
> > -		return 0;
> > -
> > -	tab_size = data->cooling_data.freq_clip_count;
> > -
> > -	if (tab_size == 0)
> > +	if (th_zone->bind == false || !th_zone->cool_dev_size)
> >  		return 0;
> >  
> >  	/* find the cooling device registered*/
> > @@ -203,23 +214,30 @@ static int exynos_unbind(struct
> > thermal_zone_device *thermal, if (i == th_zone->cool_dev_size)
> >  		return 0;
> >  
> > -	/* Bind the thermal zone to the cpufreq cooling device */
> > -	for (i = 0; i < tab_size; i++) {
> > -		switch (GET_ZONE(i)) {
> > -		case MONITOR_ZONE:
> > -		case WARN_ZONE:
> > -			if
> > (thermal_zone_unbind_cooling_device(thermal, i,
> > -
> > cdev)) {
> > -				dev_err(data->dev,
> > -					"error unbinding cdev
> > inst=%d\n", i);
> > -				ret = -EINVAL;
> > -			}
> > -			th_zone->bind = false;
> > -			break;
> > -		default:
> > +	np = of_find_node_by_path("/thermal-zones/cpu-thermal");
> > +	if (!np) {
> > +		pr_err("failed to find thmerla-zones/cpu-thermal
> > node\n");
> > +		return -ENOENT;
> > +	}
> > +
> > +	child = of_get_child_by_name(np, "cooling-maps");
> > +
> > +	i = 0;
> > +	for_each_child_of_node(child, gchild) {
> > +		if (thermal_zone_unbind_cooling_device(thermal, i,
> > +						       cdev)) {
> > +			dev_err(data->dev,
> > +				"error unbinding cdev inst=%d\n",
> > i); ret = -EINVAL;
> > +			goto end;
> >  		}
> > +		i++;
> >  	}
> > +	th_zone->bind = false;
> > +end:
> > +	of_node_put(child);
> > +	of_node_put(np);
> > +
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index 936d16f..d2d6b53 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -916,13 +916,6 @@ static int exynos_tmu_probe(struct
> > platform_device *pdev) 
> >  	sensor_conf->trip_data.trigger_falling =
> > pdata->threshold_falling; 
> > -	sensor_conf->cooling_data.freq_clip_count =
> > pdata->freq_tab_count;
> > -	for (i = 0; i < pdata->freq_tab_count; i++) {
> > -
> > sensor_conf->cooling_data.freq_data[i].freq_clip_max =
> > -
> > pdata->freq_tab[i].freq_clip_max;
> > -		sensor_conf->cooling_data.freq_data[i].temp_level =
> > -
> > pdata->freq_tab[i].temp_level;
> > -	}
> >  	sensor_conf->dev = &pdev->dev;
> >  	/* Register the sensor with thermal management interface */
> >  	ret = exynos_register_thermal(sensor_conf);
> > diff --git a/drivers/thermal/samsung/exynos_tmu.h
> > b/drivers/thermal/samsung/exynos_tmu.h index 03ebdd0..fcdb2fc 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.h
> > +++ b/drivers/thermal/samsung/exynos_tmu.h
> > @@ -73,9 +73,6 @@
> >   * @second_point_trim: temp value of the second point trimming
> >   * @default_temp_offset: default temperature offset in case of no
> > trimming
> >   * @cal_type: calibration type for temperature
> > - * @freq_clip_table: Table representing frequency reduction
> > percentage.
> > - * @freq_tab_count: Count of the above table as frequency
> > reduction may
> > - *	applicable to only some of the trigger levels.
> >   *
> >   * This structure is required for configuration of exynos_tmu
> > driver. */
> > @@ -101,8 +98,6 @@ struct exynos_tmu_platform_data {
> >  	u32 cal_type;
> >  	u32 cal_mode;
> >  	u32 type;
> > -	struct freq_clip_table freq_tab[4];
> > -	unsigned int freq_tab_count;
> >  };
> >  
> >  /**
> > diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> > b/drivers/thermal/samsung/exynos_tmu_data.c index b239100..a993f3d
> > 100644 --- a/drivers/thermal/samsung/exynos_tmu_data.c
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> > @@ -47,15 +47,6 @@ struct exynos_tmu_init_data const
> > exynos4210_default_tmu_data = { .first_point_trim = 25,
> >  		.second_point_trim = 85,
> >  		.default_temp_offset = 50,
> > -		.freq_tab[0] = {
> > -			.freq_clip_max = 800 * 1000,
> > -			.temp_level = 85,
> > -			},
> > -		.freq_tab[1] = {
> > -			.freq_clip_max = 200 * 1000,
> > -			.temp_level = 100,
> > -		},
> > -		.freq_tab_count = 2,
> >  		.type = SOC_ARCH_EXYNOS4210,
> >  		},
> >  	},
> > @@ -87,16 +78,7 @@ struct exynos_tmu_init_data const
> > exynos4210_default_tmu_data = { .max_efuse_value = 100, \
> >  	.first_point_trim = 25, \
> >  	.second_point_trim = 85, \
> > -	.default_temp_offset = 50, \
> > -	.freq_tab[0] = { \
> > -		.freq_clip_max = 800 * 1000, \
> > -		.temp_level = 70, \
> > -	}, \
> > -	.freq_tab[1] = { \
> > -		.freq_clip_max = 400 * 1000, \
> > -		.temp_level = 95, \
> > -	}, \
> > -	.freq_tab_count = 2
> > +	.default_temp_offset = 50
> >  
> >  struct exynos_tmu_init_data const exynos3250_default_tmu_data = {
> >  	.tmu_data = {
> > @@ -133,16 +115,7 @@ struct exynos_tmu_init_data const
> > exynos3250_default_tmu_data = { .max_efuse_value = 100, \
> >  	.first_point_trim = 25, \
> >  	.second_point_trim = 85, \
> > -	.default_temp_offset = 50, \
> > -	.freq_tab[0] = { \
> > -		.freq_clip_max = 1400 * 1000, \
> > -		.temp_level = 70, \
> > -	}, \
> > -	.freq_tab[1] = { \
> > -		.freq_clip_max = 400 * 1000, \
> > -		.temp_level = 95, \
> > -	}, \
> > -	.freq_tab_count = 2
> > +	.default_temp_offset = 50
> >  
> >  struct exynos_tmu_init_data const exynos4412_default_tmu_data = {
> >  	.tmu_data = {
> > @@ -189,16 +162,7 @@ struct exynos_tmu_init_data const
> > exynos5250_default_tmu_data = { .max_efuse_value = 100, \
> >  	.first_point_trim = 25, \
> >  	.second_point_trim = 85, \
> > -	.default_temp_offset = 50, \
> > -	.freq_tab[0] = { \
> > -		.freq_clip_max = 800 * 1000, \
> > -		.temp_level = 85, \
> > -	}, \
> > -	.freq_tab[1] = { \
> > -		.freq_clip_max = 200 * 1000, \
> > -		.temp_level = 103, \
> > -	}, \
> > -	.freq_tab_count = 2, \
> > +	.default_temp_offset = 50,
> >  
> >  #define EXYNOS5260_TMU_DATA \
> >  	__EXYNOS5260_TMU_DATA \
> > -- 
> > 2.0.0.rc2
> > 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group