Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-17 Thread Daniel Lezcano


Hi Anson,

On 06/11/2018 10:12, Anson Huang wrote:

[ ... ]

 Please convert to timer-of API.
>>>
>>> This patch mainly to fix the potential resource leak issue, for
>>> converting to timer-of API, should I add another patch for it? Since
>>> it is a different subject.
>>
>> Actually I was unclear. The patch is duplicating the kfree in the error path,
>> usually a rollback routine should be provided.
>>
>> So instead of creating this rollback path, you can directly use the timer-of
>> which contains the error check and rollback.
> 
> I understand the timer-of can save some code in timer driver, but current GPT
> driver is kind of complicated in order to support so many i.MX platforms with
> different GPT design, change it to timer-of will need some effort and I think 
> we
> can do it later. If the duplicating kfree in error path confuses you, I can 
> change
> it to rollback path you said. Below are 2 options in my mind:
> 
> 1. add kfree to rollback path;
> 2. skip this patch, just apply 1/2 patch, and let me find some time to change 
> GPT
> driver using timer-of. 
> 
> Which one do you prefer? Thanks.

I'm fine with option 2.

Thanks

  -- Daniel

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-17 Thread Daniel Lezcano


Hi Anson,

On 06/11/2018 10:12, Anson Huang wrote:

[ ... ]

 Please convert to timer-of API.
>>>
>>> This patch mainly to fix the potential resource leak issue, for
>>> converting to timer-of API, should I add another patch for it? Since
>>> it is a different subject.
>>
>> Actually I was unclear. The patch is duplicating the kfree in the error path,
>> usually a rollback routine should be provided.
>>
>> So instead of creating this rollback path, you can directly use the timer-of
>> which contains the error check and rollback.
> 
> I understand the timer-of can save some code in timer driver, but current GPT
> driver is kind of complicated in order to support so many i.MX platforms with
> different GPT design, change it to timer-of will need some effort and I think 
> we
> can do it later. If the duplicating kfree in error path confuses you, I can 
> change
> it to rollback path you said. Below are 2 options in my mind:
> 
> 1. add kfree to rollback path;
> 2. skip this patch, just apply 1/2 patch, and let me find some time to change 
> GPT
> driver using timer-of. 
> 
> Which one do you prefer? Thanks.

I'm fine with option 2.

Thanks

  -- Daniel

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-06 Thread Anson Huang
Hi, Daniel

Best Regards!
Anson Huang

> -Original Message-
> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> Sent: 2018年11月6日 17:06
> To: Anson Huang ; t...@linutronix.de;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid
> resource leak
> 
> 
> Hi Anson,
> 
> On 06/11/2018 02:38, Anson Huang wrote:
> > Hi, Daniel
> >
> > Best Regards!
> > Anson Huang
> >
> >> -Original Message-
> >> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> >> Sent: 2018年11月5日 21:35
> >> To: Anson Huang ; t...@linutronix.de;
> >> linux-kernel@vger.kernel.org
> >> Cc: dl-linux-imx 
> >> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree
> >> to avoid resource leak
> >>
> >> Hi Anson,
> >>
> >> On 05/11/2018 02:10, Anson Huang wrote:
> >>> kfree should be called to free resource in error path before return.
> >>>
> >>> Signed-off-by: Anson Huang 
> >>> ---
> >>>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/clocksource/timer-imx-gpt.c
> >>> b/drivers/clocksource/timer-imx-gpt.c
> >>> index a3d6ccb..0f78b30 100644
> >>> --- a/drivers/clocksource/timer-imx-gpt.c
> >>> +++ b/drivers/clocksource/timer-imx-gpt.c
> >>> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
> >> device_node *np,  enum imx_gpt_type t
> >>>   return -ENOMEM;
> >>>
> >>>   imxtm->base = of_iomap(np, 0);
> >>> - if (!imxtm->base)
> >>> + if (!imxtm->base) {
> >>> + kfree(imxtm);
> >>>   return -ENXIO;
> >>> + }
> >>>
> >>>   imxtm->irq = irq_of_parse_and_map(np, 0);
> >>> - if (imxtm->irq <= 0)
> >>> + if (imxtm->irq <= 0) {
> >>> + kfree(imxtm);
> >>>   return -EINVAL;
> >>> + }
> >>>
> >>>   imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> >>
> >> Please convert to timer-of API.
> >
> > This patch mainly to fix the potential resource leak issue, for
> > converting to timer-of API, should I add another patch for it? Since
> > it is a different subject.
> 
> Actually I was unclear. The patch is duplicating the kfree in the error path,
> usually a rollback routine should be provided.
> 
> So instead of creating this rollback path, you can directly use the timer-of
> which contains the error check and rollback.

