| Message ID | 20250226024256.1678103-3-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | Use domain number replace the hardcodes | expand |
On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > Use the domain number replace the hardcodes to uniquely identify > different controller on i.MX8MQ platforms. No function changes. > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 90ace941090f..ab9ebb783593 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -41,7 +41,6 @@ > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 Nice, thanks for getting rid of this! > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > #define IMX95_PCIE_REF_USE_PAD BIT(17) > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > struct dw_pcie *pci; > struct imx_pcie *imx_pcie; > struct device_node *np; > - struct resource *dbi_base; > struct device_node *node = dev->of_node; > int i, ret, req_cnt; > u16 val; > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > return PTR_ERR(imx_pcie->phy_base); > } > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base); > - if (IS_ERR(pci->dbi_base)) > - return PTR_ERR(pci->dbi_base); > - > /* Fetch GPIOs */ > imx_pcie->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(imx_pcie->reset_gpiod)) > @@ -1565,8 +1559,12 @@ static int imx_pcie_probe(struct platform_device *pdev) > switch (imx_pcie->drvdata->variant) { > case IMX8MQ: > case IMX8MQ_EP: > - if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > - imx_pcie->controller_id = 1; > + ret = of_get_pci_domain_nr(node); > + if (ret < 0 || ret > 1) > + return dev_err_probe(dev, -ENODEV, > + "failed to get valid pcie domain\n"); > + else > + imx_pcie->controller_id = ret; > break; > default: > break; > -- > 2.37.1 >
On Wed, Feb 26, 2025 at 04:08:26PM -0600, Bjorn Helgaas wrote: > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > Use the domain number replace the hardcodes to uniquely identify > > different controller on i.MX8MQ platforms. No function changes. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index 90ace941090f..ab9ebb783593 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -41,7 +41,6 @@ > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > Nice, thanks for getting rid of this! > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > > struct dw_pcie *pci; > > struct imx_pcie *imx_pcie; > > struct device_node *np; > > - struct resource *dbi_base; > > struct device_node *node = dev->of_node; > > int i, ret, req_cnt; > > u16 val; > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > > return PTR_ERR(imx_pcie->phy_base); > > } > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base); > > - if (IS_ERR(pci->dbi_base)) > > - return PTR_ERR(pci->dbi_base); > > - > > /* Fetch GPIOs */ > > imx_pcie->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > if (IS_ERR(imx_pcie->reset_gpiod)) > > @@ -1565,8 +1559,12 @@ static int imx_pcie_probe(struct platform_device *pdev) > > switch (imx_pcie->drvdata->variant) { > > case IMX8MQ: > > case IMX8MQ_EP: > > - if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > - imx_pcie->controller_id = 1; > > + ret = of_get_pci_domain_nr(node); > > + if (ret < 0 || ret > 1) > > + return dev_err_probe(dev, -ENODEV, > > + "failed to get valid pcie domain\n"); > > + else > > + imx_pcie->controller_id = ret; > > break; > > default: > > break; > > -- > > 2.37.1 > >
Hello, > Use the domain number replace the hardcodes to uniquely identify > different controller on i.MX8MQ platforms. No function changes. Applied to controller/imx6, thank you! Krzysztof
On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > Use the domain number replace the hardcodes to uniquely identify > different controller on i.MX8MQ platforms. No function changes. > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 90ace941090f..ab9ebb783593 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -41,7 +41,6 @@ > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > #define IMX95_PCIE_REF_USE_PAD BIT(17) > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > struct dw_pcie *pci; > struct imx_pcie *imx_pcie; > struct device_node *np; > - struct resource *dbi_base; > struct device_node *node = dev->of_node; > int i, ret, req_cnt; > u16 val; > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct platform_device *pdev) > return PTR_ERR(imx_pcie->phy_base); > } > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base); > - if (IS_ERR(pci->dbi_base)) > - return PTR_ERR(pci->dbi_base); This makes me wonder. IIUC this means that previously we set controller_id to 1 if the first item in devicetree "reg" was 0x33c00000, and now we will set controller_id to 1 if the devicetree "linux,pci-domain" property is 1. This is good, but I think this new dependency on the correct "linux,pci-domain" in devicetree should be mentioned in the commit log. My bigger worry is that we no longer set pci->dbi_base at all. I see that the only use of pci->dbi_base in pci-imx6.c was to determine the controller_id, but this is a DWC-based driver, and the DWC core certainly uses pci->dbi_base. Are we sure that none of those DWC core paths are important to pci-imx6.c? > /* Fetch GPIOs */ > imx_pcie->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(imx_pcie->reset_gpiod)) > @@ -1565,8 +1559,12 @@ static int imx_pcie_probe(struct platform_device *pdev) > switch (imx_pcie->drvdata->variant) { > case IMX8MQ: > case IMX8MQ_EP: > - if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > - imx_pcie->controller_id = 1; > + ret = of_get_pci_domain_nr(node); > + if (ret < 0 || ret > 1) > + return dev_err_probe(dev, -ENODEV, > + "failed to get valid pcie domain\n"); > + else > + imx_pcie->controller_id = ret; > break; > default: > break; > -- > 2.37.1 >
> -----Original Message----- > From: Bjorn Helgaas <helgaas@kernel.org> > Sent: 2025年3月10日 23:11 > To: Hongxing Zhu <hongxing.zhu@nxp.com> > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > hardcodes > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > Use the domain number replace the hardcodes to uniquely identify > > different controller on i.MX8MQ platforms. No function changes. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > b/drivers/pci/controller/dwc/pci-imx6.c > > index 90ace941090f..ab9ebb783593 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -41,7 +41,6 @@ > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device > *pdev) > > struct dw_pcie *pci; > > struct imx_pcie *imx_pcie; > > struct device_node *np; > > - struct resource *dbi_base; > > struct device_node *node = dev->of_node; > > int i, ret, req_cnt; > > u16 val; > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > platform_device *pdev) > > return PTR_ERR(imx_pcie->phy_base); > > } > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, > &dbi_base); > > - if (IS_ERR(pci->dbi_base)) > > - return PTR_ERR(pci->dbi_base); > > This makes me wonder. > > IIUC this means that previously we set controller_id to 1 if the first item in > devicetree "reg" was 0x33c00000, and now we will set controller_id to 1 if > the devicetree "linux,pci-domain" property is 1. > This is good, but I think this new dependency on the correct > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > My bigger worry is that we no longer set pci->dbi_base at all. I see that the > only use of pci->dbi_base in pci-imx6.c was to determine the controller_id, > but this is a DWC-based driver, and the DWC core certainly uses > pci->dbi_base. Are we sure that none of those DWC core paths are > important to pci-imx6.c? Hi Bjorn: Thanks for your concerns. Don't worry about the assignment of pci->dbi_base. If pci-imx6.c driver doesn't set it. DWC core driver would set it when dw_pcie_get_resources() is invoked. dw_pcie_host_init()/dw_pcie_ep_init() ->dw_pcie_get_resources() ... if (!pci->dbi_base) { res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi"); pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res); if (IS_ERR(pci->dbi_base)) return PTR_ERR(pci->dbi_base); pci->dbi_phys_addr = res->start; } ... Best Regards Richard Zhu > > > /* Fetch GPIOs */ > > imx_pcie->reset_gpiod = devm_gpiod_get_optional(dev, "reset", > GPIOD_OUT_HIGH); > > if (IS_ERR(imx_pcie->reset_gpiod)) > > @@ -1565,8 +1559,12 @@ static int imx_pcie_probe(struct > platform_device *pdev) > > switch (imx_pcie->drvdata->variant) { > > case IMX8MQ: > > case IMX8MQ_EP: > > - if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > - imx_pcie->controller_id = 1; > > + ret = of_get_pci_domain_nr(node); > > + if (ret < 0 || ret > 1) > > + return dev_err_probe(dev, -ENODEV, > > + "failed to get valid pcie domain\n"); > > + else > > + imx_pcie->controller_id = ret; > > break; > > default: > > break; > > -- > > 2.37.1 > >
On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > -----Original Message----- > > From: Bjorn Helgaas <helgaas@kernel.org> > > Sent: 2025年3月10日 23:11 > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > hardcodes > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > Use the domain number replace the hardcodes to uniquely identify > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > index 90ace941090f..ab9ebb783593 100644 > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > @@ -41,7 +41,6 @@ > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device > > *pdev) > > > struct dw_pcie *pci; > > > struct imx_pcie *imx_pcie; > > > struct device_node *np; > > > - struct resource *dbi_base; > > > struct device_node *node = dev->of_node; > > > int i, ret, req_cnt; > > > u16 val; > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > platform_device *pdev) > > > return PTR_ERR(imx_pcie->phy_base); > > > } > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, > > &dbi_base); > > > - if (IS_ERR(pci->dbi_base)) > > > - return PTR_ERR(pci->dbi_base); > > > > This makes me wonder. > > > > IIUC this means that previously we set controller_id to 1 if the first item in > > devicetree "reg" was 0x33c00000, and now we will set controller_id to 1 if > > the devicetree "linux,pci-domain" property is 1. > > This is good, but I think this new dependency on the correct > > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > > > My bigger worry is that we no longer set pci->dbi_base at all. I see that the > > only use of pci->dbi_base in pci-imx6.c was to determine the controller_id, > > but this is a DWC-based driver, and the DWC core certainly uses > > pci->dbi_base. Are we sure that none of those DWC core paths are > > important to pci-imx6.c? > Hi Bjorn: > Thanks for your concerns. > Don't worry about the assignment of pci->dbi_base. > If pci-imx6.c driver doesn't set it. DWC core driver would set it when > dw_pcie_get_resources() is invoked. Great, thanks! Maybe we can amend the commit log to mention that and the new "linux,pci-domain" dependency.
> -----Original Message----- > From: Bjorn Helgaas <helgaas@kernel.org> > Sent: 2025年3月11日 23:55 > To: Hongxing Zhu <hongxing.zhu@nxp.com> > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > hardcodes > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > -----Original Message----- > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > Sent: 2025年3月10日 23:11 > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > bhelgaas@google.com; > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > hardcodes > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > Use the domain number replace the hardcodes to uniquely identify > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > --- > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > index 90ace941090f..ab9ebb783593 100644 > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > @@ -41,7 +41,6 @@ > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > 8) > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > platform_device > > > *pdev) > > > > struct dw_pcie *pci; > > > > struct imx_pcie *imx_pcie; > > > > struct device_node *np; > > > > - struct resource *dbi_base; > > > > struct device_node *node = dev->of_node; > > > > int i, ret, req_cnt; > > > > u16 val; > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > platform_device *pdev) > > > > return PTR_ERR(imx_pcie->phy_base); > > > > } > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > 0, > > > &dbi_base); > > > > - if (IS_ERR(pci->dbi_base)) > > > > - return PTR_ERR(pci->dbi_base); > > > > > > This makes me wonder. > > > > > > IIUC this means that previously we set controller_id to 1 if the > > > first item in devicetree "reg" was 0x33c00000, and now we will set > > > controller_id to 1 if the devicetree "linux,pci-domain" property is 1. > > > This is good, but I think this new dependency on the correct > > > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > > > > > My bigger worry is that we no longer set pci->dbi_base at all. I > > > see that the only use of pci->dbi_base in pci-imx6.c was to > > > determine the controller_id, but this is a DWC-based driver, and the > > > DWC core certainly uses > > > pci->dbi_base. Are we sure that none of those DWC core paths are > > > important to pci-imx6.c? > > Hi Bjorn: > > Thanks for your concerns. > > Don't worry about the assignment of pci->dbi_base. > > If pci-imx6.c driver doesn't set it. DWC core driver would set it when > > dw_pcie_get_resources() is invoked. > > Great, thanks! Maybe we can amend the commit log to mention that and > the new "linux,pci-domain" dependency. How about the following updates of the commit log? Use the domain number replace the hardcodes to uniquely identify different controller on i.MX8MQ platforms. No function changes. Please make sure the " linux,pci-domain" is set for i.MX8MQ correctly, since the controller id is relied on it totally. Best Regards Richard Zhu
Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > -----Original Message----- > > From: Bjorn Helgaas <helgaas@kernel.org> > > Sent: 2025年3月11日 23:55 > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > hardcodes > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > -----Original Message----- > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > Sent: 2025年3月10日 23:11 > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > bhelgaas@google.com; > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > hardcodes > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > --- > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > @@ -41,7 +41,6 @@ > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > 8) > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > platform_device > > > > *pdev) > > > > > struct dw_pcie *pci; > > > > > struct imx_pcie *imx_pcie; > > > > > struct device_node *np; > > > > > - struct resource *dbi_base; > > > > > struct device_node *node = dev->of_node; > > > > > int i, ret, req_cnt; > > > > > u16 val; > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > platform_device *pdev) > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > } > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > 0, > > > > &dbi_base); > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > This makes me wonder. > > > > > > > > IIUC this means that previously we set controller_id to 1 if the > > > > first item in devicetree "reg" was 0x33c00000, and now we will set > > > > controller_id to 1 if the devicetree "linux,pci-domain" property is 1. > > > > This is good, but I think this new dependency on the correct > > > > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > > > > > > > My bigger worry is that we no longer set pci->dbi_base at all. I > > > > see that the only use of pci->dbi_base in pci-imx6.c was to > > > > determine the controller_id, but this is a DWC-based driver, and the > > > > DWC core certainly uses > > > > pci->dbi_base. Are we sure that none of those DWC core paths are > > > > important to pci-imx6.c? > > > Hi Bjorn: > > > Thanks for your concerns. > > > Don't worry about the assignment of pci->dbi_base. > > > If pci-imx6.c driver doesn't set it. DWC core driver would set it when > > > dw_pcie_get_resources() is invoked. > > > > Great, thanks! Maybe we can amend the commit log to mention that and > > the new "linux,pci-domain" dependency. > How about the following updates of the commit log? > > Use the domain number replace the hardcodes to uniquely identify > different controller on i.MX8MQ platforms. No function changes. > Please make sure the " linux,pci-domain" is set for i.MX8MQ correctly, since > the controller id is relied on it totally. > This breaks running a new kernel on an old DT without the linux,pci-domain property, which I'm absolutely no fan of. We tried really hard to keep this way around working in the i.MX world. I'm fine with using the property if present and even mandating it for new platforms, but getting rid of the existing code for the i.MX8MQ platform is only a marginal cleanup of the driver code with the obvious downside of the above breakage. Regards, Lucas
On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > -----Original Message----- > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > Sent: 2025年3月11日 23:55 > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > hardcodes > > > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > -----Original Message----- > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > Sent: 2025年3月10日 23:11 > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > > bhelgaas@google.com; > > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > hardcodes > > > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > --- > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > @@ -41,7 +41,6 @@ > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > 8) > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > platform_device > > > > > *pdev) > > > > > > struct dw_pcie *pci; > > > > > > struct imx_pcie *imx_pcie; > > > > > > struct device_node *np; > > > > > > - struct resource *dbi_base; > > > > > > struct device_node *node = dev->of_node; > > > > > > int i, ret, req_cnt; > > > > > > u16 val; > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > platform_device *pdev) > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > } > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > 0, > > > > > &dbi_base); > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > > > This makes me wonder. > > > > > > > > > > IIUC this means that previously we set controller_id to 1 if the > > > > > first item in devicetree "reg" was 0x33c00000, and now we will set > > > > > controller_id to 1 if the devicetree "linux,pci-domain" property is 1. > > > > > This is good, but I think this new dependency on the correct > > > > > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > > > > > > > > > My bigger worry is that we no longer set pci->dbi_base at all. I > > > > > see that the only use of pci->dbi_base in pci-imx6.c was to > > > > > determine the controller_id, but this is a DWC-based driver, and the > > > > > DWC core certainly uses > > > > > pci->dbi_base. Are we sure that none of those DWC core paths are > > > > > important to pci-imx6.c? > > > > Hi Bjorn: > > > > Thanks for your concerns. > > > > Don't worry about the assignment of pci->dbi_base. > > > > If pci-imx6.c driver doesn't set it. DWC core driver would set it when > > > > dw_pcie_get_resources() is invoked. > > > > > > Great, thanks! Maybe we can amend the commit log to mention that and > > > the new "linux,pci-domain" dependency. > > How about the following updates of the commit log? > > > > Use the domain number replace the hardcodes to uniquely identify > > different controller on i.MX8MQ platforms. No function changes. > > Please make sure the " linux,pci-domain" is set for i.MX8MQ correctly, since > > the controller id is relied on it totally. > > > This breaks running a new kernel on an old DT without the > linux,pci-domain property, which I'm absolutely no fan of. We tried > really hard to keep this way around working in the i.MX world. 8MQ already add linux,pci-domain since Jan, 2021 commit c0b70f05c87f3b09b391027c6f056d0facf331ef Author: Peng Fan <peng.fan@nxp.com> Date: Fri Jan 15 11:26:57 2021 +0800 Only missed is pcie-ep side, which have not been used at all boards dts file in upstream. Frank > > I'm fine with using the property if present and even mandating it for > new platforms, but getting rid of the existing code for the i.MX8MQ > platform is only a marginal cleanup of the driver code with the obvious > downside of the above breakage. > > Regards, > Lucas
On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > -----Original Message----- > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > Sent: 2025年3月11日 23:55 > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > -----Original Message----- > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > Sent: 2025年3月10日 23:11 > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > --- > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > @@ -41,7 +41,6 @@ > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > 8) > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > platform_device > > > > > *pdev) > > > > > > struct dw_pcie *pci; > > > > > > struct imx_pcie *imx_pcie; > > > > > > struct device_node *np; > > > > > > - struct resource *dbi_base; > > > > > > struct device_node *node = dev->of_node; > > > > > > int i, ret, req_cnt; > > > > > > u16 val; > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > platform_device *pdev) > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > } > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > 0, > > > > > &dbi_base); > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > - return PTR_ERR(pci->dbi_base); > > Use the domain number replace the hardcodes to uniquely identify > > different controller on i.MX8MQ platforms. No function changes. > > Please make sure the " linux,pci-domain" is set for i.MX8MQ correctly, since > > the controller id is relied on it totally. > > > This breaks running a new kernel on an old DT without the > linux,pci-domain property, which I'm absolutely no fan of. We tried > really hard to keep this way around working in the i.MX world. > > I'm fine with using the property if present and even mandating it for > new platforms, but getting rid of the existing code for the i.MX8MQ > platform is only a marginal cleanup of the driver code with the obvious > downside of the above breakage. I don't know the history of these DTs, but if there are any old DTs for platforms that use controller 1 but lack 'linux,pci-domain', I agree that we should not break them. If we need to retain the dbi_base check so that old DTs continue to work, I think it should look something like this: domain = of_get_pci_domain_nr(node); if (domain >= 0) { if (domain > 1) return dev_err_probe(..., "invalid domain %d\n", domain); imx_pcie->controller_id = domain; } else { dev_warn(..., "DT lacks linux,pci-domain, falling back to DBI addr\n"); dbi_res = platform_get_resource(pdev, IORESOURCE_MEM, index); if (dbi_res->start == IMX8MQ_PCIE2_BASE_ADDR) imx_pcie->controller_id = 1; } The previous code used devm_platform_get_and_ioremap_resource() and set pci->dbi_base, but (1) there's no need to set pci->dbi_base since the DWC core does that anyway, and (2) I think using ioremap() means we depend on CPU virt == CPU phys, and I don't think we need to depend on that. Bjorn
Am Mittwoch, dem 12.03.2025 um 10:22 -0400 schrieb Frank Li: > On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > > -----Original Message----- > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > Sent: 2025年3月11日 23:55 > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > hardcodes > > > > > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > > -----Original Message----- > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > Sent: 2025年3月10日 23:11 > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > > > bhelgaas@google.com; > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > hardcodes > > > > > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > > --- > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > @@ -41,7 +41,6 @@ > > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > > 8) > > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > > platform_device > > > > > > *pdev) > > > > > > > struct dw_pcie *pci; > > > > > > > struct imx_pcie *imx_pcie; > > > > > > > struct device_node *np; > > > > > > > - struct resource *dbi_base; > > > > > > > struct device_node *node = dev->of_node; > > > > > > > int i, ret, req_cnt; > > > > > > > u16 val; > > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > > platform_device *pdev) > > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > > } > > > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > > 0, > > > > > > &dbi_base); > > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > > > > > This makes me wonder. > > > > > > > > > > > > IIUC this means that previously we set controller_id to 1 if the > > > > > > first item in devicetree "reg" was 0x33c00000, and now we will set > > > > > > controller_id to 1 if the devicetree "linux,pci-domain" property is 1. > > > > > > This is good, but I think this new dependency on the correct > > > > > > "linux,pci-domain" in devicetree should be mentioned in the commit log. > > > > > > > > > > > > My bigger worry is that we no longer set pci->dbi_base at all. I > > > > > > see that the only use of pci->dbi_base in pci-imx6.c was to > > > > > > determine the controller_id, but this is a DWC-based driver, and the > > > > > > DWC core certainly uses > > > > > > pci->dbi_base. Are we sure that none of those DWC core paths are > > > > > > important to pci-imx6.c? > > > > > Hi Bjorn: > > > > > Thanks for your concerns. > > > > > Don't worry about the assignment of pci->dbi_base. > > > > > If pci-imx6.c driver doesn't set it. DWC core driver would set it when > > > > > dw_pcie_get_resources() is invoked. > > > > > > > > Great, thanks! Maybe we can amend the commit log to mention that and > > > > the new "linux,pci-domain" dependency. > > > How about the following updates of the commit log? > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > different controller on i.MX8MQ platforms. No function changes. > > > Please make sure the " linux,pci-domain" is set for i.MX8MQ correctly, since > > > the controller id is relied on it totally. > > > > > This breaks running a new kernel on an old DT without the > > linux,pci-domain property, which I'm absolutely no fan of. We tried > > really hard to keep this way around working in the i.MX world. > > 8MQ already add linux,pci-domain since Jan, 2021 > > commit c0b70f05c87f3b09b391027c6f056d0facf331ef > Author: Peng Fan <peng.fan@nxp.com> > Date: Fri Jan 15 11:26:57 2021 +0800 > > Only missed is pcie-ep side, which have not been used at all boards dts > file in upstream. I wasn't aware of this. 2021 is quite a while ago, so I suspect that nobody is going to run a new kernel with a DT this old. I retract my objection. Regards, Lucas
On Thu, Mar 13, 2025 at 09:54:25AM +0100, Lucas Stach wrote: > Am Mittwoch, dem 12.03.2025 um 10:22 -0400 schrieb Frank Li: > > On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > > > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > > > -----Original Message----- > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > Sent: 2025年3月11日 23:55 > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > > > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > > > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > hardcodes > > > > > > > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > > > -----Original Message----- > > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > > Sent: 2025年3月10日 23:11 > > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > > > > bhelgaas@google.com; > > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > > hardcodes > > > > > > > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > > > --- > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > @@ -41,7 +41,6 @@ > > > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > > > 8) > > > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > > > platform_device > > > > > > > *pdev) > > > > > > > > struct dw_pcie *pci; > > > > > > > > struct imx_pcie *imx_pcie; > > > > > > > > struct device_node *np; > > > > > > > > - struct resource *dbi_base; > > > > > > > > struct device_node *node = dev->of_node; > > > > > > > > int i, ret, req_cnt; > > > > > > > > u16 val; > > > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > > > platform_device *pdev) > > > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > > > } > > > > > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > > > 0, > > > > > > > &dbi_base); > > > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > > > > > > > This makes me wonder. > > > > > > > > > > > > > > IIUC this means that previously we set controller_id to > > > > > > > 1 if the first item in devicetree "reg" was 0x33c00000, > > > > > > > and now we will set controller_id to 1 if the devicetree > > > > > > > "linux,pci-domain" property is 1. This is good, but I > > > > > > > think this new dependency on the correct > > > > > > > "linux,pci-domain" in devicetree should be mentioned in > > > > > > > the commit log. > > > > > > > > > > > > > > My bigger worry is that we no longer set pci->dbi_base > > > > > > > at all. I see that the only use of pci->dbi_base in > > > > > > > pci-imx6.c was to determine the controller_id, but this > > > > > > > is a DWC-based driver, and the DWC core certainly uses > > > > > > > pci->dbi_base. Are we sure that none of those DWC core > > > > > > > paths are important to pci-imx6.c? > > > > > > > > > > > > Thanks for your concerns. Don't worry about the > > > > > > assignment of pci->dbi_base. If pci-imx6.c driver doesn't > > > > > > set it. DWC core driver would set it when > > > > > > dw_pcie_get_resources() is invoked. > > > > > > > > > > Great, thanks! Maybe we can amend the commit log to mention > > > > > that and the new "linux,pci-domain" dependency. > > > > > > > > How about the following updates of the commit log? > > > > > > > > Use the domain number replace the hardcodes to uniquely > > > > identify different controller on i.MX8MQ platforms. No > > > > function changes. Please make sure the " linux,pci-domain" is > > > > set for i.MX8MQ correctly, since the controller id is relied > > > > on it totally. > > > > > > > This breaks running a new kernel on an old DT without the > > > linux,pci-domain property, which I'm absolutely no fan of. We > > > tried really hard to keep this way around working in the i.MX > > > world. > > > > 8MQ already add linux,pci-domain since Jan, 2021 > > > > commit c0b70f05c87f3b09b391027c6f056d0facf331ef > > Author: Peng Fan <peng.fan@nxp.com> > > Date: Fri Jan 15 11:26:57 2021 +0800 > > > > Only missed is pcie-ep side, which have not been used at all boards dts > > file in upstream. > > I wasn't aware of this. 2021 is quite a while ago, so I suspect that > nobody is going to run a new kernel with a DT this old. I retract my > objection. Sounds good, thanks, Lucas. We really do want to avoid breaking old DTs, so I appreciate your highlighting of it. Even if we believe none of them will break, I think it's worth mentioning the 'linux,pci-domain' dependency and the commit that added it to the .dtsi in the commit log. Bjorn
On Thu, Mar 13, 2025 at 11:06:48AM -0500, Bjorn Helgaas wrote: > On Thu, Mar 13, 2025 at 09:54:25AM +0100, Lucas Stach wrote: > > Am Mittwoch, dem 12.03.2025 um 10:22 -0400 schrieb Frank Li: > > > On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > > > > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > > > > -----Original Message----- > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > Sent: 2025年3月11日 23:55 > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > > > > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > hardcodes > > > > > > > > > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > > > > -----Original Message----- > > > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > > > Sent: 2025年3月10日 23:11 > > > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > > > > > bhelgaas@google.com; > > > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > > > hardcodes > > > > > > > > > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > > > > --- > > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > @@ -41,7 +41,6 @@ > > > > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > > > > 8) > > > > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > > > > platform_device > > > > > > > > *pdev) > > > > > > > > > struct dw_pcie *pci; > > > > > > > > > struct imx_pcie *imx_pcie; > > > > > > > > > struct device_node *np; > > > > > > > > > - struct resource *dbi_base; > > > > > > > > > struct device_node *node = dev->of_node; > > > > > > > > > int i, ret, req_cnt; > > > > > > > > > u16 val; > > > > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > > > > platform_device *pdev) > > > > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > > > > } > > > > > > > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > > > > 0, > > > > > > > > &dbi_base); > > > > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > > > > > > > > > This makes me wonder. > > > > > > > > > > > > > > > > IIUC this means that previously we set controller_id to > > > > > > > > 1 if the first item in devicetree "reg" was 0x33c00000, > > > > > > > > and now we will set controller_id to 1 if the devicetree > > > > > > > > "linux,pci-domain" property is 1. This is good, but I > > > > > > > > think this new dependency on the correct > > > > > > > > "linux,pci-domain" in devicetree should be mentioned in > > > > > > > > the commit log. > > > > > > > > > > > > > > > > My bigger worry is that we no longer set pci->dbi_base > > > > > > > > at all. I see that the only use of pci->dbi_base in > > > > > > > > pci-imx6.c was to determine the controller_id, but this > > > > > > > > is a DWC-based driver, and the DWC core certainly uses > > > > > > > > pci->dbi_base. Are we sure that none of those DWC core > > > > > > > > paths are important to pci-imx6.c? > > > > > > > > > > > > > > Thanks for your concerns. Don't worry about the > > > > > > > assignment of pci->dbi_base. If pci-imx6.c driver doesn't > > > > > > > set it. DWC core driver would set it when > > > > > > > dw_pcie_get_resources() is invoked. > > > > > > > > > > > > Great, thanks! Maybe we can amend the commit log to mention > > > > > > that and the new "linux,pci-domain" dependency. > > > > > > > > > > How about the following updates of the commit log? > > > > > > > > > > Use the domain number replace the hardcodes to uniquely > > > > > identify different controller on i.MX8MQ platforms. No > > > > > function changes. Please make sure the " linux,pci-domain" is > > > > > set for i.MX8MQ correctly, since the controller id is relied > > > > > on it totally. > > > > > > > > > This breaks running a new kernel on an old DT without the > > > > linux,pci-domain property, which I'm absolutely no fan of. We > > > > tried really hard to keep this way around working in the i.MX > > > > world. > > > > > > 8MQ already add linux,pci-domain since Jan, 2021 > > > > > > commit c0b70f05c87f3b09b391027c6f056d0facf331ef > > > Author: Peng Fan <peng.fan@nxp.com> > > > Date: Fri Jan 15 11:26:57 2021 +0800 > > > > > > Only missed is pcie-ep side, which have not been used at all boards dts > > > file in upstream. > > > > I wasn't aware of this. 2021 is quite a while ago, so I suspect that > > nobody is going to run a new kernel with a DT this old. I retract my > > objection. > > Sounds good, thanks, Lucas. We really do want to avoid breaking old > DTs, so I appreciate your highlighting of it. Even if we believe none > of them will break, I think it's worth mentioning the > 'linux,pci-domain' dependency and the commit that added it to the > .dtsi in the commit log. Yes, I think you can add it? some thing like "linux,pci-domain" must match the PCIe controller instance sequence for the i.MX8MQ platform. Frank > > Bjorn
On Thu, Mar 13, 2025 at 11:06:48AM -0500, Bjorn Helgaas wrote: > On Thu, Mar 13, 2025 at 09:54:25AM +0100, Lucas Stach wrote: > > Am Mittwoch, dem 12.03.2025 um 10:22 -0400 schrieb Frank Li: > > > On Wed, Mar 12, 2025 at 09:28:02AM +0100, Lucas Stach wrote: > > > > Am Mittwoch, dem 12.03.2025 um 04:05 +0000 schrieb Hongxing Zhu: > > > > > > -----Original Message----- > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > Sent: 2025年3月11日 23:55 > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; bhelgaas@google.com; > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org; > > > > > > linux-pci@vger.kernel.org; imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > hardcodes > > > > > > > > > > > > On Tue, Mar 11, 2025 at 01:11:04AM +0000, Hongxing Zhu wrote: > > > > > > > > -----Original Message----- > > > > > > > > From: Bjorn Helgaas <helgaas@kernel.org> > > > > > > > > Sent: 2025年3月10日 23:11 > > > > > > > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > > > > > > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > > > > > > > > shawnguo@kernel.org; l.stach@pengutronix.de; lpieralisi@kernel.org; > > > > > > > > kw@linux.com; manivannan.sadhasivam@linaro.org; > > > > > > bhelgaas@google.com; > > > > > > > > s.hauer@pengutronix.de; festevam@gmail.com; > > > > > > > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; > > > > > > > > imx@lists.linux.dev; kernel@pengutronix.de; > > > > > > > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > > > > > > > Subject: Re: [PATCH v1 2/2] PCI: imx6: Use domain number replace the > > > > > > > > hardcodes > > > > > > > > > > > > > > > > On Wed, Feb 26, 2025 at 10:42:56AM +0800, Richard Zhu wrote: > > > > > > > > > Use the domain number replace the hardcodes to uniquely identify > > > > > > > > > different controller on i.MX8MQ platforms. No function changes. > > > > > > > > > > > > > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > > > > > > > --- > > > > > > > > > drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > index 90ace941090f..ab9ebb783593 100644 > > > > > > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > > > > > > > @@ -41,7 +41,6 @@ > > > > > > > > > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) > > > > > > > > > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) > > > > > > > > > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, > > > > > > 8) > > > > > > > > > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 > > > > > > > > > > > > > > > > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > > > > > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > > > > @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct > > > > > > > > > platform_device > > > > > > > > *pdev) > > > > > > > > > struct dw_pcie *pci; > > > > > > > > > struct imx_pcie *imx_pcie; > > > > > > > > > struct device_node *np; > > > > > > > > > - struct resource *dbi_base; > > > > > > > > > struct device_node *node = dev->of_node; > > > > > > > > > int i, ret, req_cnt; > > > > > > > > > u16 val; > > > > > > > > > @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct > > > > > > > > platform_device *pdev) > > > > > > > > > return PTR_ERR(imx_pcie->phy_base); > > > > > > > > > } > > > > > > > > > > > > > > > > > > - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, > > > > > > 0, > > > > > > > > &dbi_base); > > > > > > > > > - if (IS_ERR(pci->dbi_base)) > > > > > > > > > - return PTR_ERR(pci->dbi_base); > > > > > > > > > > > > > > > > This makes me wonder. > > > > > > > > > > > > > > > > IIUC this means that previously we set controller_id to > > > > > > > > 1 if the first item in devicetree "reg" was 0x33c00000, > > > > > > > > and now we will set controller_id to 1 if the devicetree > > > > > > > > "linux,pci-domain" property is 1. This is good, but I > > > > > > > > think this new dependency on the correct > > > > > > > > "linux,pci-domain" in devicetree should be mentioned in > > > > > > > > the commit log. > > > > > > > > > > > > > > > > My bigger worry is that we no longer set pci->dbi_base > > > > > > > > at all. I see that the only use of pci->dbi_base in > > > > > > > > pci-imx6.c was to determine the controller_id, but this > > > > > > > > is a DWC-based driver, and the DWC core certainly uses > > > > > > > > pci->dbi_base. Are we sure that none of those DWC core > > > > > > > > paths are important to pci-imx6.c? > > > > > > > > > > > > > > Thanks for your concerns. Don't worry about the > > > > > > > assignment of pci->dbi_base. If pci-imx6.c driver doesn't > > > > > > > set it. DWC core driver would set it when > > > > > > > dw_pcie_get_resources() is invoked. > > > > > > > > > > > > Great, thanks! Maybe we can amend the commit log to mention > > > > > > that and the new "linux,pci-domain" dependency. > > > > > > > > > > How about the following updates of the commit log? > > > > > > > > > > Use the domain number replace the hardcodes to uniquely > > > > > identify different controller on i.MX8MQ platforms. No > > > > > function changes. Please make sure the " linux,pci-domain" is > > > > > set for i.MX8MQ correctly, since the controller id is relied > > > > > on it totally. > > > > > > > > > This breaks running a new kernel on an old DT without the > > > > linux,pci-domain property, which I'm absolutely no fan of. We > > > > tried really hard to keep this way around working in the i.MX > > > > world. > > > > > > 8MQ already add linux,pci-domain since Jan, 2021 > > > > > > commit c0b70f05c87f3b09b391027c6f056d0facf331ef > > > Author: Peng Fan <peng.fan@nxp.com> > > > Date: Fri Jan 15 11:26:57 2021 +0800 > > > > > > Only missed is pcie-ep side, which have not been used at all boards dts > > > file in upstream. > > > > I wasn't aware of this. 2021 is quite a while ago, so I suspect that > > nobody is going to run a new kernel with a DT this old. I retract my > > objection. > > Sounds good, thanks, Lucas. We really do want to avoid breaking old > DTs, so I appreciate your highlighting of it. Even if we believe none > of them will break, I think it's worth mentioning the > 'linux,pci-domain' dependency and the commit that added it to the > .dtsi in the commit log. > If there is a dependency, then it should be added to the binding. Only that will ensure that the DTs will have the dependent property present. - Mani
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 90ace941090f..ab9ebb783593 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -41,7 +41,6 @@ #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11) #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12) #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8) -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000 #define IMX95_PCIE_PHY_GEN_CTRL 0x0 #define IMX95_PCIE_REF_USE_PAD BIT(17) @@ -1474,7 +1473,6 @@ static int imx_pcie_probe(struct platform_device *pdev) struct dw_pcie *pci; struct imx_pcie *imx_pcie; struct device_node *np; - struct resource *dbi_base; struct device_node *node = dev->of_node; int i, ret, req_cnt; u16 val; @@ -1515,10 +1513,6 @@ static int imx_pcie_probe(struct platform_device *pdev) return PTR_ERR(imx_pcie->phy_base); } - pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base); - if (IS_ERR(pci->dbi_base)) - return PTR_ERR(pci->dbi_base); - /* Fetch GPIOs */ imx_pcie->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(imx_pcie->reset_gpiod)) @@ -1565,8 +1559,12 @@ static int imx_pcie_probe(struct platform_device *pdev) switch (imx_pcie->drvdata->variant) { case IMX8MQ: case IMX8MQ_EP: - if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) - imx_pcie->controller_id = 1; + ret = of_get_pci_domain_nr(node); + if (ret < 0 || ret > 1) + return dev_err_probe(dev, -ENODEV, + "failed to get valid pcie domain\n"); + else + imx_pcie->controller_id = ret; break; default: break;
Use the domain number replace the hardcodes to uniquely identify different controller on i.MX8MQ platforms. No function changes. Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)