Re: [PATCH v7 2/3] clocksource: Add JH7110 timer driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Xingyu,

On 02/11/2023 14:15, Xingyu Wu wrote:

[ ... ]

+struct jh7110_clkevt { +    struct clock_event_device
evt; + struct clocksource cs; +    bool cs_is_valid; +
struct clk *clk; +    struct reset_control *rst; +    u32
rate; +    u32 reload_val; +    void __iomem *base; +
char name[sizeof("jh7110-timer.chX")]; +}; + +struct jh7110_timer_priv { + struct clk *pclk; + struct reset_control *prst; + struct jh7110_clkevt clkevt[JH7110_TIMER_CH_MAX];

Why do you need several clock events and clock sources ?

This timer has four counters (channels) which run
independently. So each counter can have its own clock event
and clock source to configure different settings.

The kernel only needs one clocksource. Usually multiple
clockevents are per-cpu based system.

The driver does not seem to have a per cpu timer but just initializing multiple clockevents which will end up unused,
wasting energy.



The board of the StarFive JH7110 SoC has two types of timer : riscv-timer and jh7110-timer. It boots by
riscv-timer(clocksource) and the jh7110-timer is optional and
additional. I think I should initialize the four channels of
jh7110-timer as clockevents not clocksource pre-cpu.

If no clocksource is needed on this SoC because riscv timers are
used, then it is not useful to register a clocksource for this
timer and the corresponding code can go away.

If the clockevent is optional why do you need this driver at all?




Hi Daniel,

Sorry, maybe I didn't express it clearly enough. I use this
jh7110-timer as a global timer on the SoC and riscv-timer as cpu
local timer. So these are something different.

These four counters in this jh7110-timer are exactly the same and
independent of each other. If this timer is used as a global timer,
do I use only one or all of the counters to register clocksource and
clockevent?

Yes.

The global timer is only there when the CPU is powered down at idle time, so the time framework will switch to the broadcast timer and there can be only one instance.

If you register all the counters, only one will be used by the kernel, so it pointless to add them all.

On the clocksource side, you may want to question if it is really useful. The riscv has a clocksource with a higher rate and flagged as continuous [1]. So if the JH7110 clocksource is registered, it won't be used too.

Hope that helps

  -- Daniel

[1] https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/clocksource/timer-riscv.c#n68

--
<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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux