From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762689AbdJRIKb (ORCPT ); Wed, 18 Oct 2017 04:10:31 -0400 Received: from foss.arm.com ([217.140.101.70]:35210 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757197AbdJRIK1 (ORCPT ); Wed, 18 Oct 2017 04:10:27 -0400 Subject: Re: [PATCH v5 1/4] clocksource: stm32: convert driver to timer_of To: Benjamin Gaignard , robh+dt@kernel.org, mark.rutland@arm.com, linux@armlinux.org.uk, mcoquelin.stm32@gmail.com, alexandre.torgue@st.com, daniel.lezcano@linaro.org, tglx@linutronix.de, ludovic.barre@st.com Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1508312614-27750-1-git-send-email-benjamin.gaignard@linaro.org> <1508312614-27750-2-git-send-email-benjamin.gaignard@linaro.org> From: Julien Thierry Message-ID: <8919cadc-2e51-4b31-2836-32c45bbfabfe@arm.com> Date: Wed, 18 Oct 2017 09:10:22 +0100 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: <1508312614-27750-2-git-send-email-benjamin.gaignard@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Benjamin, On 18/10/17 08:43, Benjamin Gaignard wrote: > Convert driver to use timer_of helpers. This allow to remove > custom proprietary structure. > > Increase min delta value because if it is too small it could > generate too much interrupts and the system will not be able > to catch them all. > > Signed-off-by: Benjamin Gaignard > --- > drivers/clocksource/Kconfig | 1 + > drivers/clocksource/timer-stm32.c | 162 +++++++++++++------------------------- > 2 files changed, 57 insertions(+), 106 deletions(-) > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index cc60620..755c0cc 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -289,6 +289,7 @@ config CLKSRC_STM32 > bool "Clocksource for STM32 SoCs" if !ARCH_STM32 > depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) > select CLKSRC_MMIO > + select TIMER_OF > > config CLKSRC_MPS2 > bool "Clocksource for MPS2 SoCs" if COMPILE_TEST > diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c > index 8f24237..abff21c 100644 > --- a/drivers/clocksource/timer-stm32.c > +++ b/drivers/clocksource/timer-stm32.c [...] > - > -static int __init stm32_clockevent_init(struct device_node *np) > +static int __init stm32_clockevent_init(struct device_node *node) > { > - struct stm32_clock_event_ddata *data = &clock_event_ddata; > - struct clk *clk; > struct reset_control *rstc; > - unsigned long rate, max_delta; > - int irq, ret, bits, prescaler = 1; > - > - clk = of_clk_get(np, 0); > - if (IS_ERR(clk)) { > - ret = PTR_ERR(clk); > - pr_err("failed to get clock for clockevent (%d)\n", ret); > - goto err_clk_get; > - } > - > - ret = clk_prepare_enable(clk); > - if (ret) { > - pr_err("failed to enable timer clock for clockevent (%d)\n", > - ret); > - goto err_clk_enable; > - } > - > - rate = clk_get_rate(clk); > - > - rstc = of_reset_control_get(np, NULL); > + unsigned long max_delta; > + int ret, bits, prescaler = 1; > + struct timer_of *to; > + > + to = kzalloc(sizeof(*to), GFP_KERNEL); > + if (!to) > + return -ENOMEM; > + > + to->flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE; > + to->clkevt.name = "stm32_clockevent"; > + to->clkevt.rating = 200; > + to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC; > + to->clkevt.set_state_shutdown = stm32_clock_event_shutdown; > + to->clkevt.set_state_periodic = stm32_clock_event_set_periodic; > + to->clkevt.set_state_oneshot = stm32_clock_event_shutdown; > + to->clkevt.tick_resume = stm32_clock_event_shutdown; > + to->clkevt.set_next_event = stm32_clock_event_set_next_event; > + > + to->of_irq.handler = stm32_clock_event_handler; > + > + ret = timer_of_init(node, to); > + if (ret) > + return ret; I don't know if we should keep some "err_*" labels at the end of the function, but here there probably should a "kfree(to)" before returning. Otherwise the patch looks fine to me. Cheers, -- Julien Thierry