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=-4.0 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no 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 36160C43214 for ; Mon, 30 Aug 2021 20:20:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1CC9960F5B for ; Mon, 30 Aug 2021 20:20:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236935AbhH3UVR (ORCPT ); Mon, 30 Aug 2021 16:21:17 -0400 Received: from mail.kernel.org ([198.145.29.99]:52516 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232906AbhH3UVO (ORCPT ); Mon, 30 Aug 2021 16:21:14 -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 B895760F5C; Mon, 30 Aug 2021 20:20:20 +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 1mKnlS-00852D-9H; Mon, 30 Aug 2021 21:20:18 +0100 Date: Mon, 30 Aug 2021 21:20:17 +0100 Message-ID: <87o89ed6ri.wl-maz@kernel.org> From: Marc Zyngier To: Rob Herring Cc: Mark Kettenis , devicetree@vger.kernel.org, Alyssa Rosenzweig , Mark Kettenis , Thomas Gleixner , Hector Martin , Bjorn Helgaas , Nicolas Saenz Julienne , Jim Quinlan , Florian Fainelli , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , Daire McNamara , Saenz Julienne , "linux-kernel@vger.kernel.org" , linux-arm-kernel , PCI , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" Subject: Re: [PATCH v4 4/4] arm64: apple: Add PCIe node In-Reply-To: References: <20210827171534.62380-1-mark.kettenis@xs4all.nl> <20210827171534.62380-5-mark.kettenis@xs4all.nl> <87pmtvcgec.wl-maz@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+dt@kernel.org, mark.kettenis@xs4all.nl, devicetree@vger.kernel.org, alyssa@rosenzweig.io, kettenis@openbsd.org, tglx@linutronix.de, marcan@marcan.st, bhelgaas@google.com, nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com, daire.mcnamara@microchip.com, nsaenzjulienne@suse.de, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-rpi-kernel@lists.infradead.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 Mon, 30 Aug 2021 16:57:59 +0100, Rob Herring wrote: > > On Mon, Aug 30, 2021 at 6:37 AM Marc Zyngier wrote: > > > > I have now implemented the MSI change on the Linux driver side, and it > > works nicely. So thumbs up from me on this front. > > > > I am now looking at the interrupts provided by each port: > > (1) a bunch of port-private interrupts (link up/down...) > > (2) INTx interrupts > > So each port has an independent INTx space? Yes. > Is that even something PCI defines or comprehends? Can't see why not. That's no different from having several PCI busses. I don't think anything enforces that INTx interrupts have to be unique across the system. As long as they are unique across a PCI hierarchy, we should be OK. > > > Given that the programming is per-port, I've implemented this as a > > per-port interrupt controller. > > > > (1) is dead easy to implement, and doesn't require any DT description. > > (2) is unfortunately exposing the limits of my DT knowledge, and I'm > > not clear how to model it. I came up with the following: > > > > port00: pci@0,0 { > > device_type = "pci"; > > reg = <0x0 0x0 0x0 0x0 0x0>; > > reset-gpios = <&pinctrl_ap 152 0>; > > max-link-speed = <2>; > > > > #address-cells = <3>; > > #size-cells = <2>; > > ranges; > > > > interrupt-controller; > > #interrupt-cells = <1>; > > interrupt-parent = <&port00>; > > interrupt-map-mask = <0 0 0 7>; > > interrupt-map = <0 0 0 1 &port00 0>, > > <0 0 0 2 &port00 1>, > > <0 0 0 3 &port00 2>, > > <0 0 0 4 &port00 3>; > > IIRC, I don't think the DT IRQ code handles a node having both > 'interrupt-controller' and 'interrupt-map' properties. Indeed, and that actually explains why the damned INTx interrupts insist on being 1-based instead of 0-based as the above mapping attempts to describe it. Turns out I can rip the interrupt-map out and it isn't worse. > I think that's why some PCI host bridge nodes have child > interrupt-controller nodes. I don't really like that work-around, > so if the above can be made to work, I'd be happy to see it. But the > DT IRQ code is some ancient code for ancient platforms (PowerMacs > being one of them). That'd probably need some massaging. I'll have a look. I checked that if I add something like: interrupts-extended = <&port02 2>; to each port, I get the PME interrupt correctly assigned should I pass pcie_pme=nomsi. Given that this IP is pretty limited in terms of MSIs, every bit that can free a MSI is welcome. I guess that it would make sense to expand this support to also match for an interrupt-map. > > > }; > > > > which vaguely seem to do the right thing for the devices behind root > > ports, but doesn't seem to work for INTx generated by the root ports > > themselves. Any clue? Alternatively, I could move it to something > > global to the whole PCIe controller, but that doesn't seem completely > > right. I've investigated this one further, and it looks like the DT IRQ code insists on trying to find the interrupt in the main pcie node instead of in the root port itself. But of course it doesn't want to parse an interrupt-map at that level either. I guess that's related to the above. > > > > It also begs the question whether the per-port interrupt to the AIC > > should be moved into each root port, should my per-port approach hold > > any water. > > I tend to think per-port is the right thing to do. However, the child > nodes are PCI devices, so that creates some restrictions. Such as the > per port registers are in the host address space, not the PCI address > space, so we can't move the registers into the child nodes. The > interrupts may be okay. Certainly, being an 'interrupt-controller' > without having an 'interrupts' property for an non root interrupt > controller is odd. That was my own impression as well. I guess there is no real canonical way to handle this particular system and to fully support it, we'll have to amend the current infrastructure. The question is: what is the least ugly way to express this that will work reasonably across implementations (OpenBSD, Linux, u-boot)? M. -- Without deviation from the norm, progress is not possible. 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=-4.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no 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 738E1C432BE for ; Mon, 30 Aug 2021 20:22:18 +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 3495C60F42 for ; Mon, 30 Aug 2021 20:22:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3495C60F42 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=6Gnib5iNdGhr3XpGhA1KKqnm6ve00uwVpMdZhdJXmK8=; b=bUgI+U3fL+Gk0p GTyyl+NZ137X8Ikv3L4eCPIueLe0dUJGN1tnivIRV++DZ+GZ7QMLxHVTKAOM0IaHbzLWFNUp0gFn0 UreV5HMNFgW9+6KrH+H5O0UYwKu/ym9PpMjDu0/qhOKNlrIVfow5qRcARkqvMm5OH4aBBZAOSjpIS ZknQjbmj+Hr0DTSScJttIiLPhi8XdzMIJOzfz6RW9LmJTHhGD1JCvH9s/UxSE0WqqpVwHpNlz3hpQ btclZVTBaFhNpzeaXdN8WXKTMBzgq1Ye1BjWtCgOb6WZggaNykmlkjncQZzfJBGUjmp0ezAwOFbwv i30nSbYbgKGsN1rV9ifg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mKnlZ-000UXQ-Ex; Mon, 30 Aug 2021 20:20:25 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mKnlV-000UWq-3p; Mon, 30 Aug 2021 20:20:22 +0000 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 B895760F5C; Mon, 30 Aug 2021 20:20:20 +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 1mKnlS-00852D-9H; Mon, 30 Aug 2021 21:20:18 +0100 Date: Mon, 30 Aug 2021 21:20:17 +0100 Message-ID: <87o89ed6ri.wl-maz@kernel.org> From: Marc Zyngier To: Rob Herring Cc: Mark Kettenis , devicetree@vger.kernel.org, Alyssa Rosenzweig , Mark Kettenis , Thomas Gleixner , Hector Martin , Bjorn Helgaas , Nicolas Saenz Julienne , Jim Quinlan , Florian Fainelli , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , Daire McNamara , Saenz Julienne , "linux-kernel@vger.kernel.org" , linux-arm-kernel , PCI , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" Subject: Re: [PATCH v4 4/4] arm64: apple: Add PCIe node In-Reply-To: References: <20210827171534.62380-1-mark.kettenis@xs4all.nl> <20210827171534.62380-5-mark.kettenis@xs4all.nl> <87pmtvcgec.wl-maz@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") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: robh+dt@kernel.org, mark.kettenis@xs4all.nl, devicetree@vger.kernel.org, alyssa@rosenzweig.io, kettenis@openbsd.org, tglx@linutronix.de, marcan@marcan.st, bhelgaas@google.com, nsaenz@kernel.org, jim2101024@gmail.com, f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com, daire.mcnamara@microchip.com, nsaenzjulienne@suse.de, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-rpi-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210830_132021_239660_24873322 X-CRM114-Status: GOOD ( 45.01 ) 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: , 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 On Mon, 30 Aug 2021 16:57:59 +0100, Rob Herring wrote: > > On Mon, Aug 30, 2021 at 6:37 AM Marc Zyngier wrote: > > > > I have now implemented the MSI change on the Linux driver side, and it > > works nicely. So thumbs up from me on this front. > > > > I am now looking at the interrupts provided by each port: > > (1) a bunch of port-private interrupts (link up/down...) > > (2) INTx interrupts > > So each port has an independent INTx space? Yes. > Is that even something PCI defines or comprehends? Can't see why not. That's no different from having several PCI busses. I don't think anything enforces that INTx interrupts have to be unique across the system. As long as they are unique across a PCI hierarchy, we should be OK. > > > Given that the programming is per-port, I've implemented this as a > > per-port interrupt controller. > > > > (1) is dead easy to implement, and doesn't require any DT description. > > (2) is unfortunately exposing the limits of my DT knowledge, and I'm > > not clear how to model it. I came up with the following: > > > > port00: pci@0,0 { > > device_type = "pci"; > > reg = <0x0 0x0 0x0 0x0 0x0>; > > reset-gpios = <&pinctrl_ap 152 0>; > > max-link-speed = <2>; > > > > #address-cells = <3>; > > #size-cells = <2>; > > ranges; > > > > interrupt-controller; > > #interrupt-cells = <1>; > > interrupt-parent = <&port00>; > > interrupt-map-mask = <0 0 0 7>; > > interrupt-map = <0 0 0 1 &port00 0>, > > <0 0 0 2 &port00 1>, > > <0 0 0 3 &port00 2>, > > <0 0 0 4 &port00 3>; > > IIRC, I don't think the DT IRQ code handles a node having both > 'interrupt-controller' and 'interrupt-map' properties. Indeed, and that actually explains why the damned INTx interrupts insist on being 1-based instead of 0-based as the above mapping attempts to describe it. Turns out I can rip the interrupt-map out and it isn't worse. > I think that's why some PCI host bridge nodes have child > interrupt-controller nodes. I don't really like that work-around, > so if the above can be made to work, I'd be happy to see it. But the > DT IRQ code is some ancient code for ancient platforms (PowerMacs > being one of them). That'd probably need some massaging. I'll have a look. I checked that if I add something like: interrupts-extended = <&port02 2>; to each port, I get the PME interrupt correctly assigned should I pass pcie_pme=nomsi. Given that this IP is pretty limited in terms of MSIs, every bit that can free a MSI is welcome. I guess that it would make sense to expand this support to also match for an interrupt-map. > > > }; > > > > which vaguely seem to do the right thing for the devices behind root > > ports, but doesn't seem to work for INTx generated by the root ports > > themselves. Any clue? Alternatively, I could move it to something > > global to the whole PCIe controller, but that doesn't seem completely > > right. I've investigated this one further, and it looks like the DT IRQ code insists on trying to find the interrupt in the main pcie node instead of in the root port itself. But of course it doesn't want to parse an interrupt-map at that level either. I guess that's related to the above. > > > > It also begs the question whether the per-port interrupt to the AIC > > should be moved into each root port, should my per-port approach hold > > any water. > > I tend to think per-port is the right thing to do. However, the child > nodes are PCI devices, so that creates some restrictions. Such as the > per port registers are in the host address space, not the PCI address > space, so we can't move the registers into the child nodes. The > interrupts may be okay. Certainly, being an 'interrupt-controller' > without having an 'interrupts' property for an non root interrupt > controller is odd. That was my own impression as well. I guess there is no real canonical way to handle this particular system and to fully support it, we'll have to amend the current infrastructure. The question is: what is the least ugly way to express this that will work reasonably across implementations (OpenBSD, Linux, u-boot)? M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel