mbox series

[v6,0/4] Exynos Thermal code improvement

Message ID 20250430123306.15072-1-linux.amoon@gmail.com
Headers show
Series Exynos Thermal code improvement | expand

Message

Anand Moon April 30, 2025, 12:32 p.m. UTC
Hi All,

This patch series is a rework of my previous patch series [1],
where the code changes were not adequately justified.

In this new series, I have improved the commit subject
and commit message to better explain the changes.

v6: Add new patch to use devm_clk_get_enabled
    and Fix few typo in subject as suggested by Daniel.
v5: Drop the guard mutex patch
v4: Tried to address Lukasz review comments.

Tested on Odroid U3 amd XU4 SoC boards.
Build with clang with W=1 enable.

[4] https://lore.kernel.org/all/20250410063754.5483-2-linux.amoon@gmail.com/
[3] https://lore.kernel.org/all/20250310143450.8276-2-linux.amoon@gmail.com/
[2] https://lore.kernel.org/all/20250216195850.5352-2-linux.amoon@gmail.com/
[1] https://lore.kernel.org/all/20220515064126.1424-1-linux.amoon@gmail.com/
[0] https://lore.kernel.org/lkml/CANAwSgS=08fVsqn95WHzSF71WTTyD2-=K2C6-BEz0tY0t6A1-g@mail.gmail.com/T/#m77e57120d230d57f34c29e1422d7fc5f5587ac30

Thanks
-Anand

Anand Moon (4):
  thermal/drivers/exynos: Refactor clk_sec initialization inside
    SOC-specific case
  thermal/drivers/exynos: Use devm_clk_get_enabled() helpers
  thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec
    clock
  thermal/drivers/exynos: Fixed the efuse min max value for exynos5422

 drivers/thermal/samsung/exynos_tmu.c | 100 ++++++++++-----------------
 1 file changed, 35 insertions(+), 65 deletions(-)


base-commit: b6ea1680d0ac0e45157a819c41b46565f4616186

Comments

Anand Moon May 8, 2025, 6:14 a.m. UTC | #1
Hi All,

On Wed, 30 Apr 2025 at 18:03, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi All,
>
> This patch series is a rework of my previous patch series [1],
> where the code changes were not adequately justified.
>
> In this new series, I have improved the commit subject
> and commit message to better explain the changes.
>
> v6: Add new patch to use devm_clk_get_enabled
>     and Fix few typo in subject as suggested by Daniel.
> v5: Drop the guard mutex patch
> v4: Tried to address Lukasz review comments.
>
> Tested on Odroid U3 amd XU4 SoC boards.
> Build with clang with W=1 enable.
>

Genital Ping!!!

Thanks
-Anand
Krzysztof Kozlowski May 8, 2025, 6:26 a.m. UTC | #2
On 08/05/2025 08:14, Anand Moon wrote:
> Hi All,
> 
> On Wed, 30 Apr 2025 at 18:03, Anand Moon <linux.amoon@gmail.com> wrote:
>>
>> Hi All,
>>
>> This patch series is a rework of my previous patch series [1],
>> where the code changes were not adequately justified.
>>
>> In this new series, I have improved the commit subject
>> and commit message to better explain the changes.
>>
>> v6: Add new patch to use devm_clk_get_enabled
>>     and Fix few typo in subject as suggested by Daniel.
>> v5: Drop the guard mutex patch
>> v4: Tried to address Lukasz review comments.
>>
>> Tested on Odroid U3 amd XU4 SoC boards.
>> Build with clang with W=1 enable.
>>
> 
> Genital Ping!!!


Huhu, nice. :)
I make typos as well, but some typos are better to avoid. :)

Anyway, !!! are exclamation marks and I think it is very difficult to
scream at someone gently. I think this is contradictory to itself, so it
does not feel gently at all.

Plus you sent it 7 days ago and you are known to send poor quality,
untested code, so just relax and wait.

