From: Sean Anderson <seanga2@gmail.com>
To: Palmer Dabbelt <palmer@dabbelt.com>, atishp@atishpatra.org
Cc: devicetree@vger.kernel.org,
Damien Le Moal <Damien.LeMoal@wdc.com>,
daniel.lezcano@linaro.org, kernel@esmil.dk,
Anup Patel <Anup.Patel@wdc.com>,
linux-kernel@vger.kernel.org, Atish Patra <Atish.Patra@wdc.com>,
robh+dt@kernel.org, Alistair Francis <Alistair.Francis@wdc.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
tglx@linutronix.de, linux-riscv@lists.infradead.org,
aou@eecs.berkeley.edu
Subject: Re: [PATCH v4 4/4] dt-bindings: timer: Add CLINT bindings
Date: Mon, 27 Jul 2020 07:47:32 -0400 [thread overview]
Message-ID: <c1a1b920-124c-23b1-6dd5-1a4cc54659cc@gmail.com> (raw)
In-Reply-To: <mhng-5833da8b-f9df-46c4-8dbe-0240e39d7bc5@palmerdabbelt-glaptop1>
On 7/26/20 2:37 PM, Palmer Dabbelt wrote:
> On Tue, 21 Jul 2020 20:55:31 PDT (-0700), anup@brainfault.org wrote:
>> On Tue, Jul 21, 2020 at 5:48 PM Sean Anderson <seanga2@gmail.com> wrote:
>>>
>>> On 7/20/20 9:15 PM, Atish Patra wrote:
>>> > On Fri, Jul 17, 2020 at 12:52 AM Anup Patel <anup.patel@wdc.com> wrote:
>>> >>
>>> >> We add DT bindings documentation for CLINT device.
>>> >>
>>> >> Signed-off-by: Anup Patel <anup.patel@wdc.com>
>>> >> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>> >> Tested-by: Emil Renner Berhing <kernel@esmil.dk>
>>> >> ---
>>> >> .../bindings/timer/sifive,clint.yaml | 58 +++++++++++++++++++
>>> >> 1 file changed, 58 insertions(+)
>>> >> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
>>> >>
>>> >> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
>>> >> new file mode 100644
>>> >> index 000000000000..8ad115611860
>>> >> --- /dev/null
>>> >> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
>>> >> @@ -0,0 +1,58 @@
>>> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> >> +%YAML 1.2
>>> >> +---
>>> >> +$id: http://devicetree.org/schemas/timer/sifive,clint.yaml#
>>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> >> +
>>> >> +title: SiFive Core Local Interruptor
>>> >> +
>>> >> +maintainers:
>>> >> + - Palmer Dabbelt <palmer@dabbelt.com>
>>> >> + - Anup Patel <anup.patel@wdc.com>
>>> >> +
>>> >> +description:
>>> >> + SiFive (and other RISC-V) SOCs include an implementation of the SiFive
>>> >> + Core Local Interruptor (CLINT) for M-mode timer and M-mode inter-processor
>>> >> + interrupts. It directly connects to the timer and inter-processor interrupt
>>> >> + lines of various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local
>>> >> + interrupt controller is the parent interrupt controller for CLINT device.
>>> >> + The clock frequency of CLINT is specified via "timebase-frequency" DT
>>> >> + property of "/cpus" DT node. The "timebase-frequency" DT property is
>>> >> + described in Documentation/devicetree/bindings/riscv/cpus.yaml
>>> >> +
>>> >> +properties:
>>> >> + compatible:
>>> >> + items:
>>> >> + - const: sifive,clint0
>>> >> + - const: sifive,fu540-c000-clint
>>> >> +
>>> >> + description:
>>> >> + Should be "sifive,<chip>-clint" and "sifive,clint<version>".
>>> >> + Supported compatible strings are -
>>> >> + "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
>>> >> + onto the SiFive FU540 chip, and "sifive,clint0" for the SiFive
>>> >> + CLINT v0 IP block with no chip integration tweaks.
>>> >> + Please refer to sifive-blocks-ip-versioning.txt for details
>>> >> +
>>> >
>>> > As the DT binding suggests that the clint device should be named as "sifive,**",
>>> > I think we should change the DT property in kendryte dts as well.
>>>
>>> The kendryte device is based on Rocket Chip, not any SiFive IP/device.
>>> If anything, the general binding should be "chipsalliance,clint" and the
>>> specific bindings should be "sifive,clint" and "kendryte,clint" (or
>>> "canaan,clint").
>>
>> AFAIK, Palmer clearly mentioned in previous discussion that CLINT
>> spec is still owned by SiFive. No matter who implements CLINT device
>> in their SOC, we will need one compatible string to represent the
>> spec version (i.e. "sifive,clint0") and another compatible representing
>> specific implementation (for kendryte this can be "kendryte,k210-clint").
>
> I think "sifive,clint*" is the way to go here. The Free Chips Foundation owns
> Rocket Chip, which contains an implementation of SiFive's CLINT specification,
> but as far as I can tell Free Chips itself does not produce a specification for
> the CLINT. The only CLINT specifications I can find are for SiFive's chips and
> are copyright SiFive, and we generally prefer sticking to specifications rather
> than implementations for these sorts of things.
Ah, I wasn't aware that compatibility string assignment was based on
published specifications.
>
> To be honest, I'm not even sure the Free Chips CLINT is an implementation of
> the SiFive specification: we don't have the source code for those chips, and
> while I'm fairly sure the Chisel source code for the CLINT itself on SiFive's
> chips is very close to what was in Rocket Chip at the time those chips were
> built (though we don't have the source, so we don't know for user), device
> specifications depend on the broader SOC context they are embedded inside. For
> example: SiFive's CLINT allows us to use simple MMIO reads of mtime to meet the
> RISC-V rdtime requirements, but other bus configurations may not meet those
> requirements.
>
> If Free Chips publishes a specification for a CLINT and it's compatible with
> this version of SiFive's CLINT then I don't see any reason why we couldn't add
> that as a compatible string, but as it currently stands there is no Free Chips
> CLINT specification. IMO it would be irresponsible for us to define one on
> their behalf.
>
> I don't know anything about Kendryte's specifications, as I haven't read them
> myself. Assuming they define the CLINT's behavior in their SOC manual, then
They don't. I emailed some people from Canaan and they said they used
rocketchip as their core. The best thing we have is the documentation in
their SDK [1]. FWIW, these comments almost exactly match the SiFive's
CLINT documentation [2]. I don't know whether that meets Linux's
standards for a "specification" or not.
> adding something along the lines of "canaan,kendryte-k210-clint" seems
> reasonable to me -- don't really care that much about the specific name, but as
> we don't define the Kendryte SOCs then calling it anything more general than
> this specific SOC's CLINT seems unreasonable. AFAIK the Kendryte people don't
> get involved with Linux development directly, so that's probably as much as we
> can define.
--Sean
[1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/include/clint.h
[2] e.g. chapter 9 of https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
prev parent reply other threads:[~2020-07-27 11:47 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-17 7:50 [PATCH v4 0/4] Dedicated CLINT timer driver Anup Patel
2020-07-17 7:50 ` [PATCH v4 1/4] RISC-V: Add mechanism to provide custom IPI operations Anup Patel
2020-07-21 0:46 ` Atish Patra
2020-07-17 7:50 ` [PATCH v4 2/4] clocksource/drivers: Add CLINT timer driver Anup Patel
2020-07-21 1:11 ` Atish Patra
2020-07-21 11:43 ` Anup Patel
2020-07-21 19:39 ` Atish Patra
2020-07-21 11:02 ` Daniel Lezcano
2020-07-21 11:49 ` Anup Patel
2020-07-21 12:15 ` Daniel Lezcano
2020-07-22 3:36 ` Anup Patel
2020-07-17 7:51 ` [PATCH v4 3/4] RISC-V: Remove CLINT related code from timer and arch Anup Patel
2020-07-17 7:51 ` [PATCH v4 4/4] dt-bindings: timer: Add CLINT bindings Anup Patel
2020-07-21 1:15 ` Atish Patra
2020-07-21 11:39 ` Anup Patel
2020-07-21 12:17 ` Sean Anderson
2020-07-22 3:55 ` Anup Patel
2020-07-26 18:37 ` Palmer Dabbelt
2020-07-27 11:47 ` Sean Anderson [this message]
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=c1a1b920-124c-23b1-6dd5-1a4cc54659cc@gmail.com \
--to=seanga2@gmail.com \
--cc=Alistair.Francis@wdc.com \
--cc=Anup.Patel@wdc.com \
--cc=Atish.Patra@wdc.com \
--cc=Damien.LeMoal@wdc.com \
--cc=aou@eecs.berkeley.edu \
--cc=atishp@atishpatra.org \
--cc=daniel.lezcano@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=kernel@esmil.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=robh+dt@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).