Thread (14 messages) 14 messages, 4 authors, 2018-01-08

Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

From: Greentime Hu <hidden>
Date: 2018-01-04 14:07:22
Also in: linux-arch, linux-devicetree, linux-serial, lkml

Hi, Daniel:

2018-01-04 21:50 GMT+08:00 Daniel Lezcano [off-list ref]:
Hi,

sorry I missed your answer. Comments below.

On 13/12/2017 07:06, Greentime Hu wrote:
quoted
Hi, Daniel:

2017-12-12 18:05 GMT+08:00 Daniel Lezcano [off-list ref]:
quoted
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.
Yes, so you don't need it to be selectable, you must select it from the
platform's Kconfig.
I am not sure that I get your point or not.
We don't have a CONFIG_PLAT_AE3XX.
Do you mean we should create one and select CLKSRC_ATCPIT100 under
CONFIG_PLAT_AE3XX?
quoted
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]
The COMPILE_TEST option is only there to allow cross-compilation test
coverage, it does not select or unselect the driver in usual way.

If the COMPILE_TEST is enabled, then the option will appear in the
menuconfig, so that gives the opportunity to select/unselect it.

Otherwise, the Kconfig's platform selects automatically the driver and
the user *can't* unselect it from the menuconfig as it is a silent
option and that is certainly what you want.
quoted
quoted
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.
You just resend an entire series V5 for the architecture. I'm confused,
what is the merging path ?
Sorry. I didn't get your point.
We sent the timer patch and the architecture patch together because it
would be easier for reviewer to check the vdso implementations.
What do you mean about the merging path?

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help