I understand the timer-of can save some code in timer driver, but current GPT
driver is kind of complicated in order to support so many i.MX platforms with
different GPT design, change it to timer-of will need some effort and I think we
can do it later. If the duplicating kfree in error path confuses you, I can 
change
it to rollback path you said. Below are 2 options in my mind:

1. add kfree to rollback path;
2. skip this patch, just apply 1/2 patch, and let me find some time to change 
GPT
driver using timer-of. 

Which one do you prefer? Thanks.

Anson.

> 
> 
> 
> --
> 
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Fdata=02%7C01%7Canson.huang%40nxp.com%7Cc675bb
> b2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636770919692475408sdata=pHaN%2BBguQo2lmVJzLYXBJ
> 12l6DwX%2BxSXo9Bil9SwYxs%3Dreserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .facebook.com%2Fpages%2FLinarodata=02%7C01%7Canson.huang%40
> nxp.com%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C636770919692475408sdata=EgyadNU
> A1XLncN5p90lkIIiiMgQ0CIO405biLpKszvE%3Dreserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt
> er.com%2F%23!%2Flinaroorgdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636770919692475408sdata=NIgQhAPZBh9
> jKcrvNrEY005pZpLZP8pWIVlKXslQ%2B5U%3Dreserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Flinaro-blog%2Fdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636770919692475408sdata=MCBh9dJbjN
> GtK6Vpsj3EfrF7OGBRAawA3jkCmZpgm0I%3Dreserved=0> Blog



RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-06 Thread Anson Huang
Hi, Daniel

Best Regards!
Anson Huang

> -Original Message-
> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> Sent: 2018年11月6日 17:06
> To: Anson Huang ; t...@linutronix.de;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid
> resource leak
> 
> 
> Hi Anson,
> 
> On 06/11/2018 02:38, Anson Huang wrote:
> > Hi, Daniel
> >
> > Best Regards!
> > Anson Huang
> >
> >> -Original Message-
> >> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> >> Sent: 2018年11月5日 21:35
> >> To: Anson Huang ; t...@linutronix.de;
> >> linux-kernel@vger.kernel.org
> >> Cc: dl-linux-imx 
> >> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree
> >> to avoid resource leak
> >>
> >> Hi Anson,
> >>
> >> On 05/11/2018 02:10, Anson Huang wrote:
> >>> kfree should be called to free resource in error path before return.
> >>>
> >>> Signed-off-by: Anson Huang 
> >>> ---
> >>>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/clocksource/timer-imx-gpt.c
> >>> b/drivers/clocksource/timer-imx-gpt.c
> >>> index a3d6ccb..0f78b30 100644
> >>> --- a/drivers/clocksource/timer-imx-gpt.c
> >>> +++ b/drivers/clocksource/timer-imx-gpt.c
> >>> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
> >> device_node *np,  enum imx_gpt_type t
> >>>   return -ENOMEM;
> >>>
> >>>   imxtm->base = of_iomap(np, 0);
> >>> - if (!imxtm->base)
> >>> + if (!imxtm->base) {
> >>> + kfree(imxtm);
> >>>   return -ENXIO;
> >>> + }
> >>>
> >>>   imxtm->irq = irq_of_parse_and_map(np, 0);
> >>> - if (imxtm->irq <= 0)
> >>> + if (imxtm->irq <= 0) {
> >>> + kfree(imxtm);
> >>>   return -EINVAL;
> >>> + }
> >>>
> >>>   imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> >>
> >> Please convert to timer-of API.
> >
> > This patch mainly to fix the potential resource leak issue, for
> > converting to timer-of API, should I add another patch for it? Since
> > it is a different subject.
> 
> Actually I was unclear. The patch is duplicating the kfree in the error path,
> usually a rollback routine should be provided.
> 
> So instead of creating this rollback path, you can directly use the timer-of
> which contains the error check and rollback.

