From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752361AbdGaO3S (ORCPT ); Mon, 31 Jul 2017 10:29:18 -0400 Received: from mail-wr0-f169.google.com ([209.85.128.169]:37051 "EHLO mail-wr0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751971AbdGaO3R (ORCPT ); Mon, 31 Jul 2017 10:29:17 -0400 Subject: Re: [PATCH V4 2/2] timer: imx-tpm: add imx tpm timer support To: Dong Aisheng , linux-kernel@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org, tglx@linutronix.de, shawnguo@kernel.org, ping.bai@nxp.com, anson.huang@nxp.com, dongas86@gmail.com, kernel@pengutronix.de, Arnd Bergmann References: <1499222112-9988-1-git-send-email-aisheng.dong@nxp.com> <1499222112-9988-3-git-send-email-aisheng.dong@nxp.com> From: Daniel Lezcano Message-ID: Date: Mon, 31 Jul 2017 16:29:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1499222112-9988-3-git-send-email-aisheng.dong@nxp.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/07/2017 04:35, Dong Aisheng wrote: > IMX Timer/PWM Module (TPM) supports both timer and pwm function while > this patch only adds the timer support. PWM would be added later. > > The TPM counter, compare and capture registers are clocked by an > asynchronous clock that can remain enabled in low power modes. > > NOTE: We observed in a very small probability, the bus fabric > contention between GPU and A7 may results a few cycles delay > of writing CNT registers which may cause the min_delta event got > missed, so we need add a ETIME check here in case it happened. > > Cc: Daniel Lezcano > Cc: Arnd Bergmann > Cc: Thomas Gleixner > Cc: Shawn Guo > Cc: Anson Huang > Cc: Bai Ping > Signed-off-by: Dong Aisheng > > --- > ChangeLog: > v3->v4: > * also add ETIME explanation in function > v2->v3: > * address all comments from Daniel Lezcano > * add more explaination on ETIME check in commit message > v1->v2: > * change to readl/writel from __raw_readl/writel according to Arnd's > suggestion to avoid endian issue > * add help information in Kconfig > * add more error checking > --- > drivers/clocksource/Kconfig | 8 ++ > drivers/clocksource/Makefile | 1 + > drivers/clocksource/timer-imx-tpm.c | 239 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 248 insertions(+) > create mode 100644 drivers/clocksource/timer-imx-tpm.c [ ... ] > +static struct irqaction tpm_timer_irq = { > + .name = "i.MX7ULP TPM Timer", > + .flags = IRQF_TIMER | IRQF_IRQPOLL, > + .handler = tpm_timer_interrupt, > + .dev_id = &clockevent_tpm, > +}; > Please remove the structure above and use request_irq instead of setup_irq below + return code checking. > +static int __init tpm_clockevent_init(unsigned long rate, int irq) > +{ > + setup_irq(irq, &tpm_timer_irq); > + > + clockevent_tpm.cpumask = cpumask_of(0); > + clockevent_tpm.irq = irq; > + clockevents_config_and_register(&clockevent_tpm, > + rate, 300, 0xfffffffe); > + > + return 0; > +} > + > +static int __init tpm_timer_init(struct device_node *np) > +{ [ ... ] > + rate = clk_get_rate(per) >> 3; Why ? > + tpm_clocksource_init(rate); > + tpm_clockevent_init(rate, irq); Check. > + return 0; > + > +err_per_clk_enable: > + clk_disable_unprepare(ipg); > +err_ipg_clk_enable: No need to add an extra label. > +err_clk_get: > + clk_put(per); > + clk_put(ipg); > +err_iomap: > + iounmap(timer_base); > + return ret; > +} > +CLOCKSOURCE_OF_DECLARE(imx7ulp, "fsl,imx7ulp-tpm", tpm_timer_init); CLOCKSOURCE_OF_DECLARE is renamed to TIMER_OF_DECLARE. Thanks! -- Daniel -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog