[+cc Frank, who can probably answer faster than I can]
On Fri, Aug 29, 2025 at 05:41:17PM +0800, Randolph Lin wrote:
> Rn Wed, Aug 20, 2025 at 10:54:44AM -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 20, 2025 at 07:18:42PM +0800, Randolph Lin wrote:
> > > Add driver support for DesignWare based PCIe controller in Andes
> > > QiLai SoC. The driver only supports the Root Complex mode.
> > > +#define PCIE_LOGIC_COHERENCY_CONTROL3 0x8e8
> > > +/* Write-Back, Read and Write Allocate */
> > > +#define IOCP_ARCACHE 0xf
> > > +/* Write-Back, Read and Write Allocate */
> > > +#define IOCP_AWCACHE 0xf
> >
> > Are IOCP_ARCACHE and IOCP_AWCACHE supposed to be identical values with
> > identical comments?
Just pointing this out since you didn't respond to it.
> > > +static u64 qilai_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
> > > +{
> > > + struct dw_pcie_rp *pp = &pci->pp;
> > > +
> > > + return cpu_addr - pp->cfg0_base;
> >
> > Sorry, we can't do this. We're removing .cpu_addr_fixup() because
> > it's a workaround for defects in the DT description. See these
> > commits, for example:
> >
> > befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
> > b9812179f601 ("PCI: imx6: Remove imx_pcie_cpu_addr_fixup()")
> > 07ae413e169d ("PCI: intel-gw: Remove intel_pcie_cpu_addr()")
>
> I’m a bit confused about the following question:
> After removing cpu_addr_fixup, should we use pci->parent_bus_offset
> to store the offset value, or should pci->parent_bus_offset remain
> 0?
If you needed qilai_pcie_cpu_addr_fixup(), I would expect your
dw_pcie.parent_bus_offset to be non-zero because parent_bus_offset is
used instead of ->cpu_addr_fixup().
> In the commit message:
> befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
> We know the parent_bus_offset, either computed from a DT reg
> property (the offset is the CPU physical addr - the
> 'config'/'addr_space' address on the parent bus) or from a
> .cpu_addr_fixup() (which may have used a host bridge window
> offset).
>
> We know that "the offset is the CPU physical addr - the
> 'config'/'addr_space' address on the parent bus".
>
> However, in dw_pcie_host_get_resources(), it passes pp->cfg0_base,
> which is parsed from the device tree using "config", as the
> cpu_phys_addr parameter to dw_pcie_parent_bus_offset(). It also
> passes "config" as the 2nd parameter to dw_pcie_parent_bus_offset().
>
> In dw_pcie_parent_bus_offset(), the 2nd parameter is used to get the
> index from the devicetree "reg-names" field, and the result is used
> as the 'config'/'addr_space' address.
>
> It seems that the same value is being obtained through a different
> method, and the return value appears to be 0. Could I be
> misunderstanding something?
