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 6020BC4167B for ; Wed, 6 Dec 2023 17:00:35 +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=wRZK//nGYC1dkeGCmy/aLMx41OZF4XcEeKUzbO03B1w=; b=eqzCpDvkNwCuDf EyraBoRBxufVg8qGA4V/phD2viaBKCEBOGQAT8iEqWPH+Z9XH7UivtQX8g5DO5JUpZd7XHaSGXtxW 3Lmtdi8hcBwbvqI7HfXzCZPBMSSsuuZTh4Zfj/pQ6ZgcXmaLFipcUqyF4upcLh/1BQCY7GALKo/Yd fgo3PkBfZ86t3xmO2VEFPwWQzv+mOErYWYnD2MM9XLY1DeOmCS5eO0Nm269MQkDyQcTMQH8kvBQM2 GqfpR8NbhDQEvfb5mSEhqf5hgISNKsl8jGyk86qllfbJZxquSoy6DIT/COyz+9WgN8VvHRkLXchrW 0eggJgkg0ssFVHXUZVlg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rAvFh-00Apnm-2q; Wed, 06 Dec 2023 17:00:01 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rAvFe-00Apms-1z for linux-arm-kernel@lists.infradead.org; Wed, 06 Dec 2023 17:00:00 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 639D4CE1FAD; Wed, 6 Dec 2023 16:59:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 62A39C433C8; Wed, 6 Dec 2023 16:59:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1701881995; bh=xdGLbLZlKz6hqDzxxH4MqX0sG3ItG6KAFGtCvH4gnsg=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=MJOLyo52HSa1ezkiZilGDaJk8L3Ol+4TVWOFf2jN/7JZLdwsFDlmL2DvKVIonkRdi 3fh5GT7VbTejQ/xnXDT4CHQXfs6JwDdajAVQGRpWv+BrPWR7T16yt2bI4sh8QmVU/U NEO/F5c0EVGAgGrAcr3sMeLNty1noQR5qil2W0V/kYk4ci4M97mv8ecRo/e3l1V5zV qQ3TqEMfeJaGNX2XEBnCPKj/J36uThgLrbIM6SOC3ilUXEAdNQ0LdOvybK/ykg4Dw2 rC8p+XzXdMWcPRwjqqjvFnFbWkbQLhfKpf7lnFWNAdDNZK7wI9Ib3eWTm5C1dlEzeU 1tWiAKmXvvduw== Date: Wed, 6 Dec 2023 10:59:53 -0600 From: Bjorn Helgaas To: Frank Li Cc: imx@lists.linux.dev, Richard Zhu , Lucas Stach , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Bjorn Helgaas , Krzysztof Kozlowski , Conor Dooley , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , "open list:PCI DRIVER FOR IMX6" , "moderated list:PCI DRIVER FOR IMX6" , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , open list Subject: Re: [PATCH 4/9] PCI: imx6: Using "linux,pci-domain" as slot ID Message-ID: <20231206165953.GA717921@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-20231206_085959_130981_98C22B5E X-CRM114-Status: GOOD ( 33.71 ) 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 Wed, Dec 06, 2023 at 11:50:16AM -0500, Frank Li wrote: > On Wed, Dec 06, 2023 at 10:36:56AM -0600, Bjorn Helgaas wrote: > > In subject, maybe you mean "Use 'linux,pci-domain' as slot ID"? > > "Using" is the wrong verb form here. > > > > On Wed, Dec 06, 2023 at 10:58:58AM -0500, Frank Li wrote: > > > Avoid use get slot id by compared with register physical address. If there > > > are more than 2 slots, compared logic will become complex. > > > > But this doesn't say anything about "linux,pci-domain", and I don't > > see anything about a register physical address in the patch. > > > > Maybe this commit log was meant for a different patch? I'm confused. > > > > > Signed-off-by: Frank Li > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > index 62d77fabd82a..239ef439ba70 100644 > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > @@ -33,6 +33,7 @@ > > > #include > > > #include > > > > > > +#include "../../pci.h" > > > #include "pcie-designware.h" > > > > > > #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9) > > > @@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev) > > > "Failed to get PCIEPHY reset control\n"); > > > } > > > > > > + /* Using linux,pci-domain as PCI slot id */ > > > + imx6_pcie->controller_id = of_get_pci_domain_nr(node); > > > + if (imx6_pcie->controller_id) > > > + imx6_pcie->controller_id = 0; > > > > I don't understand what this is doing. It looks the same as just: > > Good capture. It should be > if (imx6_pcie->controller_id < 0) > imx6_pcie->controller_id = 0; > > for only one PCI controller case. I just tested first one slot before send > patch, so not met problem. > > Previously, we use below logic > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > imx6_pcie->controller_id = 1; > > It is not good to depend on register's base address. If there are 3 > controllers, check logic will becomoe ugly. Makes sense. If the previous code depended on the base address, this patch would make more sense if it contained both the addition of the of_get_pci_domain_nr() call and the removal of the base address code. > > Maybe this is a typo? As written, it doesn't look like there's any > > point in calling of_get_pci_domain_nr(). > > > > > switch (imx6_pcie->drvdata->variant) { > > > case IMX7D: > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > -- > > > 2.34.1 > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel