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@xxxxxxxxxx> > > > 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@xxxxxxxxxx> > > > > > 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@xxxxxxx> > > > > > > --- > > > > > > 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