diff mbox series

[v4,4/4] dt-bindings: timer: Add CLINT bindings

Message ID 20200717075101.263332-5-anup.patel@wdc.com
State Superseded, archived
Headers show
Series Dedicated CLINT timer driver | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success

Commit Message

Anup Patel July 17, 2020, 7:51 a.m. UTC
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

Comments

Atish Patra July 21, 2020, 1:15 a.m. UTC | #1
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.

> +  reg:
> +    maxItems: 1
> +
> +  interrupts-extended:
> +    minItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts-extended
> +
> +examples:
> +  - |
> +    clint@2000000 {
> +      compatible = "sifive,clint0", "sifive,fu540-c000-clint";
> +      interrupts-extended = <&cpu1intc 3 &cpu1intc 7
> +                             &cpu2intc 3 &cpu2intc 7
> +                             &cpu3intc 3 &cpu3intc 7
> +                             &cpu4intc 3 &cpu4intc 7>;
> +       reg = <0x2000000 0x4000000>;
> +    };
> +...
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Otherwise,

Reviewed-by: Atish Patra <atish.patra@wdc.com>
Anup Patel July 21, 2020, 11:39 a.m. UTC | #2
On Tue, Jul 21, 2020 at 6:45 AM Atish Patra <atishp@atishpatra.org> 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.

Okay, I will do it as a separate patch.

>
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts-extended:
> > +    minItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts-extended
> > +
> > +examples:
> > +  - |
> > +    clint@2000000 {
> > +      compatible = "sifive,clint0", "sifive,fu540-c000-clint";
> > +      interrupts-extended = <&cpu1intc 3 &cpu1intc 7
> > +                             &cpu2intc 3 &cpu2intc 7
> > +                             &cpu3intc 3 &cpu3intc 7
> > +                             &cpu4intc 3 &cpu4intc 7>;
> > +       reg = <0x2000000 0x4000000>;
> > +    };
> > +...
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> Otherwise,
>
> Reviewed-by: Atish Patra <atish.patra@wdc.com>
>
> --
> Regards,
> Atish

Regards,
Anup
Sean Anderson July 21, 2020, 12:17 p.m. UTC | #3
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").

--Sean
Anup Patel July 22, 2020, 3:55 a.m. UTC | #4
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").

Regards,
Anup
Palmer Dabbelt July 26, 2020, 6:37 p.m. UTC | #5
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.

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
requirement<F6><F5>s.

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
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 Anderson July 27, 2020, 11:47 a.m. UTC | #6
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
diff mbox series

Patch

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
+
+  reg:
+    maxItems: 1
+
+  interrupts-extended:
+    minItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts-extended
+
+examples:
+  - |
+    clint@2000000 {
+      compatible = "sifive,clint0", "sifive,fu540-c000-clint";
+      interrupts-extended = <&cpu1intc 3 &cpu1intc 7
+                             &cpu2intc 3 &cpu2intc 7
+                             &cpu3intc 3 &cpu3intc 7
+                             &cpu4intc 3 &cpu4intc 7>;
+       reg = <0x2000000 0x4000000>;
+    };
+...