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

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 8/7/2024 7:04 AM, Manivannan Sadhasivam wrote:
> On Wed, Jul 31, 2024 at 06:28:23PM -0400, Jim Quinlan wrote:
>> 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 <james.quinlan@broadcom.com>
>> ---

[snip]

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