Hi, Daniel:

2017-12-12 18:05 GMT+08:00 Daniel Lezcano [off-list ref]:

On 12/12/2017 06:46, Rick Chen wrote:

quoted

ATCPIT100 is often used on the Andes architecture,
This timer provide 4 PIT channels. Each PIT channel is a
multi-function timer, can be configured as 32,16,8 bit timers
or PWM as well.

For system timer it will set channel 1 32-bit timer0 as clock
source and count downwards until underflow and restart again.

[ ... ]

quoted

+config CLKSRC_ATCPIT100
+     bool "Clocksource for AE3XX platform"
+     depends on NDS32 || COMPILE_TEST
+     depends on HAS_IOMEM
+     help
+       This option enables support for the Andestech AE3XX platform timers.

Hi Rick,

the general rule for the Kconfig is:

bool "Clocksource for AE3XX platform" if COMPILE_TEST

and no deps on the platform.

It is up to the platform Kconfig to select the option.

We want here a silent option but make it selectable in case of
compilation test coverage.

The way we like to use it is because
1. This timer is a basic component to boot an nds32 CPU and it should
be able to select without COMPILE_TEST for nds32 architecture.
2. It seems conflict with debug info. I am not sure if there is
another way to debug kernel(with debug info) with COMPILE_TEST and
DEBUG_INFO because we need this driver for nds32 architecture.

Symbol: DEBUG_INFO [=n]
Type  : boolean
Prompt: Compile the kernel with debug info
  Location:
    -> Kernel hacking
      -> Compile-time checks and compiler options
  Defined at lib/Kconfig.debug:140
  Depends on: DEBUG_KERNEL [=y] && !COMPILE_TEST [=n]

Also, this driver is not a CLKSRC but a TIMER. Rename CLKSRC_ATCPIT100
to TIMER_ATCPIT100.

Thanks. We will rename it in the next version patch.

quoted

+
 endmenu

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 72711f1..7d072f5 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile

@@ -75,3 +75,4 @@ obj-$(CONFIG_H8300_TMR16)           += h8300_timer16.o
 obj-$(CONFIG_H8300_TPU)                      += h8300_tpu.o
 obj-$(CONFIG_CLKSRC_ST_LPC)          += clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)           += numachip.o
+obj-$(CONFIG_CLKSRC_ATCPIT100)               += timer-atcpit100.o

[ ... ]

quoted

+static struct timer_of to = {
+     .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+
+     .clkevt = {
+             .name = "atcpit100_tick",
+             .rating = 300,
+             .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+             .set_state_shutdown = atcpit100_clkevt_shutdown,
+             .set_state_periodic = atcpit100_clkevt_set_periodic,
+             .set_state_oneshot = atcpit100_clkevt_set_oneshot,
+             .tick_resume = atcpit100_clkevt_shutdown,
+             .set_next_event = atcpit100_clkevt_next_event,
+             .cpumask = cpu_all_mask,

You may consider CLOCK_EVT_DYN_IRQ
https://lwn.net/Articles/540160/

Thanks.
We will consider to implement this feature once we support SMP or our
CPU has local timer.

quoted

+     },
+
+     .of_irq = {
+             .handler = atcpit100_timer_interrupt,
+             .flags = IRQF_TIMER | IRQF_IRQPOLL,
+     },
+
+     /*
+      * FIXME: we currently only support clocking using PCLK
+      * and using EXTCLK is not supported in the driver.
+      */
+     .of_clk = {
+             .name = "PCLK",
+     }

What do you mean ? We can't specify several clock names with timer-of?

It means we currently support PCLK only.
https://lkml.org/lkml/2017/12/1/316

Thanks.