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 2A816C3DA4A for ; Mon, 29 Jul 2024 08:00:56 +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=IZwZqPvJh4AbxW6ByMaS8C6OygwvpLTFq0TUS9PMs9s=; b=efVH+SVBOYfMlWy90gofx0SitQ HaJPx25OS9vjWNYJBtpJkgWjDgDs7qCPJWq6GcOzkqMj6S4rEarHoXMPGgZk9elSu90H8pYjU6k0P FFI3ashXk7L+FyI7gTzqtbAl7QaLZ5JLdLoI6fR1doK9ISpPTOc6HFc77CEnrfO4UZok56KL5f7W1 FovnP2+76Ve9x7nGWvpNtqGd4mdNe9BgnjiS1qLe0Hopd+vdIkriCe7uRCjpHRIK2ZLY0y6ZGW0j4 ujDKjD+TL5IYCHl1G8KbSS0bYY98jVDl+wfnugAhW0v92RmoxQiDVD24Jp5GeSFIrlTmlBjpknvWi 2WTtbxgA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYLJD-0000000AS73-2JuL; Mon, 29 Jul 2024 08:00:43 +0000 Received: from mail-pl1-x636.google.com ([2607:f8b0:4864:20::636]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYLIm-0000000AS0v-2U5k for linux-arm-kernel@lists.infradead.org; Mon, 29 Jul 2024 08:00:18 +0000 Received: by mail-pl1-x636.google.com with SMTP id d9443c01a7336-1fd66cddd4dso24350705ad.2 for ; Mon, 29 Jul 2024 01:00:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1722240015; x=1722844815; 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=IZwZqPvJh4AbxW6ByMaS8C6OygwvpLTFq0TUS9PMs9s=; b=xNz1Za6K2eI3cN33LD9NEmbYC5Tusu/OIhNOfktVFooVmGrukAPvDgXI57BchUf2Qh zuIKF2RyiKknOmYTjHluDJjUS8K1CR+FICqRfpzxlNQVCBzy1lvvS8tMqb2A/0OCX/kt izKastyh1c4EBiNlf0OJHEasWVvzViOtqtWRiJUWBNbIjtsxQHyK313vK0jzzFWwkcS1 WBTiqu2NKcFpexwyb97QiwAdF+NCEwolaTPMnEkDpQL6V5SJCabIypsPhEv3usI/HFmb 0742QN6cf6srLGOhCNI16j3waMRhXCjKVDh7r8BYWbaAaF6tXPL/kOIeitTUhG0jdcC4 1mOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722240015; x=1722844815; 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=IZwZqPvJh4AbxW6ByMaS8C6OygwvpLTFq0TUS9PMs9s=; b=ApHJPPizCxjRnBDhj+/figV0PGkqIkZOwSaPsTdsR8c924Eh0FOJCHOzTzoINhANVR p2rqh1Ixzu1RWOSiMit+q9KMqhRgCT3YIr912xCFT+gphyeJ2weOLi4YuNwGc96K8kCx /y7f3FQyknA2qshV4mw/EJlbm0O2+T6afGcYvPcBj6P1mFf8rbodzATscYq4VQOjg1HJ yKkMbSOjGwUnW5MlTilvZwkYlS7uB0S1AN4q7hEpRLsRP67a2LSawMjegTgAvtW8Wprh Vrg1iLVMDAfaPWhVSpgkrA8F4sBvIbmq+P9eJZYrWXsZpH7jO+ZZBz1/l3UGFdesEFTP d3Xw== X-Forwarded-Encrypted: i=1; AJvYcCWR1lfG4++vBBlJti95Z36LKJyB350B2bBdLnhZJtmFPMXqjs9hgRTDGEYaCp0J7BijLMBAL1XnyVNdgn8hUmVnEEQY+xunBsxUgOzKZWmX7sVfGtQ= X-Gm-Message-State: AOJu0YxkKlhzlqC/6KsIql1iXczjsdizSKCNEqAGsF/AFozjpomNylQA au2ADdbUN+FcqVpj1k3FZXt7JIdjr6uVqP0Ered3sv3/8/a7o06Zce87Oht7WQ== X-Google-Smtp-Source: AGHT+IGw6ntMSIZXrn+M7V9oFM6znOqalhya3M3rym9hvcc6pLj47+SXg29jXV6DF+nKd83PCONWsA== X-Received: by 2002:a17:903:22c2:b0:1fb:6663:b647 with SMTP id d9443c01a7336-1ff047e4486mr112315185ad.3.1722240014693; Mon, 29 Jul 2024 01:00:14 -0700 (PDT) Received: from thinkpad ([117.213.101.9]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fed7fbd8d4sm76322585ad.271.2024.07.29.01.00.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Jul 2024 01:00:14 -0700 (PDT) Date: Mon, 29 Jul 2024 13:30:06 +0530 From: Manivannan Sadhasivam To: Siddharth Vadapalli , robh@kernel.org Cc: bhelgaas@google.com, lpieralisi@kernel.org, kw@linux.com, 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 v2] PCI: j721e: Disable INTx mapping and swizzling Message-ID: <20240729080006.GA8698@thinkpad> References: <20240726135903.1255825-1-s-vadapalli@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240726135903.1255825-1-s-vadapalli@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240729_010016_678814_C54B42AF X-CRM114-Status: GOOD ( 31.91 ) 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 Fri, Jul 26, 2024 at 07:29:03PM +0530, Siddharth Vadapalli wrote: > Since the configuration of INTx (Legacy Interrupt) 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 > when the pcieport driver attempts to setup INTx for the Host Bridge via > "pci_assign_irq()". The device-tree node of the Host Bridge doesn't > contain Legacy Interrupts. As a result, Legacy Interrupts are searched for > in the MSI Interrupt Parent of the Host Bridge with the help of > "of_irq_parse_raw()". Since the MSI Interrupt Parent of the Host Bridge > uses 3 interrupt cells while Legacy Interrupts only use 1, the search > for Legacy Interrupts is terminated prematurely by the "of_irq_parse_raw()" > function with the -EINVAL error code (rc=-22). > > Fixes: f3e25911a430 ("PCI: j721e: Add TI J721E PCIe driver") > Cc: stable@vger.kernel.org > Reported-by: Andrew Halaney > Tested-by: Andrew Halaney > Signed-off-by: Siddharth Vadapalli TBH, I'm not comfortable with this change. Because, the INTx compatibility _should_ come from the platform description (in this case DT) and not the driver. The default map_irq() function is supposed to check the existence of INTx and correctly report it. In this case the issue is, the platform is not supporting INTx, but of_irq_parse_pci() reports a dubious error: 'of_irq_parse_pci: failed with rc=-22' instead of the actual error: `of_irq_parse_pci: no interrupt-map found, INTx interrupts not available' So I would say that the fix is in of_irq_parse_pci() implementation. of_irq_parse_pci() is supposed to find whether the PCIe controller is supporting INTx or not by parsing the 'interrupt-{map/extended}' properties till host bridge/controller node. But this API along with of_irq_parse_raw(), just walks up the tree till the top level interrupt controller and checks for INTx, which just feels wrong (that's why you are getting -EINVAL because of #interrupt-cells mismatch). I looked into the implementation over the weekend, but I'm not quite sure how to fix it though. Rob, perhaps you have any idea? - Mani > --- > > Hello, > > This patch is based on commit > 1722389b0d86 Merge tag 'net-6.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net > of Mainline Linux. > > v1: > https://lore.kernel.org/r/20240724065048.285838-1-s-vadapalli@ti.com > Changes since v1: > - Added "Cc: stable@vger.kernel.org" in the commit message. > - Based on Bjorn's feedback at: > https://lore.kernel.org/r/20240724162304.GA802428@bhelgaas/ > the $subject and commit message have been updated. Additionally, the > comment in the driver has also been updated to specify "INTx" instead of > "Legacy Interrupts". > - Collected Tested-by tag from Andrew Halaney : > https://lore.kernel.org/r/vj6vtjphpmqv6hcblaalr2m4bwjujjwvom6ca4bjdzcmgazyaa@436unrb2lav7/ > > Patch has been tested on J721e-EVM and J784S4-EVM. > > Regards, > Siddharth. > > drivers/pci/controller/cadence/pci-j721e.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 85718246016b..eaa6cfeb03c7 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -417,6 +417,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) > if (!bridge) > return -ENOMEM; > > + /* INTx is not supported */ > + bridge->map_irq = NULL; > + bridge->swizzle_irq = NULL; > + > if (!data->byte_access_allowed) > bridge->ops = &cdns_ti_pcie_host_ops; > rc = pci_host_bridge_priv(bridge); > -- > 2.40.1 > -- மணிவண்ணன் சதாசிவம்