The RISC-V PLIC specification unfortunately allows PLIC implementations
to ignore edges seen while an edge-triggered interrupt is being handled:
Depending on the design of the device and the interrupt handler,
in between sending an interrupt request and receiving notice of its
handler’s completion, the gateway might either ignore additional
matching edges or increment a counter of pending interrupts.
For PLICs with that misfeature, software needs to know the trigger type
of each interrupt. This allows it to work around the issue by completing
edge-triggered interrupts before handling them. Such a workaround is
required to avoid missing any edges.
The T-HEAD C9xx PLIC is an example of a PLIC with this behavior.
Signed-off-by: Samuel Holland <[email protected]>
---
.../sifive,plic-1.0.0.yaml | 31 ++++++++++++++++---
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index 27092c6a86c4..3c589cbca851 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -26,9 +26,13 @@ description:
with priority below this threshold will not cause the PLIC to raise its
interrupt line leading to the context.
- While the PLIC supports both edge-triggered and level-triggered interrupts,
- interrupt handlers are oblivious to this distinction and therefore it is not
- specified in the PLIC device-tree binding.
+ The PLIC supports both edge-triggered and level-triggered interrupts. For
+ edge-triggered interrupts, the RISC-V PLIC spec allows two responses to edges
+ seen while an interrupt handler is active; the PLIC may either queue them or
+ ignore them. In the first case, handlers are oblivious to the trigger type, so
+ it is not included in the interrupt specifier. In the second case, software
+ needs to know the trigger type, so it can reorder the interrupt flow to avoid
+ missing interrupts.
While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
"sifive,plic-1.0.0" device is a concrete implementation of the PLIC that
@@ -65,7 +69,8 @@ properties:
const: 0
'#interrupt-cells':
- const: 1
+ minimum: 1
+ maximum: 2
interrupt-controller: true
@@ -91,6 +96,24 @@ required:
- interrupts-extended
- riscv,ndev
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - thead,c900-plic
+
+ then:
+ properties:
+ '#interrupt-cells':
+ const: 2
+
+ else:
+ properties:
+ '#interrupt-cells':
+ const: 1
+
additionalProperties: false
examples:
--
2.35.1
On Mon, Jun 27, 2022 at 1:13 PM Samuel Holland <[email protected]> wrote:
>
> The RISC-V PLIC specification unfortunately allows PLIC implementations
> to ignore edges seen while an edge-triggered interrupt is being handled:
>
> Depending on the design of the device and the interrupt handler,
> in between sending an interrupt request and receiving notice of its
> handler’s completion, the gateway might either ignore additional
> matching edges or increment a counter of pending interrupts.
>
> For PLICs with that misfeature, software needs to know the trigger type
> of each interrupt. This allows it to work around the issue by completing
> edge-triggered interrupts before handling them. Such a workaround is
> required to avoid missing any edges.
>
> The T-HEAD C9xx PLIC is an example of a PLIC with this behavior.
Actually, C9xx support pulse signals which configed by
pad_plic_int_cfg_x for SoC vendor:
https://github.com/T-head-Semi/openc906/blob/main/C906_RTL_FACTORY/gen_rtl/plic/rtl/plic_int_kid.v
104: assign int_new_pending = pad_plic_int_cfg_x ? int_pulse
105:
: level_int_pending;
They could put pad_plic_int_cfg_x into the SoC software config
registers region or bind them to constant values.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> .../sifive,plic-1.0.0.yaml | 31 ++++++++++++++++---
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> index 27092c6a86c4..3c589cbca851 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> @@ -26,9 +26,13 @@ description:
> with priority below this threshold will not cause the PLIC to raise its
> interrupt line leading to the context.
>
> - While the PLIC supports both edge-triggered and level-triggered interrupts,
> - interrupt handlers are oblivious to this distinction and therefore it is not
> - specified in the PLIC device-tree binding.
> + The PLIC supports both edge-triggered and level-triggered interrupts. For
> + edge-triggered interrupts, the RISC-V PLIC spec allows two responses to edges
> + seen while an interrupt handler is active; the PLIC may either queue them or
> + ignore them. In the first case, handlers are oblivious to the trigger type, so
> + it is not included in the interrupt specifier. In the second case, software
> + needs to know the trigger type, so it can reorder the interrupt flow to avoid
> + missing interrupts.
>
> While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
> "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that
> @@ -65,7 +69,8 @@ properties:
> const: 0
>
> '#interrupt-cells':
> - const: 1
> + minimum: 1
> + maximum: 2
>
> interrupt-controller: true
>
> @@ -91,6 +96,24 @@ required:
> - interrupts-extended
> - riscv,ndev
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - thead,c900-plic
> +
> + then:
> + properties:
> + '#interrupt-cells':
> + const: 2
> +
> + else:
> + properties:
> + '#interrupt-cells':
> + const: 1
> +
> additionalProperties: false
>
> examples:
> --
> 2.35.1
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On 6/27/22 2:40 AM, Guo Ren wrote:
> On Mon, Jun 27, 2022 at 1:13 PM Samuel Holland <[email protected]> wrote:
>>
>> The RISC-V PLIC specification unfortunately allows PLIC implementations
>> to ignore edges seen while an edge-triggered interrupt is being handled:
>>
>> Depending on the design of the device and the interrupt handler,
>> in between sending an interrupt request and receiving notice of its
>> handler’s completion, the gateway might either ignore additional
>> matching edges or increment a counter of pending interrupts.
>>
>> For PLICs with that misfeature, software needs to know the trigger type
>> of each interrupt. This allows it to work around the issue by completing
>> edge-triggered interrupts before handling them. Such a workaround is
>> required to avoid missing any edges.
>>
>> The T-HEAD C9xx PLIC is an example of a PLIC with this behavior.
> Actually, C9xx support pulse signals which configed by
> pad_plic_int_cfg_x for SoC vendor:
>
> https://github.com/T-head-Semi/openc906/blob/main/C906_RTL_FACTORY/gen_rtl/plic/rtl/plic_int_kid.v
> 104: assign int_new_pending = pad_plic_int_cfg_x ? int_pulse
> 105:
> : level_int_pending;
>
> They could put pad_plic_int_cfg_x into the SoC software config
> registers region or bind them to constant values.
The issue here is the "!int_active" condition for int_new_set_pending,
regardless of that configuration input.
For interrupt sources that send pulses, those pulses will be lost if they are
received while int_active is true. So the driver has to make sure int_active is
false _before_ servicing an interrupt, or it may miss the next one. This means
the driver needs to know which interrupt _sources_ send pulses and which ones
assert levels.
For level interrupts, pad_plic_int_cfg_x had better be 0, but that is a
hardware/firmware configuration concern. The driver should not have to care
about that.
(On the Allwinner D1, those inputs are memory mapped, which was the reason for
the stacked interrupt controller mentioned in my other reply. But while
pad_plic_int_cfg_x == 1 only works with edge interrupts, the pad_plic_int_cfg_x
== 0 choice works with either kind of interrupt source, so the stacked driver is
not really needed.)
Regards,
Samuel
On Mon, Jun 27, 2022 at 10:14 PM Samuel Holland <[email protected]> wrote:
>
> On 6/27/22 2:40 AM, Guo Ren wrote:
> > On Mon, Jun 27, 2022 at 1:13 PM Samuel Holland <[email protected]> wrote:
> >>
> >> The RISC-V PLIC specification unfortunately allows PLIC implementations
> >> to ignore edges seen while an edge-triggered interrupt is being handled:
> >>
> >> Depending on the design of the device and the interrupt handler,
> >> in between sending an interrupt request and receiving notice of its
> >> handler’s completion, the gateway might either ignore additional
> >> matching edges or increment a counter of pending interrupts.
> >>
> >> For PLICs with that misfeature, software needs to know the trigger type
> >> of each interrupt. This allows it to work around the issue by completing
> >> edge-triggered interrupts before handling them. Such a workaround is
> >> required to avoid missing any edges.
> >>
> >> The T-HEAD C9xx PLIC is an example of a PLIC with this behavior.
> > Actually, C9xx support pulse signals which configed by
> > pad_plic_int_cfg_x for SoC vendor:
> >
> > https://github.com/T-head-Semi/openc906/blob/main/C906_RTL_FACTORY/gen_rtl/plic/rtl/plic_int_kid.v
> > 104: assign int_new_pending = pad_plic_int_cfg_x ? int_pulse
> > 105:
> > : level_int_pending;
> >
> > They could put pad_plic_int_cfg_x into the SoC software config
> > registers region or bind them to constant values.
>
> The issue here is the "!int_active" condition for int_new_set_pending,
> regardless of that configuration input.
>
> For interrupt sources that send pulses, those pulses will be lost if they are
> received while int_active is true. So the driver has to make sure int_active is
> false _before_ servicing an interrupt, or it may miss the next one. This means
> the driver needs to know which interrupt _sources_ send pulses and which ones
> assert levels.
You are right, in plus mode, we can't receive any interrupt between
claim & complete, then the irq could be lost.
I think the design follows the sentence written by PLIC spec:
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc
If the global interrupt source was edge-triggered, the gateway will
convert the first matching signal edge into an interrupt request.
Depending on the design of the device and the interrupt handler, in
between sending an interrupt request and receiving notice of its
handler’s completion, the gateway might either **ignore additional
matching edges** or increment a counter of pending interrupts.
Also, the hardware could easily support the feature, but the engineer
follows the first choice and disable the useful function. -_*!
>
> For level interrupts, pad_plic_int_cfg_x had better be 0, but that is a
> hardware/firmware configuration concern. The driver should not have to care
> about that.
>
> (On the Allwinner D1, those inputs are memory mapped, which was the reason for
> the stacked interrupt controller mentioned in my other reply. But while
> pad_plic_int_cfg_x == 1 only works with edge interrupts, the pad_plic_int_cfg_x
> == 0 choice works with either kind of interrupt source, so the stacked driver is
> not really needed.)
>
> Regards,
> Samuel
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Reviewed-by: Guo Ren <[email protected]>
On Mon, Jun 27, 2022 at 1:13 PM Samuel Holland <[email protected]> wrote:
>
> The RISC-V PLIC specification unfortunately allows PLIC implementations
> to ignore edges seen while an edge-triggered interrupt is being handled:
>
> Depending on the design of the device and the interrupt handler,
> in between sending an interrupt request and receiving notice of its
> handler’s completion, the gateway might either ignore additional
> matching edges or increment a counter of pending interrupts.
>
> For PLICs with that misfeature, software needs to know the trigger type
> of each interrupt. This allows it to work around the issue by completing
> edge-triggered interrupts before handling them. Such a workaround is
> required to avoid missing any edges.
>
> The T-HEAD C9xx PLIC is an example of a PLIC with this behavior.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> .../sifive,plic-1.0.0.yaml | 31 ++++++++++++++++---
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> index 27092c6a86c4..3c589cbca851 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> @@ -26,9 +26,13 @@ description:
> with priority below this threshold will not cause the PLIC to raise its
> interrupt line leading to the context.
>
> - While the PLIC supports both edge-triggered and level-triggered interrupts,
> - interrupt handlers are oblivious to this distinction and therefore it is not
> - specified in the PLIC device-tree binding.
> + The PLIC supports both edge-triggered and level-triggered interrupts. For
> + edge-triggered interrupts, the RISC-V PLIC spec allows two responses to edges
> + seen while an interrupt handler is active; the PLIC may either queue them or
> + ignore them. In the first case, handlers are oblivious to the trigger type, so
> + it is not included in the interrupt specifier. In the second case, software
> + needs to know the trigger type, so it can reorder the interrupt flow to avoid
> + missing interrupts.
>
> While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
> "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that
> @@ -65,7 +69,8 @@ properties:
> const: 0
>
> '#interrupt-cells':
> - const: 1
> + minimum: 1
> + maximum: 2
>
> interrupt-controller: true
>
> @@ -91,6 +96,24 @@ required:
> - interrupts-extended
> - riscv,ndev
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - thead,c900-plic
> +
> + then:
> + properties:
> + '#interrupt-cells':
> + const: 2
> +
> + else:
> + properties:
> + '#interrupt-cells':
> + const: 1
> +
> additionalProperties: false
>
> examples:
> --
> 2.35.1
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/