From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A6FCC433E0 for ; Tue, 9 Mar 2021 20:29:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D42076523A for ; Tue, 9 Mar 2021 20:29:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230320AbhCIU25 (ORCPT ); Tue, 9 Mar 2021 15:28:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51946 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229948AbhCIU2Z (ORCPT ); Tue, 9 Mar 2021 15:28:25 -0500 Received: from mail.marcansoft.com (marcansoft.com [IPv6:2a01:298:fe:f::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1028DC06174A; Tue, 9 Mar 2021 12:28:25 -0800 (PST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id 314363FA6A; Tue, 9 Mar 2021 20:28:15 +0000 (UTC) Subject: Re: [RFT PATCH v3 06/27] dt-bindings: timer: arm,arch_timer: Add interrupt-names support To: Rob Herring , Marc Zyngier Cc: linux-arm-kernel , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , Mark Rutland , Andy Shevchenko , Greg Kroah-Hartman , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree@vger.kernel.org, "open list:SERIAL DRIVERS" , Linux Doc Mailing List , linux-samsung-soc , "open list:GENERIC INCLUDE/ASM HEADER FILES" , "linux-kernel@vger.kernel.org" References: <20210304213902.83903-1-marcan@marcan.st> <20210304213902.83903-7-marcan@marcan.st> <20210308203841.GA2906683@robh.at.kernel.org> <87zgzdqnbs.wl-maz@kernel.org> From: Hector Martin Message-ID: Date: Wed, 10 Mar 2021 05:28:14 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: es-ES Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-arch@vger.kernel.org On 10/03/2021 01.11, Rob Herring wrote: > On Mon, Mar 8, 2021 at 3:42 PM Marc Zyngier wrote: >> >> On Mon, 08 Mar 2021 20:38:41 +0000, >> Rob Herring wrote: >>> >>> On Fri, Mar 05, 2021 at 06:38:41AM +0900, Hector Martin wrote: >>>> 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 >>>> --- >>>> .../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-virt >>> >>> phys-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: >> >> >> - 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. >> >> >> * 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. This makes sense. Since we aren't using the sec-hyp stuff here, and those go at the end of the list, we can omit them from this patch for now and add them whenever they're needed for a platform. Does that sound OK? -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub