niedz., 26 maj 2019 o 10:16 Bartosz Golaszewski <[email protected]> napisał(a): > > sob., 25 maj 2019 o 16:16 Daniel Lezcano <[email protected]> > napisał(a): > > > > On 24/05/2019 13:53, Bartosz Golaszewski wrote: > > > pt., 24 maj 2019 o 10:59 Daniel Lezcano <[email protected]> > > > napisał(a): > > >> > > >> On 24/05/2019 09:28, Bartosz Golaszewski wrote: > > >>> czw., 23 maj 2019 o 18:38 Daniel Lezcano <[email protected]> > > >>> napisał(a): > > >>>> > > >>>> On 23/05/2019 14:58, Bartosz Golaszewski wrote: > > >>>>> From: Bartosz Golaszewski <[email protected]> > > >>>>> > > >>>>> 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
Gentle ping. Bart
