[PATCH v8 1/3] PCI: mediatek: Use actual physical address instead of virt_to_phys()

[Manivannan Sadhasivam]

On Wed, May 20, 2026 at 02:59:00PM -0500, Bjorn Helgaas wrote:
> On Wed, May 20, 2026 at 09:17:35PM +0200, Caleb James DeLisle wrote:
> > 
> > On 20/05/2026 20:55, Bjorn Helgaas wrote:
> > > On Wed, May 20, 2026 at 06:38:25PM +0000, Caleb James DeLisle wrote:
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam at oss.qualcomm.com>
> > > > 
> > > > The driver previously used virt_to_phys() on the ioremapped register base
> > > > (port->base) to compute the MSI message address. Using virt_to_phys() on an
> > > > IO mapped address is incorrect because it expects a kernel virtual address.
> > > > 
> > > > To fix it, store the physical start of the I/O register region in
> > > > mtk_pcie_port->phys_base and use it to build the MSI address. This replaces
> > > > the incorrect virt_to_phys() usage and ensures MSI addresses are generated
> > > > correctly.
> > > > 
> > > > Fixes: 43e6409db64d ("PCI: mediatek: Add MSI support for MT2712 and MT7622")
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam at oss.qualcomm.com>
> > > > Tested-by: Caleb James DeLisle <cjd at cjdns.fr>
> > > > ---
> > > >   drivers/pci/controller/pcie-mediatek.c | 16 +++++++++++++---
> > > >   1 file changed, 13 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > > index 75722524fe74..c503fbd774d0 100644
> > > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > > @@ -175,6 +175,7 @@ struct mtk_pcie_soc {
> > > >   /**
> > > >    * struct mtk_pcie_port - PCIe port information
> > > >    * @base: IO mapped register base
> > > > + * @phys_base: Physical address of the I/O register base region
> > > >    * @list: port list
> > > >    * @pcie: pointer to PCIe host info
> > > >    * @reset: pointer to port reset control
> > > > @@ -196,6 +197,7 @@ struct mtk_pcie_soc {
> > > >    */
> > > >   struct mtk_pcie_port {
> > > >   	void __iomem *base;
> > > > +	phys_addr_t phys_base;
> > > >   	struct list_head list;
> > > >   	struct mtk_pcie *pcie;
> > > >   	struct reset_control *reset;
> > > > @@ -405,7 +407,7 @@ static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> > > >   	phys_addr_t addr;
> > > >   	/* MT2712/MT7622 only support 32-bit MSI addresses */
> > > > -	addr = virt_to_phys(port->base + PCIE_MSI_VECTOR);
> > > > +	addr = port->phys_base + PCIE_MSI_VECTOR;
> > >
> > > This doesn't look right because the MSI address is a PCI bus address,
> > > and port->phys_base is a CPU physical address.  Often a PCI bus
> > > address is the same as the CPU physical address, but not always.
> > > I think the DT 'ranges' property tells you the translation.
> 
> Oops, sorry, I muddied the waters here.
> 
> 'ranges' tells you the translation applied by a bridge, e.g., when a
> CPU does a load/store, the PCI host bridge turns it into a PCI
> read/write transaction.  The bridge might add an offset to the CPU
> load/store physical address to get the PCI read/write bus address.
> 
> But that's not the issue here.  The MSI is basically a DMA write
> performed by the PCI device, not a store done by a CPU, so I don't
> think 'ranges' is the right thing to look at.
> 

Yeah, it is so easy to confuse both. To summarise, 'ranges' describes the
outbound translation and 'dma-ranges' describes the inbound translation from
host perspective.

> Based on this:
> https://elinux.org/Device_Tree_Usage#PCI_DMA_Address_Translation
> I think 'dma-ranges' is the relevant property.  I don't think your DT
> includes a 'dma-ranges' property, and in that case the default is that 
> the system bus (CPU) address is the same as the PCI address.
> 
> So I think this patch works because it assumes DMA addresses like the
> MSI address are mapped to identical system bus addresses.
> 
> It still seems to me that drivers should be prepared for the presence
> of dma-ranges and use it when computing the MSI target address.  But I
> don't think any drivers really do that, so for now I think you should
> pretend that I never responded about this patch.
> 

Your observations are correct. This driver assumes that the identical mapping
exists between CPU and PCI bus addresses. Usually, the drivers make use of
phys_to_dma() to handle the translations. This API internally makes use of the
'dma_range_map' which gets populated by the OF core based on the 'dma-ranges'
property (if present in DT).

But it makes sense to use it irrespective of whether the platform supports
non-identical DMA/inbound translation or not. Since this API behaves like a
no-op and returns the CPU physical address if there is an identical mapping,
there is literally zero overhead in using it.

- Mani

-- 
மணிவண்ணன் சதாசிவம்