[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-id: <50922E59.6080000@samsung.com>
Date: Thu, 01 Nov 2012 17:10:01 +0900
From: jonghwa3.lee@...sung.com
To: "R, Durgadoss" <durgadoss.r@...el.com>
Cc: Jonghwa Lee <jonghwa3.lee@...sung.com>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Brown, Len" <len.brown@...el.com>,
"Rafael J. Wysocki" <rjw@...k.pl>,
Amit Dinel Kachhap <amit.kachhap@...aro.org>
Subject: Re: [PATCH v3] Thermal: exynos: Add sysfs node supporting exynos's
emulation mode.
Hi,
On 2012년 11월 01일 16:56, R, Durgadoss wrote:
> Hi,
>
>> +++ b/Documentation/thermal/exynos_thermal_emulation
>> @@ -0,0 +1,49 @@
>> +EXYNOS EMULATION MODE
>> +========================
>> +
>> +Copyright (C) 2012 Samsung Electronics
>> +
>> +Writen by Jonghwa Lee <jonghwa3.lee@...sung.com>
> s/Writen/Written
Oops, Sorry ^^;
>> +
>> +Description
>> +-----------
>> +
>> +Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
>> management unit.
>> +Thermal emulation mode supports software debug for TMU's operation.
>> User can set temperature
>> +manually with software code and TMU will read current temperature from
>> user value not from
>> +sensor's value.
>> +
>> +Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support
>> in available.
>> +When it's enabled, sysfs node will be created under
>> +/sys/bus/platform/devices/'exynos device name'/ with name of
>> 'emulation'.
>> +
>> +The sysfs node, 'emulation', will contain value 0 for the initial state. When
>> you input any
>> +temperature you want to update to sysfs node, it automatically enable
>> emulation mode and
>> +current temperature will be changed into it.
>> +(Exynos also supports user changable delay time which would be used to
>> delay of
>> + changing temperature. However, this node only uses same delay of real
>> sensing time, 938us.)
>> +
>> +Disabling emulation mode only requires writing value 0 to sysfs node.
> This documentation looks fine. As a reply to one of the comments in the
> previous version, you gave me an explanation. Could you please add that
> explanation here as well ? I think that would help all of us, after this
> patch set gets merged..
Okay, I'll add that comments to the next patch.
>> +
>> +
>> +TEMP 120 |
>> + |
>> + 100 |
>> + |
>> + 80 |
>> + | +-----------
>> + 60 | | |
>> + | +-------------| |
>> + 40 | | | |
>> + | | | |
>> + 20 | | | +----------
>> + | | | | |
>> + 0
>> |______________|_____________|__________|__________|_______
>> __
>> + A A A A TIME
>> + |<----->| |<----->| |<----->| |
>> + | 938us | | | | | |
>> +emulation : 0 50 | 70 | 20 | 0
>> +current temp : sensor 50 70 20 sensor
>> +
>> +
>> +
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index e1cb6bd..c02a66c 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -55,3 +55,12 @@ config EXYNOS_THERMAL
>> help
>> If you say yes here you get support for TMU (Thermal Managment
>> Unit) on SAMSUNG EXYNOS series of SoC.
>> +
>> +config EXYNOS_THERMAL_EMUL
>> + bool "EXYNOS TMU emulation mode support"
>> + depends on !CPU_EXYNOS4210 && EXYNOS_THERMAL
>> + help
>> + Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
>> + Enable this option will be make sysfs node in exynos thermal
>> platform
>> + device directory to support emulation mode. With emulation mode
>> sysfs
>> + node, you can manually input temperature to TMU for simulation
>> purpose.
>> diff --git a/drivers/thermal/exynos_thermal.c
>> b/drivers/thermal/exynos_thermal.c
>> index fd03e85..4bdd1eb 100644
>> --- a/drivers/thermal/exynos_thermal.c
>> +++ b/drivers/thermal/exynos_thermal.c
>> @@ -99,6 +99,14 @@
>> #define IDLE_INTERVAL 10000
>> #define MCELSIUS 1000
>>
>> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
>> +#define EXYNOS_EMUL_TIME 0x57F0
>> +#define EXYNOS_EMUL_TIME_SHIFT 16
>> +#define EXYNOS_EMUL_DATA_SHIFT 8
>> +#define EXYNOS_EMUL_DATA_MASK 0xFF
>> +#define EXYNOS_EMUL_ENABLE 0x1
>> +#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
>> +
>> /* CPU Zone information */
>> #define PANIC_ZONE 4
>> #define WARN_ZONE 3
>> @@ -832,6 +840,84 @@ static inline struct exynos_tmu_platform_data
>> *exynos_get_driver_data(
>> return (struct exynos_tmu_platform_data *)
>> platform_get_device_id(pdev)->driver_data;
>> }
>> +
>> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
>> +static ssize_t exynos_tmu_emulation_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct platform_device *pdev = container_of(dev,
>> + struct platform_device, dev);
>> + struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>> + unsigned int reg;
>> + u8 temp_code;
>> + int temp = 0;
>> +
>> + mutex_lock(&data->lock);
>> + clk_enable(data->clk);
>> + reg = readl(data->base + EXYNOS_EMUL_CON);
>> + clk_disable(data->clk);
>> + mutex_unlock(&data->lock);
>> +
>> + if (reg & EXYNOS_EMUL_ENABLE) {
>> + reg >>= EXYNOS_EMUL_DATA_SHIFT;
>> + temp_code = reg & EXYNOS_EMUL_DATA_MASK;
>> + temp = code_to_temp(data, temp_code);
>> + } else {
>> + temp = 0;
>> + }
> Do we still need the else part ?
>
No, we don't. I'll remove it.
>> +
>> + return sprintf(buf, "%d\n", temp);
>> +}
>> +
>> +static ssize_t exynos_tmu_emulation_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct platform_device *pdev = container_of(dev,
>> + struct platform_device, dev);
>> + struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>> + unsigned int reg;
>> + int temp = 0;
> Looks like we don't need to assign 0 to temp here..
Yes, you're right.
>> +
>> + if (!sscanf(buf, "%d\n", &temp) || temp < 0)
>> + return -EINVAL;
>> +
>> + mutex_lock(&data->lock);
>> + clk_enable(data->clk);
>> +
>> + reg = readl(data->base + EXYNOS_EMUL_CON);
>> +
>> + if (temp)
>> + reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT)
>> |
>> + (temp_to_code(data, temp) <<
>> EXYNOS_EMUL_DATA_SHIFT) |
>> + EXYNOS_EMUL_ENABLE;
>> + else
>> + reg &= ~EXYNOS_EMUL_ENABLE;
>> +
>> + writel(reg, data->base + EXYNOS_EMUL_CON);
>> +
>> + clk_disable(data->clk);
>> + mutex_unlock(&data->lock);
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
>> + exynos_tmu_emulation_store);
>> +static int create_emulation_sysfs(struct device *dev)
>> +{
>> + return device_create_file(dev, &dev_attr_emulation);
>> +}
>> +static void remove_emulation_sysfs(struct device *dev)
>> +{
>> + device_remove_file(dev, &dev_attr_emulation);
>> +}
>> +#else
>> +static inline int create_emulation_sysfs(struct device *dev) {return 0;}
>> +static inline void remove_emulation_sysfs(struct device *dev){}
>> +#endif
>> +
>> static int __devinit exynos_tmu_probe(struct platform_device *pdev)
>> {
>> struct exynos_tmu_data *data;
>> @@ -930,6 +1016,11 @@ static int __devinit exynos_tmu_probe(struct
>> platform_device *pdev)
>> dev_err(&pdev->dev, "Failed to register thermal
>> interface\n");
>> goto err_clk;
>> }
>> +
>> + ret = create_emulation_sysfs(&pdev->dev);
>> + if (ret)
>> + dev_err(&pdev->dev, "Faield to create emulation mode
> s/Faield/Failed
Typo again -_-;, sorry.
>> sysfs node\n");
>> +
>> return 0;
>> err_clk:
>> platform_set_drvdata(pdev, NULL);
>> @@ -941,6 +1032,8 @@ static int __devexit exynos_tmu_remove(struct
>> platform_device *pdev)
>> {
>> struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>>
>> + remove_emulation_sysfs(&pdev->dev);
>> +
> Yes, this makes it cleaner.
>
>> exynos_tmu_control(pdev, false);
>>
>> exynos_unregister_thermal();
>> --
>> 1.7.4.1
Thanks for reviewing my codes.
I'll apply your comments and re-post it as soon.
Thanks,
Jonghwa.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists