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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 4FBDCEB64DC for ; Thu, 20 Jul 2023 15:24:31 +0000 (UTC) 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:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=Q5tG86ikn1lggoIshHgqT6CnP7hxv/EQpEzSYMigZD4=; b=jUiD3blFcQEOnA y0Yl3lUkLNKhbEFEGnxAwlNkFD5W7qV0JLb/D4J4QcLodnqg8CJN6I2nRo3zauMas2Sq7wcOiqGPm 0tZm/5Q3oBdxqjbc/jbWfgonk7Fi8udd/XLGjFhnG41zqWJMzHd+/yonfqOeJizK05oxkV9K6Lqoo 62W95Oagf4xXPI+39Df2FXw95CAsufYVUGrzu+kB5Q+6Pow9ixGuHcNmxtyrnhSqApSI2UlWVAX1n SdR9jAU7ZcPgPjLHQmd3uoo0BTcO4kXu7n/4F1u/xeqO1Gaj789bIAl9RXl6gTJVQzvJ9PIBE4X/8 38S+txo/ehiuVhFnYrCA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qMVVh-00BXMr-20; Thu, 20 Jul 2023 15:24:09 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qMVVd-00BXLY-2c for linux-arm-kernel@lists.infradead.org; Thu, 20 Jul 2023 15:24:07 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9A1ED61B4E; Thu, 20 Jul 2023 15:24:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D009DC433C8; Thu, 20 Jul 2023 15:24:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689866643; bh=A9ffLkHP3EeJtWxJ/R0wqfZkmpfeCi3q5f4HAbNpLUs=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=E5ix1vEBK3ECDKaX/NVb7PYydIW8QhdbkjT2ocNoka284gQjrgp9wFUORUSOpkpio +O6I2kp27YgvAXhnFY/8V9WroAuxs7K6DkFG3ysWeYrypld4juqa2nzOnOd2KTCMWo 1c78ge0WkNgCXuXO/jUXrqs+54T0AgB80q9FfxObW7Uj8MwG/KTXCQ/xyggnxTpJ2y NYZ2RuwecYUMz3uRBY1S4c36lj9/Gt+EdGnVfsQvJPJ50PyReqbxqyxBuChtbqHwUP 5hIlVX1ctzu0yC5YvE1yOvMd3LHoZ2XrWWlV9jxc1jsbfEvUGHQ5vlNw/n2MVepr68 57COOMXu1/tfQ== Date: Thu, 20 Jul 2023 10:24:01 -0500 From: Bjorn Helgaas To: "Havalige, Thippeswamy" Cc: "krzysztof.kozlowski@linaro.org" , "devicetree@vger.kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "robh+dt@kernel.org" , "bhelgaas@google.com" , "lorenzo.pieralisi@arm.com" , "linux-arm-kernel@lists.infradead.org" , "Gogada, Bharat Kumar" , "Simek, Michal" , Thomas Gleixner Subject: Re: [PATCH V5 3/3] PCI: xilinx-xdma: Add Xilinx XDMA Root Port driver Message-ID: <20230720152401.GA523764@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230720_082406_031244_D84D4F27 X-CRM114-Status: GOOD ( 27.17 ) 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 [+cc Thomas in case he wants to comment on chained interrupts] On Thu, Jul 20, 2023 at 06:37:03AM +0000, Havalige, Thippeswamy wrote: > > From: Bjorn Helgaas > > ... > > On Wed, Jun 28, 2023 at 02:58:12PM +0530, Thippeswamy Havalige wrote: > > > Add support for Xilinx XDMA Soft IP core as Root Port. > > > ... > > > + * struct pl_dma_pcie - PCIe port information > > > + * @intx_domain: Legacy IRQ domain pointer > > > + * @pldma_domain: PL DMA IRQ domain pointer > > > + * @irq_misc: Legacy and error interrupt number > > > + * @intx_irq: legacy interrupt number > > "Legacy and error interrupt number" and "legacy interrupt number" > > sound like they overlap -- "legacy interrupt number" is part of both. > > Is that an error? > > - Agreed, I'll modify this comment to legacy interrupt number. (This > irq line is for both legacy interrupts and error interrupt bits) Does "legacy" mean "INTx" in this context? If so, I'd use "INTx" because it's much more specific. "Legacy" really doesn't contain any information other than "this is something retained for some kind of backward compatibility." If you have more detail about the "error interrupt," that would be useful as well. Does this refer to an AER interrupt, a "System Error", something else? I'm looking at the diagram in PCIe r6.0, Figure 6-3, wondering if this is related to anything there. I suppose likely it's some Xilinx-specific thing? > > > + /* Plug the INTx chained handler */ > > > + irq_set_chained_handler_and_data(port->intx_irq, > > > + xilinx_pl_dma_pcie_intx_flow, port); > > > + > > > + /* Plug the main event chained handler */ > > > + irq_set_chained_handler_and_data(port->irq, > > > + xilinx_pl_dma_pcie_event_flow, > > port); > > > > What's the reason for using chained IRQs? Can this be done > > without them? I don't claim to understand all the issues here, > > but it seems better to avoid chained IRQ handlers when possible: > > https://lore.kernel.org/all/877csohcll.ffs@tglx/ > - As per the comments in this > https://lkml.kernel.org/lkml/alpine.DEB.2.20.1705232307330.2409@nanos/T/ > "It is fine to have chained interrupts when bootloader, device tree > and kernel under control. Only if BIOS/UEFI comes into play the user > is helpless against interrupt storm which will cause system to > hangs." > > We are using ARM embedded platform with Bootloader, Devicetree flow. I read Thomas' comments as "in general it's better to use regular interrupts, but we can live with chained interrupts if we have control of bootloader, device tree, and kernel." I guess my questions are more like: - Could this be done with either chained interrupts or regular interrupts? - If so, what is the advantage to using chained interrupts? Across the entire kernel, irq_set_chained_handler_and_data() is relatively unusual, which makes me think it may be better to use the more common path if it's possible. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel