From patchwork Wed Jul 1 11:44:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Osmialowski X-Patchwork-Id: 6702911 Return-Path: X-Original-To: patchwork-dmaengine@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C72AFC05AC for ; Wed, 1 Jul 2015 11:48:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 92FF220386 for ; Wed, 1 Jul 2015 11:48:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 12B69201F5 for ; Wed, 1 Jul 2015 11:48:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751293AbbGALsR (ORCPT ); Wed, 1 Jul 2015 07:48:17 -0400 Received: from fish.king.net.pl ([79.190.246.46]:57859 "EHLO king.net.pl" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750935AbbGALsP (ORCPT ); Wed, 1 Jul 2015 07:48:15 -0400 Received: from localhost.localdomain (fish [127.0.0.1]) by king.net.pl (8.14.9/8.14.0) with ESMTP id t61BitRL011534 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 1 Jul 2015 13:44:56 +0200 Received: from localhost (newchief@localhost) by localhost.localdomain (8.14.9/8.14.9/Submit) with ESMTP id t61BikgN011529; Wed, 1 Jul 2015 13:44:47 +0200 X-Authentication-Warning: localhost.localdomain: newchief owned process doing -bs Date: Wed, 1 Jul 2015 13:44:46 +0200 (CEST) From: Paul Osmialowski X-X-Sender: newchief@localhost.localdomain To: Arnd Bergmann cc: linux-arm-kernel@lists.infradead.org, Paul Osmialowski , Greg Kroah-Hartman , Ian Campbell , Jiri Slaby , Kumar Gala , Linus Walleij , Mark Rutland , Michael Turquette , Pawel Moll , Rob Herring , Russell King , Stephen Boyd , Vinod Koul , linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org, linux-serial@vger.kernel.org, devicetree@vger.kernel.org, dmaengine@vger.kernel.org, Nicolas Pitre , Sergei Poselenov , Paul Bolle , Jingchang Lu , Yuri Tikhonov , Rob Herring , Geert Uytterhoeven , Uwe Kleine-Koenig , Alexander Potashev , Frank Li , Thomas Gleixner , Anson Huang Subject: Re: [PATCH v2 4/9] arm: twr-k70f120m: timer driver for Kinetis SoC In-Reply-To: <2939807.q7WvOoYzDX@wuerfel> Message-ID: References: <1435667250-28299-1-git-send-email-pawelo@king.net.pl> <1435667250-28299-5-git-send-email-pawelo@king.net.pl> <2939807.q7WvOoYzDX@wuerfel> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Arnd, Again, thanks for your remarks. I'm attaching timer patch candidate for the third iteration. Following your advices, I've changed following things: - not abusing aliases (same goes to pinctrl driver) - ranges for addressing particular timers (no change in code though, it's just up to the .dts implementor) - *_RD and *_WR macros removed; whole this big-endian issue was a mistake, I guess I overdid it a bit trying to make my drivers as universal as fsl-edma driver... - *_SET and *_RESET macros removed - they were giving false sense of security hiding potential race. Any comments are welcome. On Tue, 30 Jun 2015, Arnd Bergmann wrote: > On Tuesday 30 June 2015 14:27:25 Paul Osmialowski wrote: > >> +Example: >> + >> +aliases { >> + pit0 = &pit0; >> + pit1 = &pit1; >> + pit2 = &pit2; >> + pit3 = &pit3; >> +}; >> + >> +pit@40037000 { >> + compatible = "fsl,kinetis-pit-timer"; >> + reg = <0x40037000 0x100>; >> + clocks = <&mcg_pclk_gate 5 23>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; > > All the subnodes seem to fall inside of the device's own register > area, so I think it would be nicer to use a specific 'ranges' > property that only translates the registers in question. > >> / { >> + aliases { >> + pit0 = &pit0; >> + pit1 = &pit1; >> + pit2 = &pit2; >> + pit3 = &pit3; >> + }; >> + >> soc { >> + pit@40037000 { >> + compatible = "fsl,kinetis-pit-timer"; >> + reg = <0x40037000 0x100>; >> + clocks = <&mcg_pclk_gate 5 23>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + >> + pit0: timer@40037100 { >> + reg = <0x40037100 0x10>; >> + interrupts = <68>; >> + status = "disabled"; >> + }; > > I don't think it's necessary to have both an alias > and a label here. What do you use the alias for? > >> + >> +#define KINETIS_PITMCR_PTR(base, reg) \ >> + (&(((struct kinetis_pit_mcr_regs *)(base))->reg)) >> +#define KINETIS_PITMCR_RD(be, base, reg) \ >> + ((be) ? ioread32be(KINETIS_PITMCR_PTR(base, reg)) \ >> + : ioread32(KINETIS_PITMCR_PTR(base, reg))) >> +#define KINETIS_PITMCR_WR(be, base, reg, val) do { \ >> + if (be) \ >> + iowrite32be((val), KINETIS_PITMCR_PTR(base, reg)); \ >> + else \ >> + iowrite32((val), KINETIS_PITMCR_PTR(base, reg)); \ >> + } while (0) > > These should really be written as inline functions. Can you > explain why you need to deal with a big-endian version of this > hardware? Can you configure the endianess of this register block > and just set it to one of the two at boot time? > >> +#define KINETIS_PIT_PTR(base, reg) \ >> + (&(((struct kinetis_pit_channel_regs *)(base))->reg)) >> +#define KINETIS_PIT_RD(be, base, reg) \ >> + ((be) ? ioread32be(KINETIS_PIT_PTR(base, reg)) \ >> + : ioread32(KINETIS_PIT_PTR(base, reg))) >> +#define KINETIS_PIT_WR(be, base, reg, val) do { \ >> + if (be) \ >> + iowrite32be((val), KINETIS_PIT_PTR(base, reg)); \ >> + else \ >> + iowrite32((val), KINETIS_PIT_PTR(base, reg)); \ >> + } while (0) >> +#define KINETIS_PIT_SET(be, base, reg, mask) \ >> + KINETIS_PIT_WR(be, base, reg, \ >> + KINETIS_PIT_RD(be, base, reg) | (mask)) >> +#define KINETIS_PIT_RESET(be, base, reg, mask) \ >> + KINETIS_PIT_WR(be, base, reg, \ >> + KINETIS_PIT_RD(be, base, reg) & (~(mask))) > > > Functions again. Also, just pass a pointer to your own data structure > into the function, instead of the 'be' and 'base' values. > > The 'set' and 'reset' functions look like they need a spinlock > to avoid races. > > Arnd > From 562decc41dc7302c378bf6e4509a22561ff5a85e Mon Sep 17 00:00:00 2001 From: Paul Osmialowski Date: Mon, 29 Jun 2015 21:32:41 +0200 Subject: [PATCH 4/9] arm: twr-k70f120m: timer driver for Kinetis SoC Based on legacy pre-OF code by Alexander Potashev Signed-off-by: Paul Osmialowski --- .../bindings/timer/fsl,kinetis-pit-timer.txt | 43 ++++ arch/arm/Kconfig | 1 + arch/arm/boot/dts/kinetis-twr-k70f120m.dts | 4 + arch/arm/boot/dts/kinetis.dtsi | 33 +++ drivers/clocksource/Kconfig | 5 + drivers/clocksource/Makefile | 1 + drivers/clocksource/timer-kinetis.c | 280 +++++++++++++++++++++ 7 files changed, 367 insertions(+) create mode 100644 Documentation/devicetree/bindings/timer/fsl,kinetis-pit-timer.txt create mode 100644 drivers/clocksource/timer-kinetis.c diff --git a/Documentation/devicetree/bindings/timer/fsl,kinetis-pit-timer.txt b/Documentation/devicetree/bindings/timer/fsl,kinetis-pit-timer.txt new file mode 100644 index 0000000..f2930af --- /dev/null +++ b/Documentation/devicetree/bindings/timer/fsl,kinetis-pit-timer.txt @@ -0,0 +1,43 @@ +Freescale Kinetis SoC Periodic Interrupt Timer (PIT) + +Required properties: + +- compatible: Should be "fsl,kinetis-pit-timer". +- reg: Specifies base physical address and size of the register set for the + Periodic Interrupt Timer. +- clocks: The clock provided by the SoC to drive the timer. +- Set of timer devices: following properties are required for each: + - reg: Specifies base physical address and size of the register set + for given timer device. + - interrupts: Should be the clock event device interrupt. + +Example: + +pit@40037000 { + compatible = "fsl,kinetis-pit-timer"; + reg = <0x40037000 0x200>; + clocks = <&mcg_pclk_gate 5 23>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x40037100 0x100>; + + pit0: timer@00 { + reg = <0x00 0x10>; + interrupts = <68>; + }; + + pit1: timer@10 { + reg = <0x10 0x10>; + interrupts = <69>; + }; + + pit2: timer@20 { + reg = <0x20 0x10>; + interrupts = <70>; + }; + + pit3: timer@30 { + reg = <0x30 0x10>; + interrupts = <71>; + }; +}; diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9c89bdc..96ddaae 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -968,6 +968,7 @@ config ARCH_KINETIS bool "Freescale Kinetis MCU" depends on ARM_SINGLE_ARMV7M select ARMV7M_SYSTICK + select CLKSRC_KINETIS help This enables support for the Freescale Kinetis MCUs diff --git a/arch/arm/boot/dts/kinetis-twr-k70f120m.dts b/arch/arm/boot/dts/kinetis-twr-k70f120m.dts index edccf37..a6efc29 100644 --- a/arch/arm/boot/dts/kinetis-twr-k70f120m.dts +++ b/arch/arm/boot/dts/kinetis-twr-k70f120m.dts @@ -14,3 +14,7 @@ reg = <0x8000000 0x8000000>; }; }; + +&pit0 { + status = "ok"; +}; diff --git a/arch/arm/boot/dts/kinetis.dtsi b/arch/arm/boot/dts/kinetis.dtsi index ae0cf00..74b58cb 100644 --- a/arch/arm/boot/dts/kinetis.dtsi +++ b/arch/arm/boot/dts/kinetis.dtsi @@ -6,6 +6,39 @@ / { soc { + pit@40037000 { + compatible = "fsl,kinetis-pit-timer"; + reg = <0x40037000 0x200>; + clocks = <&mcg_pclk_gate 5 23>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x40037100 0x100>; + + pit0: timer@00 { + reg = <0x00 0x10>; + interrupts = <68>; + status = "disabled"; + }; + + pit1: timer@10 { + reg = <0x10 0x10>; + interrupts = <69>; + status = "disabled"; + }; + + pit2: timer@20 { + reg = <0x20 0x10>; + interrupts = <70>; + status = "disabled"; + }; + + pit3: timer@30 { + reg = <0x30 0x10>; + interrupts = <71>; + status = "disabled"; + }; + }; + cmu@40064000 { compatible = "fsl,kinetis-cmu"; reg = <0x40064000 0x14>, <0x40047000 0x1100>; diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 0f1c77e..d377395 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -106,6 +106,11 @@ config CLKSRC_EFM32 Support to use the timers of EFM32 SoCs as clock source and clock event device. +config CLKSRC_KINETIS + bool "Clocksource for Kinetis SoCs" + depends on OF && ARCH_KINETIS + select CLKSRC_OF + config CLKSRC_LPC32XX bool select CLKSRC_MMIO diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index f1ae0e7..6da77a8 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_ARCH_NSPIRE) += zevio-timer.o obj-$(CONFIG_ARCH_BCM_MOBILE) += bcm_kona_timer.o obj-$(CONFIG_CADENCE_TTC_TIMER) += cadence_ttc_timer.o obj-$(CONFIG_CLKSRC_EFM32) += time-efm32.o +obj-$(CONFIG_CLKSRC_KINETIS) += timer-kinetis.o obj-$(CONFIG_CLKSRC_STM32) += timer-stm32.o obj-$(CONFIG_CLKSRC_EXYNOS_MCT) += exynos_mct.o obj-$(CONFIG_CLKSRC_LPC32XX) += time-lpc32xx.o diff --git a/drivers/clocksource/timer-kinetis.c b/drivers/clocksource/timer-kinetis.c new file mode 100644 index 0000000..1424308 --- /dev/null +++ b/drivers/clocksource/timer-kinetis.c @@ -0,0 +1,280 @@ +/* + * timer-kinetis.c - Timer driver for Kinetis K70 + * + * Based on legacy pre-OF code by Alexander Potashev + * + * Copyright (C) 2015 Paul Osmialowski + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License version 2 as published by the + * Free Software Foundation. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * PIT Timer Control Register + */ +/* Timer Interrupt Enable Bit */ +#define KINETIS_PIT_TCTRL_TIE_MSK (1 << 1) +/* Timer Enable Bit */ +#define KINETIS_PIT_TCTRL_TEN_MSK (1 << 0) +/* + * PIT Timer Flag Register + */ +/* Timer Interrupt Flag */ +#define KINETIS_PIT_TFLG_TIF_MSK (1 << 0) + +struct kinetis_pit_mcr_regs { + u32 mcr; +}; +#define KINETIS_PITMCR_PTR(base, reg) \ + (&(((struct kinetis_pit_mcr_regs *)(base))->reg)) + +/* + * Periodic Interrupt Timer (PIT) registers + */ +struct kinetis_pit_channel_regs { + u32 ldval; /* Timer Load Value Register */ + u32 cval; /* Current Timer Value Register */ + u32 tctrl; /* Timer Control Register */ + u32 tflg; /* Timer Flag Register */ +}; +#define KINETIS_PIT_PTR(base, reg) \ + (&(((struct kinetis_pit_channel_regs *)(base))->reg)) + +struct kinetis_clock_event_ddata { + struct clock_event_device evtdev; + void __iomem *base; + void __iomem *mcr; + spinlock_t lock; +}; + +/* + * Enable or disable a PIT channel + */ +static void kinetis_pit_enable(struct kinetis_clock_event_ddata *tmr, + int enable) +{ + u32 tctrl_val; + unsigned long flags; + + spin_lock_irqsave(&tmr->lock, flags); + + tctrl_val = ioread32(KINETIS_PIT_PTR(tmr->base, tctrl)); + if (enable) + iowrite32(tctrl_val | KINETIS_PIT_TCTRL_TEN_MSK, + KINETIS_PIT_PTR(tmr->base, tctrl)); + else + iowrite32(tctrl_val & ~KINETIS_PIT_TCTRL_TEN_MSK, + KINETIS_PIT_PTR(tmr->base, tctrl)); + + spin_unlock_irqrestore(&tmr->lock, flags); +} + +/* + * Initialize a PIT channel, but do not enable it + */ +static void kinetis_pit_init(struct kinetis_clock_event_ddata *tmr, u32 ticks) +{ + unsigned long flags; + + spin_lock_irqsave(&tmr->lock, flags); + + /* + * Enable the PIT module clock + */ + iowrite32(0, KINETIS_PITMCR_PTR(tmr->mcr, mcr)); + + iowrite32(0, KINETIS_PIT_PTR(tmr->base, tctrl)); + iowrite32(KINETIS_PIT_TFLG_TIF_MSK, KINETIS_PIT_PTR(tmr->base, tflg)); + iowrite32(ticks, KINETIS_PIT_PTR(tmr->base, ldval)); + iowrite32(0, KINETIS_PIT_PTR(tmr->base, cval)); + iowrite32(KINETIS_PIT_TCTRL_TIE_MSK, KINETIS_PIT_PTR(tmr->base, tctrl)); + + spin_unlock_irqrestore(&tmr->lock, flags); +} + +static int kinetis_clockevent_tmr_set_state_periodic( + struct clock_event_device *evt) +{ + struct kinetis_clock_event_ddata *pit = + container_of(evt, struct kinetis_clock_event_ddata, evtdev); + + kinetis_pit_enable(pit, 1); + + return 0; +} + +static int kinetis_clockevent_tmr_set_state_oneshot( + struct clock_event_device *evt) +{ + struct kinetis_clock_event_ddata *pit = + container_of(evt, struct kinetis_clock_event_ddata, evtdev); + + kinetis_pit_enable(pit, 0); + + return 0; +} + +/* + * Configure the timer to generate an interrupt in the specified amount of ticks + */ +static int kinetis_clockevent_tmr_set_next_event( + unsigned long delta, struct clock_event_device *c) +{ + struct kinetis_clock_event_ddata *pit = + container_of(c, struct kinetis_clock_event_ddata, evtdev); + + kinetis_pit_init(pit, delta); + kinetis_pit_enable(pit, 1); + + return 0; +} + +/* + * Timer IRQ handler + */ +static irqreturn_t kinetis_clockevent_tmr_irq_handler(int irq, void *dev_id) +{ + struct kinetis_clock_event_ddata *tmr = dev_id; + + iowrite32(KINETIS_PIT_TFLG_TIF_MSK, KINETIS_PIT_PTR(tmr->base, tflg)); + + tmr->evtdev.event_handler(&tmr->evtdev); + + return IRQ_HANDLED; +} + +static void __init kinetis_clockevent_init(struct device_node *np) +{ + const u64 max_delay_in_sec = 5; + struct kinetis_clock_event_ddata *kinetis_tmr; + struct device_node *child; + struct clk *clk; + void __iomem *base; + void __iomem *mcr; + unsigned long rate; + int irq; + + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) { + pr_err("failed to get clock for clockevent\n"); + return; + } + + if (clk_prepare_enable(clk)) { + pr_err("failed to enable timer clock for clockevent\n"); + goto err_clk_enable; + } + + rate = clk_get_rate(clk); + if (!(rate / HZ)) { + pr_err("failed to get proper clock rate for clockevent\n"); + goto err_get_rate; + } + + mcr = of_iomap(np, 0); + if (!mcr) { + pr_err("failed to get mcr for clockevent\n"); + goto err_iomap_mcr; + } + + for_each_child_of_node(np, child) { + if (!of_device_is_available(child)) + continue; + + kinetis_tmr = kzalloc(sizeof(struct kinetis_clock_event_ddata), + GFP_KERNEL); + if (!kinetis_tmr) + continue; + + + base = of_iomap(child, 0); + if (!base) { + pr_err("failed to get registers for pit channel\n"); + kfree(kinetis_tmr); + continue; + } + + irq = irq_of_parse_and_map(child, 0); + if (irq <= 0) { + pr_err("failed to get irq for pit channel\n"); + iounmap(base); + kfree(kinetis_tmr); + continue; + } + + kinetis_tmr->evtdev.name = child->full_name; + kinetis_tmr->evtdev.rating = 200; + kinetis_tmr->evtdev.features = CLOCK_EVT_FEAT_PERIODIC | + CLOCK_EVT_FEAT_ONESHOT; + kinetis_tmr->evtdev.set_next_event = + kinetis_clockevent_tmr_set_next_event; + kinetis_tmr->evtdev.set_state_periodic = + kinetis_clockevent_tmr_set_state_periodic; + kinetis_tmr->evtdev.set_state_oneshot = + kinetis_clockevent_tmr_set_state_oneshot; + kinetis_tmr->evtdev.set_state_oneshot_stopped = + kinetis_clockevent_tmr_set_state_oneshot; + kinetis_tmr->evtdev.set_state_shutdown = + kinetis_clockevent_tmr_set_state_oneshot; + kinetis_tmr->base = base; + kinetis_tmr->mcr = mcr; + spin_lock_init(&kinetis_tmr->lock); + + /* + * Set the fields required for the set_next_event method + * (tickless kernel support) + */ + clockevents_calc_mult_shift(&kinetis_tmr->evtdev, rate, + max_delay_in_sec); + kinetis_tmr->evtdev.max_delta_ns = + max_delay_in_sec * NSEC_PER_SEC; + kinetis_tmr->evtdev.min_delta_ns = clockevent_delta2ns(0xf, + &kinetis_tmr->evtdev); + + clockevents_register_device(&kinetis_tmr->evtdev); + + kinetis_pit_init(kinetis_tmr, (rate / HZ) - 1); + kinetis_pit_enable(kinetis_tmr, 1); + + if (request_irq(irq, kinetis_clockevent_tmr_irq_handler, + IRQF_TIMER | IRQF_IRQPOLL, + "kinetis-timer", + kinetis_tmr)) { + pr_err("failed to request irq for pit channel\n"); + kinetis_pit_enable(kinetis_tmr, 0); + continue; + } + + pr_info("prepared pit channel at MMIO %#x\n", (unsigned)base); + } + + return; + +err_iomap_mcr: +err_get_rate: + + clk_disable_unprepare(clk); +err_clk_enable: + + clk_put(clk); +} + +CLOCKSOURCE_OF_DECLARE(kinetis_pit_timer, "fsl,kinetis-pit-timer", + kinetis_clockevent_init); -- 2.3.6