* [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 @ 2018-11-05 1:10 Anson Huang 2018-11-05 1:10 ` [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak Anson Huang 2018-11-18 1:54 ` [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Daniel Lezcano 0 siblings, 2 replies; 8+ messages in thread From: Anson Huang @ 2018-11-05 1:10 UTC (permalink / raw) To: daniel.lezcano, tglx, linux-kernel; +Cc: dl-linux-imx This patch allows building and compile-testing the i.MX GPT driver also for ARM64. The delay_timer is only supported on ARMv7. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- no change since V1. drivers/clocksource/Kconfig | 2 +- drivers/clocksource/timer-imx-gpt.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 55c77e4..5e892ca 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -580,7 +580,7 @@ config H8300_TPU config CLKSRC_IMX_GPT bool "Clocksource using i.MX GPT" if COMPILE_TEST - depends on ARM && CLKDEV_LOOKUP + depends on (ARM || ARM64) && CLKDEV_LOOKUP select CLKSRC_MMIO config CLKSRC_IMX_TPM diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c index 165fbbb..a3d6ccb 100644 --- a/drivers/clocksource/timer-imx-gpt.c +++ b/drivers/clocksource/timer-imx-gpt.c @@ -141,21 +141,25 @@ static u64 notrace mxc_read_sched_clock(void) return sched_clock_reg ? readl_relaxed(sched_clock_reg) : 0; } +#if defined(CONFIG_ARM) static struct delay_timer imx_delay_timer; static unsigned long imx_read_current_timer(void) { return readl_relaxed(sched_clock_reg); } +#endif static int __init mxc_clocksource_init(struct imx_timer *imxtm) { unsigned int c = clk_get_rate(imxtm->clk_per); void __iomem *reg = imxtm->base + imxtm->gpt->reg_tcn; +#if defined(CONFIG_ARM) imx_delay_timer.read_current_timer = &imx_read_current_timer; imx_delay_timer.freq = c; register_current_timer_delay(&imx_delay_timer); +#endif sched_clock_reg = reg; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-05 1:10 [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Anson Huang @ 2018-11-05 1:10 ` Anson Huang 2018-11-05 13:34 ` Daniel Lezcano 2018-11-18 1:54 ` [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Daniel Lezcano 1 sibling, 1 reply; 8+ messages in thread From: Anson Huang @ 2018-11-05 1:10 UTC (permalink / raw) To: daniel.lezcano, tglx, linux-kernel; +Cc: dl-linux-imx kfree should be called to free resource in error path before return. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-05 1:10 ` [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak Anson Huang @ 2018-11-05 13:34 ` Daniel Lezcano 2018-11-06 1:38 ` Anson Huang 0 siblings, 1 reply; 8+ messages in thread From: Daniel Lezcano @ 2018-11-05 13:34 UTC (permalink / raw) To: Anson Huang, tglx, linux-kernel; +Cc: dl-linux-imx 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 <Anson.Huang@nxp.com> > --- > 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 -- <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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-05 13:34 ` Daniel Lezcano @ 2018-11-06 1:38 ` Anson Huang 2018-11-06 9:06 ` Daniel Lezcano 0 siblings, 1 reply; 8+ messages in thread From: Anson Huang @ 2018-11-06 1:38 UTC (permalink / raw) To: Daniel Lezcano, tglx, linux-kernel; +Cc: dl-linux-imx Hi, Daniel Best Regards! Anson Huang > -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] > Sent: 2018年11月5日 21:35 > To: Anson Huang <anson.huang@nxp.com>; tglx@linutronix.de; > linux-kernel@vger.kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com> > 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 <Anson.Huang@nxp.com> > > --- > > 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%2F&data=02%7C01%7Canson.huang%40nxp.com%7Cb389e6 > 71eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd99c5c301635% > 7C0%7C0%7C636770216874548293&sdata=toynTer%2Bq7DLRizwW7JKY > qCrA8WQHeDmJRwLjXUu6XY%3D&reserved=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%2FLinaro&data=02%7C01%7Canson.huang%40 > nxp.com%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C0%7C636770216874548293&sdata=JJOClhL > pER5JMuXHs65fJD%2FPC18%2FJBCyiRw3d41ei48%3D&reserved=0> > Facebook | > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt > er.com%2F%23!%2Flinaroorg&data=02%7C01%7Canson.huang%40nxp.c > om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd > 99c5c301635%7C0%7C0%7C636770216874548293&sdata=W1QXRwgXt > e3wbbjETuDinUniIDeA8THW%2Bsz6SSPWl30%3D&reserved=0> Twitter | > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > .linaro.org%2Flinaro-blog%2F&data=02%7C01%7Canson.huang%40nxp.c > om%7Cb389e671eafb4400148208d643237383%7C686ea1d3bc2b4c6fa92cd > 99c5c301635%7C0%7C0%7C636770216874548293&sdata=9JdVMOpdiI > HdxVrFbiDgblAunod9RYkzwYRaHcsgzUo%3D&reserved=0> Blog ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-06 1:38 ` Anson Huang @ 2018-11-06 9:06 ` Daniel Lezcano 2018-11-06 9:12 ` Anson Huang 0 siblings, 1 reply; 8+ messages in thread From: Daniel Lezcano @ 2018-11-06 9:06 UTC (permalink / raw) To: Anson Huang, tglx, linux-kernel; +Cc: dl-linux-imx Hi Anson, On 06/11/2018 02:38, Anson Huang wrote: > Hi, Daniel > > Best Regards! > Anson Huang > >> -----Original Message----- >> From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] >> Sent: 2018年11月5日 21:35 >> To: Anson Huang <anson.huang@nxp.com>; tglx@linutronix.de; >> linux-kernel@vger.kernel.org >> Cc: dl-linux-imx <linux-imx@nxp.com> >> 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 <Anson.Huang@nxp.com> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-06 9:06 ` Daniel Lezcano @ 2018-11-06 9:12 ` Anson Huang 2018-11-18 1:52 ` Daniel Lezcano 0 siblings, 1 reply; 8+ messages in thread From: Anson Huang @ 2018-11-06 9:12 UTC (permalink / raw) To: Daniel Lezcano, tglx, linux-kernel; +Cc: dl-linux-imx Hi, Daniel Best Regards! Anson Huang > -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] > Sent: 2018年11月6日 17:06 > To: Anson Huang <anson.huang@nxp.com>; tglx@linutronix.de; > linux-kernel@vger.kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com> > 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.lezcano@linaro.org] > >> Sent: 2018年11月5日 21:35 > >> To: Anson Huang <anson.huang@nxp.com>; tglx@linutronix.de; > >> linux-kernel@vger.kernel.org > >> Cc: dl-linux-imx <linux-imx@nxp.com> > >> 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 <Anson.Huang@nxp.com> > >>> --- > >>> 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%2F&data=02%7C01%7Canson.huang%40nxp.com%7Cc675bb > b2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 > C0%7C0%7C636770919692475408&sdata=pHaN%2BBguQo2lmVJzLYXBJ > 12l6DwX%2BxSXo9Bil9SwYxs%3D&reserved=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%2FLinaro&data=02%7C01%7Canson.huang%40 > nxp.com%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa9 > 2cd99c5c301635%7C0%7C0%7C636770919692475408&sdata=EgyadNU > A1XLncN5p90lkIIiiMgQ0CIO405biLpKszvE%3D&reserved=0> Facebook | > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitt > er.com%2F%23!%2Flinaroorg&data=02%7C01%7Canson.huang%40nxp.c > om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9 > 9c5c301635%7C0%7C0%7C636770919692475408&sdata=NIgQhAPZBh9 > jKcrvNrEY005pZpLZP8pWIVlKXslQ%2B5U%3D&reserved=0> Twitter | > <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > .linaro.org%2Flinaro-blog%2F&data=02%7C01%7Canson.huang%40nxp.c > om%7Cc675bbb2063b43c3be1508d643c715f3%7C686ea1d3bc2b4c6fa92cd9 > 9c5c301635%7C0%7C0%7C636770919692475408&sdata=MCBh9dJbjN > GtK6Vpsj3EfrF7OGBRAawA3jkCmZpgm0I%3D&reserved=0> Blog ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak 2018-11-06 9:12 ` Anson Huang @ 2018-11-18 1:52 ` Daniel Lezcano 0 siblings, 0 replies; 8+ messages in thread From: Daniel Lezcano @ 2018-11-18 1:52 UTC (permalink / raw) To: Anson Huang, tglx, linux-kernel; +Cc: dl-linux-imx 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 -- <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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 2018-11-05 1:10 [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Anson Huang 2018-11-05 1:10 ` [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak Anson Huang @ 2018-11-18 1:54 ` Daniel Lezcano 1 sibling, 0 replies; 8+ messages in thread From: Daniel Lezcano @ 2018-11-18 1:54 UTC (permalink / raw) To: Anson Huang, tglx, linux-kernel; +Cc: dl-linux-imx On 05/11/2018 02:10, Anson Huang wrote: > This patch allows building and compile-testing the i.MX > GPT driver also for ARM64. The delay_timer is only > supported on ARMv7. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > no change since V1. Applied, thanks. -- Daniel -- <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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-18 2:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-05 1:10 [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Anson Huang 2018-11-05 1:10 ` [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak Anson Huang 2018-11-05 13:34 ` Daniel Lezcano 2018-11-06 1:38 ` Anson Huang 2018-11-06 9:06 ` Daniel Lezcano 2018-11-06 9:12 ` Anson Huang 2018-11-18 1:52 ` Daniel Lezcano 2018-11-18 1:54 ` [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64 Daniel Lezcano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).