I understand the timer-of can save some code in timer driver, but current GPT
driver is kind of complicated in order to support so many i.MX platforms with
different GPT design, change it to timer-of will need some effort and I think we
can do it later. If the duplicating kfree in error path confuses you, I can 
change
it to rollback path you said. Below are 2 options in my mind:

1. add kfree to rollback path;
2. skip this patch, just apply 1/2 patch, and let me find some time to change 
GPT
driver using timer-of. 

Which one do you prefer? Thanks.

Anson.

> 
> 
> 
> --
> 
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Fdata=02%7C01%7Canson.huang%40nxp.com%7Cc675bb
> b2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636770919692475408sdata=pHaN%2BBguQo2lmVJzLYXBJ
> 12l6DwX%2BxSXo9Bil9SwYxs%3Dreserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .facebook.com%2Fpages%2FLinarodata=02%7C01%7Canson.huang%40
> nxp.com%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C636770919692475408sdata=EgyadNU
> A1XLncN5p90lkIIiiMgQ0CIO405biLpKszvE%3Dreserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt
> er.com%2F%23!%2Flinaroorgdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636770919692475408sdata=NIgQhAPZBh9
> jKcrvNrEY005pZpLZP8pWIVlKXslQ%2B5U%3Dreserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Flinaro-blog%2Fdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636770919692475408sdata=MCBh9dJbjN
> GtK6Vpsj3EfrF7OGBRAawA3jkCmZpgm0I%3Dreserved=0> Blog



Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-06 Thread Daniel Lezcano


Hi Anson,

On 06/11/2018 02:38, Anson Huang wrote:
> Hi, Daniel
> 
> Best Regards!
> Anson Huang
> 
>> -Original Message-
>> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
>> Sent: 2018年11月5日 21:35
>> To: Anson Huang ; t...@linutronix.de;
>> linux-kernel@vger.kernel.org
>> Cc: dl-linux-imx 
>> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to 
>> avoid
>> resource leak
>>
>> Hi Anson,
>>
>> On 05/11/2018 02:10, Anson Huang wrote:
>>> kfree should be called to free resource in error path before return.
>>>
>>> Signed-off-by: Anson Huang 
>>> ---
>>>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/timer-imx-gpt.c
>>> b/drivers/clocksource/timer-imx-gpt.c
>>> index a3d6ccb..0f78b30 100644
>>> --- a/drivers/clocksource/timer-imx-gpt.c
>>> +++ b/drivers/clocksource/timer-imx-gpt.c
>>> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
>> device_node *np,  enum imx_gpt_type t
>>> return -ENOMEM;
>>>
>>> imxtm->base = of_iomap(np, 0);
>>> -   if (!imxtm->base)
>>> +   if (!imxtm->base) {
>>> +   kfree(imxtm);
>>> return -ENXIO;
>>> +   }
>>>
>>> imxtm->irq = irq_of_parse_and_map(np, 0);
>>> -   if (imxtm->irq <= 0)
>>> +   if (imxtm->irq <= 0) {
>>> +   kfree(imxtm);
>>> return -EINVAL;
>>> +   }
>>>
>>> imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
>>
>> Please convert to timer-of API.
> 
> This patch mainly to fix the potential resource leak issue, for converting
> to timer-of API, should I add another patch for it? Since it is a different
> subject.

