From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>, Sekhar Nori <nsekhar@ti.com>,
Kevin Hilman <khilman@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
David Lechner <david@lechnology.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 01/11] clocksource: davinci-timer: new driver
Date: Mon, 15 Apr 2019 14:36:39 +0200 [thread overview]
Message-ID: <d3d05858-966b-cd4a-3286-97e7d7bde0bc@linaro.org> (raw)
In-Reply-To: <20190318121100.28132-2-brgl@bgdev.pl>
On 18/03/2019 13:10, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Currently the clocksource and clockevent support for davinci platforms
> lives in mach-davinci. It hard-codes many things, uses global variables,
> implements functionalities unused by any platform and has code fragments
> scattered across many (often unrelated) files.
>
> Implement a new, modern and simplified timer driver and put it into
> drivers/clocksource. We still need to support legacy board files so
> export a config structure and a function that allows machine code to
> register the timer.
>
> We don't bother freeing resources on errors in davinci_timer_register()
> as the system won't boot without a timer anyway.
Can you give a quick description of the timer hardware and how it works?
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: David Lechner <david@lechnology.com>
> ---
> drivers/clocksource/Kconfig | 5 +
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-davinci.c | 438 ++++++++++++++++++++++++++++
> include/clocksource/timer-davinci.h | 44 +++
> 4 files changed, 488 insertions(+)
> create mode 100644 drivers/clocksource/timer-davinci.c
> create mode 100644 include/clocksource/timer-davinci.h
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 171502a356aa..08b1f539cfc4 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -42,6 +42,11 @@ config BCM_KONA_TIMER
> help
> Enables the support for the BCM Kona mobile timer driver.
>
> +config DAVINCI_TIMER
> + bool "Texas Instruments DaVinci timer driver"
Make the option silent (eg. if COMPILE_TEST)
> + help
> + Enables the support for the TI DaVinci timer driver.
> +
> config DIGICOLOR_TIMER
> bool "Digicolor timer driver" if COMPILE_TEST
> select CLKSRC_MMIO
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index be6e0fbc7489..3c73d0e58b45 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o
> obj-$(CONFIG_EM_TIMER_STI) += em_sti.o
> obj-$(CONFIG_CLKBLD_I8253) += i8253.o
> obj-$(CONFIG_CLKSRC_MMIO) += mmio.o
> +obj-$(CONFIG_DAVINCI_TIMER) += timer-davinci.o
> obj-$(CONFIG_DIGICOLOR_TIMER) += timer-digicolor.o
> obj-$(CONFIG_OMAP_DM_TIMER) += timer-ti-dm.o
> obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> new file mode 100644
> index 000000000000..46dfc4d457fc
> --- /dev/null
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// TI DaVinci clocksource driver
> +//
> +// Copyright (C) 2019 Texas Instruments
> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +// (with some parts adopted from code by Kevin Hilman <khilman@baylibre.com>)
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/sched_clock.h>
> +
> +#include <clocksource/timer-davinci.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "%s: " fmt "\n", __func__
> +
> +#define DAVINCI_TIMER_REG_TIM12 0x10
> +#define DAVINCI_TIMER_REG_TIM34 0x14
> +#define DAVINCI_TIMER_REG_PRD12 0x18
> +#define DAVINCI_TIMER_REG_PRD34 0x1c
> +#define DAVINCI_TIMER_REG_TCR 0x20
> +#define DAVINCI_TIMER_REG_TGCR 0x24
> +
> +#define DAVINCI_TIMER_TIMMODE_MASK GENMASK(3, 2)
> +#define DAVINCI_TIMER_RESET_MASK GENMASK(1, 0)
> +#define DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED BIT(2)
> +#define DAVINCI_TIMER_UNRESET GENMASK(1, 0)
> +
> +/* Shift depends on timer. */
> +#define DAVINCI_TIMER_ENAMODE_MASK GENMASK(1, 0)
> +#define DAVINCI_TIMER_ENAMODE_DISABLED 0x00
> +#define DAVINCI_TIMER_ENAMODE_ONESHOT BIT(0)
> +#define DAVINCI_TIMER_ENAMODE_PERIODIC BIT(1)
> +
> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM12 6
> +#define DAVINCI_TIMER_ENAMODE_SHIFT_TIM34 22
> +
> +#define DAVINCI_TIMER_MIN_DELTA 0x01
> +#define DAVINCI_TIMER_MAX_DELTA 0xfffffffe
> +
> +#define DAVINCI_TIMER_CLKSRC_BITS 32
> +
> +#define DAVINCI_TIMER_TGCR_DEFAULT \
> + (DAVINCI_TIMER_TIMMODE_32BIT_UNCHAINED | DAVINCI_TIMER_UNRESET)
> +
> +enum {
> + DAVINCI_TIMER_MODE_DISABLED = 0,
> + DAVINCI_TIMER_MODE_ONESHOT,
> + DAVINCI_TIMER_MODE_PERIODIC,
> +};
This is replicating what is already available. Right after set_periodic
or set_oneshot are called, the timer state is changed to respectively
CLOCK_EVT_STATE_PERIODIC and CLOCK_EVT_STATE_ONESHOT. So it is useless
to define those enum again as what you want is to check the timer mode.
[ ... ]
> +
> + clocksource = kzalloc(sizeof(*clocksource), GFP_KERNEL);
> + if (!clocksource) {
> + pr_err("Error allocating memory for clocksource data");
> + return -ENOMEM;
> + }
> +
> + clocksource->dev.rating = 300;
> + clocksource->dev.read = davinci_timer_clksrc_read;
> + clocksource->dev.mask = CLOCKSOURCE_MASK(DAVINCI_TIMER_CLKSRC_BITS);
> + clocksource->dev.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>>>
> + clocksource->timer.set_period = davinci_timer_set_period_std;
> + clocksource->timer.mode = DAVINCI_TIMER_MODE_PERIODIC;
> + clocksource->timer.base = base;
What for?
<<<
> + if (timer_cfg->cmp_off) {
> + clocksource->timer.regs = &davinci_timer_tim12_regs;
> + clocksource->dev.name = "tim12";
> + } else {
> + clocksource->timer.regs = &davinci_timer_tim34_regs;
> + clocksource->dev.name = "tim34";
> + }
> +
> + rv = request_irq(timer_cfg->irq[DAVINCI_TIMER_CLOCKSOURCE_IRQ].start,
> + davinci_timer_irq_freerun, IRQF_TIMER,
> + "free-run counter", clocksource);
> + if (rv) {
> + pr_err("Unable to request the clocksource interrupt");
> + return rv;
> + }
Why do you have to request an interrupt to do nothing? Isn't possible to
let the timer run and wrap without generating interrupts?
[ ... ]
--
<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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-04-15 12:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-18 12:10 [PATCH v4 00/11] ARM: davinci: modernize the timer support Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 01/11] clocksource: davinci-timer: new driver Bartosz Golaszewski
2019-04-02 9:21 ` Daniel Lezcano
2019-04-02 12:28 ` Bartosz Golaszewski
2019-04-08 13:49 ` Bartosz Golaszewski
2019-04-08 13:53 ` Daniel Lezcano
2019-04-15 12:36 ` Daniel Lezcano [this message]
2019-04-16 13:44 ` Bartosz Golaszewski
2019-04-16 13:56 ` Bartosz Golaszewski
2019-04-16 16:05 ` Daniel Lezcano
2019-03-18 12:10 ` [PATCH v4 02/11] ARM: davinci: enable the clocksource driver for DT mode Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 03/11] ARM: davinci: WARN_ON() if clk_get() fails Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 04/11] ARM: davinci: da850: switch to using the clocksource driver Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 05/11] ARM: davinci: da830: " Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 06/11] ARM: davinci: move timer definitions to davinci.h Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 07/11] ARM: davinci: dm355: switch to using the clocksource driver Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 08/11] ARM: davinci: dm365: " Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 09/11] ARM: davinci: dm644x: " Bartosz Golaszewski
2019-03-18 12:10 ` [PATCH v4 10/11] ARM: davinci: dm646x: " Bartosz Golaszewski
2019-03-18 12:11 ` [PATCH v4 11/11] ARM: davinci: remove legacy timer support Bartosz Golaszewski
2019-03-27 15:36 ` [PATCH v4 00/11] ARM: davinci: modernize the " Sekhar Nori
2019-03-27 17:49 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3d05858-966b-cd4a-3286-97e7d7bde0bc@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=bgolaszewski@baylibre.com \
--cc=brgl@bgdev.pl \
--cc=david@lechnology.com \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nsekhar@ti.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).