Best regards,
Krzysztof
Anand Moon May 8, 2025, 11:36 a.m. UTC | #3
Hi Krzysztof,
On Thu, 8 May 2025 at 11:57, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 08/05/2025 08:14, Anand Moon wrote:
> > Hi All,
> >
> > On Wed, 30 Apr 2025 at 18:03, Anand Moon <linux.amoon@gmail.com> wrote:
> >>
> >> Hi All,
> >>
> >> This patch series is a rework of my previous patch series [1],
> >> where the code changes were not adequately justified.
> >>
> >> In this new series, I have improved the commit subject
> >> and commit message to better explain the changes.
> >>
> >> v6: Add new patch to use devm_clk_get_enabled
> >>     and Fix few typo in subject as suggested by Daniel.
> >> v5: Drop the guard mutex patch
> >> v4: Tried to address Lukasz review comments.
> >>
> >> Tested on Odroid U3 amd XU4 SoC boards.
> >> Build with clang with W=1 enable.
> >>
> >
> > Genital Ping!!!
>
>
> Huhu, nice. :)
> I make typos as well, but some typos are better to avoid. :)
>
> Anyway, !!! are exclamation marks and I think it is very difficult to
> scream at someone gently. I think this is contradictory to itself, so it
> does not feel gently at all.
>
I did not mean anything harsh with these !!! marks.

> Plus you sent it 7 days ago and you are known to send poor quality,
> untested code, so just relax and wait.
>
NAK - Do not merge these changes. The original code is in better condition,
and the proposed modifications introduce issues.

> Best regards,
> Krzysztof

Thanks
-Anand
Daniel Lezcano May 14, 2025, 11:23 a.m. UTC | #4
On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
> Hi All,

Hi Anand,

if the goal of the changes is to do cleanups, I recommend to rework
how the code is organized. Instead of having the data->soc check all
around the functions, write per platform functions and store them in
struct of_device_id data field instead of the soc version.

Basically get rid of exynos_map_dt_data by settings the different ops
in a per platform structure.

Then the initialization routine would be simpler to clean.

> This patch series is a rework of my previous patch series [1],
> where the code changes were not adequately justified.
> 
> In this new series, I have improved the commit subject
> and commit message to better explain the changes.
> 
> v6: Add new patch to use devm_clk_get_enabled
>     and Fix few typo in subject as suggested by Daniel.
> v5: Drop the guard mutex patch
> v4: Tried to address Lukasz review comments.
> 
> Tested on Odroid U3 amd XU4 SoC boards.
> Build with clang with W=1 enable.
> 
> [4] https://lore.kernel.org/all/20250410063754.5483-2-linux.amoon@gmail.com/
> [3] https://lore.kernel.org/all/20250310143450.8276-2-linux.amoon@gmail.com/
> [2] https://lore.kernel.org/all/20250216195850.5352-2-linux.amoon@gmail.com/
> [1] https://lore.kernel.org/all/20220515064126.1424-1-linux.amoon@gmail.com/
> [0] https://lore.kernel.org/lkml/CANAwSgS=08fVsqn95WHzSF71WTTyD2-=K2C6-BEz0tY0t6A1-g@mail.gmail.com/T/#m77e57120d230d57f34c29e1422d7fc5f5587ac30
> 
> Thanks
> -Anand
> 
> Anand Moon (4):
>   thermal/drivers/exynos: Refactor clk_sec initialization inside
>     SOC-specific case
>   thermal/drivers/exynos: Use devm_clk_get_enabled() helpers
>   thermal/drivers/exynos: Remove redundant IS_ERR() checks for clk_sec
>     clock
>   thermal/drivers/exynos: Fixed the efuse min max value for exynos5422
> 
>  drivers/thermal/samsung/exynos_tmu.c | 100 ++++++++++-----------------
>  1 file changed, 35 insertions(+), 65 deletions(-)
> 
> 
> base-commit: b6ea1680d0ac0e45157a819c41b46565f4616186
> -- 
> 2.49.0
>
Anand Moon May 15, 2025, 11:10 a.m. UTC | #5
Hi Daniel,

On Wed, 14 May 2025 at 16:53, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
> > Hi All,
>
> Hi Anand,
>
> if the goal of the changes is to do cleanups, I recommend to rework
> how the code is organized. Instead of having the data->soc check all
> around the functions, write per platform functions and store them in
> struct of_device_id data field instead of the soc version.
>
> Basically get rid of exynos_map_dt_data by settings the different ops
> in a per platform structure.
>
> Then the initialization routine would be simpler to clean.
>

Thanks, I had previously attempted this approach.
The goal is to split the exynos_tmu_data structure to accommodate
SoC-specific callbacks for initialization and configuration.

In my earlier attempt, I tried to refactor the code to achieve this.
However, the main challenge I encountered was that the
exynos_sensor_ops weren’t being correctly mapped for each SoC.

Some SoC have multiple sensor
exynos4x12
                    tmu: tmu@100c0000
