On Wed, 2020-01-15 at 10:00 +0000, Lorenzo Pieralisi wrote:

On Tue, Jan 14, 2020 at 07:18:46PM +0100, Nicolas Saenz Julienne wrote:

quoted

Hi Lorenzo,

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

quoted

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.

There is no need for a new posting, either write that comment here
and I update the code or inline the patch or just resend *this* updated
patch to the list.

OK, hope this sounds good enough:

	/*
	 * The controller expects the inbound window offset to be calculated as
	 * the difference between PCIe's address space and CPU's. The offset
	 * provided by the firmware is calculated the opposite way, so we
	 * negate it.
	 */

Regards,
Nicolas