On 14-05-2013 05:58, Amit Daniel Kachhap wrote:
> This patch removes the dependency on CPU_THERMAL for compiling TMU driver.
> This is useful for cases when only TMU controller needs to be initialised
> without cpu cooling action.

Agreed with your intention. I just do not know if it makes sense to make
this change at this point. Maybe after you have split the code?

The above concern is simply from a non-functional perspective. If you do
this at this point of your series, your driver may have compilation
issues at this specific commit, in case your config does not have
CONFIG_CPU_THERMAL.

I recommend you moving this patch further in your series, to a place
where you have isolated the code that depends on CPU_THERMAL.

>
> Acked-by: Kukjin Kim <[email protected]>
> Signed-off-by: Amit Daniel Kachhap <[email protected]>
> ---
> drivers/thermal/samsung/Kconfig | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/thermal/samsung/Kconfig b/drivers/thermal/samsung/Kconfig
> index 145a55d..b5ab971 100644
> --- a/drivers/thermal/samsung/Kconfig
> +++ b/drivers/thermal/samsung/Kconfig
> @@ -4,7 +4,6 @@ config ARCH_HAS_TMU
> config EXYNOS_THERMAL
> tristate "Temperature sensor on Samsung EXYNOS"
> depends on ARCH_HAS_TMU
> - depends on CPU_THERMAL
> help
> If you say yes here you get support for TMU (Thermal Management
> Unit) on SAMSUNG EXYNOS series of SoC. This helps in registering
>

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

Eduardo Valentin

Attachments:

signature.asc (295.00 B)
OpenPGP digital signature

2013-05-31 15:29:22