exynos5420
                tmu_cpu0: tmu@10060000
                tmu_cpu1: tmu@10064000
                tmu_cpu2: tmu@10068000
                tmu_cpu3: tmu@1006c000
                tmu_gpu: tmu@100a0000
 exynos5433
                tmu_atlas0: tmu@10060000
                tmu_atlas1: tmu@10068000
                tmu_g3d: tmu@10070000
exynos7
                tmu@10060000

It could be a design issue of the structure.or some DTS issue.
So what I found in debugging it is not working correctly.

static const struct thermal_zone_device_ops exynos_sensor_ops = {
        .get_temp = exynos_get_temp,
        .set_emul_temp = exynos_tmu_set_emulation,
        .set_trips = exynos_set_trips,
};

The sensor callback will not return a valid pointer and soc id for the get_temp.

Here is my earlier version of local changes.
[1] https://pastebin.com/bbEP04Zh exynos_tmu.c
[2] https://pastebin.com/PzNz5yve Odroid U3 dmesg.log
[3] https://pastebin.com/4Yjt2d2u    Odroid Xu4 dmesg.log

I want to re-model the structure to improve the code.
Once Its working condition I will send this for review.

If you have some suggestions please let me know.

Thanks
-Anand
Daniel Lezcano May 15, 2025, 1:28 p.m. UTC | #6
On 5/15/25 13:10, Anand Moon wrote:
> Hi Daniel,
> 
> On Wed, 14 May 2025 at 16:53, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
>>> Hi All,
>>
>> Hi Anand,
>>
>> if the goal of the changes is to do cleanups, I recommend to rework
>> how the code is organized. Instead of having the data->soc check all
>> around the functions, write per platform functions and store them in
>> struct of_device_id data field instead of the soc version.
>>
>> Basically get rid of exynos_map_dt_data by settings the different ops
>> in a per platform structure.
>>
>> Then the initialization routine would be simpler to clean.
>>
> 
> Thanks, I had previously attempted this approach.
> The goal is to split the exynos_tmu_data structure to accommodate
> SoC-specific callbacks for initialization and configuration.
> 
> In my earlier attempt, I tried to refactor the code to achieve this.
> However, the main challenge I encountered was that the
> exynos_sensor_ops weren’t being correctly mapped for each SoC.
> 
> Some SoC have multiple sensor
> exynos4x12
>                      tmu: tmu@100c0000
> exynos5420
>                  tmu_cpu0: tmu@10060000
>                  tmu_cpu1: tmu@10064000
>                  tmu_cpu2: tmu@10068000
>                  tmu_cpu3: tmu@1006c000
>                  tmu_gpu: tmu@100a0000
>   exynos5433
>                  tmu_atlas0: tmu@10060000
>                  tmu_atlas1: tmu@10068000
>                  tmu_g3d: tmu@10070000
> exynos7
>                  tmu@10060000
> 
> It could be a design issue of the structure.or some DTS issue.
> So what I found in debugging it is not working correctly.
> 
> static const struct thermal_zone_device_ops exynos_sensor_ops = {
>          .get_temp = exynos_get_temp,
>          .set_emul_temp = exynos_tmu_set_emulation,
>          .set_trips = exynos_set_trips,
> };
> 
> The sensor callback will not return a valid pointer and soc id for the get_temp.
> 
> Here is my earlier version of local changes.
> [1] https://pastebin.com/bbEP04Zh exynos_tmu.c
> [2] https://pastebin.com/PzNz5yve Odroid U3 dmesg.log
> [3] https://pastebin.com/4Yjt2d2u    Odroid Xu4 dmesg.log
> 
> I want to re-model the structure to improve the code.
> Once Its working condition I will send this for review.
> 
> If you have some suggestions please let me know.

I suggest to do the conversion step by step beginning by 
exynos4210_tmu_clear_irqs, then by exynos_map_dt_data as the first 
cleanup iteration
Anand Moon May 15, 2025, 6:01 p.m. UTC | #7
Hi Daniel,

