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 4692DC49EA1 for ; Fri, 26 Jul 2024 11:56:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References: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:List-Owner; bh=5y7YOEDfU4JS5fx5scmrf6hqB3/+VWCj5aY8K/rMjG8=; b=myemj9/SAjTisMm/a2VSOUzznN fjb5sJ8NE219uAXI+JVfYFfkju2xZp26Cq9KTwlpXNKBq2oWRt6uMkr0DSB08Au6sEHqOyHTAvNwx wEm+XGxIJ2zMGS/HOG3MM9mWcmkB+9r9kGZt9MnVDAMIdqj63qOxipBuBwqi7xJGou3VgzqUbvIXE snBApYuSW996ERq5NWjrFOcG6FseGkk1dz0z8lhE21bCuWzskBLxQqjHTQv059jkSz8TE3uh/AGNT hBPfs7SETdH07Wq7Fff6MAe9yKgbRzemeXuVuzYGR7KdBlwZA2mRFskcKMPkK0e2Xeya3aS3HkEOf tmqzeWRg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sXJYx-00000003mJC-2qeS; Fri, 26 Jul 2024 11:56:43 +0000 Received: from mail-pl1-x62a.google.com ([2607:f8b0:4864:20::62a]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sXJYZ-00000003mDt-0AEk for linux-arm-kernel@lists.infradead.org; Fri, 26 Jul 2024 11:56:20 +0000 Received: by mail-pl1-x62a.google.com with SMTP id d9443c01a7336-1fd69e44596so4190645ad.1 for ; Fri, 26 Jul 2024 04:56:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1721994978; x=1722599778; darn=lists.infradead.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=5y7YOEDfU4JS5fx5scmrf6hqB3/+VWCj5aY8K/rMjG8=; b=xyjBaaI4VBheRolcG6XKZ6MCJRjXHPI4m9+zPI3cv7QFZmzJeyFfgr/21JimZxssYV oOxQ4xVkZIY2q7KapSSU5/0LeZCgvMODb5Nwy3uTAwvoJTBFL1BBTqWK/hjeXf896A7N hlSnX2ie7oHXkx28NBR5qegW1xOUfKU4FRQWfTDKYRDPNaIjQ5/HK3aW08oeCPLEUx1m iCmbLum6NvnjoCrtdM0tg5CYJRSgfw5cfBOxtKaf+YaehLfBZOvs0LZBO8BhT3n6Tn9d tPvLiHPti+ZQDMUF92z0Bx/RcXMKwdPuyb3r/w5XgKj9e49o0HwmXKH/coFXgbBIVLg8 kTgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721994978; x=1722599778; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=5y7YOEDfU4JS5fx5scmrf6hqB3/+VWCj5aY8K/rMjG8=; b=JlmJ22ODJstI69LdEDnp0X+PoJdwFPsA71hQbxCpy39KP6ERZvY29Y3qE4FtqwWL7B JLY+6GNwaNeWHTi3KRwD0+Dwuz1DBqkesjoK59lezMpBZ8cjU+jz0eTzqjo2aWso0tS1 hbBh5/Ov03CTSQDHw3sSHE/CDgH4d0u7HBTR/d3QQBMAnWHKVQ3BvVoxxSnYdagAqQ5T 2/OR3JfAfGYA2zXB2RmcZ07J8cY9gvJy4uwpYShiHseFM8B/+sR0h+7M2KrcL5FPNU3K 26RwLAEq0d8vJgVGTTZo0vjCXWo0lvkUnIY9rrk5zYDfaA+nQH8TuFGAxlZ224d+0byS TfPQ== X-Forwarded-Encrypted: i=1; AJvYcCXtesCnldq8K0XCKbWeIF/K6frOWfx5W4ja7at71YHsNKTOHXnh4zxLcileaiaFm/Pn0lB5lIIArMkDuYjZNzOk0hDUxOmYXf+qlGGEq0cBUte/YVE= X-Gm-Message-State: AOJu0YwvC58nudjgaf0vdOTr9RJyEkrJoQ6Nj3CNog6PtfBFt44RDXoX w6JfERFTY663uKDdSGGlRaDas/Z+kvCsCej9xD6slDXmTSy/hcP11itkxm4XpA== X-Google-Smtp-Source: AGHT+IEcrjndBXB2ZtsnkLons2ztafN8jRqGa2IoiheYYReC037fHhFd+JnG6qPN10f0KGDBK21ZYw== X-Received: by 2002:a17:902:e550:b0:1fc:5b81:729f with SMTP id d9443c01a7336-1fdd6e89195mr109783765ad.32.1721994977780; Fri, 26 Jul 2024 04:56:17 -0700 (PDT) Received: from thinkpad ([2409:40f4:201d:928a:9e8:14a5:7572:42b6]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fed7fb67e4sm30343095ad.265.2024.07.26.04.56.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Jul 2024 04:56:17 -0700 (PDT) Date: Fri, 26 Jul 2024 17:26:09 +0530 From: Manivannan Sadhasivam To: Siddharth Vadapalli Cc: bhelgaas@google.com, lpieralisi@kernel.org, kw@linux.com, robh@kernel.org, vigneshr@ti.com, kishon@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org, ahalaney@redhat.com, srk@ti.com Subject: Re: [PATCH] PCI: j721e: Set .map_irq and .swizzle_irq to NULL Message-ID: <20240726115609.GF2628@thinkpad> References: <20240724065048.285838-1-s-vadapalli@ti.com> <20240724161916.GG3349@thinkpad> <20240725042001.GC2317@thinkpad> <93e864fb-cf52-4cc0-84a0-d689dd829afb@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <93e864fb-cf52-4cc0-84a0-d689dd829afb@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240726_045619_283965_F02D753B X-CRM114-Status: GOOD ( 29.54 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jul 25, 2024 at 01:50:16PM +0530, Siddharth Vadapalli wrote: > On Thu, Jul 25, 2024 at 09:50:01AM +0530, Manivannan Sadhasivam wrote: > > On Wed, Jul 24, 2024 at 09:49:21PM +0530, Manivannan Sadhasivam wrote: > > > On Wed, Jul 24, 2024 at 12:20:48PM +0530, Siddharth Vadapalli wrote: > > > > Since the configuration of Legacy Interrupts (INTx) is not supported, set > > > > the .map_irq and .swizzle_irq callbacks to NULL. This fixes the error: > > > > of_irq_parse_pci: failed with rc=-22 > > > > due to the absence of Legacy Interrupts in the device-tree. > > > > > > > > > > Do you really need to set 'swizzle_irq' to NULL? pci_assign_irq() will bail out > > > if 'map_irq' is set to NULL. > > > > > > > Hold on. The errono of of_irq_parse_pci() is not -ENOENT. So the INTx interrupts > > are described in DT? Then why are they not supported? > > No, the INTx interrupts are not described in the DT. It is the pcieport > driver that is attempting to setup INTx via "of_irq_parse_and_map_pci()" > which is the .map_irq callback. The sequence of execution leading to the > error is as follows: > > pcie_port_probe_service() > pci_device_probe() > pci_assign_irq() > hbrg->map_irq > of_pciof_irq_parse_and_map_pci() > of_irq_parse_pci() > of_irq_parse_raw() > rc = -EINVAL > ... > [DEBUG] OF: of_irq_parse_raw: ipar=/bus@100000/interrupt-controller@1800000, size=3 > if (out_irq->args_count != intsize) > goto fail > return rc > > The call to of_irq_parse_raw() results in the Interrupt-Parent for the > PCIe node in the device-tree being found via of_irq_find_parent(). The > Interrupt-Parent for the PCIe node for MSI happens to be GIC_ITS: > msi-map = <0x0 &gic_its 0x0 0x10000>; > and the parent of GIC_ITS is: > gic500: interrupt-controller@1800000 > which has the following: > #interrupt-cells = <3>; > > The "size=3" portion of the DEBUG print above corresponds to the > #interrupt-cells property above. Now, "out_irq->args_count" is set to 1 > as __assumed__ by of_irq_parse_pci() and mentioned as a comment in that > function: > /* > * Ok, we don't, time to have fun. Let's start by building up an > * interrupt spec. we assume #interrupt-cells is 1, which is standard > * for PCI. If you do different, then don't use that routine. > */ > > In of_irq_parse_pci(), since the PCIe-Port driver doesn't have a > device-tree node, the following doesn't apply: > dn = pci_device_to_OF_node(pdev); > and we skip to the __assumption__ above and proceed as explained in the > execution sequence above. > > If the device-tree nodes for the INTx interrupts were present, the > "ipar" sequence to find the interrupt parent would be skipped and we > wouldn't end up with the -22 (-EINVAL) error code. > > I hope this clarifies the relation between the -22 error code and the > missing device-tree nodes for INTx. > Thanks for explaining the logic. Still I think the logic is flawed. Because the parent (host bridge) doesn't have 'interrupt-map', which means INTx is not supported. But parsing one level up to the GIC node and not returning -ENOENT doesn't make sense to me. Rob, what is your opinion on this behavior? - Mani -- மணிவண்ணன் சதாசிவம்