Re: [PATCH v8 11/11] PCI: imx6: Add i.MX8Q PCIe root complex (RC) support

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Jul 29, 2024 at 04:18:18PM -0400, Frank Li wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Implement i.MX8Q (i.MX8QM, i.MX8QXP, and i.MX8DXL) PCIe RC support. While
> the controller resembles that of iMX8MP, the PHY differs significantly.
> Notably, there's a distinction between PCI bus addresses and CPU addresses.

This bus/CPU address distinction is unrelated to the PHY despite the
fact that this phrasing suggests they might be related.

> Introduce IMX_PCIE_FLAG_CPU_ADDR_FIXUP in drvdata::flags to indicate driver
> need the cpu_addr_fixup() callback to facilitate CPU address to PCI bus
> address conversion according to "ranges" property.

I actually don't understand why the .cpu_addr_fixup() callback exists
at all.  I guess this is my lack of understanding here, but on the
ACPI side, if CPU addresses and PCI bus addresses are different, ACPI
tells us how to convert them.  It seems like it should be analogous
for DT.

> +static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
> +{
> +	struct imx_pcie *imx_pcie = to_imx_pcie(pcie);
> +	struct dw_pcie_rp *pp = &pcie->pp;
> +	struct resource_entry *entry;
> +	unsigned int offset;
> +
> +	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_CPU_ADDR_FIXUP))
> +		return cpu_addr;
> +
> +	entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	offset = entry->offset;

I would have assumed that if the DT is correct, "offset" will be zero
for platforms where PCI bus addresses are identical to CPU addresses,
so we could (and *should*) do this for all platforms, not just IMX8Q.
But I must be missing something?

> +	return (cpu_addr - offset);
> +}