On Wed, Jan 29, 2025 at 05:19:37PM -0600, Bjorn Helgaas wrote: > On Tue, Jan 28, 2025 at 05:07:34PM -0500, Frank Li wrote: > > Pass resource start as the input argument to iomap() instead of > > atu.cpu_address in dw_pcie_pme_turn_off(). While atu.cpu_address happens to > > be the same here, it actually represents the parent bus address before ATU, > > which may not always align with the CPU address. Using resource start > > ensures correctness and clarity. > > 1) "atu.cpu_address happens to be the same here" is currently true > but only because this function is buggy and assumes the ATU input > address is the same as a CPU physical address. > > 2) We're trying to make a correctness fix, not a clarity fix. This > patch by itself isn't a functional change because of the cpu_addr > bug, but without this patch, fixing the cpu_addr bug would break > the iomap. > > 3) "Pass resource start as the input to iomap()" just restates the > patch. We should say *why* this is important. Something like > this: > > The msg_res region translates writes into PCIe Message TLPs. > Previously we mapped this region using atu.cpu_addr, the input > address programmed into the ATU. > > "cpu_addr" is a misnomer because when a bus fabric translates > addresses between the CPU and the ATU, the ATU input address is > different from the CPU address. A future patch will rename > "cpu_addr" and correct the value to be the ATU input address > instead of the CPU physical address. > > Map the msg_res region before writing to it using the msg_res > resource start, a CPU physical address. Thanks, will update commit message. The key change is patch 3 PCI: Add parent_bus_offset to resource_entry Frank > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > --- > > change from v9 to v10 > > - new patch > > --- > > drivers/pci/controller/dwc/pcie-designware-host.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index ffaded8f2df7b..ae3fd2a5dbf85 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -908,7 +908,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci) > > if (ret) > > return ret; > > > > - mem = ioremap(atu.cpu_addr, pci->region_align); > > + mem = ioremap(pci->pp.msg_res->start, pci->region_align); > > if (!mem) > > return -ENOMEM; > > > > > > -- > > 2.34.1 > >
