diff mbox

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

Message ID 1421666462-7606-8-git-send-email-l.majewski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lukasz Majewski Jan. 19, 2015, 11:20 a.m. UTC
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(-)

Comments

Eduardo Valentin Jan. 21, 2015, 2:18 a.m. UTC | #1
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
>
Lukasz Majewski Jan. 21, 2015, 8:21 a.m. UTC | #2
Hi Eduardo,

> 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.

Please note that exynos-cpufreq part has around 30 LOC (which only
prevents from a regression). 

When I split it, then if by some mishap this not find its way to
mainline in the same time as thermal, then we would have pretty nasty
regression.

I will ask Viresh to look into this code and NAK/ACK it.

> 
> > 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
> >
Lukasz Majewski Jan. 21, 2015, 8:33 a.m. UTC | #3
Hi Viresh,

> 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:

Viresh, the above is a small part of the cpufreq related code, which is
necessary for Exynos thermal rework to use device tree.

It is NOT adding any new functionality, but preserves possibility to
use cpufreq as a colling device.

Normally I would exclude this part from this commit, but then I cannot
assure that between commits no regression is slipping in.

> 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 \
Viresh Kumar Jan. 21, 2015, 9:08 a.m. UTC | #4
On 21 January 2015 at 14:03, Lukasz Majewski <l.majewski@samsung.com> wrote:
>> diff --git a/drivers/cpufreq/exynos-cpufreq.c

>>  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))

You should return the error returned by cpufreq_register_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)) {

Shouldn't you be checking this just for cpu 0 ?

>> +                     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:
>
> Viresh, the above is a small part of the cpufreq related code, which is
> necessary for Exynos thermal rework to use device tree.
>
> It is NOT adding any new functionality, but preserves possibility to
> use cpufreq as a colling device.
>
> Normally I would exclude this part from this commit, but then I cannot
> assure that between commits no regression is slipping in.

Mostly looks fine. Just few nits.

But another important thing is to split this patch, so that there is a separate
patch for cpufreq driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Majewski Jan. 21, 2015, 9:47 a.m. UTC | #5
Hi Viresh,

> On 21 January 2015 at 14:03, Lukasz Majewski <l.majewski@samsung.com>
> wrote:
> >> diff --git a/drivers/cpufreq/exynos-cpufreq.c
> 
> >>  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))
> 
> You should return the error returned by cpufreq_register_driver().

OK.

> 
> >> +             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)) {
> 
> Shouldn't you be checking this just for cpu 0 ?

In previous versions I've only checked for cpu 0.

If you think that it is enough to explicitly check only for cpu 0 and
forget about above "fail safe" code (when. e.g. CPU3 has defined
cooling-cells), then I'm fine with it.

> 
> >> +                     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:
> >
> > Viresh, the above is a small part of the cpufreq related code,
> > which is necessary for Exynos thermal rework to use device tree.
> >
> > It is NOT adding any new functionality, but preserves possibility to
> > use cpufreq as a colling device.
> >
> > Normally I would exclude this part from this commit, but then I
> > cannot assure that between commits no regression is slipping in.
> 
> Mostly looks fine. Just few nits.
> 
> But another important thing is to split this patch, so that there is
> a separate patch for cpufreq driver.

As I've mention - it would be maintainer's call if one trades potential
regression for patch separation.

Thanks for a prompt reply.
Viresh Kumar Jan. 21, 2015, 9:55 a.m. UTC | #6
On 21 January 2015 at 15:17, Lukasz Majewski <l.majewski@samsung.com> wrote:
> In previous versions I've only checked for cpu 0.
>
> If you think that it is enough to explicitly check only for cpu 0 and
> forget about above "fail safe" code (when. e.g. CPU3 has defined
> cooling-cells), then I'm fine with it.

I don't know what bindings are you following, but cpufreq-dt's bindings
say that it has to be present in cpu0. Anyway, this driver isn't for a
multi-cluster system and so cpu0 should be fine.

> As I've mention - it would be maintainer's call if one trades potential
> regression for patch separation.

I am just asking it to split into a separate patch, not that I will
get it through
cpufreq. Eduardo can take it, but it should be a separate patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Majewski Jan. 21, 2015, 10:09 a.m. UTC | #7
Hi Viresh,

> On 21 January 2015 at 15:17, Lukasz Majewski <l.majewski@samsung.com>
> wrote:
> > In previous versions I've only checked for cpu 0.
> >
> > If you think that it is enough to explicitly check only for cpu 0
> > and forget about above "fail safe" code (when. e.g. CPU3 has defined
> > cooling-cells), then I'm fine with it.
> 
> I don't know what bindings are you following, but cpufreq-dt's
> bindings say that it has to be present in cpu0. Anyway, this driver
> isn't for a multi-cluster system and so cpu0 should be fine.

Ok.

> 
> > As I've mention - it would be maintainer's call if one trades
> > potential regression for patch separation.
> 
> I am just asking it to split into a separate patch, not that I will
> get it through
> cpufreq. Eduardo can take it, but it should be a separate patch.

Lets do it in this way :-).

Thanks.
diff mbox

Patch

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:
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 \