On Thu, 15 May 2025 at 18:59, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 5/15/25 13:10, Anand Moon wrote:
> > Hi Daniel,
> >
> > On Wed, 14 May 2025 at 16:53, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >> On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
> >>> Hi All,
> >>
> >> Hi Anand,
> >>
> >> if the goal of the changes is to do cleanups, I recommend to rework
> >> how the code is organized. Instead of having the data->soc check all
> >> around the functions, write per platform functions and store them in
> >> struct of_device_id data field instead of the soc version.
> >>
> >> Basically get rid of exynos_map_dt_data by settings the different ops
> >> in a per platform structure.
> >>
> >> Then the initialization routine would be simpler to clean.
> >>
> >
> > Thanks, I had previously attempted this approach.
> > The goal is to split the exynos_tmu_data structure to accommodate
> > SoC-specific callbacks for initialization and configuration.
> >
> > In my earlier attempt, I tried to refactor the code to achieve this.
> > However, the main challenge I encountered was that the
> > exynos_sensor_ops weren’t being correctly mapped for each SoC.
> >
> > Some SoC have multiple sensor
> > exynos4x12
> >                      tmu: tmu@100c0000
> > exynos5420
> >                  tmu_cpu0: tmu@10060000
> >                  tmu_cpu1: tmu@10064000
> >                  tmu_cpu2: tmu@10068000
> >                  tmu_cpu3: tmu@1006c000
> >                  tmu_gpu: tmu@100a0000
> >   exynos5433
> >                  tmu_atlas0: tmu@10060000
> >                  tmu_atlas1: tmu@10068000
> >                  tmu_g3d: tmu@10070000
> > exynos7
> >                  tmu@10060000
> >
> > It could be a design issue of the structure.or some DTS issue.
> > So what I found in debugging it is not working correctly.
> >
> > static const struct thermal_zone_device_ops exynos_sensor_ops = {
> >          .get_temp = exynos_get_temp,
> >          .set_emul_temp = exynos_tmu_set_emulation,
> >          .set_trips = exynos_set_trips,
> > };
> >
> > The sensor callback will not return a valid pointer and soc id for the get_temp.
> >
> > Here is my earlier version of local changes.
> > [1] https://pastebin.com/bbEP04Zh exynos_tmu.c
> > [2] https://pastebin.com/PzNz5yve Odroid U3 dmesg.log
> > [3] https://pastebin.com/4Yjt2d2u    Odroid Xu4 dmesg.log
> >
> > I want to re-model the structure to improve the code.
> > Once Its working condition I will send this for review.
> >
> > If you have some suggestions please let me know.
>
> I suggest to do the conversion step by step beginning by
> exynos4210_tmu_clear_irqs, then by exynos_map_dt_data as the first
> cleanup iteration
>
Ok you want IRQ handle per SoC call back functions?
so on all the changes depending on SoC id should be moved to
respective callback functions to reduce the code.

Thank  you for having a look into my changes

Thanks
-Anand
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
Daniel Lezcano May 16, 2025, 2:45 p.m. UTC | #8
On 5/15/25 20:01, Anand Moon wrote:
> Hi Daniel,
> 
> On Thu, 15 May 2025 at 18:59, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 5/15/25 13:10, Anand Moon wrote:
>>> Hi Daniel,
>>>
>>> On Wed, 14 May 2025 at 16:53, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>>
>>>> On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
>>>>> Hi All,
>>>>
>>>> Hi Anand,
>>>>
>>>> if the goal of the changes is to do cleanups, I recommend to rework
>>>> how the code is organized. Instead of having the data->soc check all
>>>> around the functions, write per platform functions and store them in
>>>> struct of_device_id data field instead of the soc version.
>>>>
>>>> Basically get rid of exynos_map_dt_data by settings the different ops
>>>> in a per platform structure.
>>>>
>>>> Then the initialization routine would be simpler to clean.
>>>>
>>>
>>> Thanks, I had previously attempted this approach.
>>> The goal is to split the exynos_tmu_data structure to accommodate
>>> SoC-specific callbacks for initialization and configuration.
>>>
>>> In my earlier attempt, I tried to refactor the code to achieve this.
>>> However, the main challenge I encountered was that the
>>> exynos_sensor_ops weren’t being correctly mapped for each SoC.
>>>
>>> Some SoC have multiple sensor
>>> exynos4x12
>>>                       tmu: tmu@100c0000
>>> exynos5420
>>>                   tmu_cpu0: tmu@10060000
>>>                   tmu_cpu1: tmu@10064000
>>>                   tmu_cpu2: tmu@10068000
>>>                   tmu_cpu3: tmu@1006c000
>>>                   tmu_gpu: tmu@100a0000
>>>    exynos5433
>>>                   tmu_atlas0: tmu@10060000
>>>                   tmu_atlas1: tmu@10068000
>>>                   tmu_g3d: tmu@10070000
>>> exynos7
>>>                   tmu@10060000
>>>
>>> It could be a design issue of the structure.or some DTS issue.
>>> So what I found in debugging it is not working correctly.
>>>
>>> static const struct thermal_zone_device_ops exynos_sensor_ops = {
>>>           .get_temp = exynos_get_temp,
>>>           .set_emul_temp = exynos_tmu_set_emulation,
>>>           .set_trips = exynos_set_trips,
>>> };
>>>
>>> The sensor callback will not return a valid pointer and soc id for the get_temp.
>>>
>>> Here is my earlier version of local changes.
>>> [1] https://pastebin.com/bbEP04Zh exynos_tmu.c
>>> [2] https://pastebin.com/PzNz5yve Odroid U3 dmesg.log
>>> [3] https://pastebin.com/4Yjt2d2u    Odroid Xu4 dmesg.log
>>>
>>> I want to re-model the structure to improve the code.
>>> Once Its working condition I will send this for review.
>>>
>>> If you have some suggestions please let me know.
>>
>> I suggest to do the conversion step by step beginning by
>> exynos4210_tmu_clear_irqs, then by exynos_map_dt_data as the first
>> cleanup iteration
>>
> Ok you want IRQ handle per SoC call back functions?
> so on all the changes depending on SoC id should be moved to
> respective callback functions to reduce the code.