On 04-06-2013 00:44, amit daniel kachhap wrote:
> Hi Jonghwa,
>
> Sorry for the late reply as I was on leave.
>
> On Sat, May 18, 2013 at 10:53 AM, <[email protected]> wrote:
>> On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:
>>
>>> This patch modifies TMU controller to add changes needed to work with
>>> exynos5440 platform. This sensor registers 3 instance of the tmu controller
>>> with the thermal zone and hence reports 3 temperature output. This controller
>>> supports upto five trip points. For critical threshold the driver uses the
>>> core driver thermal framework for shutdown.
>>>
>>> Acked-by: Kukjin Kim <[email protected]>
>>> Signed-off-by: Amit Daniel Kachhap <[email protected]>
>>> ---
>>> .../devicetree/bindings/thermal/exynos-thermal.txt | 28 ++++++++++++-
>>> drivers/thermal/samsung/exynos_tmu.c | 43 +++++++++++++++++--
>>> drivers/thermal/samsung/exynos_tmu.h | 6 +++
>>> drivers/thermal/samsung/exynos_tmu_data.h | 2 +
>>> 4 files changed, 72 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>> index 535fd0e..970eeba 100644
>>> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>> @@ -6,13 +6,16 @@
>>> "samsung,exynos4412-tmu"
>>> "samsung,exynos4210-tmu"
>>> "samsung,exynos5250-tmu"
>>> + "samsung,exynos5440-tmu"
>>> - interrupt-parent : The phandle for the interrupt controller
>>> -- reg : Address range of the thermal registers
>>> +- reg : Address range of the thermal registers. For exynos5440-tmu which has 3
>>> + instances of TMU, 2 set of register has to supplied. First set belongs
>>> + to each instance of TMU and second set belongs to common TMU registers.
>>> - interrupts : Should contain interrupt for thermal system
>>> - clocks : The main clock for TMU device
>>> - clock-names : Thermal system clock name
>>>
>>> -Example:
>>> +Example 1):
>>>
>>> tmu@100C0000 {
>>> compatible = "samsung,exynos4412-tmu";
>>> @@ -23,3 +26,24 @@ Example:
>>> clock-names = "tmu_apbif";
>>> status = "disabled";
>>> };
>>> +
>>> +Example 2):
>>> +
>>> + tmuctrl_0: tmuctrl@160118 {
>>> + compatible = "samsung,exynos5440-tmu";
>>> + reg = <0x160118 0x230>, <0x160368 0x10>;
>>> + interrupts = <0 58 0>;
>>> + clocks = <&clock 21>;
>>> + clock-names = "tmu_apbif";
>>> + };
>>> +
>>> +Note: For multi-instance tmu each instance should have an alias correctly
>>> +numbered in "aliases" node.
>>> +
>>> +Example:
>>> +
>>> +aliases {
>>> + tmuctrl0 = &tmuctrl_0;
>>> + tmuctrl1 = &tmuctrl_1;
>>> + tmuctrl2 = &tmuctrl_2;
>>> +};
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>> index 7f7b1cf..7ca9c4d 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>> @@ -185,9 +185,11 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>> reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>>>
>>> writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>>> - } else if (data->soc == SOC_ARCH_EXYNOS) {
>>> + } else if (data->soc == SOC_ARCH_EXYNOS ||
>>> + data->soc == SOC_ARCH_EXYNOS5440) {
>>> /* Write temperature code for rising and falling threshold */
>>> - for (i = 0; i < trigger_levs; i++) {
>>> + for (i = 0;
>>> + i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>>> threshold_code = temp_to_code(data,
>>> pdata->trigger_levels[i]);
>>> if (threshold_code < 0) {
>>> @@ -218,7 +220,30 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>> writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>>> (reg->inten_fall_mask << reg->inten_fall_shift),
>>> data->base + reg->tmu_intclear);
>>> +
>>> + /* if 5th threshold limit is also present, use TH2 register */
>>> + i = EXYNOS_MAX_TRIGGER_PER_REG;
>>> + if (pdata->trigger_levels[i]) {
>>> + threshold_code = temp_to_code(data,
>>> + pdata->trigger_levels[i]);
>>> + if (threshold_code < 0) {
>>> + ret = threshold_code;
>>> + goto out;
>>> + }
>>> + rising_threshold =
>>> + threshold_code << reg->threshold_th3_l0_shift;
>>> + writel(rising_threshold,
>>> + data->base + reg->threshold_th2);
>>> + if (pdata->trigger_type[i] == HW_TRIP) {
>>> + con = readl(data->base + reg->tmu_ctrl);
>>> + con |= (1 << reg->therm_trip_en_shift);
>>> + writel(con, data->base + reg->tmu_ctrl);
>>> + }
>>> + }
>>> }
>>> + /*Clear the PMIN in the common TMU register*/
>>> + if (reg->tmu_pmin && !data->id)
>>> + writel(0, data->base_common + reg->tmu_pmin);
>>> out:
>>> clk_disable(data->clk);
>>> mutex_unlock(&data->lock);
>>> @@ -345,7 +370,14 @@ static void exynos_tmu_work(struct work_struct *work)
>>> struct exynos_tmu_data, irq_work);
>>> struct exynos_tmu_platform_data *pdata = data->pdata;
>>> const struct exynos_tmu_registers *reg = pdata->registers;
>>> - unsigned int val_irq;
>>> + unsigned int val_irq, val_type;
>>> +
>>> + /* Find which sensor generated this interrupt */
>>> + if (reg->tmu_irqstatus) {
>>> + val_type = readl(data->base_common + reg->tmu_irqstatus);
>>> + if (!((val_type >> data->id) & 0x1))
>>> + goto out;
>>
>>
>> I have a question about your implementation for supporting EXYNOS5440.
>> I don't know exactly how EXYNO5440's tmu is working, but just guess it would be
>> similar with other EXYNOS series's without number of thermal sensors. (exclusive
>> register map and threshold level). Due to the multiple number of thermal sensor
>> in EXYNOS5440, it have multiple thermal zone devices and that's why it just
>> leave interrupt pin in pending if interrupt is not its, right?
> Yes in 5440 the interrupt line is shared so pending bit is left uncleared.
>>
>> So, my curious is, why we make all platform devices for each of thermal zone
>> devices? Why don't you just handle all thermal zone devices with one platform
>> device?
> Your doubt is genuine. Let me justify my design decision.
> Initially I also thought of making a single platform device but since
> there are 3 different TMU controllers and register maps for 4 more so
> I followed this design as the driver looks clean and can be scalable
> easily. Also I agree that some resources like IRQ line is shared but
> it is due to h/w limitation.
> Also it is easy to cleanly control each TMU instance with the device
> tree data similar to I2C/SPI/MMC instance based device driver. Say I
> do not want to use 2nd sensor then just pass device tree data for 1st
> and 3rd sensor.
>>
>> Yes, It's probably right to make multiple devices node to support them, because
>> it has different physical hardware(sensors). But we have one TMU , don't we?
>> (Maybe my assumption is wrong, I assume that it has one TMU because it looks
>> like it has only one irq line.). If I'm right, I think it is better to manage
>> all thermal zone devices with one platform device. Then, we don't need to leave
>> irq handler with leaving it pendded like above and also we may not need other
>> your patches like adding base_common iomem variable.
> Agreed that base_common variables is extra and present to handle the
> common part. I will further analyse your suggestion.
>

What is the relation TMU <--> temperature sensor? Is it one to one or
one to many?

> Thanks,
> Amit Daniel
>>
>> I'd like to listen your opinion about this.
>>
>> Thanks,
>> Jonghwa
>>
>>> + }
>>>
>>> exynos_report_trigger(data->reg_conf);
>>> mutex_lock(&data->lock);
>>> @@ -358,7 +390,7 @@ static void exynos_tmu_work(struct work_struct *work)
>>>
>>> clk_disable(data->clk);
>>> mutex_unlock(&data->lock);
>>> -
>>> +out:
>>> enable_irq(data->irq);
>>> }
>>>
>>> @@ -520,7 +552,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>> return ret;
>>>
>>> if (pdata->type == SOC_ARCH_EXYNOS ||
>>> - pdata->type == SOC_ARCH_EXYNOS4210)
>>> + pdata->type == SOC_ARCH_EXYNOS4210 ||
>>> + pdata->type == SOC_ARCH_EXYNOS5440)
>>> data->soc = pdata->type;
>>> else {
>>> ret = -EINVAL;
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
>>> index 65443d7..9151a30 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.h
>>> +++ b/drivers/thermal/samsung/exynos_tmu.h
>>> @@ -44,6 +44,7 @@ enum trigger_type {
>>> enum soc_type {
>>> SOC_ARCH_EXYNOS4210 = 1,
>>> SOC_ARCH_EXYNOS,
>>> + SOC_ARCH_EXYNOS5440,
>>> };
>>>
>>> /**
>>> @@ -132,6 +133,8 @@ enum soc_type {
>>> * @emul_temp_shift: shift bits of emulation temperature.
>>> * @emul_time_shift: shift bits of emulation time.
>>> * @emul_time_mask: mask bits of emulation time.
>>> + * @tmu_irqstatus: register to find which TMU generated interrupts.
>>> + * @tmu_pmin: register to get/set the Pmin value.
>>> */
>>> struct exynos_tmu_registers {
>>> u32 triminfo_data;
>>> @@ -199,6 +202,9 @@ struct exynos_tmu_registers {
>>> u32 emul_temp_shift;
>>> u32 emul_time_shift;
>>> u32 emul_time_mask;
>>> +
>>> + u32 tmu_irqstatus;
>>> + u32 tmu_pmin;
>>> };
>>>
>>> /**
>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
>>> index 0e2244f..4acf070 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu_data.h
>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
>>> @@ -91,6 +91,8 @@
>>> #define EXYNOS_EMUL_DATA_MASK 0xFF
>>> #define EXYNOS_EMUL_ENABLE 0x1
>>>
>>> +#define EXYNOS_MAX_TRIGGER_PER_REG 4
>>> +
>>> #if defined(CONFIG_CPU_EXYNOS4210)
>>> extern struct exynos_tmu_platform_data const exynos4210_default_tmu_data;
>>> #define EXYNOS4210_TMU_DRV_DATA (&exynos4210_default_tmu_data)
>>
>>
>
>

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

Eduardo Valentin

Attachments:

signature.asc (295.00 B)
OpenPGP digital signature

2013-06-04 12:57:26

by Eduardo Valentin

[permalink] [raw]

Subject: Re: [PATCH V4 00/30] thermal: exynos: Add thermal driver for exynos5440

On 04-06-2013 00:55, amit daniel kachhap wrote:
> Hi Eduardo,
>
> On Wed, May 15, 2013 at 8:14 PM, Eduardo Valentin
> <[email protected]> wrote:
>> On 14-05-2013 05:58, Amit Daniel Kachhap wrote:
>>> Changes in V4:
>>> Almost all the changes in this version is as per suggestion from Eduardo.The
>>> major ones are listed below,
>>> * Added kconfig symbol ARCH_HAS_TMU which needs to be enabled by platform. With
>>> this change existing symbol EXYNOS_TMU_DATA is not needed.
>>
>> I was more thinking if we could have a single flag that we could use in
>> thermal drivers. Besides, my proposal for ARCH_HAS_BANDGAP is still not
>> merged yet.
>
> yes a single flag might be useful such as ARCH_HAS_THERMAL but this
> patch can work as an intermediate solution.

I think bandgap is a generic enough term IMO.

[1] - http://en.wikipedia.org/wiki/Bandgap_voltage_reference
[2] - http://en.wikipedia.org/wiki/Silicon_bandgap_temperature_sensor

>
> Thanks,
> Amit Daniel
>>
>>> * Movement of freq_clip_table from exynos_tmu.h to exynos_thermal_common.h is
>>> explained in the commit logs.
>>> * Wrote all register description documentation.
>>> * Split 5440 TMU support patch into controller change, configuration data and
>>> feature addition patches.
>>> * Remove all *LINUX_* in the header files.
>>> * Still regulator enable is kept optional but a TODO: comment is added to fix
>>> it later.
>>>
>>> Changes in V3:
>>> * Added proper dependency of different exynos thermal Kconfig symbols. Basically 3
>>> Kconfig can be enabled now and corresponds to tmu driver. exynos common part
>>> and exynos configuration data. This issue was raised by Rui Zhang.
>>>
>>> Changes in V2:
>>> * Separated SOC data from TMU driver. This is as per suggestion from Eduardo.
>>> * Merged the new file created for exynos5440 TMU controller with the existing
>>> TMU controller code.
>>> * Removed the DT parsing code as now the SOC specific data are cleanly put
>>> inside the data specific file.
>>> * Even the register definations/bitfields are treated as data as there is
>>> some variation across SOC's.
>>>
>>> This patchset adds TMU(Thermal management Unit) driver support for
>>> exynos5440 platform. There are 3 instances of the TMU controllers so
>>> necessary cleanup/re-structure is done to handle multiple thermal zone.
>>>
>>> Patch (exynos4: Add documentation for Exynos SoC thermal bindings) from
>>> Lukasz Majewski is already posted to mainline. Adding it here for completeness.
>>> (http://www.mail-archive.com/[email protected]/msg17817.html)
>>>
>>> Patch (thermal: exynos: Support thermal tripping ) from Jonghwan Choi is
>>> added here with some changes.
>>> (https://patchwork.kernel.org/patch/1668371/)
>>>
>>> Patch (thermal: exynos: Support for TMU regulator defined at device tree)
>>> is a repost of my earlier patch(https://patchwork-mail1.kernel.org/patch/2510771/)
>>> and adds regulator support.
>>>
>>> Patch (ARM: dts: Add device tree node for exynos5440 TMU controller) and
>>> patch (arm: exynos: enable ARCH_HAS_TMU) can be merged through exynos platform
>>> maintainer as this can cause merge conflict.
>>>
>>> All these patches are based on thermal maintainers git tree,
>>> git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next.
>>>
>>> Amit Daniel Kachhap (29):
>>> thermal: exynos: Moving exynos thermal files into samsung directory
>>> thermal: exynos: Add ARCH_HAS_TMU config to know the supported soc's
>>> thermal: exynos: Remove CPU_THERMAL dependency for using TMU driver
>>> thermal: exynos: Bifurcate exynos thermal common and tmu controller
>>> code
>>> thermal: exynos: Rename exynos_thermal.c to exynos_tmu.c
>>> thermal: exynos: Move exynos_thermal.h from include/* to driver/*
>>> folder
>>> thermal: exynos: Bifurcate exynos tmu driver and configuration data
>>> thermal: exynos: Add missing definations and code cleanup
>>> thermal: exynos: Add extra entries in the tmu platform data
>>> thermal: exynos: Support thermal tripping
>>> thermal: exynos: Move register definitions from driver file to data
>>> file
>>> thermal: exynos: Fix to clear only the generated interrupts
>>> thermal: exynos: Add support for instance based register/unregister
>>> thermal: exynos: Modify private_data to appropriate name driver_data
>>> thermal: exynos: Return success even if no cooling data supplied
>>> thermal: exynos: Make the zone handling dependent on trip count
>>> thermal: exynos: Add support to handle many instances of TMU
>>> thermal: exynos: Add TMU features to check instead of using SOC type
>>> thermal: exynos: use device resource management infrastructure
>>> thermal: exynos: Add support to access common register for
>>> multistance
>>> thermal: exynos: Add support for exynos5440 TMU sensor.
>>> thermal: exynos: Fix to set the second point correction value
>>> properly
>>> thermal: exynos: Add thermal configuration data for exynos5440 TMU
>>> sensor
>>> thermal: exynos: Add a compensation logic on swapped e-fuse values
>>> thermal: exynos: Add hardware mode thermal calibration support
>>> Documentation: thermal: Explain the exynos thermal driver model
>>> thermal: exynos: Support for TMU regulator defined at device tree
>>> ARM: dts: Add device tree node for exynos5440 TMU controller
>>> arm: exynos: enable ARCH_HAS_TMU
>>>
>>> Lukasz Majewski (1):
>>> ARM: dts: thermal: exynos4: Add documentation for Exynos SoC thermal
>>> bindings
>>>
>>> .../devicetree/bindings/thermal/exynos-thermal.txt | 53 +
>>> Documentation/thermal/exynos_thermal | 43 +-
>>> arch/arm/boot/dts/exynos5440.dtsi | 30 +
>>> arch/arm/mach-exynos/Kconfig | 5 +
>>> drivers/thermal/Kconfig | 13 +-
>>> drivers/thermal/Makefile | 2 +-
>>> drivers/thermal/exynos_thermal.c | 1066 --------------------
>>> drivers/thermal/samsung/Kconfig | 21 +
>>> drivers/thermal/samsung/Makefile | 7 +
>>> drivers/thermal/samsung/exynos_thermal_common.c | 407 ++++++++
>>> drivers/thermal/samsung/exynos_thermal_common.h | 98 ++
>>> drivers/thermal/samsung/exynos_tmu.c | 741 ++++++++++++++
>>> drivers/thermal/samsung/exynos_tmu.h | 294 ++++++
>>> drivers/thermal/samsung/exynos_tmu_data.c | 225 ++++
>>> drivers/thermal/samsung/exynos_tmu_data.h | 155 +++
>>> include/linux/platform_data/exynos_thermal.h | 119 ---
>>> 16 files changed, 2076 insertions(+), 1203 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>> delete mode 100644 drivers/thermal/exynos_thermal.c
>>> create mode 100644 drivers/thermal/samsung/Kconfig
>>> create mode 100644 drivers/thermal/samsung/Makefile
>>> create mode 100644 drivers/thermal/samsung/exynos_thermal_common.c
>>> create mode 100644 drivers/thermal/samsung/exynos_thermal_common.h
>>> create mode 100644 drivers/thermal/samsung/exynos_tmu.c
>>> create mode 100644 drivers/thermal/samsung/exynos_tmu.h
>>> create mode 100644 drivers/thermal/samsung/exynos_tmu_data.c
>>> create mode 100644 drivers/thermal/samsung/exynos_tmu_data.h
>>> delete mode 100644 include/linux/platform_data/exynos_thermal.h
>>>
>>>
>>>
>>
>>
>
>

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

Eduardo Valentin

Attachments:

signature.asc (295.00 B)
OpenPGP digital signature

2013-06-04 13:01:34

On 04-06-2013 23:20, amit daniel kachhap wrote:
> Hi Eduardo,
>
> On Tue, Jun 4, 2013 at 6:25 PM, Eduardo Valentin
> <[email protected]> wrote:
>> On 04-06-2013 00:44, amit daniel kachhap wrote:
>>> Hi Jonghwa,
>>>
>>> Sorry for the late reply as I was on leave.
>>>
>>> On Sat, May 18, 2013 at 10:53 AM, <[email protected]> wrote:
>>>> On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:
>>>>
>>>>> This patch modifies TMU controller to add changes needed to work with
>>>>> exynos5440 platform. This sensor registers 3 instance of the tmu controller
>>>>> with the thermal zone and hence reports 3 temperature output. This controller
>>>>> supports upto five trip points. For critical threshold the driver uses the
>>>>> core driver thermal framework for shutdown.
>>>>>
>>>>> Acked-by: Kukjin Kim <[email protected]>
>>>>> Signed-off-by: Amit Daniel Kachhap <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/thermal/exynos-thermal.txt | 28 ++++++++++++-
>>>>> drivers/thermal/samsung/exynos_tmu.c | 43 +++++++++++++++++--
>>>>> drivers/thermal/samsung/exynos_tmu.h | 6 +++
>>>>> drivers/thermal/samsung/exynos_tmu_data.h | 2 +
>>>>> 4 files changed, 72 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>> index 535fd0e..970eeba 100644
>>>>> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>> @@ -6,13 +6,16 @@
>>>>> "samsung,exynos4412-tmu"
>>>>> "samsung,exynos4210-tmu"
>>>>> "samsung,exynos5250-tmu"
>>>>> + "samsung,exynos5440-tmu"
>>>>> - interrupt-parent : The phandle for the interrupt controller
>>>>> -- reg : Address range of the thermal registers
>>>>> +- reg : Address range of the thermal registers. For exynos5440-tmu which has 3
>>>>> + instances of TMU, 2 set of register has to supplied. First set belongs
>>>>> + to each instance of TMU and second set belongs to common TMU registers.
>>>>> - interrupts : Should contain interrupt for thermal system
>>>>> - clocks : The main clock for TMU device
>>>>> - clock-names : Thermal system clock name
>>>>>
>>>>> -Example:
>>>>> +Example 1):
>>>>>
>>>>> tmu@100C0000 {
>>>>> compatible = "samsung,exynos4412-tmu";
>>>>> @@ -23,3 +26,24 @@ Example:
>>>>> clock-names = "tmu_apbif";
>>>>> status = "disabled";
>>>>> };
>>>>> +
>>>>> +Example 2):
>>>>> +
>>>>> + tmuctrl_0: tmuctrl@160118 {
>>>>> + compatible = "samsung,exynos5440-tmu";
>>>>> + reg = <0x160118 0x230>, <0x160368 0x10>;
>>>>> + interrupts = <0 58 0>;
>>>>> + clocks = <&clock 21>;
>>>>> + clock-names = "tmu_apbif";
>>>>> + };
>>>>> +
>>>>> +Note: For multi-instance tmu each instance should have an alias correctly
>>>>> +numbered in "aliases" node.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +aliases {
>>>>> + tmuctrl0 = &tmuctrl_0;
>>>>> + tmuctrl1 = &tmuctrl_1;
>>>>> + tmuctrl2 = &tmuctrl_2;
>>>>> +};
>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>>>> index 7f7b1cf..7ca9c4d 100644
>>>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>>>> @@ -185,9 +185,11 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>>>> reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>>>>>
>>>>> writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>>>>> - } else if (data->soc == SOC_ARCH_EXYNOS) {
>>>>> + } else if (data->soc == SOC_ARCH_EXYNOS ||
>>>>> + data->soc == SOC_ARCH_EXYNOS5440) {
>>>>> /* Write temperature code for rising and falling threshold */
>>>>> - for (i = 0; i < trigger_levs; i++) {
>>>>> + for (i = 0;
>>>>> + i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>>>>> threshold_code = temp_to_code(data,
>>>>> pdata->trigger_levels[i]);
>>>>> if (threshold_code < 0) {
>>>>> @@ -218,7 +220,30 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>>>> writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>>>>> (reg->inten_fall_mask << reg->inten_fall_shift),
>>>>> data->base + reg->tmu_intclear);
>>>>> +
>>>>> + /* if 5th threshold limit is also present, use TH2 register */
>>>>> + i = EXYNOS_MAX_TRIGGER_PER_REG;
>>>>> + if (pdata->trigger_levels[i]) {
>>>>> + threshold_code = temp_to_code(data,
>>>>> + pdata->trigger_levels[i]);
>>>>> + if (threshold_code < 0) {
>>>>> + ret = threshold_code;
>>>>> + goto out;
>>>>> + }
>>>>> + rising_threshold =
>>>>> + threshold_code << reg->threshold_th3_l0_shift;
>>>>> + writel(rising_threshold,
>>>>> + data->base + reg->threshold_th2);
>>>>> + if (pdata->trigger_type[i] == HW_TRIP) {
>>>>> + con = readl(data->base + reg->tmu_ctrl);
>>>>> + con |= (1 << reg->therm_trip_en_shift);
>>>>> + writel(con, data->base + reg->tmu_ctrl);
>>>>> + }
>>>>> + }
>>>>> }
>>>>> + /*Clear the PMIN in the common TMU register*/
>>>>> + if (reg->tmu_pmin && !data->id)
>>>>> + writel(0, data->base_common + reg->tmu_pmin);
>>>>> out:
>>>>> clk_disable(data->clk);
>>>>> mutex_unlock(&data->lock);
>>>>> @@ -345,7 +370,14 @@ static void exynos_tmu_work(struct work_struct *work)
>>>>> struct exynos_tmu_data, irq_work);
>>>>> struct exynos_tmu_platform_data *pdata = data->pdata;
>>>>> const struct exynos_tmu_registers *reg = pdata->registers;
>>>>> - unsigned int val_irq;
>>>>> + unsigned int val_irq, val_type;
>>>>> +
>>>>> + /* Find which sensor generated this interrupt */
>>>>> + if (reg->tmu_irqstatus) {
>>>>> + val_type = readl(data->base_common + reg->tmu_irqstatus);
>>>>> + if (!((val_type >> data->id) & 0x1))
>>>>> + goto out;
>>>>
>>>>
>>>> I have a question about your implementation for supporting EXYNOS5440.
>>>> I don't know exactly how EXYNO5440's tmu is working, but just guess it would be
>>>> similar with other EXYNOS series's without number of thermal sensors. (exclusive
>>>> register map and threshold level). Due to the multiple number of thermal sensor
>>>> in EXYNOS5440, it have multiple thermal zone devices and that's why it just
>>>> leave interrupt pin in pending if interrupt is not its, right?
>>> Yes in 5440 the interrupt line is shared so pending bit is left uncleared.
>>>>
>>>> So, my curious is, why we make all platform devices for each of thermal zone
>>>> devices? Why don't you just handle all thermal zone devices with one platform
>>>> device?
>>> Your doubt is genuine. Let me justify my design decision.
>>> Initially I also thought of making a single platform device but since
>>> there are 3 different TMU controllers and register maps for 4 more so
>>> I followed this design as the driver looks clean and can be scalable
>>> easily. Also I agree that some resources like IRQ line is shared but
>>> it is due to h/w limitation.
>>> Also it is easy to cleanly control each TMU instance with the device
>>> tree data similar to I2C/SPI/MMC instance based device driver. Say I
>>> do not want to use 2nd sensor then just pass device tree data for 1st
>>> and 3rd sensor.
>>>>
>>>> Yes, It's probably right to make multiple devices node to support them, because
>>>> it has different physical hardware(sensors). But we have one TMU , don't we?
>>>> (Maybe my assumption is wrong, I assume that it has one TMU because it looks
>>>> like it has only one irq line.). If I'm right, I think it is better to manage
>>>> all thermal zone devices with one platform device. Then, we don't need to leave
>>>> irq handler with leaving it pendded like above and also we may not need other
>>>> your patches like adding base_common iomem variable.
>>> Agreed that base_common variables is extra and present to handle the
>>> common part. I will further analyse your suggestion.
>>>
>>
>> What is the relation TMU <--> temperature sensor? Is it one to one or
>> one to many?
> 1 TMU --- > 1 temp sensor.(one to one)
>

OK. Then it is different to TI bandgap, which has one to many relation.
Does every TMU has its own resources, like a register map and IRQ?

> Thanks,
> Amit Daniel
>>
>>> Thanks,
>>> Amit Daniel
>>>>
>>>> I'd like to listen your opinion about this.
>>>>
>>>> Thanks,
>>>> Jonghwa
>>>>
>>>>> + }
>>>>>
>>>>> exynos_report_trigger(data->reg_conf);
>>>>> mutex_lock(&data->lock);
>>>>> @@ -358,7 +390,7 @@ static void exynos_tmu_work(struct work_struct *work)
>>>>>
>>>>> clk_disable(data->clk);
>>>>> mutex_unlock(&data->lock);
>>>>> -
>>>>> +out:
>>>>> enable_irq(data->irq);
>>>>> }
>>>>>
>>>>> @@ -520,7 +552,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>>>> return ret;
>>>>>
>>>>> if (pdata->type == SOC_ARCH_EXYNOS ||
>>>>> - pdata->type == SOC_ARCH_EXYNOS4210)
>>>>> + pdata->type == SOC_ARCH_EXYNOS4210 ||
>>>>> + pdata->type == SOC_ARCH_EXYNOS5440)
>>>>> data->soc = pdata->type;
>>>>> else {
>>>>> ret = -EINVAL;
>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
>>>>> index 65443d7..9151a30 100644
>>>>> --- a/drivers/thermal/samsung/exynos_tmu.h
>>>>> +++ b/drivers/thermal/samsung/exynos_tmu.h
>>>>> @@ -44,6 +44,7 @@ enum trigger_type {
>>>>> enum soc_type {
>>>>> SOC_ARCH_EXYNOS4210 = 1,
>>>>> SOC_ARCH_EXYNOS,
>>>>> + SOC_ARCH_EXYNOS5440,
>>>>> };
>>>>>
>>>>> /**
>>>>> @@ -132,6 +133,8 @@ enum soc_type {
>>>>> * @emul_temp_shift: shift bits of emulation temperature.
>>>>> * @emul_time_shift: shift bits of emulation time.
>>>>> * @emul_time_mask: mask bits of emulation time.
>>>>> + * @tmu_irqstatus: register to find which TMU generated interrupts.
>>>>> + * @tmu_pmin: register to get/set the Pmin value.
>>>>> */
>>>>> struct exynos_tmu_registers {
>>>>> u32 triminfo_data;
>>>>> @@ -199,6 +202,9 @@ struct exynos_tmu_registers {
>>>>> u32 emul_temp_shift;
>>>>> u32 emul_time_shift;
>>>>> u32 emul_time_mask;
>>>>> +
>>>>> + u32 tmu_irqstatus;
>>>>> + u32 tmu_pmin;
>>>>> };
>>>>>
>>>>> /**
>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
>>>>> index 0e2244f..4acf070 100644
>>>>> --- a/drivers/thermal/samsung/exynos_tmu_data.h
>>>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
>>>>> @@ -91,6 +91,8 @@
>>>>> #define EXYNOS_EMUL_DATA_MASK 0xFF
>>>>> #define EXYNOS_EMUL_ENABLE 0x1
>>>>>
>>>>> +#define EXYNOS_MAX_TRIGGER_PER_REG 4
>>>>> +
>>>>> #if defined(CONFIG_CPU_EXYNOS4210)
>>>>> extern struct exynos_tmu_platform_data const exynos4210_default_tmu_data;
>>>>> #define EXYNOS4210_TMU_DRV_DATA (&exynos4210_default_tmu_data)
>>>>
>>>>
>>>
>>>
>>
>>
>> --
>> You have got to be excited about what you are doing. (L. Lamport)
>>
>> Eduardo Valentin
>>
>
>

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

Eduardo Valentin

Attachments:

signature.asc (295.00 B)
OpenPGP digital signature

2013-06-06 06:19:07

by amit daniel kachhap

[permalink] [raw]

Subject: Re: [PATCH V4 22/30] thermal: exynos: Add support for exynos5440 TMU sensor.

Hi,
On Wed, Jun 5, 2013 at 6:23 PM, Eduardo Valentin
<[email protected]> wrote:
> On 04-06-2013 23:20, amit daniel kachhap wrote:
>> Hi Eduardo,
>>
>> On Tue, Jun 4, 2013 at 6:25 PM, Eduardo Valentin
>> <[email protected]> wrote:
>>> On 04-06-2013 00:44, amit daniel kachhap wrote:
>>>> Hi Jonghwa,
>>>>
>>>> Sorry for the late reply as I was on leave.
>>>>
>>>> On Sat, May 18, 2013 at 10:53 AM, <[email protected]> wrote:
>>>>> On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:
>>>>>
>>>>>> This patch modifies TMU controller to add changes needed to work with
>>>>>> exynos5440 platform. This sensor registers 3 instance of the tmu controller
>>>>>> with the thermal zone and hence reports 3 temperature output. This controller
>>>>>> supports upto five trip points. For critical threshold the driver uses the
>>>>>> core driver thermal framework for shutdown.
>>>>>>
>>>>>> Acked-by: Kukjin Kim <[email protected]>
>>>>>> Signed-off-by: Amit Daniel Kachhap <[email protected]>
>>>>>> ---
>>>>>> .../devicetree/bindings/thermal/exynos-thermal.txt | 28 ++++++++++++-
>>>>>> drivers/thermal/samsung/exynos_tmu.c | 43 +++++++++++++++++--
>>>>>> drivers/thermal/samsung/exynos_tmu.h | 6 +++
>>>>>> drivers/thermal/samsung/exynos_tmu_data.h | 2 +
>>>>>> 4 files changed, 72 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>>> index 535fd0e..970eeba 100644
>>>>>> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>>> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>>> @@ -6,13 +6,16 @@
>>>>>> "samsung,exynos4412-tmu"
>>>>>> "samsung,exynos4210-tmu"
>>>>>> "samsung,exynos5250-tmu"
>>>>>> + "samsung,exynos5440-tmu"
>>>>>> - interrupt-parent : The phandle for the interrupt controller
>>>>>> -- reg : Address range of the thermal registers
>>>>>> +- reg : Address range of the thermal registers. For exynos5440-tmu which has 3
>>>>>> + instances of TMU, 2 set of register has to supplied. First set belongs
>>>>>> + to each instance of TMU and second set belongs to common TMU registers.
>>>>>> - interrupts : Should contain interrupt for thermal system
>>>>>> - clocks : The main clock for TMU device
>>>>>> - clock-names : Thermal system clock name
>>>>>>
>>>>>> -Example:
>>>>>> +Example 1):
>>>>>>
>>>>>> tmu@100C0000 {
>>>>>> compatible = "samsung,exynos4412-tmu";
>>>>>> @@ -23,3 +26,24 @@ Example:
>>>>>> clock-names = "tmu_apbif";
>>>>>> status = "disabled";
>>>>>> };
>>>>>> +
>>>>>> +Example 2):
>>>>>> +
>>>>>> + tmuctrl_0: tmuctrl@160118 {
>>>>>> + compatible = "samsung,exynos5440-tmu";
>>>>>> + reg = <0x160118 0x230>, <0x160368 0x10>;
>>>>>> + interrupts = <0 58 0>;
>>>>>> + clocks = <&clock 21>;
>>>>>> + clock-names = "tmu_apbif";
>>>>>> + };
>>>>>> +
>>>>>> +Note: For multi-instance tmu each instance should have an alias correctly
>>>>>> +numbered in "aliases" node.
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +aliases {
>>>>>> + tmuctrl0 = &tmuctrl_0;
>>>>>> + tmuctrl1 = &tmuctrl_1;
>>>>>> + tmuctrl2 = &tmuctrl_2;
>>>>>> +};
>>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>>>>> index 7f7b1cf..7ca9c4d 100644
>>>>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>>>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>>>>> @@ -185,9 +185,11 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>>>>> reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>>>>>>
>>>>>> writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>>>>>> - } else if (data->soc == SOC_ARCH_EXYNOS) {
>>>>>> + } else if (data->soc == SOC_ARCH_EXYNOS ||
>>>>>> + data->soc == SOC_ARCH_EXYNOS5440) {
>>>>>> /* Write temperature code for rising and falling threshold */
>>>>>> - for (i = 0; i < trigger_levs; i++) {
>>>>>> + for (i = 0;
>>>>>> + i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>>>>>> threshold_code = temp_to_code(data,
>>>>>> pdata->trigger_levels[i]);
>>>>>> if (threshold_code < 0) {
>>>>>> @@ -218,7 +220,30 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>>>>> writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>>>>>> (reg->inten_fall_mask << reg->inten_fall_shift),
>>>>>> data->base + reg->tmu_intclear);
>>>>>> +
>>>>>> + /* if 5th threshold limit is also present, use TH2 register */
>>>>>> + i = EXYNOS_MAX_TRIGGER_PER_REG;
>>>>>> + if (pdata->trigger_levels[i]) {
>>>>>> + threshold_code = temp_to_code(data,
>>>>>> + pdata->trigger_levels[i]);
>>>>>> + if (threshold_code < 0) {
>>>>>> + ret = threshold_code;
>>>>>> + goto out;
>>>>>> + }
>>>>>> + rising_threshold =
>>>>>> + threshold_code << reg->threshold_th3_l0_shift;
>>>>>> + writel(rising_threshold,
>>>>>> + data->base + reg->threshold_th2);
>>>>>> + if (pdata->trigger_type[i] == HW_TRIP) {
>>>>>> + con = readl(data->base + reg->tmu_ctrl);
>>>>>> + con |= (1 << reg->therm_trip_en_shift);
>>>>>> + writel(con, data->base + reg->tmu_ctrl);
>>>>>> + }
>>>>>> + }
>>>>>> }
>>>>>> + /*Clear the PMIN in the common TMU register*/
>>>>>> + if (reg->tmu_pmin && !data->id)
>>>>>> + writel(0, data->base_common + reg->tmu_pmin);
>>>>>> out:
>>>>>> clk_disable(data->clk);
>>>>>> mutex_unlock(&data->lock);
>>>>>> @@ -345,7 +370,14 @@ static void exynos_tmu_work(struct work_struct *work)
>>>>>> struct exynos_tmu_data, irq_work);
>>>>>> struct exynos_tmu_platform_data *pdata = data->pdata;
>>>>>> const struct exynos_tmu_registers *reg = pdata->registers;
>>>>>> - unsigned int val_irq;
>>>>>> + unsigned int val_irq, val_type;
>>>>>> +
>>>>>> + /* Find which sensor generated this interrupt */
>>>>>> + if (reg->tmu_irqstatus) {
>>>>>> + val_type = readl(data->base_common + reg->tmu_irqstatus);
>>>>>> + if (!((val_type >> data->id) & 0x1))
>>>>>> + goto out;
>>>>>
>>>>>
>>>>> I have a question about your implementation for supporting EXYNOS5440.
>>>>> I don't know exactly how EXYNO5440's tmu is working, but just guess it would be
>>>>> similar with other EXYNOS series's without number of thermal sensors. (exclusive
>>>>> register map and threshold level). Due to the multiple number of thermal sensor
>>>>> in EXYNOS5440, it have multiple thermal zone devices and that's why it just
>>>>> leave interrupt pin in pending if interrupt is not its, right?
>>>> Yes in 5440 the interrupt line is shared so pending bit is left uncleared.
>>>>>
>>>>> So, my curious is, why we make all platform devices for each of thermal zone
>>>>> devices? Why don't you just handle all thermal zone devices with one platform
>>>>> device?
>>>> Your doubt is genuine. Let me justify my design decision.
>>>> Initially I also thought of making a single platform device but since
>>>> there are 3 different TMU controllers and register maps for 4 more so
>>>> I followed this design as the driver looks clean and can be scalable
>>>> easily. Also I agree that some resources like IRQ line is shared but
>>>> it is due to h/w limitation.
>>>> Also it is easy to cleanly control each TMU instance with the device
>>>> tree data similar to I2C/SPI/MMC instance based device driver. Say I
>>>> do not want to use 2nd sensor then just pass device tree data for 1st
>>>> and 3rd sensor.
>>>>>
>>>>> Yes, It's probably right to make multiple devices node to support them, because
>>>>> it has different physical hardware(sensors). But we have one TMU , don't we?
>>>>> (Maybe my assumption is wrong, I assume that it has one TMU because it looks
>>>>> like it has only one irq line.). If I'm right, I think it is better to manage
>>>>> all thermal zone devices with one platform device. Then, we don't need to leave
>>>>> irq handler with leaving it pendded like above and also we may not need other
>>>>> your patches like adding base_common iomem variable.
>>>> Agreed that base_common variables is extra and present to handle the
>>>> common part. I will further analyse your suggestion.
>>>>
>>>
>>> What is the relation TMU <--> temperature sensor? Is it one to one or
>>> one to many?
>> 1 TMU --- > 1 temp sensor.(one to one)
>>
>
>
> OK. Then it is different to TI bandgap, which has one to many relation.
> Does every TMU has its own resources, like a register map and IRQ?
Yes register maps are different but some registers are common like ISR
related registers. IRQ line is common. However all TMU's are totally
independent devices.
>
>> Thanks,
>> Amit Daniel
>>>
>>>> Thanks,
>>>> Amit Daniel
>>>>>
>>>>> I'd like to listen your opinion about this.
>>>>>
>>>>> Thanks,
>>>>> Jonghwa
>>>>>
>>>>>> + }
>>>>>>
>>>>>> exynos_report_trigger(data->reg_conf);
>>>>>> mutex_lock(&data->lock);
>>>>>> @@ -358,7 +390,7 @@ static void exynos_tmu_work(struct work_struct *work)
>>>>>>
>>>>>> clk_disable(data->clk);
>>>>>> mutex_unlock(&data->lock);
>>>>>> -
>>>>>> +out:
>>>>>> enable_irq(data->irq);
>>>>>> }
>>>>>>
>>>>>> @@ -520,7 +552,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>>>>> return ret;
>>>>>>
>>>>>> if (pdata->type == SOC_ARCH_EXYNOS ||
>>>>>> - pdata->type == SOC_ARCH_EXYNOS4210)
>>>>>> + pdata->type == SOC_ARCH_EXYNOS4210 ||
>>>>>> + pdata->type == SOC_ARCH_EXYNOS5440)
>>>>>> data->soc = pdata->type;
>>>>>> else {
>>>>>> ret = -EINVAL;
>>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
>>>>>> index 65443d7..9151a30 100644
>>>>>> --- a/drivers/thermal/samsung/exynos_tmu.h
>>>>>> +++ b/drivers/thermal/samsung/exynos_tmu.h
>>>>>> @@ -44,6 +44,7 @@ enum trigger_type {
>>>>>> enum soc_type {
>>>>>> SOC_ARCH_EXYNOS4210 = 1,
>>>>>> SOC_ARCH_EXYNOS,
>>>>>> + SOC_ARCH_EXYNOS5440,
>>>>>> };
>>>>>>
>>>>>> /**
>>>>>> @@ -132,6 +133,8 @@ enum soc_type {
>>>>>> * @emul_temp_shift: shift bits of emulation temperature.
>>>>>> * @emul_time_shift: shift bits of emulation time.
>>>>>> * @emul_time_mask: mask bits of emulation time.
>>>>>> + * @tmu_irqstatus: register to find which TMU generated interrupts.
>>>>>> + * @tmu_pmin: register to get/set the Pmin value.
>>>>>> */
>>>>>> struct exynos_tmu_registers {
>>>>>> u32 triminfo_data;
>>>>>> @@ -199,6 +202,9 @@ struct exynos_tmu_registers {
>>>>>> u32 emul_temp_shift;
>>>>>> u32 emul_time_shift;
>>>>>> u32 emul_time_mask;
>>>>>> +
>>>>>> + u32 tmu_irqstatus;
>>>>>> + u32 tmu_pmin;
>>>>>> };
>>>>>>
>>>>>> /**
>>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
>>>>>> index 0e2244f..4acf070 100644
>>>>>> --- a/drivers/thermal/samsung/exynos_tmu_data.h
>>>>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
>>>>>> @@ -91,6 +91,8 @@
>>>>>> #define EXYNOS_EMUL_DATA_MASK 0xFF
>>>>>> #define EXYNOS_EMUL_ENABLE 0x1
>>>>>>
>>>>>> +#define EXYNOS_MAX_TRIGGER_PER_REG 4
>>>>>> +
>>>>>> #if defined(CONFIG_CPU_EXYNOS4210)
>>>>>> extern struct exynos_tmu_platform_data const exynos4210_default_tmu_data;
>>>>>> #define EXYNOS4210_TMU_DRV_DATA (&exynos4210_default_tmu_data)
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> You have got to be excited about what you are doing. (L. Lamport)
>>>
>>> Eduardo Valentin
>>>
>>
>>
>
>
> --
> You have got to be excited about what you are doing. (L. Lamport)
>
> Eduardo Valentin
>

2013-06-06 13:48:53

by Eduardo Valentin

[permalink] [raw]

Subject: Re: [PATCH V4 00/30] thermal: exynos: Add thermal driver for exynos5440

Hi,

On 04-06-2013 23:49, amit daniel kachhap wrote:
> On Tue, Jun 4, 2013 at 6:31 PM, Eduardo Valentin
> <[email protected]> wrote:
>>
>> Hi,
>>
>> On 04-06-2013 08:57, Eduardo Valentin wrote:
>>> On 04-06-2013 00:55, amit daniel kachhap wrote:
>>>> Hi Eduardo,
>>>>
>>>> On Wed, May 15, 2013 at 8:14 PM, Eduardo Valentin
>>>> <[email protected]> wrote:
>>>>> On 14-05-2013 05:58, Amit Daniel Kachhap wrote:
>>>>>> Changes in V4:
>>>>>> Almost all the changes in this version is as per suggestion from Eduardo.The
>>>>>> major ones are listed below,
>>>>>> * Added kconfig symbol ARCH_HAS_TMU which needs to be enabled by platform. With
>>>>>> this change existing symbol EXYNOS_TMU_DATA is not needed.
>>>>>
>>>>> I was more thinking if we could have a single flag that we could use in
>>>>> thermal drivers. Besides, my proposal for ARCH_HAS_BANDGAP is still not
>>>>> merged yet.
>>>>
>>>> yes a single flag might be useful such as ARCH_HAS_THERMAL but this
>>>> patch can work as an intermediate solution.
>>>
>>> I think bandgap is a generic enough term IMO.
>>>
>>> [1] - http://en.wikipedia.org/wiki/Bandgap_voltage_reference
>>> [2] - http://en.wikipedia.org/wiki/Silicon_bandgap_temperature_sensor
>>>
>>
>> Hit the send button too early. So, I think bandgap is a generic enough
>> term. Using bandgap I believe it is better because its meaning has the
>> implication of having a device to control thermal.
>>
>> But if you feel more confident with the thermal term we can switch to
>> something like HAS_THERMAL_CONTROL.
>
> Even in my case TMU uses bandgap voltage reference circuit. so
> ARCH_HAS_BANDGAP should be fine.
>

OK cool! then we have a consensus.

I am trying for some time already to push this patch here:
https://patchwork.kernel.org/patch/2659001/

Tony, Arnd?

Any objections having the patch above in?

> Thanks,
> Amit Daniel
>>
>>>
>>>>
>>>> Thanks,
>>>> Amit Daniel
>>>>>
>>>>>> * Movement of freq_clip_table from exynos_tmu.h to exynos_thermal_common.h is
>>>>>> explained in the commit logs.
>>>>>> * Wrote all register description documentation.
>>>>>> * Split 5440 TMU support patch into controller change, configuration data and
>>>>>> feature addition patches.
>>>>>> * Remove all *LINUX_* in the header files.
>>>>>> * Still regulator enable is kept optional but a TODO: comment is added to fix
>>>>>> it later.
>>>>>>
>>>>>> Changes in V3:
>>>>>> * Added proper dependency of different exynos thermal Kconfig symbols. Basically 3
>>>>>> Kconfig can be enabled now and corresponds to tmu driver. exynos common part
>>>>>> and exynos configuration data. This issue was raised by Rui Zhang.
>>>>>>
>>>>>> Changes in V2:
>>>>>> * Separated SOC data from TMU driver. This is as per suggestion from Eduardo.
>>>>>> * Merged the new file created for exynos5440 TMU controller with the existing
>>>>>> TMU controller code.
>>>>>> * Removed the DT parsing code as now the SOC specific data are cleanly put
>>>>>> inside the data specific file.
>>>>>> * Even the register definations/bitfields are treated as data as there is
>>>>>> some variation across SOC's.
>>>>>>
>>>>>> This patchset adds TMU(Thermal management Unit) driver support for
>>>>>> exynos5440 platform. There are 3 instances of the TMU controllers so
>>>>>> necessary cleanup/re-structure is done to handle multiple thermal zone.
>>>>>>
>>>>>> Patch (exynos4: Add documentation for Exynos SoC thermal bindings) from
>>>>>> Lukasz Majewski is already posted to mainline. Adding it here for completeness.
>>>>>> (http://www.mail-archive.com/[email protected]/msg17817.html)
>>>>>>
>>>>>> Patch (thermal: exynos: Support thermal tripping ) from Jonghwan Choi is
>>>>>> added here with some changes.
>>>>>> (https://patchwork.kernel.org/patch/1668371/)
>>>>>>
>>>>>> Patch (thermal: exynos: Support for TMU regulator defined at device tree)
>>>>>> is a repost of my earlier patch(https://patchwork-mail1.kernel.org/patch/2510771/)
>>>>>> and adds regulator support.
>>>>>>
>>>>>> Patch (ARM: dts: Add device tree node for exynos5440 TMU controller) and
>>>>>> patch (arm: exynos: enable ARCH_HAS_TMU) can be merged through exynos platform
>>>>>> maintainer as this can cause merge conflict.
>>>>>>
>>>>>> All these patches are based on thermal maintainers git tree,
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next.
>>>>>>
>>>>>> Amit Daniel Kachhap (29):
>>>>>> thermal: exynos: Moving exynos thermal files into samsung directory
>>>>>> thermal: exynos: Add ARCH_HAS_TMU config to know the supported soc's
>>>>>> thermal: exynos: Remove CPU_THERMAL dependency for using TMU driver
>>>>>> thermal: exynos: Bifurcate exynos thermal common and tmu controller
>>>>>> code
>>>>>> thermal: exynos: Rename exynos_thermal.c to exynos_tmu.c
>>>>>> thermal: exynos: Move exynos_thermal.h from include/* to driver/*
>>>>>> folder
>>>>>> thermal: exynos: Bifurcate exynos tmu driver and configuration data
>>>>>> thermal: exynos: Add missing definations and code cleanup
>>>>>> thermal: exynos: Add extra entries in the tmu platform data
>>>>>> thermal: exynos: Support thermal tripping
>>>>>> thermal: exynos: Move register definitions from driver file to data
>>>>>> file
>>>>>> thermal: exynos: Fix to clear only the generated interrupts
>>>>>> thermal: exynos: Add support for instance based register/unregister
>>>>>> thermal: exynos: Modify private_data to appropriate name driver_data
>>>>>> thermal: exynos: Return success even if no cooling data supplied
>>>>>> thermal: exynos: Make the zone handling dependent on trip count
>>>>>> thermal: exynos: Add support to handle many instances of TMU
>>>>>> thermal: exynos: Add TMU features to check instead of using SOC type
>>>>>> thermal: exynos: use device resource management infrastructure
>>>>>> thermal: exynos: Add support to access common register for
>>>>>> multistance
>>>>>> thermal: exynos: Add support for exynos5440 TMU sensor.
>>>>>> thermal: exynos: Fix to set the second point correction value
>>>>>> properly
>>>>>> thermal: exynos: Add thermal configuration data for exynos5440 TMU
>>>>>> sensor
>>>>>> thermal: exynos: Add a compensation logic on swapped e-fuse values
>>>>>> thermal: exynos: Add hardware mode thermal calibration support
>>>>>> Documentation: thermal: Explain the exynos thermal driver model
>>>>>> thermal: exynos: Support for TMU regulator defined at device tree
>>>>>> ARM: dts: Add device tree node for exynos5440 TMU controller
>>>>>> arm: exynos: enable ARCH_HAS_TMU
>>>>>>
>>>>>> Lukasz Majewski (1):
>>>>>> ARM: dts: thermal: exynos4: Add documentation for Exynos SoC thermal
>>>>>> bindings
>>>>>>
>>>>>> .../devicetree/bindings/thermal/exynos-thermal.txt | 53 +
>>>>>> Documentation/thermal/exynos_thermal | 43 +-
>>>>>> arch/arm/boot/dts/exynos5440.dtsi | 30 +
>>>>>> arch/arm/mach-exynos/Kconfig | 5 +
>>>>>> drivers/thermal/Kconfig | 13 +-
>>>>>> drivers/thermal/Makefile | 2 +-
>>>>>> drivers/thermal/exynos_thermal.c | 1066 --------------------
>>>>>> drivers/thermal/samsung/Kconfig | 21 +
>>>>>> drivers/thermal/samsung/Makefile | 7 +
>>>>>> drivers/thermal/samsung/exynos_thermal_common.c | 407 ++++++++
>>>>>> drivers/thermal/samsung/exynos_thermal_common.h | 98 ++
>>>>>> drivers/thermal/samsung/exynos_tmu.c | 741 ++++++++++++++
>>>>>> drivers/thermal/samsung/exynos_tmu.h | 294 ++++++
>>>>>> drivers/thermal/samsung/exynos_tmu_data.c | 225 ++++
>>>>>> drivers/thermal/samsung/exynos_tmu_data.h | 155 +++
>>>>>> include/linux/platform_data/exynos_thermal.h | 119 ---
>>>>>> 16 files changed, 2076 insertions(+), 1203 deletions(-)
>>>>>> create mode 100644 Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>>>>> delete mode 100644 drivers/thermal/exynos_thermal.c
>>>>>> create mode 100644 drivers/thermal/samsung/Kconfig
>>>>>> create mode 100644 drivers/thermal/samsung/Makefile
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_thermal_common.c
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_thermal_common.h
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_tmu.c
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_tmu.h
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_tmu_data.c
>>>>>> create mode 100644 drivers/thermal/samsung/exynos_tmu_data.h
>>>>>> delete mode 100644 include/linux/platform_data/exynos_thermal.h
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>> --
>> You have got to be excited about what you are doing. (L. Lamport)
>>
>> Eduardo Valentin
>>
>
>

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

Eduardo Valentin

Attachments:

signature.asc (295.00 B)
OpenPGP digital signature