Actually I was unclear. The patch is duplicating the kfree in the error
path, usually a rollback routine should be provided.

So instead of creating this rollback path, you can directly use the
timer-of which contains the error check and rollback.



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



Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-06 Thread Daniel Lezcano


Hi Anson,

On 06/11/2018 02:38, Anson Huang wrote:
> Hi, Daniel
> 
> Best Regards!
> Anson Huang
> 
>> -Original Message-
>> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
>> Sent: 2018年11月5日 21:35
>> To: Anson Huang ; t...@linutronix.de;
>> linux-kernel@vger.kernel.org
>> Cc: dl-linux-imx 
>> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to 
>> avoid
>> resource leak
>>
>> Hi Anson,
>>
>> On 05/11/2018 02:10, Anson Huang wrote:
>>> kfree should be called to free resource in error path before return.
>>>
>>> Signed-off-by: Anson Huang 
>>> ---
>>>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/timer-imx-gpt.c
>>> b/drivers/clocksource/timer-imx-gpt.c
>>> index a3d6ccb..0f78b30 100644
>>> --- a/drivers/clocksource/timer-imx-gpt.c
>>> +++ b/drivers/clocksource/timer-imx-gpt.c
>>> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
>> device_node *np,  enum imx_gpt_type t
>>> return -ENOMEM;
>>>
>>> imxtm->base = of_iomap(np, 0);
>>> -   if (!imxtm->base)
>>> +   if (!imxtm->base) {
>>> +   kfree(imxtm);
>>> return -ENXIO;
>>> +   }
>>>
>>> imxtm->irq = irq_of_parse_and_map(np, 0);
>>> -   if (imxtm->irq <= 0)
>>> +   if (imxtm->irq <= 0) {
>>> +   kfree(imxtm);
>>> return -EINVAL;
>>> +   }
>>>
>>> imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
>>
>> Please convert to timer-of API.
> 
> This patch mainly to fix the potential resource leak issue, for converting
> to timer-of API, should I add another patch for it? Since it is a different
> subject.

Actually I was unclear. The patch is duplicating the kfree in the error
path, usually a rollback routine should be provided.

So instead of creating this rollback path, you can directly use the
timer-of which contains the error check and rollback.



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



RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-05 Thread Anson Huang
Hi, Daniel

Best Regards!
Anson Huang

> -Original Message-
> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> Sent: 2018年11月5日 21:35
> To: Anson Huang ; t...@linutronix.de;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid
> resource leak
> 
> Hi Anson,
> 
> On 05/11/2018 02:10, Anson Huang wrote:
> > kfree should be called to free resource in error path before return.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  drivers/clocksource/timer-imx-gpt.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-imx-gpt.c
> > b/drivers/clocksource/timer-imx-gpt.c
> > index a3d6ccb..0f78b30 100644
> > --- a/drivers/clocksource/timer-imx-gpt.c
> > +++ b/drivers/clocksource/timer-imx-gpt.c
> > @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
> device_node *np,  enum imx_gpt_type t
> > return -ENOMEM;
> >
> > imxtm->base = of_iomap(np, 0);
> > -   if (!imxtm->base)
> > +   if (!imxtm->base) {
> > +   kfree(imxtm);
> > return -ENXIO;
> > +   }
> >
> > imxtm->irq = irq_of_parse_and_map(np, 0);
> > -   if (imxtm->irq <= 0)
> > +   if (imxtm->irq <= 0) {
> > +   kfree(imxtm);
> > return -EINVAL;
> > +   }
> >
> > imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> 
> Please convert to timer-of API.

This patch mainly to fix the potential resource leak issue, for converting
to timer-of API, should I add another patch for it? Since it is a different
subject.

Thanks.

Anson

