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.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 E8C20C43461 for ; Fri, 4 Sep 2020 15:48:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5749206E7 for ; Fri, 4 Sep 2020 15:48:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599234531; bh=vCpPDFBI/YTj3r0gZ1Lk+JMh06anXpHdDelgk/Yroqs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=NOSXabgqUAJjepDLCP7AryjmT8AiI0UbilpHFOBFjMzB+rjdAUyYotvnzImRDm4mm 6QiZOD1yQeBCoIbECHlVV5Bkc1MKZOu0OrOi290Qs132uHnzeYWNdliHT5Q1aqRdGc TipNuMyBLMwKanZn4g+3JDzZI14iJe783WtgKp0Y= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727884AbgIDPsr (ORCPT ); Fri, 4 Sep 2020 11:48:47 -0400 Received: from mail.kernel.org ([198.145.29.99]:42856 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727877AbgIDPso (ORCPT ); Fri, 4 Sep 2020 11:48:44 -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 615A3206E7; Fri, 4 Sep 2020 15:48:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599234523; bh=vCpPDFBI/YTj3r0gZ1Lk+JMh06anXpHdDelgk/Yroqs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=hblZpg2RnolUwCmaVnfkcv5nnfC5YRQi4H9Ze3Ui/uhjJPtpTJh2aNSU+BOVck5sc U/fNvP/qOcuRApyEZEUD357dQuODreuxbxdIHcZDEuKG3RHfAQjH+ZadT0PzhyPUfo 5gL3OzvsTXG3GrIDp79qT6DXJrIEETiTjgnBqwrs= Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kEDxB-009DDO-MC; Fri, 04 Sep 2020 16:48:41 +0100 Date: Fri, 04 Sep 2020 16:48:35 +0100 Message-ID: <87ft7xr19o.wl-maz@kernel.org> From: Marc Zyngier To: Thierry Reding Cc: Bjorn Helgaas , Lorenzo Pieralisi , Jonathan Hunter , Jason Gunthorpe , Thomas Gleixner , linux-pci@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: tegra: Convert to MSI domains In-Reply-To: <20200904105613.444945-1-thierry.reding@gmail.com> References: <20200904105613.444945-1-thierry.reding@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26.3 (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: 62.31.163.78 X-SA-Exim-Rcpt-To: thierry.reding@gmail.com, bhelgaas@google.com, lorenzo.pieralisi@arm.com, jonathanh@nvidia.com, jgg@nvidia.com, tglx@linutronix.de, linux-pci@vger.kernel.org, linux-tegra@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 Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org Hi Thierry, On Fri, 04 Sep 2020 11:56:13 +0100, Thierry Reding wrote: > > From: Marc Zyngier > > In anticipation of the removal of the msi_controller structure, convert > the Tegra host controller driver to MSI domains. > > We end-up with the usual two domain structure, the top one being a > generic PCI/MSI domain, the bottom one being Tegra-specific and handling > the actual HW interrupt allocation. > > While at it, convert the normal interrupt handler to a chained handler, > handle the controller's MSI IRQ edge triggered, support multiple MSIs > per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking. > > Signed-off-by: Marc Zyngier > [treding@nvidia.com: fix, clean up and address TODOs from Marc's draft] > Signed-off-by: Thierry Reding > --- > This is basically Marc's patch from here: > > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/tegra-msi&id=8ba6858f07a7554d4e56a6d5103cd914dcf23af0 > > That didn't work as-is, but it was pretty easy to get it to build and > run properly. Once I had that I went through Marc's list of TODOs and > addressed them, which was also not very difficult. > > I haven't done any extensive testing, but I can boot fine over NFS with > a PCI Ethernet card. Given how fundamental this is, I would expect that > to be a good enough test. > > Marc, I've kept your authorship on this. There are substantial changes > from your version, but you had laid out the plan for all these changes, > so it seemed fair. However, let me know if you're not comfortable with > taking responsibility for any potential bugs I've introduced. Thanks a lot for picking this up at short notice. Definitely happy with you preserving the authorship, though I wouldn't object if you did it the other way around. > I had originally considered sending this out as a series with more > intermediate steps, but it turns out most of the TODO's that Marc had > listed are related, so this could've been at maximum two or three > patches. That didn't seem worth splitting up for. I think most of this > work is still all related to the MSI domain conversion, so it makes > sense to keep it in one patch. > > drivers/pci/controller/pci-tegra.c | 316 +++++++++++++++-------------- > 1 file changed, 165 insertions(+), 151 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index c1d34353c29b..f7f92718ce40 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c [...] > +static void tegra_msi_irq_mask(struct irq_data *d) > +{ > + struct tegra_msi *msi = irq_data_get_irq_chip_data(d); > + struct tegra_pcie *pcie = msi_to_pcie(msi); > + unsigned int index = d->hwirq / 32; > + u32 value; > + > + value = afi_readl(pcie, AFI_MSI_EN_VEC(index)); > + value &= ~BIT(d->hwirq % 32); > + afi_writel(pcie, value, AFI_MSI_EN_VEC(index)); You need some extra locking here, as this register looks shared with another 31 MSIs. Same for the unmask. [...] > +static int tegra_allocate_domains(struct tegra_msi *msi) > { > - irq_set_chip_and_handler(irq, &tegra_msi_irq_chip, handle_simple_irq); > - irq_set_chip_data(irq, domain->host_data); > + struct tegra_pcie *pcie = container_of(msi, struct tegra_pcie, msi); > + struct fwnode_handle *fwnode = of_node_to_fwnode(pcie->dev->of_node); > + struct irq_domain *parent; > > - tegra_cpuidle_pcie_irqs_in_use(); > + msi->top.name = "Tegra PCIe MSI"; > + msi->top.irq_ack = tegra_msi_top_irq_ack; > + msi->top.irq_mask = tegra_msi_top_irq_mask; > + msi->top.irq_unmask = tegra_msi_top_irq_unmask; > + > + msi->info.flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_PCI_MSIX; > + msi->info.chip = &msi->top; > + > + msi->bottom.name = "Tegra MSI"; > + msi->bottom.irq_ack = tegra_msi_irq_ack; > + msi->bottom.irq_mask = tegra_msi_irq_mask; > + msi->bottom.irq_unmask = tegra_msi_irq_unmask; > + msi->bottom.irq_set_affinity = tegra_msi_set_affinity; > + msi->bottom.irq_compose_msi_msg = tegra_compose_msi_msg; It's not obvious to me why these irq_chip and msi_domain_info are dynamically allocated, given that they only contain static data, which should be common across PCIe controllers. Am I missing something here? The reason I'm being picky about it is that I'm trying to get to a point where we can make most struct irq_chip to be const and thus marked read-only (PM is getting in the way at the moment). The rest looks good to me, and only the above locking issue is a real showstopper. Thanks, M. -- Without deviation from the norm, progress is not possible.