Skip to content

Commit 3b69e1d

Browse files
nxpfranklibjorn-helgaas
authored andcommitted
PCI: dwc: Add dw_pcie_parent_bus_offset() checking and debug
dw_pcie_parent_bus_offset() looks up the parent bus address of a PCI controller 'reg' property in devicetree. If implemented, .cpu_addr_fixup() is a hard-coded way to get the parent bus address corresponding to a CPU physical address. Add debug code to compare the address from .cpu_addr_fixup() with the address from devicetree. If they match, warn that .cpu_addr_fixup() is redundant and should be removed; if they differ, warn that something is wrong with the devicetree. If .cpu_addr_fixup() is not implemented, the parent bus address should be identical to the CPU physical address because we previously ignored the parent bus address from devicetree. If the devicetree has a different parent bus address, warn about it being broken. [bhelgaas: split debug to separate patch for easier future revert, commit log] Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Frank Li <[email protected]> [bhelgaas: squash Ioana Ciornei <[email protected]> fix for NULL pointer deref when driver doesn't supply dw_pcie_ops, e.g., layerscape-pcie https://lore.kernel.org/r/[email protected]] Signed-off-by: Bjorn Helgaas <[email protected]>
1 parent 9de3f3c commit 3b69e1d

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

drivers/pci/controller/dwc/pcie-designware.c

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,8 @@ resource_size_t dw_pcie_parent_bus_offset(struct dw_pcie *pci,
11141114
struct device *dev = pci->dev;
11151115
struct device_node *np = dev->of_node;
11161116
int index;
1117-
u64 reg_addr;
1117+
u64 reg_addr, fixup_addr;
1118+
u64 (*fixup)(struct dw_pcie *pcie, u64 cpu_addr);
11181119

11191120
/* Look up reg_name address on parent bus */
11201121
index = of_property_match_string(np, "reg-names", reg_name);
@@ -1126,5 +1127,42 @@ resource_size_t dw_pcie_parent_bus_offset(struct dw_pcie *pci,
11261127

11271128
of_property_read_reg(np, index, &reg_addr, NULL);
11281129

1130+
fixup = pci->ops ? pci->ops->cpu_addr_fixup : NULL;
1131+
if (fixup) {
1132+
fixup_addr = fixup(pci, cpu_phys_addr);
1133+
if (reg_addr == fixup_addr) {
1134+
dev_info(dev, "%s reg[%d] %#010llx == %#010llx == fixup(cpu %#010llx); %ps is redundant with this devicetree\n",
1135+
reg_name, index, reg_addr, fixup_addr,
1136+
(unsigned long long) cpu_phys_addr, fixup);
1137+
} else {
1138+
dev_warn(dev, "%s reg[%d] %#010llx != %#010llx == fixup(cpu %#010llx); devicetree is broken\n",
1139+
reg_name, index, reg_addr, fixup_addr,
1140+
(unsigned long long) cpu_phys_addr);
1141+
reg_addr = fixup_addr;
1142+
}
1143+
1144+
return cpu_phys_addr - reg_addr;
1145+
}
1146+
1147+
if (pci->use_parent_dt_ranges) {
1148+
1149+
/*
1150+
* This platform once had a fixup, presumably because it
1151+
* translates between CPU and PCI controller addresses.
1152+
* Log a note if devicetree didn't describe a translation.
1153+
*/
1154+
if (reg_addr == cpu_phys_addr)
1155+
dev_info(dev, "%s reg[%d] %#010llx == cpu %#010llx\n; no fixup was ever needed for this devicetree\n",
1156+
reg_name, index, reg_addr,
1157+
(unsigned long long) cpu_phys_addr);
1158+
} else {
1159+
if (reg_addr != cpu_phys_addr) {
1160+
dev_warn(dev, "%s reg[%d] %#010llx != cpu %#010llx; no fixup and devicetree \"ranges\" is broken, assuming no translation\n",
1161+
reg_name, index, reg_addr,
1162+
(unsigned long long) cpu_phys_addr);
1163+
return 0;
1164+
}
1165+
}
1166+
11291167
return cpu_phys_addr - reg_addr;
11301168
}

drivers/pci/controller/dwc/pcie-designware.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,19 @@ struct dw_pcie {
465465
struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS];
466466
struct gpio_desc *pe_rst;
467467
bool suspended;
468+
469+
/*
470+
* If iATU input addresses are offset from CPU physical addresses,
471+
* we previously required .cpu_addr_fixup() to convert them. We
472+
* now rely on the devicetree instead. If .cpu_addr_fixup()
473+
* exists, we compare its results with devicetree.
474+
*
475+
* If .cpu_addr_fixup() does not exist, we assume the offset is
476+
* zero and warn if devicetree claims otherwise. If we know all
477+
* devicetrees correctly describe the offset, set
478+
* use_parent_dt_ranges to true to avoid this warning.
479+
*/
480+
bool use_parent_dt_ranges;
468481
};
469482

470483
#define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)

0 commit comments

Comments
 (0)