> 
> Thanks
> 
>   -- Daniel
> 
> 
> --
> 
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Fdata=02%7C01%7Canson.huang%40nxp.com%7Cb389e6
> 71eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C636770216874548293sdata=toynTer%2Bq7DLRizwW7JKY
> qCrA8WQHeDmJRwLjXUu6XY%3Dreserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .facebook.com%2Fpages%2FLinarodata=02%7C01%7Canson.huang%40
> nxp.com%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C0%7C636770216874548293sdata=JJOClhL
> pER5JMuXHs65fJD%2FPC18%2FJBCyiRw3d41ei48%3Dreserved=0>
> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt
> er.com%2F%23!%2Flinaroorgdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C636770216874548293sdata=W1QXRwgXt
> e3wbbjETuDinUniIDeA8THW%2Bsz6SSPWl30%3Dreserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Flinaro-blog%2Fdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C636770216874548293sdata=9JdVMOpdiI
> HdxVrFbiDgblAunod9RYkzwYRaHcsgzUo%3Dreserved=0> Blog



RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-05 Thread Anson Huang
Hi, Daniel

Best Regards!
Anson Huang

> -Original Message-
> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> Sent: 2018年11月5日 21:35
> To: Anson Huang ; t...@linutronix.de;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx 
> Subject: Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid
> resource leak
> 
> Hi Anson,
> 
> On 05/11/2018 02:10, Anson Huang wrote:
> > kfree should be called to free resource in error path before return.
> >
> > Signed-off-by: Anson Huang 
> > ---
> >  drivers/clocksource/timer-imx-gpt.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-imx-gpt.c
> > b/drivers/clocksource/timer-imx-gpt.c
> > index a3d6ccb..0f78b30 100644
> > --- a/drivers/clocksource/timer-imx-gpt.c
> > +++ b/drivers/clocksource/timer-imx-gpt.c
> > @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct
> device_node *np,  enum imx_gpt_type t
> > return -ENOMEM;
> >
> > imxtm->base = of_iomap(np, 0);
> > -   if (!imxtm->base)
> > +   if (!imxtm->base) {
> > +   kfree(imxtm);
> > return -ENXIO;
> > +   }
> >
> > imxtm->irq = irq_of_parse_and_map(np, 0);
> > -   if (imxtm->irq <= 0)
> > +   if (imxtm->irq <= 0) {
> > +   kfree(imxtm);
> > return -EINVAL;
> > +   }
> >
> > imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> 
> Please convert to timer-of API.

This patch mainly to fix the potential resource leak issue, for converting
to timer-of API, should I add another patch for it? Since it is a different
subject.

Thanks.

Anson

> 
> Thanks
> 
>   -- Daniel
> 
> 
> --
> 
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Fdata=02%7C01%7Canson.huang%40nxp.com%7Cb389e6
> 71eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C636770216874548293sdata=toynTer%2Bq7DLRizwW7JKY
> qCrA8WQHeDmJRwLjXUu6XY%3Dreserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .facebook.com%2Fpages%2FLinarodata=02%7C01%7Canson.huang%40
> nxp.com%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C0%7C636770216874548293sdata=JJOClhL
> pER5JMuXHs65fJD%2FPC18%2FJBCyiRw3d41ei48%3Dreserved=0>
> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt
> er.com%2F%23!%2Flinaroorgdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C636770216874548293sdata=W1QXRwgXt
> e3wbbjETuDinUniIDeA8THW%2Bsz6SSPWl30%3Dreserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .linaro.org%2Flinaro-blog%2Fdata=02%7C01%7Canson.huang%40nxp.c
> om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C636770216874548293sdata=9JdVMOpdiI
> HdxVrFbiDgblAunod9RYkzwYRaHcsgzUo%3Dreserved=0> Blog



Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-05 Thread Daniel Lezcano
Hi Anson,

