Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 21 Jan 2021 10:28:27 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc:     linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        bhelgaas@google.com, Rob Herring <robh@kernel.org>
Subject: Re: [PATCH] PCI: xilinx-nwl: Enable coherenct PCIe traffic using CCI
Message-ID: <20210121162827.GA2658969@bjorn-Precision-5520>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1611223156-8787-1-git-send-email-bharat.kumar.gogada@xilinx.com>
Precedence: bulk

[+cc Rob]

s/coherenct/coherent/ in subject
s/traffic/DMA/ (this applies specifically to DMA, not to MMIO)

On Thu, Jan 21, 2021 at 03:29:16PM +0530, Bharat Kumar Gogada wrote:
> - Add support for routing PCIe traffic coherently when
>  Cache Coherent Interconnect(CCI) is enabled in the system.

s/- Add/Add/
s/Interconnect(CCI)/Interconnect (CCI)/

Can you include a URL to a CCI spec?  I'm not familiar with it.  I
guess this is something upstream from the host bridge, i.e., between
the CPU and the host bridge, so it's outside the PCI domain?

I'd like to mention the DT "dma-coherent" property in the commit log
to help connect this with the knob that controls it.

The "dma-coherent" property is mentioned several places in
Documentation/devicetree/bindings/pci/ (but not anything obviously
related to xilinx-nwl).  Should it be moved to something like
Documentation/devicetree/bindings/pci/pci.txt to make it more generic?

> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 07e3666..08e06057 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -26,6 +26,7 @@
>  
>  /* Bridge core config registers */
>  #define BRCFG_PCIE_RX0			0x00000000
> +#define BRCFG_PCIE_RX1			0x00000004
>  #define BRCFG_INTERRUPT			0x00000010
>  #define BRCFG_PCIE_RX_MSG_FILTER	0x00000020
>  
> @@ -128,6 +129,7 @@
>  #define NWL_ECAM_VALUE_DEFAULT		12
>  
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
> +#define CFG_PCIE_CACHE			GENMASK(7, 0)
>  
>  #define INT_PCI_MSI_NR			(2 * 32)
>  
> @@ -675,6 +677,12 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	nwl_bridge_writel(pcie, CFG_ENABLE_MSG_FILTER_MASK,
>  			  BRCFG_PCIE_RX_MSG_FILTER);
>  
> +	/* This routes the PCIe DMA traffic to go through CCI path */
> +	if (of_dma_is_coherent(dev->of_node)) {
> +		nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, BRCFG_PCIE_RX1) |
> +				  CFG_PCIE_CACHE, BRCFG_PCIE_RX1);
> +	}
> +
>  	err = nwl_wait_for_link(pcie);
>  	if (err)
>  		return err;
> -- 
> 2.7.4
>