Re: [PATCH V6 18/30] thermal: exynos: Add support to handle manyinstances of TMU

From: Eduardo Valentin
Date: Wed Jun 19 2013 - 19:28:13 EST

Next message: Eduardo Valentin: "Re: [PATCH V6 19/30] thermal: exynos: Add TMU features to check insteadof using SOC type"
Previous message: Seth Heasley: "[PATCH 1/4] ata_piix: IDE-mode SATA patch for Intel Coleto Creek DeviceIDs"
In reply to: Amit Daniel Kachhap: "[PATCH V6 18/30] thermal: exynos: Add support to handle many instances of TMU"
Next in thread: amit daniel kachhap: "Re: [PATCH V6 18/30] thermal: exynos: Add support to handle manyinstances of TMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 17-06-2013 02:46, Amit Daniel Kachhap wrote:
> This patch adds support to handle multiple instances of the TMU controllers.
> This is done by removing the static structure to register with the core thermal
> and creating it dynamically for each instance of the TMU controller. The
> interrupt is made shared type to handle shared interrupts. Also
> the identifier of the TMU controller is extracted from device tree alias.
>
> Acked-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
> Acked-by: Jonghwa Lee <jonghwa3.lee@xxxxxxxxxxx>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@xxxxxxxxxxx>
> ---
> drivers/thermal/samsung/exynos_thermal_common.h | 1 +
> drivers/thermal/samsung/exynos_tmu.c | 147 ++++++++++++++++-------
> drivers/thermal/samsung/exynos_tmu.h | 13 ++
> drivers/thermal/samsung/exynos_tmu_data.c | 145 ++++++++++++----------
> drivers/thermal/samsung/exynos_tmu_data.h | 4 +-
> 5 files changed, 197 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
> index dd0077e..0c189d6 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.h
> +++ b/drivers/thermal/samsung/exynos_thermal_common.h
> @@ -84,6 +84,7 @@ struct thermal_sensor_conf {
> struct thermal_cooling_conf cooling_data;
> void *driver_data;
> void *pzone_data;
> + struct device *dev;
> };
>
> /*Functions used exynos based thermal sensor driver*/
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 4356118..1880c4e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -26,15 +26,32 @@
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> #include <linux/platform_device.h>
>
> #include "exynos_thermal_common.h"
> #include "exynos_tmu.h"
> #include "exynos_tmu_data.h"
>
> +/**
> + * struct exynos_tmu_data : A structure to hold the private data of the TMU
> + driver
> + * @id: identifier of the one instance of the TMU controller.
> + * @pdata: pointer to the tmu platform/configuration data
> + * @base: base address of the single instance of the TMU controller.
> + * @irq: irq number of the TMU controller.
> + * @soc: id of the SOC type.
> + * @irq_work: pointer to the irq work structure.
> + * @lock: lock to implement synchronization.
> + * @clk: pointer to the clock structure.
> + * @temp_error1: fused value of the first point trim.
> + * @temp_error2: fused value of the second point trim.
> + * @reg_conf: pointer to structure to register with core thermal.
> + */
> struct exynos_tmu_data {
> + int id;
> struct exynos_tmu_platform_data *pdata;
> - struct resource *mem;
> void __iomem *base;
> int irq;
> enum soc_type soc;
> @@ -42,6 +59,7 @@ struct exynos_tmu_data {
> struct mutex lock;
> struct clk *clk;
> u8 temp_error1, temp_error2;
> + struct thermal_sensor_conf *reg_conf;
> };
>
> /*
> @@ -345,12 +363,6 @@ static int exynos_tmu_set_emulation(void *drv_data, unsigned long temp)
> { return -EINVAL; }
> #endif/*CONFIG_THERMAL_EMULATION*/
>
> -static struct thermal_sensor_conf exynos_sensor_conf = {
> - .name = "exynos-therm",
> - .read_temperature = (int (*)(void *))exynos_tmu_read,
> - .write_emul_temp = exynos_tmu_set_emulation,
> -};
> -
> static void exynos_tmu_work(struct work_struct *work)
> {
> struct exynos_tmu_data *data = container_of(work,
> @@ -359,7 +371,7 @@ static void exynos_tmu_work(struct work_struct *work)
> const struct exynos_tmu_registers *reg = pdata->registers;
> unsigned int val_irq;
>
> - exynos_report_trigger(&exynos_sensor_conf);
> + exynos_report_trigger(data->reg_conf);
> mutex_lock(&data->lock);
> clk_enable(data->clk);
>
> @@ -404,33 +416,73 @@ MODULE_DEVICE_TABLE(of, exynos_tmu_match);
> #endif
>
> static inline struct exynos_tmu_platform_data *exynos_get_driver_data(
> - struct platform_device *pdev)
> + struct platform_device *pdev, int id)
> {
> + struct exynos_tmu_init_data *data_table;
> + struct exynos_tmu_platform_data *tmu_data;
> #ifdef CONFIG_OF
> if (pdev->dev.of_node) {
> const struct of_device_id *match;
> match = of_match_node(exynos_tmu_match, pdev->dev.of_node);
> if (!match)
> return NULL;
> - return (struct exynos_tmu_platform_data *) match->data;
> + data_table = (struct exynos_tmu_init_data *) match->data;
> + if (!data_table || id >= data_table->tmu_count)
> + return NULL;
> + tmu_data = data_table->tmu_data;
> + return (struct exynos_tmu_platform_data *) (tmu_data + id);
> }
> #endif
> return NULL;
> }
>
> -static int exynos_tmu_probe(struct platform_device *pdev)
> +static int exynos_map_dt_data(struct platform_device *pdev)
> {
> - struct exynos_tmu_data *data;
> - struct exynos_tmu_platform_data *pdata = pdev->dev.platform_data;
> - int ret, i;
> + struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> + struct exynos_tmu_platform_data *pdata;
> + struct resource res;
>
> - if (!pdata)
> - pdata = exynos_get_driver_data(pdev);
> + if (!data)
> + return -ENODEV;
> +
> + data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
> + if (data->id < 0)
> + data->id = 0;
>
> + data->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + if (data->irq <= 0) {
> + dev_err(&pdev->dev, "failed to get IRQ\n");
> + return -ENODEV;
> + }
> +
> + if (of_address_to_resource(pdev->dev.of_node, 0, &res)) {
> + dev_err(&pdev->dev, "failed to get Resource 0\n");
> + return -ENODEV;
> + }
> +
> + data->base = devm_ioremap(&pdev->dev, res.start, resource_size(&res));
> + if (!data->base) {
> + dev_err(&pdev->dev, "Failed to ioremap memory\n");
> + return -EADDRNOTAVAIL;
> + }
> +
> + pdata = exynos_get_driver_data(pdev, data->id);
> if (!pdata) {
> dev_err(&pdev->dev, "No platform init data supplied.\n");
> return -ENODEV;
> }
> + data->pdata = pdata;
> +
> + return 0;
> +}
> +
> +static int exynos_tmu_probe(struct platform_device *pdev)
> +{
> + struct exynos_tmu_data *data;
> + struct exynos_tmu_platform_data *pdata;
> + struct thermal_sensor_conf *sensor_conf;
> + int ret, i;
> +
> data = devm_kzalloc(&pdev->dev, sizeof(struct exynos_tmu_data),
> GFP_KERNEL);
> if (!data) {
> @@ -438,21 +490,18 @@ static int exynos_tmu_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - data->irq = platform_get_irq(pdev, 0);
> - if (data->irq < 0) {
> - dev_err(&pdev->dev, "Failed to get platform irq\n");
> - return data->irq;
> - }
> + platform_set_drvdata(pdev, data);
> + mutex_init(&data->lock);
>
> - INIT_WORK(&data->irq_work, exynos_tmu_work);
> + ret = exynos_map_dt_data(pdev);
> + if (ret)
> + return ret;
>
> - data->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->base = devm_ioremap_resource(&pdev->dev, data->mem);
> - if (IS_ERR(data->base))
> - return PTR_ERR(data->base);
> + pdata = data->pdata;
>
> + INIT_WORK(&data->irq_work, exynos_tmu_work);
> ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq,
> - IRQF_TRIGGER_RISING, "exynos-tmu", data);
> + IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);

Lets think about this poing: You request the IRQ before data->reg_conf
is allocated (below). Does it mean, from this point you can get called
to the IRQ handler even if reg_conf is not set? What happens then? I see
your irqhandler/worker use it.

> if (ret) {
> dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
> return ret;
> @@ -477,10 +526,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> - data->pdata = pdata;
> - platform_set_drvdata(pdev, data);
> - mutex_init(&data->lock);
> -
> ret = exynos_tmu_initialize(pdev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to initialize TMU\n");
> @@ -489,31 +534,43 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>
> exynos_tmu_control(pdev, true);
>
> - /* Register the sensor with thermal management interface */
> - (&exynos_sensor_conf)->driver_data = data;
> - exynos_sensor_conf.trip_data.trip_count = pdata->trigger_enable[0] +
> + /* Allocate a structure to register with the exynos core thermal */
> + sensor_conf = devm_kzalloc(&pdev->dev,
> + sizeof(struct thermal_sensor_conf), GFP_KERNEL);
> + if (!sensor_conf) {
> + dev_err(&pdev->dev, "Failed to allocate registration struct\n");
> + ret = -ENOMEM;
> + goto err_clk;
> + }
> + data->reg_conf = sensor_conf;
> + sprintf(sensor_conf->name, "therm_zone%d", data->id);
> + sensor_conf->read_temperature = (int (*)(void *))exynos_tmu_read;
> + sensor_conf->write_emul_temp =
> + (int (*)(void *, unsigned long))exynos_tmu_set_emulation;
> + sensor_conf->driver_data = data;
> + sensor_conf->trip_data.trip_count = pdata->trigger_enable[0] +
> pdata->trigger_enable[1] + pdata->trigger_enable[2]+
> pdata->trigger_enable[3];
>
> - for (i = 0; i < exynos_sensor_conf.trip_data.trip_count; i++) {
> - exynos_sensor_conf.trip_data.trip_val[i] =
> + for (i = 0; i < sensor_conf->trip_data.trip_count; i++) {
> + sensor_conf->trip_data.trip_val[i] =
> pdata->threshold + pdata->trigger_levels[i];
> - exynos_sensor_conf.trip_data.trip_type[i] =
> + sensor_conf->trip_data.trip_type[i] =
> pdata->trigger_type[i];
> }
>
> - exynos_sensor_conf.trip_data.trigger_falling = pdata->threshold_falling;
> + sensor_conf->trip_data.trigger_falling = pdata->threshold_falling;
>
> - exynos_sensor_conf.cooling_data.freq_clip_count =
> - pdata->freq_tab_count;
> + sensor_conf->cooling_data.freq_clip_count = pdata->freq_tab_count;
> for (i = 0; i < pdata->freq_tab_count; i++) {
> - exynos_sensor_conf.cooling_data.freq_data[i].freq_clip_max =
> + sensor_conf->cooling_data.freq_data[i].freq_clip_max =
> pdata->freq_tab[i].freq_clip_max;
> - exynos_sensor_conf.cooling_data.freq_data[i].temp_level =
> + sensor_conf->cooling_data.freq_data[i].temp_level =
> pdata->freq_tab[i].temp_level;
> }
> -
> - ret = exynos_register_thermal(&exynos_sensor_conf);
> + sensor_conf->dev = &pdev->dev;
> + /* Register the sensor with thermal management interface */
> + ret = exynos_register_thermal(sensor_conf);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register thermal interface\n");
> goto err_clk;
> @@ -531,7 +588,7 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>
> exynos_tmu_control(pdev, false);
>
> - exynos_unregister_thermal(&exynos_sensor_conf);
> + exynos_unregister_thermal(data->reg_conf);
>
> clk_unprepare(data->clk);
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> index 619a34c..b614407 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -248,4 +248,17 @@ struct exynos_tmu_platform_data {
> unsigned int freq_tab_count;
> const struct exynos_tmu_registers *registers;
> };
> +
> +/**
> + * struct exynos_tmu_init_data
> + * @tmu_count: number of TMU instances.
> + * @tmu_data: platform data of all TMU instances.
> + * This structure is required to store data for multi-instance exynos tmu
> + * driver.
> + */
> +struct exynos_tmu_init_data {
> + int tmu_count;
> + struct exynos_tmu_platform_data tmu_data[];
> +};
> +
> #endif /* _EXYNOS_TMU_H */
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> index 7fcf183..06ecfb0 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -48,38 +48,44 @@ static const struct exynos_tmu_registers exynos4210_tmu_registers = {
> .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
> .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
> };
> -struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
> - .threshold = 80,
> - .trigger_levels[0] = 5,
> - .trigger_levels[1] = 20,
> - .trigger_levels[2] = 30,
> - .trigger_enable[0] = 1,
> - .trigger_enable[1] = 1,
> - .trigger_enable[2] = 1,
> - .trigger_enable[3] = 0,
> - .trigger_type[0] = THROTTLE_ACTIVE,
> - .trigger_type[1] = THROTTLE_ACTIVE,
> - .trigger_type[2] = SW_TRIP,
> - .max_trigger_level = 4,
> - .gain = 15,
> - .reference_voltage = 7,
> - .cal_type = TYPE_ONE_POINT_TRIMMING,
> - .min_efuse_value = 40,
> - .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 = 100,
> +
> +struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
> + .tmu_data = {
> + {
> + .threshold = 80,
> + .trigger_levels[0] = 5,
> + .trigger_levels[1] = 20,
> + .trigger_levels[2] = 30,
> + .trigger_enable[0] = 1,
> + .trigger_enable[1] = 1,
> + .trigger_enable[2] = 1,
> + .trigger_enable[3] = 0,
> + .trigger_type[0] = THROTTLE_ACTIVE,
> + .trigger_type[1] = THROTTLE_ACTIVE,
> + .trigger_type[2] = SW_TRIP,
> + .max_trigger_level = 4,
> + .gain = 15,
> + .reference_voltage = 7,
> + .cal_type = TYPE_ONE_POINT_TRIMMING,
> + .min_efuse_value = 40,
> + .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 = 100,
> + },
> + .freq_tab_count = 2,
> + .type = SOC_ARCH_EXYNOS4210,
> + .registers = &exynos4210_tmu_registers,
> + },
> },
> - .freq_tab_count = 2,
> - .type = SOC_ARCH_EXYNOS4210,
> - .registers = &exynos4210_tmu_registers,
> + .tmu_count = 1,
> };
> #endif
>
> @@ -120,41 +126,48 @@ static const struct exynos_tmu_registers exynos5250_tmu_registers = {
> .emul_time_shift = EXYNOS_EMUL_TIME_SHIFT,
> .emul_time_mask = EXYNOS_EMUL_TIME_MASK,
> };
> -struct exynos_tmu_platform_data const exynos5250_default_tmu_data = {
> - .threshold_falling = 10,
> - .trigger_levels[0] = 85,
> - .trigger_levels[1] = 103,
> - .trigger_levels[2] = 110,
> - .trigger_levels[3] = 120,
> - .trigger_enable[0] = 1,
> - .trigger_enable[1] = 1,
> - .trigger_enable[2] = 1,
> - .trigger_enable[3] = 0,
> - .trigger_type[0] = THROTTLE_ACTIVE,
> - .trigger_type[1] = THROTTLE_ACTIVE,
> - .trigger_type[2] = SW_TRIP,
> - .trigger_type[3] = HW_TRIP,
> - .max_trigger_level = 4,
> - .gain = 8,
> - .reference_voltage = 16,
> - .noise_cancel_mode = 4,
> - .cal_type = TYPE_ONE_POINT_TRIMMING,
> - .efuse_value = 55,
> - .min_efuse_value = 40,
> - .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,
> - .type = SOC_ARCH_EXYNOS,
> +
> +#define EXYNOS5250_TMU_DATA \
> + .threshold_falling = 10, \
> + .trigger_levels[0] = 85, \
> + .trigger_levels[1] = 103, \
> + .trigger_levels[2] = 110, \
> + .trigger_levels[3] = 120, \
> + .trigger_enable[0] = 1, \
> + .trigger_enable[1] = 1, \
> + .trigger_enable[2] = 1, \
> + .trigger_enable[3] = 0, \
> + .trigger_type[0] = THROTTLE_ACTIVE, \
> + .trigger_type[1] = THROTTLE_ACTIVE, \
> + .trigger_type[2] = SW_TRIP, \
> + .trigger_type[3] = HW_TRIP, \
> + .max_trigger_level = 4, \
> + .gain = 8, \
> + .reference_voltage = 16, \
> + .noise_cancel_mode = 4, \
> + .cal_type = TYPE_ONE_POINT_TRIMMING, \
> + .efuse_value = 55, \
> + .min_efuse_value = 40, \
> + .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, \
> + .type = SOC_ARCH_EXYNOS, \
> .registers = &exynos5250_tmu_registers,
> +
> +struct exynos_tmu_init_data const exynos5250_default_tmu_data = {
> + .tmu_data = {
> + { EXYNOS5250_TMU_DATA },
> + },
> + .tmu_count = 1,
> };
> #endif
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> index 4acf070..139dbbb 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -94,14 +94,14 @@
> #define EXYNOS_MAX_TRIGGER_PER_REG 4
>
> #if defined(CONFIG_CPU_EXYNOS4210)
> -extern struct exynos_tmu_platform_data const exynos4210_default_tmu_data;
> +extern struct exynos_tmu_init_data const exynos4210_default_tmu_data;
> #define EXYNOS4210_TMU_DRV_DATA (&exynos4210_default_tmu_data)
> #else
> #define EXYNOS4210_TMU_DRV_DATA (NULL)
> #endif
>
> #if (defined(CONFIG_SOC_EXYNOS5250) || defined(CONFIG_SOC_EXYNOS4412))
> -extern struct exynos_tmu_platform_data const exynos5250_default_tmu_data;
> +extern struct exynos_tmu_init_data const exynos5250_default_tmu_data;
> #define EXYNOS5250_TMU_DRV_DATA (&exynos5250_default_tmu_data)
> #else
> #define EXYNOS5250_TMU_DRV_DATA (NULL)
>

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

Eduardo Valentin

Attachment: signature.asc
Description: OpenPGP digital signature

Next message: Eduardo Valentin: "Re: [PATCH V6 19/30] thermal: exynos: Add TMU features to check insteadof using SOC type"
Previous message: Seth Heasley: "[PATCH 1/4] ata_piix: IDE-mode SATA patch for Intel Coleto Creek DeviceIDs"
In reply to: Amit Daniel Kachhap: "[PATCH V6 18/30] thermal: exynos: Add support to handle many instances of TMU"
Next in thread: amit daniel kachhap: "Re: [PATCH V6 18/30] thermal: exynos: Add support to handle manyinstances of TMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]