On 05/11/2018 02:10, Anson Huang wrote:
> kfree should be called to free resource in error path
> before return.
> 
> Signed-off-by: Anson Huang 
> ---
>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-imx-gpt.c 
> b/drivers/clocksource/timer-imx-gpt.c
> index a3d6ccb..0f78b30 100644
> --- a/drivers/clocksource/timer-imx-gpt.c
> +++ b/drivers/clocksource/timer-imx-gpt.c
> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct device_node 
> *np,  enum imx_gpt_type t
>   return -ENOMEM;
>  
>   imxtm->base = of_iomap(np, 0);
> - if (!imxtm->base)
> + if (!imxtm->base) {
> + kfree(imxtm);
>   return -ENXIO;
> + }
>  
>   imxtm->irq = irq_of_parse_and_map(np, 0);
> - if (imxtm->irq <= 0)
> + if (imxtm->irq <= 0) {
> + kfree(imxtm);
>   return -EINVAL;
> + }
>  
>   imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");

Please convert to timer-of API.

Thanks

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-05 Thread Daniel Lezcano
Hi Anson,

On 05/11/2018 02:10, Anson Huang wrote:
> kfree should be called to free resource in error path
> before return.
> 
> Signed-off-by: Anson Huang 
> ---
>  drivers/clocksource/timer-imx-gpt.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-imx-gpt.c 
> b/drivers/clocksource/timer-imx-gpt.c
> index a3d6ccb..0f78b30 100644
> --- a/drivers/clocksource/timer-imx-gpt.c
> +++ b/drivers/clocksource/timer-imx-gpt.c
> @@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct device_node 
> *np,  enum imx_gpt_type t
>   return -ENOMEM;
>  
>   imxtm->base = of_iomap(np, 0);
> - if (!imxtm->base)
> + if (!imxtm->base) {
> + kfree(imxtm);
>   return -ENXIO;
> + }
>  
>   imxtm->irq = irq_of_parse_and_map(np, 0);
> - if (imxtm->irq <= 0)
> + if (imxtm->irq <= 0) {
> + kfree(imxtm);
>   return -EINVAL;
> + }
>  
>   imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");

Please convert to timer-of API.

Thanks

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog



[PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-04 Thread Anson Huang
kfree should be called to free resource in error path
before return.

Signed-off-by: Anson Huang 
---
 drivers/clocksource/timer-imx-gpt.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-imx-gpt.c 
b/drivers/clocksource/timer-imx-gpt.c
index a3d6ccb..0f78b30 100644
--- a/drivers/clocksource/timer-imx-gpt.c
+++ b/drivers/clocksource/timer-imx-gpt.c
@@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct device_node 
*np,  enum imx_gpt_type t
return -ENOMEM;
 
imxtm->base = of_iomap(np, 0);
-   if (!imxtm->base)
+   if (!imxtm->base) {
+   kfree(imxtm);
return -ENXIO;
+   }
 
imxtm->irq = irq_of_parse_and_map(np, 0);
-   if (imxtm->irq <= 0)
+   if (imxtm->irq <= 0) {
+   kfree(imxtm);
return -EINVAL;
+   }
 
imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
 
-- 
2.7.4



[PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

2018-11-04 Thread Anson Huang
kfree should be called to free resource in error path
before return.

Signed-off-by: Anson Huang 
---
 drivers/clocksource/timer-imx-gpt.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-imx-gpt.c 
b/drivers/clocksource/timer-imx-gpt.c
index a3d6ccb..0f78b30 100644
--- a/drivers/clocksource/timer-imx-gpt.c
+++ b/drivers/clocksource/timer-imx-gpt.c
@@ -477,12 +477,16 @@ static int __init mxc_timer_init_dt(struct device_node 
*np,  enum imx_gpt_type t
return -ENOMEM;
 
imxtm->base = of_iomap(np, 0);
-   if (!imxtm->base)
+   if (!imxtm->base) {
+   kfree(imxtm);
return -ENXIO;
+   }
 
imxtm->irq = irq_of_parse_and_map(np, 0);
-   if (imxtm->irq <= 0)
+   if (imxtm->irq <= 0) {
+   kfree(imxtm);
return -EINVAL;
+   }
 
imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
 
-- 
2.7.4