Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 6 Dec 2023 12:07:00 -0500
From:   Frank Li <Frank.li@nxp.com>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     imx@lists.linux.dev, Richard Zhu <hongxing.zhu@nxp.com>,
        Lucas Stach <l.stach@pengutronix.de>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        "open list:PCI DRIVER FOR IMX6" <linux-pci@vger.kernel.org>,
        "moderated list:PCI DRIVER FOR IMX6" 
        <linux-arm-kernel@lists.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/9] PCI: imx6: Using "linux,pci-domain" as slot ID
Message-ID: <ZXCqNI3VZ8Sn23WM@lizhi-Precision-Tower-5810>
References: <ZXCmSOwTWR6AVpGB@lizhi-Precision-Tower-5810>
 <20231206165953.GA717921@bhelgaas>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231206165953.GA717921@bhelgaas>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?iIz5Y76F2jzW29g9dAfkJJINQdSo5eoacjD4xoEiq0+gOe+xwlUJ0sbD2BHG?=
 =?us-ascii?Q?OMzS9avHtS+KEpydpJcfp/7HhzyQFAg0Qf/u4PjmRAr475iUGkcAt7hdq7Z+?=
 =?us-ascii?Q?Bpe861zCkuU4zb2BaZ/NjblRM7ffcsFORovf4TOKdX/Dk97DKWgpgYj9XCAd?=
 =?us-ascii?Q?lcPAjmXcd+cBZvE9/LX2TJcpPGtivhzTsyBGF4djrsJVaAYsFyJNoTujAbM3?=
 =?us-ascii?Q?5PNhoZqyFcB3LmRteps4OO6uCAi8kfsrQaL5ndoBugmLMKK/IpmpwYUbfB4/?=
 =?us-ascii?Q?pHWOJiiJjoqvu6DkZHKlkdgMBCmlVVIIK5187g+G2TJdT06zJ/sCliUpeMOr?=
 =?us-ascii?Q?z7IThryE+VYrFi4mM8IeUQ85c2dHuisr8yopDJBQJqTGjY2lxluJ9TrXODH0?=
 =?us-ascii?Q?U2xML5QoPhxcQKnz/zhddlwSjKvh2A20GYeW47aJ660uXAcA6D6UiySKK5B2?=
 =?us-ascii?Q?jInY6bLouWn32WMYcKoC4HaqNyNa29Dn6W0SafLk8Ce5fG6tNFdou9+ToLGc?=
 =?us-ascii?Q?fLRKdfKzsJBM5T3BeAgBmtbFLqcWl2PyfrHhXobb56ug6Wcf3TsqRocpq3qc?=
 =?us-ascii?Q?5MHQ7UwPEr9EDptqu72HunnWYabzlh2fqx1Oq47Higs6g0XCc1ciiIAGJQ1Y?=
 =?us-ascii?Q?sh/PxCfhdKD+q/wkBuiL4yE/5+enZk8E8Xqw7EEf0xkXix4MGNIM44mAXVFR?=
 =?us-ascii?Q?+THwEv3mbBmWoWqCJJikwBns1U5LBidB5zuaErSg3JeB5ejEv7CfL1jbsiXj?=
 =?us-ascii?Q?QCgdouW1n11o2J8ixPXEzrOAaeB2hKqSiKfJCYKcLEo13bLuAaDV2qOESW17?=
 =?us-ascii?Q?1ybUt4tTzmE68dUMlnKvMunna9nAmy9KPs8m435n6/GrM2ZrCbx7UCrhJJFE?=
 =?us-ascii?Q?pTNDWzj3TkrP5bB+zFt4431KsERk8C8si7NYz+F/FVuHkNZCYIIqFrJROpnW?=
 =?us-ascii?Q?cSianWAYDjF/DVVwf5rMNP6EFWdhJCF9z6T6EHksG4DNU5AoTdXw+bowoOKk?=
 =?us-ascii?Q?Bk/Yz3XP3S4RvABJZ1CMACUF3HBxm2xPl6HAW3cAM2Jbc3QOBj9WwyDdEwaE?=
 =?us-ascii?Q?qUdakuGeNgln7uhdF6yAjFsBomW2EfCAWJX6EEYfNLd+BGzsUMTCTBjjMwSx?=
 =?us-ascii?Q?v3ieWCHdivunjfU1pTY038WOFngDac2t4vZlM6mWL7gfr1MXNFQYcAeY7A85?=
 =?us-ascii?Q?jz/E3Rkwu/Y5gkradbpmsnSYJ2jvtDUY1swTKFnTPbji9rfZs6HsEvU03ufA?=
 =?us-ascii?Q?0DS6Vn9+kwHZeYvtmv1pdiR+CsUZ1SgQRov/wDN321xKvei8qwr4ZKfaglzP?=
 =?us-ascii?Q?nBh8uuDQbxqsHQXsh2Q4lT2cbWAryx8UZvPZQ3+qFG32zfKzPTFl0EepvZ4p?=
 =?us-ascii?Q?p6ecrxH9F4S3aMPerq79o5cPC7QFcwLpdWPK0cJ+ovai1u4JVZ+PsN3bFHLL?=
 =?us-ascii?Q?QFPXgWnSm5Noqw9ztQAe0gRVfXHlvVBH3ZI2ZX+cEH9sjaguj4Bcy6gdxjLL?=
 =?us-ascii?Q?gMhoOvZMlLd8M+PrgT9FaXi5eMH+zZZDxYYFYaitdt1/ctWAkE3wuu8iwcKo?=
 =?us-ascii?Q?+lEvjisXdjhiOacx74KaUkkUzFqNArfNDp+ygN7K?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 470673d6-267c-41af-f3e0-08dbf67dc82b
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2023 17:07:16.2747
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mdM9Esu3sM9WKT/5C7cQINGq6W6yWR6hHJfmS2wrWA0TXlnp7n1ujW3PvA//TGGy0oYcZqXVFkiw1o1NpUmAaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB9253
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 06 Dec 2023 09:07:32 -0800 (PST)

On Wed, Dec 06, 2023 at 10:59:53AM -0600, Bjorn Helgaas wrote:
> 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 <Frank.Li@nxp.com>
> > > > ---
> > > >  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 <linux/pm_domain.h>
> > > >  #include <linux/pm_runtime.h>
> > > >  
> > > > +#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.

Remove base address code will block existed functions. My plan is
1. this patch upstreamed
2. related dts add linux,pci-domain.
3. removed base address compare code.

Frank

> 
> > > 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
> > > >