Re: [PATCH] PCI: xilinx-nwl: Enable coherenct PCIe traffic using CCI

[Rob Herring <robh@kernel.org>]

On Thu, Jan 21, 2021 at 10:28 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+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?

It is generic (beyond PCI), but needs to be documented as used in each
PCI host binding. Don't need any more than 'dma-coherent: true'.

>
> > 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
> >