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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 Jan 2018 14:50:09 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Greentime Hu <green.hu@...il.com>,
        Greentime <greentime@...estech.com>
Cc:     Rick Chen <rickchen36@...il.com>, Rick Chen <rick@...estech.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Rob Herring <robh+dt@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Vincent Chen <deanbo422@...il.com>,
        DTML <devicetree@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        David Howells <dhowells@...hat.com>,
        Will Deacon <will.deacon@....com>, linux-serial@...r.kernel.org
Subject: Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech
 atcpit100 timer


Hi,

sorry I missed your answer. Comments below.

On 13/12/2017 07:06, Greentime Hu wrote:
> Hi, Daniel:
> 
> 2017-12-12 18:05 GMT+08:00 Daniel Lezcano <daniel.lezcano@...aro.org>:
>> On 12/12/2017 06:46, Rick Chen wrote:
>>> 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.
>>
>> [ ... ]
>>
>>> +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.

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

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

Thanks.
  -- Daniel

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