Hi Lorenzo,

On Tue, 2020-01-14 at 17:11 +0000, Lorenzo Pieralisi wrote:

On Mon, Dec 16, 2019 at 12:01:09PM +0100, Nicolas Saenz Julienne wrote:

quoted

From: Jim Quinlan <redacted>

This adds a basic driver for Broadcom's STB PCIe controller, for now
aimed at Raspberry Pi 4's SoC, bcm2711.

Signed-off-by: Jim Quinlan <redacted>
Co-developed-by: Nicolas Saenz Julienne <redacted>
Signed-off-by: Nicolas Saenz Julienne <redacted>
Reviewed-by: Andrew Murray <redacted>
Reviewed-by: Jeremy Linton <redacted>

---

Changes since v3:
  - Update commit message
  - rollback roundup_pow_two usage, it'll be updated later down the line
  - Remove comment in register definition

Changes since v2:
  - Correct rc_bar2_offset sign

In relation to this change.

[...]

quoted

+static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie
*pcie,
+							u64 *rc_bar2_size,
+							u64 *rc_bar2_offset)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	struct device *dev = pcie->dev;
+	struct resource_entry *entry;
+
+	entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
+	if (!entry)
+		return -ENODEV;
+
+	*rc_bar2_offset = -entry->offset;

I think this deserves a comment - I guess it has to do with how the
controller expects CPU<->PCI offsets to be expressed compared to how it
is computed in dma_ranges entries.

You're right, OF code calculates it by doing:

	offset = cpu_start_addr - pci_start_addr (see
devm_of_pci_get_host_bridge_resources())

While the RC_BAR2_CONFIG register expects the opposite subtraction. I'll add a
comment on the next revision.

I will try to complete the review shortly and try to apply it given
that it has already been reviewed by others.

Thanks!

Regards,
Nicolas

Lorenzo