Re: [PATCH v5 07/18] thermal: exynos: Modify exynos thermal code to use device tree for cpu cooling configuration

[Eduardo Valentin <edubezval@gmail.com>]

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

On Mon, Jan 19, 2015 at 12:20:51PM +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.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> ---
> Changes for v2:
> - None
> Changes for v3:
> - Rewrite code responsible for registering CPU cooling device to not depend
>   on explicit "/cpus/cpu@0" path since now Exynos SoCs use new cpu node
>   names (e.g. cpu@A00). New approach iterates over "cpus" node children.
> - Patch title changed to thermal: exynos
> Changes for v4:
> - None
> Changes for v5:
> - None
> 
> ---
>  drivers/cpufreq/exynos-cpufreq.c                |  30 +++++-
>  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, 101 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
> index f99a0b0..32bc64d 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 *cpus, *np;
>  	int ret = -EINVAL;
>  
>  	exynos_info = kzalloc(sizeof(*exynos_info), GFP_KERNEL);
> @@ -198,9 +202,31 @@ 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;
>  
> +	cpus = of_find_node_by_path("/cpus");
> +	if (!cpus) {
> +		pr_err("failed to find cpus node\n");
> +		return -ENOENT;
> +	}
> +
> +	for (np = of_get_next_child(cpus, NULL); np;
> +	     of_node_put(np), np = of_get_next_child(cpus, np)) {
> +		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));
> +			break;
> +		}
> +	}
> +	of_node_put(np);
> +	of_node_put(cpus);
> +
> +	return 0;
> + err:
>  	dev_err(&pdev->dev, "failed to register cpufreq driver\n");
>  	regulator_put(arm_regulator);
>  err_vdd_arm:

You need at least an ack from a cpufreq maintainer to get this patch in.
I would prefer if you split the cpufreq part from the thermal part. It
avoids merge conflicts in the upstreaming process.

> 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);
> +		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 5000727..ae30f6a 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 7f880d2..627dec9 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -83,9 +83,6 @@ enum soc_type {
>   * @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.
>   */
> @@ -111,8 +108,6 @@ struct exynos_tmu_platform_data {
>  	enum soc_type type;
>  	u32 cal_type;
>  	u32 cal_mode;
> -	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
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]