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=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=unavailable 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 1C825C4320A for ; Wed, 18 Aug 2021 19:56:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 055B6610A5 for ; Wed, 18 Aug 2021 19:56:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233496AbhHRT5K (ORCPT ); Wed, 18 Aug 2021 15:57:10 -0400 Received: from sibelius.xs4all.nl ([83.163.83.176]:56752 "EHLO sibelius.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232969AbhHRT4y (ORCPT ); Wed, 18 Aug 2021 15:56:54 -0400 Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id e0d5b866; Wed, 18 Aug 2021 21:56:10 +0200 (CEST) Date: Wed, 18 Aug 2021 21:56:10 +0200 (CEST) From: Mark Kettenis To: Rob Herring Cc: maz@kernel.org, devicetree@vger.kernel.org, robin.murphy@arm.com, sven@svenpeter.dev, kettenis@openbsd.org, marcan@marcan.st, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: (message from Rob Herring on Sun, 15 Aug 2021 14:19:57 -0500) Subject: Re: [PATCH v3 1/2] dt-bindings: pci: Add DT bindings for apple,pcie References: <20210726083204.93196-1-mark.kettenis@xs4all.nl> <20210726083204.93196-2-mark.kettenis@xs4all.nl> <20210726231848.GA1025245@robh.at.kernel.org> <87sfzt1pg9.wl-maz@kernel.org> <8735ra1x8t.wl-maz@kernel.org> Message-ID: <56140331bd735d61@bloch.sibelius.xs4all.nl> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > From: Rob Herring > Date: Sun, 15 Aug 2021 14:19:57 -0500 > > On Sun, Aug 15, 2021 at 11:36 AM Marc Zyngier wrote: > > > > Hi Rob, > > > > Apologies for the delay, I somehow misplaced this email... > > > > On Mon, 02 Aug 2021 17:10:39 +0100, > > Rob Herring wrote: > > > > > > On Sun, Aug 1, 2021 at 3:31 AM Marc Zyngier wrote: > > > > > > > > On Tue, 27 Jul 2021 00:18:48 +0100, > > > > Rob Herring wrote: > > > > > > > > > > On Mon, Jul 26, 2021 at 10:32:00AM +0200, Mark Kettenis wrote: > > > > > > From: Mark Kettenis > > > > > > > > > > > > The Apple PCIe host controller is a PCIe host controller with > > > > > > multiple root ports present in Apple ARM SoC platforms, including > > > > > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > > > > > > > > > Signed-off-by: Mark Kettenis > > > > > > --- > > > > > > .../devicetree/bindings/pci/apple,pcie.yaml | 166 ++++++++++++++++++ > > > > > > MAINTAINERS | 1 + > > > > > > 2 files changed, 167 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..bfcbdee79c64 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > @@ -0,0 +1,166 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Apple PCIe host controller > > > > > > + > > > > > > +maintainers: > > > > > > + - Mark Kettenis > > > > > > + > > > > > > +description: | > > > > > > + The Apple PCIe host controller is a PCIe host controller with > > > > > > + multiple root ports present in Apple ARM SoC platforms, including > > > > > > + various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > > > + The controller incorporates Synopsys DesigWare PCIe logic to > > > > > > + implements its root ports. But the ATU found on most DesignWare > > > > > > + PCIe host bridges is absent. > > > > > > > > > > blank line > > > > > > > > > > > + All root ports share a single ECAM space, but separate GPIOs are > > > > > > + used to take the PCI devices on those ports out of reset. Therefore > > > > > > + the standard "reset-gpio" and "max-link-speed" properties appear on > > > > > > > > > > reset-gpios > > > > > > > > > > > + the child nodes that represent the PCI bridges that correspond to > > > > > > + the individual root ports. > > > > > > > > > > blank line > > > > > > > > > > > + MSIs are handled by the PCIe controller and translated into regular > > > > > > + interrupts. A range of 32 MSIs is provided. These 32 MSIs can be > > > > > > + distributed over the root ports as the OS sees fit by programming > > > > > > + the PCIe controller's port registers. > > > > > > + > > > > > > +allOf: > > > > > > + - $ref: /schemas/pci/pci-bus.yaml# > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + items: > > > > > > + - const: apple,t8103-pcie > > > > > > + - const: apple,pcie > > > > > > + > > > > > > + reg: > > > > > > + minItems: 3 > > > > > > + maxItems: 5 > > > > > > + > > > > > > + reg-names: > > > > > > + minItems: 3 > > > > > > + maxItems: 5 > > > > > > + items: > > > > > > + - const: config > > > > > > + - const: rc > > > > > > + - const: port0 > > > > > > + - const: port1 > > > > > > + - const: port2 > > > > > > + > > > > > > + ranges: > > > > > > + minItems: 2 > > > > > > + maxItems: 2 > > > > > > + > > > > > > + interrupts: > > > > > > + description: > > > > > > + Interrupt specifiers, one for each root port. > > > > > > + minItems: 1 > > > > > > + maxItems: 3 > > > > > > + > > > > > > + msi-controller: true > > > > > > + msi-parent: true > > > > > > + > > > > > > + msi-ranges: > > > > > > + description: > > > > > > + A list of pairs , where "intid" is the first > > > > > > + interrupt number that can be used as an MSI, and "span" the size > > > > > > + of that range. > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > > > > > + items: > > > > > > + minItems: 2 > > > > > > + maxItems: 2 > > > > > > > > > > I still have issues I raised on v1 with this property. It's genericish > > > > > looking, but not generic. 'intid' as a single cell can't specify any > > > > > parent interrupt such as a GIC which uses 3 cells. You could put in all > > > > > the cells, but you'd still be assuming which cell you can increment. > > > > > > > > The GIC bindings already use similar abstractions, see what we do for > > > > both GICv2m and GICv3 MBIs. Other MSI controllers use similar > > > > properties (alpine and loongson, for example). > > > > > > That's the problem. Everyone making up their own crap. > > > > And that crap gets approved: > > > > https://lore.kernel.org/lkml/20200512205704.GA10412@bogus/ > > > > I'm not trying to be antagonistic here, but it seems that your > > position on this very subject has changed recently. > > Not really, I think it's not the first time we've discussed this. But > as I see things over and over, my tolerance for another instance > without solving the problem for everyone diminishes. And what other > leverage do I have? > > Additionally, how long we have to support something comes into play. I > have no idea for a Loongson MSI controller. I have a better idea on an > Apple product... > > > > > > I think you should just list all these under 'interrupts' using > > > > > interrupt-names to make your life easier: > > > > > > > > > > interrupt-names: > > > > > items: > > > > > - const: port0 > > > > > - const: port1 > > > > > - const: port2 > > > > > - const: msi0 > > > > > - const: msi1 > > > > > - const: msi2 > > > > > - const: msi3 > > > > > ... > > > > > > > > > > Yeah, it's kind of verbose, but if the h/w block handles N interrupts, > > > > > you should list N interrupts. The worst case for the above is N entries > > > > > too if not contiguous. > > > > > > > > And that's where I beg to differ, again. > > > > > > > > Specifying interrupts like this gives the false impression that these > > > > interrupts are generated by the device that owns them (the RC). Which > > > > for MSIs is not the case. > > > > > > It's no different than an interrupt controller node having an > > > interrupts property. The source is downstream and the interrupt > > > controller is combining/translating the interrupts. > > > > > > The physical interrupt signals are connected to and originating in > > > this block. > > > > Oh, I also object to this, for the same reasons. The only case where > > it makes sense IMHO is when the interrupt controller is a multiplexer. > > So we've had the same kind of property for interrupt multiplexers. I'm > fine if you think an 'MSI to interrupts mapping property' should be > named something else. > > > > That sounds like perfectly 'describing the h/w' to me. > > > > I guess we have a different view of about these things. At the end of > > the day, I don't care enough as long as we can expose a range of > > interrupts one way or another. > > I don't really either. I just don't want 10 ways AND another... > > > > > This is not only verbose, this is > > > > semantically dubious. And what should we do when the number of > > > > possible interrupt is ridiculously large, as it is for the GICv3 ITS? > > > > > > I don't disagree with the verbose part. But that's not really an issue > > > in this case. > > > > > > > I wish we had a standard way to express these constraints. Until we > > > > do, I don't think enumerating individual interrupts is a practical > > > > thing to do, nor that it actually represents the topology of the > > > > system. > > > > > > The only way a standard way will happen is to stop accepting the > > > custom properties. > > > > > > All the custom properties suffer from knowledge of what the parent > > > interrupt controller is. To fix that, I think we need something like > > > this: > > > > > > msi-ranges = , , ; > > > > > > 'intspec' is defined by the parent interrupt-controller cells. step is > > > the value to add. And end is what to match on to stop aka the last > > > interrupt in the range. For example, if the GIC is the parent, we'd > > > have something like this: > > > > > > , <0 1 0>, > > > > > > Does this apply to cases other than MSI? I think so as don't we have > > > the same type of properties with the low power mode shadow interrupt > > > controllers? So 'interrupt-ranges'? > > > > This would work, though the increment seems a bit over-engineered. You > > also may need this property to accept multiple ranges. > > Yes, certainly. Worst case is a map. > > > > It looks to me like there's an assumption in the kernel that an MSI > > > controller has a linear range of parent interrupts? Is that correct > > > and something that's guaranteed? That assumption leaks into the > > > existing bindings. > > > > Depends on how the controller works. In general, the range maps to the > > MultiMSI requirements where the message is an offset from the base of > > the interrupt range. So you generally end-up with ranges of at least > > 32 contiguous MSIs. Anything under that is sub-par and probably not > > worth supporting. > > Maybe just this is enough: > msi-ranges = , , , , ... > > While I say 'length' here, that's really up to the interrupt parent to > interpret the intspec cells. So for the Apple PCIe controller we'd have: msi-ranges = ; That would work just fine. Should this be documented in the apple,pcie binding, or somewhere more generic? > > Of course, the controller may have some mapping facilities, which > > makes things more... interesting. > > > > > It's fine for the kernel to assume that until there's a case that's > > > not linear, but a common binding needs to be able handle a > > > non-linear case. > > > > Fair enough. I can probably work with Mark to upgrade the binding and > > the M1 PCIe code. Could you come up with a more formalised proposal? > > Not my itch. > > Rob > 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=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, URIBL_BLOCKED autolearn=unavailable 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 926CCC4338F for ; Wed, 18 Aug 2021 19:58:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 65E0D60720 for ; Wed, 18 Aug 2021 19:58:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 65E0D60720 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=xs4all.nl Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:Message-ID:References:Subject :In-Reply-To:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vcj0/x9T73I+mg+yMqLrD6yJl3RFLYboo9nyuhVCdr0=; b=xPRVpqpzsEIzgi AvQa4UQagfpfmFUEEbzwqHfpb/zYV2BQC/7PWSVU7tlitKFtnK8dWcPXlecDQ5vpWv5R2KaEW3ry7 M6dsEsY3ArD2kkGFcuj8JfYv8U6kF0NNG8CEKkg5RTQq92BviSfnmMrBP/rcguc1FVuSTeDgdjrho +cNx5Lz5mCiADh8tYMjiAfjd0I6anQ3ZAxfiy4b6WpiNxRRxZDTH7i1VuyYbHOibsgBNSSzoJqopn jtCwQVGTtghlF1ylaHaGKbjH8vFNHJtalQNPBUzAvBdOH0Bi5iHaACEh4HkzPS9htIfQFjGr/odH8 fBN5A3mZpWguaOZpzJdA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGRfj-006alN-Ki; Wed, 18 Aug 2021 19:56:23 +0000 Received: from sibelius.xs4all.nl ([83.163.83.176]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGRfc-006akG-HO for linux-arm-kernel@lists.infradead.org; Wed, 18 Aug 2021 19:56:21 +0000 Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id e0d5b866; Wed, 18 Aug 2021 21:56:10 +0200 (CEST) Date: Wed, 18 Aug 2021 21:56:10 +0200 (CEST) From: Mark Kettenis To: Rob Herring Cc: maz@kernel.org, devicetree@vger.kernel.org, robin.murphy@arm.com, sven@svenpeter.dev, kettenis@openbsd.org, marcan@marcan.st, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: (message from Rob Herring on Sun, 15 Aug 2021 14:19:57 -0500) Subject: Re: [PATCH v3 1/2] dt-bindings: pci: Add DT bindings for apple,pcie References: <20210726083204.93196-1-mark.kettenis@xs4all.nl> <20210726083204.93196-2-mark.kettenis@xs4all.nl> <20210726231848.GA1025245@robh.at.kernel.org> <87sfzt1pg9.wl-maz@kernel.org> <8735ra1x8t.wl-maz@kernel.org> Message-ID: <56140331bd735d61@bloch.sibelius.xs4all.nl> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210818_125616_952028_34399264 X-CRM114-Status: GOOD ( 73.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > From: Rob Herring > Date: Sun, 15 Aug 2021 14:19:57 -0500 > > On Sun, Aug 15, 2021 at 11:36 AM Marc Zyngier wrote: > > > > Hi Rob, > > > > Apologies for the delay, I somehow misplaced this email... > > > > On Mon, 02 Aug 2021 17:10:39 +0100, > > Rob Herring wrote: > > > > > > On Sun, Aug 1, 2021 at 3:31 AM Marc Zyngier wrote: > > > > > > > > On Tue, 27 Jul 2021 00:18:48 +0100, > > > > Rob Herring wrote: > > > > > > > > > > On Mon, Jul 26, 2021 at 10:32:00AM +0200, Mark Kettenis wrote: > > > > > > From: Mark Kettenis > > > > > > > > > > > > The Apple PCIe host controller is a PCIe host controller with > > > > > > multiple root ports present in Apple ARM SoC platforms, including > > > > > > various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > > > > > > > > > Signed-off-by: Mark Kettenis > > > > > > --- > > > > > > .../devicetree/bindings/pci/apple,pcie.yaml | 166 ++++++++++++++++++ > > > > > > MAINTAINERS | 1 + > > > > > > 2 files changed, 167 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..bfcbdee79c64 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml > > > > > > @@ -0,0 +1,166 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/pci/apple,pcie.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Apple PCIe host controller > > > > > > + > > > > > > +maintainers: > > > > > > + - Mark Kettenis > > > > > > + > > > > > > +description: | > > > > > > + The Apple PCIe host controller is a PCIe host controller with > > > > > > + multiple root ports present in Apple ARM SoC platforms, including > > > > > > + various iPhone and iPad devices and the "Apple Silicon" Macs. > > > > > > + The controller incorporates Synopsys DesigWare PCIe logic to > > > > > > + implements its root ports. But the ATU found on most DesignWare > > > > > > + PCIe host bridges is absent. > > > > > > > > > > blank line > > > > > > > > > > > + All root ports share a single ECAM space, but separate GPIOs are > > > > > > + used to take the PCI devices on those ports out of reset. Therefore > > > > > > + the standard "reset-gpio" and "max-link-speed" properties appear on > > > > > > > > > > reset-gpios > > > > > > > > > > > + the child nodes that represent the PCI bridges that correspond to > > > > > > + the individual root ports. > > > > > > > > > > blank line > > > > > > > > > > > + MSIs are handled by the PCIe controller and translated into regular > > > > > > + interrupts. A range of 32 MSIs is provided. These 32 MSIs can be > > > > > > + distributed over the root ports as the OS sees fit by programming > > > > > > + the PCIe controller's port registers. > > > > > > + > > > > > > +allOf: > > > > > > + - $ref: /schemas/pci/pci-bus.yaml# > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + items: > > > > > > + - const: apple,t8103-pcie > > > > > > + - const: apple,pcie > > > > > > + > > > > > > + reg: > > > > > > + minItems: 3 > > > > > > + maxItems: 5 > > > > > > + > > > > > > + reg-names: > > > > > > + minItems: 3 > > > > > > + maxItems: 5 > > > > > > + items: > > > > > > + - const: config > > > > > > + - const: rc > > > > > > + - const: port0 > > > > > > + - const: port1 > > > > > > + - const: port2 > > > > > > + > > > > > > + ranges: > > > > > > + minItems: 2 > > > > > > + maxItems: 2 > > > > > > + > > > > > > + interrupts: > > > > > > + description: > > > > > > + Interrupt specifiers, one for each root port. > > > > > > + minItems: 1 > > > > > > + maxItems: 3 > > > > > > + > > > > > > + msi-controller: true > > > > > > + msi-parent: true > > > > > > + > > > > > > + msi-ranges: > > > > > > + description: > > > > > > + A list of pairs , where "intid" is the first > > > > > > + interrupt number that can be used as an MSI, and "span" the size > > > > > > + of that range. > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > > > > > > + items: > > > > > > + minItems: 2 > > > > > > + maxItems: 2 > > > > > > > > > > I still have issues I raised on v1 with this property. It's genericish > > > > > looking, but not generic. 'intid' as a single cell can't specify any > > > > > parent interrupt such as a GIC which uses 3 cells. You could put in all > > > > > the cells, but you'd still be assuming which cell you can increment. > > > > > > > > The GIC bindings already use similar abstractions, see what we do for > > > > both GICv2m and GICv3 MBIs. Other MSI controllers use similar > > > > properties (alpine and loongson, for example). > > > > > > That's the problem. Everyone making up their own crap. > > > > And that crap gets approved: > > > > https://lore.kernel.org/lkml/20200512205704.GA10412@bogus/ > > > > I'm not trying to be antagonistic here, but it seems that your > > position on this very subject has changed recently. > > Not really, I think it's not the first time we've discussed this. But > as I see things over and over, my tolerance for another instance > without solving the problem for everyone diminishes. And what other > leverage do I have? > > Additionally, how long we have to support something comes into play. I > have no idea for a Loongson MSI controller. I have a better idea on an > Apple product... > > > > > > I think you should just list all these under 'interrupts' using > > > > > interrupt-names to make your life easier: > > > > > > > > > > interrupt-names: > > > > > items: > > > > > - const: port0 > > > > > - const: port1 > > > > > - const: port2 > > > > > - const: msi0 > > > > > - const: msi1 > > > > > - const: msi2 > > > > > - const: msi3 > > > > > ... > > > > > > > > > > Yeah, it's kind of verbose, but if the h/w block handles N interrupts, > > > > > you should list N interrupts. The worst case for the above is N entries > > > > > too if not contiguous. > > > > > > > > And that's where I beg to differ, again. > > > > > > > > Specifying interrupts like this gives the false impression that these > > > > interrupts are generated by the device that owns them (the RC). Which > > > > for MSIs is not the case. > > > > > > It's no different than an interrupt controller node having an > > > interrupts property. The source is downstream and the interrupt > > > controller is combining/translating the interrupts. > > > > > > The physical interrupt signals are connected to and originating in > > > this block. > > > > Oh, I also object to this, for the same reasons. The only case where > > it makes sense IMHO is when the interrupt controller is a multiplexer. > > So we've had the same kind of property for interrupt multiplexers. I'm > fine if you think an 'MSI to interrupts mapping property' should be > named something else. > > > > That sounds like perfectly 'describing the h/w' to me. > > > > I guess we have a different view of about these things. At the end of > > the day, I don't care enough as long as we can expose a range of > > interrupts one way or another. > > I don't really either. I just don't want 10 ways AND another... > > > > > This is not only verbose, this is > > > > semantically dubious. And what should we do when the number of > > > > possible interrupt is ridiculously large, as it is for the GICv3 ITS? > > > > > > I don't disagree with the verbose part. But that's not really an issue > > > in this case. > > > > > > > I wish we had a standard way to express these constraints. Until we > > > > do, I don't think enumerating individual interrupts is a practical > > > > thing to do, nor that it actually represents the topology of the > > > > system. > > > > > > The only way a standard way will happen is to stop accepting the > > > custom properties. > > > > > > All the custom properties suffer from knowledge of what the parent > > > interrupt controller is. To fix that, I think we need something like > > > this: > > > > > > msi-ranges = , , ; > > > > > > 'intspec' is defined by the parent interrupt-controller cells. step is > > > the value to add. And end is what to match on to stop aka the last > > > interrupt in the range. For example, if the GIC is the parent, we'd > > > have something like this: > > > > > > , <0 1 0>, > > > > > > Does this apply to cases other than MSI? I think so as don't we have > > > the same type of properties with the low power mode shadow interrupt > > > controllers? So 'interrupt-ranges'? > > > > This would work, though the increment seems a bit over-engineered. You > > also may need this property to accept multiple ranges. > > Yes, certainly. Worst case is a map. > > > > It looks to me like there's an assumption in the kernel that an MSI > > > controller has a linear range of parent interrupts? Is that correct > > > and something that's guaranteed? That assumption leaks into the > > > existing bindings. > > > > Depends on how the controller works. In general, the range maps to the > > MultiMSI requirements where the message is an offset from the base of > > the interrupt range. So you generally end-up with ranges of at least > > 32 contiguous MSIs. Anything under that is sub-par and probably not > > worth supporting. > > Maybe just this is enough: > msi-ranges = , , , , ... > > While I say 'length' here, that's really up to the interrupt parent to > interpret the intspec cells. So for the Apple PCIe controller we'd have: msi-ranges = ; That would work just fine. Should this be documented in the apple,pcie binding, or somewhere more generic? > > Of course, the controller may have some mapping facilities, which > > makes things more... interesting. > > > > > It's fine for the kernel to assume that until there's a case that's > > > not linear, but a common binding needs to be able handle a > > > non-linear case. > > > > Fair enough. I can probably work with Mark to upgrade the binding and > > the M1 PCIe code. Could you come up with a more formalised proposal? > > Not my itch. > > Rob > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel