From: Guo Ren <guoren@kernel.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-csky@vger.kernel.org, Guo Ren <guoren@linux.alibaba.com>,
Thomas Gleixner <tglx@linutronix.de>,
Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer compile error with CPU_CK610
Date: Wed, 10 Feb 2021 11:03:42 +0800 [thread overview]
Message-ID: <CAJF2gTTP47MZaX4eh5RV-GRABtDpyxjd3C3kH8kyvafjBNGoig@mail.gmail.com> (raw)
In-Reply-To: <a2f43b21-2e56-040f-b658-ccafa3b5fdc4@linaro.org>
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/
next prev parent reply other threads:[~2021-02-10 3:05 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-04 7:46 [PATCH v2 1/2] drivers/irqchip: Fixup csky,mpintc compile error with CPU_CK610 guoren
2021-02-04 7:46 ` [PATCH v2 2/2] drivers/clocksource: Fixup csky,mptimer " guoren
2021-02-04 8:48 ` Daniel Lezcano
2021-02-07 3:31 ` Guo Ren
2021-02-07 3:34 ` Guo Ren
2021-02-07 9:29 ` Daniel Lezcano
2021-02-09 16:02 ` Guo Ren
2021-02-09 20:26 ` Daniel Lezcano
2021-02-10 3:03 ` Guo Ren [this message]
2021-02-04 15:55 ` (subset) [PATCH v2 1/2] drivers/irqchip: Fixup csky,mpintc " Marc Zyngier
2021-02-04 16:02 ` [irqchip: irq/irqchip-next] irqchip/csky-mpintc: Prevent selection on unsupported platforms irqchip-bot for Guo Ren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJF2gTTP47MZaX4eh5RV-GRABtDpyxjd3C3kH8kyvafjBNGoig@mail.gmail.com \
--to=guoren@kernel.org \
--cc=daniel.lezcano@linaro.org \
--cc=guoren@linux.alibaba.com \
--cc=linux-csky@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).