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.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 3D2AFC4338F for ; Sun, 1 Aug 2021 09:31:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 14A9560560 for ; Sun, 1 Aug 2021 09:31:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231653AbhHAJbf (ORCPT ); Sun, 1 Aug 2021 05:31:35 -0400 Received: from mail.kernel.org ([198.145.29.99]:51688 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231464AbhHAJb3 (ORCPT ); Sun, 1 Aug 2021 05:31:29 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1BBCE60243; Sun, 1 Aug 2021 09:31:21 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mA7oU-002GjF-W4; Sun, 01 Aug 2021 10:31:19 +0100 Date: Sun, 01 Aug 2021 10:31:18 +0100 Message-ID: <87sfzt1pg9.wl-maz@kernel.org> From: Marc Zyngier To: Rob Herring Cc: Mark Kettenis , devicetree@vger.kernel.org, robin.murphy@arm.com, sven@svenpeter.dev, Mark Kettenis , Hector Martin , Bjorn Helgaas , linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/2] dt-bindings: pci: Add DT bindings for apple,pcie In-Reply-To: <20210726231848.GA1025245@robh.at.kernel.org> References: <20210726083204.93196-1-mark.kettenis@xs4all.nl> <20210726083204.93196-2-mark.kettenis@xs4all.nl> <20210726231848.GA1025245@robh.at.kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: robh@kernel.org, mark.kettenis@xs4all.nl, 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 X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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). > 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. 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 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. Thanks, M. -- Without deviation from the norm, progress is not possible.