Re: [RFT PATCH v3 06/27] dt-bindings: timer: arm,arch_timer: Add interrupt-names support
From: Rob Herring <robh@kernel.org>
Date: 2021-03-09 16:12:26
Also in:
linux-arch, linux-arm-kernel, linux-devicetree, linux-samsung-soc, linux-serial, lkml
On Mon, Mar 8, 2021 at 3:42 PM Marc Zyngier [off-list ref] wrote:
On Mon, 08 Mar 2021 20:38:41 +0000, Rob Herring [off-list ref] wrote:quoted
On Fri, Mar 05, 2021 at 06:38:41AM +0900, Hector Martin wrote:quoted
Not all platforms provide the same set of timers/interrupts, and Linux only needs one (plus kvm/guest ones); some platforms are working around this by using dummy fake interrupts. Implementing interrupt-names allows the devicetree to specify an arbitrary set of available interrupts, so the timer code can pick the right one. This also adds the hyp-virt timer/interrupt, which was previously not expressed in the fixed 4-interrupt form. Signed-off-by: Hector Martin <redacted> --- .../devicetree/bindings/timer/arm,arch_timer.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml index 2c75105c1398..ebe9b0bebe41 100644 --- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml +++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml@@ -34,11 +34,25 @@ properties: - arm,armv8-timer interrupts: + minItems: 1 + maxItems: 5 items: - description: secure timer irq - description: non-secure timer irq - description: virtual timer irq - description: hypervisor timer irq + - description: hypervisor virtual timer irq + + interrupt-names: + minItems: 1 + maxItems: 5 + items: + enum: + - phys-secure + - phys + - virt + - hyp-phys + - hyp-virtphys-secure and hyp-phys is not very consistent. secure-phys or sec-phys instead? This allows any order which is not ideal (unfortunately json-schema doesn't have a way to define order with optional entries in the middle). How many possible combinations are there which make sense? If that's a reasonable number, I'd rather see them listed out.The available of interrupts are a function of the number of security states, privileged exception levels and architecture revisions, as described in D11.1.1: <quote> - An EL1 physical timer. - A Non-secure EL2 physical timer. - An EL3 physical timer. - An EL1 virtual timer. - A Non-secure EL2 virtual timer. - A Secure EL2 virtual timer. - A Secure EL2 physical timer. </quote> * Single security state, EL1 only, ARMv7 & ARMv8.0+ (assumed NS): - physical, virtual * Single security state, EL1 + EL2, ARMv7 & ARMv8.0 (assumed NS) - physical, virtual, hyp physical * Single security state, EL1 + EL2, ARMv8.1+ (assumed NS) - physical, virtual, hyp physical, hyp virtual * Two security states, EL1 + EL3, ARMv7 & ARMv8.0+: - secure physical, physical, virtual * Two security states, EL1 + EL2 + EL3, ARMv7 & ARMv8.0 - secure physical, physical, virtual, hyp physical * Two security states, EL1 + EL2 + EL3, ARMv8.1+ - secure physical, physical, virtual, hyp physical, hyp virtual * Two security states, EL1 + EL2 + S-EL2 + EL3, ARMv8.4+ - secure physical, physical, virtual, hyp physical, hyp virtual, secure hyp physical, secure hyp virtual Nobody has seen the last combination in the wild (that is, outside of a SW model). I'm really not convinced we want to express this kind of complexity in the binding (each of the 7 cases), specially given that we don't encode the underlying HW architecture level or number of exception levels anywhere, and have ho way to validate such information.
Actually, we can simplify this down to 2 cases:
oneOf:
- minItems: 2
items:
- const: phys
- const: virt
- const: hyp-phys
- const: hyp-virt
- minItems: 3
items:
- const: sec-phys
- const: phys
- const: virt
- const: hyp-phys
- const: hyp-virt
- const: sec-hyp-phy
- const: sec-hyp-virt
And that's below my threshold for not worth the complexity.
Rob