I think you can keep the same irq handler function but move the 
tmu_intstat, tmu_intclear in the persoc structure and remove the 
exynos4210_tmu_clear_irqs function.

You should end up with something like:

static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
{
	struct exynos_tmu_data *data = id;
	unsigned int val_irq;

	thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);

	mutex_lock(&data->lock);
	clk_enable(data->clk);

	val_irq = readl(data->base + data->tmu_intstat);
         writel(val_irq, data->base + data->tmu_intclear);

	clk_disable(data->clk);
	mutex_unlock(&data->lock);

	return IRQ_HANDLED;
}

But if the irq handler has some soc specific code, then it should be a 
separate function
Anand Moon May 17, 2025, 7:41 p.m. UTC | #9
Hi Daniel,

On Fri, 16 May 2025 at 20:15, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 5/15/25 20:01, Anand Moon wrote:
> > Hi Daniel,
> >
> > On Thu, 15 May 2025 at 18:59, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 5/15/25 13:10, Anand Moon wrote:
> >>> Hi Daniel,
> >>>
> >>> On Wed, 14 May 2025 at 16:53, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>>>
> >>>> On Wed, Apr 30, 2025 at 06:02:56PM +0530, Anand Moon wrote:
> >>>>> Hi All,
> >>>>
> >>>> Hi Anand,
> >>>>
> >>>> if the goal of the changes is to do cleanups, I recommend to rework
> >>>> how the code is organized. Instead of having the data->soc check all
> >>>> around the functions, write per platform functions and store them in
> >>>> struct of_device_id data field instead of the soc version.
> >>>>
> >>>> Basically get rid of exynos_map_dt_data by settings the different ops
> >>>> in a per platform structure.
> >>>>
> >>>> Then the initialization routine would be simpler to clean.
> >>>>
> >>>
> >>> Thanks, I had previously attempted this approach.
> >>> The goal is to split the exynos_tmu_data structure to accommodate
> >>> SoC-specific callbacks for initialization and configuration.
> >>>
> >>> In my earlier attempt, I tried to refactor the code to achieve this.
> >>> However, the main challenge I encountered was that the
> >>> exynos_sensor_ops weren’t being correctly mapped for each SoC.
> >>>
> >>> Some SoC have multiple sensor
> >>> exynos4x12
> >>>                       tmu: tmu@100c0000
> >>> exynos5420
> >>>                   tmu_cpu0: tmu@10060000
> >>>                   tmu_cpu1: tmu@10064000
> >>>                   tmu_cpu2: tmu@10068000
> >>>                   tmu_cpu3: tmu@1006c000
> >>>                   tmu_gpu: tmu@100a0000
> >>>    exynos5433
> >>>                   tmu_atlas0: tmu@10060000
> >>>                   tmu_atlas1: tmu@10068000
> >>>                   tmu_g3d: tmu@10070000
> >>> exynos7
> >>>                   tmu@10060000
> >>>
> >>> It could be a design issue of the structure.or some DTS issue.
> >>> So what I found in debugging it is not working correctly.
> >>>
> >>> static const struct thermal_zone_device_ops exynos_sensor_ops = {
> >>>           .get_temp = exynos_get_temp,
> >>>           .set_emul_temp = exynos_tmu_set_emulation,
> >>>           .set_trips = exynos_set_trips,
> >>> };
> >>>
> >>> The sensor callback will not return a valid pointer and soc id for the get_temp.
> >>>
> >>> Here is my earlier version of local changes.
> >>> [1] https://pastebin.com/bbEP04Zh exynos_tmu.c
> >>> [2] https://pastebin.com/PzNz5yve Odroid U3 dmesg.log
> >>> [3] https://pastebin.com/4Yjt2d2u    Odroid Xu4 dmesg.log
> >>>
> >>> I want to re-model the structure to improve the code.
> >>> Once Its working condition I will send this for review.
> >>>
> >>> If you have some suggestions please let me know.
> >>
> >> I suggest to do the conversion step by step beginning by
> >> exynos4210_tmu_clear_irqs, then by exynos_map_dt_data as the first
> >> cleanup iteration
> >>
> > Ok you want IRQ handle per SoC call back functions?
> > so on all the changes depending on SoC id should be moved to
> > respective callback functions to reduce the code.
>
> I think you can keep the same irq handler function but move the
> tmu_intstat, tmu_intclear in the persoc structure and remove the
> exynos4210_tmu_clear_irqs function.
>
> You should end up with something like:
>
> static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
> {
>         struct exynos_tmu_data *data = id;
>         unsigned int val_irq;
>
>         thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
>
>         mutex_lock(&data->lock);
>         clk_enable(data->clk);
>
>         val_irq = readl(data->base + data->tmu_intstat);
>          writel(val_irq, data->base + data->tmu_intclear);
>
>         clk_disable(data->clk);
>         mutex_unlock(&data->lock);
>
>         return IRQ_HANDLED;
> }

