From: Guo Ren <[email protected]>
The timer-mp-csky.c only could support CPU_CK860 and it will
compile error with CPU_CK610.
It has been selected in arch/csky/Kconfig.
Signed-off-by: Guo Ren <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Marc Zyngier <[email protected]>
---
drivers/clocksource/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
v2: Drop the string after bool,
as suggested by Marc Zyngier <[email protected]>
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 14c7c4712478..d39e6ca86d25 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -662,7 +662,7 @@ config CLINT_TIMER
driver is usually used for NoMMU RISC-V systems.
config CSKY_MP_TIMER
- bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
+ bool
depends on CSKY
select TIMER_OF
help
--
2.17.1
On 04/02/2021 08:46, [email protected] wrote:
> From: Guo Ren <[email protected]>
>
> The timer-mp-csky.c only could support CPU_CK860 and it will
> compile error with CPU_CK610.
>
> It has been selected in arch/csky/Kconfig.
It would be better if you fix the root cause of the compilation error.
The COMPILE_TEST is there for compilation test coverage when the changes
are touching a lot of drivers.
> Signed-off-by: Guo Ren <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> ---
> drivers/clocksource/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> ---
> v2: Drop the string after bool,
> as suggested by Marc Zyngier <[email protected]>
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 14c7c4712478..d39e6ca86d25 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -662,7 +662,7 @@ config CLINT_TIMER
> driver is usually used for NoMMU RISC-V systems.
>
> config CSKY_MP_TIMER
> - bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
> + bool
> depends on CSKY
> select TIMER_OF
> help
>
--
<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
Hi Daniel,
On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
>
> On 04/02/2021 08:46, [email protected] wrote:
> > From: Guo Ren <[email protected]>
> >
> > The timer-mp-csky.c only could support CPU_CK860 and it will
> > compile error with CPU_CK610.
> >
> > It has been selected in arch/csky/Kconfig.
>
> It would be better if you fix the root cause of the compilation error.
The timer-mp-csky.c has used specific instructions which only
supported by CK860 and timer-mp-csky.c is only design for CK860.
In arch/csky/Konfig we only select it with CK860.
select CSKY_MPINTC if CPU_CK860
select CSKY_MP_TIMER if CPU_CK860
So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
drivers/clocksource/Kconfig.
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
The same way in IRQ chip:
Link: https://lore.kernel.org/lkml/[email protected]/
On Sun, Feb 7, 2021 at 11:31 AM Guo Ren <[email protected]> wrote:
>
> Hi Daniel,
>
> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
> >
> > On 04/02/2021 08:46, [email protected] wrote:
> > > From: Guo Ren <[email protected]>
> > >
> > > The timer-mp-csky.c only could support CPU_CK860 and it will
> > > compile error with CPU_CK610.
> > >
> > > It has been selected in arch/csky/Kconfig.
> >
> > It would be better if you fix the root cause of the compilation error.
> The timer-mp-csky.c has used specific instructions which only
> supported by CK860 and timer-mp-csky.c is only design for CK860.
>
> In arch/csky/Konfig we only select it with CK860.
> select CSKY_MPINTC if CPU_CK860
> select CSKY_MP_TIMER if CPU_CK860
>
> So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
> drivers/clocksource/Kconfig.
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On 07/02/2021 04:31, Guo Ren wrote:
> Hi Daniel,
>
> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
>>
>> On 04/02/2021 08:46, [email protected] wrote:
>>> From: Guo Ren <[email protected]>
>>>
>>> The timer-mp-csky.c only could support CPU_CK860 and it will
>>> compile error with CPU_CK610.
>>>
>>> It has been selected in arch/csky/Kconfig.
>>
>> It would be better if you fix the root cause of the compilation error.
> The timer-mp-csky.c has used specific instructions which only
> supported by CK860 and timer-mp-csky.c is only design for CK860.
I guess you are referring to mfcr() ?
> In arch/csky/Konfig we only select it with CK860.
> select CSKY_MPINTC if CPU_CK860
> select CSKY_MP_TIMER if CPU_CK860
>
> So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
> drivers/clocksource/Kconfig.
The COMPILE_TEST option is there to let other architecture to compile
drivers and increase the compilation test coverage.
The proposed change just removes the driver from this coverage.
Ideally, it would be better to keep it with the COMPILE_TEST option, so
changes impacting all the drivers can be caught before submitting the
patches.
By just adding
#ifndef mfcr
#define mfcr(a) 0
#endif
shoud fix the compilation issue, no ?
--
<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
On 09/02/2021 17:02, Guo Ren wrote:
> Hi Daniel,
>
> On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <[email protected]> wrote:
>>
>> On 07/02/2021 04:31, Guo Ren wrote:
>>> Hi Daniel,
>>>
>>> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
>>>>
>>>> On 04/02/2021 08:46, [email protected] wrote:
>>>>> From: Guo Ren <[email protected]>
>>>>>
>>>>> The timer-mp-csky.c only could support CPU_CK860 and it will
>>>>> compile error with CPU_CK610.
>>>>>
>>>>> It has been selected in arch/csky/Kconfig.
>>>>
>>>> It would be better if you fix the root cause of the compilation error.
>>> The timer-mp-csky.c has used specific instructions which only
>>> supported by CK860 and timer-mp-csky.c is only design for CK860.
>>
>> I guess you are referring to mfcr() ?
>>
>>> In arch/csky/Konfig we only select it with CK860.
>>> select CSKY_MPINTC if CPU_CK860
>>> select CSKY_MP_TIMER if CPU_CK860
>>>
>>> So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
>>> drivers/clocksource/Kconfig.
>>
>> The COMPILE_TEST option is there to let other architecture to compile
>> drivers and increase the compilation test coverage.
>>
>> The proposed change just removes the driver from this coverage.
> When we compile the csky arch with C860, it will be selected.
>
>>
>> Ideally, it would be better to keep it with the COMPILE_TEST option, so
>> changes impacting all the drivers can be caught before submitting the
>> patches.
>>
>> By just adding
>>
>> #ifndef mfcr
>> #define mfcr(a) 0
>> #endif
>
> 610 couldn't support CSKY_MP_TIMER and it's only for 860. So it's not
> a coding skill issue.
I think there is a misunderstanding.
When I want to compile on x64 all the timer drivers, I do enable
COMPILE_TEST, then the strings appear and the drivers can be selected.
If the COMPILE_TEST is not enabled, the string does not appear, it is
not possible to enable/disable it and the platform must enable it from
the aforementioned arch/csky/Konfig.
Actually, the timer drivers policy is : drivers can not be enabled from
the drivers/clocksource/Kconfig, it is up to the platform Kconfig to
select them. The exception is when the COMPILE_TEST option is set for
testing purpose.
The timer must compile on any other archs and the stubs for the platform
specific calls must be provided.
Did I miss something with your changes ?
--
<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
Hi Daniel,
On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <[email protected]> wrote:
>
> On 07/02/2021 04:31, Guo Ren wrote:
> > Hi Daniel,
> >
> > On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
> >>
> >> On 04/02/2021 08:46, [email protected] wrote:
> >>> From: Guo Ren <[email protected]>
> >>>
> >>> The timer-mp-csky.c only could support CPU_CK860 and it will
> >>> compile error with CPU_CK610.
> >>>
> >>> It has been selected in arch/csky/Kconfig.
> >>
> >> It would be better if you fix the root cause of the compilation error.
> > The timer-mp-csky.c has used specific instructions which only
> > supported by CK860 and timer-mp-csky.c is only design for CK860.
>
> I guess you are referring to mfcr() ?
>
> > In arch/csky/Konfig we only select it with CK860.
> > select CSKY_MPINTC if CPU_CK860
> > select CSKY_MP_TIMER if CPU_CK860
> >
> > So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
> > drivers/clocksource/Kconfig.
>
> The COMPILE_TEST option is there to let other architecture to compile
> drivers and increase the compilation test coverage.
>
> The proposed change just removes the driver from this coverage.
When we compile the csky arch with C860, it will be selected.
>
> Ideally, it would be better to keep it with the COMPILE_TEST option, so
> changes impacting all the drivers can be caught before submitting the
> patches.
>
> By just adding
>
> #ifndef mfcr
> #define mfcr(a) 0
> #endif
610 couldn't support CSKY_MP_TIMER and it's only for 860. So it's not
a coding skill issue.
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Hi Daniel,
On Wed, Feb 10, 2021 at 4:26 AM Daniel Lezcano
<[email protected]> wrote:
>
> On 09/02/2021 17:02, Guo Ren wrote:
> > Hi Daniel,
> >
> > On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <[email protected]> wrote:
> >>
> >> On 07/02/2021 04:31, Guo Ren wrote:
> >>> Hi Daniel,
> >>>
> >>> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <[email protected]> wrote:
> >>>>
> >>>> On 04/02/2021 08:46, [email protected] wrote:
> >>>>> From: Guo Ren <[email protected]>
> >>>>>
> >>>>> The timer-mp-csky.c only could support CPU_CK860 and it will
> >>>>> compile error with CPU_CK610.
> >>>>>
> >>>>> It has been selected in arch/csky/Kconfig.
> >>>>
> >>>> It would be better if you fix the root cause of the compilation error.
> >>> The timer-mp-csky.c has used specific instructions which only
> >>> supported by CK860 and timer-mp-csky.c is only design for CK860.
> >>
> >> I guess you are referring to mfcr() ?
> >>
> >>> In arch/csky/Konfig we only select it with CK860.
> >>> select CSKY_MPINTC if CPU_CK860
> >>> select CSKY_MP_TIMER if CPU_CK860
> >>>
> >>> So here let's select timer-mp-csky.c in arch/csky/Kconfig, not in
> >>> drivers/clocksource/Kconfig.
> >>
> >> The COMPILE_TEST option is there to let other architecture to compile
> >> drivers and increase the compilation test coverage.
> >>
> >> The proposed change just removes the driver from this coverage.
> > When we compile the csky arch with C860, it will be selected.
> >
> >>
> >> Ideally, it would be better to keep it with the COMPILE_TEST option, so
> >> changes impacting all the drivers can be caught before submitting the
> >> patches.
> >>
> >> By just adding
> >>
> >> #ifndef mfcr
> >> #define mfcr(a) 0
> >> #endif
> >
> > 610 couldn't support CSKY_MP_TIMER and it's only for 860. So it's not
> > a coding skill issue.
>
> I think there is a misunderstanding.
>
> When I want to compile on x64 all the timer drivers, I do enable
> COMPILE_TEST, then the strings appear and the drivers can be selected.
>
> If the COMPILE_TEST is not enabled, the string does not appear, it is
> not possible to enable/disable it and the platform must enable it from
> the aforementioned arch/csky/Konfig.
>
> Actually, the timer drivers policy is : drivers can not be enabled from
> the drivers/clocksource/Kconfig, it is up to the platform Kconfig to
> select them. The exception is when the COMPILE_TEST option is set for
> testing purpose.
>
> The timer must compile on any other archs and the stubs for the platform
> specific calls must be provided.
>
> Did I miss something with your changes ?
I think our biggest difference is:
- You think that CSKY_MPTIMER should not be related to the
architecture, but can be compiled with any architecture.
- But I think CSKY_MPTIMER only could to be compiled with CSKY C860.
But from the perspective of easy maintenance, I agree with your
suggestion. I will adopt in next patch:
> >> #ifndef mfcr
> >> #define mfcr(a) 0
> >> #endif
Thx
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/