[PATCH v2 1/2] drivers/irqchip: Fixup csky,mpintc compile error with CPU_CK610

[PATCH v2 1/2] drivers/irqchip: Fixup csky,mpintc compile error with CPU_CK610

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The irq-csky-mpintc.c only could support CPU_CK860 and it will
compile error with CPU_CK610.

It has beed selected in arch/csky/Kconfig

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

---
v2: Drop the string after bool,
    as suggested by Marc Zyngier <maz@kernel.org>

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b147f22a78f4..53abecb8c792 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -432,7 +432,7 @@ config QCOM_PDC
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
 config CSKY_MPINTC
-	bool "C-SKY Multi Processor Interrupt Controller"
+	bool
 	depends on CSKY
 	help
 	  Say yes here to enable C-SKY SMP interrupt controller driver used
-- 
2.17.1

[PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

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 <guoren@linux.alibaba.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

---
v2: Drop the string after bool,
    as suggested by Marc Zyngier <maz@kernel.org>

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

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Daniel Lezcano]

On 04/02/2021 08:46, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> 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 <guoren@linux.alibaba.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/clocksource/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ---
> v2: Drop the string after bool,
>     as suggested by Marc Zyngier <maz@kernel.org>
> 
> 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

Re: (subset) [PATCH v2 1/2] drivers/irqchip: Fixup csky,mpintc compile error with CPU_CK610

[Marc Zyngier]

On Thu, 4 Feb 2021 15:46:08 +0800, guoren@kernel.org wrote:
> The irq-csky-mpintc.c only could support CPU_CK860 and it will
> compile error with CPU_CK610.
> 
> It has beed selected in arch/csky/Kconfig

Applied to irq/irqchip-next, thanks!

[1/2] drivers/irqchip: Fixup csky,mpintc compile error with CPU_CK610
      commit: be1abc5ba4d2082df6749ab95ec6f87c4d3dbb23

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.

[irqchip: irq/irqchip-next] irqchip/csky-mpintc: Prevent selection on unsupported platforms

[irqchip-bot for Guo Ren]

The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     be1abc5ba4d2082df6749ab95ec6f87c4d3dbb23
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/be1abc5ba4d2082df6749ab95ec6f87c4d3dbb23
Author:        Guo Ren <guoren@linux.alibaba.com>
AuthorDate:    Thu, 04 Feb 2021 15:46:08 +08:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Thu, 04 Feb 2021 10:37:28 

irqchip/csky-mpintc: Prevent selection on unsupported platforms

The irq-csky-mpintc driver is only supported on CPU_CK860 and
it will generate a compilation error when selected with CPU_CK610.

As it is already selected directly in the architecture Kconfig,
drop the option to select it manually.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
[maz: rewrote commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20210204074609.3553018-1-guoren@kernel.org
---
 drivers/irqchip/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f95d114..030895c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -427,7 +427,7 @@ config QCOM_PDC
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
 config CSKY_MPINTC
-	bool "C-SKY Multi Processor Interrupt Controller"
+	bool
 	depends on CSKY
 	help
 	  Say yes here to enable C-SKY SMP interrupt controller driver used

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Guo Ren]

Hi Daniel,

On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 04/02/2021 08:46, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > 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/

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Guo Ren]

The same way in IRQ chip:

Link: https://lore.kernel.org/lkml/161245410060.1472442.13488057609914274236.b4-ty@kernel.org/

On Sun, Feb 7, 2021 at 11:31 AM Guo Ren <guoren@kernel.org> wrote:
>
> Hi Daniel,
>
> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >
> > On 04/02/2021 08:46, guoren@kernel.org wrote:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > 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/

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Daniel Lezcano]

On 07/02/2021 04:31, Guo Ren wrote:
> Hi Daniel,
> 
> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 04/02/2021 08:46, guoren@kernel.org wrote:
>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>
>>> 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

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Guo Ren]

Hi Daniel,

On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 07/02/2021 04:31, Guo Ren wrote:
> > Hi Daniel,
> >
> > On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 04/02/2021 08:46, guoren@kernel.org wrote:
> >>> From: Guo Ren <guoren@linux.alibaba.com>
> >>>
> >>> 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/

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Daniel Lezcano]

On 09/02/2021 17:02, Guo Ren wrote:
> Hi Daniel,
> 
> On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 07/02/2021 04:31, Guo Ren wrote:
>>> Hi Daniel,
>>>
>>> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>>
>>>> On 04/02/2021 08:46, guoren@kernel.org wrote:
>>>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>>>
>>>>> 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

Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610

[Guo Ren]

Hi Daniel,

On Wed, Feb 10, 2021 at 4:26 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 09/02/2021 17:02, Guo Ren wrote:
> > Hi Daniel,
> >
> > On Sun, Feb 7, 2021 at 5:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 07/02/2021 04:31, Guo Ren wrote:
> >>> Hi Daniel,
> >>>
> >>> On Thu, Feb 4, 2021 at 4:48 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>>>
> >>>> On 04/02/2021 08:46, guoren@kernel.org wrote:
> >>>>> From: Guo Ren <guoren@linux.alibaba.com>
> >>>>>
> >>>>> 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/