Thread (52 messages) 52 messages, 5 authors, 2024-08-14

Re: [PATCH v5 09/12] PCI: brcmstb: Refactor for chips with many regular inbound windows

From: Manivannan Sadhasivam <hidden>
Date: 2024-08-07 15:04:15
Also in: linux-pci, lkml

On Wed, Aug 07, 2024 at 07:16:44AM -0700, Florian Fainelli wrote:

On 8/7/2024 7:04 AM, Manivannan Sadhasivam wrote:
quoted
On Wed, Jul 31, 2024 at 06:28:23PM -0400, Jim Quinlan wrote:
quoted
Provide support for new chips with multiple inbound windows while
keeping the legacy support for the older chips.

In existing chips there are three inbound windows with fixed purposes: the
first was for mapping SoC internal registers, the second was for memory,
and the third was for memory but with the endian swapped.  Typically, only
one window was used.

Complicating the inbound window usage was the fact that the PCIe HW would
do a baroque internal mapping of system memory, and concatenate the regions
of multiple memory controllers.

Newer chips such as the 7712 and Cable Modem SOCs take a step forward and
drop the internal mapping while providing for multiple inbound windows.
This works in concert with the dma-ranges property, where each provided
range becomes an inbound window.

Signed-off-by: Jim Quinlan <redacted>
---
[snip]
quoted
quoted
+static void set_inbound_win_registers(struct brcm_pcie *pcie,
+				      const struct inbound_win *inbound_wins,
+				      int num_inbound_wins)
+{
+	void __iomem *base = pcie->base;
+	int i;
+
+	for (i = 1; i <= num_inbound_wins; i++) {
+		u64 pci_offset = inbound_wins[i].pci_offset;
+		u64 cpu_addr = inbound_wins[i].cpu_addr;
+		u64 size = inbound_wins[i].size;
+		u32 reg_offset = brcm_bar_reg_offset(i);
+		u32 tmp = lower_32_bits(pci_offset);
+
+		u32p_replace_bits(&tmp, brcm_pcie_encode_ibar_size(size),
+				  PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK);
+
+		/* Write low */
+		writel(tmp, base + reg_offset);
Can you use writel_relaxed() instead? Here and below. I don't see a necessity to
use the barrier that comes with non-relaxed version of writel.
Out of curiosity what is the reasoning here for asking to use
writel_relaxed(), this is not a hot path, this is a configuration path
anyway. I am not certain clear on the implication of using writel_relaxed()
on systems like 7712/2712 where the busing is different from the other STB
chips.
It is the general recommendation (although not documented). If the code path do
not need ordering/barrier, then there is no need for non-relaxed variants.

Btw, if the register accesses are to the same domain (like PCIe), then certainly
you do not need barrier as writes to the same domain are ordered.

Problem with readl/writel is that the drivers started using non-relaxed variants
as if it is a norm and completely ignored the relaxed variants.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help