2021-01-21 10:02:50

by Bharat Kumar Gogada

[permalink] [raw]
Subject: [PATCH] PCI: xilinx-nwl: Enable coherenct PCIe traffic using CCI

- Add support for routing PCIe traffic coherently when
Cache Coherent Interconnect(CCI) is enabled in the system.

Signed-off-by: Bharat Kumar Gogada <[email protected]>
---
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


2021-01-21 16:31:30

by Bjorn Helgaas

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

[+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 <[email protected]>
> ---
> 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
>

2021-01-27 21:14:56

by Bharat Kumar Gogada

[permalink] [raw]
Subject: RE: [PATCH] PCI: xilinx-nwl: Enable coherenct PCIe traffic using CCI

> [+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)/
>
Thanks Bjorn for corrections.
Here is the CCI spec
https://developer.arm.com/documentation/ddi0470/k/preface

> 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.
Yes will add 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?
>
I will update this property in documentation as optional property.
Rob can confirm if this can be moved to generic.
> > Signed-off-by: Bharat Kumar Gogada <[email protected]>
> > ---
> > 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
> >

2021-01-28 00:03:16

by Bjorn Helgaas

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

On Wed, Jan 27, 2021 at 05:03:12AM +0000, Bharat Kumar Gogada wrote:
> > On Thu, Jan 21, 2021 at 03:29:16PM +0530, Bharat Kumar Gogada wrote:

> Here is the CCI spec
> https://developer.arm.com/documentation/ddi0470/k/preface

I'm sure it was obvious, but please include this in the commit log as
well.

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

2021-01-28 00:08:55

by Rob Herring

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

On Thu, Jan 21, 2021 at 10:28 AM Bjorn Helgaas <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >