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.
> 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
>
