On 27/10/2023 11:17, Xingyu Wu wrote:
On 2023/10/25 22:39, Daniel Lezcano wrote:
Hi Xingyu,
On 25/10/2023 11:04, Xingyu Wu wrote:
On 2023/10/24 22:56, Daniel Lezcano wrote:
Hi Xingyu,
On 19/10/2023 07:35, Xingyu Wu wrote:
Add timer driver for the StarFive JH7110 SoC.
As it is a new timer, please add a proper nice description
explaining the timer hardware, thanks.
OK. Will add the description in next version.
Signed-off-by: Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx> ---
MAINTAINERS | 7 +
drivers/clocksource/Kconfig | 11 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-jh7110.c | 380
+++++++++++++++++++++++++++++ 4 files changed, 399
insertions(+) create mode 100644
drivers/clocksource/timer-jh7110.c
diff --git a/MAINTAINERS b/MAINTAINERS index
7a7bd8bd80e9..91c09b399131 100644 --- a/MAINTAINERS +++
b/MAINTAINERS @@ -20473,6 +20473,13 @@ S: Maintained F:
Documentation/devicetree/bindings/sound/starfive,jh7110-tdm.yaml
F: sound/soc/starfive/jh7110_tdm.c
+STARFIVE JH7110 TIMER DRIVER +M: Samin Guo
<samin.guo@xxxxxxxxxxxxxxxx> +M: Xingyu Wu
<xingyu.wu@xxxxxxxxxxxxxxxx> +S: Supported +F:
Documentation/devicetree/bindings/timer/starfive,jh7110-timer.yaml
+F: drivers/clocksource/timer-jh7110.c
+ STARFIVE JH71X0 CLOCK DRIVERS M: Emil Renner Berthing
<kernel@xxxxxxxx> M: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
diff --git a/drivers/clocksource/Kconfig
b/drivers/clocksource/Kconfig index
0ba0dc4ecf06..821abcc1e517 100644 ---
a/drivers/clocksource/Kconfig +++
b/drivers/clocksource/Kconfig @@ -641,6 +641,17 @@ config
RISCV_TIMER is accessed via both the SBI and the rdcycle
instruction. This is required for all RISC-V systems.
+config STARFIVE_JH7110_TIMER + bool "Timer for the
STARFIVE JH7110 SoC" + depends on ARCH_STARFIVE ||
COMPILE_TEST
You may want to use ARCH_STARFIVE only if the platform can make
this timer optional. Otherwise, set the option from the
platform Kconfig and put the bool "bla bla" if COMPILE_TEST
Yes, this timer only be used on the StarFive SoC. So I intend to
modify to this:
bool "Timer for the STARFIVE JH7110 SoC" if COMPILE_TEST depends
on ARCH_STARFIVE
In this case, you should change the platform config and select the
timer from there. Remove the depends on ARCH_STARFIVE so it is
possible enable cross test compilation. Otherwise COMPILE_TEST will
not work on other platforms.
[ ... ]
It is not a kernel timer or clocksource. It will not work on other
platforms and is just used on the JH7110 SoC. I think I needn't
remove it. Maybe I modify to this:
bool "Timer for the STARFIVE JH7110 SoC" if COMPILE_TEST depends on
ARCH_STARFIVE || COMPILE_TEST
I think there is a misunderstanding.
If we want to compile on x86 drivers for other platforms, we select
COMPILE_TEST so we can enable the timer and do compilation testing.
In this case, we may want to compile the STARFIVE JH7110 on x86 just to
double check it is correctly compiling (eg. we do changes impacting all
the drivers). If the ARCH_STARFIVE dependency is set, then that won't be
possible.
So it should be:
bool "Timer for the STARFIVE JH7110 SoC" if COMPILE_TEST
...
And in arch/riscv/Kconfig.socs
config SOC_STARFIVE
...
select STARFIVE_JH7110_TIMER
...
+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?
--
<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
