Messages in this thread | | | From | Bartosz Golaszewski <> | Date | Sun, 26 May 2019 10:16:34 +0200 | Subject | Re: [RFC v2 1/2] clocksource: davinci-timer: add support for clockevents |
| |
sob., 25 maj 2019 o 16:16 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a): > > On 24/05/2019 13:53, Bartosz Golaszewski wrote: > > pt., 24 maj 2019 o 10:59 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a): > >> > >> On 24/05/2019 09:28, Bartosz Golaszewski wrote: > >>> czw., 23 maj 2019 o 18:38 Daniel Lezcano <daniel.lezcano@linaro.org> napisał(a): > >>>> > >>>> On 23/05/2019 14:58, 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. > >>>>> > >>>>> The timer we're using is 64-bit but can be programmed in dual 32-bit > >>>>> mode (both chained and unchained). We're using dual 32-bit mode to > >>>>> have separate counters for clockevents and clocksource. > >>>>> > >>>>> This patch contains the core code and support for clockevent. The > >>>>> clocksource code will be included in a subsequent patch. > >>>>> > > [ ... ] > > >>>>> +static unsigned int > >>>>> +davinci_clockevent_read(struct davinci_clockevent *clockevent, > >>>>> + unsigned int reg) > >>>>> +{ > >>>>> + return readl_relaxed(clockevent->base + reg); > >>>>> +} > >>>>> + > >>>>> +static void davinci_clockevent_write(struct davinci_clockevent *clockevent, > >>>>> + unsigned int reg, unsigned int val) > >>>>> +{ > >>>>> + writel_relaxed(val, clockevent->base + reg); > >>>>> +} > >>>>> + > >>>>> +static void davinci_tcr_update(void __iomem *base, > >>>>> + unsigned int mask, unsigned int val) > >>>>> +{ > >>>>> + davinci_tcr &= ~mask; > >>>>> + davinci_tcr |= val & mask; > >>>> > >>>> > >>>> I don't see when the davinci_tcr is initialized. > >>>> > >>> > >>> It's set to 0x0 by the compiler and we're setting the register to 0x0 > >>> in davinci_timer_init(). > >> > >> Why did you need to readl before in the previous version? The idea of > >> caching the value was to save an extra readl. > >> > >> If it is always zero, then we don't need this variable neither the read, > >> just doing: > >> > >> writel_relaxed(val & mask, base + DAVINCI_TIMER_REG_TCR); > >> > >> should work no ? > > > > It's not always zero. Its reset value is zero and we write 0 to it at > > init time just to make sure, but then we modify it according to the > > configuration. The single TCR register controls both halves of the > > timer, so we do need an actual update, not a simple write. > > Ok but the driver can be oneshot or disabled in the code (mutually > exclusive), no ? > > So doing > > - writel(oneshot, base); > - writel(disabled, base); > > works without any mask computation, no? > > Well the above assumes other part of the register aren't changed by > other subsystems (or by the timer itself). > >
I'm not sure I understand. You can be using two timers. Both controlled by a single TCR register. In your example oneshot can equal (0x00, or 0x01) and either be shifted left by 6 or 22 for TIM12 and TIM34 respectively. If you do writel(oneshot-for-time12, base) you'll set tim34 to disabled.
Bart
> > > > > > > > > > -- : > <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 >
| |