No this will not work,
>
> But if the irq handler has some soc specific code, then it should be a
> separate function
>

INTSTAT interrupt status register holds the pending status of
rising and falling edge of IRQ

#define INTSTAT_FALL2   BIT(24)
#define INTSTAT_FALL1   BIT(20)
#define INTSTAT_FALL0   BIT(16)
#define INTSTAT_RISE2   BIT(8)
#define INTSTAT_RISE1   BIT(4)
#define INTSTAT_RISE0   BIT(0)

#define INTCLEAR_FALL2  BIT(24)
#define INTCLEAR_FALL1  BIT(20)
#define INTCLEAR_FALL0  BIT(16)
#define INTCLEAR_RISE2  BIT(8)
#define INTCLEAR_RISE1  BIT(4)
#define INTCLEAR_RISE0  BIT(0)

static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data)
{
        u32 tmu_intstat, tmu_intclear;
        u32 val_irq = 0, clear_mask = 0;

        if (data->soc == SOC_ARCH_EXYNOS5260) {
                tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT;
                tmu_intclear = EXYNOS5260_TMU_REG_INTCLEAR;
        } else if (data->soc == SOC_ARCH_EXYNOS7) {
                tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
                tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
        } else if (data->soc == SOC_ARCH_EXYNOS5433) {
                tmu_intstat = EXYNOS5433_TMU_REG_INTPEND;
                tmu_intclear = EXYNOS5433_TMU_REG_INTPEND;
        } else {
                tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
                tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
        }

        val_irq = readl(data->base + tmu_intstat);
        /*
         * Clear the interrupts.  Please note that the documentation for
         * Exynos3250, Exynos4412, Exynos5250 and Exynos5260 incorrectly
         * states that INTCLEAR register has a different placing of bits
         * responsible for FALL IRQs than INTSTAT register.  Exynos5420
         * and Exynos5440 documentation is correct (Exynos4210 doesn't
         * support FALL IRQs at all).
         */

        /* Map INTSTAT bits to INTCLEAR bits */
        if (val_irq & BIT(24))
                clear_mask |= BIT(24);
        else if (val_irq & BIT(20))
                clear_mask |= BIT(20);
        else if (val_irq & BIT(16))
                clear_mask |= BIT(16);
        else if (val_irq & BIT(8))
                clear_mask |= BIT(8);
        else if (val_irq & BIT(4))
                clear_mask |= BIT(4);
        else if (val_irq & BIT(0))
                clear_mask |= BIT(0);

        /* Perform proper task for decrease temperature */
        if (clear_mask)
                writel(clear_mask, data->base + tmu_intclear);
}

TMU clears the rising and falling interrupt according to the user manual.

Thanks
-Anand
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog